Module: Mesa Branch: staging/23.0 Commit: 76f3dbde624a31af394b37e3a7428799de1a863b URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=76f3dbde624a31af394b37e3a7428799de1a863b
Author: Lionel Landwerlin <[email protected]> Date: Tue Apr 4 13:32:53 2023 +0300 isl: fix a number of errors on storage format support on Gfx9/12.5 Signed-off-by: Lionel Landwerlin <[email protected]> Cc: mesa-stable Reviewed-by: Ivan Briano <[email protected]> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22302> (cherry picked from commit d4f498a5835f20fde39c0980268dcfbf7b4037b8) --- .pick_status.json | 2 +- src/intel/isl/isl.h | 3 +- src/intel/isl/isl_format.c | 12 +++---- src/intel/isl/isl_storage_image.c | 68 ++++++++++++++++++++++++++---------- src/intel/vulkan/anv_formats.c | 2 +- src/intel/vulkan/anv_image.c | 4 +-- src/intel/vulkan_hasvk/anv_formats.c | 2 +- src/intel/vulkan_hasvk/anv_image.c | 4 +-- 8 files changed, 65 insertions(+), 32 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 2151ff1464d..feea2958e97 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -2674,7 +2674,7 @@ "description": "isl: fix a number of errors on storage format support on Gfx9/12.5", "nominated": true, "nomination_type": 0, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": null, "notes": null diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h index f3abbd5db0e..511c9473637 100644 --- a/src/intel/isl/isl.h +++ b/src/intel/isl/isl.h @@ -2078,7 +2078,8 @@ void isl_color_value_unpack(union isl_color_value *value, enum isl_format format, const uint32_t *data_in); -bool isl_is_storage_image_format(enum isl_format fmt); +bool isl_is_storage_image_format(const struct intel_device_info *devinfo, + enum isl_format fmt); enum isl_format isl_lower_storage_image_format(const struct intel_device_info *devinfo, diff --git a/src/intel/isl/isl_format.c b/src/intel/isl/isl_format.c index db8d146b4e2..6425f9ac875 100644 --- a/src/intel/isl/isl_format.c +++ b/src/intel/isl/isl_format.c @@ -147,12 +147,12 @@ static const struct surface_format_info format_info[] = { SF( x, x, x, x, x, x, Y, x, x, x, x, x, x, R32G32_USCALED) SF( x, x, x, x, x, x, 75, x, x, x, x, x, x, R32G32_SFIXED) SF( x, x, x, x, x, x, 80, x, x, x, x, x, 90, R64_PASSTHRU) - SF( Y, Y, x, Y, Y, Y, Y, x, 60, 70, x, 90, x, B8G8R8A8_UNORM) + SF( Y, Y, x, Y, Y, Y, Y, x, 60, 70, 125, 90, x, B8G8R8A8_UNORM) SF( Y, Y, x, x, Y, Y, x, x, x, x, x, 110, x, B8G8R8A8_UNORM_SRGB) /* smpl filt shad CK RT AB VB SO color TW TR ccs_e TA */ - SF( Y, Y, x, x, Y, Y, Y, x, 60, 70, x, 110, x, R10G10B10A2_UNORM) + SF( Y, Y, x, x, Y, Y, Y, x, 60, 70, 125, 110, x, R10G10B10A2_UNORM) SF( Y, Y, x, x, x, x, x, x, 60, x, x, 120, x, R10G10B10A2_UNORM_SRGB) - SF( Y, x, x, x, Y, x, Y, x, x, 70, x, 110, x, R10G10B10A2_UINT) + SF( Y, x, x, x, Y, x, Y, x, x, 70, 125, 110, x, R10G10B10A2_UINT) SF( Y, Y, x, x, x, x, Y, x, x, x, x, x, x, R10G10B10_SNORM_A2_UNORM) SF( Y, Y, x, x, Y, Y, Y, x, 60, 70, 110, 90, x, R8G8B8A8_UNORM) SF( Y, Y, x, x, Y, Y, x, x, 60, x, x, 110, x, R8G8B8A8_UNORM_SRGB) @@ -164,10 +164,10 @@ static const struct surface_format_info format_info[] = { SF( Y, x, x, x, Y, x, Y, x, x, 70, 90, 90, x, R16G16_SINT) SF( Y, x, x, x, Y, x, Y, x, x, 70, 75, 90, x, R16G16_UINT) SF( Y, Y, x, x, Y, Y, Y, x, x, 70, 90, 90, x, R16G16_FLOAT) - SF( Y, Y, x, x, Y, Y, 75, x, 60, 70, x, 110, x, B10G10R10A2_UNORM) + SF( Y, Y, x, x, Y, Y, 75, x, 60, 70, 125, 110, x, B10G10R10A2_UNORM) SF( Y, Y, x, x, Y, Y, x, x, 60, x, x, 110, x, B10G10R10A2_UNORM_SRGB) - SF( Y, Y, x, x, Y, Y, Y, x, x, 70, x, 110, x, R11G11B10_FLOAT) - SF(120, 120, x, x, 120, 120, x, x, x, x, x, 120, x, R10G10B10_FLOAT_A2_UNORM) + SF( Y, Y, x, x, Y, Y, Y, x, x, 70, 125, 110, x, R11G11B10_FLOAT) + SF(120, 120, x, x, 120, 120, x, x, x, 125, 125, 120, x, R10G10B10_FLOAT_A2_UNORM) SF( Y, x, x, x, Y, x, Y, Y, x, 70, 70, 90, 70, R32_SINT) SF( Y, x, x, x, Y, x, Y, Y, x, 70, 70, 90, 70, R32_UINT) SF( Y, 50, Y, x, Y, Y, Y, Y, x, 70, 70, 90, 110, R32_FLOAT) diff --git a/src/intel/isl/isl_storage_image.c b/src/intel/isl/isl_storage_image.c index 356a5ae8f19..bdec1e36d85 100644 --- a/src/intel/isl/isl_storage_image.c +++ b/src/intel/isl/isl_storage_image.c @@ -25,7 +25,8 @@ #include "compiler/brw_compiler.h" bool -isl_is_storage_image_format(enum isl_format format) +isl_is_storage_image_format(const struct intel_device_info *devinfo, + enum isl_format format) { /* XXX: Maybe we should put this in the CSV? */ @@ -94,9 +95,24 @@ isl_lower_storage_image_format(const struct intel_device_info *devinfo, * * "The surface format for the typed atomic integer operations must * be R32_UINT or R32_SINT." + * + * But checking the BSpec 1706, you find a different restriction. There the + * wording is : + * + * "The surface format must be one of R32_UINT, R32_SINT or R32_FLOAT" + * + * The confusion is probably related to atomic integer messages. For + * example an IADD instruction would require a R32_UINT/R32_SINT surface. + * But a CMPXCHG instruction does not really care about the type, it just + * does bit to bit comparison and swap. + * + * The confusion seems to have propagated to the simulation environment. + * Gfx12 has the same restrictions as Gfx11 regarding doing a CMPXCHG on a + * R32_FLOAT surface, but the Gfx11 environment will report an error while + * Gfx12 passes fine. More importantly HW doesn't seem to mind. */ case ISL_FORMAT_R32_FLOAT: - return ISL_FORMAT_R32_UINT; + return format; /* From HSW to BDW the only 64bpp format supported for typed access is * RGBA_UINT16. IVB falls back to untyped. @@ -150,39 +166,55 @@ isl_lower_storage_image_format(const struct intel_device_info *devinfo, case ISL_FORMAT_R8_SINT: return (devinfo->ver >= 9 ? format : ISL_FORMAT_R8_UINT); - /* Neither the 2/10/10/10 nor the 11/11/10 packed formats are supported + /* Here the PRMs are a bit out of date. But according to BSpec 47635 + * (Gfx12.5), the 2/10/10/10 and the 11/11/10 packed formats are supported * by the hardware. */ case ISL_FORMAT_R10G10B10A2_UINT: case ISL_FORMAT_R10G10B10A2_UNORM: case ISL_FORMAT_R11G11B10_FLOAT: - return ISL_FORMAT_R32_UINT; + return devinfo->verx10 >= 125 ? format : ISL_FORMAT_R32_UINT; - /* No normalized fixed-point formats are supported by the hardware. */ + /* No normalized fixed-point formats are supported by the hardware until Gfx11. */ case ISL_FORMAT_R16G16B16A16_UNORM: case ISL_FORMAT_R16G16B16A16_SNORM: - return (devinfo->ver >= 11 ? format : - devinfo->verx10 >= 75 ? - ISL_FORMAT_R16G16B16A16_UINT : - ISL_FORMAT_R32G32_UINT); + if (devinfo->ver >= 11) + return format; + if (devinfo->ver >= 9) + return ISL_FORMAT_R32G32_UINT; + if (devinfo->verx10 >= 75) + return ISL_FORMAT_R16G16B16A16_UINT; + return ISL_FORMAT_R32G32_UINT; case ISL_FORMAT_R8G8B8A8_UNORM: case ISL_FORMAT_R8G8B8A8_SNORM: - return (devinfo->ver >= 11 ? format : - devinfo->verx10 >= 75 ? - ISL_FORMAT_R8G8B8A8_UINT : ISL_FORMAT_R32_UINT); + if (devinfo->ver >= 11) + return format; + if (devinfo->ver >= 9) + return ISL_FORMAT_R32_UINT; + if (devinfo->verx10 >= 75) + return ISL_FORMAT_R8G8B8A8_UINT; + return ISL_FORMAT_R32_UINT; case ISL_FORMAT_R16G16_UNORM: case ISL_FORMAT_R16G16_SNORM: - return (devinfo->ver >= 11 ? format : - devinfo->verx10 >= 75 ? - ISL_FORMAT_R16G16_UINT : ISL_FORMAT_R32_UINT); + if (devinfo->ver >= 11) + return format; + if (devinfo->ver >= 9) + return ISL_FORMAT_R32_UINT; + if (devinfo->verx10 >= 75) + return ISL_FORMAT_R16G16_UINT; + return ISL_FORMAT_R32_UINT; case ISL_FORMAT_R8G8_UNORM: case ISL_FORMAT_R8G8_SNORM: - return (devinfo->ver >= 11 ? format : - devinfo->verx10 >= 75 ? - ISL_FORMAT_R8G8_UINT : ISL_FORMAT_R16_UINT); + if (devinfo->ver >= 11) + return format; + if (devinfo->ver >= 9) + return ISL_FORMAT_R16_UINT; + if (devinfo->verx10 >= 75) + return ISL_FORMAT_R8G8_UINT; + return ISL_FORMAT_R16_UINT; case ISL_FORMAT_R16_UNORM: case ISL_FORMAT_R16_SNORM: diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c index e361036367f..03eb80a6509 100644 --- a/src/intel/vulkan/anv_formats.c +++ b/src/intel/vulkan/anv_formats.c @@ -834,7 +834,7 @@ get_buffer_format_features2(const struct intel_device_info *devinfo, if (isl_format_supports_vertex_fetch(devinfo, isl_format)) flags |= VK_FORMAT_FEATURE_2_VERTEX_BUFFER_BIT; - if (isl_is_storage_image_format(isl_format)) + if (isl_is_storage_image_format(devinfo, isl_format)) flags |= VK_FORMAT_FEATURE_2_STORAGE_TEXEL_BUFFER_BIT; if (isl_format == ISL_FORMAT_R32_SINT || isl_format == ISL_FORMAT_R32_UINT) diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c index ef31b327ead..ae8a39dd189 100644 --- a/src/intel/vulkan/anv_image.c +++ b/src/intel/vulkan/anv_image.c @@ -430,7 +430,7 @@ anv_get_isl_format_with_usage(const struct intel_device_info *devinfo, vk_tiling); if ((vk_usage == VK_IMAGE_USAGE_STORAGE_BIT) && - isl_is_storage_image_format(format.isl_format)) { + isl_is_storage_image_format(devinfo, format.isl_format)) { enum isl_format lowered_format = isl_lower_storage_image_format(devinfo, format.isl_format); @@ -2635,7 +2635,7 @@ anv_CreateImageView(VkDevice _device, iview->planes[vplane].lowered_storage_surface_state.state = alloc_bindless_surface_state(device); - if (isl_is_storage_image_format(format.isl_format)) { + if (isl_is_storage_image_format(device->info, format.isl_format)) { anv_image_fill_surface_state(device, image, 1ULL << iaspect_bit, &iview->planes[vplane].isl, ISL_SURF_USAGE_STORAGE_BIT, diff --git a/src/intel/vulkan_hasvk/anv_formats.c b/src/intel/vulkan_hasvk/anv_formats.c index 8c54f7bddc7..2e05bc5a8e7 100644 --- a/src/intel/vulkan_hasvk/anv_formats.c +++ b/src/intel/vulkan_hasvk/anv_formats.c @@ -839,7 +839,7 @@ get_buffer_format_features2(const struct intel_device_info *devinfo, if (isl_format_supports_vertex_fetch(devinfo, isl_format)) flags |= VK_FORMAT_FEATURE_2_VERTEX_BUFFER_BIT; - if (isl_is_storage_image_format(isl_format)) + if (isl_is_storage_image_format(devinfo, isl_format)) flags |= VK_FORMAT_FEATURE_2_STORAGE_TEXEL_BUFFER_BIT; if (isl_format == ISL_FORMAT_R32_SINT || isl_format == ISL_FORMAT_R32_UINT) diff --git a/src/intel/vulkan_hasvk/anv_image.c b/src/intel/vulkan_hasvk/anv_image.c index d989f968124..6fc9c61a293 100644 --- a/src/intel/vulkan_hasvk/anv_image.c +++ b/src/intel/vulkan_hasvk/anv_image.c @@ -448,7 +448,7 @@ anv_get_isl_format_with_usage(const struct intel_device_info *devinfo, vk_tiling); if ((vk_usage == VK_IMAGE_USAGE_STORAGE_BIT) && - isl_is_storage_image_format(format.isl_format)) { + isl_is_storage_image_format(devinfo, format.isl_format)) { enum isl_format lowered_format = isl_lower_storage_image_format(devinfo, format.isl_format); @@ -2519,7 +2519,7 @@ anv_CreateImageView(VkDevice _device, &iview->planes[vplane].storage_surface_state, NULL); - if (isl_is_storage_image_format(format.isl_format)) { + if (isl_is_storage_image_format(device->info, format.isl_format)) { iview->planes[vplane].lowered_storage_surface_state.state = alloc_surface_state(device);
