[Mesa-dev] [PATCH v3 27/43] anv/pipeline: Use 32-bit surface formats for 16-bit formats

[Alejandro Piñeiro]

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 <apinheiro at 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 <jmcasanova at igalia.com>
>> Signed-off-by: Alejandro Piñeiro <apinheiro at 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.

> In blorp we use simpply UINTs, see, for example,
> blorp_blit.c::get_ccs_compatible_uint_format().
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev