[PATCH v3 5/7] drm/i915/mtl: Handle wopcm per-GT and limit calculations.

John Harrison john.c.harrison at intel.com
Mon Oct 24 21:46:05 UTC 2022


On 10/24/2022 14:39, Ceraolo Spurio, Daniele wrote:
> On 10/24/2022 2:33 PM, John Harrison wrote:
>> On 10/21/2022 17:10, Daniele Ceraolo Spurio wrote:
>>> From: Aravind Iddamsetty <aravind.iddamsetty at intel.com>
>>>
>>> With MTL standalone media architecture the wopcm layout has changed 
>>> with
>>> separate partitioning in WOPCM for GCD/GT GuC and SA Media GuC. The 
>>> size
>>> of WOPCM is 4MB with lower 2MB for SA Media and upper 2MB for GCD/GT.
>> Given that GCD is not a term used anywhere in the driver, I think it 
>> either needs to be either explained or dropped. Plus, Graphics 
>> Companion Die seems a confusing name for the root GT. Surely the 
>> media GT is the companion? Especially as the code seems to be written 
>> such that the 'companion' is required but the media is optional.
>>
>> While on the subject, the explanation of SA should be more explicit. 
>> E.g. "With MTL Stand Alone Media architecture, the wopcm...".
>
> would this work:
>
> With MTL standalone media architecture the wopcm layout has changed, with
> separate partitioning in WOPCM for the root GT GuC and the media GT GuC.
> The size of WOPCM is 4MB with the lower 2MB reserved for the media GT and
> the upper 2MB for the root GT.
Except that the diagram below still talks about GCD and SA...

>
>>
>>>
>>>      +=====+===> +====================+ <== WOPCM TOP
>>>      ^     ^     |                    |
>>>      |     |     |                    |
>>>      |    GCD    |   GCD RC6 Image    |
>>>      |    GuC    |    Power Context   |
>>>      |    WOPCM  |                    |
>>>      |    Size   +--------------------+
>>>      |     |     |   GCD GuC Image    |
>>>      |     |     |                    |
>>>      |     v     |                    |
>>>      |     +===> +====================+ <== SA Media GuC WOPCM Top
>>>      |     ^     |                    |
>>>      |   SA Media|                    |
>>>      |    GuC    | SA Media RC6 Image |
>>>      |   WOPCM   |    Power Context   |
>>>      |    Size   |                    |
>>>    WOPCM   |     +--------------------+
>>>      |     |     |                    |
>>>      |     |     | SA Media GuC Image |
>>>      |     v     |                    |
>>>      |     +===> +====================+ <== GuC WOPCM base
>>>      |           |     WOPCM RSVD     |
>>>      |           +------------------- + <== HuC Firmware Top
>>>      v           |      HuC FW        |
>>>      +=========> +====================+ <== WOPCM Base
>>>
>>> Given that MTL has GuC deprivilege, the WOPCM registers are pre-locked
>>> by the bios. Therefore, we can skip all the math for the partitioning
>>> and just limit ourselves to sanity checking the values.
>>>
>>> v2: fix makefile file ordering (Jani)
>>> v3: drop XELPM_SAMEDIA_WOPCM_SIZE, check huc instead of VDBOX (John)
>>>
>>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty at intel.com>
>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>>> Cc: Matt Roper <matthew.d.roper at intel.com>
>>> Cc: John Harrison <john.c.harrison at intel.com>
>>> Cc: Alan Previn <alan.previn.teres.alexis at intel.com>
>>> Cc: Jani Nikula <jani.nikula at linux.intel.com>
>>> ---
>>>   Documentation/gpu/i915.rst                  |  2 +-
>>>   drivers/gpu/drm/i915/Makefile               |  5 ++-
>>>   drivers/gpu/drm/i915/gt/intel_ggtt.c        |  2 +-
>>>   drivers/gpu/drm/i915/gt/intel_gt.c          |  1 +
>>>   drivers/gpu/drm/i915/gt/intel_gt_types.h    |  2 +
>>>   drivers/gpu/drm/i915/{ => gt}/intel_wopcm.c | 44 
>>> +++++++++++++++------
>>>   drivers/gpu/drm/i915/{ => gt}/intel_wopcm.h |  0
>>>   drivers/gpu/drm/i915/gt/uc/intel_uc.c       |  4 +-
>>>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c    | 14 ++++---
>>>   drivers/gpu/drm/i915/i915_driver.c          |  2 -
>>>   drivers/gpu/drm/i915/i915_drv.h             |  3 --
>>>   drivers/gpu/drm/i915/i915_gem.c             |  5 ++-
>>>   12 files changed, 52 insertions(+), 32 deletions(-)
>>>   rename drivers/gpu/drm/i915/{ => gt}/intel_wopcm.c (87%)
>>>   rename drivers/gpu/drm/i915/{ => gt}/intel_wopcm.h (100%)
>>>
>>> diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
>>> index 4e59db1cfb00..60ea21734902 100644
>>> --- a/Documentation/gpu/i915.rst
>>> +++ b/Documentation/gpu/i915.rst
>>> @@ -494,7 +494,7 @@ WOPCM
>>>   WOPCM Layout
>>>   ~~~~~~~~~~~~
>>>   -.. kernel-doc:: drivers/gpu/drm/i915/intel_wopcm.c
>>> +.. kernel-doc:: drivers/gpu/drm/i915/gt/intel_wopcm.c
>>>      :doc: WOPCM Layout
>>>     GuC
>>> diff --git a/drivers/gpu/drm/i915/Makefile 
>>> b/drivers/gpu/drm/i915/Makefile
>>> index 2535593ab379..cf3a96b3cd58 100644
>>> --- a/drivers/gpu/drm/i915/Makefile
>>> +++ b/drivers/gpu/drm/i915/Makefile
>>> @@ -127,9 +127,11 @@ gt-y += \
>>>       gt/intel_sseu.o \
>>>       gt/intel_sseu_debugfs.o \
>>>       gt/intel_timeline.o \
>>> +    gt/intel_wopcm.o \
>>>       gt/intel_workarounds.o \
>>>       gt/shmem_utils.o \
>>>       gt/sysfs_engines.o
>>> +
>>>   # x86 intel-gtt module support
>>>   gt-$(CONFIG_X86) += gt/intel_ggtt_gmch.o
>>>   # autogenerated null render state
>>> @@ -183,8 +185,7 @@ i915-y += \
>>>         i915_trace_points.o \
>>>         i915_ttm_buddy_manager.o \
>>>         i915_vma.o \
>>> -      i915_vma_resource.o \
>>> -      intel_wopcm.o
>>> +      i915_vma_resource.o
>>>     # general-purpose microcontroller (GuC) support
>>>   i915-y += gt/uc/intel_uc.o \
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
>>> b/drivers/gpu/drm/i915/gt/intel_ggtt.c
>>> index 6b58c95ad6a0..9263f10ecd28 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
>>> @@ -560,7 +560,7 @@ static int init_ggtt(struct i915_ggtt *ggtt)
>>>        * why.
>>>        */
>>>       ggtt->pin_bias = max_t(u32, I915_GTT_PAGE_SIZE,
>>> - intel_wopcm_guc_size(&ggtt->vm.i915->wopcm));
>>> + intel_wopcm_guc_size(&ggtt->vm.gt->wopcm));
>>>         ret = intel_vgt_balloon(ggtt);
>>>       if (ret)
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c 
>>> b/drivers/gpu/drm/i915/gt/intel_gt.c
>>> index 27dbb9e4bd6c..8c751314df3d 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>>> @@ -56,6 +56,7 @@ void intel_gt_common_init_early(struct intel_gt *gt)
>>>       seqcount_mutex_init(&gt->tlb.seqno, &gt->tlb.invalidate_lock);
>>>       intel_gt_pm_init_early(gt);
>>>   +    intel_wopcm_init_early(&gt->wopcm);
>>>       intel_uc_init_early(&gt->uc);
>>>       intel_rps_init_early(&gt->rps);
>>>   }
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h 
>>> b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>> index 64aa2ba624fc..2d18fd9ab11f 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>> @@ -30,6 +30,7 @@
>>>   #include "intel_migrate_types.h"
>>>   #include "intel_wakeref.h"
>>>   #include "pxp/intel_pxp_types.h"
>>> +#include "intel_wopcm.h"
>>>     struct drm_i915_private;
>>>   struct i915_ggtt;
>>> @@ -100,6 +101,7 @@ struct intel_gt {
>>>         struct intel_uc uc;
>>>       struct intel_gsc gsc;
>>> +    struct intel_wopcm wopcm;
>>>         struct {
>>>           /* Serialize global tlb invalidations */
>>> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c 
>>> b/drivers/gpu/drm/i915/gt/intel_wopcm.c
>>> similarity index 87%
>>> rename from drivers/gpu/drm/i915/intel_wopcm.c
>>> rename to drivers/gpu/drm/i915/gt/intel_wopcm.c
>>> index 322fb9eeb880..c91f234adc55 100644
>>> --- a/drivers/gpu/drm/i915/intel_wopcm.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_wopcm.c
>>> @@ -64,9 +64,9 @@
>>>   #define GEN9_GUC_FW_RESERVED    SZ_128K
>>>   #define GEN9_GUC_WOPCM_OFFSET    (GUC_WOPCM_RESERVED + 
>>> GEN9_GUC_FW_RESERVED)
>>>   -static inline struct drm_i915_private *wopcm_to_i915(struct 
>>> intel_wopcm *wopcm)
>>> +static inline struct intel_gt *wopcm_to_gt(struct intel_wopcm *wopcm)
>>>   {
>>> -    return container_of(wopcm, struct drm_i915_private, wopcm);
>>> +    return container_of(wopcm, struct intel_gt, wopcm);
>>>   }
>>>     /**
>>> @@ -77,7 +77,8 @@ static inline struct drm_i915_private 
>>> *wopcm_to_i915(struct intel_wopcm *wopcm)
>>>    */
>>>   void intel_wopcm_init_early(struct intel_wopcm *wopcm)
>>>   {
>>> -    struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
>>> +    struct intel_gt *gt = wopcm_to_gt(wopcm);
>>> +    struct drm_i915_private *i915 = gt->i915;
>>>         if (!HAS_GT_UC(i915))
>>>           return;
>>> @@ -157,14 +158,16 @@ static bool check_hw_restrictions(struct 
>>> drm_i915_private *i915,
>>>       return true;
>>>   }
>>>   -static bool __check_layout(struct drm_i915_private *i915, u32 
>>> wopcm_size,
>>> +static bool __check_layout(struct intel_gt *gt, u32 wopcm_size,
>>>                  u32 guc_wopcm_base, u32 guc_wopcm_size,
>>>                  u32 guc_fw_size, u32 huc_fw_size)
>>>   {
>>> +    struct drm_i915_private *i915 = gt->i915;
>> This is no longer required now that the MEDIA_VER check is gone?
>
> This is still needed because i915 is still used (e.g. in the line 
> below). It was passed as a parameter before, but now that we pass in 
> intel_gt we need to define it locally.
Doh! Just call me blind.

John.

>
>>>       const u32 ctx_rsvd = context_reserved_size(i915);
>>>       u32 size;
>>>         size = wopcm_size - ctx_rsvd;
>>> +
>> Likewise, is this blank line still intended?
>
> No, I'll drop it.
>
> Daniele
>
>>
>> John.
>>
>>>       if (unlikely(range_overflows(guc_wopcm_base, guc_wopcm_size, 
>>> size))) {
>>>           drm_err(&i915->drm,
>>>               "WOPCM: invalid GuC region layout: %uK + %uK > %uK\n",
>>> @@ -181,12 +184,14 @@ static bool __check_layout(struct 
>>> drm_i915_private *i915, u32 wopcm_size,
>>>           return false;
>>>       }
>>>   -    size = huc_fw_size + WOPCM_RESERVED_SIZE;
>>> -    if (unlikely(guc_wopcm_base < size)) {
>>> -        drm_err(&i915->drm, "WOPCM: no space for %s: %uK < %uK\n",
>>> -            intel_uc_fw_type_repr(INTEL_UC_FW_TYPE_HUC),
>>> -            guc_wopcm_base / SZ_1K, size / SZ_1K);
>>> -        return false;
>>> +    if (intel_uc_supports_huc(&gt->uc)) {
>>> +        size = huc_fw_size + WOPCM_RESERVED_SIZE;
>>> +        if (unlikely(guc_wopcm_base < size)) {
>>> +            drm_err(&i915->drm, "WOPCM: no space for %s: %uK < %uK\n",
>>> +                intel_uc_fw_type_repr(INTEL_UC_FW_TYPE_HUC),
>>> +                guc_wopcm_base / SZ_1K, size / SZ_1K);
>>> +            return false;
>>> +        }
>>>       }
>>>         return check_hw_restrictions(i915, guc_wopcm_base, 
>>> guc_wopcm_size,
>>> @@ -228,8 +233,8 @@ static bool __wopcm_regs_writable(struct 
>>> intel_uncore *uncore)
>>>    */
>>>   void intel_wopcm_init(struct intel_wopcm *wopcm)
>>>   {
>>> -    struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
>>> -    struct intel_gt *gt = to_gt(i915);
>>> +    struct intel_gt *gt = wopcm_to_gt(wopcm);
>>> +    struct drm_i915_private *i915 = gt->i915;
>>>       u32 guc_fw_size = intel_uc_fw_get_upload_size(&gt->uc.guc.fw);
>>>       u32 huc_fw_size = intel_uc_fw_get_upload_size(&gt->uc.huc.fw);
>>>       u32 ctx_rsvd = context_reserved_size(i915);
>>> @@ -274,6 +279,19 @@ void intel_wopcm_init(struct intel_wopcm *wopcm)
>>>           goto check;
>>>       }
>>>   +    /*
>>> +     * On platforms with a media GT, the WOPCM is partitioned 
>>> between the
>>> +     * two GTs, so we would have to take that into account when 
>>> doing the
>>> +     * math below. There is also a new section reserved for the GSC 
>>> context
>>> +     * that would have to be factored in. However, all platforms 
>>> with a
>>> +     * media GT also have GuC depriv enabled, so the WOPCM regs are
>>> +     * pre-locked and therefore we don't have to do the math 
>>> ourselves.
>>> +     */
>>> +    if (unlikely(i915->media_gt)) {
>>> +        drm_err(&i915->drm, "Unlocked WOPCM regs with media GT\n");
>>> +        return;
>>> +    }
>>> +
>>>       /*
>>>        * Aligned value of guc_wopcm_base will determine available 
>>> WOPCM space
>>>        * for HuC firmware and mandatory reserved area.
>>> @@ -295,7 +313,7 @@ void intel_wopcm_init(struct intel_wopcm *wopcm)
>>>           guc_wopcm_base / SZ_1K, guc_wopcm_size / SZ_1K);
>>>     check:
>>> -    if (__check_layout(i915, wopcm_size, guc_wopcm_base, 
>>> guc_wopcm_size,
>>> +    if (__check_layout(gt, wopcm_size, guc_wopcm_base, guc_wopcm_size,
>>>                  guc_fw_size, huc_fw_size)) {
>>>           wopcm->guc.base = guc_wopcm_base;
>>>           wopcm->guc.size = guc_wopcm_size;
>>> diff --git a/drivers/gpu/drm/i915/intel_wopcm.h 
>>> b/drivers/gpu/drm/i915/gt/intel_wopcm.h
>>> similarity index 100%
>>> rename from drivers/gpu/drm/i915/intel_wopcm.h
>>> rename to drivers/gpu/drm/i915/gt/intel_wopcm.h
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c 
>>> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>>> index dbd048b77e19..4cd8a787f9e5 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>>> @@ -357,8 +357,8 @@ static int uc_init_wopcm(struct intel_uc *uc)
>>>   {
>>>       struct intel_gt *gt = uc_to_gt(uc);
>>>       struct intel_uncore *uncore = gt->uncore;
>>> -    u32 base = intel_wopcm_guc_base(&gt->i915->wopcm);
>>> -    u32 size = intel_wopcm_guc_size(&gt->i915->wopcm);
>>> +    u32 base = intel_wopcm_guc_base(&gt->wopcm);
>>> +    u32 size = intel_wopcm_guc_size(&gt->wopcm);
>>>       u32 huc_agent = intel_uc_uses_huc(uc) ? HUC_LOADING_AGENT_GUC 
>>> : 0;
>>>       u32 mask;
>>>       int err;
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c 
>>> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> index 81e06d71c1a8..0c80ba51a4bd 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> @@ -478,10 +478,11 @@ static int check_gsc_manifest(const struct 
>>> firmware *fw,
>>>       return 0;
>>>   }
>>>   -static int check_ccs_header(struct drm_i915_private *i915,
>>> +static int check_ccs_header(struct intel_gt *gt,
>>>                   const struct firmware *fw,
>>>                   struct intel_uc_fw *uc_fw)
>>>   {
>>> +    struct drm_i915_private *i915 = gt->i915;
>>>       struct uc_css_header *css;
>>>       size_t size;
>>>   @@ -523,10 +524,10 @@ static int check_ccs_header(struct 
>>> drm_i915_private *i915,
>>>         /* Sanity check whether this fw is not larger than whole 
>>> WOPCM memory */
>>>       size = __intel_uc_fw_get_upload_size(uc_fw);
>>> -    if (unlikely(size >= i915->wopcm.size)) {
>>> +    if (unlikely(size >= gt->wopcm.size)) {
>>>           drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu > 
>>> %zu\n",
>>>                intel_uc_fw_type_repr(uc_fw->type), 
>>> uc_fw->file_selected.path,
>>> -             size, (size_t)i915->wopcm.size);
>>> +             size, (size_t)gt->wopcm.size);
>>>           return -E2BIG;
>>>       }
>>>   @@ -554,7 +555,8 @@ static int check_ccs_header(struct 
>>> drm_i915_private *i915,
>>>    */
>>>   int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>>>   {
>>> -    struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915;
>>> +    struct intel_gt *gt = __uc_fw_to_gt(uc_fw);
>>> +    struct drm_i915_private *i915 = gt->i915;
>>>       struct intel_uc_fw_file file_ideal;
>>>       struct device *dev = i915->drm.dev;
>>>       struct drm_i915_gem_object *obj;
>>> @@ -562,7 +564,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>>>       bool old_ver = false;
>>>       int err;
>>>   -    GEM_BUG_ON(!i915->wopcm.size);
>>> +    GEM_BUG_ON(!gt->wopcm.size);
>>>       GEM_BUG_ON(!intel_uc_fw_is_enabled(uc_fw));
>>>         err = i915_inject_probe_error(i915, -ENXIO);
>>> @@ -615,7 +617,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>>>       if (uc_fw->loaded_via_gsc)
>>>           err = check_gsc_manifest(fw, uc_fw);
>>>       else
>>> -        err = check_ccs_header(i915, fw, uc_fw);
>>> +        err = check_ccs_header(gt, fw, uc_fw);
>>>       if (err)
>>>           goto fail;
>>>   diff --git a/drivers/gpu/drm/i915/i915_driver.c 
>>> b/drivers/gpu/drm/i915/i915_driver.c
>>> index ffff49868dc5..ba4b71aedc40 100644
>>> --- a/drivers/gpu/drm/i915/i915_driver.c
>>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>>> @@ -371,8 +371,6 @@ static int i915_driver_early_probe(struct 
>>> drm_i915_private *dev_priv)
>>>       if (ret)
>>>           goto err_ttm;
>>>   -    intel_wopcm_init_early(&dev_priv->wopcm);
>>> -
>>>       ret = intel_root_gt_init_early(dev_priv);
>>>       if (ret < 0)
>>>           goto err_rootgt;
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>>> b/drivers/gpu/drm/i915/i915_drv.h
>>> index 9453fdd4205f..66aa2cd9aefe 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -62,7 +62,6 @@
>>>   #include "intel_runtime_pm.h"
>>>   #include "intel_step.h"
>>>   #include "intel_uncore.h"
>>> -#include "intel_wopcm.h"
>>>     struct drm_i915_clock_gating_funcs;
>>>   struct drm_i915_gem_object;
>>> @@ -235,8 +234,6 @@ struct drm_i915_private {
>>>         struct intel_gvt *gvt;
>>>   -    struct intel_wopcm wopcm;
>>> -
>>>       struct pci_dev *bridge_dev;
>>>         struct rb_root uabi_engines;
>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c 
>>> b/drivers/gpu/drm/i915/i915_gem.c
>>> index 9093d2be9e1c..7a9ce81600a0 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>>> @@ -1140,9 +1140,10 @@ int i915_gem_init(struct drm_i915_private 
>>> *dev_priv)
>>>       if (ret)
>>>           return ret;
>>>   -    for_each_gt(gt, dev_priv, i)
>>> +    for_each_gt(gt, dev_priv, i) {
>>>           intel_uc_fetch_firmwares(&gt->uc);
>>> -    intel_wopcm_init(&dev_priv->wopcm);
>>> +        intel_wopcm_init(&gt->wopcm);
>>> +    }
>>>         ret = i915_init_ggtt(dev_priv);
>>>       if (ret) {
>>
>



More information about the dri-devel mailing list