[Intel-gfx] [PATCH v11 2/6] drm/i915: Implement dynamic GuC WOPCM offset and size calculation
Michal Wajdeczko
michal.wajdeczko at intel.com
Thu Mar 1 12:56:34 UTC 2018
On Thu, 01 Mar 2018 02:01:36 +0100, Jackie Li <yaodong.li at intel.com> wrote:
> Hardware may have specific restrictions on GuC WOPCM offset and size. On
> Gen9, the value of the GuC WOPCM size register needs to be larger than
> the
> value of GuC WOPCM offset register + a Gen9 specific offset (144KB) for
> reserved GuC WOPCM. Fail to enforce such a restriction on GuC WOPCM size
> will lead to GuC firmware execution failures. On the other hand, with
> current static GuC WOPCM offset and size values (512KB for both offset
> and
> size), the GuC WOPCM size verification will fail on Gen9 even if it can
> be
> fixed by lowering the GuC WOPCM offset by calculating its value based on
> HuC firmware size (which is likely less than 200KB on Gen9), so that we
> can
> have a GuC WOPCM size value which is large enough to pass the GuC WOPCM
> size check.
>
> This patch updates the reserved GuC WOPCM size for RC6 context on Gen9 to
> 24KB to strictly align with the Gen9 GuC WOPCM layout. It also adds
> support
> to verify the GuC WOPCM size aganist the Gen9 hardware restrictions. To
> meet all above requirements, let's provide dynamic partitioning of the
> WOPCM that will be based on platform specific HuC/GuC firmware sizes.
>
> 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
>
> v3:
> - Fixed indentation issues (Chris)
> - Removed layering violation code (Chris/Michal)
> - Created separat files for GuC wopcm code (Michal)
> - Used inline function to avoid code duplication (Michal)
>
> v4:
> - Preset the GuC WOPCM top during early GuC init (Chris)
> - Fail intel_uc_init_hw() as soon as GuC WOPCM partitioning failed
>
> v5:
> - Moved GuC DMA WOPCM register updating code into intel_wopcm.c
> - Took care of the locking status before writing to GuC DMA
> Write-Once registers. (Joonas)
>
> v6:
> - Made sure the GuC WOPCM size to be multiple of 4K (4K aligned)
>
> v8:
> - Updated comments and fixed naming issues (Sagar/Joonas)
> - Updated commit message to include more description about the hardware
> restriction on GuC WOPCM size (Sagar)
>
> v9:
> - Minor changes variable names and code comments (Sagar)
> - Added detailed GuC WOPCM layout drawing (Sagar/Michal)
> - Refined macro definitions to be reader friendly (Michal)
> - Removed redundent check to valid flag (Michal)
> - Unified first parameter for exported GuC WOPCM functions (Michal)
> - Refined the name and parameter list of hardware restriction checking
> functions (Michal)
>
> v10:
> - Used shorter function name for internal functions (Joonas)
> - Moved init-ealry function into c file (Joonas)
> - Consolidated and removed redundant size checks (Joonas/Michal)
> - Removed unnecessary unlikely() from code which is only called once
> during boot (Joonas)
> - More fixes to kernel-doc format and content (Michal)
> - Avoided the use of PAGE_MASK for 4K pages (Michal)
> - Added error log messages to error paths (Michal)
>
> v11:
> - Replaced intel_guc_wopcm with more generic intel_wopcm and attached
> intel_wopcm to drm_i915_private instead intel_guc (Michal)
> - dynamic calculation of GuC non-wopcm memory start (a.k.a WOPCM Top
> offset from GuC WOPCM base) (Michal)
> - Moved WOPCM marco definitions into .c source file (Michal)
> - Exported WOPCM layout diagram as kernel-doc (Michal)
>
> Bspec: 12690
>
> 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>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble at intel.com> (v8)
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com> (v9)
> Signed-off-by: Jackie Li <yaodong.li at intel.com>
> ---
> drivers/gpu/drm/i915/Makefile | 3 +-
> drivers/gpu/drm/i915/i915_drv.c | 1 +
> drivers/gpu/drm/i915/i915_drv.h | 8 ++
> drivers/gpu/drm/i915/i915_gem.c | 4 +
> drivers/gpu/drm/i915/i915_gem_context.c | 5 +-
> drivers/gpu/drm/i915/intel_guc.c | 66 ++++++++---
> drivers/gpu/drm/i915/intel_guc.h | 16 ++-
> drivers/gpu/drm/i915/intel_guc_reg.h | 8 +-
> drivers/gpu/drm/i915/intel_huc.c | 2 +-
> drivers/gpu/drm/i915/intel_uc.c | 6 +-
> drivers/gpu/drm/i915/intel_uc_fw.c | 13 +--
> drivers/gpu/drm/i915/intel_uc_fw.h | 16 +++
> drivers/gpu/drm/i915/intel_wopcm.c | 195
> ++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_wopcm.h | 34 ++++++
> 14 files changed, 337 insertions(+), 40 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/intel_wopcm.c
> create mode 100644 drivers/gpu/drm/i915/intel_wopcm.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile
> b/drivers/gpu/drm/i915/Makefile
> index 881d712..652549e 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -78,7 +78,8 @@ i915-y += i915_cmd_parser.o \
> intel_lrc.o \
> intel_mocs.o \
> intel_ringbuffer.o \
> - intel_uncore.o
> + intel_uncore.o \
> + intel_wopcm.o
> # general-purpose microcontroller (GuC) support
> i915-y += intel_uc.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> index aaa861b..4c093bc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -918,6 +918,7 @@ static int i915_driver_init_early(struct
> drm_i915_private *dev_priv,
> mutex_init(&dev_priv->wm.wm_mutex);
> mutex_init(&dev_priv->pps_mutex);
> + intel_wopcm_init_early(&dev_priv->wopcm);
> intel_uc_init_early(dev_priv);
> i915_memcpy_init_early(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 7bbec55..7a6b906 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -65,6 +65,7 @@
> #include "intel_ringbuffer.h"
> #include "intel_uncore.h"
> #include "intel_uc.h"
> +#include "intel_wopcm.h"
bikeshed: maybe we should include wopcm before uc to match field
order in the struct drm_i915_private?
> #include "i915_gem.h"
> #include "i915_gem_context.h"
> @@ -1860,6 +1861,8 @@ struct drm_i915_private {
> struct intel_gvt *gvt;
> + struct intel_wopcm wopcm;
> +
> struct intel_huc huc;
> struct intel_guc guc;
> @@ -2391,6 +2394,11 @@ static inline struct drm_i915_private
> *kdev_to_i915(struct device *kdev)
> return to_i915(dev_get_drvdata(kdev));
> }
> +static inline struct drm_i915_private *wopcm_to_i915(struct intel_wopcm
> *wopcm)
> +{
> + return container_of(wopcm, struct drm_i915_private, wopcm);
> +}
> +
> static inline struct drm_i915_private *guc_to_i915(struct intel_guc
> *guc)
> {
> return container_of(guc, struct drm_i915_private, guc);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> b/drivers/gpu/drm/i915/i915_gem.c
> index 14c855b..d31ad0b 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5279,6 +5279,10 @@ int i915_gem_init(struct drm_i915_private
> *dev_priv)
> if (ret)
> return ret;
> + ret = intel_wopcm_init(&dev_priv->wopcm);
> + if (ret)
> + return ret;
> +
> ret = intel_uc_init_misc(dev_priv);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index a73340ae..285e80c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -312,12 +312,13 @@ __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 in Non-WOPCM memory. 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 (USES_GUC(dev_priv))
> - ctx->ggtt_offset_bias = GUC_WOPCM_TOP;
> + ctx->ggtt_offset_bias = dev_priv->guc.ggtt_pin_bias;
> else
> ctx->ggtt_offset_bias = I915_GTT_PAGE_SIZE;
> diff --git a/drivers/gpu/drm/i915/intel_guc.c
> b/drivers/gpu/drm/i915/intel_guc.c
> index a788e15..67f412a 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -470,6 +470,57 @@ int intel_guc_resume(struct drm_i915_private
> *dev_priv)
> }
> /**
> + * DOC: GuC Address Space
> + *
> + * The layout of GuC address space is shown as below:
> + *
> + * +==============> +====================+ <== GUC_GGTT_TOP
> + * ^ | |
> + * | | |
> + * | | DRAM |
> + * | | Memory |
> + * | | |
> + * GuC | |
> + * Address +========> +====================+ <== WOPCM Top
> + * Space ^ | HW contexts RSVD |
> + * | | | WOPCM |
> + * | | +==> +--------------------+ <== GuC WOPCM Top
> + * | GuC ^ | |
> + * | GGTT | | |
> + * | Pin GuC | GuC |
> + * | Bias WOPCM | WOPCM |
> + * | | Size | |
> + * | | | | |
> + * v v v | |
> + * +=====+=====+==> +====================+ <== GuC WOPCM Base
> + * | Non-GuC WOPCM |
> + * | (HuC/Reserved) |
> + * +====================+ <== WOPCM Base
> + *
> + * The lower part [0, GuC ggtt_pin_bias) was mapped to WOPCM which
> consists of
> + * GuC WOPCM and WOPCM reserved for other usage (e.g.RC6 context). The
> value of
> + * the GuC ggtt_pin_bias is determined by the actually GuC WOPCM size
> which is
> + * set in GUC_WOPCM_SIZE register.
> + */
> +
> +/**
> + * intel_guc_init_ggtt_pin_bias() - Initialize the GuC ggtt_pin_bias
> value.
> + * @guc: intel_guc structure.
> + *
> + * This functional will calculate and initialize the ggtt_pin_bias
> value based
> + * on overall WOPCM size and GuC WOPCM size.
> + */
> +void intel_guc_init_ggtt_pin_bias(struct intel_guc *guc)
> +{
> + struct drm_i915_private *i915 = guc_to_i915(guc);
> +
> + GEM_BUG_ON(!i915->wopcm.size);
> + GEM_BUG_ON(i915->wopcm.size < i915->wopcm.guc.base);
> +
> + guc->ggtt_pin_bias = i915->wopcm.size - i915->wopcm.guc.base;
> +}
> +
> +/**
> * intel_guc_allocate_vma() - Allocate a GGTT VMA for GuC usage
> * @guc: the guc
> * @size: size of area to allocate (both virtual space and memory)
> @@ -477,7 +528,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 ggtt_pin_bias)
> because that
> * range is reserved inside GuC.
> *
> * Return: A i915_vma if successful, otherwise an ERR_PTR.
> @@ -498,7 +549,7 @@ 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 | guc->ggtt_pin_bias);
> if (ret) {
> vma = ERR_PTR(ret);
> goto err;
> @@ -510,14 +561,3 @@ struct i915_vma *intel_guc_allocate_vma(struct
> intel_guc *guc, u32 size)
> i915_gem_object_put(obj);
> return vma;
> }
> -
> -u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv)
> -{
> - 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;
> -
> - return wopcm_size;
> -}
> diff --git a/drivers/gpu/drm/i915/intel_guc.h
> b/drivers/gpu/drm/i915/intel_guc.h
> index 0c8b10a..d5077c6 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -49,6 +49,9 @@ struct intel_guc {
> struct intel_guc_log log;
> struct intel_guc_ct ct;
> + /** @ggtt_pin_bias: offset where Non-WOPCM memory starts. */
> + u32 ggtt_pin_bias;
> +
> /* Log snapshot if GuC errors during load */
> struct drm_i915_gem_object *load_err_log;
> @@ -108,10 +111,11 @@ static inline void intel_guc_notify(struct
> intel_guc *guc)
> * @guc: intel_guc structure.
> * @vma: i915 graphics virtual memory area.
> *
> - * 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.
> + * GuC does not allow any gfx GGTT address that falls into range
> + * [0, GuC ggtt_pin_bias), which is reserved for Boot ROM, SRAM and
> WOPCM.
> + * Currently, in order to exclude [0, GuC ggtt_pin_bias) address space
> from
> + * GGTT, all gfx objects used by GuC is allocated with
> intel_guc_allocate_vma()
> + * and pinned with PIN_OFFSET_BIAS along with the value of GuC
> ggtt_pin_bias.
> *
> * Return: GGTT offset that meets the GuC gfx address requirement.
> */
> @@ -120,7 +124,7 @@ static inline u32 intel_guc_ggtt_offset(struct
> intel_guc *guc,
> {
> u32 offset = i915_ggtt_offset(vma);
> - GEM_BUG_ON(offset < GUC_WOPCM_TOP);
> + GEM_BUG_ON(offset < guc->ggtt_pin_bias);
> GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
> return offset;
> @@ -129,6 +133,7 @@ static inline u32 intel_guc_ggtt_offset(struct
> intel_guc *guc,
> 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);
> +void intel_guc_init_ggtt_pin_bias(struct intel_guc *guc);
> int intel_guc_init_wq(struct intel_guc *guc);
> void intel_guc_fini_wq(struct intel_guc *guc);
> int intel_guc_init(struct intel_guc *guc);
> @@ -140,6 +145,5 @@ 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);
> #endif
> diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h
> b/drivers/gpu/drm/i915/intel_guc_reg.h
> index 711e9e9..01963d0 100644
> --- a/drivers/gpu/drm/i915/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/intel_guc_reg.h
> @@ -68,17 +68,15 @@
> #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_WOPCM_OFFSET_SHIFT 14
> #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 */
> #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 */
> +#define GUC_WOPCM_SIZE_SHIFT 12
> +#define GUC_WOPCM_SIZE_MASK (0xfffff << GUC_WOPCM_SIZE_SHIFT)
> #define GEN8_GT_PM_CONFIG _MMIO(0x138140)
> #define GEN9LP_GT_PM_CONFIG _MMIO(0x138140)
> diff --git a/drivers/gpu/drm/i915/intel_huc.c
> b/drivers/gpu/drm/i915/intel_huc.c
> index 70a5282..258a488 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -207,7 +207,7 @@ int intel_huc_auth(struct intel_huc *huc)
> return -ENOEXEC;
> vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0,
> - PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
> + PIN_OFFSET_BIAS | guc->ggtt_pin_bias);
> if (IS_ERR(vma)) {
> ret = PTR_ERR(vma);
> DRM_ERROR("HuC: Failed to pin huc fw object %d\n", ret);
> diff --git a/drivers/gpu/drm/i915/intel_uc.c
> b/drivers/gpu/drm/i915/intel_uc.c
> index 9f1bac6..964e49f 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -248,6 +248,8 @@ int intel_uc_init_misc(struct drm_i915_private
> *dev_priv)
> if (!USES_GUC(dev_priv))
> return 0;
> + intel_guc_init_ggtt_pin_bias(guc);
> +
> ret = intel_guc_init_wq(guc);
> if (ret) {
> DRM_ERROR("Couldn't allocate workqueues for GuC\n");
> @@ -340,9 +342,9 @@ int intel_uc_init_hw(struct drm_i915_private
> *dev_priv)
> gen9_reset_guc_interrupts(dev_priv);
> /* init WOPCM */
> - I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(dev_priv));
> + I915_WRITE(GUC_WOPCM_SIZE, dev_priv->wopcm.guc.size);
> I915_WRITE(DMA_GUC_WOPCM_OFFSET,
> - GUC_WOPCM_OFFSET_VALUE | HUC_LOADING_AGENT_GUC);
> + dev_priv->wopcm.guc.base | HUC_LOADING_AGENT_GUC);
> /* WaEnableuKernelHeaderValidFix:skl */
> /* WaEnableGuCBootHashCheckNotSet:skl,bxt,kbl */
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c
> b/drivers/gpu/drm/i915/intel_uc_fw.c
> index 3ec0ce5..30c7324 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.c
> @@ -95,15 +95,6 @@ void intel_uc_fw_fetch(struct drm_i915_private
> *dev_priv,
> uc_fw->ucode_offset = uc_fw->header_offset + uc_fw->header_size;
> uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
> - /* Header and uCode will be loaded to WOPCM */
> - size = uc_fw->header_size + uc_fw->ucode_size;
> - if (size > intel_guc_wopcm_size(dev_priv)) {
> - DRM_WARN("%s: Firmware is too large to fit in WOPCM\n",
> - intel_uc_fw_type_repr(uc_fw->type));
> - err = -E2BIG;
> - goto fail;
> - }
> -
> /* now RSA */
> if (css->key_size_dw != UOS_RSA_SCRATCH_COUNT) {
> DRM_WARN("%s: Mismatched firmware RSA key size (%u)\n",
> @@ -208,6 +199,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
> int (*xfer)(struct intel_uc_fw *uc_fw,
> struct i915_vma *vma))
> {
> + struct drm_i915_private *i915 = to_i915(uc_fw->obj->base.dev);
> struct i915_vma *vma;
> int err;
> @@ -231,7 +223,8 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
> }
> vma = i915_gem_object_ggtt_pin(uc_fw->obj, NULL, 0, 0,
> - PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
> + PIN_OFFSET_BIAS |
> + i915->guc.ggtt_pin_bias);
> if (IS_ERR(vma)) {
> err = PTR_ERR(vma);
> DRM_DEBUG_DRIVER("%s fw ggtt-pin err=%d\n",
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h
> b/drivers/gpu/drm/i915/intel_uc_fw.h
> index d5fd460..6ebb9cc 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.h
> @@ -115,6 +115,22 @@ static inline bool intel_uc_fw_is_selected(struct
> intel_uc_fw *uc_fw)
> return uc_fw->path != NULL;
> }
> +/**
> + * intel_uc_fw_get_size() - Get the size of the firmware.
> + * @uc_fw: uC firmware.
> + *
> + * Get the size of the firmware that will be placed in WOPCM.
> + *
> + * Return: Size of the firmware, or zero on firmware fetch failure.
> + */
> +static inline u32 intel_uc_fw_get_size(struct intel_uc_fw *uc_fw)
bikeshed: as we already have size field in uc_fw maybe this function
should be named as intel_uc_fw_get_upload_size() to show difference?
> +{
> + if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
> + return 0;
> +
> + return uc_fw->header_size + uc_fw->ucode_size;
> +}
> +
> void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
> struct intel_uc_fw *uc_fw);
> int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c
> b/drivers/gpu/drm/i915/intel_wopcm.c
> new file mode 100644
> index 0000000..83516c9
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
> @@ -0,0 +1,195 @@
> +/*
> + * SPDX-License-Identifier: MIT
> + *
> + * Copyright © 2017-2018 Intel Corporation
> + */
> +
> +#include "intel_wopcm.h"
> +#include "i915_drv.h"
> +
> +/**
> + * DOC: WOPCM Layout.
> + *
> + * The layout of the WOPCM will be fixed after writing to GuC WOPCM
> size and
> + * offset registers whose are calculated are determined by size of
> HuC/GuC
> + * firmware size and set of hw requirements/restrictions as shown
> below:.
> + *
> + * +=========> +====================+ <== WOPCM Top
> + * ^ | HW contexts RSVD |
> + * | +===> +====================+ <== GuC WOPCM Top
> + * | ^ | |
> + * | | | |
> + * | | | |
> + * | GuC | |
> + * | WOPCM | |
> + * | Size +--------------------+ <== (Platform specific reserved
> size)
bikeshed: as most of the values here are (or maybe in the future)
platform specific, I would drop this extra comment from diagram.
> + * WOPCM | | GuC FW RSVD |
> + * | | +--------------------+
> + * | | | GuC Stack RSVD |
> + * | | +------------------- +
> + * | v | GuC WOPCM RSVD |
> + * | +===> +====================+ <== GuC WOPCM base
> + * | | WOPCM RSVD |
> + * | +------------------- + <== HuC Firmware Top
> + * v | HuC FW |
> + * +=========> +====================+ <== WOPCM Base
> + *
> + * GuC accessible WOPCM starts at GuC WOPCM base and ends at GuC WOPCM
> top.
> + * The top part of the WOPCM is reserved for hardware contexts (e.g. RC6
> + * context).
> + */
> +
> +/* Default WOPCM size 1MB. */
> +#define GEN9_WOPCM_SIZE (1024 * 1024)
> +/* 16KB WOPCM (RSVD WOPCM) is reserved from HuC firmware top. */
> +#define WOPCM_RESERVED_SIZE (16 * 1024)
> +
> +/* 16KB reserved at the beginning of GuC WOPCM. */
> +#define GUC_WOPCM_RESERVED (16 * 1024)
> +/* 8KB from GUC_WOPCM_RESERVED is reserved for GuC stack. */
> +#define GUC_WOPCM_STACK_RESERVED (8 * 1024)
> +
> +/* GuC WOPCM Offset value needs to be aligned to 16KB. */
> +#define GUC_WOPCM_OFFSET_ALIGNMENT (1UL << GUC_WOPCM_OFFSET_SHIFT)
> +
> +/* 24KB at the end of WOPCM is reserved for RC6 CTX on BXT. */
> +#define BXT_WOPCM_RC6_CTX_RESERVED (24 * 1024)
> +
> +/* 128KB from GUC_WOPCM_RESERVED is reserved for FW on Gen9. */
> +#define GEN9_GUC_FW_RESERVED (128 * 1024)
> +#define GEN9_GUC_WOPCM_OFFSET (GUC_WOPCM_RESERVED +
> GEN9_GUC_FW_RESERVED)
> +
> +/**
> + * intel_wopcm_init_early() - Early initialization of the GuC WOPCM.
> + * @wopcm: pointer to intel_wopcm.
> + *
> + * Setup the GuC WOPCM top to the top of the overall WOPCM. This will
> guarantee
hmm, I'm not sure that description matches code: s/top/size ?
> + * that the allocation of the GuC accessible objects won't fall into
> WOPCM when
> + * GuC partition isn't present.
hmm, I'm not sure referring GuC here is correct:
GuC partition will not exist only when there is no Guc or whole WOPCM
partitioning failed, and in both cases we will not use the Guc...
> + */
> +void intel_wopcm_init_early(struct intel_wopcm *wopcm)
> +{
> + wopcm->size = GEN9_WOPCM_SIZE;
> +
> + DRM_DEBUG_DRIVER("WOPCM size: %uKiB\n", wopcm->size / 1024);
> +}
> +
> +static inline u32 context_reserved_size(struct drm_i915_private *i915)
> +{
> + if (IS_GEN9_LP(i915))
> + return BXT_WOPCM_RC6_CTX_RESERVED;
> + else
> + return 0;
> +}
> +
> +static inline int gen9_check_dword_gap(u32 guc_wopcm_base, u32
> guc_wopcm_size)
> +{
> + u32 offset;
> +
> + /*
> + * GuC WOPCM size shall be at least a dword larger than the offset from
> + * WOPCM base (GuC WOPCM offset from WOPCM base +
> GEN9_GUC_WOPCM_OFFSET)
> + * due to hardware limitation on Gen9.
> + */
> + offset = guc_wopcm_base + GEN9_GUC_WOPCM_OFFSET;
> + if (offset > guc_wopcm_size ||
> + (guc_wopcm_size - offset) < sizeof(u32)) {
> + DRM_ERROR("GuC WOPCM size(%uKiB) is too small.%uKiB needed.\n",
> + guc_wopcm_size / 1024,
> + (u32)(offset + sizeof(u32)) / 1024);
> + return -E2BIG;
> + }
> +
> + return 0;
> +}
> +
> +static inline int check_hw_restriction(struct drm_i915_private *i915,
> + u32 guc_wopcm_base, u32 guc_wopcm_size)
> +{
> + int err = 0;
> +
> + if (IS_GEN9(i915))
> + err = gen9_check_dword_gap(guc_wopcm_base, guc_wopcm_size);
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +/**
> + * intel_wopcm_init() - Initialize the WOPCM structure.
> + * @wopcm: pointer to intel_wopcm.
> + *
> + * This function will partition WOPCM space based on GuC and HuC
> firmware sizes
> + * and will allocate max remaining for use by GuC. This function will
> also
> + * enforce platform dependent hardware restrictions on GuC WOPCM offset
> and
> + * size. It will fail the WOPCM init if any of these checks were
> failed, so that
> + * the following GuC firmware uploading would be aborted.
> + *
> + * Return: 0 on success, non-zero error code on failure.
> + */
> +int intel_wopcm_init(struct intel_wopcm *wopcm)
> +{
> + struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
> + u32 guc_fw_size = intel_uc_fw_get_size(&i915->guc.fw);
> + u32 huc_fw_size = intel_uc_fw_get_size(&i915->huc.fw);
> + u32 ctx_rsvd = context_reserved_size(i915);
> + u32 guc_wopcm_base;
> + u32 guc_wopcm_size;
> + u32 guc_wopcm_rsvd;
> + int err;
> +
bikeshed: in case we want to be paranoic: GEM_BUG_ON(!wopcm->size)
> + if (guc_fw_size >= wopcm->size) {
> + DRM_ERROR("GuC FW (%uKiB) is too big to fit in WOPCM.",
> + guc_fw_size / 1024);
> + return -E2BIG;
> + }
> +
> + if (huc_fw_size >= wopcm->size) {
> + DRM_ERROR("HuC FW (%uKiB) is too big to fit in WOPCM.",
> + huc_fw_size / 1024);
> + return -E2BIG;
> + }
> +
> + /* Hardware requires GuC WOPCM base to be 16K aligned. */
> + guc_wopcm_base = ALIGN(huc_fw_size + WOPCM_RESERVED_SIZE,
> + GUC_WOPCM_OFFSET_ALIGNMENT);
> + if ((guc_wopcm_base + ctx_rsvd) >= wopcm->size) {
> + DRM_ERROR("GuC WOPCM base(%uKiB) is too big.\n",
> + guc_wopcm_base / 1024);
> + return -E2BIG;
> + }
hmm, all above checks are very unlikely, and are also covered by the
next check below, so maybe we can drop them?
> +
> + guc_wopcm_size = wopcm->size - guc_wopcm_base - ctx_rsvd;
> + /*
> + * GuC WOPCM size must be multiple of 4K pages. We've got the maximum
> + * WOPCM size available for GuC. Trim the size value to 4K boundary.
> + */
> + guc_wopcm_size &= GUC_WOPCM_SIZE_MASK;
> +
> + DRM_DEBUG_DRIVER("Calculated GuC WOPCM Region: [%uKiB, %uKiB)\n",
> + guc_wopcm_base / 1024, guc_wopcm_size / 1024);
> +
bikeshed: [n, m) notation suggests range n..m, so maybe better to use
DRM_DEBUG_DRIVER("Calculated GuC WOPCM: base %uKiB size %uKiB\n",
> + /*
> + * GuC WOPCM size needs to be big enough to include all GuC firmware,
> + * extra 8KiB stack for GuC firmware and GUC_WOPCM_RESERVED.
> + */
> + guc_wopcm_rsvd = GUC_WOPCM_RESERVED + GUC_WOPCM_STACK_RESERVED;
> + if ((guc_fw_size + guc_wopcm_rsvd) > guc_wopcm_size) {
> + DRM_ERROR("Need %uKiB WOPCM for GuC, %uKiB available.\n",
> + (guc_fw_size + guc_wopcm_rsvd) / 1024,
> + guc_wopcm_size / 1024);
hmm, maybe to simplify calculations (and match them directly with diagram)
we should introduce guc_wopcm_size_min:
guc_wopcm_size_min = GUC_WOPCM_RESERVED + GUC_WOPCM_STACK_RESERVED +
guc_fw_size;
if (guc_wopcm_size_min > guc_wopcm_size) {
DRM_ERROR("Need %uKiB WOPCM for GuC, %uKiB available.\n",
guc_wopcm_size_min/1024, guc_wopcm_size/1024);
> + return -E2BIG;
> + }
> +
> + err = check_hw_restriction(i915, guc_wopcm_base, guc_wopcm_size);
> + if (err) {
> + DRM_ERROR("Failed to meet HW restriction.\n");
> + return err;
> + }
> +
> + wopcm->guc.base = guc_wopcm_base;
> + wopcm->guc.size = guc_wopcm_size;
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.h
> b/drivers/gpu/drm/i915/intel_wopcm.h
> new file mode 100644
> index 0000000..39d4847
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_wopcm.h
> @@ -0,0 +1,34 @@
> +/*
> + * SPDX-License-Identifier: MIT
> + *
> + * Copyright © 2017-2018 Intel Corporation
> + */
> +
> +#ifndef _INTEL_WOPCM_H_
> +#define _INTEL_WOPCM_H_
> +
> +#include <linux/types.h>
> +
> +/**
> + * struct intel_wopcm - overall WOPCM info and WOPCM regions.
> + * @size: size of overall WOPCM.
bikeshed: maybe better to place this doc would be inside struct
to do not mix documentation style ?
> + * @guc: GuC WOPCM Region info.
> + */
> +struct intel_wopcm {
> + u32 size;
> + struct {
> + /**
> + * @base: GuC WOPCM base which is offset from WOPCM base.
> + */
> + u32 base;
> + /**
> + * @size: size of the GuC WOPCM region.
> + */
> + u32 size;
> + } guc;
> +};
> +
> +void intel_wopcm_init_early(struct intel_wopcm *wopcm);
> +int intel_wopcm_init(struct intel_wopcm *wopcm);
> +
> +#endif
only few bikesheds plus one suggestion, with that:
Reviewed-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
More information about the Intel-gfx
mailing list