On Wed, 19 Oct 2022 08:06:26 -0700, Rodrigo Vivi wrote:
>
Hi Rodrigo,
> On Tue, Oct 18, 2022 at 10:20:40PM -0700, Ashutosh Dixit wrote:
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index 36d95b79022c0..a7a0129d0e3fc 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -1543,6 +1543,8 @@
> >
> > #define GEN12_RPSTAT1 _MMIO(0x1381b4)
> > #define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
> > +#define GEN12_CAGF_SHIFT 11
>
> we don't need to define the shift if we use the REG_FIELD_GET
Yes I was also suggesting this but then went ahead with the mask/shift
based code to match previous style in the function.
In any case based on your suggestions I have added a new patch is series
version v8 which converts all previous branches in intel_rps_get_cagf to
REG_FIELD_GET so that the new code can also consistently use REG_FIELD_GET.
>
> > +#define GEN12_CAGF_MASK REG_GENMASK(19, 11)
>
> ah, cool, this is already right and in place
> (ignore my comment about this in the other patch)
> > u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
> > {
> > struct drm_i915_private *i915 = rps_to_i915(rps);
> > u32 cagf;
> >
> > - if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> > + if (GRAPHICS_VER(i915) >= 12)
> > + cagf = (rpstat & GEN12_CAGF_MASK) >> GEN12_CAGF_SHIFT;
>
> cagf = REG_FIELD_GET(GEN12_CAGF_MASK, rpstat);
>
> > + else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> > cagf = (rpstat >> 8) & 0xff;
> > else if (GRAPHICS_VER(i915) >= 9)
> > cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
Thanks.
--
Ashutosh
