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

[John Harrison <john.c.harrison@intel.com>]

On 10/9/2023 13:02, Andi Shyti wrote:
> 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.
I did start splitting it into errors vs debugs but then decided it 
wasn't worth the effort ;)

>
> 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.shyti@linux.intel.com>
Thanks :).

>
> Andi