[PATCH] drm/xe/client: Check return value of xe_force_wake_get

[Lucas De Marchi]

On Wed, May 29, 2024 at 10:50:34AM GMT, Nirmoy Das wrote:
>Hi Rodrigo,
>
>On 5/28/2024 7:22 PM, Rodrigo Vivi wrote:
>>On Tue, May 28, 2024 at 12:11:32PM +0200, Nirmoy Das wrote:
>>>xe_force_wake_get() can return error so check it's return value
>>>before reading gpu_timestamp value.
>>>
>>>Fixes: 188ced1e0ff8 ("drm/xe/client: Print runtime to fdinfo")
>>>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>>>Signed-off-by: Nirmoy Das <nirmoy.das at intel.com>
>>>---
>>>  drivers/gpu/drm/xe/xe_drm_client.c | 6 ++++--
>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>>diff --git a/drivers/gpu/drm/xe/xe_drm_client.c b/drivers/gpu/drm/xe/xe_drm_client.c
>>>index 4a19b771e3a0..0c13395ffd34 100644
>>>--- a/drivers/gpu/drm/xe/xe_drm_client.c
>>>+++ b/drivers/gpu/drm/xe/xe_drm_client.c
>>>@@ -245,7 +245,7 @@ static void show_run_ticks(struct drm_printer *p, struct drm_file *file)
>>>  	struct xe_gt *gt;
>>>  	struct xe_hw_engine *hwe;
>>>  	struct xe_exec_queue *q;
>>>-	u64 gpu_timestamp;
>>>+	u64 gpu_timestamp = 0;
>>>  	xe_pm_runtime_get(xe);
>>>@@ -264,7 +264,9 @@ static void show_run_ticks(struct drm_printer *p, struct drm_file *file)
>>>  		if (!hwe)
>>>  			continue;
>>>-		xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
>>>+		if (xe_force_wake_get(gt_to_fw(gt), XE_FW_GT))
>>>+			break;
>>worth adding a debug/err msg?
>
>xe_force_wake_get() will throw a drm_notice on error. May be we should 
>add a __builtin_return_address() in xe_force_wake_get() to know the 
>caller as
>
>the current pattern seems to be to just return early on error ?

humn... but just continuing here and returning something with a gpu
timestamp == 0 doesn't seem something good. I think we should set hwe to
NULL, so the function doesn't continue and just doesn't print anything
rather than printing wrong data.

Lucas De Marchi

>
>>
>>but up to you:
>>
>>Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
>Thanks,
>
>Nirmoy
>
>>
>>>+
>>>  		gpu_timestamp = xe_hw_engine_read_timestamp(hwe);
>>>  		xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
>>>  		break;
>>>-- 
>>>2.42.0
>>>