On Tue, 1 Nov 2011 23:20:28 -0700 Keith Packard keithp@keithp.com wrote:
Make sure the sequence of operations in all three functions makes sense:
- The backlight must be off unless the screen is running
- The link must be running to turn the eDP panel on/off
- The CPU eDP PLL must be running until everything is off
A few comments on this one (also, is it strictly required to fix your bug)?
Signed-off-by: Keith Packard keithp@keithp.com
drivers/gpu/drm/i915/intel_dp.c | 22 +++++++++++++--------- 1 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d6c6608..6be6a04 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1234,17 +1234,18 @@ static void intel_dp_prepare(struct drm_encoder *encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
ironlake_edp_backlight_off(intel_dp);
ironlake_edp_panel_off(intel_dp);
/* Wake up the sink first */ ironlake_edp_panel_vdd_on(intel_dp); intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_link_down(intel_dp); ironlake_edp_panel_vdd_off(intel_dp, false);
/* Make sure the panel is off before trying to
- change the mode
*/
- ironlake_edp_backlight_off(intel_dp);
- intel_dp_link_down(intel_dp);
- ironlake_edp_panel_off(intel_dp);
}
Ok so you're making sure the panel has power to down the link, I think that's fine.
static void intel_dp_commit(struct drm_encoder *encoder) @@ -1276,16 +1277,20 @@ intel_dp_dpms(struct drm_encoder *encoder, int mode) uint32_t dp_reg = I915_READ(intel_dp->output_reg);
if (mode != DRM_MODE_DPMS_ON) {
ironlake_edp_backlight_off(intel_dp);
ironlake_edp_panel_off(intel_dp);
- ironlake_edp_panel_vdd_on(intel_dp);
if (is_edp(intel_dp))
intel_dp_sink_dpms(intel_dp, mode); intel_dp_link_down(intel_dp);ironlake_edp_backlight_off(intel_dp);
ironlake_edp_panel_off(intel_dp);
if (is_edp(intel_dp) && !is_pch_edp(intel_dp))
ironlake_edp_panel_vdd_off(intel_dp, false);ironlake_edp_pll_off(encoder);
if (is_cpu_edp(intel_dp))
ironlake_edp_pll_off(encoder);
But here it looks like you're shutting it off, then downing the link? Should this be reordered?