[Intel-gfx] [PATCH] drm/i915: [GEN7] Use HW scheduler for fixed function shaders

[Daniel Vetter]

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