Re: [PATCH 03/11] drm/i915: Rename and comment all the RPS *stuff*

Ben Widawsky
2014-02-25T00:54:15Z

On Sat, Feb 22, 2014 at 08:08:23PM +0000, Chris Wilson wrote:
> On Sat, Feb 22, 2014 at 11:40:43AM -0800, Ben Widawsky wrote:
> > On Sat, Feb 22, 2014 at 07:37:04PM +0000, Chris Wilson wrote:
> > > On Sat, Feb 22, 2014 at 11:34:16AM -0800, Ben Widawsky wrote:
> > > > On Sat, Feb 22, 2014 at 01:37:16PM +0000, Chris Wilson wrote:
> > > > > On Mon, Feb 17, 2014 at 07:01:44PM -0800, Ben Widawsky wrote:
> > > > > > The names of the struct members for RPS are stupid. Every time I need to
> > > > > > do anything in this code I have to spend a significant amount of time to
> > > > > > remember what it all means. By renaming the variables (and adding the
> > > > > > comments) I hope to clear up the situation. Indeed doing this make some
> > > > > > upcoming patches more readable.
> > > > > >
> > > > > > I've avoided ILK because it's possible that the naming used for Ironlake
> > > > > > matches what is in the docs. I believe the ILK power docs were never
> > > > > > published, and I am too lazy to dig them up.
> > > > > >
> > > > > > While there may be mistakes, this patch was mostly done via sed. The
> > > > > > renaming of "hw_max" required a bit of interactivity.
> > > > >
> > > > > It lost the distinction between RPe and RPn. I am in favour of keeping
> > > > > RP0, RP1, RPe, RPn for the hardware/spec values and adding the set of
> > > > > soft values used for actual interaction.
> > > > > -Chris
> > > > >
> > > >
> > > > And what is the difference between RPe, and RPn? I honestly have no clue
> > > > what RPe is.
> > >
> > > It's a mythical beast on Valleyview for an inflection point in the
> > > power/performance curve that the hardware designers recommend using.
> > > -Chris
> > >
> >
> > Isn't that exactly what RPn is supposed to be?
>
> No, RPn is as low as they allow the hardware to drop the frequency. RPe
> was a different beast for power-efficient rendering. As usual it comes
> without a single piece of substantiating or verifiable piece of
> evidence, nor even if it applies to Linux workloads. Yet it is lore that
> we are expected to follow on faith.
> -Chris

Indeed you are right. I somehow came to believe RPe was equal to RPn,
but I have one platform where RPn is less than RP1, which would mean
that the preferred frequency is lower than the min frequency... so let
me try to fix up the patch based on the comment from the other mail. I
still aim to kill off RP* as much as possible.

>
> --
> Chris Wilson, Intel Open Source Technology Centre

--
Ben Widawsky, Intel Open Source Technology Center