<div class="gmail_quote">On 11 July 2012 11:01, Chad Versace <span dir="ltr"><<a href="mailto:chad.versace@linux.intel.com" target="_blank">chad.versace@linux.intel.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="im">On 07/06/2012 03:29 PM, Paul Berry wrote:<br>
> This patch updates the blorp engine to properly handle the case where<br>
> the surface being textured from uses Gen7's CMS MSAA layout.  The<br>
> following changes were necessary:<br>
><br>
> - Before reading color values from the surface, we need to read from<br>
>   the MCS buffer using the ld_mcs sampler message.  This is done by<br>
>   the mcs_fetch() function, and the result is stored in the mcs_data<br>
>   register.  This only needs to be done once per pixel, since the MCS<br>
>   value is shared between all samples belonging to a pixel.<br>
><br>
> - When reading color values from the surface, we need to use the<br>
>   ld2dms sampler message instead of the ld2dss message, and we need to<br>
>   provide the value read from the MCS buffer as an argument.<br>
> ---<br>
>  src/mesa/drivers/dri/i965/brw_blorp_blit.cpp |   45 ++++++++++++++++++++++++-<br>
>  1 files changed, 43 insertions(+), 2 deletions(-)<br>
<br>
</div>I see an easy optimization that's suitable for a small follow-up patch. But this<br>
code works and passes tests, so let's commit it as-is first.<br></blockquote><div><br></div><div>Cool.  I'm curious to hear what your optimization idea is.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<br>
This patch is<br>
Reviewed-by: Chad Versace <<a href="mailto:chad.versace@linux.intel.com">chad.versace@linux.intel.com</a>><br>
<div class="im"><br>
> @@ -1126,8 +1148,13 @@ brw_blorp_blit_program::texel_fetch(struct brw_reg dst)<br>
>        break;<br>
>     case 7:<br>
>        if (key->tex_samples > 0) {<br>
> -         texture_lookup(dst, GEN7_SAMPLER_MESSAGE_SAMPLE_LD2DSS,<br>
> -                        gen7_ld2dss_args, ARRAY_SIZE(gen7_ld2dss_args));<br>
> +         if (key->tex_layout == INTEL_MSAA_LAYOUT_CMS) {<br>
> +            texture_lookup(dst, GEN7_SAMPLER_MESSAGE_SAMPLE_LD2DMS,<br>
> +                           gen7_ld2dms_args, ARRAY_SIZE(gen7_ld2dms_args));<br>
> +         } else {<br>
> +            texture_lookup(dst, GEN7_SAMPLER_MESSAGE_SAMPLE_LD2DSS,<br>
> +                           gen7_ld2dss_args, ARRAY_SIZE(gen7_ld2dss_args));<br>
> +         }<br>
<br>
</div>The hw docs suggest here "a simple optimization with probable large return in<br>
performance". If mcs_data == 0, which should be the common case for most<br>
fragments, then only sample array slice 0 needs to be accessed. That is, when<br>
tex_layout is CMS then we should emit code like this here:<br>
<br>
  if (mcs_data == 0) {<br>
    ld2dss(S, X, Y);<br>
  } else {<br>
    ld2dms(S, mcs_data, X, Y);<br>
  }<br></blockquote><div><br></div><div>I actually believe this will be slightly faster:</div><div><br></div><div>ld2dms(0, mcs_data, X, Y);</div><div>if (mcs_data != 0) {</div><div>  for (S = 1; S < num_samples; ++S)</div>
<div>    ld2dms(S, mcs_data, X, Y);</div><div>    average samples together;</div><div>  }</div><div>}</div><div><br></div><div>Because it will avoid issuing an extra sample instruction in the case where some pixels have a zero value for mcs_data and others have a nonzero value for mcs_data.  I've got that queued up for a patch series after this one is reviewed :)</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
----<br>
Chad Versace<br>
<a href="mailto:chad.versace@linux.intel.com">chad.versace@linux.intel.com</a><br>
<br>
<br>
</blockquote></div><br>