diff mbox series

[2/2] drm/i915/display: Do not re-enable PSR after it was marked as not reliable

Message ID 20220308154136.406578-2-jose.souza@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] Revert "drm/i915/edp: Ignore short pulse when panel powered off" | expand

Commit Message

Souza, Jose March 8, 2022, 3:41 p.m. UTC
If a error happens and sink_not_reliable is set, PSR should be disabled
for good but that is not happening.
It would be disabled by the function handling the PSR error but then
on the next fastset it would be enabled again in
_intel_psr_post_plane_update().
It would only be disabled for good in the next modeset where has_psr
will be set false.

Fixes: 9ce5884e5139 ("drm/i915/display: Only keep PSR enabled if there is active planes")
Reported-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
Reported-by: Charlton Lin <charlton.lin@intel.com>
Cc: Jouni Högander <jouni.hogander@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Hogander, Jouni March 9, 2022, 1:51 p.m. UTC | #1
Hello Jose,

See my question/comment below.

On Tue, 2022-03-08 at 07:41 -0800, José Roberto de Souza wrote:
> If a error happens and sink_not_reliable is set, PSR should be
> disabled
> for good but that is not happening.
> It would be disabled by the function handling the PSR error but then
> on the next fastset it would be enabled again in
> _intel_psr_post_plane_update().
> It would only be disabled for good in the next modeset where has_psr
> will be set false.

How about invalidate/flush? If you get error between
intel_psr_invalidate and intel_psr_flush psr is activated
even sink_not_reliable is true?

> 
> Fixes: 9ce5884e5139 ("drm/i915/display: Only keep PSR enabled if
> there is active planes")
> Reported-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
> Reported-by: Charlton Lin <charlton.lin@intel.com>
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index bbd581ed08159..cd05e5fdc8ca9 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1853,6 +1853,9 @@ static void _intel_psr_post_plane_update(const
> struct intel_atomic_state *state,
>  
>  		mutex_lock(&psr->lock);
>  
> +		if (psr->sink_not_reliable)
> +			continue;
> +
>  		drm_WARN_ON(&dev_priv->drm, psr->enabled &&
> !crtc_state->active_planes);
>  
>  		/* Only enable if there is active planes */

BR,

Jouni Högander
Souza, Jose March 9, 2022, 5:58 p.m. UTC | #2
On Wed, 2022-03-09 at 13:51 +0000, Hogander, Jouni wrote:
> Hello Jose,
> 
> See my question/comment below.
> 
> On Tue, 2022-03-08 at 07:41 -0800, José Roberto de Souza wrote:
> > If a error happens and sink_not_reliable is set, PSR should be
> > disabled
> > for good but that is not happening.
> > It would be disabled by the function handling the PSR error but then
> > on the next fastset it would be enabled again in
> > _intel_psr_post_plane_update().
> > It would only be disabled for good in the next modeset where has_psr
> > will be set false.
> 
> How about invalidate/flush? If you get error between
> intel_psr_invalidate and intel_psr_flush psr is activated
> even sink_not_reliable is true?

enabled != activated.

flush and invalidate functions checks if PSR is enabled, if not it returns and do a thing.

> 
> > 
> > Fixes: 9ce5884e5139 ("drm/i915/display: Only keep PSR enabled if
> > there is active planes")
> > Reported-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
> > Reported-by: Charlton Lin <charlton.lin@intel.com>
> > Cc: Jouni Högander <jouni.hogander@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index bbd581ed08159..cd05e5fdc8ca9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1853,6 +1853,9 @@ static void _intel_psr_post_plane_update(const
> > struct intel_atomic_state *state,
> > 
> >               mutex_lock(&psr->lock);
> > 
> > +             if (psr->sink_not_reliable)
> > +                     continue;
> > +
> >               drm_WARN_ON(&dev_priv->drm, psr->enabled &&
> > !crtc_state->active_planes);
> > 
> >               /* Only enable if there is active planes */
> 
> BR,
> 
> Jouni Högander
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index bbd581ed08159..cd05e5fdc8ca9 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1853,6 +1853,9 @@  static void _intel_psr_post_plane_update(const struct intel_atomic_state *state,
 
 		mutex_lock(&psr->lock);
 
+		if (psr->sink_not_reliable)
+			continue;
+
 		drm_WARN_ON(&dev_priv->drm, psr->enabled && !crtc_state->active_planes);
 
 		/* Only enable if there is active planes */