diff mbox series

drm/i915/display: Exit PSR when doing async flips

Message ID 20211030001801.237548-1-jose.souza@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/display: Exit PSR when doing async flips | expand

Commit Message

Souza, Jose Oct. 30, 2021, 12:18 a.m. UTC
Changing the buffer in the middle of the scanout then entering an
period of flip idleness will cause part of the previous buffer being
diplayed to user when PSR is enabled.

So here disabling and scheduling activation after a few milliseconds
when async flip is enabled in the state.

The async flip check that we had in PSR compute is not executed at
every flip so it was not doing anything useful and is also being
dropped here.

Cc: Karthik B S <karthik.b.s@intel.com>
Cc: Vandita Kulkarni <vandita.kulkarni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Rodrigo Vivi Nov. 1, 2021, 8:36 p.m. UTC | #1
On Fri, Oct 29, 2021 at 05:18:01PM -0700, José Roberto de Souza wrote:
> Changing the buffer in the middle of the scanout then entering an
> period of flip idleness will cause part of the previous buffer being
> diplayed to user when PSR is enabled.
> 
> So here disabling and scheduling activation after a few milliseconds
> when async flip is enabled in the state.
> 
> The async flip check that we had in PSR compute is not executed at
> every flip so it was not doing anything useful and is also being
> dropped here.
> 
> Cc: Karthik B S <karthik.b.s@intel.com>
> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 9d589d471e335..d1301e2729553 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -731,12 +731,6 @@ static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
>  		return false;
>  	}
>  
> -	if (crtc_state->uapi.async_flip) {
> -		drm_dbg_kms(&dev_priv->drm,
> -			    "PSR2 sel fetch not enabled, async flip enabled\n");
> -		return false;
> -	}
> -
>  	/* Wa_14010254185 Wa_14010103792 */
>  	if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
>  		drm_dbg_kms(&dev_priv->drm,
> @@ -1780,6 +1774,11 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
>  		if (psr->enabled && needs_to_disable)
>  			intel_psr_disable_locked(intel_dp);
>  
> +		if (psr->enabled && crtc_state->uapi.async_flip) {
> +			intel_psr_exit(intel_dp);
> +			schedule_work(&intel_dp->psr.work);

wouldn't it be better(safer?) to reschedule it back in a later stage?

> +		}
> +
>  		mutex_unlock(&psr->lock);
>  	}
>  }
> -- 
> 2.33.1
>
Souza, Jose Nov. 1, 2021, 8:56 p.m. UTC | #2
On Mon, 2021-11-01 at 16:36 -0400, Rodrigo Vivi wrote:
> On Fri, Oct 29, 2021 at 05:18:01PM -0700, José Roberto de Souza wrote:
> > Changing the buffer in the middle of the scanout then entering an
> > period of flip idleness will cause part of the previous buffer being
> > diplayed to user when PSR is enabled.
> > 
> > So here disabling and scheduling activation after a few milliseconds
> > when async flip is enabled in the state.
> > 
> > The async flip check that we had in PSR compute is not executed at
> > every flip so it was not doing anything useful and is also being
> > dropped here.
> > 
> > Cc: Karthik B S <karthik.b.s@intel.com>
> > Cc: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 9d589d471e335..d1301e2729553 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -731,12 +731,6 @@ static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
> >  		return false;
> >  	}
> >  
> > -	if (crtc_state->uapi.async_flip) {
> > -		drm_dbg_kms(&dev_priv->drm,
> > -			    "PSR2 sel fetch not enabled, async flip enabled\n");
> > -		return false;
> > -	}
> > -
> >  	/* Wa_14010254185 Wa_14010103792 */
> >  	if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
> >  		drm_dbg_kms(&dev_priv->drm,
> > @@ -1780,6 +1774,11 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
> >  		if (psr->enabled && needs_to_disable)
> >  			intel_psr_disable_locked(intel_dp);
> >  
> > +		if (psr->enabled && crtc_state->uapi.async_flip) {
> > +			intel_psr_exit(intel_dp);
> > +			schedule_work(&intel_dp->psr.work);
> 
> wouldn't it be better(safer?) to reschedule it back in a later stage?

good idea, do that in intel_psr_post_plane_update().

thanks

> 
> > +		}
> > +
> >  		mutex_unlock(&psr->lock);
> >  	}
> >  }
> > -- 
> > 2.33.1
> >
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 9d589d471e335..d1301e2729553 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -731,12 +731,6 @@  static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
 		return false;
 	}
 
-	if (crtc_state->uapi.async_flip) {
-		drm_dbg_kms(&dev_priv->drm,
-			    "PSR2 sel fetch not enabled, async flip enabled\n");
-		return false;
-	}
-
 	/* Wa_14010254185 Wa_14010103792 */
 	if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
 		drm_dbg_kms(&dev_priv->drm,
@@ -1780,6 +1774,11 @@  void intel_psr_pre_plane_update(struct intel_atomic_state *state,
 		if (psr->enabled && needs_to_disable)
 			intel_psr_disable_locked(intel_dp);
 
+		if (psr->enabled && crtc_state->uapi.async_flip) {
+			intel_psr_exit(intel_dp);
+			schedule_work(&intel_dp->psr.work);
+		}
+
 		mutex_unlock(&psr->lock);
 	}
 }