On Thu, 16 Jun 2022, Anshuman Gupta <anshuman.gu...@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.v...@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gu...@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 {

-- 
Jani Nikula, Intel Open Source Graphics Center

Reply via email to