Re: [PATCH v5 06/10] drm/xe: Improve unexpected state error messages

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 10.06.2024 18:36, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Matthew Brost
> Sent: Monday, June 10, 2024 7:18 AM
> To: intel-xe@lists.freedesktop.org
> Subject: [PATCH v5 06/10] drm/xe: Improve unexpected state error messages
>>
>> Include G2H handler name when an unexpected error state messages.
>>
>> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> 
> Having a separation for these two error messages is a good idea.
> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> -Jonathan Cavitt
> 
>> ---
>>  drivers/gpu/drm/xe/xe_guc_submit.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
>> index 4464ba337d12..766ff8e48dde 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
>> @@ -1632,8 +1632,8 @@ int xe_guc_sched_done_handler(struct xe_guc *guc, u32 *msg, u32 len)
>>  
>>  	if (unlikely(!exec_queue_pending_enable(q) &&
>>  		     !exec_queue_pending_disable(q))) {
>> -		drm_err(&xe->drm, "Unexpected engine state 0x%04x",
>> -			atomic_read(&q->guc->state));
>> +		drm_err(&xe->drm, "SCHED_DONE: Unexpected engine state 0x%04x, guc_id=%d",
>> +			atomic_read(&q->guc->state), q->guc->id);

but please at least use xe_gt_err()

other option would be to use xe_gt_err_printer() and dump engine state
in more friendly/detailed way

>>  		return -EPROTO;
>>  	}
>>  
>> @@ -1671,8 +1671,8 @@ int xe_guc_deregister_done_handler(struct xe_guc *guc, u32 *msg, u32 len)
>>  
>>  	if (!exec_queue_destroyed(q) || exec_queue_pending_disable(q) ||
>>  	    exec_queue_pending_enable(q) || exec_queue_enabled(q)) {
>> -		drm_err(&xe->drm, "Unexpected engine state 0x%04x",
>> -			atomic_read(&q->guc->state));
>> +		drm_err(&xe->drm, "DEREGISTER_DONE: Unexpected engine state 0x%04x, guc_id=%d",
>> +			atomic_read(&q->guc->state), q->guc->id);

ditto

>>  		return -EPROTO;
>>  	}
>>  
>> -- 
>> 2.34.1
>>
>>