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

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

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;