[PATCH v2 0/1] drm/i915/rpm: Enable runtime pm autosuspend by default

[PATCH v2 0/1] drm/i915/rpm: Enable runtime pm autosuspend by default

[Tilak Tangudu]

Enable runtime pm autosuspend by default for all
platforms

Signed-off-by: Tilak Tangudu <tilak.tangudu@intel.com>

Tilak Tangudu (1):
  drm/i915/rpm: Enable runtime pm autosuspend by default

 drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
 1 file changed, 3 insertions(+)

-- 
2.25.1

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

[Tilak Tangudu]

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
-- 
2.25.1

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

[Jani Nikula]

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. Just the changelog will not be enough.

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

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

[Rodrigo Vivi]

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

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

[Rodrigo Vivi]

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

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@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