From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 07/12] drm/i915/bdw: Set initial rps freq to RP0 Date: Mon, 24 Mar 2014 12:27:43 -0700 Message-ID: <20140324192742.GA27066@bwidawsk.net> References: <1392692512-2268-1-git-send-email-benjamin.widawsky@intel.com> <1395279079-12704-1-git-send-email-benjamin.widawsky@intel.com> <1395279079-12704-8-git-send-email-benjamin.widawsky@intel.com> <20140320072438.GE4890@nuc-i3427.alporthouse.com> <20140322184217.GB25681@intel.com> <20140322210600.GD4366@nuc-i3427.alporthouse.com> 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 B60DD89CAC for ; Mon, 24 Mar 2014 12:27:57 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140322210600.GD4366@nuc-i3427.alporthouse.com> 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 Sat, Mar 22, 2014 at 09:06:00PM +0000, Chris Wilson wrote: > On Sat, Mar 22, 2014 at 11:42:17AM -0700, Ben Widawsky wrote: > > On Thu, Mar 20, 2014 at 07:24:38AM +0000, Chris Wilson wrote: > > > On Wed, Mar 19, 2014 at 06:31:14PM -0700, Ben Widawsky wrote: > > > > Programming it outside of the rp0-rp1 range is considered a programming > > > > error. Since we do not know that the previous value would actually be in > > > > the range, program something we've read from the hardware, and therefore > > > > know will work. > > > > > > > > This is potentially an issue for platforms whose ranges are outside the > > > > norms given in the programming guide (ie. early silicon) > > > > > > > > v2: Use RP1 instead of RPn > > > > > > > > Signed-off-by: Ben Widawsky > > > > > > Do you have a reference for GEN6_RC_VIDEO_FREQ? I still have no idea > > > what that controls, nor its valid range. > > > -Chris > > > > > > > I have no reference for the video freq other than the brief mention in > > the programming guide, and know nothing more than you do about it. It's > > there because the original spec I had said to program it to 600MHz. The > > reason for /this/ patch was that I noticed the default values happened > > to be a *really* close to our max freq. and figured someone, somewhere > > might get a part whose lower, or upper bound precludes setting the > > constant provided in the programming guide. > > > > Interestingly, the programming guide has been updated since I originally > > wrote this patch to clearly indicate both of these registers need to be > > programmed between Rp1-Rp0. So I guess that means that Rp1-Rp0 is the > > valid range. Therefore, I think we should either proceed with this > > patch, or create a new patch to avoid writing it at all. The current > > code seems like the worst solution of all. > > > > If you want to argue we can drop the write to GEN6_RPNSWREQ since we do > > the correct thing after step 6: > > gen6_set_rps(dev, (I915_READ(GEN6_GT_PERF_STATUS) & 0xff00) >> 8); > > > > I wouldn't be too opposed. I was just trying to follow the spec as > > closely as possible, and it says to write the register value in this > > sequence, so I did. > > Let's mark that as > > Reviewed-by: Chris Wilson > > and move on. Though I may double check to see if I can find some > information on the video frequency. > -Chris > Danvet if/when this is merged, can you please reword the subject: s/RP0/RP1 I'd say it was originally a typo, but that seems unlikely. > -- > 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