From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 03/11] drm/i915: Rename and comment all the RPS *stuff* Date: Tue, 18 Mar 2014 19:38:43 -0700 Message-ID: <20140319023842.GA31496@bwidawsk.net> References: <1390969547-1018-2-git-send-email-benjamin.widawsky@intel.com> <1392692512-2268-1-git-send-email-benjamin.widawsky@intel.com> <1392692512-2268-4-git-send-email-benjamin.widawsky@intel.com> <20140222133716.GS17020@nuc-i3427.alporthouse.com> <20140319012703.GC7956@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail.bwidawsk.net (bwidawsk.net [166.78.191.112]) by gabe.freedesktop.org (Postfix) with ESMTP id 7ABA44A2F4 for ; Tue, 18 Mar 2014 19:38:58 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140319012703.GC7956@bwidawsk.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Ben Widawsky , Intel GFX List-Id: intel-gfx@lists.freedesktop.org On Tue, Mar 18, 2014 at 06:27:03PM -0700, 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 > > > > Okay, as stated before, you are correct - I need to bring back RPe/RPn > distinction. I think using the mix of values (basically s/_delay/_freq) > doesn't fully relize what I was hoping to achieve. I don't think there > is ever a case, except when debugging where it's easier to refer to the > RP mnemonic. How strongly do you feel about this one? I'd really prefer > to just fix RPe/RPn. > > Does anyone else have an opinion on: > "max_freq_hardlimit" vs. "rp0" > > Does anyone else want to review this one? > Okay, I started on this, and I somewhat agree. How about: u8 cur_freq; /* Current frequency (cached, may not == HW) */ u8 min_freq_softlimit; /* Minimum frequency permitted by the driver */ u8 max_freq_softlimit; /* Max frequency permitted by the driver */ u8 max_freq; /* Maximum frequency, RP0 if not overclocking */ u8 min_freq; /* AKA RPn. Minimum frequency */ u8 efficient_freq; /* AKA RPe. Pre-determined balanced frequency */ u8 rp1_freq; /* "less than" RP0 power/freqency */ u8 rp0_freq; /* Non-overclocked max frequency. */ Conveniently, this matches sysfs, minus the efficiency one, and I don't think there's a point in explicitly storing RPn, since it's always == min_freq. > > -- > > Chris Wilson, Intel Open Source Technology Centre > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ben Widawsky, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ben Widawsky, Intel Open Source Technology Center