[Intel-xe] [PATCH v2 05/17] drm/xe: Rename instruction field to avoid confusion

[Matt Roper]

On Fri, Apr 21, 2023 at 03:32:46PM -0700, Lucas De Marchi wrote:
> There was both BLT_DEPTH_32 and XY_FAST_COLOR_BLT_DEPTH_32 - also add
> the prefix to the first to make it clear this is about the FAST_**COPY**
> operation. While at it, remove the GEN9_ prefix.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>

Looks like i915 (correctly) uses the same definition for a bunch of
different instructions (XY_SRC_COPY_BLT, XY_COLOR_BLT, XY_FAST_COPY_BLT,
etc.).  But Xe only uses it for XY_FAST_COPY_BLT, so this rename matches
the usage.

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>

I do wonder if we should think about changing how we emit GPU
instructions at some point.  Even when we get the bit shifting correct,
there's still potential mistakes if you OR the definitions into the
wrong dword (which has happened at least a few times in i915 history).
Maybe we could do something like create a C structure per instruction
(e.g., "struct xy_fast_copy_blt") and fill in its fields to build up
these batchbuffers.


Matt

> ---
>  drivers/gpu/drm/xe/regs/xe_gpu_commands.h | 4 ++--
>  drivers/gpu/drm/xe/xe_migrate.c           | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_gpu_commands.h b/drivers/gpu/drm/xe/regs/xe_gpu_commands.h
> index 9d6508d74d62..05531d43514f 100644
> --- a/drivers/gpu/drm/xe/regs/xe_gpu_commands.h
> +++ b/drivers/gpu/drm/xe/regs/xe_gpu_commands.h
> @@ -55,8 +55,8 @@
>  #define   XY_FAST_COLOR_BLT_MOCS_MASK	GENMASK(27, 21)
>  #define   XY_FAST_COLOR_BLT_MEM_TYPE_SHIFT 31
>  
> -#define GEN9_XY_FAST_COPY_BLT_CMD	(2 << 29 | 0x42 << 22)
> -#define   BLT_DEPTH_32			(3<<24)
> +#define XY_FAST_COPY_BLT_CMD		(2 << 29 | 0x42 << 22)
> +#define   XY_FAST_COPY_BLT_DEPTH_32	(3<<24)
>  
>  #define	PVC_MEM_SET_CMD		(2 << 29 | 0x5b << 22)
>  #define   PVC_MEM_SET_CMD_LEN_DW	7
> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index 5acb227f2b5a..f40f47ccb76f 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
> @@ -518,8 +518,8 @@ static void emit_copy(struct xe_gt *gt, struct xe_bb *bb,
>  	XE_BUG_ON(pitch / 4 > S16_MAX);
>  	XE_BUG_ON(pitch > U16_MAX);
>  
> -	bb->cs[bb->len++] = GEN9_XY_FAST_COPY_BLT_CMD | (10 - 2);
> -	bb->cs[bb->len++] = BLT_DEPTH_32 | pitch;
> +	bb->cs[bb->len++] = XY_FAST_COPY_BLT_CMD | (10 - 2);
> +	bb->cs[bb->len++] = XY_FAST_COPY_BLT_DEPTH_32 | pitch;
>  	bb->cs[bb->len++] = 0;
>  	bb->cs[bb->len++] = (size / pitch) << 16 | pitch / 4;
>  	bb->cs[bb->len++] = lower_32_bits(dst_ofs);
> -- 
> 2.39.0
> 

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