Re: [Intel-gfx] [PATCH 7/7] drm/i915/rpm: Enable D3Cold VRAM SR Support

From: "Gupta, Anshuman" <anshuman.gupta@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: "Nikula, Jani" <jani.nikula@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Wilson, Chris P" <chris.p.wilson@intel.com>,
	"Vivi, Rodrigo" <rodrigo.vivi@intel.com>
Subject: Re: [Intel-gfx] [PATCH 7/7] drm/i915/rpm: Enable D3Cold VRAM SR Support
Date: Wed, 18 May 2022 15:19:27 +0000	[thread overview]
Message-ID: <ee7969aef0594aa7bb9d76e8e657db6e@intel.com> (raw)
In-Reply-To: <YoT/iw8w06n+ITP4@intel.com>


> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Wednesday, May 18, 2022 7:46 PM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>;
> Wilson, Chris P <chris.p.wilson@intel.com>; Vivi, Rodrigo
> <rodrigo.vivi@intel.com>
> Subject: Re: [Intel-gfx] [PATCH 7/7] drm/i915/rpm: Enable D3Cold VRAM SR
> Support
> 
> On Wed, May 18, 2022 at 06:37:16PM +0530, Anshuman Gupta wrote:
> > Intel Client DGFX card supports D3Cold with two option.
> > D3Cold-off zero watt, D3Cold-VRAM Self Refresh.
> >
> > i915 requires to evict the lmem objects to smem in order to support
> > D3Cold-Off, which increases i915 the suspend/resume latency. Enabling
> > VRAM Self Refresh feature optimize the latency with additional power
> > cost which required to retain the lmem.
> >
> > Adding intel_runtime_idle (runtime_idle callback) to enable VRAM_SR,
> > it will be used for policy to choose between D3Cold-off vs
> > D3Cold-VRAM_SR.
> >
> > Since we have introduced i915 runtime_idle callback.
> > It need to be warranted that Runtime PM Core invokes runtime_idle
> > callback when runtime usages count becomes zero. That requires to use
> > pm_runtime_put instead of pm_runtime_put_autosuspend.
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Chris Wilson <chris.p.wilson@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_driver.c      | 26 +++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_runtime_pm.c |  3 +--
> >  2 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_driver.c
> > b/drivers/gpu/drm/i915/i915_driver.c
> > index 5a9d5529fc90..bbb11c632799 100644
> > --- a/drivers/gpu/drm/i915/i915_driver.c
> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > @@ -1541,6 +1541,31 @@ static int i915_pm_restore(struct device *kdev)
> >  	return i915_pm_resume(kdev);
> >  }
> >
> > +static int intel_runtime_idle(struct device *kdev) {
> > +	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> > +	int ret = 1;
> > +
> > +	if (!HAS_LMEM_SR(dev_priv)) {
> > +		/*TODO: Prepare for D3Cold-Off */
> > +		goto out;
> > +	}
> > +
> > +	disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
> > +
> > +	ret = intel_pm_vram_sr(dev_priv, true)
> 
> I don't get why this idle callback is here. Why aren't you just calling that directly
> from the suspend handler?
We will be having a D3Cold policy in future to decide between D3Cold-VRAM_SR and
D3Cold-Off based upon lmem usages also  we need to evict the lmem content if 
D3cold-off option is used. It will be better to keep it in separate runtime idle call
back to prepare the actual suspend.
Thanks,
Anshuman Gupta.
> 
> > +	if (!ret)
> > +		drm_dbg(&dev_priv->drm, "VRAM Self Refresh enabled\n");
> > +
> > +	enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
> > +
> > +out:
> > +	pm_runtime_mark_last_busy(kdev);
> > +	pm_runtime_autosuspend(kdev);
> > +
> > +	return ret;
> > +}
> > +
> >  static int intel_runtime_suspend(struct device *kdev)  {
> >  	struct drm_i915_private *dev_priv = kdev_to_i915(kdev); @@ -1726,6
> > +1751,7 @@ const struct dev_pm_ops i915_pm_ops = {
> >  	.restore = i915_pm_restore,
> >
> >  	/* S0ix (via runtime suspend) event handlers */
> > +	.runtime_idle = intel_runtime_idle,
> >  	.runtime_suspend = intel_runtime_suspend,
> >  	.runtime_resume = intel_runtime_resume,  }; diff --git
> > a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 6ed5786bcd29..4dade7e8a795 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -492,8 +492,7 @@ static void __intel_runtime_pm_put(struct
> > intel_runtime_pm *rpm,
> >
> >  	intel_runtime_pm_release(rpm, wakelock);
> >
> > -	pm_runtime_mark_last_busy(kdev);
> > -	pm_runtime_put_autosuspend(kdev);
> > +	pm_runtime_put(kdev);
> >  }
> >
> >  /**
> > --
> > 2.26.2
> 
> --
> Ville Syrjälä
> Intel