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

[Tomasz Figa]

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.
Anyway, for what the patch does:

Tested-by: Tomasz Figa <tfiga at chromium.org>

Best regards,
Tomasz