[Mesa-dev] [PATCH] anv/blorp: sample input attachments with resolves on BDW

[Jason Ekstrand]

On Thu, Apr 6, 2017 at 1:58 AM, Samuel Iglesias Gonsálvez <
siglesias at igalia.com> wrote:

> On Broadwell we still need to do a resolve between the subpass
> that writes and the subpass that reads when there is a
> self-dependency because HW could not see fast-clears and works
> on the render cache as if there was regular non-fast-clear surface.
>
> Fixes 16 tests on BDW:
>
> dEQP-VK.renderpass.formats.*.input.clear.store.self_dep*
>
> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> ---
>  src/intel/vulkan/anv_blorp.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index 72a468a744a..daf13d360c4 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -1457,6 +1457,7 @@ ccs_resolve_attachment(struct anv_cmd_buffer
> *cmd_buffer,
>      * still hot in the cache.
>      */
>     bool found_draw = false;
> +   bool self_dep = false;
>     enum anv_subpass_usage usage = 0;
>     for (uint32_t s = subpass_idx + 1; s < pass->subpass_count; s++) {
>        usage |= pass->attachments[att].subpass_usage[s];
> @@ -1466,6 +1467,8 @@ ccs_resolve_attachment(struct anv_cmd_buffer
> *cmd_buffer,
>            * wait to resolve until then.
>            */
>           found_draw = true;
> +         if (pass->attachments[att].subpass_usage[s] &
> ANV_SUBPASS_USAGE_INPUT)
> +            self_dep = true;
>           break;
>        }
>     }
> @@ -1509,10 +1512,19 @@ ccs_resolve_attachment(struct anv_cmd_buffer
> *cmd_buffer,
>            */
>        }
>     } else if (usage & ANV_SUBPASS_USAGE_INPUT) {
> -      /* Input attachments are clear-color aware so, at least on Sky
> Lake, we
> +      /* Input attachments are clear-color aware, at least on Sky Lake,
> so we
>         * can frequently sample from them with no resolves at all.
> +       *
> +       * However on Broadwell we still need to do resolves when there is a
> +       * self-dependency because HW could not see fast-clears and works
> +       * on the render cache as if there was regular non-fast-clear
> surface.
> +       * To avoid any inconsistency, we force the resolve.
>         */
> -      if (att_state->aux_usage != att_state->input_aux_usage) {
> +      if (cmd_buffer->device->info.gen == 8 && self_dep &&
> +          att_state->aux_usage == ISL_AUX_USAGE_CCS_D &&
> +          att_state->input_aux_usage == ISL_AUX_USAGE_CCS_D) {
> +         resolve_op = BLORP_FAST_CLEAR_OP_RESOLVE_FULL;
>

Would it make a tad bit more sense to move this down in the if-ladder a
bit?  It would let us simplify the condition because we would already know
that aux_usage == input_aux_usage but I think it's also the most special of
our special cases.

Either way,

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


> +      } else if (att_state->aux_usage != att_state->input_aux_usage) {
>           assert(att_state->input_aux_usage == ISL_AUX_USAGE_NONE);
>           resolve_op = BLORP_FAST_CLEAR_OP_RESOLVE_FULL;
>        } else if (!att_state->clear_color_is_zero_one) {
> --
> 2.11.0
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170406/e8c02c10/attachment.html>