Re: [PATCH] drm/xe: Always check force_wake_get return code

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

The commit title seems a bit too generic, given that you're only fixing 
a single occurrence. maybe switch to something like "check 
force_wake_get return code in show_run_ticks" ?

On 6/3/2024 4:30 AM, Tejas Upadhyay wrote:
> A force_wake_get failure means that the HW might not be awake for the
> access we're doing; this can lead to an immediate error or it can be a
> more subtle problem.
>
> In this case, user has asked information and in case of forcewake
> failure, it might lead to wrong information, so return in case of
> error.
>
> Signed-off-by: Tejas Upadhyay <tejas.upadhyay@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_drm_client.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_drm_client.c b/drivers/gpu/drm/xe/xe_drm_client.c
> index 4a19b771e3a0..e050fc6d8dd7 100644
> --- a/drivers/gpu/drm/xe/xe_drm_client.c
> +++ b/drivers/gpu/drm/xe/xe_drm_client.c
> @@ -264,7 +264,8 @@ 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))
> +			return;

Since we're not returning any error code, there doesn't seem to be a 
direct way to know that a forcewake error happened; we can probably 
infer it by the lack of messages being printed, but there is another 
(unlikely) exit point that causes no prints to occur. Should we print an 
error to dmesg, just to be safe?

Daniele

>   		gpu_timestamp = xe_hw_engine_read_timestamp(hwe);
>   		xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
>   		break;