[PATCH] drm/xe: Check return values of functions in xe_gt_shutdown()

[Ghimiray, Himal Prasad]

On 26-09-2024 16:41, Maarten Lankhorst wrote:
> Hey,
> 
> Den 2024-09-26 kl. 13:00, skrev apoorva.singh at intel.com:
>> From: Apoorva Singh <apoorva.singh at intel.com>
>>
>> Check the return values of the functions xe_force_wake_get()
>> and xe_force_wake_put() to prevent mistakenly treating them as
>> void returns.
>>
>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>
>> Signed-off-by: Apoorva Singh <apoorva.singh at intel.com>
>> Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_gt.c | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
>> index 274737417b0f..317640554310 100644
>> --- a/drivers/gpu/drm/xe/xe_gt.c
>> +++ b/drivers/gpu/drm/xe/xe_gt.c
>> @@ -890,9 +890,12 @@ int xe_gt_suspend(struct xe_gt *gt)
>>   
>>   void xe_gt_shutdown(struct xe_gt *gt)
>>   {
>> -	xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>> +	int err;
>> +	err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>> +	xe_gt_WARN(gt, err, "Acknowledgment for domain awake timedout");
>>   	do_gt_reset(gt);
>> -	xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>> +	err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>> +	xe_gt_WARN(gt, err, "Acknowledgment for domain sleep timedout");
>>   }
> 
> There's absolutely nothing we can do with xe_force_wake_put, the only thing you can do here is:
> 
> int ret = xe_force_wake_get(..);
> do_gt_reset;
> if (!ret)
> xe_force_wake_put

This is wrong in current scenario. force_wake_get() fails but it still 
increases refcount. So xe_force_wake_put needs to be called here to 
decrease the refcount. I am already working on solving the problem in 
entire driver.

https://lore.kernel.org/intel-xe/20240924121641.1045763-1-himal.prasad.ghimiray@intel.com/T/#t 


> 
> Also no point in complaining, the hardware may already be hosed. :)
> 
> I believe that xe_force_wake_put should return void, there's nothing we can do except log the error here.

Agreed. Above series covers same too.

> 
> Cheers,
> Maarten