[Mesa-dev] [PATCH v4] anv: add VK_EXT_sampler_filter_minmax support

[Lionel Landwerlin]

On 17/08/18 17:57, Yunchao He wrote:
> This extension can be supported on SKL+. With this patch,
> all corresponding tests (6K+) in CTS can pass. No test fails.
>
> I verified CTS with the command below:
> deqp-vk --deqp-case=dEQP-VK.pipeline.sampler.view_type.*reduce*
>
> v2: 1) support all depth formats, not depth-only formats, 2) fix
> a wrong indention (Jason).
>
> v3: fix a few nits (Lionel).
>
> v4: fix failures in CI: disable sampler reduction mode when compare
> mode in sampler state is enabled (Lionel).
> ---
>   src/intel/vulkan/anv_device.c      |  8 ++++++++
>   src/intel/vulkan/anv_extensions.py |  1 +
>   src/intel/vulkan/anv_formats.c     |  6 ++++++
>   src/intel/vulkan/genX_state.c      | 26 ++++++++++++++++++++++++++
>   4 files changed, 41 insertions(+)
>
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index 04fd6a829e..e45ba4b3af 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -1116,6 +1116,14 @@ void anv_GetPhysicalDeviceProperties2(
>            break;
>         }
>   
> +      case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SAMPLER_FILTER_MINMAX_PROPERTIES_EXT: {
> +         VkPhysicalDeviceSamplerFilterMinmaxPropertiesEXT *properties =
> +            (VkPhysicalDeviceSamplerFilterMinmaxPropertiesEXT *)ext;
> +         properties->filterMinmaxImageComponentMapping = pdevice->info.gen >= 9;
> +         properties->filterMinmaxSingleComponentFormats = true;
> +         break;
> +      }
> +
>         case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SUBGROUP_PROPERTIES: {
>            VkPhysicalDeviceSubgroupProperties *properties = (void *)ext;
>   
> diff --git a/src/intel/vulkan/anv_extensions.py b/src/intel/vulkan/anv_extensions.py
> index ea837744b4..e165bd371d 100644
> --- a/src/intel/vulkan/anv_extensions.py
> +++ b/src/intel/vulkan/anv_extensions.py
> @@ -125,6 +125,7 @@ EXTENSIONS = [
>       Extension('VK_EXT_shader_stencil_export',             1, 'device->info.gen >= 9'),
>       Extension('VK_EXT_vertex_attribute_divisor',          2, True),
>       Extension('VK_EXT_post_depth_coverage',               1, 'device->info.gen >= 9'),
> +    Extension('VK_EXT_sampler_filter_minmax',             1, 'device->info.gen >= 9'),
>   ]
>   
>   class VkVersion:
> diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c
> index 815b320a82..33faf7cc37 100644
> --- a/src/intel/vulkan/anv_formats.c
> +++ b/src/intel/vulkan/anv_formats.c
> @@ -489,6 +489,9 @@ get_image_format_features(const struct gen_device_info *devinfo,
>         if (aspects == VK_IMAGE_ASPECT_DEPTH_BIT || devinfo->gen >= 8)
>            flags |= VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT;
>   
> +      if ((aspects & VK_IMAGE_ASPECT_DEPTH_BIT) && devinfo->gen >= 9)
> +         flags |= VK_FORMAT_FEATURE_SAMPLED_IMAGE_FILTER_MINMAX_BIT_EXT;
> +
>         flags |= VK_FORMAT_FEATURE_BLIT_SRC_BIT |
>                  VK_FORMAT_FEATURE_BLIT_DST_BIT |
>                  VK_FORMAT_FEATURE_TRANSFER_SRC_BIT_KHR |
> @@ -521,6 +524,9 @@ get_image_format_features(const struct gen_device_info *devinfo,
>      if (isl_format_supports_sampling(devinfo, plane_format.isl_format)) {
>         flags |= VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT;
>   
> +      if (devinfo->gen >= 9)
> +         flags |= VK_FORMAT_FEATURE_SAMPLED_IMAGE_FILTER_MINMAX_BIT_EXT;
> +
>         if (isl_format_supports_filtering(devinfo, plane_format.isl_format))
>            flags |= VK_FORMAT_FEATURE_SAMPLED_IMAGE_FILTER_LINEAR_BIT;
>      }
> diff --git a/src/intel/vulkan/genX_state.c b/src/intel/vulkan/genX_state.c
> index b1014d9e79..cc64fe6d4a 100644
> --- a/src/intel/vulkan/genX_state.c
> +++ b/src/intel/vulkan/genX_state.c
> @@ -245,6 +245,14 @@ static const uint32_t vk_to_gen_shadow_compare_op[] = {
>      [VK_COMPARE_OP_ALWAYS]                       = PREFILTEROPNEVER,
>   };
>   
> +#if GEN_GEN >= 9
> +static const uint32_t vk_to_gen_sampler_reduction_mode[] = {
> +   [VK_SAMPLER_REDUCTION_MODE_WEIGHTED_AVERAGE_EXT] = STD_FILTER,
> +   [VK_SAMPLER_REDUCTION_MODE_MIN_EXT]              = MINIMUM,
> +   [VK_SAMPLER_REDUCTION_MODE_MAX_EXT]              = MAXIMUM,
> +};
> +#endif
> +
>   VkResult genX(CreateSampler)(
>       VkDevice                                    _device,
>       const VkSamplerCreateInfo*                  pCreateInfo,
> @@ -266,6 +274,10 @@ VkResult genX(CreateSampler)(
>      uint32_t border_color_offset = device->border_colors.offset +
>                                     pCreateInfo->borderColor * 64;
>   
> +#if GEN_GEN >= 9
> +   unsigned sampler_reduction_mode = STD_FILTER;
> +#endif
> +
>      vk_foreach_struct(ext, pCreateInfo->pNext) {
>         switch (ext->sType) {
>         case VK_STRUCTURE_TYPE_SAMPLER_YCBCR_CONVERSION_INFO: {
> @@ -281,6 +293,15 @@ VkResult genX(CreateSampler)(
>            sampler->conversion = conversion;
>            break;
>         }
> +#if GEN_GEN >= 9
> +      case VK_STRUCTURE_TYPE_SAMPLER_REDUCTION_MODE_CREATE_INFO_EXT: {
> +         struct VkSamplerReductionModeCreateInfoEXT *sampler_reduction =
> +            (struct VkSamplerReductionModeCreateInfoEXT *) ext;
> +         sampler_reduction_mode =
> +            vk_to_gen_sampler_reduction_mode[sampler_reduction->reductionMode];
> +         break;
> +      }
> +#endif
>         default:
>            anv_debug_ignored_stype(ext->sType);
>            break;
> @@ -348,6 +369,11 @@ VkResult genX(CreateSampler)(
>            .TCXAddressControlMode = vk_to_gen_tex_address[pCreateInfo->addressModeU],
>            .TCYAddressControlMode = vk_to_gen_tex_address[pCreateInfo->addressModeV],
>            .TCZAddressControlMode = vk_to_gen_tex_address[pCreateInfo->addressModeW],
> +
> +#if GEN_GEN >= 9
> +         .ReductionType = sampler_reduction_mode,
> +         .ReductionTypeEnable = !pCreateInfo->compareEnable,
> +#endif

Hey Yunchao,

I think we should just disable the reduction if it's not specified 
through pNext.
Although there are restriction on the kind of reduction that can be set 
when compareEnable is true, the spec doesn't say that it should disable 
reduction.

Thanks,

-
Lionel

>         };
>   
>         GENX(SAMPLER_STATE_pack)(NULL, sampler->state[p], &sampler_state);