On Tue, 1 Nov 2011 23:20:28 -0700 Keith Packard <kei...@keithp.com> wrote:
> Make sure the sequence of operations in all three functions makes > sense: > > 1) The backlight must be off unless the screen is running > 2) The link must be running to turn the eDP panel on/off > 3) 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 <kei...@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)) > - ironlake_edp_backlight_off(intel_dp); > intel_dp_sink_dpms(intel_dp, mode); > intel_dp_link_down(intel_dp); > - ironlake_edp_panel_off(intel_dp); > - if (is_edp(intel_dp) && !is_pch_edp(intel_dp)) > - ironlake_edp_pll_off(encoder); > ironlake_edp_panel_vdd_off(intel_dp, false); > + > + 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? -- Jesse Barnes, Intel Open Source Technology Center
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel