[Mesa-dev] [PATCH] intel/isl: Stop padding surfaces

[Jason Ekstrand]

On Wed, Aug 2, 2017 at 8:23 AM, Tomasz Figa <tfiga at chromium.org> wrote:

> On Tue, Aug 1, 2017 at 3:27 AM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> > The docs contain a bunch of commentary about the need to pad various
> > surfaces out to multiples of something or other.  However, all of those
> > requirements are about avoiding GTT errors due to missing pages when the
> > data port or sampler accesses slightly out-of-bounds.  However, because
> > the kernel already fills all the empty space in our GTT with the scratch
> > page, we never have to worry about faulting due to OOB reads.  There are
> > two caveats to this:
> >
> >  1) There is some potential for issues with caches here if extra data
> >     ends up in a cache we don't expect due to OOB reads.  However,
> >     because we always trash the entire cache whenever we need to move
> >     anything between cache domains, this shouldn't be an issue.
> >
> >  2) There is a potential issue if a surface gets placed at the very top
> >     of the GTT by the kernel.  In this case, the hardware could
> >     potentially end up trying to read past the top of the GTT.  If it
> >     nicely wraps around at the 48-bit (or 32-bit) boundary, then this
> >     shouldn't be an issue thanks to the scratch page.  If it doesn't,
> >     then we need to come up with something to handle it.
> >
> > Up until some of the GL move to ISL, having the padding code in there
> > just caused us to harmlessly use a bit more memory in Vulkan.  However,
> > now that we're using ISL sizes to validate external dma-buf images,
> > these padding requirements are causing us to reject otherwise valid
> > images due to the size of the BO being too small.
> >
> > Cc: "17.2" <mesa-stable at lists.freedesktop.org>
> > Cc: Chad Versace <chadversary at chromium.org>
> > Cc: Tomasz Figa <tfiga at chromium.org>
> > Cc: Tapani Palli <tapani.palli at intel.com>
> > ---
> >  src/intel/isl/isl.c | 119 +-----------------------------
> ----------------------
> >  1 file changed, 2 insertions(+), 117 deletions(-)
> >
>
> Did some testing on our systems and didn't seem to regress anything
> (obviously), but neither was enough to actually make post-ISL Mesa
> accept buffers that pre-ISL Mesa would normally accept and which had
> otherwise worked fine for us. For example YVU420 with height not
> aligned to 4, which is especially important since Android's YV12
> format is actually YVU420 with further restrictions, such as requested
> height not being aligned further than to the nearest multiple of 2.
>

Ugh... Ok, I think I found the issue.  I'll send it out after a bit.

--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170802/0bf9340d/attachment.html>