[Intel-gfx] [RFC 1/7] drm/i915/guc: Add basic support for error capture lists

[Teres Alexis, Alan Previn]

I missed responding to this.
Thanks for the review Michal - will fix them on next rev.
...alan

On Tue, 2021-11-23 at 22:12 +0100, Michal Wajdeczko wrote:
> 
> On 23.11.2021 00:03, Alan Previn wrote:
> > From: John Harrison <John.C.Harrison at Intel.com>
> ...
> 
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > index 77fbcd8730ee..0bfc92b1b982 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > @@ -4003,6 +4003,24 @@ int intel_guc_context_reset_process_msg(struct intel_guc *guc,
> >  	return 0;
> >  }
> >  
> > +int intel_guc_error_capture_process_msg(struct intel_guc *guc,
> > +					 const u32 *msg, u32 len)
> > +{
> > +	int status;
> 
> likely it should be "u32" as few lines below you're using msg[0];
> 
> > +
> > +	if (unlikely(len != 1)) {
> > +		drm_dbg(&guc_to_gt(guc)->i915->drm, "Invalid length %u", len);
> 
> any error returned by the CTB message handler will trigger full dump of
> unexpected message - do we really need this unlikely dbg message here ?
> 
> > +		return -EPROTO;
> > +	}
> > +
> > +	status = msg[0];
> > +	drm_info(&guc_to_gt(guc)->i915->drm, "Got error capture: status = %d", status);
> 
> IIRC all notification status are defined in GuC spec in hex, so maybe we
> should also print it as %#x ?
> 
> -Michal
> 
> > +
> > +	/* Add extraction of error capture dump */
> > +
> > +	return 0;
> > +}
> > +
> >  static struct intel_engine_cs *
> >  guc_lookup_engine(struct intel_guc *guc, u8 guc_class, u8 instance)
> >  {
> >