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

[Matthew Brost <matthew.brost@intel.com>]

On Mon, Jun 10, 2024 at 06:45:12PM +0200, Michal Wajdeczko wrote:
> 
> 
> 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()
> 

I think I'll just switch to xe_gt_err in next rev or at merge time.

Matt

> 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
> >>
> >>