[Mesa-dev] [PATCH 11/13] anv: introduce device compatibility checks for format query
andrey simiklit
asimiklit.work at gmail.com
Tue Nov 6 14:23:26 UTC 2018
Hello,
Please find my comment below:
On Mon, Nov 5, 2018 at 5:36 PM Lionel Landwerlin <
lionel.g.landwerlin at intel.com> wrote:
> We add a function that allows us to check whether a particular
> description of a Vulkan format is available for a given device.
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
> src/intel/vulkan/anv_formats.c | 65 ++++++++++++++++++++++++-------
> src/intel/vulkan/anv_image.c | 4 +-
> src/intel/vulkan/anv_private.h | 4 +-
> src/intel/vulkan/vk_format_info.h | 2 +-
> 4 files changed, 57 insertions(+), 18 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_formats.c
> b/src/intel/vulkan/anv_formats.c
> index 250af958d2e..c23fc1c5b1f 100644
> --- a/src/intel/vulkan/anv_formats.c
> +++ b/src/intel/vulkan/anv_formats.c
> @@ -42,6 +42,16 @@
> ISL_CHANNEL_SELECT_##a, \
> }
>
> +static bool all_gens_compatible(const struct gen_device_info *devinfo)
> +{
> + return true;
> +}
> +
> +static bool gen11_compatible(const struct gen_device_info *devinfo)
> +{
> + return devinfo->gen >= 11;
> +}
> +
> #define RGBA _ISL_SWIZZLE(RED, GREEN, BLUE, ALPHA)
> #define BGRA _ISL_SWIZZLE(BLUE, GREEN, RED, ALPHA)
> #define RGB1 _ISL_SWIZZLE(RED, GREEN, BLUE, ONE)
> @@ -55,6 +65,7 @@
> }, \
> }, \
> .n_planes = 1, \
> + .compatible = all_gens_compatible, \
> }
>
> #define fmt1(__hw_fmt) \
> @@ -69,6 +80,7 @@
> }, \
> }, \
> .n_planes = 1, \
> + .compatible = all_gens_compatible, \
> }
>
> #define s_fmt(__hw_fmt) \
> @@ -80,6 +92,7 @@
> }, \
> }, \
> .n_planes = 1, \
> + .compatible = all_gens_compatible, \
> }
>
> #define ds_fmt(__fmt1, __fmt2) \
> @@ -95,6 +108,7 @@
> }, \
> }, \
> .n_planes = 2, \
> + .compatible = all_gens_compatible, \
> }
>
> #define fmt_unsupported \
> @@ -130,6 +144,17 @@
> }, \
> .n_planes = __n_planes, \
> .can_ycbcr = true, \
> + .compatible = all_gens_compatible, \
> + }
> +
> +#define ycbcr_gen_fmt(__n_planes, __compatible, ...) \
> + { \
> + .planes = { \
> + __VA_ARGS__, \
> + }, \
> + .n_planes = __n_planes, \
> + .can_ycbcr = true, \
> + .compatible = __compatible, \
> }
>
> #define fmt_list(__vk_fmt, ...) \
> @@ -335,12 +360,22 @@ static const struct anv_format *main_formats[] = {
> };
>
> static const struct anv_format *ycbcr_formats[] = {
> + /**
> + * On Gen10 and below, the HW sampler won't allow us to have 2
> different
> + * view of a same buffer. This was changed on Gen11, so we can now
> make the
> + * 2 format below use 2 planes. This gives more flexibility in terms
> of how
> + * we want to same chroma components.
> + */
> fmt_list(VK_FORMAT_G8B8G8R8_422_UNORM,
> - ycbcr_fmt(1,
> - y_plane(ISL_FORMAT_YCRCB_SWAPUV, RGBA,
> _ISL_SWIZZLE(BLUE, GREEN, RED, ZERO)))),
> + ycbcr_gen_fmt(2, gen11_compatible,
> + y_plane(ISL_FORMAT_R8G8_UNORM, RGBA,
> _ISL_SWIZZLE(GREEN, ZERO, ZERO, ZERO)),
> + chroma_plane(0, ISL_FORMAT_R8G8B8A8_UNORM,
> RGBA, _ISL_SWIZZLE(ZERO, BLUE, ZERO, RED), 2, 1)),
> + ycbcr_fmt(1, y_plane(ISL_FORMAT_YCRCB_SWAPUV, RGBA,
> _ISL_SWIZZLE(BLUE, GREEN, RED, ZERO)))),
> fmt_list(VK_FORMAT_B8G8R8G8_422_UNORM,
> - ycbcr_fmt(1,
> - y_plane(ISL_FORMAT_YCRCB_SWAPUVY, RGBA,
> _ISL_SWIZZLE(BLUE, GREEN, RED, ZERO)))),
> + ycbcr_gen_fmt(2, gen11_compatible,
> + y_plane(ISL_FORMAT_R8G8_UNORM, RGBA,
> _ISL_SWIZZLE(ZERO, GREEN, ZERO, ZERO)),
> + chroma_plane(0, ISL_FORMAT_R8G8B8A8_UNORM,
> RGBA, _ISL_SWIZZLE(BLUE, ZERO, RED, ZERO), 2, 1)),
> + ycbcr_fmt(1, y_plane(ISL_FORMAT_YCRCB_SWAPUVY, RGBA,
> _ISL_SWIZZLE(BLUE, GREEN, RED, ZERO)))),
> fmt_list(VK_FORMAT_G8_B8_R8_3PLANE_420_UNORM,
> ycbcr_fmt(3,
> y_plane(ISL_FORMAT_R8_UNORM, RGBA,
> _ISL_SWIZZLE(GREEN, ZERO, ZERO, ZERO)),
> @@ -436,7 +471,7 @@ static const struct {
> };
>
> const struct anv_format *
> -anv_get_format(VkFormat vk_format)
> +anv_get_format(const struct gen_device_info *devinfo, VkFormat vk_format)
> {
> uint32_t enum_offset = VK_ENUM_OFFSET(vk_format);
> uint32_t ext_number = VK_ENUM_EXTENSION(vk_format);
> @@ -450,17 +485,19 @@ anv_get_format(VkFormat vk_format)
> if (!format)
> return NULL;
>
> - if (format[0].n_planes == 0)
> - return NULL;
> + for (int i = 0; format[i].n_planes != 0; i++) {
> + if (!devinfo || format[i].compatible(devinfo))
>
I may be wrong but seems like it could be checked just once before this
loop:
if (!devinfo)
return (format[0].n_planes != 0) ? &format[0] : NULL;
I guess it is not really matter but anyway just for case if you make v2
patch for some reason :-)
> + return &format[i];
> + }
>
> - return &format[0];
> + return NULL;
> }
>
> struct anv_format_plane
> anv_get_format_nth_plane(const struct gen_device_info *devinfo, VkFormat
> vk_format,
> uint32_t plane, VkImageTiling tiling)
> {
> - const struct anv_format *format = anv_get_format(vk_format);
> + const struct anv_format *format = anv_get_format(devinfo, vk_format);
> const struct anv_format_plane unsupported = {
> .isl_format = ISL_FORMAT_UNSUPPORTED,
> };
> @@ -520,7 +557,7 @@ anv_get_format_plane(const struct gen_device_info
> *devinfo, VkFormat vk_format,
> VkImageAspectFlagBits aspect, VkImageTiling tiling)
> {
> uint32_t plane =
> - anv_format_aspect_to_plane(anv_get_format(vk_format), aspect);
> + anv_format_aspect_to_plane(anv_get_format(devinfo, vk_format),
> aspect);
> return anv_get_format_nth_plane(devinfo, vk_format, plane, tiling);
> }
>
> @@ -746,7 +783,7 @@ get_wsi_format_modifier_properties_list(const struct
> anv_physical_device *physic
> VkFormat vk_format,
> struct
> wsi_format_modifier_properties_list *list)
> {
> - const struct anv_format *anv_format = anv_get_format(vk_format);
> + const struct anv_format *anv_format =
> anv_get_format(&physical_device->info, vk_format);
>
> VK_OUTARRAY_MAKE(out, list->modifier_properties,
> &list->modifier_count);
>
> @@ -789,7 +826,7 @@ void anv_GetPhysicalDeviceFormatProperties(
> {
> ANV_FROM_HANDLE(anv_physical_device, physical_device, physicalDevice);
> const struct gen_device_info *devinfo = &physical_device->info;
> - const struct anv_format *anv_format = anv_get_format(vk_format);
> + const struct anv_format *anv_format = anv_get_format(devinfo,
> vk_format);
>
> *pFormatProperties = (VkFormatProperties) {
> .linearTilingFeatures =
> @@ -839,7 +876,7 @@ anv_get_image_format_properties(
> uint32_t maxArraySize;
> VkSampleCountFlags sampleCounts = VK_SAMPLE_COUNT_1_BIT;
> const struct gen_device_info *devinfo = &physical_device->info;
> - const struct anv_format *format = anv_get_format(info->format);
> + const struct anv_format *format = anv_get_format(devinfo,
> info->format);
>
> if (format == NULL)
> goto unsupported;
> @@ -1188,7 +1225,7 @@ VkResult anv_CreateSamplerYcbcrConversion(
>
> memset(conversion, 0, sizeof(*conversion));
>
> - conversion->format = anv_get_format(pCreateInfo->format);
> + conversion->format = anv_get_format(&device->info,
> pCreateInfo->format);
> conversion->ycbcr_model = pCreateInfo->ycbcrModel;
> conversion->ycbcr_range = pCreateInfo->ycbcrRange;
> conversion->mapping[0] = pCreateInfo->components.r;
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index d0cfd4deef3..d33ca427c5d 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -583,7 +583,7 @@ anv_image_create(VkDevice _device,
> image->type = pCreateInfo->imageType;
> image->extent = pCreateInfo->extent;
> image->vk_format = pCreateInfo->format;
> - image->format = anv_get_format(pCreateInfo->format);
> + image->format = anv_get_format(&device->info, pCreateInfo->format);
> image->aspects = vk_format_aspects(image->vk_format);
> image->levels = pCreateInfo->mipLevels;
> image->array_size = pCreateInfo->arrayLayers;
> @@ -1397,7 +1397,7 @@ anv_CreateImageView(VkDevice _device,
> * in expanded_aspects to the image view.
> */
> const struct anv_format *vformat =
> - anv_get_format(iview->vk_format);
> + anv_get_format(&device->info, iview->vk_format);
> bool iplane_bound[3] = { false, };
> for (uint32_t vfplane = 0; vfplane < vformat->n_planes; vfplane++) {
> if ((iview->aspect_mask & vformat->planes[vfplane].aspect) == 0)
> diff --git a/src/intel/vulkan/anv_private.h
> b/src/intel/vulkan/anv_private.h
> index a64db7d952c..4f63bc539ff 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -2559,6 +2559,8 @@ struct anv_format {
> struct anv_format_plane planes[3];
> uint8_t n_planes;
> bool can_ycbcr;
> +
> + bool (*compatible)(const struct gen_device_info *devinfo);
> };
>
> uint32_t
> @@ -2595,7 +2597,7 @@ anv_plane_to_aspect(VkImageAspectFlags image_aspects,
> for_each_bit(b, anv_image_expand_aspects(image, aspects))
>
> const struct anv_format *
> -anv_get_format(VkFormat format);
> +anv_get_format(const struct gen_device_info *devinfo, VkFormat format);
>
> struct anv_format_plane
> anv_get_format_nth_plane(const struct gen_device_info *devinfo, VkFormat
> vk_format,
> diff --git a/src/intel/vulkan/vk_format_info.h
> b/src/intel/vulkan/vk_format_info.h
> index 9bcef17b4da..3799b5c3b99 100644
> --- a/src/intel/vulkan/vk_format_info.h
> +++ b/src/intel/vulkan/vk_format_info.h
> @@ -30,7 +30,7 @@
> static inline VkImageAspectFlags
> vk_format_aspects(VkFormat vk_format)
> {
> - const struct anv_format *format = anv_get_format(vk_format);
> + const struct anv_format *format = anv_get_format(NULL, vk_format);
> VkImageAspectFlags aspects = 0;
> for (int p = 0; p < format->n_planes; p++)
> aspects |= format->planes[p].aspect;
> --
> 2.19.1
>
> _______________________________________________
> 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/20181106/be1ba618/attachment-0001.html>
More information about the mesa-dev
mailing list