On 24 July 2012 12:10, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><div class="gmail_quote"><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 07/18/2012 09:21 AM, Paul Berry wrote:<br>
> MCS buffers use 32 bits per pixel in 8x MSAA, and 8 bits per pixel in<br>
> 4x MSAA.  This patch adjusts the format we use to allocate the buffer<br>
> so that enough memory is set aside for 8x MSAA.<br>
> ---<br>
>  src/mesa/drivers/dri/intel/intel_mipmap_tree.c |   27 ++++++++++++++++++++++-<br>
>  1 files changed, 25 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c<br>
> index d6572cd..f30f92b 100644<br>
> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c<br>
> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c<br>
> @@ -672,7 +672,30 @@ intel_miptree_alloc_mcs(struct intel_context *intel,<br>
>  {<br>
>     assert(mt->mcs_mt == NULL);<br>
>     assert(intel->gen >= 7); /* MCS only used on Gen7+ */<br>
> -   assert(num_samples == 4); /* TODO: support 8x MSAA */<br>
> +<br>
> +   /* Choose the correct format for the MCS buffer.  All that really matters<br>
> +    * is that we allocate the right buffer size, since we'll always be<br>
> +    * accessing this miptree using MCS-specific hardware mechanisms, which<br>
> +    * infer the correct format based on num_samples.<br>
> +    */<br>
> +   gl_format format;<br>
> +   switch (num_samples) {<br>
> +   case 4:<br>
> +      /* 8 bits/pixel are required for MCS data when using 4x MSAA (2 bits for<br>
> +       * each sample).<br>
> +       */<br>
> +      format = MESA_FORMAT_A8;<br>
<br>
</div></div>I'm surprised to see A8 here...it would be nicer, I think, to use R8.  I<br>
guess all that matters here is that you get an 8-bit, 1-component<br>
format...but the luminance/alpha formats are pretty awful and require<br>
special cases in a lot of places, so I'd rather avoid using them<br>
wherever possible.<br>
<br>
That said, I would change it in a separate patch.<br></blockquote><div><br>Ok, will do.  Thanks for the review.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

<div class="HOEnZb"><div class="h5"><br>
> +      break;<br>
> +   case 8:<br>
> +      /* 32 bits/pixel are required for MCS data when using 8x MSAA (3 bits<br>
> +       * for each sample, plus 8 padding bits).<br>
> +       */<br>
> +      format = MESA_FORMAT_R_UINT32;<br>
> +      break;<br>
> +   default:<br>
> +      assert(!"Unrecognized sample count in intel_miptree_alloc_mcs");<br>
> +      break;<br>
> +   };<br>
><br>
>     /* From the Ivy Bridge PRM, Vol4 Part1 p76, "MCS Base Address":<br>
>      *<br>
> @@ -684,7 +707,7 @@ intel_miptree_alloc_mcs(struct intel_context *intel,<br>
>      */<br>
>     mt->mcs_mt = intel_miptree_create(intel,<br>
>                                       mt->target,<br>
> -                                     MESA_FORMAT_A8,<br>
> +                                     format,<br>
>                                       mt->first_level,<br>
>                                       mt->last_level,<br>
>                                       mt->width0,<br>
><br>
<br>
</div></div></blockquote></div><br>