Re: [Intel-gfx] [PATCH 1/3] drm/i915/display: Do not schedule DRRS work thread when it is not active

[Souza, Jose]

On Fri, 2022-04-29 at 19:00 +0300, Ville Syrjälä wrote:
> On Thu, Apr 28, 2022 at 02:10:56PM -0700, José Roberto de Souza wrote:
> > Frontbuffer updates were scheduling the execution of DRRS work thread
> > even if DRRS is not active.
> > There was no issues with it because intel_drrs_downclock_work() checks
> > if DRRS is active but there is no reason to keep scheduling this work
> > thread and wasting CPU time.
> > 
> > Cc: Ville Syrjälä 
> > Signed-off-by: José Roberto de Souza 
> > ---
> >  drivers/gpu/drm/i915/display/intel_drrs.c | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c 
> > b/drivers/gpu/drm/i915/display/intel_drrs.c
> > index 166caf293f7bc..04bc296761be0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_drrs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_drrs.c
> > @@ -236,6 +236,11 @@ static void intel_drrs_frontbuffer_update(struct 
> > drm_i915_private *dev_priv,
> > else
> > crtc->drrs.busy_frontbuffer_bits &= ~frontbuffer_bits;
> >  
> > +   if (!intel_drrs_is_active(crtc)) {
> > +   mutex_unlock(&crtc->drrs.mutex);
> > +   continue;
> > +   }
> 
> Should be impossible due to crtc->drrs.frontbuffer_bits==0.

Yep, my bad this patch is not needed.
Can you review the remaining?

I have one more patch to this series that avoids DP link configuration change 
during seamless mode change, when possible.
Planning to send new version with this patch any other changes that you request.

thanks

> 
> > +
> > /* flush/invalidate means busy screen hence upclock */
> > intel_drrs_set_state(crtc, DRRS_REFRESH_RATE_HIGH);
> >  
> > -- 
> > 2.36.0
> 

Re: [Intel-gfx] [PATCH 1/3] drm/i915/display: Do not schedule DRRS work thread when it is not active

[Ville Syrjälä]

On Thu, Apr 28, 2022 at 02:10:56PM -0700, José Roberto de Souza wrote:
> Frontbuffer updates were scheduling the execution of DRRS work thread
> even if DRRS is not active.
> There was no issues with it because intel_drrs_downclock_work() checks
> if DRRS is active but there is no reason to keep scheduling this work
> thread and wasting CPU time.
> 
> Cc: Ville Syrjälä 
> Signed-off-by: José Roberto de Souza 
> ---
>  drivers/gpu/drm/i915/display/intel_drrs.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c 
> b/drivers/gpu/drm/i915/display/intel_drrs.c
> index 166caf293f7bc..04bc296761be0 100644
> --- a/drivers/gpu/drm/i915/display/intel_drrs.c
> +++ b/drivers/gpu/drm/i915/display/intel_drrs.c
> @@ -236,6 +236,11 @@ static void intel_drrs_frontbuffer_update(struct 
> drm_i915_private *dev_priv,
>   else
>   crtc->drrs.busy_frontbuffer_bits &= ~frontbuffer_bits;
>  
> + if (!intel_drrs_is_active(crtc)) {
> + mutex_unlock(&crtc->drrs.mutex);
> + continue;
> + }

Should be impossible due to crtc->drrs.frontbuffer_bits==0.

> +
>   /* flush/invalidate means busy screen hence upclock */
>   intel_drrs_set_state(crtc, DRRS_REFRESH_RATE_HIGH);
>  
> -- 
> 2.36.0

-- 
Ville Syrjälä
Intel

[Intel-gfx] [PATCH 1/3] drm/i915/display: Do not schedule DRRS work thread when it is not active

[José Roberto de Souza]

Frontbuffer updates were scheduling the execution of DRRS work thread
even if DRRS is not active.
There was no issues with it because intel_drrs_downclock_work() checks
if DRRS is active but there is no reason to keep scheduling this work
thread and wasting CPU time.

Cc: Ville Syrjälä 
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/display/intel_drrs.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c 
b/drivers/gpu/drm/i915/display/intel_drrs.c
index 166caf293f7bc..04bc296761be0 100644
--- a/drivers/gpu/drm/i915/display/intel_drrs.c
+++ b/drivers/gpu/drm/i915/display/intel_drrs.c
@@ -236,6 +236,11 @@ static void intel_drrs_frontbuffer_update(struct 
drm_i915_private *dev_priv,
else
crtc->drrs.busy_frontbuffer_bits &= ~frontbuffer_bits;
 
+   if (!intel_drrs_is_active(crtc)) {
+   mutex_unlock(&crtc->drrs.mutex);
+   continue;
+   }
+
/* flush/invalidate means busy screen hence upclock */
intel_drrs_set_state(crtc, DRRS_REFRESH_RATE_HIGH);
 
-- 
2.36.0