On Wed, Apr 18, 2012 at 5:33 PM, Daniel Vetter <daniel at ffwll.ch> wrote: > Jesse, can you also please check whether we need the same thing on vlv? > atm the the clock gating functions are almost identical safe for this wrt > touching registers in the gt core. Prod. Now that you have a vlv, no more excuses ;-) -Daniel > -Daniel > > On Sun, Apr 15, 2012 at 10:14:24AM -0700, Ben Widawsky wrote: >> On Sun, 15 Apr 2012 11:55:36 -0300 >> Eugeni Dodonov <eugeni at dodonov.net> wrote: >> >> > On Sat, Apr 14, 2012 at 22:41, Ben Widawsky <ben at bwidawsk.net> wrote: >> > >> > > This originally started as a patch from Bernard as a way of simply >> > > setting the VS scheduler. After submitting the RFC patch, we >> > > decided to also modify the DS scheduler. To be most explicit, I've >> > > made the patch explicitly set all scheduler modes, and included the >> > > defines for other modes (in case someone feels frisky later). >> > > >> > > The rest of the story gets a bit weird. The first version of the >> > > patch showed an almost unbelievable performance improvement. Since >> > > rebasing my branch it appears the performance improvement has gone, >> > > unfortunately. But setting these bits seem to be the right thing to >> > > do given that the docs describe corruption that can occur with the >> > > default settings. >> > > >> > > In summary, I am seeing no more perf improvements (or regressions) >> > > in my limited testing, but we believe this should be set to prevent >> > > rendering corruption, therefore cc stable. >> > > >> > > v1: Clear bit 4 also (Ken + Eugeni) >> > > Do a full clear + set of the bits we want (Me). >> > > >> > > Cc: Bernard Kilarski <bernard.r.kilarski at intel.com> >> > > Cc: stable <stable at vger.kernel.org> >> > > Reviewed-by (RFC): Kenneth Graunke <kenneth at whitecape.org> >> > > Signed-off-by: Ben Widawsky <benjamin.widawsky at intel.com> >> > > >> > >> > Very nice! >> > >> > I also suspect that maybe the initial performance improvement you've >> > seen with previous testing could be related to the occasional turbo >> > disabling we've been seeing in other cases as well (e.g., >> > https://bugs.freedesktop.org/show_bug.cgi?id=44006). >> > >> > But as for this patch, I have just one comment/suggestion below, but >> > other than that: >> > >> > Reviewed-by: Eugeni Dodonov <eugeni.dodonov at intel.com> >> > >> > +static void gen7_setup_fixed_func_scheduler(struct drm_i915_private >> > > *dev_priv) >> > > >> > >> > Perhaps this functions should be named >> > ivybridge_setup_fixed_func_scheduler instead? >> > >> > Even if those bits are not ivy bridge-exclusive, this specific >> > explicit setup applies to ivb only.. >> > >> >> I wasn't sure if we wanted this for VLV or not. In fact, originally the >> patch did call this in the VLV setup, but since I decided to CC stable >> (per Ken's idea) I removed the VLV part. >> >> If Jesse, or someone could confirm we don't want this for VLV, I agree >> with your commen, and I'd probably just go back and inline the register >> write. FWIW, it does *seem* like we don't want to set this on HSW. >> >> Anyway, thanks for your review. >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx at lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Mail: daniel at ffwll.ch > Mobile: +41 (0)79 365 57 48 -- Daniel Vetter daniel.vetter at ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch
