Re: [Intel-gfx] [PATCH 3/8] drm/i915/pcode: Extend pcode functions for multiple gt's

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Tue, 10 May 2022 00:43:29 -0700, Jani Nikula wrote:
> On Tue, 10 May 2022, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> > On 29/04/2022 20:56, Ashutosh Dixit wrote:
> >> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> >> index 90b0ce5051af..bc49eff38c6a 100644
> >> --- a/drivers/gpu/drm/i915/i915_driver.c
> >> +++ b/drivers/gpu/drm/i915/i915_driver.c
> >> @@ -520,6 +520,22 @@ static int i915_set_dma_info(struct drm_i915_private *i915)
> >>	return ret;
> >>   }
> >>
> >> +static int i915_pcode_init(struct drm_i915_private *i915)
> >> +{
> >> +	struct intel_gt *gt;
> >> +	int id, ret;
> >> +
> >> +	for_each_gt(gt, i915, id) {
> >> +		ret = intel_pcode_init(gt->uncore);
> >> +		if (ret) {
> >> +			drm_err(&gt->i915->drm, "gt %d: intel_pcode_init failed %d\n", id, ret);
> >
> > A few nits..
> >
> > 1) All other/current logs use "gt%d" (no space).
> >
> > 2) intel_pcode_init also logs a drm_err - do we need two? I suggest
> > leaving this one only since it has more information.
> >
> > 3) It would have been nicer to have refactoring of intel_pcode_ to work
> > on uncore separate from adding for_each_gt.
>
> Yeah.
>
> Also the obvious first patch would've been to convert intel_pcode.c
> functions from struct drm_i915_private * to intel_uncore *.

Will fix up the first 2 points but about this last point, to not break
incremental compile all callers of the pcode functions also need to be
converted to i915->uncore or gt->uncore (so it's not possible to convert
just intel_pcode.c functions without also converting all callers, if that
was the intent of this comment, unless I am missing something).

But yes the i915_pcode_init() above can be separated out to a separate
patch so I can do that.

Thanks.
--
Ashutosh