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

[Samuel Iglesias Gonsálvez]

On Thu, 2017-04-06 at 11:59 -0700, Jason Ekstrand wrote:
> On Thu, Apr 6, 2017 at 1:58 AM, Samuel Iglesias Gonsálvez <siglesias@
> 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>
>  

I will move it to the end of the if-ladder.

Thanks!

Sam

> > +      } 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 --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170407/b4d9aa75/attachment.sig>