Re: [Intel-gfx] [PATCH v2 9/9] drm/i915/rpm: d3cold Policy
> -Original Message-
> From: Jani Nikula
> Sent: Thursday, June 16, 2022 7:58 PM
> To: Gupta, Anshuman ; intel-
> g...@lists.freedesktop.org; dri-de...@lists.freedesktop.org
> Cc: Vivi, Rodrigo
> Subject: Re: [Intel-gfx] [PATCH v2 9/9] drm/i915/rpm: d3cold Policy
>
> On Thu, 16 Jun 2022, Anshuman Gupta 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
> > Signed-off-by: Anshuman Gupta
> > ---
> > 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) \
> >
Re: [Intel-gfx] [PATCH v2 9/9] drm/i915/rpm: d3cold Policy
On Thu, 16 Jun 2022, Anshuman Gupta 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
> Signed-off-by: Anshuman Gupta
> ---
> 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 {
--
Jani Nikula, Intel Open Source Graphics Center
[Intel-gfx] [PATCH v2 9/9] drm/i915/rpm: d3cold Policy
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
Signed-off-by: Anshuman Gupta
---
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;
+ }
+
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 {
--
2.26.2
