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@xxxxxxxxx>
> > ---
> > 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;
> >
