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@xxxxxxxxx> > > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx> > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx