[Mesa-dev] [RFC PATCH v1 23/30] RFC: anv: Support VkDrmFormatModifierPropertiesListEXT

Jason Ekstrand jason at jlekstrand.net
Tue Nov 7 20:33:12 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_formats.c | 144 ++++++++++++++++++++++++++++++
> +++++++----
>  1 file changed, 132 insertions(+), 12 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_
> formats.c
> index 0dd990bb9a8..dc46fdb5425 100644
> --- a/src/intel/vulkan/anv_formats.c
> +++ b/src/intel/vulkan/anv_formats.c
> @@ -21,6 +21,8 @@
>   * IN THE SOFTWARE.
>   */
>
> +#include <drm_fourcc.h>
> +
>  #include "anv_private.h"
>  #include "vk_enum_to_str.h"
>  #include "vk_format_info.h"
> @@ -425,6 +427,9 @@ anv_get_format_plane(const struct gen_device_info
> *devinfo, VkFormat vk_format,
>        return unsupported;
>
>     if (aspect & (VK_IMAGE_ASPECT_DEPTH_BIT |
> VK_IMAGE_ASPECT_STENCIL_BIT)) {
> +      if (tiling == VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT)
> +         return unsupported;
> +
>        assert(vk_format_aspects(vk_format) &
>               (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT));
>        return plane_format;
> @@ -435,6 +440,18 @@ anv_get_format_plane(const struct gen_device_info
> *devinfo, VkFormat vk_format,
>     const struct isl_format_layout *isl_layout =
>        isl_format_get_layout(plane_format.isl_format);
>
> +   /* For VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT, the image's
> driver-internal
> +    * format must be the user-facing VkFormat. Modifying the VkFormat in
> any
> +    * way, including swizzling, is illegal.
> +    */
>

What are you doing with this comment?  There's no code to go with it.


> +
> +   /* For now, for no reason other than FUD, we decline to support texture
> +    * compression with modifiers.
>
+    */
> +   if (tiling == VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT &&
> +       isl_layout->txc != ISL_TXC_NONE)
> +      return unsupported;
> +
>     if (tiling == VK_IMAGE_TILING_OPTIMAL &&
>         !util_is_power_of_two(isl_layout->bpb)) {
>        /* Tiled formats *must* be power-of-two because we need up upload
> @@ -456,7 +473,8 @@ anv_get_format_plane(const struct gen_device_info
> *devinfo, VkFormat vk_format,
>     /* The B4G4R4A4 format isn't available prior to Broadwell so we have
> to fall
>      * back to a format with a more complex swizzle.
>      */
> -   if (vk_format == VK_FORMAT_B4G4R4A4_UNORM_PACK16 && devinfo->gen < 8)
> {
> +   if (tiling != VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT &&
> +       vk_format == VK_FORMAT_B4G4R4A4_UNORM_PACK16 && devinfo->gen < 8)
> {
>

This is not the only way you can get swizzling.  This one is just a special
case because it's gen-dependent.  Instead, we should just have a thing at
the end which checks whether or not it's a swizzled format and bails.


>        plane_format.isl_format = ISL_FORMAT_B4G4R4A4_UNORM;
>        plane_format.swizzle = ISL_SWIZZLE(GREEN, RED, ALPHA, BLUE);
>     }
> @@ -466,21 +484,29 @@ anv_get_format_plane(const struct gen_device_info
> *devinfo, VkFormat vk_format,
>
>  // Format capabilities
>
> +/**
> + * Parameter drm_format_mod must be DRM_FORMAT_MOD_INVALID unless
> vk_tiling is
> + * VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT.
> + */
>  static VkFormatFeatureFlags
>  get_image_format_features(const struct gen_device_info *devinfo,
>                            VkFormat vk_format,
>                            const struct anv_format *anv_format,
> -                          VkImageTiling vk_tiling)
> +                          VkImageTiling vk_tiling,
> +                          uint64_t drm_format_mod)
>  {
>     VkFormatFeatureFlags flags = 0;
>
> +   if (vk_tiling != VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT)
> +      assert(drm_format_mod == DRM_FORMAT_MOD_INVALID);
> +
>     if (anv_format == NULL)
>        return 0;
>
>     const VkImageAspectFlags aspects = vk_format_aspects(vk_format);
>
>     if (aspects & (VK_IMAGE_ASPECT_DEPTH_BIT |
> VK_IMAGE_ASPECT_STENCIL_BIT)) {
> -      if (vk_tiling == VK_IMAGE_TILING_LINEAR)
> +      if (vk_tiling != VK_IMAGE_TILING_OPTIMAL)
>           return 0;
>
>        flags |= VK_FORMAT_FEATURE_DEPTH_STENCIL_ATTACHMENT_BIT;
> @@ -503,6 +529,17 @@ get_image_format_features(const struct
> gen_device_info *devinfo,
>     if (plane_format.isl_format == ISL_FORMAT_UNSUPPORTED)
>        return 0;
>
> +   const struct isl_format_layout *isl_layout =
> +      isl_format_get_layout(plane_format.isl_format);
> +
> +   if (vk_tiling == VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT)
> +      assert(isl_layout->txc == ISL_TXC_NONE);
> +
> +   /* For VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT, the base format and
> +    * non-base format must be the same, because the image's
> driver-internal
> +    * format must be the user-facing VkFormat. Modifying the VkFormat in
> any
> +    * way, including swizzling, is illegal.
> +    */
>

Again, this comment seems to be dangling.


>     struct anv_format_plane base_plane_format = plane_format;
>     if (vk_tiling == VK_IMAGE_TILING_OPTIMAL) {
>        base_plane_format = anv_get_format_plane(devinfo, vk_format,
> @@ -513,8 +550,8 @@ get_image_format_features(const struct
> gen_device_info *devinfo,
>     enum isl_format base_isl_format = base_plane_format.isl_format;
>
>     /* ASTC textures must be in Y-tiled memory */
> -   if (vk_tiling == VK_IMAGE_TILING_LINEAR &&
> -       isl_format_get_layout(plane_format.isl_format)->txc ==
> ISL_TXC_ASTC)
> +   if (vk_tiling != VK_IMAGE_TILING_OPTIMAL &&
> +       isl_layout->txc == ISL_TXC_ASTC)
>        return 0;
>
>     if (isl_format_supports_sampling(devinfo, plane_format.isl_format)) {
> @@ -569,6 +606,11 @@ get_image_format_features(const struct
> gen_device_info *devinfo,
>     }
>
>     if (anv_format->can_ycbcr) {
> +      if (vk_tiling == VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT) {
> +         /* FINISHME(chadv): Support YUV with DRM format modifiers. */
> +         return 0;
> +      }
> +
>        /* The sampler doesn't have support for mid point when it handles
> YUV on
>         * its own.
>         */
> @@ -608,6 +650,22 @@ get_image_format_features(const struct
> gen_device_info *devinfo,
>        flags &= ~disallowed_ycbcr_image_features;
>     }
>
> +   if (vk_tiling == VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT) {
> +      switch (drm_format_mod) {
> +      default:
> +         unreachable("bad DRM format modifier");
> +      case DRM_FORMAT_MOD_LINEAR:
> +         break;
> +      case I915_FORMAT_MOD_X_TILED:
> +         /* TODO(chadv): Should we support X-tiled storage images? */
>

Storaage images should work fine X-tiled.


> +         flags &= ~VK_FORMAT_FEATURE_STORAGE_IMAGE_BIT;
> +         flags &= ~VK_FORMAT_FEATURE_STORAGE_IMAGE_ATOMIC_BIT;
> +         break;
> +      case I915_FORMAT_MOD_Y_TILED:
> +         break;
> +      }
> +   }
> +
>     return flags;
>  }
>
> @@ -651,6 +709,60 @@ get_buffer_format_features(const struct
> gen_device_info *devinfo,
>     return flags;
>  }
>
> +/**
> + * Fill the VkDrmFormatModifierPropertiesEXT struct if the VkFormat
> supports
> + * the DRM format modifier, and return true.  On failure, the output
> struct
> + * has undefined content.
> + */
> +static bool
> +get_drm_format_modifier_properties(const struct anv_physical_device
> *physical_device,
> +                                   VkFormat vk_format,
> +                                   const struct anv_format *anv_format,
> +                                   uint64_t drm_format_mod,
> +                                   VkDrmFormatModifierPropertiesEXT
> *props)
> +{
> +   const struct gen_device_info *devinfo = &physical_device->info;
> +
> +   *props = (VkDrmFormatModifierPropertiesEXT) {
> +      .drmFormatModifier = drm_format_mod,
> +      .drmFormatModifierPlaneCount = anv_format->n_planes,
> +      .drmFormatModifierTilingFeatures =
> +         get_image_format_features(devinfo, vk_format, anv_format,
> +                                   VK_IMAGE_TILING_DRM_FORMAT_
> MODIFIER_EXT,
> +                                   drm_format_mod),
> +   };
> +
> +   if (props->drmFormatModifierTilingFeatures == 0)
> +      return false;
> +
> +   return true;
> +}
> +
> +static void
> +get_drm_format_modifier_properties_list(const struct anv_physical_device
> *physical_device,
> +                                        VkFormat vk_format,
> +                                        VkDrmFormatModifierPropertiesListEXT
> *drm_list)
>

This is exactly the kind of function I was hoping you'd make so we can
expose it to WSI. :-)


> +{
> +   const struct anv_format *anv_format = anv_get_format(vk_format);
> +
> +   VK_OUTARRAY_MAKE(out, drm_list->pDrmFormatModifierProperties,
> +                    &drm_list->drmFormatModifierCount);
> +
> +   #define TRY_MOD(mod) ({ \
> +      VkDrmFormatModifierPropertiesEXT tmp_props; \
> +      if (get_drm_format_modifier_properties(physical_device, vk_format,
> \
> +                                             anv_format, (mod),
> &tmp_props)) { \
> +         vk_outarray_append(&out, drm_props) { *drm_props = tmp_props; };
> \
> +      } \
> +   })
> +
> +   TRY_MOD(I915_FORMAT_MOD_Y_TILED);
> +   TRY_MOD(I915_FORMAT_MOD_X_TILED);
> +   TRY_MOD(DRM_FORMAT_MOD_LINEAR);
> +
> +   #undef TRY_MOD
> +}
> +
>  void anv_GetPhysicalDeviceFormatProperties(
>      VkPhysicalDevice                            physicalDevice,
>      VkFormat                                    vk_format,
> @@ -663,10 +775,12 @@ void anv_GetPhysicalDeviceFormatProperties(
>     *pFormatProperties = (VkFormatProperties) {
>        .linearTilingFeatures =
>           get_image_format_features(devinfo, vk_format, anv_format,
> -                                   VK_IMAGE_TILING_LINEAR),
> +                                   VK_IMAGE_TILING_LINEAR,
> +                                   DRM_FORMAT_MOD_INVALID),
>        .optimalTilingFeatures =
>           get_image_format_features(devinfo, vk_format, anv_format,
> -                                   VK_IMAGE_TILING_OPTIMAL),
> +                                   VK_IMAGE_TILING_OPTIMAL,
> +                                   DRM_FORMAT_MOD_INVALID),
>        .bufferFeatures =
>           get_buffer_format_features(devinfo, vk_format, anv_format),
>     };
> @@ -674,14 +788,20 @@ void anv_GetPhysicalDeviceFormatProperties(
>
>  void anv_GetPhysicalDeviceFormatProperties2KHR(
>      VkPhysicalDevice                            physicalDevice,
> -    VkFormat                                    format,
> +    VkFormat                                    vk_format,
>      VkFormatProperties2KHR*                     pFormatProperties)
>  {
> -   anv_GetPhysicalDeviceFormatProperties(physicalDevice, format,
> +   ANV_FROM_HANDLE(anv_physical_device, physical_device, physicalDevice);
> +
> +   anv_GetPhysicalDeviceFormatProperties(physicalDevice, vk_format,
>                                           &pFormatProperties->
> formatProperties);
>
>     vk_foreach_struct(ext, pFormatProperties->pNext) {
>        switch (ext->sType) {
> +      case VK_STRUCTURE_TYPE_DRM_FORMAT_MODIFIER_PROPERTIES_LIST_EXT:
> +         get_drm_format_modifier_properties_list(physical_device,
> vk_format,
> +            (VkDrmFormatModifierPropertiesListEXT *)ext);
> +         break;
>        default:
>           anv_debug_ignored_stype(ext->sType);
>           break;
> @@ -696,7 +816,6 @@ anv_get_image_format_properties(
>     VkImageFormatProperties *pImageFormatProperties,
>     VkSamplerYcbcrConversionImageFormatPropertiesKHR
> *pYcbcrImageFormatProperties)
>  {
> -   VkFormatFeatureFlags format_feature_flags;
>     VkExtent3D maxExtent;
>     uint32_t maxMipLevels;
>     uint32_t maxArraySize;
> @@ -707,8 +826,9 @@ anv_get_image_format_properties(
>     if (format == NULL)
>        goto unsupported;
>
> -   format_feature_flags = get_image_format_features(devinfo,
> info->format,
> -                                                    format, info->tiling);
> +   VkFormatFeatureFlags format_feature_flags =
> +      get_image_format_features(devinfo, info->format, format,
> info->tiling,
> +                                DRM_FORMAT_MOD_INVALID);
>
>     switch (info->type) {
>     default:
> --
> 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/70419169/attachment-0001.html>


More information about the mesa-dev mailing list