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

Ben Widawsky
2014-02-22T19:39:01Z

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.
>

I sent too soon. One other thing I want to make clear with a patch [like
this] is what the min and max values always are. Names RP0, and RP1,
while matching the spec do not do this. They have no meaning to anyone
not well versed in the spec.

When one wants to find out min/max, or rp0/rp1, they'd always have to do
a conversion from one to the other by looking at the code (unless we
added a #define or something to alias it). My point really is, it's
almost always more useful to know MIN and MAX, and if you must figure
out if it's RP0, or RP1, then go back to the code to find it.

Still would like an answer about RPe...

--
Ben Widawsky, Intel Open Source Technology Center