[PATCH 3/3] drm/xe/client: Skip drm-total-cycles if unable to read timestamp

[Lucas De Marchi]

On Tue, Feb 04, 2025 at 07:56:03PM +0100, Michal Wajdeczko wrote:
>
>
>On 04.02.2025 19:05, Marcin Bernatowicz wrote:
>> Ensure show_run_ticks() only prints drm-total-cycles when timestamp
>> retrieval succeeds.
>>
>> Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz at linux.intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Michał Winiarski <michal.winiarski at intel.com>
>> Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_drm_client.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_drm_client.c b/drivers/gpu/drm/xe/xe_drm_client.c
>> index 63f30b6df70b..e5c4f342380e 100644
>> --- a/drivers/gpu/drm/xe/xe_drm_client.c
>> +++ b/drivers/gpu/drm/xe/xe_drm_client.c
>> @@ -323,6 +323,7 @@ static void show_run_ticks(struct drm_printer *p, struct drm_file *file)
>>  	struct xe_exec_queue *q;
>>  	u64 gpu_timestamp;
>>  	unsigned int fw_ref;
>> +	int err;
>>
>>  	/*
>>  	 * Wait for any exec queue going away: their cycles will get updated on
>> @@ -350,7 +351,7 @@ static void show_run_ticks(struct drm_printer *p, struct drm_file *file)
>>  	}
>>  	mutex_unlock(&xef->exec_queue.lock);
>>
>> -	gpu_timestamp = xe_hw_engine_read_timestamp(hwe);
>> +	err = xe_hw_engine_read_timestamp(hwe, &gpu_timestamp);
>
>can't we just check for !IS_SRIOV_VF here?
>
>>
>>  	xe_force_wake_put(gt_to_fw(hwe->gt), fw_ref);
>>  	xe_pm_runtime_put(xe);
>> @@ -371,8 +372,10 @@ static void show_run_ticks(struct drm_printer *p, struct drm_file *file)
>>  		class_name = xe_hw_engine_class_to_str(class);
>>  		drm_printf(p, "drm-cycles-%s:\t%llu\n",
>>  			   class_name, xef->run_ticks[class]);
>> -		drm_printf(p, "drm-total-cycles-%s:\t%llu\n",
>> -			   class_name, gpu_timestamp);
>> +
>> +		if (!err)
>> +			drm_printf(p, "drm-total-cycles-%s:\t%llu\n",
>
>are we sure we don't break any tools that might look for this?

It may break if the tool is expecting drm-total-cycles-* to exist when it
reads a drm-cycles-*. Also there isn't much value afaik from showing just
the total cycles. IMO we could just skip the entire function
on IS_SRIOV_VF().

Lucas De Marchi

>
>> +				   class_name, gpu_timestamp);
>>
>>  		if (capacity[class] > 1)
>>  			drm_printf(p, "drm-engine-capacity-%s:\t%lu\n",
>