[Mesa-dev] [PATCH v4 12/12] anv: enable VK_KHR_sampler_ycbcr_conversion
Jason Ekstrand
jason at jlekstrand.net
Thu Oct 5 21:50:31 UTC 2017
Two trivial comments which you can take or leave below. With that, 1-10
and 12 are
Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
Once we sort out patch 11, we should be good to land. Thanks for the good
work!
--Jason
On Thu, Oct 5, 2017 at 9:30 AM, Lionel Landwerlin <
lionel.g.landwerlin at intel.com> wrote:
> v2: Make GetImageMemoryRequirements2KHR() iterate over all pInfo
> structs (Lionel)
> Handle VkSamplerYcbcrConversionImageFormatPropertiesKHR (Andrew/Jason)
> Iterator over BindImageMemory2KHR's pNext structs correctly (Jason)
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
> src/intel/vulkan/anv_device.c | 51 +++++++++++++++++++++++-
> src/intel/vulkan/anv_extensions.py | 1 +
> src/intel/vulkan/anv_formats.c | 82 ++++++++++++++++++++++++++++++
> ++++----
> src/intel/vulkan/anv_image.c | 21 +++++++++-
> src/intel/vulkan/anv_private.h | 4 ++
> src/intel/vulkan/genX_state.c | 50 ++++++++++++++++++-----
> 6 files changed, 189 insertions(+), 20 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index d576bb55315..798fdff3c89 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -703,6 +703,13 @@ void anv_GetPhysicalDeviceFeatures2KHR(
> break;
> }
>
> + case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SAMPLER_YCBCR_CONVERSION_FEATURES_KHR:
> {
> + VkPhysicalDeviceSamplerYcbcrConversionFeaturesKHR *features =
> + (VkPhysicalDeviceSamplerYcbcrConversionFeaturesKHR *) ext;
> + features->samplerYcbcrConversion = true;
> + break;
> + }
> +
> default:
> anv_debug_ignored_stype(ext->sType);
> break;
> @@ -1826,8 +1833,48 @@ void anv_GetImageMemoryRequirements2KHR(
> const VkImageMemoryRequirementsInfo2KHR* pInfo,
> VkMemoryRequirements2KHR* pMemoryRequirements)
> {
> - anv_GetImageMemoryRequirements(_device, pInfo->image,
> - &pMemoryRequirements->
> memoryRequirements);
> + vk_foreach_struct_const(ext, pInfo) {
> + switch (ext->sType) {
> + case VK_STRUCTURE_TYPE_IMAGE_MEMORY_REQUIREMENTS_INFO_2_KHR: {
> + anv_GetImageMemoryRequirements(_device, pInfo->image,
> + &pMemoryRequirements->
> memoryRequirements);
> + break;
>
In general, I prefer to handle pInfo directly and only use foreach_struct
for the extensions. I don't care too much though.
> + }
> + case VK_STRUCTURE_TYPE_IMAGE_PLANE_MEMORY_REQUIREMENTS_INFO_KHR: {
> + ANV_FROM_HANDLE(anv_image, image, pInfo->image);
> + ANV_FROM_HANDLE(anv_device, device, _device);
> + struct anv_physical_device *pdevice = &device->instance->
> physicalDevice;
> + const VkImagePlaneMemoryRequirementsInfoKHR *plane_reqs =
> + (const VkImagePlaneMemoryRequirementsInfoKHR *) ext;
> + uint32_t plane = anv_image_aspect_to_plane(image->aspects,
> +
> plane_reqs->planeAspect);
> +
> + assert(image->planes[plane].offset == 0);
> +
> + /* The Vulkan spec (git aaed022) says:
> + *
> + * memoryTypeBits is a bitfield and contains one bit set for
> every
> + * supported memory type for the resource. The bit `1<<i` is
> set
> + * if and only if the memory type `i` in the
> + * VkPhysicalDeviceMemoryProperties structure for the
> physical
> + * device is supported.
> + *
> + * All types are currently supported for images.
> + */
> + pMemoryRequirements->memoryRequirements.memoryTypeBits =
> + (1ull << pdevice->memory.type_count) - 1;
> +
> + pMemoryRequirements->memoryRequirements.size =
> image->planes[plane].size;
> + pMemoryRequirements->memoryRequirements.alignment =
> + image->planes[plane].alignment;
> + break;
> + }
> +
> + default:
> + anv_debug_ignored_stype(ext->sType);
> + break;
> + }
> + }
>
> vk_foreach_struct(ext, pMemoryRequirements->pNext) {
> switch (ext->sType) {
> diff --git a/src/intel/vulkan/anv_extensions.py b/src/intel/vulkan/anv_
> extensions.py
> index 491e7086838..a828a668d65 100644
> --- a/src/intel/vulkan/anv_extensions.py
> +++ b/src/intel/vulkan/anv_extensions.py
> @@ -74,6 +74,7 @@ EXTENSIONS = [
> Extension('VK_KHR_push_descriptor', 1, True),
> Extension('VK_KHR_relaxed_block_layout', 1, True),
> Extension('VK_KHR_sampler_mirror_clamp_to_edge', 1, True),
> + Extension('VK_KHR_sampler_ycbcr_conversion', 1, True),
> Extension('VK_KHR_shader_draw_parameters', 1, True),
> Extension('VK_KHR_storage_buffer_storage_class', 1, True),
> Extension('VK_KHR_surface', 25,
> 'ANV_HAS_SURFACE'),
> diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_
> formats.c
> index 879eb072b10..1d92b29c7e6 100644
> --- a/src/intel/vulkan/anv_formats.c
> +++ b/src/intel/vulkan/anv_formats.c
> @@ -683,7 +683,8 @@ static VkResult
> anv_get_image_format_properties(
> struct anv_physical_device *physical_device,
> const VkPhysicalDeviceImageFormatInfo2KHR *info,
> - VkImageFormatProperties *pImageFormatProperties)
> + VkImageFormatProperties *pImageFormatProperties,
> + uint32_t *out_planes)
>
Why not just make this a VkSamplerYcbcrConversionImageFormatPropertiesKHR
*? It doesn't really matter but it seems a tiny bit cleaner.
> {
> VkFormatProperties format_props;
> VkFormatFeatureFlags format_feature_flags;
> @@ -816,6 +817,9 @@ anv_get_image_format_properties(
> .maxResourceSize = UINT32_MAX,
> };
>
> + if (out_planes)
> + *out_planes = format->n_planes;
> +
> return VK_SUCCESS;
>
> unsupported:
> @@ -852,7 +856,7 @@ VkResult anv_GetPhysicalDeviceImageFormatProperties(
> };
>
> return anv_get_image_format_properties(physical_device, &info,
> - pImageFormatProperties);
> + pImageFormatProperties, NULL);
> }
>
> static const VkExternalMemoryPropertiesKHR prime_fd_props = {
> @@ -874,13 +878,9 @@ VkResult anv_GetPhysicalDeviceImageFormatPr
> operties2KHR(
> ANV_FROM_HANDLE(anv_physical_device, physical_device, physicalDevice);
> const VkPhysicalDeviceExternalImageFormatInfoKHR *external_info =
> NULL;
> VkExternalImageFormatPropertiesKHR *external_props = NULL;
> + VkSamplerYcbcrConversionImageFormatPropertiesKHR *ycbcr_props = NULL;
> VkResult result;
>
> - result = anv_get_image_format_properties(physical_device, base_info,
> - &base_props->imageFormatProperties);
> - if (result != VK_SUCCESS)
> - goto fail;
> -
> /* Extract input structs */
> vk_foreach_struct_const(s, base_info->pNext) {
> switch (s->sType) {
> @@ -899,12 +899,21 @@ VkResult anv_GetPhysicalDeviceImageFormatPr
> operties2KHR(
> case VK_STRUCTURE_TYPE_EXTERNAL_IMAGE_FORMAT_PROPERTIES_KHR:
> external_props = (void *) s;
> break;
> + case VK_STRUCTURE_TYPE_SAMPLER_YCBCR_CONVERSION_IMAGE_FORMAT_
> PROPERTIES_KHR:
> + ycbcr_props = (void *) s;
> + break;
> default:
> anv_debug_ignored_stype(s->sType);
> break;
> }
> }
>
> + result = anv_get_image_format_properties(physical_device, base_info,
> + &base_props->imageFormatProperties,
> + ycbcr_props ? &ycbcr_props->combinedImageSamplerDescriptorCount
> : NULL);
> + if (result != VK_SUCCESS)
> + goto fail;
> +
> /* From the Vulkan 1.0.42 spec:
> *
> * If handleType is 0, vkGetPhysicalDeviceImageFormatProperties2KHR
> will
> @@ -1006,3 +1015,62 @@ void anv_GetPhysicalDeviceExternalBuffe
> rPropertiesKHR(
> pExternalBufferProperties->externalMemoryProperties =
> (VkExternalMemoryPropertiesKHR) {0};
> }
> +
> +VkResult anv_CreateSamplerYcbcrConversionKHR(
> + VkDevice _device,
> + const VkSamplerYcbcrConversionCreateInfoKHR* pCreateInfo,
> + const VkAllocationCallbacks* pAllocator,
> + VkSamplerYcbcrConversionKHR* pYcbcrConversion)
> +{
> + ANV_FROM_HANDLE(anv_device, device, _device);
> + struct anv_ycbcr_conversion *conversion;
> +
> + assert(pCreateInfo->sType == VK_STRUCTURE_TYPE_SAMPLER_
> YCBCR_CONVERSION_CREATE_INFO_KHR);
> +
> + conversion = vk_alloc2(&device->alloc, pAllocator,
> sizeof(*conversion), 8,
> + VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
> + if (!conversion)
> + return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> +
> + memset(conversion, 0, sizeof(*conversion));
> +
> + conversion->format = anv_get_format(pCreateInfo->format);
> + conversion->ycbcr_model = pCreateInfo->ycbcrModel;
> + conversion->ycbcr_range = pCreateInfo->ycbcrRange;
> + conversion->mapping[0] = pCreateInfo->components.r;
> + conversion->mapping[1] = pCreateInfo->components.g;
> + conversion->mapping[2] = pCreateInfo->components.b;
> + conversion->mapping[3] = pCreateInfo->components.a;
> + conversion->chroma_offsets[0] = pCreateInfo->xChromaOffset;
> + conversion->chroma_offsets[1] = pCreateInfo->yChromaOffset;
> + conversion->chroma_filter = pCreateInfo->chromaFilter;
> +
> + bool has_chroma_subsampled = false;
> + for (uint32_t p = 0; p < conversion->format->n_planes; p++) {
> + if (conversion->format->planes[p].has_chroma &&
> + (conversion->format->planes[p].denominator_scales[0] > 1 ||
> + conversion->format->planes[p].denominator_scales[1] > 1))
> + has_chroma_subsampled = true;
> + }
> + conversion->chroma_reconstruction = has_chroma_subsampled &&
> + (conversion->chroma_offsets[0] == VK_CHROMA_LOCATION_COSITED_EVEN_KHR
> ||
> + conversion->chroma_offsets[1] == VK_CHROMA_LOCATION_COSITED_
> EVEN_KHR);
> +
> + *pYcbcrConversion = anv_ycbcr_conversion_to_handle(conversion);
> +
> + return VK_SUCCESS;
> +}
> +
> +void anv_DestroySamplerYcbcrConversionKHR(
> + VkDevice _device,
> + VkSamplerYcbcrConversionKHR YcbcrConversion,
> + const VkAllocationCallbacks* pAllocator)
> +{
> + ANV_FROM_HANDLE(anv_device, device, _device);
> + ANV_FROM_HANDLE(anv_ycbcr_conversion, conversion, YcbcrConversion);
> +
> + if (!conversion)
> + return;
> +
> + vk_free2(&device->alloc, pAllocator, conversion);
> +}
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index 3845ae0713c..770c096c8aa 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -515,6 +515,7 @@ anv_image_create(VkDevice _device,
> image->samples = pCreateInfo->samples;
> image->usage = pCreateInfo->usage;
> image->tiling = pCreateInfo->tiling;
> + image->disjoint = pCreateInfo->flags & VK_IMAGE_CREATE_DISJOINT_BIT_
> KHR;
>
> const struct anv_format *format = anv_get_format(image->vk_format);
> assert(format != NULL);
> @@ -612,9 +613,25 @@ VkResult anv_BindImageMemory2KHR(
> const VkBindImageMemoryInfoKHR *bind_info = &pBindInfos[i];
> ANV_FROM_HANDLE(anv_device_memory, mem, bind_info->memory);
> ANV_FROM_HANDLE(anv_image, image, bind_info->image);
> - uint32_t aspect_bit;
> + VkImageAspectFlags aspects = image->aspects;
> +
> + vk_foreach_struct_const(s, bind_info->pNext) {
> + switch (s->sType) {
> + case VK_STRUCTURE_TYPE_BIND_IMAGE_PLANE_MEMORY_INFO_KHR: {
> + const VkBindImagePlaneMemoryInfoKHR *plane_info =
> + (const VkBindImagePlaneMemoryInfoKHR *) s;
>
> - anv_foreach_image_aspect_bit(aspect_bit, image, image->aspects) {
> + aspects = plane_info->planeAspect;
> + break;
> + }
> + default:
> + anv_debug_ignored_stype(s->sType);
> + break;
> + }
> + }
> +
> + uint32_t aspect_bit;
> + anv_foreach_image_aspect_bit(aspect_bit, image, aspects) {
> uint32_t plane =
> anv_image_aspect_to_plane(image->aspects, 1UL << aspect_bit);
> anv_image_bind_memory_plane(device, image, plane,
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> private.h
> index 4fd716d6a6f..d12e2478a48 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -2338,6 +2338,9 @@ struct anv_image {
> VkDeviceSize size;
> uint32_t alignment;
>
> + /* Whether the image is made of several underlying buffer objects
> rather a
> + * single one with different offsets.
> + */
> bool disjoint;
>
> /**
> @@ -2858,6 +2861,7 @@ ANV_DEFINE_NONDISP_HANDLE_CASTS(anv_sampler,
> VkSampler)
> ANV_DEFINE_NONDISP_HANDLE_CASTS(anv_semaphore, VkSemaphore)
> ANV_DEFINE_NONDISP_HANDLE_CASTS(anv_shader_module, VkShaderModule)
> ANV_DEFINE_NONDISP_HANDLE_CASTS(anv_debug_report_callback,
> VkDebugReportCallbackEXT)
> +ANV_DEFINE_NONDISP_HANDLE_CASTS(anv_ycbcr_conversion,
> VkSamplerYcbcrConversionKHR)
>
> /* Gen-specific function declarations */
> #ifdef genX
> diff --git a/src/intel/vulkan/genX_state.c b/src/intel/vulkan/genX_state.c
> index 91da05cddbf..b7e4e5bceab 100644
> --- a/src/intel/vulkan/genX_state.c
> +++ b/src/intel/vulkan/genX_state.c
> @@ -33,6 +33,8 @@
> #include "genxml/gen_macros.h"
> #include "genxml/genX_pack.h"
>
> +#include "vk_util.h"
> +
> VkResult
> genX(init_device_state)(struct anv_device *device)
> {
> @@ -176,12 +178,44 @@ VkResult genX(CreateSampler)(
> uint32_t border_color_offset = device->border_colors.offset +
> pCreateInfo->borderColor * 64;
>
> - bool enable_min_filter_addr_rounding =
> - pCreateInfo->minFilter != VK_FILTER_NEAREST;
> - bool enable_mag_filter_addr_rounding =
> - pCreateInfo->magFilter != VK_FILTER_NEAREST;
> + vk_foreach_struct(ext, pCreateInfo->pNext) {
> + switch (ext->sType) {
> + case VK_STRUCTURE_TYPE_SAMPLER_YCBCR_CONVERSION_INFO_KHR: {
> + VkSamplerYcbcrConversionInfoKHR *pSamplerConversion =
> + (VkSamplerYcbcrConversionInfoKHR *) ext;
> + ANV_FROM_HANDLE(anv_ycbcr_conversion, conversion,
> + pSamplerConversion->conversion);
> +
> + if (conversion == NULL)
> + break;
> +
> + sampler->n_planes = conversion->format->n_planes;
> + sampler->conversion = conversion;
> + break;
> + }
> + default:
> + anv_debug_ignored_stype(ext->sType);
> + break;
> + }
> + }
>
> for (unsigned p = 0; p < sampler->n_planes; p++) {
> + const bool plane_has_chroma =
> + sampler->conversion && sampler->conversion->format->
> planes[p].has_chroma;
> + const VkFilter min_filter =
> + plane_has_chroma ? sampler->conversion->chroma_filter :
> pCreateInfo->minFilter;
> + const VkFilter mag_filter =
> + plane_has_chroma ? sampler->conversion->chroma_filter :
> pCreateInfo->magFilter;
> + const bool enable_min_filter_addr_rounding = min_filter !=
> VK_FILTER_NEAREST;
> + const bool enable_mag_filter_addr_rounding = mag_filter !=
> VK_FILTER_NEAREST;
> + /* From Broadwell PRM, SAMPLER_STATE:
> + * "Mip Mode Filter must be set to MIPFILTER_NONE for Planar YUV
> surfaces."
> + */
> + const uint32_t mip_filter_mode =
> + (sampler->conversion &&
> + isl_format_is_yuv(sampler->conversion->format->planes[0].isl_format))
> ?
> + MIPFILTER_NONE : vk_to_gen_mipmap_mode[pCreateInfo->mipmapMode];
> +
> struct GENX(SAMPLER_STATE) sampler_state = {
> .SamplerDisable = false,
> .TextureBorderColorMode = DX10OGL,
> @@ -195,11 +229,9 @@ VkResult genX(CreateSampler)(
> #if GEN_GEN == 8
> .BaseMipLevel = 0.0,
> #endif
> - .MipModeFilter = vk_to_gen_mipmap_mode[pCreateInfo->mipmapMode],
> - .MagModeFilter = vk_to_gen_tex_filter(pCreateInfo->magFilter,
> -
> pCreateInfo->anisotropyEnable),
> - .MinModeFilter = vk_to_gen_tex_filter(pCreateInfo->minFilter,
> -
> pCreateInfo->anisotropyEnable),
> + .MipModeFilter = mip_filter_mode,
> + .MagModeFilter = vk_to_gen_tex_filter(mag_filter,
> pCreateInfo->anisotropyEnable),
> + .MinModeFilter = vk_to_gen_tex_filter(min_filter,
> pCreateInfo->anisotropyEnable),
> .TextureLODBias = anv_clamp_f(pCreateInfo->mipLodBias, -16,
> 15.996),
> .AnisotropicAlgorithm = EWAApproximation,
> .MinLOD = anv_clamp_f(pCreateInfo->minLod, 0, 14),
> --
> 2.14.2
>
> _______________________________________________
> 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/20171005/c5e69f7b/attachment-0001.html>
More information about the mesa-dev
mailing list