Re: [PATCH] drm/i915: Force PSR exit when IRQ_HPD is detected on eDP.

Ville Syrjälä
2015-12-01T19:05:37Z

On Wed, Nov 18, 2015 at 11:19:06AM -0800, Rodrigo Vivi wrote:
> According to VESA spec: "If a Source device receives and IRQ_HPD
> while in a PSR active state, and cannot identify what caused the
> IRQ_HPD to be generated, based on Sink device status registers,
> the Source device can take implementation-specific action.
> One such action can be to exit and then re-enter a PSR active
> state."
>
> Since we aren't checking for any sink status registers and we
> aren't looking for any other implementation-specific action,
> in case we receive any IRQ_HPD and psr is active let's force
> the exit and reschedule it back.
>
> v2: IRQ_HPD means short HPD so handle it correctly
> as Ville pointed out.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Do we have a device that needs this for something? Since it's totally
implementation specific I would expect that something like VBT would
tell us if we need to do something on random short HPDs from the panel.

As far as the implementation goes, can't we just call intel_psr_flush()
so that the frontbuffer tracking keeps in sync with the actual PSR
state?

> ---
> drivers/gpu/drm/i915/intel_dp.c | 3 +++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_psr.c | 33 +++++++++++++++++++++++++++++++++
> 3 files changed, 37 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index bec443a..ca7a798 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5004,6 +5004,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> goto mst_fail;
> }
> } else {
> + if (intel_dig_port->base.type == INTEL_OUTPUT_EDP)
> + intel_psr_irq_hpd(dev);
> +
> if (intel_dp->is_mst) {
> if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
> goto mst_fail;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ab5c147..1d61551 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1401,6 +1401,7 @@ void intel_psr_flush(struct drm_device *dev,
> void intel_psr_init(struct drm_device *dev);
> void intel_psr_single_frame_update(struct drm_device *dev,
> unsigned frontbuffer_bits);
> +void intel_psr_irq_hpd(struct drm_device *dev);
>
> /* intel_runtime_pm.c */
> int intel_power_domains_init(struct drm_i915_private *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index bc5ea2a..465d36b 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -765,6 +765,39 @@ void intel_psr_flush(struct drm_device *dev,
> }
>
> /**
> + * intel_psr_irq_hpd - Let PSR aware of IRQ_HPD
> + * @dev: DRM device
> + *
> + * This function is called when IRQ_HPD is received on eDP.
> + */
> +void intel_psr_irq_hpd(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + int delay_ms = HAS_DDI(dev) ? 100 : 500;
> +
> + mutex_lock(&dev_priv->psr.lock);
> +
> + /*
> + * According to VESA spec "If a Source device receives and IRQ_HPD
> + * while in a PSR active state, and cannot identify what caused the
> + * IRQ_HPD to be generated, based on Sink device status registers,
> + * the Source device can take implementation-specific action.
> + * One such action can be to exit and then re-enter a PSR active
> + * state." Since we aren't checking for any sink status registers
> + * and we aren't looking for any other implementation-specific
> + * action, in case we receive any IRQ_HPD and psr is active let's
> + * force the exit and reschedule it back.
> + */
> + if (dev_priv->psr.active) {
> + intel_psr_exit(dev);
> + schedule_delayed_work(&dev_priv->psr.work,
> + msecs_to_jiffies(delay_ms));
> + }
> +
> + mutex_unlock(&dev_priv->psr.lock);
> +}
> +
> +/**
> * intel_psr_init - Init basic PSR work and mutex.
> * @dev: DRM device
> *
> --
> 2.4.3

--
Ville Syrjälä
Intel OTC