[Mesa-dev] [Mesa-stable] [PATCH] i965/skl: Use larger URB size where available.

[Ben Widawsky]

On Thu, Sep 24, 2015 at 11:54:57AM +0100, Emil Velikov wrote:
> Hi Ben,
> 
> On 11 September 2015 at 20:15, Ben Widawsky <ben at bwidawsk.net> wrote:
> > On Fri, Sep 11, 2015 at 12:12:15PM -0700, Jordan Justen wrote:
> >> On 2015-09-10 16:59:12, Ben Widawsky wrote:
> >> > All SKL SKUs except the lowest one which has half the L3 size actually have 384K
> >>
> >> These commit message lines seem to wrap a bit long. This first line is
> >> 80 characters.
> >>
> >> > of URB per slice.
> >> >
> >> > For once, I can explain how this mistake was made and how it was missed in
> >> > review...  Historically when we enable a platform and put the production sizes,
> >> > you can simply look at the "smallest" SKU and see what its URB size is (and we
> >> > assumed it was the 1 slice variant). Since on newer platforms the URB sizes are
> >> > scaled automatically by HW, this was sufficient. On SKL, this is a bit different
> >> > as the lowest SKU actually has half of the L3 fused off. GT2 is the 1 slice (not
> >> > GT1) variant and it has 384K.
> >> >
> >> > There are no Jenkins tests fixed (or regressions) and we don't expect any fixes
> >> > here because you can always run with less URB size - this potentially improves
> >> > performance.
> >>
> >> It would be nice if we were able to find a benchmark that improves
> >> from this change. If we can't then maybe we should just remove this
> >> paragraph. It seems like the right change regardless.
> >>
> >> Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
> >
> > I think what I'd like to do is run the perf data to make sure there are at least
> > no regressions since I am proposing it for stable... Maybe if I don't get around
> > to that before the next stable release, we'll bail on it for 10.6
> >
> Did you get the chance to give this a perf test ? I don't see the
> commit landing in master, regardless of the mesa-stable tag.
> 
> Thanks
> Emil

For other reasons I was unable to collect sufficient performance data. The
commit is in master (c1e38ad37042b0ec261eb0ba5631b7ff0ee7a9da). I think without
performance data we should probably leave it out of stable.

As a side note, if this patch does anything but help our current workloads it'd
be really interesting...