Re: [PATCH] drm/xe/wopcm: Move WOPCM size to platform info

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 6/3/2026 12:48 PM, Matt Roper wrote:
> On Wed, Jun 03, 2026 at 12:41:46PM -0700, Summers, Stuart wrote:
>> On Wed, 2026-06-03 at 12:36 -0700, Matt Roper wrote:
>>> On Wed, Jun 03, 2026 at 12:25:49PM -0700, Matt Roper wrote:
>>>> On Wed, Jun 03, 2026 at 07:18:06PM +0200, Michal Wajdeczko wrote:
>>>>>
>>>>> On 6/3/2026 6:26 PM, Shuicheng Lin wrote:
>>>>>> Replace the per-platform ternary chain in xe_wopcm_size() with
>>>>>> a
>>>>>> field read from xe->info.wopcm_size, populated from the
>>>>>> platform
>>>>>> descriptor.
>>>>> we might want to update check_platform_desc test to catch any
>>>>> missing
>>>>> definitions of wopcm_size field in new device descriptors
>>>>>
>>>>>> Per-platform values are unchanged: SZ_4M for DGFX and MTL,
>>>>>> SZ_2M
>>>>>> for all other platforms.
>>>> Assuming the values here are the total WOPCM size (for the entire
>>>> PCI
>>>> device, not the per-GT subregion), then are these values even
>>>> correct
>>>> and being used for any of the platforms officially supported by Xe?
>>>> From
>>>> what I see on bspec 67090 it sounds like the total size should
>>>> always
>>>> either be 8MB or 16MB for all of our POR platforms?
>>>>
>>>> I think in practice we always find on modern platforms that the
>>>> BIOS has
>>>> already programmed the registers that control the GuC-managed WOPCM
>>>> subregion and locked them before the driver even starts up; in that
>>>> case
>>>> we don't use the value from xe_wopcm_size() at all and instead just
>>>> verify that the pre-programmed regions fit into the larger sizes
>>>> returned by max_wopcm_size().
>>>>
>>>> So I guess that raises the questions:
>>>>
>>>>   - Do we need both xe_wopcm_size() and max_wopcm_size(), and if so
>>>> what
>>>>     is supposed to be the difference in what they represent?  I
>>>> think
>>>>     maybe xe_wopcm_size() was trying to represent some subregion
>>>> that is
>>>>     assumed to be "safe" from collisions with other sub-areas of the
>>>>     WOPCM in cases where we needed to explicitly prorgam the GuC
>>>>     subregion.  But if that's the case, is there a way for us to
>>>> probe
>>>>     those other subregions' sizes/locations directly to avoid
>>>> collisions
>>>>     rather than just using an arbitrary small size that doesn't
>>>> match the
>>>>     total WOPCM size?
>>>>
>>>>   - Do we even need to worry about programming the GuC-managed area
>>>> of
>>>>     the WOPCM anymore on modern platforms or is that a cargo-cult
>>>> from
>>>>     i915 that just isn't relevant anymore?  If it is still relevant,
>>>> is
>>> I see on Bspec 60549 that the GUC_WOPCM_SIZE register can only be
>>> programmed by the GSC on platforms with a de-privileged GuC, so that
>>> does seem to confirm that reprogramming the GuC-specific area of the
>>> WOPCM isn't relevant to any platforms that the Xe driver officially
>>> supports (Xe2 and later), although it's possible that it might be
>>> relevant for some of the unsupported, developer-only platforms (Xe1).
>> That's true, but we do have this code there today. Should we not make
>> it a little more common in the meantime? Or we could hold this under a
>> debug configfs entry or something so it only ever gets used in that
>> case? Maybe we need a debug version of these structures in xe_pci.c?
> The motivation on this patch for moving the numbers into the device
> descriptor was to prepare for future platforms.  But it sounds like that
> will never be the case now since no current or future platforms will
> ever use these numbers.  So if we're going to put anything in the device
> descriptor it sounds like it should be the max_wopcm_size() numbers, not
> the xe_wopcm_size() numbers.
>
> For the old legacy GuC wopcm region programming (which may or may not
> even be relevant for the old Xe1 platforms), I don't know if we really
> need to touch it at all.  But if we do touch it, we should probably
> rename the misleading xe_wopcm_size() and more clearly document what the
> numbers are since they're not actually the total size.

Please note that the WOPCM size can be read from HW (see 
get_wopcm_size() in xe_ttm_stolen_mgr.c). In i915 the WOPCM size was 
initialized before we had HW access and so we used the hardcoded values 
(which work perfectly fine when the driver does the partitioning, 
because we don't have to use the actual HW programmed value as long as 
the one we're using is <= to it), and I think that got transplanted 
as-is to Xe. I believe that the best solution is to just switch to 
reading the WOPCM size from HW everywhere.

Note that there are multiple places where the WOPCM size is required
1 - During stolen init, as mentioned above
2 - During GGTT early init, because the [0, WOPCM_SIZE) range is 
inaccessible to GuC and so we can't map stuff in that range. To be more 
correct, the range that can't be accessed is [0, GUC_WOPCM_TOP), but 
with multiple GTs we have different values of TOP per-GT so it is easier 
to just use the [0, WOPCM_SIZE) range to cover everything.
3 - For calculating the GuC/HuC area partitioning, or validating the 
values found if already programmed

Regarding #3, as Matt mentioned the partitioning is done by IFWI on 
MTL+, and I'd argue that we should just trust IFWI and stop validating 
these values; as long as we're still logging them, if things go wrong at 
GuC load time due to bad partitioning we'll have the info in dmesg for 
debug. We can keep the partitioning logic restricted to legacy non-POR 
platform.

Daniele

>
>
> Matt
>
>> Thanks,
>> Stuart
>>
>>>
>>> Matt
>>>
>>>>     the GuC subregion the only one that we'd need to program in the
>>>> KMD?
>>>>
>>>> If we're moving the size(s) into the xe_device structure, then I
>>>> think
>>>> we'd expect the field to either contain the total size (which would
>>>> be
>>>> the values from max_wopcm_size rather than xe_wopcm_size) or we'd
>>>> expect
>>>> the field name + kerneldoc to very clearly explain what subset of
>>>> the
>>>> WOPCM it's actually trying to represent and what it can/should be
>>>> used
>>>> for.
>>>>
>>>> +Cc Daniele since I think he understands the WOPCM stuff a lot
>>>> better
>>>> than I do.
>>>>
>>>>
>>>> Matt
>>>>
>>>>>> This prepares for newer platforms that may need a different
>>>>>> size.
>>>>>> No functional change.
>>>>>>
>>>>>> Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
>>>>>> ---
>>>>>> Cc: Stuart Summers <stuart.summers@intel.com>
>>>>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/xe/xe_device_types.h |  2 ++
>>>>>>   drivers/gpu/drm/xe/xe_pci.c          | 15 ++++++++++++++-
>>>>>>   drivers/gpu/drm/xe/xe_pci_types.h    |  1 +
>>>>>>   drivers/gpu/drm/xe/xe_wopcm.c        | 11 +----------
>>>>>>   4 files changed, 18 insertions(+), 11 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h
>>>>>> b/drivers/gpu/drm/xe/xe_device_types.h
>>>>>> index 32dd2ffbc796..f44c031e3553 100644
>>>>>> --- a/drivers/gpu/drm/xe/xe_device_types.h
>>>>>> +++ b/drivers/gpu/drm/xe/xe_device_types.h
>>>>>> @@ -139,6 +139,8 @@ struct xe_device {
>>>>>>                  u8 vm_max_level;
>>>>>>                  /** @info.va_bits: Maximum bits of a virtual
>>>>>> address */
>>>>>>                  u8 va_bits;
>>>>>> +               /** @info.wopcm_size: Size of WOPCM region for
>>>>>> this platform */
>>>>>> +               u32 wopcm_size;
>>>>>>   
>>>>>>                  /*
>>>>>>                   * Keep all flags below alphabetically sorted
>>>>>> diff --git a/drivers/gpu/drm/xe/xe_pci.c
>>>>>> b/drivers/gpu/drm/xe/xe_pci.c
>>>>>> index 3165686e3e04..6b7dd8661f64 100644
>>>>>> --- a/drivers/gpu/drm/xe/xe_pci.c
>>>>>> +++ b/drivers/gpu/drm/xe/xe_pci.c
>>>>>> @@ -10,6 +10,7 @@
>>>>>>   #include <linux/module.h>
>>>>>>   #include <linux/pci.h>
>>>>>>   #include <linux/pm_runtime.h>
>>>>>> +#include <linux/sizes.h>
>>>>>>   
>>>>>>   #include <drm/drm_color_mgmt.h>
>>>>>>   #include <drm/drm_drv.h>
>>>>>> @@ -206,6 +207,7 @@ static const struct xe_device_desc tgl_desc
>>>>>> = {
>>>>>>          .require_force_probe = true,
>>>>>>          .va_bits = 48,
>>>>>>          .vm_max_level = 3,
>>>>>> +       .wopcm_size = SZ_2M,
>>>>>>   };
>>>>>>   
>>>>>>   static const struct xe_device_desc rkl_desc = {
>>>>>> @@ -221,6 +223,7 @@ static const struct xe_device_desc rkl_desc
>>>>>> = {
>>>>>>          .require_force_probe = true,
>>>>>>          .va_bits = 48,
>>>>>>          .vm_max_level = 3,
>>>>>> +       .wopcm_size = SZ_2M,
>>>>>>   };
>>>>>>   
>>>>>>   static const u16 adls_rpls_ids[] = { INTEL_RPLS_IDS(NOP), 0 };
>>>>>> @@ -243,6 +246,7 @@ static const struct xe_device_desc
>>>>>> adl_s_desc = {
>>>>>>          },
>>>>>>          .va_bits = 48,
>>>>>>          .vm_max_level = 3,
>>>>>> +       .wopcm_size = SZ_2M,
>>>>>>   };
>>>>>>   
>>>>>>   static const u16 adlp_rplu_ids[] = { INTEL_RPLU_IDS(NOP), 0 };
>>>>>> @@ -265,6 +269,7 @@ static const struct xe_device_desc
>>>>>> adl_p_desc = {
>>>>>>          },
>>>>>>          .va_bits = 48,
>>>>>>          .vm_max_level = 3,
>>>>>> +       .wopcm_size = SZ_2M,
>>>>>>   };
>>>>>>   
>>>>>>   static const struct xe_device_desc adl_n_desc = {
>>>>>> @@ -281,10 +286,12 @@ static const struct xe_device_desc
>>>>>> adl_n_desc = {
>>>>>>          .require_force_probe = true,
>>>>>>          .va_bits = 48,
>>>>>>          .vm_max_level = 3,
>>>>>> +       .wopcm_size = SZ_2M,
>>>>>>   };
>>>>>>   
>>>>>>   #define DGFX_FEATURES \
>>>>>> -       .is_dgfx = 1
>>>>>> +       .is_dgfx = 1, \
>>>>>> +       .wopcm_size = SZ_4M
>>>>>>   
>>>>>>   static const struct xe_device_desc dg1_desc = {
>>>>>>          .pre_gmdid_graphics_ip = &graphics_ip_xelpp,
>>>>>> @@ -378,6 +385,7 @@ static const struct xe_device_desc mtl_desc
>>>>>> = {
>>>>>>          MULTI_LRC_MASK,
>>>>>>          .va_bits = 48,
>>>>>>          .vm_max_level = 3,
>>>>>> +       .wopcm_size = SZ_4M,
>>>>>>   };
>>>>>>   
>>>>>>   static const struct xe_device_desc lnl_desc = {
>>>>>> @@ -391,6 +399,7 @@ static const struct xe_device_desc lnl_desc
>>>>>> = {
>>>>>>          .needs_scratch = true,
>>>>>>          .va_bits = 48,
>>>>>>          .vm_max_level = 4,
>>>>>> +       .wopcm_size = SZ_2M,
>>>>>>   };
>>>>>>   
>>>>>>   static const u16 bmg_g21_ids[] = { INTEL_BMG_G21_IDS(NOP), 0
>>>>>> };
>>>>>> @@ -436,6 +445,7 @@ static const struct xe_device_desc ptl_desc
>>>>>> = {
>>>>>>          .needs_shared_vf_gt_wq = true,
>>>>>>          .va_bits = 48,
>>>>>>          .vm_max_level = 4,
>>>>>> +       .wopcm_size = SZ_2M,
>>>>>>   };
>>>>>>   
>>>>>>   static const struct xe_device_desc nvls_desc = {
>>>>>> @@ -450,6 +460,7 @@ static const struct xe_device_desc
>>>>>> nvls_desc = {
>>>>>>          .require_force_probe = true,
>>>>>>          .va_bits = 48,
>>>>>>          .vm_max_level = 4,
>>>>>> +       .wopcm_size = SZ_2M,
>>>>>>   };
>>>>>>   
>>>>>>   static const struct xe_device_desc cri_desc = {
>>>>>> @@ -489,6 +500,7 @@ static const struct xe_device_desc
>>>>>> nvlp_desc = {
>>>>>>          .require_force_probe = true,
>>>>>>          .va_bits = 48,
>>>>>>          .vm_max_level = 4,
>>>>>> +       .wopcm_size = SZ_2M,
>>>>>>   };
>>>>>>   
>>>>>>   #undef PLATFORM
>>>>>> @@ -757,6 +769,7 @@ static int xe_info_init_early(struct
>>>>>> xe_device *xe,
>>>>>>          xe->info.va_bits = desc->va_bits;
>>>>>>          xe->info.vm_max_level = desc->vm_max_level;
>>>>>>          xe->info.vram_flags = desc->vram_flags;
>>>>>> +       xe->info.wopcm_size = desc->wopcm_size;
>>>>>>   
>>>>>>          xe->info.is_dgfx = desc->is_dgfx;
>>>>>>          xe->info.has_cached_pt = desc->has_cached_pt;
>>>>>> diff --git a/drivers/gpu/drm/xe/xe_pci_types.h
>>>>>> b/drivers/gpu/drm/xe/xe_pci_types.h
>>>>>> index 5b85e2c24b7b..35acb878c111 100644
>>>>>> --- a/drivers/gpu/drm/xe/xe_pci_types.h
>>>>>> +++ b/drivers/gpu/drm/xe/xe_pci_types.h
>>>>>> @@ -34,6 +34,7 @@ struct xe_device_desc {
>>>>>>          u8 va_bits;
>>>>>>          u8 vm_max_level;
>>>>>>          u8 vram_flags;
>>>>>> +       u32 wopcm_size;
>>>>>>   
>>>>>>          u8 require_force_probe:1;
>>>>>>          u8 is_dgfx:1;
>>>>>> diff --git a/drivers/gpu/drm/xe/xe_wopcm.c
>>>>>> b/drivers/gpu/drm/xe/xe_wopcm.c
>>>>>> index 900daf1d1b1b..783bf37c87ef 100644
>>>>>> --- a/drivers/gpu/drm/xe/xe_wopcm.c
>>>>>> +++ b/drivers/gpu/drm/xe/xe_wopcm.c
>>>>>> @@ -48,13 +48,6 @@
>>>>>>    * context).
>>>>>>    */
>>>>>>   
>>>>>> -/* Default WOPCM size is 2MB from Gen11, 1MB on previous
>>>>>> platforms */
>>>>>> -/* FIXME: Larger size require for 2 tile PVC, do a proper
>>>>>> probe sooner or later */
>>>>>> -#define DGFX_WOPCM_SIZE                        SZ_4M
>>>>>> -/* FIXME: Larger size require for MTL, do a proper probe
>>>>>> sooner or later */
>>>>>> -#define MTL_WOPCM_SIZE                 SZ_4M
>>>>>> -#define WOPCM_SIZE                     SZ_2M
>>>>>> -
>>>>>>   /* 16KB WOPCM (RSVD WOPCM) is reserved from HuC firmware top.
>>>>>> */
>>>>>>   #define WOPCM_RESERVED_SIZE            SZ_16K
>>>>>>   
>>>>>> @@ -179,9 +172,7 @@ static int __wopcm_init_regs(struct
>>>>>> xe_device *xe, struct xe_gt *gt,
>>>>>>   
>>>>> while around, can you add kernel-doc here?
>>>>>
>>>>>>   u32 xe_wopcm_size(struct xe_device *xe)
>>>>>>   {
>>>>>> -       return IS_DGFX(xe) ? DGFX_WOPCM_SIZE :
>>>>>> -               xe->info.platform == XE_METEORLAKE ?
>>>>>> MTL_WOPCM_SIZE :
>>>>>> -               WOPCM_SIZE;
>>>>>> +       return xe->info.wopcm_size;
>>>>>>   }
>>>>>>   
>>>>>>   static u32 max_wopcm_size(struct xe_device *xe)
>>>> -- 
>>>> Matt Roper
>>>> Graphics Software Engineer
>>>> Linux GPU Platform Enablement
>>>> Intel Corporation