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@xxxxxxxxxxxx> > > > > > > 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@xxxxxxxxxxxxxxxxxx> > > 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ben Widawsky, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
