Re: [PATCH] drm/xe: Fix xe_force_wake_assert_held for enum XE_FORCEWAKE_ALL

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

On 30-05-2024 20:14, Nilawar, Badal wrote:
>
>
> On 30-05-2024 19:51, Nilawar, Badal wrote:
>>
>>
>> On 30-05-2024 19:55, Himal Prasad Ghimiray wrote:
>>> Make sure that the assertion condition covers the wakefulness of all
>>> domains for XE_FORCEWAKE_ALL.
>>>
>>> Fixes: c73acc1eeba5 ("drm/xe: Use Xe assert macros instead of 
>>> XE_WARN_ON macro")
>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> Cc: Badal Nilawar <badal.nilawar@intel.com>
>>> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>>> ---
>>>   drivers/gpu/drm/xe/xe_force_wake.h | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_force_wake.h 
>>> b/drivers/gpu/drm/xe/xe_force_wake.h
>>> index 83cb157da7cc..9008928b187f 100644
>>> --- a/drivers/gpu/drm/xe/xe_force_wake.h
>>> +++ b/drivers/gpu/drm/xe/xe_force_wake.h
>>> @@ -32,7 +32,7 @@ static inline void
>>>   xe_force_wake_assert_held(struct xe_force_wake *fw,
>>>                 enum xe_force_wake_domains domain)
>>>   {
>>> -    xe_gt_assert(fw->gt, fw->awake_domains & domain);
>>> +    xe_gt_assert(fw->gt, (fw->awake_domains & domain) == domain);
>> This will always assert for when domain FORCEWAKE_ALL (0xFF).
>> Not all the platforms support all the domains.
>> e.g. MTL GT0 support GT and RENDER domain. So for forcewake all use 
>> case only bits for GT and RENDER will be set.
> I think to handle this correctly in struct xe_force_wake you can add 
> new enum xe_force_wake_domains supported_domains to hold bitmap of 
> supported forcewake domains. Use this bit map to check appropriate 
> domains are set.

Hi Badal,

Thanks for taking time to review this. Agreed the check should be based 
on supported domains.  Will look into this.

BR

Himal


>
> Regards,
> Badal
>>
>> Regards,
>> Badal
>>>   }
>>>   #endif