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

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

On Sun, 24 Apr 2022 12:08:18 -0700, Andi Shyti wrote:
>
> Hi Ashutosh,

Hi Andi,

> [...]
>
> > -static bool skl_pcode_try_request(struct drm_i915_private *i915, u32 mbox,
> > -				  u32 request, u32 reply_mask, u32 reply,
> > -				  u32 *status)
> > +static bool __gt_pcode_try_request(struct intel_gt *gt, u32 mbox,
>
> why is this becoming a '__' function?

Fixed in v3.

> >  int intel_pcode_init(struct drm_i915_private *i915)
> >  {
> > -	int ret = 0;
> > +	struct intel_gt *gt;
> > +	int i, ret = 0;
> >
> >	if (!IS_DGFX(i915))
> >		return ret;
>
> we can take some freedom, if you don't mind, and declare ret
> inside the for_each, and return 0 here. Just a small cosmetic.

Good idea, changed in v3.

> > +#define skl_pcode_request(i915, mbox, request, reply_mask, reply, timeout_base_ms) \
> > +	intel_gt_pcode_request(&(i915)->gt0, mbox, request, reply_mask, reply, timeout_base_ms)
>
> to_gt(i915)

Not needed in v3 due to interface change to uncore.

> I guess this is just a replacement i915 to gt, I think it's all
> correct and with the latter changed:
>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

I've removed the R-b from this patch due to interface change to uncore
since it's a significant change. I have retained R-b on the following
patches since those changes are just s/gt/gt->uncore/ .

Thanks.
--
Ashutosh