On Tue, 10 May 2022 00:43:29 -0700, Jani Nikula wrote:
> On Tue, 10 May 2022, Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> 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(>->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
