Re: [Mesa-dev] [PATCH 2/4] radeonsi: enable DCC fast clear for 128-bit formats

[Bas Nieuwenhuizen]

On Wed, Sep 7, 2016 at 3:42 PM, Marek Olšák  wrote:
> On Wed, Sep 7, 2016 at 2:11 PM, Bas Nieuwenhuizen
>  wrote:
>> On Wed, Sep 7, 2016 at 1:46 PM, Marek Olšák  wrote:
>>> From: Marek Olšák 
>>>
>>> ---
>>>  src/gallium/drivers/radeon/r600_texture.c | 45 
>>> ++-
>>>  1 file changed, 32 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/radeon/r600_texture.c 
>>> b/src/gallium/drivers/radeon/r600_texture.c
>>> index aee768f..074fed8 100644
>>> --- a/src/gallium/drivers/radeon/r600_texture.c
>>> +++ b/src/gallium/drivers/radeon/r600_texture.c
>>> @@ -2193,112 +2193,127 @@ void 
>>> vi_separate_dcc_process_and_reset_stats(struct pipe_context *ctx,
>>>  /* FAST COLOR CLEAR */
>>>
>>>  static void evergreen_set_clear_color(struct r600_texture *rtex,
>>>   enum pipe_format surface_format,
>>>   const union pipe_color_union *color)
>>>  {
>>> union util_color uc;
>>>
>>> memset(, 0, sizeof(uc));
>>>
>>> -   if (util_format_is_pure_uint(surface_format)) {
>>> +   if (util_format_get_blocksizebits(surface_format) == 128) {
>>> +   /* DCC fast clear only:
>>> +*   CLEAR_WORD0 = R = G = B
>>> +*   CLEAR_WORD1 = A
>>> +*/
>>> +   assert(color->ui[0] == color->ui[1] &&
>>> +  color->ui[0] == color->ui[2]);
>>> +   uc.ui[0] = color->ui[0];
>>> +   uc.ui[1] = color->ui[3];
>>> +   } else if (util_format_is_pure_uint(surface_format)) {
>>> util_format_write_4ui(surface_format, color->ui, 0, , 0, 
>>> 0, 0, 1, 1);
>>> } else if (util_format_is_pure_sint(surface_format)) {
>>> util_format_write_4i(surface_format, color->i, 0, , 0, 
>>> 0, 0, 1, 1);
>>> } else {
>>> util_pack_color(color->f, surface_format, );
>>> }
>>>
>>> memcpy(rtex->color_clear_value, , 2 * sizeof(uint32_t));
>>>  }
>>>
>>> -static void vi_get_fast_clear_parameters(enum pipe_format surface_format,
>>> +static bool vi_get_fast_clear_parameters(enum pipe_format surface_format,
>>>  const union pipe_color_union 
>>> *color,
>>>  uint32_t* reset_value,
>>>  bool* clear_words_needed)
>>>  {
>>> bool values[4] = {};
>>> int i;
>>> bool main_value = false;
>>> bool extra_value = false;
>>> int extra_channel;
>>> const struct util_format_description *desc = 
>>> util_format_description(surface_format);
>>>
>>> +   if (desc->block.bits == 128 &&
>>> +   (color->ui[0] != color->ui[1] ||
>>> +color->ui[0] != color->ui[2]))
>>> +   return false;
>>> +
>>
>> Don't we also need to return false if the pixel size is 128 bits and
>> and the clear values aren't 0/1 (or integer format equivalents)? You
>> can probably do that by replacing most of the "return true;"
>> statements with "return desc->bloc.bits <= 64;".
>
> No. The only requirement is that R==G==B. 0/1 values are only for the
> TC-compatible fast clear without the elimination pass, but any values
> are allowed with the elimination pass.
>
>>
>> Furthermore, as far as I understood, if the DCC texture is bound to
>> CB, then tiles always get expanded to the clear color that we have in
>> the registers. However there is only space for 64 bits in there. Does
>> the hardware automatically ignore these values for 128 bit formats?
>
> CLEAR_WORD0 is used for R,G,B which must be equal like the first code
> comment shows.

Ok, for some reason I had a TC-compatible fast clear in mind.

This series is:

Reviewed-by: Bas Nieuwenhuizen 

>
> Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/4] radeonsi: enable DCC fast clear for 128-bit formats

[Marek Olšák]

On Wed, Sep 7, 2016 at 2:11 PM, Bas Nieuwenhuizen
 wrote:
> On Wed, Sep 7, 2016 at 1:46 PM, Marek Olšák  wrote:
>> From: Marek Olšák 
>>
>> ---
>>  src/gallium/drivers/radeon/r600_texture.c | 45 
>> ++-
>>  1 file changed, 32 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeon/r600_texture.c 
>> b/src/gallium/drivers/radeon/r600_texture.c
>> index aee768f..074fed8 100644
>> --- a/src/gallium/drivers/radeon/r600_texture.c
>> +++ b/src/gallium/drivers/radeon/r600_texture.c
>> @@ -2193,112 +2193,127 @@ void 
>> vi_separate_dcc_process_and_reset_stats(struct pipe_context *ctx,
>>  /* FAST COLOR CLEAR */
>>
>>  static void evergreen_set_clear_color(struct r600_texture *rtex,
>>   enum pipe_format surface_format,
>>   const union pipe_color_union *color)
>>  {
>> union util_color uc;
>>
>> memset(, 0, sizeof(uc));
>>
>> -   if (util_format_is_pure_uint(surface_format)) {
>> +   if (util_format_get_blocksizebits(surface_format) == 128) {
>> +   /* DCC fast clear only:
>> +*   CLEAR_WORD0 = R = G = B
>> +*   CLEAR_WORD1 = A
>> +*/
>> +   assert(color->ui[0] == color->ui[1] &&
>> +  color->ui[0] == color->ui[2]);
>> +   uc.ui[0] = color->ui[0];
>> +   uc.ui[1] = color->ui[3];
>> +   } else if (util_format_is_pure_uint(surface_format)) {
>> util_format_write_4ui(surface_format, color->ui, 0, , 0, 
>> 0, 0, 1, 1);
>> } else if (util_format_is_pure_sint(surface_format)) {
>> util_format_write_4i(surface_format, color->i, 0, , 0, 0, 
>> 0, 1, 1);
>> } else {
>> util_pack_color(color->f, surface_format, );
>> }
>>
>> memcpy(rtex->color_clear_value, , 2 * sizeof(uint32_t));
>>  }
>>
>> -static void vi_get_fast_clear_parameters(enum pipe_format surface_format,
>> +static bool vi_get_fast_clear_parameters(enum pipe_format surface_format,
>>  const union pipe_color_union *color,
>>  uint32_t* reset_value,
>>  bool* clear_words_needed)
>>  {
>> bool values[4] = {};
>> int i;
>> bool main_value = false;
>> bool extra_value = false;
>> int extra_channel;
>> const struct util_format_description *desc = 
>> util_format_description(surface_format);
>>
>> +   if (desc->block.bits == 128 &&
>> +   (color->ui[0] != color->ui[1] ||
>> +color->ui[0] != color->ui[2]))
>> +   return false;
>> +
>
> Don't we also need to return false if the pixel size is 128 bits and
> and the clear values aren't 0/1 (or integer format equivalents)? You
> can probably do that by replacing most of the "return true;"
> statements with "return desc->bloc.bits <= 64;".

No. The only requirement is that R==G==B. 0/1 values are only for the
TC-compatible fast clear without the elimination pass, but any values
are allowed with the elimination pass.

>
> Furthermore, as far as I understood, if the DCC texture is bound to
> CB, then tiles always get expanded to the clear color that we have in
> the registers. However there is only space for 64 bits in there. Does
> the hardware automatically ignore these values for 128 bit formats?

CLEAR_WORD0 is used for R,G,B which must be equal like the first code
comment shows.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/4] radeonsi: enable DCC fast clear for 128-bit formats

[Bas Nieuwenhuizen]

On Wed, Sep 7, 2016 at 1:46 PM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> ---
>  src/gallium/drivers/radeon/r600_texture.c | 45 
> ++-
>  1 file changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/src/gallium/drivers/radeon/r600_texture.c 
> b/src/gallium/drivers/radeon/r600_texture.c
> index aee768f..074fed8 100644
> --- a/src/gallium/drivers/radeon/r600_texture.c
> +++ b/src/gallium/drivers/radeon/r600_texture.c
> @@ -2193,112 +2193,127 @@ void vi_separate_dcc_process_and_reset_stats(struct 
> pipe_context *ctx,
>  /* FAST COLOR CLEAR */
>
>  static void evergreen_set_clear_color(struct r600_texture *rtex,
>   enum pipe_format surface_format,
>   const union pipe_color_union *color)
>  {
> union util_color uc;
>
> memset(, 0, sizeof(uc));
>
> -   if (util_format_is_pure_uint(surface_format)) {
> +   if (util_format_get_blocksizebits(surface_format) == 128) {
> +   /* DCC fast clear only:
> +*   CLEAR_WORD0 = R = G = B
> +*   CLEAR_WORD1 = A
> +*/
> +   assert(color->ui[0] == color->ui[1] &&
> +  color->ui[0] == color->ui[2]);
> +   uc.ui[0] = color->ui[0];
> +   uc.ui[1] = color->ui[3];
> +   } else if (util_format_is_pure_uint(surface_format)) {
> util_format_write_4ui(surface_format, color->ui, 0, , 0, 
> 0, 0, 1, 1);
> } else if (util_format_is_pure_sint(surface_format)) {
> util_format_write_4i(surface_format, color->i, 0, , 0, 0, 
> 0, 1, 1);
> } else {
> util_pack_color(color->f, surface_format, );
> }
>
> memcpy(rtex->color_clear_value, , 2 * sizeof(uint32_t));
>  }
>
> -static void vi_get_fast_clear_parameters(enum pipe_format surface_format,
> +static bool vi_get_fast_clear_parameters(enum pipe_format surface_format,
>  const union pipe_color_union *color,
>  uint32_t* reset_value,
>  bool* clear_words_needed)
>  {
> bool values[4] = {};
> int i;
> bool main_value = false;
> bool extra_value = false;
> int extra_channel;
> const struct util_format_description *desc = 
> util_format_description(surface_format);
>
> +   if (desc->block.bits == 128 &&
> +   (color->ui[0] != color->ui[1] ||
> +color->ui[0] != color->ui[2]))
> +   return false;
> +

Don't we also need to return false if the pixel size is 128 bits and
and the clear values aren't 0/1 (or integer format equivalents)? You
can probably do that by replacing most of the "return true;"
statements with "return desc->bloc.bits <= 64;".

Furthermore, as far as I understood, if the DCC texture is bound to
CB, then tiles always get expanded to the clear color that we have in
the registers. However there is only space for 64 bits in there. Does
the hardware automatically ignore these values for 128 bit formats?

- Bas

> *clear_words_needed = true;
> *reset_value = 0x20202020U;
>
> /* If we want to clear without needing a fast clear eliminate step, we
>  * can set each channel to 0 or 1 (or 0/max for integer formats). We
>  * have two sets of flags, one for the last or first channel(extra) 
> and
>  * one for the other channels(main).
>  */
>
> if (surface_format == PIPE_FORMAT_R11G11B10_FLOAT ||
> surface_format == PIPE_FORMAT_B5G6R5_UNORM ||
> surface_format == PIPE_FORMAT_B5G6R5_SRGB) {
> extra_channel = -1;
> } else if (desc->layout == UTIL_FORMAT_LAYOUT_PLAIN) {
> if(r600_translate_colorswap(surface_format, false) <= 1)
> extra_channel = desc->nr_channels - 1;
> else
> extra_channel = 0;
> } else
> -   return;
> +   return true;
>
> for (i = 0; i < 4; ++i) {
> int index = desc->swizzle[i] - PIPE_SWIZZLE_X;
>
> if (desc->swizzle[i] < PIPE_SWIZZLE_X ||
> desc->swizzle[i] > PIPE_SWIZZLE_W)
> continue;
>
> if (desc->channel[i].pure_integer &&
> desc->channel[i].type == UTIL_FORMAT_TYPE_SIGNED) {
> /* Use the maximum value for clamping the clear 
> color. */
> int max = u_bit_consecutive(0, desc->channel[i].size 
> - 1);
>
> values[i] = color->i[i] != 0;
> if (color->i[i] != 0 && MIN2(color->i[i], max) != max)
> -   return;
> +   return true;
> } else if