[PATCH 0/2] Refactor some graphics descriptor flags

[PATCH 0/2] Refactor some graphics descriptor flags

[Sai Teja Pottumuttu]

Refactor dma_mask_size and max_remote_tiles and place them in
the platform descriptor as they are more related to the platform
than the graphics IP.

Sai Teja Pottumuttu (2):
  drm/xe: Refactor dma_mask_size
  drm/xe: Refactor max_remote_tiles

 drivers/gpu/drm/xe/xe_pci.c       | 31 +++++++++++++++++++------------
 drivers/gpu/drm/xe/xe_pci_types.h |  3 ---
 2 files changed, 19 insertions(+), 15 deletions(-)

-- 
2.34.1

[PATCH 1/2] drm/xe: Refactor dma_mask_size

[Sai Teja Pottumuttu]

dma_mask_size is more related to the platform than the GT IP. Thus
move it to platform descriptors.

Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com>
---
 drivers/gpu/drm/xe/xe_pci.c       | 22 ++++++++++++++++------
 drivers/gpu/drm/xe/xe_pci_types.h |  1 -
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index 48d1c81d441e..f59c1acae23f 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -54,6 +54,8 @@ struct xe_device_desc {
 
 	enum xe_platform platform;
 
+	u8 dma_mask_size;
+
 	u8 require_force_probe:1;
 	u8 is_dgfx:1;
 
@@ -83,7 +85,6 @@ static const struct xe_graphics_desc graphics_xelp = {
 
 	.hw_engine_mask = BIT(XE_HW_ENGINE_RCS0) | BIT(XE_HW_ENGINE_BCS0),
 
-	.dma_mask_size = 39,
 	.va_bits = 48,
 	.vm_max_level = 3,
 };
@@ -95,14 +96,12 @@ static const struct xe_graphics_desc graphics_xelpp = {
 
 	.hw_engine_mask = BIT(XE_HW_ENGINE_RCS0) | BIT(XE_HW_ENGINE_BCS0),
 
-	.dma_mask_size = 39,
 	.va_bits = 48,
 	.vm_max_level = 3,
 };
 
 #define XE_HP_FEATURES \
 	.has_range_tlb_invalidation = true, \
-	.dma_mask_size = 46, \
 	.va_bits = 48, \
 	.vm_max_level = 3
 
@@ -137,7 +136,6 @@ static const struct xe_graphics_desc graphics_xehpc = {
 		BIT(XE_HW_ENGINE_CCS2) | BIT(XE_HW_ENGINE_CCS3),
 
 	XE_HP_FEATURES,
-	.dma_mask_size = 52,
 	.max_remote_tiles = 1,
 	.va_bits = 57,
 	.vm_max_level = 4,
@@ -158,7 +156,6 @@ static const struct xe_graphics_desc graphics_xelpg = {
 };
 
 #define XE2_GFX_FEATURES \
-	.dma_mask_size = 46, \
 	.has_asid = 1, \
 	.has_atomic_enable_pte_bit = 1, \
 	.has_flat_ccs = 1, \
@@ -218,6 +215,7 @@ static const struct xe_device_desc tgl_desc = {
 	.graphics = &graphics_xelp,
 	.media = &media_xem,
 	PLATFORM(TIGERLAKE),
+	.dma_mask_size = 39,
 	.has_display = true,
 	.has_llc = true,
 	.require_force_probe = true,
@@ -227,6 +225,7 @@ static const struct xe_device_desc rkl_desc = {
 	.graphics = &graphics_xelp,
 	.media = &media_xem,
 	PLATFORM(ROCKETLAKE),
+	.dma_mask_size = 39,
 	.has_display = true,
 	.has_llc = true,
 	.require_force_probe = true,
@@ -238,6 +237,7 @@ static const struct xe_device_desc adl_s_desc = {
 	.graphics = &graphics_xelp,
 	.media = &media_xem,
 	PLATFORM(ALDERLAKE_S),
+	.dma_mask_size = 39,
 	.has_display = true,
 	.has_llc = true,
 	.require_force_probe = true,
@@ -253,6 +253,7 @@ static const struct xe_device_desc adl_p_desc = {
 	.graphics = &graphics_xelp,
 	.media = &media_xem,
 	PLATFORM(ALDERLAKE_P),
+	.dma_mask_size = 39,
 	.has_display = true,
 	.has_llc = true,
 	.require_force_probe = true,
@@ -266,6 +267,7 @@ static const struct xe_device_desc adl_n_desc = {
 	.graphics = &graphics_xelp,
 	.media = &media_xem,
 	PLATFORM(ALDERLAKE_N),
+	.dma_mask_size = 39,
 	.has_display = true,
 	.has_llc = true,
 	.require_force_probe = true,
@@ -279,6 +281,7 @@ static const struct xe_device_desc dg1_desc = {
 	.media = &media_xem,
 	DGFX_FEATURES,
 	PLATFORM(DG1),
+	.dma_mask_size = 39,
 	.has_display = true,
 	.has_heci_gscfi = 1,
 	.require_force_probe = true,
@@ -302,6 +305,7 @@ static const u16 dg2_g12_ids[] = { INTEL_DG2_G12_IDS(NOP), 0 };
 static const struct xe_device_desc ats_m_desc = {
 	.graphics = &graphics_xehpg,
 	.media = &media_xehpm,
+	.dma_mask_size = 46,
 	.require_force_probe = true,
 
 	DG2_FEATURES,
@@ -311,6 +315,7 @@ static const struct xe_device_desc ats_m_desc = {
 static const struct xe_device_desc dg2_desc = {
 	.graphics = &graphics_xehpg,
 	.media = &media_xehpm,
+	.dma_mask_size = 46,
 	.require_force_probe = true,
 
 	DG2_FEATURES,
@@ -321,6 +326,7 @@ static const __maybe_unused struct xe_device_desc pvc_desc = {
 	.graphics = &graphics_xehpc,
 	DGFX_FEATURES,
 	PLATFORM(PVC),
+	.dma_mask_size = 52,
 	.has_display = false,
 	.has_heci_gscfi = 1,
 	.require_force_probe = true,
@@ -330,23 +336,27 @@ static const struct xe_device_desc mtl_desc = {
 	/* .graphics and .media determined via GMD_ID */
 	.require_force_probe = true,
 	PLATFORM(METEORLAKE),
+	.dma_mask_size = 46,
 	.has_display = true,
 };
 
 static const struct xe_device_desc lnl_desc = {
 	PLATFORM(LUNARLAKE),
+	.dma_mask_size = 46,
 	.has_display = true,
 };
 
 static const struct xe_device_desc bmg_desc = {
 	DGFX_FEATURES,
 	PLATFORM(BATTLEMAGE),
+	.dma_mask_size = 46,
 	.has_display = true,
 	.has_heci_cscfi = 1,
 };
 
 static const struct xe_device_desc ptl_desc = {
 	PLATFORM(PANTHERLAKE),
+	.dma_mask_size = 46,
 	.has_display = true,
 	.require_force_probe = true,
 };
@@ -612,6 +622,7 @@ static int xe_info_init_early(struct xe_device *xe,
 	xe->info.subplatform = subplatform_desc ?
 		subplatform_desc->subplatform : XE_SUBPLATFORM_NONE;
 
+	xe->info.dma_mask_size = desc->dma_mask_size;
 	xe->info.is_dgfx = desc->is_dgfx;
 	xe->info.has_heci_gscfi = desc->has_heci_gscfi;
 	xe->info.has_heci_cscfi = desc->has_heci_cscfi;
@@ -676,7 +687,6 @@ static int xe_info_init(struct xe_device *xe,
 	xe->info.graphics_name = graphics_desc->name;
 	xe->info.media_name = media_desc ? media_desc->name : "none";
 
-	xe->info.dma_mask_size = graphics_desc->dma_mask_size;
 	xe->info.vram_flags = graphics_desc->vram_flags;
 	xe->info.va_bits = graphics_desc->va_bits;
 	xe->info.vm_max_level = graphics_desc->vm_max_level;
diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
index 873efec5cdee..7437415a54d5 100644
--- a/drivers/gpu/drm/xe/xe_pci_types.h
+++ b/drivers/gpu/drm/xe/xe_pci_types.h
@@ -13,7 +13,6 @@ struct xe_graphics_desc {
 	u8 ver;
 	u8 rel;
 
-	u8 dma_mask_size;	/* available DMA address bits */
 	u8 va_bits;
 	u8 vm_max_level;
 	u8 vram_flags;
-- 
2.34.1

Re: [PATCH 1/2] drm/xe: Refactor dma_mask_size

[Raag Jadav]

On Thu, Jan 16, 2025 at 06:24:46PM +0530, Sai Teja Pottumuttu wrote:
> dma_mask_size is more related to the platform than the GT IP.

Nit: Unless there's uncertainty intended here, I'd drop 'more' and
make it definitive.

Raag

Re: [PATCH 1/2] drm/xe: Refactor dma_mask_size

[Summers, Stuart]

On Thu, 2025-01-16 at 18:24 +0530, Sai Teja Pottumuttu wrote:
> dma_mask_size is more related to the platform than the GT IP. Thus
> move it to platform descriptors.

Can you give some more reasoning for why you think this needs to
change? The phys address bits are specific to the graphics IP
typically. Are you expecting this to be different for different
platform SKUs?

Thanks,
Stuart

> 
> Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pci.c       | 22 ++++++++++++++++------
>  drivers/gpu/drm/xe/xe_pci_types.h |  1 -
>  2 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pci.c
> b/drivers/gpu/drm/xe/xe_pci.c
> index 48d1c81d441e..f59c1acae23f 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -54,6 +54,8 @@ struct xe_device_desc {
>  
>         enum xe_platform platform;
>  
> +       u8 dma_mask_size;
> +
>         u8 require_force_probe:1;
>         u8 is_dgfx:1;
>  
> @@ -83,7 +85,6 @@ static const struct xe_graphics_desc graphics_xelp
> = {
>  
>         .hw_engine_mask = BIT(XE_HW_ENGINE_RCS0) |
> BIT(XE_HW_ENGINE_BCS0),
>  
> -       .dma_mask_size = 39,
>         .va_bits = 48,
>         .vm_max_level = 3,
>  };
> @@ -95,14 +96,12 @@ static const struct xe_graphics_desc
> graphics_xelpp = {
>  
>         .hw_engine_mask = BIT(XE_HW_ENGINE_RCS0) |
> BIT(XE_HW_ENGINE_BCS0),
>  
> -       .dma_mask_size = 39,
>         .va_bits = 48,
>         .vm_max_level = 3,
>  };
>  
>  #define XE_HP_FEATURES \
>         .has_range_tlb_invalidation = true, \
> -       .dma_mask_size = 46, \
>         .va_bits = 48, \
>         .vm_max_level = 3
>  
> @@ -137,7 +136,6 @@ static const struct xe_graphics_desc
> graphics_xehpc = {
>                 BIT(XE_HW_ENGINE_CCS2) | BIT(XE_HW_ENGINE_CCS3),
>  
>         XE_HP_FEATURES,
> -       .dma_mask_size = 52,
>         .max_remote_tiles = 1,
>         .va_bits = 57,
>         .vm_max_level = 4,
> @@ -158,7 +156,6 @@ static const struct xe_graphics_desc
> graphics_xelpg = {
>  };
>  
>  #define XE2_GFX_FEATURES \
> -       .dma_mask_size = 46, \
>         .has_asid = 1, \
>         .has_atomic_enable_pte_bit = 1, \
>         .has_flat_ccs = 1, \
> @@ -218,6 +215,7 @@ static const struct xe_device_desc tgl_desc = {
>         .graphics = &graphics_xelp,
>         .media = &media_xem,
>         PLATFORM(TIGERLAKE),
> +       .dma_mask_size = 39,
>         .has_display = true,
>         .has_llc = true,
>         .require_force_probe = true,
> @@ -227,6 +225,7 @@ static const struct xe_device_desc rkl_desc = {
>         .graphics = &graphics_xelp,
>         .media = &media_xem,
>         PLATFORM(ROCKETLAKE),
> +       .dma_mask_size = 39,
>         .has_display = true,
>         .has_llc = true,
>         .require_force_probe = true,
> @@ -238,6 +237,7 @@ static const struct xe_device_desc adl_s_desc = {
>         .graphics = &graphics_xelp,
>         .media = &media_xem,
>         PLATFORM(ALDERLAKE_S),
> +       .dma_mask_size = 39,
>         .has_display = true,
>         .has_llc = true,
>         .require_force_probe = true,
> @@ -253,6 +253,7 @@ static const struct xe_device_desc adl_p_desc = {
>         .graphics = &graphics_xelp,
>         .media = &media_xem,
>         PLATFORM(ALDERLAKE_P),
> +       .dma_mask_size = 39,
>         .has_display = true,
>         .has_llc = true,
>         .require_force_probe = true,
> @@ -266,6 +267,7 @@ static const struct xe_device_desc adl_n_desc = {
>         .graphics = &graphics_xelp,
>         .media = &media_xem,
>         PLATFORM(ALDERLAKE_N),
> +       .dma_mask_size = 39,
>         .has_display = true,
>         .has_llc = true,
>         .require_force_probe = true,
> @@ -279,6 +281,7 @@ static const struct xe_device_desc dg1_desc = {
>         .media = &media_xem,
>         DGFX_FEATURES,
>         PLATFORM(DG1),
> +       .dma_mask_size = 39,
>         .has_display = true,
>         .has_heci_gscfi = 1,
>         .require_force_probe = true,
> @@ -302,6 +305,7 @@ static const u16 dg2_g12_ids[] = {
> INTEL_DG2_G12_IDS(NOP), 0 };
>  static const struct xe_device_desc ats_m_desc = {
>         .graphics = &graphics_xehpg,
>         .media = &media_xehpm,
> +       .dma_mask_size = 46,
>         .require_force_probe = true,
>  
>         DG2_FEATURES,
> @@ -311,6 +315,7 @@ static const struct xe_device_desc ats_m_desc = {
>  static const struct xe_device_desc dg2_desc = {
>         .graphics = &graphics_xehpg,
>         .media = &media_xehpm,
> +       .dma_mask_size = 46,
>         .require_force_probe = true,
>  
>         DG2_FEATURES,
> @@ -321,6 +326,7 @@ static const __maybe_unused struct xe_device_desc
> pvc_desc = {
>         .graphics = &graphics_xehpc,
>         DGFX_FEATURES,
>         PLATFORM(PVC),
> +       .dma_mask_size = 52,
>         .has_display = false,
>         .has_heci_gscfi = 1,
>         .require_force_probe = true,
> @@ -330,23 +336,27 @@ static const struct xe_device_desc mtl_desc = {
>         /* .graphics and .media determined via GMD_ID */
>         .require_force_probe = true,
>         PLATFORM(METEORLAKE),
> +       .dma_mask_size = 46,
>         .has_display = true,
>  };
>  
>  static const struct xe_device_desc lnl_desc = {
>         PLATFORM(LUNARLAKE),
> +       .dma_mask_size = 46,
>         .has_display = true,
>  };
>  
>  static const struct xe_device_desc bmg_desc = {
>         DGFX_FEATURES,
>         PLATFORM(BATTLEMAGE),
> +       .dma_mask_size = 46,
>         .has_display = true,
>         .has_heci_cscfi = 1,
>  };
>  
>  static const struct xe_device_desc ptl_desc = {
>         PLATFORM(PANTHERLAKE),
> +       .dma_mask_size = 46,
>         .has_display = true,
>         .require_force_probe = true,
>  };
> @@ -612,6 +622,7 @@ static int xe_info_init_early(struct xe_device
> *xe,
>         xe->info.subplatform = subplatform_desc ?
>                 subplatform_desc->subplatform : XE_SUBPLATFORM_NONE;
>  
> +       xe->info.dma_mask_size = desc->dma_mask_size;
>         xe->info.is_dgfx = desc->is_dgfx;
>         xe->info.has_heci_gscfi = desc->has_heci_gscfi;
>         xe->info.has_heci_cscfi = desc->has_heci_cscfi;
> @@ -676,7 +687,6 @@ static int xe_info_init(struct xe_device *xe,
>         xe->info.graphics_name = graphics_desc->name;
>         xe->info.media_name = media_desc ? media_desc->name : "none";
>  
> -       xe->info.dma_mask_size = graphics_desc->dma_mask_size;
>         xe->info.vram_flags = graphics_desc->vram_flags;
>         xe->info.va_bits = graphics_desc->va_bits;
>         xe->info.vm_max_level = graphics_desc->vm_max_level;
> diff --git a/drivers/gpu/drm/xe/xe_pci_types.h
> b/drivers/gpu/drm/xe/xe_pci_types.h
> index 873efec5cdee..7437415a54d5 100644
> --- a/drivers/gpu/drm/xe/xe_pci_types.h
> +++ b/drivers/gpu/drm/xe/xe_pci_types.h
> @@ -13,7 +13,6 @@ struct xe_graphics_desc {
>         u8 ver;
>         u8 rel;
>  
> -       u8 dma_mask_size;       /* available DMA address bits */
>         u8 va_bits;
>         u8 vm_max_level;
>         u8 vram_flags;

Re: [PATCH 1/2] drm/xe: Refactor dma_mask_size

[Matt Roper]

On Thu, Jan 16, 2025 at 11:10:22AM -0800, Summers, Stuart wrote:
> On Thu, 2025-01-16 at 18:24 +0530, Sai Teja Pottumuttu wrote:
> > dma_mask_size is more related to the platform than the GT IP. Thus
> > move it to platform descriptors.
> 
> Can you give some more reasoning for why you think this needs to
> change? The phys address bits are specific to the graphics IP
> typically. Are you expecting this to be different for different
> platform SKUs?

I don't know if I agree with that.  The graphics version (as read from
the GT's GMD_ID register) is really concerned with the stuff inside the
primary GT, not the GPU as a whole.  The physical address size is more
related to the platform itself, and concepts like the GTT that live
outside the GT at the tile level.  So in my mind this seems like a
correct refactor from a design perspective, even if it's not super
critical to anything today.

I don't know if we'll ever actually try to do it, but there have been
suggestions in the past that it might be useful to be able to load the
driver with the primary GT uninitialized/untouched on an igpu[*] (e.g.,
if you have defective hardware/firmware and don't want to touch the
graphics, but still want to use display + media).  It would definitely
require a fair amount of driver work to make that possible, but patches
like this would be an important step since we'd have a graphics version
of 0.0 when running in such a mode.  The media and display IP would
still be using the GTT, and we should be able to determine the
characteristics of the address spaces from the platform itself rather
than relying on inspection of the primary GT.

[*] Doing this on a dgpu would be significantly harder than an igpu
    since we generally want to use the BCS engines inside the primary GT
    for vram fills/copies.


Matt

> 
> Thanks,
> Stuart
> 
> > 
> > Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_pci.c       | 22 ++++++++++++++++------
> >  drivers/gpu/drm/xe/xe_pci_types.h |  1 -
> >  2 files changed, 16 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pci.c
> > b/drivers/gpu/drm/xe/xe_pci.c
> > index 48d1c81d441e..f59c1acae23f 100644
> > --- a/drivers/gpu/drm/xe/xe_pci.c
> > +++ b/drivers/gpu/drm/xe/xe_pci.c
> > @@ -54,6 +54,8 @@ struct xe_device_desc {
> >  
> >         enum xe_platform platform;
> >  
> > +       u8 dma_mask_size;
> > +
> >         u8 require_force_probe:1;
> >         u8 is_dgfx:1;
> >  
> > @@ -83,7 +85,6 @@ static const struct xe_graphics_desc graphics_xelp
> > = {
> >  
> >         .hw_engine_mask = BIT(XE_HW_ENGINE_RCS0) |
> > BIT(XE_HW_ENGINE_BCS0),
> >  
> > -       .dma_mask_size = 39,
> >         .va_bits = 48,
> >         .vm_max_level = 3,
> >  };
> > @@ -95,14 +96,12 @@ static const struct xe_graphics_desc
> > graphics_xelpp = {
> >  
> >         .hw_engine_mask = BIT(XE_HW_ENGINE_RCS0) |
> > BIT(XE_HW_ENGINE_BCS0),
> >  
> > -       .dma_mask_size = 39,
> >         .va_bits = 48,
> >         .vm_max_level = 3,
> >  };
> >  
> >  #define XE_HP_FEATURES \
> >         .has_range_tlb_invalidation = true, \
> > -       .dma_mask_size = 46, \
> >         .va_bits = 48, \
> >         .vm_max_level = 3
> >  
> > @@ -137,7 +136,6 @@ static const struct xe_graphics_desc
> > graphics_xehpc = {
> >                 BIT(XE_HW_ENGINE_CCS2) | BIT(XE_HW_ENGINE_CCS3),
> >  
> >         XE_HP_FEATURES,
> > -       .dma_mask_size = 52,
> >         .max_remote_tiles = 1,
> >         .va_bits = 57,
> >         .vm_max_level = 4,
> > @@ -158,7 +156,6 @@ static const struct xe_graphics_desc
> > graphics_xelpg = {
> >  };
> >  
> >  #define XE2_GFX_FEATURES \
> > -       .dma_mask_size = 46, \
> >         .has_asid = 1, \
> >         .has_atomic_enable_pte_bit = 1, \
> >         .has_flat_ccs = 1, \
> > @@ -218,6 +215,7 @@ static const struct xe_device_desc tgl_desc = {
> >         .graphics = &graphics_xelp,
> >         .media = &media_xem,
> >         PLATFORM(TIGERLAKE),
> > +       .dma_mask_size = 39,
> >         .has_display = true,
> >         .has_llc = true,
> >         .require_force_probe = true,
> > @@ -227,6 +225,7 @@ static const struct xe_device_desc rkl_desc = {
> >         .graphics = &graphics_xelp,
> >         .media = &media_xem,
> >         PLATFORM(ROCKETLAKE),
> > +       .dma_mask_size = 39,
> >         .has_display = true,
> >         .has_llc = true,
> >         .require_force_probe = true,
> > @@ -238,6 +237,7 @@ static const struct xe_device_desc adl_s_desc = {
> >         .graphics = &graphics_xelp,
> >         .media = &media_xem,
> >         PLATFORM(ALDERLAKE_S),
> > +       .dma_mask_size = 39,
> >         .has_display = true,
> >         .has_llc = true,
> >         .require_force_probe = true,
> > @@ -253,6 +253,7 @@ static const struct xe_device_desc adl_p_desc = {
> >         .graphics = &graphics_xelp,
> >         .media = &media_xem,
> >         PLATFORM(ALDERLAKE_P),
> > +       .dma_mask_size = 39,
> >         .has_display = true,
> >         .has_llc = true,
> >         .require_force_probe = true,
> > @@ -266,6 +267,7 @@ static const struct xe_device_desc adl_n_desc = {
> >         .graphics = &graphics_xelp,
> >         .media = &media_xem,
> >         PLATFORM(ALDERLAKE_N),
> > +       .dma_mask_size = 39,
> >         .has_display = true,
> >         .has_llc = true,
> >         .require_force_probe = true,
> > @@ -279,6 +281,7 @@ static const struct xe_device_desc dg1_desc = {
> >         .media = &media_xem,
> >         DGFX_FEATURES,
> >         PLATFORM(DG1),
> > +       .dma_mask_size = 39,
> >         .has_display = true,
> >         .has_heci_gscfi = 1,
> >         .require_force_probe = true,
> > @@ -302,6 +305,7 @@ static const u16 dg2_g12_ids[] = {
> > INTEL_DG2_G12_IDS(NOP), 0 };
> >  static const struct xe_device_desc ats_m_desc = {
> >         .graphics = &graphics_xehpg,
> >         .media = &media_xehpm,
> > +       .dma_mask_size = 46,
> >         .require_force_probe = true,
> >  
> >         DG2_FEATURES,
> > @@ -311,6 +315,7 @@ static const struct xe_device_desc ats_m_desc = {
> >  static const struct xe_device_desc dg2_desc = {
> >         .graphics = &graphics_xehpg,
> >         .media = &media_xehpm,
> > +       .dma_mask_size = 46,
> >         .require_force_probe = true,
> >  
> >         DG2_FEATURES,
> > @@ -321,6 +326,7 @@ static const __maybe_unused struct xe_device_desc
> > pvc_desc = {
> >         .graphics = &graphics_xehpc,
> >         DGFX_FEATURES,
> >         PLATFORM(PVC),
> > +       .dma_mask_size = 52,
> >         .has_display = false,
> >         .has_heci_gscfi = 1,
> >         .require_force_probe = true,
> > @@ -330,23 +336,27 @@ static const struct xe_device_desc mtl_desc = {
> >         /* .graphics and .media determined via GMD_ID */
> >         .require_force_probe = true,
> >         PLATFORM(METEORLAKE),
> > +       .dma_mask_size = 46,
> >         .has_display = true,
> >  };
> >  
> >  static const struct xe_device_desc lnl_desc = {
> >         PLATFORM(LUNARLAKE),
> > +       .dma_mask_size = 46,
> >         .has_display = true,
> >  };
> >  
> >  static const struct xe_device_desc bmg_desc = {
> >         DGFX_FEATURES,
> >         PLATFORM(BATTLEMAGE),
> > +       .dma_mask_size = 46,
> >         .has_display = true,
> >         .has_heci_cscfi = 1,
> >  };
> >  
> >  static const struct xe_device_desc ptl_desc = {
> >         PLATFORM(PANTHERLAKE),
> > +       .dma_mask_size = 46,
> >         .has_display = true,
> >         .require_force_probe = true,
> >  };
> > @@ -612,6 +622,7 @@ static int xe_info_init_early(struct xe_device
> > *xe,
> >         xe->info.subplatform = subplatform_desc ?
> >                 subplatform_desc->subplatform : XE_SUBPLATFORM_NONE;
> >  
> > +       xe->info.dma_mask_size = desc->dma_mask_size;
> >         xe->info.is_dgfx = desc->is_dgfx;
> >         xe->info.has_heci_gscfi = desc->has_heci_gscfi;
> >         xe->info.has_heci_cscfi = desc->has_heci_cscfi;
> > @@ -676,7 +687,6 @@ static int xe_info_init(struct xe_device *xe,
> >         xe->info.graphics_name = graphics_desc->name;
> >         xe->info.media_name = media_desc ? media_desc->name : "none";
> >  
> > -       xe->info.dma_mask_size = graphics_desc->dma_mask_size;
> >         xe->info.vram_flags = graphics_desc->vram_flags;
> >         xe->info.va_bits = graphics_desc->va_bits;
> >         xe->info.vm_max_level = graphics_desc->vm_max_level;
> > diff --git a/drivers/gpu/drm/xe/xe_pci_types.h
> > b/drivers/gpu/drm/xe/xe_pci_types.h
> > index 873efec5cdee..7437415a54d5 100644
> > --- a/drivers/gpu/drm/xe/xe_pci_types.h
> > +++ b/drivers/gpu/drm/xe/xe_pci_types.h
> > @@ -13,7 +13,6 @@ struct xe_graphics_desc {
> >         u8 ver;
> >         u8 rel;
> >  
> > -       u8 dma_mask_size;       /* available DMA address bits */
> >         u8 va_bits;
> >         u8 vm_max_level;
> >         u8 vram_flags;
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

[PATCH 2/2] drm/xe: Refactor max_remote_tiles

[Sai Teja Pottumuttu]

max_remote_tiles is more related to the platform than the GT IP. Thus
move it to platform descriptor from graphics descriptor. Note that the
FIXME is no more required, thus it can be dropped.

Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com>
---
 drivers/gpu/drm/xe/xe_pci.c       | 9 +++------
 drivers/gpu/drm/xe/xe_pci_types.h | 2 --
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index f59c1acae23f..7f17e5ea6edd 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -55,6 +55,7 @@ struct xe_device_desc {
 	enum xe_platform platform;
 
 	u8 dma_mask_size;
+	u8 max_remote_tiles:2;
 
 	u8 require_force_probe:1;
 	u8 is_dgfx:1;
@@ -136,7 +137,6 @@ static const struct xe_graphics_desc graphics_xehpc = {
 		BIT(XE_HW_ENGINE_CCS2) | BIT(XE_HW_ENGINE_CCS3),
 
 	XE_HP_FEATURES,
-	.max_remote_tiles = 1,
 	.va_bits = 57,
 	.vm_max_level = 4,
 	.vram_flags = XE_VRAM_FLAGS_NEED64K,
@@ -329,6 +329,7 @@ static const __maybe_unused struct xe_device_desc pvc_desc = {
 	.dma_mask_size = 52,
 	.has_display = false,
 	.has_heci_gscfi = 1,
+	.max_remote_tiles = 1,
 	.require_force_probe = true,
 };
 
@@ -635,6 +636,7 @@ static int xe_info_init_early(struct xe_device *xe,
 	xe->info.probe_display = IS_ENABLED(CONFIG_DRM_XE_DISPLAY) &&
 				 xe_modparam.probe_display &&
 				 desc->has_display;
+	xe->info.tile_count = 1 + desc->max_remote_tiles;
 
 	err = xe_tile_init_early(xe_device_get_root_tile(xe), xe, 0);
 	if (err)
@@ -706,12 +708,7 @@ static int xe_info_init(struct xe_device *xe,
 	 * version 13 or higher has an additional dedicated media GT.  And
 	 * depending on the graphics IP there may be additional "remote tiles."
 	 * All of these together determine the overall GT count.
-	 *
-	 * FIXME: 'tile_count' here is misnamed since the rest of the driver
-	 * treats it as the number of GTs rather than just the number of tiles.
 	 */
-	xe->info.tile_count = 1 + graphics_desc->max_remote_tiles;
-
 	for_each_remote_tile(tile, xe, id) {
 		int err;
 
diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
index 7437415a54d5..b96423844952 100644
--- a/drivers/gpu/drm/xe/xe_pci_types.h
+++ b/drivers/gpu/drm/xe/xe_pci_types.h
@@ -19,8 +19,6 @@ struct xe_graphics_desc {
 
 	u64 hw_engine_mask;	/* hardware engines provided by graphics IP */
 
-	u8 max_remote_tiles:2;
-
 	u8 has_asid:1;
 	u8 has_atomic_enable_pte_bit:1;
 	u8 has_flat_ccs:1;
-- 
2.34.1

Re: [PATCH 2/2] drm/xe: Refactor max_remote_tiles

[Summers, Stuart]

On Thu, 2025-01-16 at 18:24 +0530, Sai Teja Pottumuttu wrote:
> max_remote_tiles is more related to the platform than the GT IP. Thus
> move it to platform descriptor from graphics descriptor. Note that
> the
> FIXME is no more required, thus it can be dropped.

I think we still want this FIXME. Has anything fundamentally changed in
the driver wrt tiles vs gts?

Thanks,
Stuart

> 
> Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pci.c       | 9 +++------
>  drivers/gpu/drm/xe/xe_pci_types.h | 2 --
>  2 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pci.c
> b/drivers/gpu/drm/xe/xe_pci.c
> index f59c1acae23f..7f17e5ea6edd 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -55,6 +55,7 @@ struct xe_device_desc {
>         enum xe_platform platform;
>  
>         u8 dma_mask_size;
> +       u8 max_remote_tiles:2;
>  
>         u8 require_force_probe:1;
>         u8 is_dgfx:1;
> @@ -136,7 +137,6 @@ static const struct xe_graphics_desc
> graphics_xehpc = {
>                 BIT(XE_HW_ENGINE_CCS2) | BIT(XE_HW_ENGINE_CCS3),
>  
>         XE_HP_FEATURES,
> -       .max_remote_tiles = 1,
>         .va_bits = 57,
>         .vm_max_level = 4,
>         .vram_flags = XE_VRAM_FLAGS_NEED64K,
> @@ -329,6 +329,7 @@ static const __maybe_unused struct xe_device_desc
> pvc_desc = {
>         .dma_mask_size = 52,
>         .has_display = false,
>         .has_heci_gscfi = 1,
> +       .max_remote_tiles = 1,
>         .require_force_probe = true,
>  };
>  
> @@ -635,6 +636,7 @@ static int xe_info_init_early(struct xe_device
> *xe,
>         xe->info.probe_display = IS_ENABLED(CONFIG_DRM_XE_DISPLAY) &&
>                                  xe_modparam.probe_display &&
>                                  desc->has_display;
> +       xe->info.tile_count = 1 + desc->max_remote_tiles;
>  
>         err = xe_tile_init_early(xe_device_get_root_tile(xe), xe, 0);
>         if (err)
> @@ -706,12 +708,7 @@ static int xe_info_init(struct xe_device *xe,
>          * version 13 or higher has an additional dedicated media
> GT.  And
>          * depending on the graphics IP there may be additional
> "remote tiles."
>          * All of these together determine the overall GT count.
> -        *
> -        * FIXME: 'tile_count' here is misnamed since the rest of the
> driver
> -        * treats it as the number of GTs rather than just the number
> of tiles.
>          */
> -       xe->info.tile_count = 1 + graphics_desc->max_remote_tiles;
> -
>         for_each_remote_tile(tile, xe, id) {
>                 int err;
>  
> diff --git a/drivers/gpu/drm/xe/xe_pci_types.h
> b/drivers/gpu/drm/xe/xe_pci_types.h
> index 7437415a54d5..b96423844952 100644
> --- a/drivers/gpu/drm/xe/xe_pci_types.h
> +++ b/drivers/gpu/drm/xe/xe_pci_types.h
> @@ -19,8 +19,6 @@ struct xe_graphics_desc {
>  
>         u64 hw_engine_mask;     /* hardware engines provided by
> graphics IP */
>  
> -       u8 max_remote_tiles:2;
> -
>         u8 has_asid:1;
>         u8 has_atomic_enable_pte_bit:1;
>         u8 has_flat_ccs:1;

Re: [PATCH 2/2] drm/xe: Refactor max_remote_tiles

[Matt Roper]

On Thu, Jan 16, 2025 at 11:16:26AM -0800, Summers, Stuart wrote:
> On Thu, 2025-01-16 at 18:24 +0530, Sai Teja Pottumuttu wrote:
> > max_remote_tiles is more related to the platform than the GT IP. Thus
> > move it to platform descriptor from graphics descriptor. Note that
> > the
> > FIXME is no more required, thus it can be dropped.
> 
> I think we still want this FIXME. Has anything fundamentally changed in
> the driver wrt tiles vs gts?

I think at this point we've fixed most of the code to use
xe->info.gt_count in the proper places instead of xe->info.tile_count.
E.g., commit 37efea9ca258 ("drm/xe: Allow GT looping and lookup on
standalone media") and similar changes.  If there are places still
incorrectly using tile_count then we should fix those, but I'm not aware
of any off the top of my head.

The thing that is still a bit of an open issue is if we eventually have
a platform that has both multiple tiles _and_ multiple GTs per tile,
we'll need to figure out how we want to number the GTs (for example,
0=p 1=m 2=p 3=m vs 0=p 1=p 2=m 3=m) and then rework code like
xe_device_get_gt() to use that.  But that decision can wait until we
actually have such a platform.


Matt

> 
> Thanks,
> Stuart
> 
> > 
> > Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_pci.c       | 9 +++------
> >  drivers/gpu/drm/xe/xe_pci_types.h | 2 --
> >  2 files changed, 3 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pci.c
> > b/drivers/gpu/drm/xe/xe_pci.c
> > index f59c1acae23f..7f17e5ea6edd 100644
> > --- a/drivers/gpu/drm/xe/xe_pci.c
> > +++ b/drivers/gpu/drm/xe/xe_pci.c
> > @@ -55,6 +55,7 @@ struct xe_device_desc {
> >         enum xe_platform platform;
> >  
> >         u8 dma_mask_size;
> > +       u8 max_remote_tiles:2;
> >  
> >         u8 require_force_probe:1;
> >         u8 is_dgfx:1;
> > @@ -136,7 +137,6 @@ static const struct xe_graphics_desc
> > graphics_xehpc = {
> >                 BIT(XE_HW_ENGINE_CCS2) | BIT(XE_HW_ENGINE_CCS3),
> >  
> >         XE_HP_FEATURES,
> > -       .max_remote_tiles = 1,
> >         .va_bits = 57,
> >         .vm_max_level = 4,
> >         .vram_flags = XE_VRAM_FLAGS_NEED64K,
> > @@ -329,6 +329,7 @@ static const __maybe_unused struct xe_device_desc
> > pvc_desc = {
> >         .dma_mask_size = 52,
> >         .has_display = false,
> >         .has_heci_gscfi = 1,
> > +       .max_remote_tiles = 1,
> >         .require_force_probe = true,
> >  };
> >  
> > @@ -635,6 +636,7 @@ static int xe_info_init_early(struct xe_device
> > *xe,
> >         xe->info.probe_display = IS_ENABLED(CONFIG_DRM_XE_DISPLAY) &&
> >                                  xe_modparam.probe_display &&
> >                                  desc->has_display;
> > +       xe->info.tile_count = 1 + desc->max_remote_tiles;
> >  
> >         err = xe_tile_init_early(xe_device_get_root_tile(xe), xe, 0);
> >         if (err)
> > @@ -706,12 +708,7 @@ static int xe_info_init(struct xe_device *xe,
> >          * version 13 or higher has an additional dedicated media
> > GT.  And
> >          * depending on the graphics IP there may be additional
> > "remote tiles."
> >          * All of these together determine the overall GT count.
> > -        *
> > -        * FIXME: 'tile_count' here is misnamed since the rest of the
> > driver
> > -        * treats it as the number of GTs rather than just the number
> > of tiles.
> >          */
> > -       xe->info.tile_count = 1 + graphics_desc->max_remote_tiles;
> > -
> >         for_each_remote_tile(tile, xe, id) {
> >                 int err;
> >  
> > diff --git a/drivers/gpu/drm/xe/xe_pci_types.h
> > b/drivers/gpu/drm/xe/xe_pci_types.h
> > index 7437415a54d5..b96423844952 100644
> > --- a/drivers/gpu/drm/xe/xe_pci_types.h
> > +++ b/drivers/gpu/drm/xe/xe_pci_types.h
> > @@ -19,8 +19,6 @@ struct xe_graphics_desc {
> >  
> >         u64 hw_engine_mask;     /* hardware engines provided by
> > graphics IP */
> >  
> > -       u8 max_remote_tiles:2;
> > -
> >         u8 has_asid:1;
> >         u8 has_atomic_enable_pte_bit:1;
> >         u8 has_flat_ccs:1;
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

✗ CI.Patch_applied: failure for Refactor some graphics descriptor flags

[Patchwork]

== Series Details ==

Series: Refactor some graphics descriptor flags
URL   : https://patchwork.freedesktop.org/series/143608/
State : failure

== Summary ==

=== Applying kernel patches on branch 'drm-tip' with base: ===
Base commit: 50d15a728295 drm-tip: 2025y-01m-16d-11h-45m-35s UTC integration manifest
=== git am output follows ===
error: patch failed: drivers/gpu/drm/xe/xe_pci.c:238
error: drivers/gpu/drm/xe/xe_pci.c: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Applying: drm/xe: Refactor dma_mask_size
Patch failed at 0001 drm/xe: Refactor dma_mask_size
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".