| Message ID | 20220616120106.24353-10-anshuman.gupta@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DG2 VRAM_SR Support | expand |
On Thu, 16 Jun 2022, Anshuman Gupta <anshuman.gupta@intel.com> wrote: > Add d3cold_sr_lmem_threshold modparam to choose between > d3cold-off zero watt and d3cold-VRAM Self Refresh. > i915 requires to evict the lmem objects to smem in order to > support d3cold-Off. > > If gfx root port is not capable of sending PME from d3cold > then i915 don't need to program d3cold-off/d3cold-vram_sr > sequence. > > FIXME: Eviction of lmem objects in case of D3Cold off is wip. > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> > --- > drivers/gpu/drm/i915/i915_driver.c | 27 ++++++++++++++++++++++++--- > drivers/gpu/drm/i915/i915_params.c | 4 ++++ > drivers/gpu/drm/i915/i915_params.h | 3 ++- > 3 files changed, 30 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c > index fcff5f3fe05e..aef4b17efdbe 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -1560,15 +1560,36 @@ static int i915_pm_restore(struct device *kdev) > static int intel_runtime_idle(struct device *kdev) > { > struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > + struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); > + u64 lmem_total = to_gt(dev_priv)->lmem->total; > + u64 lmem_avail = to_gt(dev_priv)->lmem->avail; > + u64 lmem_used = lmem_total - lmem_avail; > + struct pci_dev *root_pdev; > int ret = 1; > > - if (!HAS_LMEM_SR(dev_priv)) { > - /*TODO: Prepare for D3Cold-Off */ > + root_pdev = pcie_find_root_port(pdev); > + if (!root_pdev) > + goto out; > + > + if (!pci_pme_capable(root_pdev, PCI_D3cold)) > goto out; > - } > > disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > > + if (lmem_used < dev_priv->params.d3cold_sr_lmem_threshold * 1024 * 1024) { > + drm_dbg(&dev_priv->drm, "Prepare for D3Cold off\n"); > + pci_d3cold_enable(root_pdev); > + /* FIXME: Eviction of lmem objects and guc reset is wip */ > + intel_pm_vram_sr(dev_priv, false); > + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > + goto out; > + } else if (!HAS_LMEM_SR(dev_priv)) { > + /* Disable D3Cold to reduce the eviction latency */ > + pci_d3cold_disable(root_pdev); > + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > + goto out; > + } This is *way* too low level code for such high level function. This needs to be abstracted better. > + > ret = intel_pm_vram_sr(dev_priv, true); > if (!ret) > drm_dbg(&dev_priv->drm, "VRAM Self Refresh enabled\n"); > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > index 701fbc98afa0..6c6b3c372d4d 100644 > --- a/drivers/gpu/drm/i915/i915_params.c > +++ b/drivers/gpu/drm/i915/i915_params.c > @@ -197,6 +197,10 @@ i915_param_named(enable_gvt, bool, 0400, > "Enable support for Intel GVT-g graphics virtualization host support(default:false)"); > #endif > > +i915_param_named_unsafe(d3cold_sr_lmem_threshold, int, 0400, > + "Enable Vidoe RAM Self refresh when size of lmem is greater to this threshold. " > + "It helps to optimize the suspend/resume latecy. (default: 300mb)"); > + > #if CONFIG_DRM_I915_REQUEST_TIMEOUT > i915_param_named_unsafe(request_timeout_ms, uint, 0600, > "Default request/fence/batch buffer expiration timeout."); > diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h > index b5e7ea45d191..28f20ebaf41f 100644 > --- a/drivers/gpu/drm/i915/i915_params.h > +++ b/drivers/gpu/drm/i915/i915_params.h > @@ -83,7 +83,8 @@ struct drm_printer; > param(bool, verbose_state_checks, true, 0) \ > param(bool, nuclear_pageflip, false, 0400) \ > param(bool, enable_dp_mst, true, 0600) \ > - param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0) > + param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0) \ > + param(int, d3cold_sr_lmem_threshold, 300, 0600) \ What's the point of the parameter? Also, please read the comment /* leave bools at the end to not create holes */ above. BR, Jani. > > #define MEMBER(T, member, ...) T member; > struct i915_params {
> -----Original Message----- > From: Jani Nikula <jani.nikula@linux.intel.com> > Sent: Thursday, June 16, 2022 7:58 PM > To: Gupta, Anshuman <anshuman.gupta@intel.com>; intel- > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com> > Subject: Re: [Intel-gfx] [PATCH v2 9/9] drm/i915/rpm: d3cold Policy > > On Thu, 16 Jun 2022, Anshuman Gupta <anshuman.gupta@intel.com> wrote: > > Add d3cold_sr_lmem_threshold modparam to choose between d3cold-off > > zero watt and d3cold-VRAM Self Refresh. > > i915 requires to evict the lmem objects to smem in order to support > > d3cold-Off. > > > > If gfx root port is not capable of sending PME from d3cold then i915 > > don't need to program d3cold-off/d3cold-vram_sr sequence. > > > > FIXME: Eviction of lmem objects in case of D3Cold off is wip. > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> > > --- > > drivers/gpu/drm/i915/i915_driver.c | 27 ++++++++++++++++++++++++--- > > drivers/gpu/drm/i915/i915_params.c | 4 ++++ > > drivers/gpu/drm/i915/i915_params.h | 3 ++- > > 3 files changed, 30 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_driver.c > > b/drivers/gpu/drm/i915/i915_driver.c > > index fcff5f3fe05e..aef4b17efdbe 100644 > > --- a/drivers/gpu/drm/i915/i915_driver.c > > +++ b/drivers/gpu/drm/i915/i915_driver.c > > @@ -1560,15 +1560,36 @@ static int i915_pm_restore(struct device > > *kdev) static int intel_runtime_idle(struct device *kdev) { > > struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > > + struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); > > + u64 lmem_total = to_gt(dev_priv)->lmem->total; > > + u64 lmem_avail = to_gt(dev_priv)->lmem->avail; > > + u64 lmem_used = lmem_total - lmem_avail; > > + struct pci_dev *root_pdev; > > int ret = 1; > > > > - if (!HAS_LMEM_SR(dev_priv)) { > > - /*TODO: Prepare for D3Cold-Off */ > > + root_pdev = pcie_find_root_port(pdev); > > + if (!root_pdev) > > + goto out; > > + > > + if (!pci_pme_capable(root_pdev, PCI_D3cold)) > > goto out; > > - } > > > > disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > > > > + if (lmem_used < dev_priv->params.d3cold_sr_lmem_threshold * 1024 * > 1024) { > > + drm_dbg(&dev_priv->drm, "Prepare for D3Cold off\n"); > > + pci_d3cold_enable(root_pdev); > > + /* FIXME: Eviction of lmem objects and guc reset is wip */ > > + intel_pm_vram_sr(dev_priv, false); > > + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > > + goto out; > > + } else if (!HAS_LMEM_SR(dev_priv)) { > > + /* Disable D3Cold to reduce the eviction latency */ > > + pci_d3cold_disable(root_pdev); > > + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > > + goto out; > > + } > > This is *way* too low level code for such high level function. This needs to be > abstracted better. > > > + > > ret = intel_pm_vram_sr(dev_priv, true); > > if (!ret) > > drm_dbg(&dev_priv->drm, "VRAM Self Refresh enabled\n"); diff > --git > > a/drivers/gpu/drm/i915/i915_params.c > > b/drivers/gpu/drm/i915/i915_params.c > > index 701fbc98afa0..6c6b3c372d4d 100644 > > --- a/drivers/gpu/drm/i915/i915_params.c > > +++ b/drivers/gpu/drm/i915/i915_params.c > > @@ -197,6 +197,10 @@ i915_param_named(enable_gvt, bool, 0400, > > "Enable support for Intel GVT-g graphics virtualization host > > support(default:false)"); #endif > > > > +i915_param_named_unsafe(d3cold_sr_lmem_threshold, int, 0400, > > + "Enable Vidoe RAM Self refresh when size of lmem is greater to this > threshold. " > > + "It helps to optimize the suspend/resume latecy. (default: 300mb)"); > > + > > #if CONFIG_DRM_I915_REQUEST_TIMEOUT > > i915_param_named_unsafe(request_timeout_ms, uint, 0600, > > "Default request/fence/batch buffer expiration > timeout."); diff > > --git a/drivers/gpu/drm/i915/i915_params.h > > b/drivers/gpu/drm/i915/i915_params.h > > index b5e7ea45d191..28f20ebaf41f 100644 > > --- a/drivers/gpu/drm/i915/i915_params.h > > +++ b/drivers/gpu/drm/i915/i915_params.h > > @@ -83,7 +83,8 @@ struct drm_printer; > > param(bool, verbose_state_checks, true, 0) \ > > param(bool, nuclear_pageflip, false, 0400) \ > > param(bool, enable_dp_mst, true, 0600) \ > > - param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? > 0400 : 0) > > + param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? > 0400 : 0) \ > > + param(int, d3cold_sr_lmem_threshold, 300, 0600) \ > > What's the point of the parameter? We want a configurable option to choose an optimum lmem usages threshold on which, i915 can choose lmem self-refresh. This threshold value would require some profiling as well. That is the reason this threshold kept as module param. > > Also, please read the comment /* leave bools at the end to not create holes */ > above. Thanks for review comment , I will re order the module param. Regards , Anshuman Gupta. > > > BR, > Jani. > > > > > > #define MEMBER(T, member, ...) T member; struct i915_params { > > -- > Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index fcff5f3fe05e..aef4b17efdbe 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -1560,15 +1560,36 @@ static int i915_pm_restore(struct device *kdev) static int intel_runtime_idle(struct device *kdev) { struct drm_i915_private *dev_priv = kdev_to_i915(kdev); + struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); + u64 lmem_total = to_gt(dev_priv)->lmem->total; + u64 lmem_avail = to_gt(dev_priv)->lmem->avail; + u64 lmem_used = lmem_total - lmem_avail; + struct pci_dev *root_pdev; int ret = 1; - if (!HAS_LMEM_SR(dev_priv)) { - /*TODO: Prepare for D3Cold-Off */ + root_pdev = pcie_find_root_port(pdev); + if (!root_pdev) + goto out; + + if (!pci_pme_capable(root_pdev, PCI_D3cold)) goto out; - } disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); + if (lmem_used < dev_priv->params.d3cold_sr_lmem_threshold * 1024 * 1024) { + drm_dbg(&dev_priv->drm, "Prepare for D3Cold off\n"); + pci_d3cold_enable(root_pdev); + /* FIXME: Eviction of lmem objects and guc reset is wip */ + intel_pm_vram_sr(dev_priv, false); + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); + goto out; + } else if (!HAS_LMEM_SR(dev_priv)) { + /* Disable D3Cold to reduce the eviction latency */ + pci_d3cold_disable(root_pdev); + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); + goto out; + } + ret = intel_pm_vram_sr(dev_priv, true); if (!ret) drm_dbg(&dev_priv->drm, "VRAM Self Refresh enabled\n"); diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 701fbc98afa0..6c6b3c372d4d 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -197,6 +197,10 @@ i915_param_named(enable_gvt, bool, 0400, "Enable support for Intel GVT-g graphics virtualization host support(default:false)"); #endif +i915_param_named_unsafe(d3cold_sr_lmem_threshold, int, 0400, + "Enable Vidoe RAM Self refresh when size of lmem is greater to this threshold. " + "It helps to optimize the suspend/resume latecy. (default: 300mb)"); + #if CONFIG_DRM_I915_REQUEST_TIMEOUT i915_param_named_unsafe(request_timeout_ms, uint, 0600, "Default request/fence/batch buffer expiration timeout."); diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index b5e7ea45d191..28f20ebaf41f 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -83,7 +83,8 @@ struct drm_printer; param(bool, verbose_state_checks, true, 0) \ param(bool, nuclear_pageflip, false, 0400) \ param(bool, enable_dp_mst, true, 0600) \ - param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0) + param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 : 0) \ + param(int, d3cold_sr_lmem_threshold, 300, 0600) \ #define MEMBER(T, member, ...) T member; struct i915_params {
Add d3cold_sr_lmem_threshold modparam to choose between d3cold-off zero watt and d3cold-VRAM Self Refresh. i915 requires to evict the lmem objects to smem in order to support d3cold-Off. If gfx root port is not capable of sending PME from d3cold then i915 don't need to program d3cold-off/d3cold-vram_sr sequence. FIXME: Eviction of lmem objects in case of D3Cold off is wip. Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> --- drivers/gpu/drm/i915/i915_driver.c | 27 ++++++++++++++++++++++++--- drivers/gpu/drm/i915/i915_params.c | 4 ++++ drivers/gpu/drm/i915/i915_params.h | 3 ++- 3 files changed, 30 insertions(+), 4 deletions(-)