[Intel-gfx] [PATCH] [v3] drm/i915/hsw: Change default LLC age to 3

[Ben Widawsky]

On Mon, Aug 05, 2013 at 01:03:29AM +0200, Daniel Vetter wrote:
> On Sun, Aug 4, 2013 at 10:55 PM, Ben Widawsky <ben at bwidawsk.net> wrote:
> > On Sun, Aug 04, 2013 at 09:10:25PM +0200, Daniel Vetter wrote:
> >> On Fri, Aug 2, 2013 at 12:56 AM, Ben Widawsky <ben at bwidawsk.net> wrote:
> >> > The default LLC age was changed:
> >> > commit 0d8ff15e9a15f2b393e53337a107b7a1e5919b6d
> >> > Author: Ben Widawsky <benjamin.widawsky at intel.com>
> >> > Date:   Thu Jul 4 11:02:03 2013 -0700
> >> >
> >> > drm/i915/hsw: Set correct Haswell PTE encodings.
> >> >
> >> > This caused a regression in performance on certain benchmarks. While I
> >> > think a discussion still needs to happen about how the kernel should
> >> > default for both eLLC, and LLC - just revert this behavior for now.
> >> >
> >> > v2: Drop the extra #define (Chad)
> >> >
> >> > v3: Actually git add
> >> >
> >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67062
> >> > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> >> > Reviewed-by: Chad Versace <chad.versace at linux.intel.com>
> >>
> >> iris_pte_encode was split up after the regressing commit in
> >>
> >> commit 4d15c145a6234d999c0452eec0d275c1fbf0688c
> >> Author: Ben Widawsky <ben at bwidawsk.net>
> >> Date:   Thu Jul 4 11:02:06 2013 -0700
> >>
> >>     drm/i915: Use eLLC/LLC by default when available
> >>
> >> Hence I think that one should be patched, too (QA doesn't have an iris
> >> machine afaik, so no regression report expected). And since I have
> >> some open questions about Chris WT-for-Iris patch too I think it's
> >> best to spin a v4 of this one here.
> >> -Daniel
> >>
> >
> > We've come up with a theory as to why this was a regression (which I
> > think nullifies your request for the eLLC version)
> >
> > On the surface, one would think that aging all objects equally, whether
> > they be oldest, or youngest, makes no difference. The key with LLC is
> > that it is shared with the CPU, and presumably the CPU follows the
> > normal cache aging FIFO rules (ie. everything is aged 3). On this logic,
> > eLLC would not share the same flaw. I think it makes sense to have
> > someone prove we need it for eLLC, as this does allow mesa/DDX to get 2
> > differently aged eLLC objects, which they cannot do if we default to 3.
> > On the same note, even LLC age 0 is still ideal in my opinion, but was
> > asked to not do this so that meas 9.2 doesn't have performance
> > regressions.
> 
> Well we have a get/set_caching ioctl, explictly with room for stuff
> like aging.

It's like someone wrote such patches almost a year ago, and they were
rejected as overly complicated by some maintainer.

> So this can be added in any case. But yeah if someone can
> check whether we have the same regression on iris or not would be
> good. Can you please poke QA (or whoever has one of these boxen)?
> -Daniel

Well, forgetting regressions for just one sec, what do you think makes
the most sense? If you think it should be 3, it's probably not even
worth the effort since I barely care - and I'm happy to just change it..

-- 
Ben Widawsky, Intel Open Source Technology Center