[Intel-gfx] [PATCH 4/4] drm/i915: Prevent needless toggling of DC states during modesets

[Hogander, Jouni]

On Thu, 2023-06-22 at 15:05 +0300, Imre Deak wrote:
> On Thu, Jun 22, 2023 at 12:38:17PM +0300, Hogander, Jouni wrote:
> > On Fri, 2023-06-16 at 21:51 +0300, Imre Deak wrote:
> > > DC states are disabled / re-enabled around each modeset, which
> > > may
> > > lead to a needless off->on->off toggling of the DC_off power
> > > well.
> > > This has some overhead as toggling DC states involves running a
> > > DMC
> > > firmware handler and also running a periodic firmware handler
> > > while
> > > DC states are enabled. The limit of when DC states have a benefit
> > > is
> > > at 30 FPS (using DC3co) and below 30 FPS (using DC5/6), where the
> > > firmware can actually disable clocks / power off power wells.
> > > Accordingly delay powering off the DC_off powerwell (which
> > > re-enables DC states) by 17 ms at the end of a modeset to avoid
> > > the
> > > above overhead at or above 60 FPS.
> > 
> > I'm wondering in which case this toggling would happen and how the
> > patch helps it.
> 
> It happens around each modeset in cases DC states are allowed by the
> enabled outputs. For instance a single eDP output enabled and user
> space
> doing back-to-back page flipping (issuing a new flip once the
> previous
> one completes). The reason to avoid the
> disable DC state -> flip -> re-enable DC state
> toggling of DC states around each flip in this case is to avoid the
> overhead of the DMC FW handlers running I described above (as the
> firmware won't actually get to enter DC states anyway).
> 
> > We have PSR idle frames configured > 6. I.e. Only after 6 frames
> > without changes our HW is supposed to enter DC5/6. This is purely
> > with
> > PSR glasses on. Do we have some other (than PSR) use-case this is
> > targeting where DC5/DC6 would be possible while display is on?
> 
> DC3co would allow the DMC FW to enter DC states (actually disabling
> clocks/power wells) after each frame at 30 FPS.

Thank you for the clarification. For the whole set:

Reviewed-by: Jouni Högander <jouni.hogander at intel.com>

> 
> > > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index dd52c689267f4..de77d130d6f9f 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -7148,7 +7148,11 @@ static void
> > > intel_atomic_commit_tail(struct
> > > intel_atomic_state *state)
> > >                  */
> > >                
> > > intel_uncore_arm_unclaimed_mmio_detection(&dev_priv-
> > > > uncore);
> > >         }
> > > -       intel_display_power_put(dev_priv, POWER_DOMAIN_DC_OFF,
> > > wakeref);
> > > +       /*
> > > +        * Delay re-enabling DC states by 17 ms to avoid the off-
> > > >on-
> > > > off
> > > +        * toggling overhead at and above 60 FPS.
> > > +        */
> > > +       intel_display_power_put_async_delay(dev_priv,
> > > POWER_DOMAIN_DC_OFF, wakeref, 17);
> > >         intel_runtime_pm_put(&dev_priv->runtime_pm, state-
> > > >wakeref);
> > > 
> > >         /*
> >