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