Re: [Intel-gfx] [PATCH 1/4] drm/i915: Use GEN12_RPSTAT register for GT freq

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

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