Re: [Intel-gfx] [PATCH 1/2] drm/i915/gt: More use of GT specific print helpers

[Andi Shyti]

Hi John,

...

> > >           if (intf_id >= INTEL_GSC_NUM_INTERFACES) {
> > > -         drm_warn_once(&gt->i915->drm, "GSC irq: intf_id %d is out of 
> > > range", intf_id);
> > > +         gt_warn_once(gt, "GSC irq: intf_id %d is out of range", 
> > > intf_id);
> > >                   return;
> > >           }
> > >           if (!HAS_HECI_GSC(gt->i915)) {
> > > -         drm_warn_once(&gt->i915->drm, "GSC irq: not supported");
> > > +         gt_warn_once(gt, "GSC irq: not supported");
> > >                   return;
> > >           }
> > > @@ -300,7 +301,7 @@ static void gsc_irq_handler(struct intel_gt *gt, 
> > > unsigned int intf_id)
> > >           ret = generic_handle_irq(gt->gsc.intf[intf_id].irq);
> > >           if (ret)
> > > -         drm_err_ratelimited(&gt->i915->drm, "error handling GSC irq: 
> > > %d\n", ret);
> > > +         gt_err_ratelimited(gt, "error handling GSC irq: %d\n", ret);
> > >   }
> > >   void intel_gsc_irq_handler(struct intel_gt *gt, u32 iir)
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_print.h 
> > > b/drivers/gpu/drm/i915/gt/intel_gt_print.h
> > > index 55a336a9ff061..7fdc78c79273e 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_gt_print.h
> > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_print.h
> > > @@ -16,6 +16,9 @@
> > >   #define gt_warn(_gt, _fmt, ...) \
> > >           drm_warn(&(_gt)->i915->drm, "GT%u: " _fmt, (_gt)->info.id, 
> > > ##__VA_ARGS__)
> > > +#define gt_warn_once(_gt, _fmt, ...) \
> > > + drm_warn_once(&(_gt)->i915->drm, "GT%u: " _fmt, (_gt)->info.id, 
> > > ##__VA_ARGS__)
> > > +
> > I would add the gt_warn_once() part in a different patch.
> But this is the patch that uses it. You should not add dead code. The only
> exception being if it is something large and complex that needs to be added
> in stages for ease of code review. But this really doesn't count as large or
> complex!

I wouldn't call it dead code if it's used right after... you
could also put all the *_warn_* changes in different patch.

Anyway, I don't have a strong opinion for such a straight forward
patch, so that I'm fine with it as it is:

Reviewed-by: Andi Shyti <andi.sh...@linux.intel.com> 

Andi