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

[Lucas De Marchi]

On Wed, Jul 17, 2024 at 08:24:55AM GMT, Upadhyay, Tejas wrote:
>
>
>> -----Original Message-----
>> From: De Marchi, Lucas <lucas.demarchi at intel.com>
>> Sent: Tuesday, July 16, 2024 8:07 PM
>> To: intel-xe at lists.freedesktop.org
>> Cc: Upadhyay, Tejas <tejas.upadhyay at intel.com>; De Marchi, Lucas
>> <lucas.demarchi at intel.com>
>> Subject: [PATCH] drm/xe: Add assert for XE_WA() usage
>>
>> 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 at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_gt_types.h | 6 ++++++
>>  drivers/gpu/drm/xe/xe_wa.c       | 2 ++
>>  drivers/gpu/drm/xe/xe_wa.h       | 7 ++++++-
>>  3 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h
>> b/drivers/gpu/drm/xe/xe_gt_types.h
>> index 38a0d0e178c8..1e790ca0be43 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_types.h
>> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
>> @@ -378,6 +378,12 @@ struct xe_gt {
>>  		unsigned long *lrc;
>>  		/** @wa_active.oob: bitmap with active OOB workaroudns
>> */
>>  		unsigned long *oob;
>> +		/**
>> +		 * @wa_active.oob: mark oob as initialized to help detecting
>
>s/ wa_active.oob /wa_active.oob_initialized

thanks for catching that

>
>> +		 * 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..de356c024d5d 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);
>>  }
>>
>> @@ -838,6 +839,7 @@ int xe_wa_init(struct xe_gt *gt)
>>  	gt->wa_active.lrc = p;
>>  	p += n_lrc;
>>  	gt->wa_active.oob = p;
>> +	gt->wa_active.oob_initialized = false;
>
>Before this init, mmio read in xe_mmio_probe_tiles() will hit in some case, right?

actually it's before the hunk above, when setting it to true.

> I think gt->wa_active.oob_initialized automatically default to false by kzalloc, do will need to set here?

true... I was just not very confortable assuming the allocation is done
elsewhere and with this it should be more contained on behavior changes.
But I think it's ok to just remove this line, will do.

thanks
Lucas De Marchi