[Intel-gfx] [PATCH] drm/i915/dgfx: Enable d3cold at s2idle

[Rodrigo Vivi]

On Tue, Aug 15, 2023 at 03:29:12AM -0400, Gupta, Anshuman wrote:
> 
> 
> > -----Original Message-----
> > From: Vivi, Rodrigo <rodrigo.vivi at intel.com>
> > Sent: Monday, August 14, 2023 9:33 PM
> > To: Gupta, Anshuman <anshuman.gupta at intel.com>
> > Cc: intel-gfx at lists.freedesktop.org; Nilawar, Badal
> > <badal.nilawar at intel.com>; Tauro, Riana <riana.tauro at intel.com>
> > Subject: Re: [PATCH] drm/i915/dgfx: Enable d3cold at s2idle
> > 
> > On Mon, Aug 14, 2023 at 04:34:18PM +0530, Anshuman Gupta wrote:
> > > System wide suspend already has support for lmem save/restore during
> > > suspend therefore enabling d3cold for s2idle and keepng it disable for
> > > runtime PM.(Refer below commit for d3cold runtime PM disable
> > > justification) 'commit 66eb93e71a7a ("drm/i915/dgfx: Keep PCI
> > > autosuspend control 'on' by default on all dGPU")'
> > >
> > > It will reduce the DG2 Card power consumption to ~0 Watt for s2idle
> > > power KPI.
> > >
> > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8755
> > > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta at intel.com>
> > 
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Hi Rodrigo,
> Thanks for review, will this be good candidate for Linux stable tree ?

Yes, I think so.

do we have a good commit to mark for 'Fixes:'?

but we can add the cc: stable even without the Fixes already in
advance anyway.

otherwise we need to wait for this to land in Linus tree and then
send directly again to the stable mailing list.

> Thanks,
> Anshuman Gupta.
> > 
> > > ---
> > >  drivers/gpu/drm/i915/i915_driver.c | 33
> > > ++++++++++++++++--------------
> > >  1 file changed, 18 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_driver.c
> > > b/drivers/gpu/drm/i915/i915_driver.c
> > > index b870c0df081a..ec4d26b3c17c 100644
> > > --- a/drivers/gpu/drm/i915/i915_driver.c
> > > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > > @@ -443,7 +443,6 @@ static int i915_pcode_init(struct drm_i915_private
> > > *i915)  static int i915_driver_hw_probe(struct drm_i915_private
> > > *dev_priv)  {
> > >  	struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> > > -	struct pci_dev *root_pdev;
> > >  	int ret;
> > >
> > >  	if (i915_inject_probe_failure(dev_priv))
> > > @@ -557,15 +556,6 @@ static int i915_driver_hw_probe(struct
> > > drm_i915_private *dev_priv)
> > >
> > >  	intel_bw_init_hw(dev_priv);
> > >
> > > -	/*
> > > -	 * FIXME: Temporary hammer to avoid freezing the machine on our
> > DGFX
> > > -	 * This should be totally removed when we handle the pci states
> > properly
> > > -	 * on runtime PM and on s2idle cases.
> > > -	 */
> > > -	root_pdev = pcie_find_root_port(pdev);
> > > -	if (root_pdev)
> > > -		pci_d3cold_disable(root_pdev);
> > > -
> > >  	return 0;
> > >
> > >  err_opregion:
> > > @@ -591,7 +581,6 @@ static int i915_driver_hw_probe(struct
> > > drm_i915_private *dev_priv)  static void i915_driver_hw_remove(struct
> > > drm_i915_private *dev_priv)  {
> > >  	struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> > > -	struct pci_dev *root_pdev;
> > >
> > >  	i915_perf_fini(dev_priv);
> > >
> > > @@ -599,10 +588,6 @@ static void i915_driver_hw_remove(struct
> > > drm_i915_private *dev_priv)
> > >
> > >  	if (pdev->msi_enabled)
> > >  		pci_disable_msi(pdev);
> > > -
> > > -	root_pdev = pcie_find_root_port(pdev);
> > > -	if (root_pdev)
> > > -		pci_d3cold_enable(root_pdev);
> > >  }
> > >
> > >  /**
> > > @@ -1519,6 +1504,8 @@ static int intel_runtime_suspend(struct device
> > > *kdev)  {
> > >  	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> > >  	struct intel_runtime_pm *rpm = &dev_priv->runtime_pm;
> > > +	struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> > > +	struct pci_dev *root_pdev;
> > >  	struct intel_gt *gt;
> > >  	int ret, i;
> > >
> > > @@ -1570,6 +1557,15 @@ static int intel_runtime_suspend(struct device
> > *kdev)
> > >  		drm_err(&dev_priv->drm,
> > >  			"Unclaimed access detected prior to suspending\n");
> > >
> > > +	/*
> > > +	 * FIXME: Temporary hammer to avoid freezing the machine on our
> > DGFX
> > > +	 * This should be totally removed when we handle the pci states
> > properly
> > > +	 * on runtime PM.
> > > +	 */
> > > +	root_pdev = pcie_find_root_port(pdev);
> > > +	if (root_pdev)
> > > +		pci_d3cold_disable(root_pdev);
> > > +
> > >  	rpm->suspended = true;
> > >
> > >  	/*
> > > @@ -1608,6 +1604,8 @@ static int intel_runtime_resume(struct device
> > > *kdev)  {
> > >  	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> > >  	struct intel_runtime_pm *rpm = &dev_priv->runtime_pm;
> > > +	struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> > > +	struct pci_dev *root_pdev;
> > >  	struct intel_gt *gt;
> > >  	int ret, i;
> > >
> > > @@ -1621,6 +1619,11 @@ static int intel_runtime_resume(struct device
> > > *kdev)
> > >
> > >  	intel_opregion_notify_adapter(dev_priv, PCI_D0);
> > >  	rpm->suspended = false;
> > > +
> > > +	root_pdev = pcie_find_root_port(pdev);
> > > +	if (root_pdev)
> > > +		pci_d3cold_enable(root_pdev);
> > > +
> > >  	if (intel_uncore_unclaimed_mmio(&dev_priv->uncore))
> > >  		drm_dbg(&dev_priv->drm,
> > >  			"Unclaimed access during suspend, bios?\n");
> > > --
> > > 2.25.1
> > >