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

[Chris Wilson]

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

-- 
Chris Wilson, Intel Open Source Technology Centre