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

[Imre Deak <imre.deak@intel.com>]

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.

> > Signed-off-by: Imre Deak <imre.deak@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);
> >
> >         /*
>