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

["Summers, Stuart" <stuart.summers@intel.com>]

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?

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
>