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