[Intel-gfx] [PATCH 07/12] drm/i915/bdw: Set initial rps freq to RP0

[Ben Widawsky]

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 <ben at bwidawsk.net>
> > > 
> > > 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 <chris at chris-wilson.co.uk>
> 
> 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center