[Mesa-dev] i965: Transition the rest of surfaces (i.e., color) to isl

[Jason Ekstrand]

On Fri, Jul 28, 2017 at 10:14 PM, Tomasz Figa <tfiga at chromium.org> wrote:

> On Sat, Jul 29, 2017 at 2:06 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> > On Fri, Jul 28, 2017 at 8:17 PM, Tomasz Figa <tfiga at chromium.org> wrote:
> >>
> >> Hi Topi, Jason,
> >>
> >> On Sat, Jul 22, 2017 at 12:00 AM, Topi Pohjolainen
> >> <topi.pohjolainen at gmail.com> wrote:
> >> > First patch actually should have been included already when
> >> > gen6 stencil got transitioned - it has been giving warning ever
> >> > since.
> >> >
> >> > Most of the work actually got already done for depth surfaces (which
> >> > is y-tiled such as color surfaces). What is left are color surface
> >> > specifics, mostly preparing for corner cases.
> >> >
> >> > This is now all green in ci-system. For snb and older i965 wasn't
> >> > checking hardware incapabilities as hard as isl does. Certain
> >> > format/size/msaa combinations were allowed that shouldn't have.
> >> > Moving to isl exposed code paths that didn't report surface creation
> >> > failures resulting in asserts firing later on. Patches 10 and 11
> >> > now properly tell the client if the surface type can't be supported
> >> > allowing piglit tests to skip them.
> >>
> >> I think it might be related to previously merged patches, but the
> >> topic is still ISL, so let me ask my question here. Is there any
> >> possibility to add some diagnostic information to the validation code?
> >> We've been seeing EGL image import failures on new Mesa as a result of
> >> ISL catching issues in our allocator (cros_gralloc on top of ChromeOS
> >> minigbm), but it's close to impossible to identify the cause without
> >> manually inserting some printfs and recompiling the code. I think
> >> having some error messages printed in case of a buffer validation
> >> failure would be a great benefit.
> >
> >
> > What kind of prints are you looking for exactly?  I've run into some
> issues
> > myself but if you could provide a concrete example, that would help.
>
> For example, we hit two different issues where the total size check in
> intel_create_image_from_fds_common() was failing and the only way to
> figure out the exact reason was digging deep into the calculations in
> ISL:
>
> 1) missing 64-byte padding from the end of linear buffers,
>
> 2) (total) height of the buffer not aligned to 4-lines - actually the
> problem is much more interesting, because this was specifically with
> Android's HAL_PIXEL_FORMAT_YV12 (3-plane YVU420), which doesn't permit
> height alignment higher than 2 defined by the spec. Mesa seems to care
> about total size of the buffer only, so we could work around this by
> simply adding enough padding at the end of the buffer.
>

Right... I was concerned about that when I landed those changes but never
saw a problem in my testing (probably due to piglit always choosing "nice"
sizes).  I've considered simply deleting that code from ISL because I'm not
at all convinced that it's needed.  Chad, do you have any thoughts on
this?  my understanding is that the padding is only needed to avoid page
faults in the GPU and, since every byte of our address space has pages
(thanks to the scratch page) this shouldn't actually be a problem.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170728/b03ab44d/attachment.html>