<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Nov 29, 2017 at 2:24 PM, Chad Versace <span dir="ltr"><<a href="mailto:chadversary@chromium.org" target="_blank">chadversary@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 28 Nov 2017, Jason Ekstrand wrote:<br>
> This lets us set the BO tiling when we allocate the memory.  This is<br>
> required for GL to work properly.<br>
> ---<br>
>  src/intel/vulkan/anv_device.c | 53 ++++++++++++++++++++++++++++++<wbr>+++++++++----<br>
>  1 file changed, 49 insertions(+), 4 deletions(-)<br>
><br>
> diff --git a/src/intel/vulkan/anv_device.<wbr>c b/src/intel/vulkan/anv_device.<wbr>c<br>
> index df929e4..d82d1f7 100644<br>
> --- a/src/intel/vulkan/anv_device.<wbr>c<br>
> +++ b/src/intel/vulkan/anv_device.<wbr>c<br>
> @@ -29,6 +29,7 @@<br>
>  #include <unistd.h><br>
>  #include <fcntl.h><br>
>  #include <xf86drm.h><br>
> +#include <drm_fourcc.h><br>
><br>
>  #include "anv_private.h"<br>
>  #include "util/strtod.h"<br>
> @@ -1612,6 +1613,40 @@ VkResult anv_AllocateMemory(<br>
>                                    &mem->bo);<br>
>        if (result != VK_SUCCESS)<br>
>           goto fail;<br>
> +<br>
> +      const VkMemoryDedicatedAllocateInfoK<wbr>HR *dedicated_info =<br>
> +         vk_find_struct_const(<wbr>pAllocateInfo->pNext, MEMORY_DEDICATED_ALLOCATE_<wbr>INFO_KHR);<br>
> +      if (dedicated_info && dedicated_info->image != VK_NULL_HANDLE) {<br>
> +         ANV_FROM_HANDLE(anv_image, image, dedicated_info->image);<br>
> +<br>
> +         /* For images using modifiers, we require a dedicated allocation<br>
> +          * and we set the BO tiling to match the tiling of the underlying<br>
> +          * modifier.  This is a bit unfortunate as this is completely<br>
> +          * pointless for Vulkan.  However, GL needs to be able to map things<br>
> +          * so it needs the tiling to be set.  The only way to do this in a<br>
> +          * non-racy way is to set the tiling in the creator of the BO so that<br>
> +          * makes it our job.<br>
> +          *<br>
> +          * One of these days, once the GL driver learns to not map things<br>
> +          * through the GTT in random places, we can drop this and start<br>
> +          * allowing multiple modified images in the same BO.<br>
> +          */<br>
> +         if (image->drm_format_mod != DRM_FORMAT_MOD_INVALID) {<br>
> +            assert(isl_drm_modifier_get_<wbr>info(image->drm_format_mod)-><wbr>tiling ==<br>
> +                   image->planes[0].surface.isl.<wbr>tiling);<br>
> +            const uint32_t i915_tiling =<br>
> +               isl_tiling_to_i915_tiling(<wbr>image->planes[0].surface.isl.<wbr>tiling);<br>
> +            int ret = anv_gem_set_tiling(device, mem->bo->gem_handle,<br>
> +                                         image->planes[0].surface.isl.<wbr>row_pitch,<br>
> +                                         i915_tiling);<br>
> +            if (ret) {<br>
> +               anv_bo_cache_release(device, &device->bo_cache, mem->bo);<br>
> +               return vk_errorf(device->instance, NULL,<br>
> +                                VK_ERROR_OUT_OF_DEVICE_MEMORY,<br>
> +                                "failed to set BO tiling: %m");<br>
> +            }<br>
> +         }<br>
> +      }<br>
>     }<br>
<br>
</div></div>This hunk of code is run only for non-imported memory. It's in the<br>
'else' branch of `if (fd_info && fd_info->handleTypes)`. I believe it<br>
needs to be run in both branches. Otherwise, if an app creates<br>
a dma_buf, imports it into Vulkan, binds the image to the dma_buf, and<br>
afterwards imports the same dma_buf into GL as an EGLImage, then<br>
intel_create_image_from_fds_<wbr>common() will fail to discover the bo's<br>
tiling.<br></blockquote><div><br></div><div>Yes, that was a bit intentional because setting it any time other than image creation is racy.  If we were going to set it on import as well, we may as well just do that in the GL driver and call it a day.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Everything else in this patch looks correct to me, even though it makes<br>
me sad.<br>
</blockquote></div></div><div class="gmail_extra"><br></div><div class="gmail_extra">Agreed. :(</div><div class="gmail_extra"><br></div><div class="gmail_extra">--Jason<br></div></div>