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 <imre.deak@xxxxxxxxx>
> ---
> 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
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
