[Intel-gfx] [PATCH v2 4/5] drm/i915/psr: Check if sink PSR capability changed

[Matt Roper]

On Mon, Nov 25, 2019 at 04:53:59PM -0800, José Roberto de Souza wrote:
> eDP specification states that sink can have its PSR capability
> changed, I have never found any panel doing that but lets add that
> for completeness.
> For now it is not reading back the PSR capabilities and if possible
> re-enabling PSR, this will be added if a panel is found using this
> feature.

I'm not super familiar with PSR details, but is it required for us to
disable PSR in this case?  From a quick skim of the spec it sounds like
the sink is required to keep operating with the same capabilities until
the source clears the CAPS_CHANGE bit (which we never do in the patch
below).  What happens if we just ignore the sink's notification and
never ack it by writing a 1 to clear the bit?

I guess disabling is still probably the safest thing to do if we're not
sure and don't have any real panels to test it out with.  Should we
still clean by clearing the bit even though we disabled PSR?

Otherwise,

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>


> 
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index a757b6445f21..ce76e1c6a0b9 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1437,6 +1437,26 @@ static void psr_alpm_check(struct intel_dp *intel_dp)
>  	}
>  }
>  
> +static void psr_capability_changed_check(struct intel_dp *intel_dp)
> +{
> +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +	struct i915_psr *psr = &dev_priv->psr;
> +	u8 val;
> +	int r;
> +
> +	r = drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_ESI, &val);
> +	if (r != 1) {
> +		DRM_ERROR("Error reading DP_PSR_ESI\n");
> +		return;
> +	}
> +
> +	if (val & DP_PSR_CAPS_CHANGE) {
> +		intel_psr_disable_locked(intel_dp);
> +		psr->sink_not_reliable = true;
> +		DRM_DEBUG_KMS("Sink PSR capability changed, disabling PSR\n");
> +	}
> +}
> +
>  void intel_psr_short_pulse(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> @@ -1480,6 +1500,7 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp)
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, error_status);
>  
>  	psr_alpm_check(intel_dp);
> +	psr_capability_changed_check(intel_dp);
>  
>  exit:
>  	mutex_unlock(&psr->lock);
> -- 
> 2.24.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795