<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jul 21, 2017 at 8:37 AM, Pohjolainen, Topi <span dir="ltr"><<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed, Jul 19, 2017 at 02:01:41PM -0700, Jason Ekstrand wrote:<br>
> The only real change here is that we now reject clear colors for MCS<br>
> with certain formats on gen < 9 because we can't trust that the<br>
> reinterpretation will work.  This may cause some MCS partial resolves.<br>
> ---<br>
>  src/mesa/drivers/dri/i965/brw_<wbr>blorp.c | 57 +++++++++++++++++++++++++++---<wbr>-----<br>
>  1 file changed, 44 insertions(+), 13 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/<wbr>brw_blorp.c b/src/mesa/drivers/dri/i965/<wbr>brw_blorp.c<br>
> index 5b5b4bc..4c61afc 100644<br>
> --- a/src/mesa/drivers/dri/i965/<wbr>brw_blorp.c<br>
> +++ b/src/mesa/drivers/dri/i965/<wbr>brw_blorp.c<br>
> @@ -389,23 +389,51 @@ brw_blorp_copy_miptrees(struct brw_context *brw,<br>
>         dst_mt->num_samples, _mesa_get_format_name(dst_mt-><wbr>format), dst_mt,<br>
>         dst_level, dst_layer, dst_x, dst_y);<br>
><br>
> -   enum isl_aux_usage src_aux_usage =<br>
> -      blorp_get_aux_usage(brw, src_mt,<br>
> -                          (1 << ISL_AUX_USAGE_MCS) |<br>
> -                          (1 << ISL_AUX_USAGE_CCS_E));<br>
> +   enum isl_aux_usage src_aux_usage, dst_aux_usage;<br>
> +   bool src_clear_supported, dst_clear_supported;<br>
> +<br>
> +   switch (src_mt->aux_usage) {<br>
> +   case ISL_AUX_USAGE_MCS:<br>
> +   case ISL_AUX_USAGE_CCS_E:<br>
> +      src_aux_usage = src_mt->aux_usage;<br>
> +      /* Prior to gen9, fast-clear only supported 0/1 clear colors.  Since<br>
> +       * we're going to re-interpret the format as an integer format, a 0/1 in<br>
> +       * a non-integer format would end up not being 0/1 so we can't handle it<br>
> +       * until gen9.<br>
> +       */<br>
> +      src_clear_supported =<br>
> +         brw->gen >= 9 && !_mesa_is_format_integer(src_<wbr>mt->format);<br>
<br>
</div></div>Shouldn't this be || ?<span class=""><br></span></blockquote><div><br></div><div>Actually, it should just be "brw->gen >= 9" and it needs a new comment:<br><br></div><div>Prior to gen9, fast-clear only supported 0/1 clear colors.  Since we're going to re-interpret the format as an integer format possibly with a different channel layout, we can't handle clear colors until gen9.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> +      break;<br>
> +   default:<br>
> +      src_aux_usage = ISL_AUX_USAGE_NONE;<br>
> +      src_clear_supported = false;<br>
> +      break;<br>
> +   }<br>
> +<br>
> +   switch (dst_mt->aux_usage) {<br>
> +   case ISL_AUX_USAGE_MCS:<br>
> +   case ISL_AUX_USAGE_CCS_E:<br>
> +      dst_aux_usage = dst_mt->aux_usage;<br>
> +      /* Prior to gen9, fast-clear only supported 0/1 clear colors.  Since<br>
> +       * we're going to re-interpret the format as an integer format, a 0/1 in<br>
> +       * a non-integer format would end up not being 0/1 so we can't handle it<br>
> +       * until gen9.<br>
> +       */<br>
> +      dst_clear_supported =<br>
> +         brw->gen >= 9 && !_mesa_is_format_integer(dst_<wbr>mt->format);<br>
<br>
</span>Same here.<br><div><div class="h5"></div></div></blockquote><div><br></div><div>same as above.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
> +      break;<br>
> +   default:<br>
> +      dst_aux_usage = ISL_AUX_USAGE_NONE;<br>
> +      dst_clear_supported = false;<br>
> +      break;<br>
> +   }<br>
> +<br>
>     intel_miptree_prepare_access(<wbr>brw, src_mt, src_level, 1, src_layer, 1,<br>
>                                  src_aux_usage != ISL_AUX_USAGE_NONE,<br>
> -                                src_aux_usage != ISL_AUX_USAGE_NONE);<br>
> -<br>
> -   enum isl_aux_usage dst_aux_usage =<br>
> -      blorp_get_aux_usage(brw, dst_mt,<br>
> -                          (1 << ISL_AUX_USAGE_MCS) |<br>
> -                          (1 << ISL_AUX_USAGE_CCS_E));<br>
> +                                src_clear_supported);<br>
>     intel_miptree_prepare_access(<wbr>brw, dst_mt, dst_level, 1, dst_layer, 1,<br>
>                                  dst_aux_usage != ISL_AUX_USAGE_NONE,<br>
> -                                dst_aux_usage != ISL_AUX_USAGE_NONE);<br>
> -   intel_miptree_finish_write(<wbr>brw, dst_mt, dst_level, dst_layer, 1,<br>
> -                              dst_aux_usage != ISL_AUX_USAGE_NONE);<br>
> +                                dst_clear_supported);<br>
><br>
>     struct isl_surf tmp_surfs[2];<br>
>     struct blorp_surf src_surf, dst_surf;<br>
> @@ -420,6 +448,9 @@ brw_blorp_copy_miptrees(struct brw_context *brw,<br>
>                &dst_surf, dst_level, dst_layer,<br>
>                src_x, src_y, dst_x, dst_y, src_width, src_height);<br>
>     blorp_batch_finish(&batch);<br>
> +<br>
> +   intel_miptree_finish_write(<wbr>brw, dst_mt, dst_level, dst_layer, 1,<br>
> +                              dst_aux_usage != ISL_AUX_USAGE_NONE);<br>
<br>
</div></div>You are also moving this after the copy(). It only sets the state so I think<br>
it works either way, before or after the blorp op. Or does this really make<br>
a difference?<br>
<span class=""><br>
>  }<br>
><br>
>  static struct intel_mipmap_tree *<br>
> --<br>
> 2.5.0.400.gff86faf<br>
><br>
</span>> ______________________________<wbr>_________________<br>
> mesa-dev mailing list<br>
> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</blockquote></div><br></div></div>