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

Ben Widawsky
2014-03-22T18:42:17Z

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@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.

--
Ben Widawsky, Intel Open Source Technology Center