El 16/10/17 a las 08:57, Alejandro Piñeiro escribió: > On 15/10/17 12:14, Pohjolainen, Topi wrote: >> On Thu, Oct 12, 2017 at 08:38:16PM +0200, Jose Maria Casanova Crespo wrote: >>> From: Alejandro Piñeiro <apinhe...@igalia.com> >>> >>> From Vulkan 1.0.50 spec, Section 3.30.1. Format Definition: >>> VK_FORMAT_R16G16_SFLOAT >>> >>> A two-component, 32-bit signed floating-point format that has a >>> 16-bit R component in bytes 0..1, and a 16-bit G component in >>> bytes 2..3. >>> >>> So this format expects those 16-bit floats to be passed without any >>> conversion (applies too using 2/3/4 components, and with int formats) >>> >>> But from skl PRM, vol 07, section FormatConversion, page 445 there is >>> a table that points that *16*FLOAT formats are converted to FLOAT, >>> that in that context, is a 32-bit float. This is similar to the >>> *64*FLOAT formats, that converts 64-bit floats to 32-bit floats. >>> >>> Unfortunately, while with 64-bit floats we have the alternative to use >>> *64*PASSTHRU formats, it is not the case with 16-bits. >>> >>> This issue happens too with 16-bit int surface formats. >>> >>> As a workaround, if we are using a 16-bit location at the shader, we >>> use 32-bit formats to avoid the conversion, and will fix getting the >>> proper content later. Note that as we are using 32-bit formats, we can >>> use formats with less components (example: use *R32* for *R16G16*). >>> >>> Signed-off-by: Jose Maria Casanova Crespo <jmcasan...@igalia.com> >>> Signed-off-by: Alejandro Piñeiro <apinhe...@igalia.com> >>> --- >>> src/intel/vulkan/genX_pipeline.c | 47 >>> ++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 47 insertions(+) >>> >>> diff --git a/src/intel/vulkan/genX_pipeline.c >>> b/src/intel/vulkan/genX_pipeline.c >>> index c2fa9c0ff7..8b2d472787 100644 >>> --- a/src/intel/vulkan/genX_pipeline.c >>> +++ b/src/intel/vulkan/genX_pipeline.c >>> @@ -83,6 +83,44 @@ vertex_element_comp_control(enum isl_format format, >>> unsigned comp) >>> } >>> } >>> >>> +#if GEN_GEN >= 8 >>> +static enum isl_format >>> +adjust_16bit_format(enum isl_format format) >>> +{ >>> + switch(format) { >>> + case ISL_FORMAT_R16_FLOAT: >>> + return ISL_FORMAT_R32_FLOAT; >>> + case ISL_FORMAT_R16G16_FLOAT: >>> + return ISL_FORMAT_R32_FLOAT; >>> + case ISL_FORMAT_R16G16B16_FLOAT: >>> + return ISL_FORMAT_R32G32_FLOAT; >>> + case ISL_FORMAT_R16G16B16A16_FLOAT: >>> + return ISL_FORMAT_R32G32_FLOAT; >>> + >>> + case ISL_FORMAT_R16_SINT: >>> + return ISL_FORMAT_R32_SINT; >>> + case ISL_FORMAT_R16G16_SINT: >>> + return ISL_FORMAT_R32_SINT; >>> + case ISL_FORMAT_R16G16B16_SINT: >>> + return ISL_FORMAT_R32G32_SINT; >>> + case ISL_FORMAT_R16G16B16A16_SINT: >>> + return ISL_FORMAT_R32G32_SINT; >>> + >>> + case ISL_FORMAT_R16_UINT: >>> + return ISL_FORMAT_R32_UINT; >>> + case ISL_FORMAT_R16G16_UINT: >>> + return ISL_FORMAT_R32_UINT; >>> + case ISL_FORMAT_R16G16B16_UINT: >>> + return ISL_FORMAT_R32G32_UINT; >>> + case ISL_FORMAT_R16G16B16A16_UINT: >>> + return ISL_FORMAT_R32G32_UINT; >>> + >>> + default: >>> + return format; >>> + } >>> +} >> Just wondering aloud. As we are going to reinterpret the data in any case >> we could simply use _UINT variants even for FLOAT and SINT. It doesn't really >> make any difference only that here someone might think it is somehow relevant >> to keep the base type. > FWIW, I also don't mind too much. So ok, we could try that change too. I've just sent a v2 of this patch. The code is simplified and doesn't introduce regressions on 16-bit tests.
Chema _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev