[Intel-gfx] [PATCH v2 1/2] drm/i915: Impletments dynamic WOPCM partitioning.

Chris Wilson chris at chris-wilson.co.uk
Wed Nov 15 17:58:39 UTC 2017


Quoting Jackie Li (2017-11-15 17:17:08)
> Static WOPCM partitioning will lead to GuC loading failure
> if the GuC/HuC firmware size exceeded current static 512KB
> partition boundary.
> 
> This patch enables the dynamical calculation of the WOPCM aperture
> used by GuC/HuC firmware. GuC WOPCM offset is  set to
> HuC size + reserved WOPCM size. GuC WOPCM size is set to
> total WOPCM size - GuC WOPCM offset - RC6CTX size. In this case,
> GuC WOPCM offset will be updated based on the size of HuC firmware
> while GuC WOPCM size will be set to use all the remaining WOPCM space.
> 
> v2:
>  - Removed intel_wopcm_init (Ville/Sagar/Joonas)
>  - Renamed and Moved the intel_wopcm_partition into intel_guc (Sagar)
>  - Removed unnecessary function calls (Joonas)
>  - Init GuC WOPCM partition as soon as firmware fetching is completed

Fix your indenting. Use tabs, align to brackets etc.

> Signed-off-by: Jackie Li <yaodong.li at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Sujaritha Sundaresan <sujaritha.sundaresan at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: John Spotswood <john.a.spotswood at intel.com>
> Cc: Oscar Mateo <oscar.mateo at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c |   4 +-
>  drivers/gpu/drm/i915/i915_guc_reg.h     |  18 +++--
>  drivers/gpu/drm/i915/intel_guc.c        |  25 ++++---
>  drivers/gpu/drm/i915/intel_guc.h        |  25 +++----
>  drivers/gpu/drm/i915/intel_huc.c        |   2 +-
>  drivers/gpu/drm/i915/intel_uc.c         | 116 +++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_uc_fw.c      |  11 ++-
>  7 files changed, 163 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 2db0406..285c310 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -312,12 +312,12 @@ __create_hw_context(struct drm_i915_private *dev_priv,
>         ctx->desc_template =
>                 default_desc_template(dev_priv, dev_priv->mm.aliasing_ppgtt);
>  
> -       /* GuC requires the ring to be placed above GUC_WOPCM_TOP. If GuC is not
> +       /* GuC requires the ring to be placed above GuC WOPCM top. if GuC is not
>          * present or not in use we still need a small bias as ring wraparound
>          * at offset 0 sometimes hangs. No idea why.
>          */
>         if (HAS_GUC(dev_priv) && i915_modparams.enable_guc_loading)
> -               ctx->ggtt_offset_bias = GUC_WOPCM_TOP;
> +               ctx->ggtt_offset_bias = dev_priv->guc.wopcm.top;
>         else
>                 ctx->ggtt_offset_bias = I915_GTT_PAGE_SIZE;
>  
> diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h b/drivers/gpu/drm/i915/i915_guc_reg.h
> index bc1ae7d..567df12 100644
> --- a/drivers/gpu/drm/i915/i915_guc_reg.h
> +++ b/drivers/gpu/drm/i915/i915_guc_reg.h
> @@ -67,17 +67,27 @@
>  #define DMA_GUC_WOPCM_OFFSET           _MMIO(0xc340)
>  #define   HUC_LOADING_AGENT_VCR                  (0<<1)
>  #define   HUC_LOADING_AGENT_GUC                  (1<<1)
> -#define   GUC_WOPCM_OFFSET_VALUE         0x80000       /* 512KB */
>  #define GUC_MAX_IDLE_COUNT             _MMIO(0xC3E4)
>  
>  #define HUC_STATUS2             _MMIO(0xD3B0)
>  #define   HUC_FW_VERIFIED       (1<<7)
>  
>  /* Defines WOPCM space available to GuC firmware */
> +/* Default WOPCM size 1MB */
> +#define WOPCM_DEFAULT_SIZE             (0x1 << 20)
> +/* Reserved WOPCM size 16KB */
> +#define WOPCM_RESERVED_SIZE            (0x4000)
> +/* GUC WOPCM Offset need to be 16KB aligned */
> +#define WOPCM_OFFSET_ALIGNMENT         (0x4000)
> +/* 8KB stack reserved for GuC FW*/
> +#define GUC_WOPCM_STACK_RESERVED       (0x2000)
> +/* 24KB WOPCM reserved for RC6 CTX on BXT */
> +#define BXT_WOPCM_RC6_RESERVED         (0x6000)
> +
> +#define GEN9_GUC_WOPCM_DELTA           4
> +#define GEN9_GUC_WOPCM_OFFSET          (0x24000)
> +
>  #define GUC_WOPCM_SIZE                 _MMIO(0xc050)
> -/* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */
> -#define   GUC_WOPCM_TOP                          (0x80 << 12)  /* 512KB */
> -#define   BXT_GUC_WOPCM_RC6_RESERVED     (0x10 << 12)  /* 64KB  */
>  
>  /* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
>  #define GUC_GGTT_TOP                   0xFEE00000
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index 9678630..0c6bd63 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -337,7 +337,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
>   * This is a wrapper to create an object for use with the GuC. In order to
>   * use it inside the GuC, an object needs to be pinned lifetime, so we allocate
>   * both some backing storage and a range inside the Global GTT. We must pin
> - * it in the GGTT somewhere other than than [0, GUC_WOPCM_TOP) because that
> + * it in the GGTT somewhere other than than [0, GuC WOPCM top) because that
>   * range is reserved inside GuC.
>   *
>   * Return:     A i915_vma if successful, otherwise an ERR_PTR.
> @@ -358,7 +358,8 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
>                 goto err;
>  
>         ret = i915_vma_pin(vma, 0, PAGE_SIZE,
> -                          PIN_GLOBAL | PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
> +                          PIN_GLOBAL | PIN_OFFSET_BIAS |
> +                          dev_priv->guc.wopcm.top);
>         if (ret) {
>                 vma = ERR_PTR(ret);
>                 goto err;
> @@ -371,13 +372,19 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
>         return vma;
>  }
>  
> -u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv)
> +/*
> + * GuC does not allow any gfx GGTT address that falls into range
> + * [0, GuC WOPCM top), which is reserved for Boot ROM, SRAM and WOPCM.
> + * All gfx objects used by GuC is pinned with PIN_OFFSET_BIAS along with
> + * top of WOPCM.
> + */
> +u32 guc_ggtt_offset(struct i915_vma *vma)
>  {
> -       u32 wopcm_size = GUC_WOPCM_TOP;
> -
> -       /* On BXT, the top of WOPCM is reserved for RC6 context */
> -       if (IS_GEN9_LP(dev_priv))
> -               wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
> +       u32 guc_wopcm_top = vma->vm->i915->guc.wopcm.top;
> +       u32 offset = i915_ggtt_offset(vma);
>  
> -       return wopcm_size;
> +       GEM_BUG_ON(!guc_wopcm_top);
> +       GEM_BUG_ON(offset < guc_wopcm_top);
> +       GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
> +       return offset;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 607e025..77f619b 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -39,6 +39,12 @@ struct guc_preempt_work {
>         struct intel_engine_cs *engine;
>  };
>  
> +struct guc_wopcm_partition {
> +       u32 offset;
> +       u32 size;
> +       u32 top;
> +};
> +
>  /*
>   * Top level structure of GuC. It handles firmware loading and manages client
>   * pool and doorbells. intel_guc owns a i915_guc_client to replace the legacy
> @@ -46,6 +52,7 @@ struct guc_preempt_work {
>   */
>  struct intel_guc {
>         struct intel_uc_fw fw;
> +       struct guc_wopcm_partition wopcm;
>         struct intel_guc_log log;
>         struct intel_guc_ct ct;
>  
> @@ -100,22 +107,6 @@ static inline void intel_guc_notify(struct intel_guc *guc)
>         guc->notify(guc);
>  }
>  
> -/*
> - * GuC does not allow any gfx GGTT address that falls into range [0, WOPCM_TOP),
> - * which is reserved for Boot ROM, SRAM and WOPCM. Currently this top address is
> - * 512K. In order to exclude 0-512K address space from GGTT, all gfx objects
> - * used by GuC is pinned with PIN_OFFSET_BIAS along with size of WOPCM.
> - */
> -static inline u32 guc_ggtt_offset(struct i915_vma *vma)
> -{
> -       u32 offset = i915_ggtt_offset(vma);
> -
> -       GEM_BUG_ON(offset < GUC_WOPCM_TOP);
> -       GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
> -
> -       return offset;
> -}
> -
>  void intel_guc_init_early(struct intel_guc *guc);
>  void intel_guc_init_send_regs(struct intel_guc *guc);
>  void intel_guc_init_params(struct intel_guc *guc);
> @@ -126,6 +117,6 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
>  int intel_guc_suspend(struct drm_i915_private *dev_priv);
>  int intel_guc_resume(struct drm_i915_private *dev_priv);
>  struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
> -u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
> +u32 guc_ggtt_offset(struct i915_vma *vma);
>  
>  #endif
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index 98d1725..0baedc4 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -202,7 +202,7 @@ void intel_huc_auth(struct intel_huc *huc)
>                 return;
>  
>         vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0,
> -                               PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
> +                               PIN_OFFSET_BIAS | guc->wopcm.top);
>         if (IS_ERR(vma)) {
>                 DRM_ERROR("failed to pin huc fw object %d\n",
>                                 (int)PTR_ERR(vma));
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index aec2954..4f6fa67 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -86,10 +86,122 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
>         intel_guc_init_early(&dev_priv->guc);
>  }
>  
> +static inline u32 reserved_wopcm_size(struct drm_i915_private *i915)
> +{
> +       /* On BXT, the top of WOPCM is reserved for RC6 context */
> +       if (IS_GEN9_LP(i915))
> +               return BXT_WOPCM_RC6_RESERVED;
> +
> +       return 0;
> +}
> +
> +static int do_wopcm_partition(struct drm_i915_private *i915, u32 offset,
> +                              u32 reserved_size)

do ?

From that I was expecting action, talking to hw and excitement.

Pass in guc_wopcm_partition and call this

static int wocpm_partition_init(struct guc_wocpm_partition *wp,
				u32 offset, u32 size)


> +{
> +       struct guc_wopcm_partition *wp = &i915->guc.wopcm;
> +       u32 aligned_offset = ALIGN(offset, WOPCM_OFFSET_ALIGNMENT);
> +
> +       if (offset >= WOPCM_DEFAULT_SIZE)
> +               return -E2BIG;
> +
> +       if (reserved_size >= WOPCM_DEFAULT_SIZE)
> +               return -E2BIG;
> +
> +       if ((aligned_offset + reserved_size) >= WOPCM_DEFAULT_SIZE)
> +               return -E2BIG;
> +
> +       wp->offset = aligned_offset;
> +       wp->top = WOPCM_DEFAULT_SIZE - wp->offset;
> +       wp->size = wp->top - reserved_size;
> +
> +       return 0;
> +}
> +
> +static void guc_init_wopcm_partition(struct drm_i915_private *i915)
> +{
> +       struct intel_uc_fw *guc_fw = &i915->guc.fw;
> +       struct intel_uc_fw *huc_fw = &i915->huc.fw;
> +       struct guc_wopcm_partition *wp = &i915->guc.wopcm;
> +       size_t huc_size, guc_size;
> +       u32 offset;
> +       u32 reserved;
> +       u32 wopcm_base;
> +       u32 delta;
> +       int err;
> +
> +       if (!i915_modparams.enable_guc_loading)
> +               return;
> +
> +       /* No need to do partition if failed to fetch both GuC and HuC FW */
> +       if (guc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS &&
> +               huc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
> +               goto fail;
> +
> +       huc_size = 0;
> +       guc_size = 0;
> +       offset = WOPCM_RESERVED_SIZE;
> +       reserved = reserved_wopcm_size(i915);
> +
> +       if (huc_fw->fetch_status == INTEL_UC_FIRMWARE_SUCCESS)
> +               huc_size = huc_fw->header_size + huc_fw->ucode_size;
> +
> +       err = do_wopcm_partition(i915, offset + huc_size, reserved);
> +       if (err) {
> +               if (!huc_size)
> +                       goto fail;
> +
> +               /* Partition without loading HuC FW. */
> +               err = do_wopcm_partition(i915, offset, reserved);
> +               if (err)
> +                       goto fail;
> +       }
> +
> +       /*
> +        * Check hardware restriction on Gen9
> +        * GuC WOPCM size is at least 4 bytes larger than GuC WOPCM base due
> +        * to hardware limitation on Gen9.
> +        */
> +       if (IS_GEN9(i915)) {
> +               wopcm_base = wp->offset + GEN9_GUC_WOPCM_OFFSET;
> +               if (unlikely(wopcm_base > wp->size))
> +                       goto fail;
> +
> +               delta = wp->size - wopcm_base;
> +               if (unlikely(delta < GEN9_GUC_WOPCM_DELTA))
> +                       goto fail;
> +       }
> +
> +       if (guc_fw->fetch_status == INTEL_UC_FIRMWARE_SUCCESS) {
> +               guc_size = guc_fw->header_size + guc_fw->ucode_size;
> +               /* Need 8K stack for GuC */
> +               guc_size += GUC_WOPCM_STACK_RESERVED;
> +       }
> +
> +       if (guc_size > wp->size)
> +               goto fail;
> +
> +       DRM_DEBUG_DRIVER("GuC WOPCM offset %dKB, size %dKB, top %dKB\n",
> +               wp->offset >> 10, wp->size >> 10, wp->top >> 10);
> +
> +       return;
> +
> +fail:
> +       DRM_ERROR("WOPCM partitioning failed. Falling back to execlist mode\n");

As CI tells you using DRM_ERROR() here is a no-no. Talk to Michal to
determine the right message and when to present it to the *user*.

> +
> +       intel_uc_fini_fw(i915);

This is not yours to fini.

> +       /* GuC submission won't work without valid GuC WOPCM partition */
> +       if (i915_modparams.enable_guc_submission)
> +               i915_modparams.enable_guc_submission = 0;
> +
> +       i915_modparams.enable_guc_loading = 0;

Also feels like a layering violation.

> +}
> +
>  void intel_uc_init_fw(struct drm_i915_private *dev_priv)
>  {
>         intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw);
>         intel_uc_fw_fetch(dev_priv, &dev_priv->guc.fw);
> +       guc_init_wopcm_partition(dev_priv);
>  }


More information about the Intel-gfx mailing list