Re: [PATCH] drm/xe: Ensure XE_BO_FLAG_CPU_ADDR_MIRROR has a unique value

[Matthew Auld <matthew.auld@intel.com>]

On 04/04/2025 23:00, Matt Roper wrote:
> When XE_BO_FLAG_PINNED_NORESTORE and XE_BO_FLAG_PINNED_LATE_RESTORE were
> added, they were assigned BO flag values in the middle of the flag
> range, requiring renumbering of the higher flags.  In both cases,
> XE_BO_FLAG_CPU_ADDR_MIRROR was overlooked during renumbering because it
> was defined below XE_BO_FLAG_GGTT_ALL and thus was not immediately
> visible in code diffs changing this area of the code; this resulted in
> XE_BO_FLAG_CPU_ADDR_MIRROR clashing with another flag.
> 
> Assign XE_BO_FLAG_CPU_ADDR_MIRROR a unique value, and also move the
> definition of XE_BO_FLAG_GGTT_ALL down below all of the individual flags
> so that this kind of mistake is less likely in the future.  Also, while
> we're at it, fix up some space vs tab whitespace inconsistency in these
> flag definitions.
> 
> Fixes: 7f387e6012b6 ("drm/xe: add XE_BO_FLAG_PINNED_LATE_RESTORE")
> Fixes: 045448da87bf ("drm/xe: Add XE_BO_FLAG_PINNED_NORESTORE")
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Sorry for missing this,
Reviewed-by: Matthew Auld <matthew.auld@intel.com>

> ---
>   drivers/gpu/drm/xe/xe_bo.h | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
> index f7e716f59948..0a19b50045b2 100644
> --- a/drivers/gpu/drm/xe/xe_bo.h
> +++ b/drivers/gpu/drm/xe/xe_bo.h
> @@ -40,21 +40,22 @@
>   #define XE_BO_FLAG_NEEDS_2M		BIT(16)
>   #define XE_BO_FLAG_GGTT_INVALIDATE	BIT(17)
>   #define XE_BO_FLAG_PINNED_NORESTORE	BIT(18)
> -#define XE_BO_FLAG_PINNED_LATE_RESTORE BIT(19)
> -#define XE_BO_FLAG_GGTT0                BIT(20)
> -#define XE_BO_FLAG_GGTT1                BIT(21)
> -#define XE_BO_FLAG_GGTT2                BIT(22)
> -#define XE_BO_FLAG_GGTT3                BIT(23)
> -#define XE_BO_FLAG_GGTT_ALL             (XE_BO_FLAG_GGTT0 | \
> -					 XE_BO_FLAG_GGTT1 | \
> -					 XE_BO_FLAG_GGTT2 | \
> -					 XE_BO_FLAG_GGTT3)
> -#define XE_BO_FLAG_CPU_ADDR_MIRROR	BIT(22)
> +#define XE_BO_FLAG_PINNED_LATE_RESTORE	BIT(19)
> +#define XE_BO_FLAG_GGTT0		BIT(20)
> +#define XE_BO_FLAG_GGTT1		BIT(21)
> +#define XE_BO_FLAG_GGTT2		BIT(22)
> +#define XE_BO_FLAG_GGTT3		BIT(23)
> +#define XE_BO_FLAG_CPU_ADDR_MIRROR	BIT(24)
>   
>   /* this one is trigger internally only */
>   #define XE_BO_FLAG_INTERNAL_TEST	BIT(30)
>   #define XE_BO_FLAG_INTERNAL_64K		BIT(31)
>   
> +#define XE_BO_FLAG_GGTT_ALL		(XE_BO_FLAG_GGTT0 | \
> +					 XE_BO_FLAG_GGTT1 | \
> +					 XE_BO_FLAG_GGTT2 | \
> +					 XE_BO_FLAG_GGTT3)
> +
>   #define XE_BO_FLAG_GGTTx(tile) \
>   	(XE_BO_FLAG_GGTT0 << (tile)->id)
>