Re: [PATCH 1/3] drm/xe: Introduced needs_scratch bit in device descriptor

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Feb 04, 2025 at 01:45:56PM -0500, Oak Zeng wrote:
> On some platform, scratch page is needed for out of bound prefetch
> to work. Introduce a bit in device descriptor to specify whether
> this device needs scratch page to work.
> 
> v2: introduce a needs_scratch bit in device info (Thomas, Jonathan)
> 
> Signed-off-by: Oak Zeng <oak.zeng@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device_types.h | 3 +++
>  drivers/gpu/drm/xe/xe_pci.c          | 5 +++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index c0e886bac183..1197ab31e528 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -45,6 +45,7 @@ struct xe_pxp;
>  #define GRAPHICS_VERx100(xe) ((xe)->info.graphics_verx100)
>  #define MEDIA_VERx100(xe) ((xe)->info.media_verx100)
>  #define IS_DGFX(xe) ((xe)->info.is_dgfx)
> +#define NEEDS_SCRATCH(xe) ((xe)->info.needs_scratch)

I'd drop this macro. These are leftovers from the i915 code and I see
like value in a macro looking directly at a field. In the existing cases
these macros are used all over the code so they make a little more sense
so if change the variable we only have change this macro, but in this
new case the macro is used 2 places in the code so I'd say just look at
the variable.

Matt
 
>  
>  #define XE_VRAM_FLAGS_NEED64K		BIT(0)
>  
> @@ -318,6 +319,8 @@ struct xe_device {
>  		u8 has_usm:1;
>  		/** @info.is_dgfx: is discrete device */
>  		u8 is_dgfx:1;
> +		/** @info.needs_scratch: needs scratch page for oob prefetch to work */
> +		u8 needs_scratch:1;
>  		/**
>  		 * @info.probe_display: Probe display hardware.  If set to
>  		 * false, the driver will behave as if there is no display
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index e8ef7d6b4db8..09736674a77e 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -67,6 +67,7 @@ struct xe_device_desc {
>  	u8 has_llc:1;
>  	u8 has_pxp:1;
>  	u8 has_sriov:1;
> +	u8 needs_scratch:1;
>  	u8 skip_guc_pc:1;
>  	u8 skip_mtcfg:1;
>  	u8 skip_pcode:1;
> @@ -353,6 +354,7 @@ static const struct xe_device_desc lnl_desc = {
>  	.dma_mask_size = 46,
>  	.has_display = true,
>  	.has_pxp = true,
> +	.needs_scratch = true,
>  };
>  
>  static const struct xe_device_desc bmg_desc = {
> @@ -361,6 +363,7 @@ static const struct xe_device_desc bmg_desc = {
>  	.dma_mask_size = 46,
>  	.has_display = true,
>  	.has_heci_cscfi = 1,
> +	.needs_scratch = true,
>  };
>  
>  static const struct xe_device_desc ptl_desc = {
> @@ -368,6 +371,7 @@ static const struct xe_device_desc ptl_desc = {
>  	.dma_mask_size = 46,
>  	.has_display = true,
>  	.require_force_probe = true,
> +	.needs_scratch = true,
>  };
>  
>  #undef PLATFORM
> @@ -643,6 +647,7 @@ static int xe_info_init_early(struct xe_device *xe,
>  	xe->info.skip_guc_pc = desc->skip_guc_pc;
>  	xe->info.skip_mtcfg = desc->skip_mtcfg;
>  	xe->info.skip_pcode = desc->skip_pcode;
> +	xe->info.needs_scratch = desc->needs_scratch;
>  
>  	xe->info.probe_display = IS_ENABLED(CONFIG_DRM_XE_DISPLAY) &&
>  				 xe_modparam.probe_display &&
> -- 
> 2.26.3
>