[2/3] radv: remove unnecessary FLUSH_AND_INV_CB when initializing DCC

Submitted by Samuel Pitoiset on March 19, 2019, 11:18 a.m.

Details

Message ID 20190319111814.24375-2-samuel.pitoiset@gmail.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Samuel Pitoiset March 19, 2019, 11:18 a.m.
The clear operation (ie. compute) doesn't use the CB caches.

Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
---
 src/amd/vulkan/radv_cmd_buffer.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 5bb3b51684e..b6035dfbbc5 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -4583,8 +4583,7 @@  static void radv_init_color_image_metadata(struct radv_cmd_buffer *cmd_buffer,
 
 		state->flush_bits |= radv_clear_dcc(cmd_buffer, image, value);
 
-		state->flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB |
-				     RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
+		state->flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
 
 		radv_update_fce_metadata(cmd_buffer, image,
 					 need_decompress_pass);

Comments

That it does not use it is exactly why we need to make sure the CB
data is not in the CB cache by flushing it?

On Tue, Mar 19, 2019 at 12:15 PM Samuel Pitoiset
<samuel.pitoiset@gmail.com> wrote:
>
> The clear operation (ie. compute) doesn't use the CB caches.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> ---
>  src/amd/vulkan/radv_cmd_buffer.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
> index 5bb3b51684e..b6035dfbbc5 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -4583,8 +4583,7 @@ static void radv_init_color_image_metadata(struct radv_cmd_buffer *cmd_buffer,
>
>                 state->flush_bits |= radv_clear_dcc(cmd_buffer, image, value);
>
> -               state->flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB |
> -                                    RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
> +               state->flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
>
>                 radv_update_fce_metadata(cmd_buffer, image,
>                                          need_decompress_pass);
> --
> 2.21.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On 3/19/19 1:56 PM, Bas Nieuwenhuizen wrote:
> That it does not use it is exactly why we need to make sure the CB
> data is not in the CB cache by flushing it?
Why only for DCC?
>
> On Tue, Mar 19, 2019 at 12:15 PM Samuel Pitoiset
> <samuel.pitoiset@gmail.com> wrote:
>> The clear operation (ie. compute) doesn't use the CB caches.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
>> ---
>>   src/amd/vulkan/radv_cmd_buffer.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
>> index 5bb3b51684e..b6035dfbbc5 100644
>> --- a/src/amd/vulkan/radv_cmd_buffer.c
>> +++ b/src/amd/vulkan/radv_cmd_buffer.c
>> @@ -4583,8 +4583,7 @@ static void radv_init_color_image_metadata(struct radv_cmd_buffer *cmd_buffer,
>>
>>                  state->flush_bits |= radv_clear_dcc(cmd_buffer, image, value);
>>
>> -               state->flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB |
>> -                                    RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
>> +               state->flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
>>
>>                  radv_update_fce_metadata(cmd_buffer, image,
>>                                           need_decompress_pass);
>> --
>> 2.21.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
After having a deeper look at the cache flushes when initializing DCC, 
it seems like we are dumb.

In case the compute path is used for clearing DCC, we set the following 
flags:

- CS_PARTIAL_FLUSH

- INV_VMEM_L1

- WRITEBACK_GLOBAL_L2

- FLUSH_AND_INV_CB

- FLUSH_ANV_INV_CB_META

On GFX9, the driver will emit:

- one RELEASE_MEM with TC_ACTION_ENA | TC_MD_ACTION_ENA

- one ACQUIRE_MEM with TC_WB_ACTION_ENA | TC_NC_ACTION_ENA

- one ACQUIRE_MEM with TCL1_ACTION_ENA

This seems overkill to me... Emitting one RELEASE_MEM with TC_ACTION_ENA 
| TC_WB_ACTION_ENA (ie. INV_GLOBAL_L2) should be enough?

On 3/19/19 2:03 PM, Samuel Pitoiset wrote:
>
> On 3/19/19 1:56 PM, Bas Nieuwenhuizen wrote:
>> That it does not use it is exactly why we need to make sure the CB
>> data is not in the CB cache by flushing it?
> Why only for DCC?
>>
>> On Tue, Mar 19, 2019 at 12:15 PM Samuel Pitoiset
>> <samuel.pitoiset@gmail.com> wrote:
>>> The clear operation (ie. compute) doesn't use the CB caches.
>>>
>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
>>> ---
>>>   src/amd/vulkan/radv_cmd_buffer.c | 3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/src/amd/vulkan/radv_cmd_buffer.c 
>>> b/src/amd/vulkan/radv_cmd_buffer.c
>>> index 5bb3b51684e..b6035dfbbc5 100644
>>> --- a/src/amd/vulkan/radv_cmd_buffer.c
>>> +++ b/src/amd/vulkan/radv_cmd_buffer.c
>>> @@ -4583,8 +4583,7 @@ static void 
>>> radv_init_color_image_metadata(struct radv_cmd_buffer *cmd_buffer,
>>>
>>>                  state->flush_bits |= radv_clear_dcc(cmd_buffer, 
>>> image, value);
>>>
>>> -               state->flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB |
>>> - RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
>>> +               state->flush_bits |= 
>>> RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
>>>
>>>                  radv_update_fce_metadata(cmd_buffer, image,
>>> need_decompress_pass);
>>> -- 
>>> 2.21.0
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev