On Sat, Feb 22, 2014 at 11:38:55AM -0800, Ben Widawsky 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. > > > > 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. Right, I think we can have both. We can have RPx for the values we read out of registers since we often have the spec open at the same time. And from those we can store a second set of derived values that make sense for implementing algorithms That way, it should be easy enough to match what we see in the spec to the limits we find and how they are transformed into human readable values, both for exposing to userspace and for driving the gpufreq algorithms. It also means that we can place sanity checks at the end that we are still within the original hw limits. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
