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@xxxxxxxxxxxxxxx>
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
