[Intel-gfx] [RFC PATCH 35/97] drm/i915/guc: Improve error message for unsolicited CT response

[Matthew Brost]

On Mon, May 24, 2021 at 01:59:54PM +0200, Michal Wajdeczko wrote:
> 
> 
> On 06.05.2021 21:13, Matthew Brost wrote:
> > Improve the error message when a unsolicited CT response is received by
> > printing fence that couldn't be found, the last fence, and all requests
> > with a response outstanding.
> > 
> > Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> > index 217ab3ebd1af..a76603537fa8 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> > @@ -703,12 +703,16 @@ static int ct_handle_response(struct intel_guc_ct *ct, struct ct_incoming_msg *r
> >  		found = true;
> >  		break;
> >  	}
> > -	spin_unlock_irqrestore(&ct->requests.lock, flags);
> > -
> >  	if (!found) {
> >  		CT_ERROR(ct, "Unsolicited response (fence %u)\n", fence);
> > -		return -ENOKEY;
> > +		CT_ERROR(ct, "Could not find fence=%u, last_fence=%u\n", fence,
> > +			 ct->requests.last_fence);
> 
> nit: this new wording may suggest that it's our fault, but that's not
> necessary true
> 

I don't think is implies whos fault this is either way.

> > +		list_for_each_entry(req, &ct->requests.pending, link)
> > +			CT_ERROR(ct, "request %u awaits response\n",
> > +				 req->fence);
> 
> usually we don't send multiple requests that expects responses, so it's
> very likely that list with pending requests will be empty, and even if
> list is not empty, I'm not sure what is the relation between those
> pending requests to this unsolicited response, thus wondering how these
> extra errors could improve our debugging experience ?
> 

The more information when this occurs the better.

Matt

> > +		err = -ENOKEY;
> >  	}
> > +	spin_unlock_irqrestore(&ct->requests.lock, flags);
> >  
> >  	if (unlikely(err))
> >  		return err;
> >