On Mon, Aug 11, 2014 at 10:04:50PM +0300, Imre Deak wrote: > Atm we may leave eDP VDD enabled during system suspend after the CRTCs > are disabled through an HPD->DPCD read event. So disable VDD during > suspend at a point when no HPDs can occur.
And vdd itself holds a runtime pm ref so this problem can't occur there. > > Signed-off-by: Imre Deak <[email protected]> > --- > drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++++++++ > drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++++-- > drivers/gpu/drm/i915/intel_drv.h | 6 ++++++ > 3 files changed, 32 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 0653761..1c7979e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -501,6 +501,19 @@ void intel_hpd_cancel_work(struct drm_i915_private > *dev_priv) > cancel_delayed_work_sync(&dev_priv->hotplug_reenable_work); > } > > +static void intel_suspend_encoders(struct drm_i915_private *dev_priv) > +{ > + struct drm_device *dev = dev_priv->dev; > + struct intel_encoder *intel_encoder; > + > + drm_modeset_lock_all(dev); > + for_each_intel_encoder(dev, intel_encoder) { > + if (intel_encoder->suspend) > + intel_encoder->suspend(intel_encoder); > + } > + drm_modeset_unlock_all(dev); > +} > + > static int i915_drm_freeze(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -547,6 +560,8 @@ static int i915_drm_freeze(struct drm_device *dev) > intel_runtime_pm_disable_interrupts(dev); > intel_hpd_cancel_work(dev_priv); > > + intel_suspend_encoders(dev_priv); > + > intel_suspend_gt_powersave(dev); > > intel_modeset_suspend_hw(dev); > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 34e3c47..d7f5d0a 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1291,8 +1291,6 @@ static void edp_panel_vdd_off(struct intel_dp > *intel_dp, bool sync) > if (!is_edp(intel_dp)) > return; > > - WARN(!intel_dp->want_panel_vdd, "eDP VDD not forced on"); > - Should we perhaps keep this warn here and call the _sync() version directly from ->suspend()? I think we already do it that way when tearing down the encoder. > intel_dp->want_panel_vdd = false; > > if (sync) > @@ -4003,6 +4001,16 @@ void intel_dp_encoder_destroy(struct drm_encoder > *encoder) > kfree(intel_dig_port); > } > > +void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder) > +{ > + struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base); > + > + if (!is_edp(intel_dp)) > + return; edp_panel_vdd_off() already has the same check, but if you convert it to use the _sync() directly then the check is needed. > + > + edp_panel_vdd_off(intel_dp, true); > +} > + > static void intel_dp_encoder_reset(struct drm_encoder *encoder) > { > intel_edp_panel_vdd_sanitize(to_intel_encoder(encoder)); > @@ -4722,6 +4730,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, > enum port port) > intel_encoder->disable = intel_disable_dp; > intel_encoder->get_hw_state = intel_dp_get_hw_state; > intel_encoder->get_config = intel_dp_get_config; > + intel_encoder->suspend = intel_dp_encoder_suspend; > if (IS_CHERRYVIEW(dev)) { > intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable; > intel_encoder->pre_enable = chv_pre_enable_dp; > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 1b3d1d7..cefd337 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -153,6 +153,12 @@ struct intel_encoder { > * be set correctly before calling this function. */ > void (*get_config)(struct intel_encoder *, > struct intel_crtc_config *pipe_config); > + /* > + * Called during system suspend after all pending requests for the > + * encoder are flushed (for example for DP AUX transactions) and > + * device interrupts are disabled. > + */ > + void (*suspend)(struct intel_encoder *); > int crtc_mask; > enum hpd_pin hpd_pin; > }; > -- > 1.8.4 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
