[Mesa-dev] [RFC PATCH v1 27/30] RFC: anv: Support VkImageExplicitDrmFormatModifierCreateInfoEXT

Jason Ekstrand jason at jlekstrand.net
Tue Nov 7 21:00:24 UTC 2017


On Tue, Nov 7, 2017 at 6:48 AM, Chad Versace <chadversary at chromium.org>
wrote:

> Incremental implementation of VK_EXT_image_drm_format_modifier.
> ---
>  src/intel/vulkan/anv_image.c | 84 ++++++++++++++++++++++++++++++
> +++++++++-----
>  1 file changed, 75 insertions(+), 9 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index ec6cdbc6168..bf636ce4b65 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -288,6 +288,7 @@ static VkResult
>  make_surface(const struct anv_device *dev,
>               struct anv_image *image,
>               const struct anv_image_create_info *anv_info,
> +             const VkImageExplicitDrmFormatModifierCreateInfoEXT
> *explicit_drm_info,
>               isl_tiling_flags_t tiling_flags,
>               VkImageAspectFlagBits aspect)
>  {
> @@ -308,9 +309,16 @@ make_surface(const struct anv_device *dev,
>        anv_get_format_plane(&dev->info, image->vk_format, aspect,
> image->tiling);
>     struct anv_surface *anv_surf = &image->planes[plane].surface;
>
> +   const VkSubresourceLayout *drm_plane_layout = explicit_drm_info ?
> +      &explicit_drm_info->pPlaneLayouts[plane] : NULL;
> +
>     const isl_surf_usage_flags_t usage =
>        choose_isl_surf_usage(vk_info, anv_info->isl_extra_usage_flags,
> aspect);
>
> +   uint32_t row_pitch = anv_info->stride;
> +   if (explicit_drm_info)
> +      row_pitch = drm_plane_layout->rowPitch;
> +
>     /* If an image is created as BLOCK_TEXEL_VIEW_COMPATIBLE, then we need
> to
>      * fall back to linear on Broadwell and earlier because we aren't
>      * guaranteed that we can handle offsets correctly.  On Sky Lake, the
> @@ -336,18 +344,71 @@ make_surface(const struct anv_device *dev,
>        .array_len = vk_info->arrayLayers,
>        .samples = vk_info->samples,
>        .min_alignment = 0,
> -      .row_pitch = anv_info->stride,
> +      .row_pitch = row_pitch,
>        .usage = usage,
>        .tiling_flags = tiling_flags);
>
> -   /* isl_surf_init() will fail only if provided invalid input. Invalid
> input
> -    * is illegal in Vulkan.
> -    */
> -   assert(ok);
> +   if (!ok) {
> +      /* isl_surf_init() fails only when provided invalid input. Invalid
> input
> +       * is illegal in Vulkan unless
> +       * VkImageExplicitDrmFormatModifierCreateInfoEXT is given.
> +       */
> +      assert(explicit_drm_info);
> +      return vk_errorf(dev->instance, dev,
> +                       VK_ERROR_INVALID_DRM_FORMAT_
> MODIFIER_PLANE_LAYOUT_EXT,
> +                       "isl_surf_init() failed for plane %u", plane);
> +   }
>
> -   image->planes[plane].aux_usage = ISL_AUX_USAGE_NONE;
> +   if (explicit_drm_info) {
> +      /* The VK_EXT_image_drm_format_modifier spec permits support of any
> +       * image, but we restrict support to simple images.
> +       */
> +      assert(aspect == VK_IMAGE_ASPECT_COLOR_BIT);
> +      assert(image->type == VK_IMAGE_TYPE_2D);
> +      assert(image->array_size == 1);
> +      assert(image->samples == 1);
> +
> +      /* FINISHME: YCbCr images with DRM format modifiers */
> +      assert(!anv_get_format(image->vk_format)->can_ycbcr);
> +
> +      if (drm_plane_layout->size < anv_surf->isl.size) {
> +         return vk_errorf(dev->instance, dev,
> +                          VK_ERROR_INVALID_DRM_FORMAT_
> MODIFIER_PLANE_LAYOUT_EXT,
> +                          "VkSubresourceLayout::size too small for plane
> %u", plane);
> +      }
>
> -   add_surface(image, anv_surf, plane);
> +      if (drm_plane_layout->offset & (anv_surf->isl.alignment - 1)) {
> +         return vk_errorf(dev->instance, dev,
> +                          VK_ERROR_INVALID_DRM_FORMAT_
> MODIFIER_PLANE_LAYOUT_EXT,
> +                          "VkSubresourceLayout::offset misaligned for
> plane "
> +                          "%u", plane);
> +      }
> +
> +      if (drm_plane_layout->arrayPitch != 0) {
> +         return vk_errorf(dev->instance, dev,
> +                          VK_ERROR_INVALID_DRM_FORMAT_
> MODIFIER_PLANE_LAYOUT_EXT,
> +                          "VkSubresourceLayout::arrayPitch must be 0");
> +      }
> +
> +      if (drm_plane_layout->depthPitch != 0) {
> +         return vk_errorf(dev->instance, dev,
> +                          VK_ERROR_INVALID_DRM_FORMAT_
> MODIFIER_PLANE_LAYOUT_EXT,
> +                          "VkSubresourceLayout::depthPitch must be 0");
> +      }
> +
> +      anv_surf->offset = drm_plane_layout->offset;
> +
> +      image->planes[plane].offset = drm_plane_layout->offset;
> +      image->planes[plane].alignment = anv_surf->isl.alignment;
> +      image->planes[plane].size = drm_plane_layout->size;
> +
> +      image->size = image->planes[plane].offset +
> image->planes[plane].size;
> +      image->alignment = image->planes[plane].alignment;
>

This all looks correct for now, but I'm having trouble seeing how CCS will
fit into all of this.


> +   } else {
> +      add_surface(image, anv_surf, plane);
> +   }
> +
> +   image->planes[plane].aux_usage = ISL_AUX_USAGE_NONE;
>
>     /* If an image is created as BLOCK_TEXEL_VIEW_COMPATIBLE, then we need
> to
>      * create an identical tiled shadow surface for use while texturing so
> we
> @@ -543,6 +604,7 @@ anv_image_create(VkDevice _device,
>     ANV_FROM_HANDLE(anv_device, device, _device);
>     const VkImageCreateInfo *pCreateInfo = create_info->vk_info;
>     const VkImageDrmFormatModifierListCreateInfoEXT *vk_mod_list = NULL;
> +   const VkImageExplicitDrmFormatModifierCreateInfoEXT
> *explicit_drm_info = NULL;
>     const struct isl_drm_modifier_info *isl_mod_info = NULL;
>     struct anv_image *image = NULL;
>     VkResult r;
> @@ -556,6 +618,10 @@ anv_image_create(VkDevice _device,
>           vk_mod_list = (const VkImageDrmFormatModifierListCreateInfoEXT
> *) s;
>           isl_mod_info = choose_drm_format_mod(vk_mod_list);
>           break;
> +      case VK_STRUCTURE_TYPE_IMAGE_EXCPLICIT_DRM_FORMAT_MODIFIER_
> CREATE_INFO_EXT:
> +         explicit_drm_info = (const VkImageExplicitDrmFormatModifierCreateInfoEXT
> *) s;
> +         isl_mod_info = isl_drm_modifier_get_info(explicit_drm_info->
> drmFormatModifier);
>

Let's assert isl_mod_info == NULL both here and in the above case as well.
I'd like to prevent anyone from trying to use both at the same time.


> +         break;
>        default:
>           anv_debug_ignored_stype(s->sType);
>           break;
> @@ -599,8 +665,8 @@ anv_image_create(VkDevice _device,
>
>     uint32_t b;
>     for_each_bit(b, image->aspects) {
> -      r = make_surface(device, image, create_info, isl_tiling_flags,
> -                       (1 << b));
> +      r = make_surface(device, image, create_info, explicit_drm_info,
> +                       isl_tiling_flags, (1 << b));
>        if (r != VK_SUCCESS)
>           goto fail;
>     }
> --
> 2.13.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171107/636b4653/attachment-0001.html>


More information about the mesa-dev mailing list