On Mon, 21 Mar 2022 11:17:46 -0700, Lucas De Marchi wrote:
>
> On Mon, Mar 21, 2022 at 10:56:04AM -0700, Ashutosh Dixit wrote:
> > diff --git a/drivers/gpu/drm/i915/gt/intel_rps_types.h b/drivers/gpu/drm/i915/gt/intel_rps_types.h
> > index 3941d8551f52..5990df35b393 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_rps_types.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_rps_types.h
> > @@ -37,6 +37,13 @@ enum {
> > INTEL_RPS_TIMER,
> > };
> >
> > +/* Freq caps exposed by HW, in units of 16.67 MHz for recent gens */
>
> "recent gens" is probably too broad. Since we are exporting this struct
> to other parts of the driver and we are already abstracting the
> register location and bit position, maybe we should also already
> abstract the unit in the same place? i.e. just make it always be
> "units of 16.67 MHz", or even just make it a standard Hz unit.
>
> If this would rather make things more complicated for places that expect
> "hw units", then maybe just say in this comment the value is in "HW
> units" and intel_gpu_freq() should be used to convert to hz.
OK, let me investigate some more, see what's possible and then get back
regarding this. Thanks for reviewing.
