On Mon, Nov 15, 2021 at 09:58:56AM -0500, Rodrigo Vivi wrote:
> On Mon, Nov 15, 2021 at 01:44:57PM +0200, Jani Nikula wrote:
> > On Mon, 15 Nov 2021, Tilak Tangudu <tilak.tangudu@xxxxxxxxx> wrote:
> >
> > The actual commit message with explanations why it will work now and
> > didn't work before go here.
>
> The truth is that:
>
> 1. We don't have a good track of *all* the issues with the past attempts.
> 2. But apparently in every attempt we hit some other bug, like the latest
> one with GuC PM...
> 3. All the attempts we also tried to do multiple changes at the same time,
> including reducing the autosuspend delay.
>
> > Just the changelog will not be enough.
>
> But yes, you are absolutely right here. changelogs are not enough and
> we need some explanation in the commit itself.
>
> I'd suggest something like:
>
> """
> Let's enable runtime pm autosuspend by default everywhere. So we can
> allow D3hot and bigger power savings on idle scenarios.
>
> But at this time let's not touch the autosuspend_delay time,
> what caused some regression on our previous attempt.
>
> Also, the latest identified issue on GuC PM has been fixed by
> 1a52faed3131 ("drm/i915/guc: Take GT PM ref when deregistering context")
>
> """
>
> While writing that I remembered that we cannot do this just yet.
> We need to do a further work and block the d3cold on discrete.
> D3cold is not ready yet and enabling this autosuspend by default
> will blow up some discrete experimental usages of upstream i915
> out there. Let's protect that first.
we now have the d3cold in place. Please proceed with the spin on the
patch with the commit message improvement.
Thanks,
Rodrigo.
>
> Thanks,
> Rodrigo.
>
> >
> > BR,
> > Jani.
> >
> >
> > > v1: Enable runtime pm autosuspend by default for Gen12
> > > and later versions.
> > >
> > > v2: Enable runtime pm autosuspend by default for all
> > > platforms(Syrjala Ville)
> > >
> > > Signed-off-by: Tilak Tangudu <tilak.tangudu@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index eaf7688f517d..f26ed1427fdc 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -600,6 +600,9 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
> > > pm_runtime_use_autosuspend(kdev);
> > > }
> > >
> > > + /* Enable by default */
> > > + pm_runtime_allow(kdev);
> > > +
> > > /*
> > > * The core calls the driver load handler with an RPM reference held.
> > > * We drop that here and will reacquire it during unloading in
> >
> > --
> > Jani Nikula, Intel Open Source Graphics Center
