[2/2] radv: only flush CB meta in pipeline image barriers when needed

Submitted by Samuel Pitoiset on July 5, 2018, 10:54 a.m.

Details

Message ID 20180705105418.21831-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 July 5, 2018, 10:54 a.m.
If the given image doesn't enable CMASK, FMASK or DCC that's
useless to flush CB metadata.

Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
---
 src/amd/vulkan/radv_cmd_buffer.c |  6 ++++--
 src/amd/vulkan/radv_private.h    | 11 +++++++++++
 2 files changed, 15 insertions(+), 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 3a017b5fef..0c44c29b88 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -1971,8 +1971,10 @@  radv_src_access_flush(struct radv_cmd_buffer *cmd_buffer,
 			flush_bits |= RADV_CMD_FLAG_WRITEBACK_GLOBAL_L2;
 			break;
 		case VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT:
-			flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB |
-			              RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
+			flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB;
+			if (!image || (image && radv_image_has_CB_metadata(image))) {
+				flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
+			}
 			break;
 		case VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT:
 			flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_DB;
diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
index 0a34d79a20..4e4b3a6037 100644
--- a/src/amd/vulkan/radv_private.h
+++ b/src/amd/vulkan/radv_private.h
@@ -1486,6 +1486,17 @@  radv_dcc_enabled(const struct radv_image *image, unsigned level)
 	       level < image->surface.num_dcc_levels;
 }
 
+/**
+ * Return whether the image has CB metadata.
+ */
+static inline bool
+radv_image_has_CB_metadata(const struct radv_image *image)
+{
+	return radv_image_has_cmask(image) ||
+	       radv_image_has_fmask(image) ||
+	       radv_image_has_dcc(image);
+}
+
 /**
  * Return whether the image has HTILE metadata for depth surfaces.
  */

Comments

On Thu, Jul 5, 2018 at 12:54 PM, Samuel Pitoiset
<samuel.pitoiset@gmail.com> wrote:
> If the given image doesn't enable CMASK, FMASK or DCC that's
> useless to flush CB metadata.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> ---
>  src/amd/vulkan/radv_cmd_buffer.c |  6 ++++--
>  src/amd/vulkan/radv_private.h    | 11 +++++++++++
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
> index 3a017b5fef..0c44c29b88 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -1971,8 +1971,10 @@ radv_src_access_flush(struct radv_cmd_buffer *cmd_buffer,
>                         flush_bits |= RADV_CMD_FLAG_WRITEBACK_GLOBAL_L2;
>                         break;
>                 case VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT:
> -                       flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB |
> -                                     RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
> +                       flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB;
> +                       if (!image || (image && radv_image_has_CB_metadata(image))) {
> +                               flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
> +                       }
>                         break;
>                 case VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT:
>                         flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_DB;
> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
> index 0a34d79a20..4e4b3a6037 100644
> --- a/src/amd/vulkan/radv_private.h
> +++ b/src/amd/vulkan/radv_private.h
> @@ -1486,6 +1486,17 @@ radv_dcc_enabled(const struct radv_image *image, unsigned level)
>                level < image->surface.num_dcc_levels;
>  }
>
> +/**
> + * Return whether the image has CB metadata.
> + */
> +static inline bool
> +radv_image_has_CB_metadata(const struct radv_image *image)
> +{
> +       return radv_image_has_cmask(image) ||
> +              radv_image_has_fmask(image) ||
> +              radv_image_has_dcc(image);

AFAIU DCC actually is not included in META? The
V_028A90_FLUSH_AND_INV_CB_DATA_TS we do with normal CB was for DCC, I
don't know whether it also needs META.

> +}
> +
>  /**
>   * Return whether the image has HTILE metadata for depth surfaces.
>   */
> --
> 2.18.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On 07/05/2018 01:36 PM, Bas Nieuwenhuizen wrote:
> On Thu, Jul 5, 2018 at 12:54 PM, Samuel Pitoiset
> <samuel.pitoiset@gmail.com> wrote:
>> If the given image doesn't enable CMASK, FMASK or DCC that's
>> useless to flush CB metadata.

According to RadeonSI and AMDVLK, it seems like DCC is included in the 
CB metadata, so we probably need to emit INV_CB_META as well.

>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
>> ---
>>   src/amd/vulkan/radv_cmd_buffer.c |  6 ++++--
>>   src/amd/vulkan/radv_private.h    | 11 +++++++++++
>>   2 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
>> index 3a017b5fef..0c44c29b88 100644
>> --- a/src/amd/vulkan/radv_cmd_buffer.c
>> +++ b/src/amd/vulkan/radv_cmd_buffer.c
>> @@ -1971,8 +1971,10 @@ radv_src_access_flush(struct radv_cmd_buffer *cmd_buffer,
>>                          flush_bits |= RADV_CMD_FLAG_WRITEBACK_GLOBAL_L2;
>>                          break;
>>                  case VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT:
>> -                       flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB |
>> -                                     RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
>> +                       flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB;
>> +                       if (!image || (image && radv_image_has_CB_metadata(image))) {
>> +                               flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
>> +                       }
>>                          break;
>>                  case VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT:
>>                          flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_DB;
>> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
>> index 0a34d79a20..4e4b3a6037 100644
>> --- a/src/amd/vulkan/radv_private.h
>> +++ b/src/amd/vulkan/radv_private.h
>> @@ -1486,6 +1486,17 @@ radv_dcc_enabled(const struct radv_image *image, unsigned level)
>>                 level < image->surface.num_dcc_levels;
>>   }
>>
>> +/**
>> + * Return whether the image has CB metadata.
>> + */
>> +static inline bool
>> +radv_image_has_CB_metadata(const struct radv_image *image)
>> +{
>> +       return radv_image_has_cmask(image) ||
>> +              radv_image_has_fmask(image) ||
>> +              radv_image_has_dcc(image);
> 
> AFAIU DCC actually is not included in META? The
> V_028A90_FLUSH_AND_INV_CB_DATA_TS we do with normal CB was for DCC, I
> don't know whether it also needs META.
> 
>> +}
>> +
>>   /**
>>    * Return whether the image has HTILE metadata for depth surfaces.
>>    */
>> --
>> 2.18.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On Thu, Jul 5, 2018 at 1:51 PM, Samuel Pitoiset
<samuel.pitoiset@gmail.com> wrote:
>
>
> On 07/05/2018 01:36 PM, Bas Nieuwenhuizen wrote:
>>
>> On Thu, Jul 5, 2018 at 12:54 PM, Samuel Pitoiset
>> <samuel.pitoiset@gmail.com> wrote:
>>>
>>> If the given image doesn't enable CMASK, FMASK or DCC that's
>>> useless to flush CB metadata.
>
>
> According to RadeonSI and AMDVLK, it seems like DCC is included in the CB
> metadata, so we probably need to emit INV_CB_META as well.

Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>
>
>>>
>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
>>> ---
>>>   src/amd/vulkan/radv_cmd_buffer.c |  6 ++++--
>>>   src/amd/vulkan/radv_private.h    | 11 +++++++++++
>>>   2 files changed, 15 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/amd/vulkan/radv_cmd_buffer.c
>>> b/src/amd/vulkan/radv_cmd_buffer.c
>>> index 3a017b5fef..0c44c29b88 100644
>>> --- a/src/amd/vulkan/radv_cmd_buffer.c
>>> +++ b/src/amd/vulkan/radv_cmd_buffer.c
>>> @@ -1971,8 +1971,10 @@ radv_src_access_flush(struct radv_cmd_buffer
>>> *cmd_buffer,
>>>                          flush_bits |= RADV_CMD_FLAG_WRITEBACK_GLOBAL_L2;
>>>                          break;
>>>                  case VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT:
>>> -                       flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB |
>>> -
>>> RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
>>> +                       flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_CB;
>>> +                       if (!image || (image &&
>>> radv_image_has_CB_metadata(image))) {
>>> +                               flush_bits |=
>>> RADV_CMD_FLAG_FLUSH_AND_INV_CB_META;
>>> +                       }
>>>                          break;
>>>                  case VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT:
>>>                          flush_bits |= RADV_CMD_FLAG_FLUSH_AND_INV_DB;
>>> diff --git a/src/amd/vulkan/radv_private.h
>>> b/src/amd/vulkan/radv_private.h
>>> index 0a34d79a20..4e4b3a6037 100644
>>> --- a/src/amd/vulkan/radv_private.h
>>> +++ b/src/amd/vulkan/radv_private.h
>>> @@ -1486,6 +1486,17 @@ radv_dcc_enabled(const struct radv_image *image,
>>> unsigned level)
>>>                 level < image->surface.num_dcc_levels;
>>>   }
>>>
>>> +/**
>>> + * Return whether the image has CB metadata.
>>> + */
>>> +static inline bool
>>> +radv_image_has_CB_metadata(const struct radv_image *image)
>>> +{
>>> +       return radv_image_has_cmask(image) ||
>>> +              radv_image_has_fmask(image) ||
>>> +              radv_image_has_dcc(image);
>>
>>
>> AFAIU DCC actually is not included in META? The
>> V_028A90_FLUSH_AND_INV_CB_DATA_TS we do with normal CB was for DCC, I
>> don't know whether it also needs META.
>>
>>> +}
>>> +
>>>   /**
>>>    * Return whether the image has HTILE metadata for depth surfaces.
>>>    */
>>> --
>>> 2.18.0
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev