Re: [Intel-gfx] [PATCH v2 1/1] drm/i915/rpm: Enable runtime pm autosuspend by default

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Nov 15, 2021 at 01:44:57PM +0200, Jani Nikula wrote:
> On Mon, 15 Nov 2021, Tilak Tangudu <tilak.tangudu@intel.com> 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.

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@intel.com>
> > ---
> >  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