Re: [PATCH v2] drm/xe: Add assert for XE_WA() usage

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

On 22-07-2024 21:36, Lucas De Marchi wrote:
> It's not always safe to call XE_WA() in the driver initialization. Add a
> xe_gt_assert() so this doesn't go unnoticed.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> 
> v2:
>    - Fix kernel-doc (Tejas)
>    - Remove needless initialization (Tejas)
> 
>   drivers/gpu/drm/xe/xe_gt_types.h | 6 ++++++
>   drivers/gpu/drm/xe/xe_wa.c       | 1 +
>   drivers/gpu/drm/xe/xe_wa.h       | 7 ++++++-
>   3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
> index ef68c4a92972..4dd01adf63c6 100644
> --- a/drivers/gpu/drm/xe/xe_gt_types.h
> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
> @@ -389,6 +389,12 @@ struct xe_gt {
>   		unsigned long *lrc;
>   		/** @wa_active.oob: bitmap with active OOB workaroudns */

Since you're here, please attend the existing typo in kernel-doc.
s/workaroudns/workarounds


>   		unsigned long *oob;
> +		/**
> +		 * @wa_active.oob_initialized: mark oob as initialized to help
> +		 * detecting misuse of XE_WA() - it can only be called on
> +		 * initialization after OOB WAs have being processed
> +		 */
> +		bool oob_initialized;
>   	} wa_active;
>   
>   	/** @user_engines: engines present in GT and available to userspace */
> diff --git a/drivers/gpu/drm/xe/xe_wa.c b/drivers/gpu/drm/xe/xe_wa.c
> index fd009b2c68fa..564e32e44e3b 100644
> --- a/drivers/gpu/drm/xe/xe_wa.c
> +++ b/drivers/gpu/drm/xe/xe_wa.c
> @@ -755,6 +755,7 @@ void xe_wa_process_oob(struct xe_gt *gt)
>   
>   	xe_rtp_process_ctx_enable_active_tracking(&ctx, gt->wa_active.oob,
>   						  ARRAY_SIZE(oob_was));
> +	gt->wa_active.oob_initialized = true;
>   	xe_rtp_process(&ctx, oob_was);
>   }
>   
> diff --git a/drivers/gpu/drm/xe/xe_wa.h b/drivers/gpu/drm/xe/xe_wa.h
> index db9ddeaf69bf..52337405b5bc 100644
> --- a/drivers/gpu/drm/xe/xe_wa.h
> +++ b/drivers/gpu/drm/xe/xe_wa.h
> @@ -6,6 +6,8 @@
>   #ifndef _XE_WA_
>   #define _XE_WA_
>   
> +#include "xe_assert.h"
> +
>   struct drm_printer;
>   struct xe_gt;
>   struct xe_hw_engine;
> @@ -25,6 +27,9 @@ void xe_wa_dump(struct xe_gt *gt, struct drm_printer *p);
>    * @gt__: gt instance
>    * @id__: XE_OOB_<id__>, as generated by build system in generated/xe_wa_oob.h
>    */
> -#define XE_WA(gt__, id__) test_bit(XE_WA_OOB_ ## id__, (gt__)->wa_active.oob)
> +#define XE_WA(gt__, id__) ({						\
> +	xe_gt_assert(gt__, (gt__)->wa_active.oob_initialized);		\
> +	test_bit(XE_WA_OOB_ ## id__, (gt__)->wa_active.oob);		\
> +})
>   
>   #endif