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@xxxxxxxxx>
> ...
>
> > 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)
> > {
> >
