[Mesa-dev] [PATCH 06/16] i965/miptree: Do not rely on msaa type to decide if aux is needed

Jason Ekstrand jason at jlekstrand.net
Mon Jul 17 16:27:57 UTC 2017


Oops, forgot to add:

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

On Mon, Jul 17, 2017 at 9:27 AM, Jason Ekstrand <jason at jlekstrand.net>
wrote:

> On Mon, Jul 17, 2017 at 6:34 AM, Topi Pohjolainen <
> topi.pohjolainen at gmail.com> wrote:
>
>> Once the driver moves to ISL both compressed and uncompressed have
>> the same type. One needs to tell them apart by other means. This
>> can be done by checking the existence of mcs_buf.
>>
>> There is a short period of time within intel_miptree_create()
>> where mcs_buf doesn't exist yet (between calls to
>> intel_miptree_create_layout() and intel_miptree_alloc_mcs()).
>> First compute_msaa_layout() makes the decision if compression is
>> to be used and sets the msaa_layout type. Then based on the type
>> one sets aux_usage and finally decides if mcs_buf is needed.
>>
>> This patch duplicates the logic in compute_msaa_layout() and uses
>> that to make the decision on aux_usage and mcs_buf allocation.
>> Most of the original logic in compute_msaa_layout() will be gone
>> in the following patch leaving only one version.
>>
>
> Not true since you pulled this patch out for early merging. :-)
>
> That said, I think it's ok to duplicate here.  I'm not sure what we're
> going to want all this to look like in the ISLified future anyway.
> Probably something like Vulkan where we just call isl_surf_init_mcs and set
> aux_layout to NONE if it fails.  In any case, this doesn't seem to make
> anything worse.
>
>
>> Elsewhere only brw_populate_sampler_prog_key_data() needs to know
>> if compression is used based on the msaa_type. This is now
>> replaced with consideration for number of samples and existence
>> of mcs_buf. All other occurrences consider CMS || UMS which can
>> be represented using single the type of ISL_MSAA_LAYOUT_ARRAY
>> without any tweaks.
>>
>> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>> ---
>>  src/mesa/drivers/dri/i965/brw_wm.c            |  8 +++--
>>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 43
>> ++++++++++++++++++++++++++-
>>  2 files changed, 47 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c
>> b/src/mesa/drivers/dri/i965/brw_wm.c
>> index 3a5fcf5485..18056d51d0 100644
>> --- a/src/mesa/drivers/dri/i965/brw_wm.c
>> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
>> @@ -396,9 +396,11 @@ brw_populate_sampler_prog_key_data(struct
>> gl_context *ctx,
>>           /* From gen9 onwards some single sampled buffers can also be
>>            * compressed. These don't need ld2dms sampling along with mcs
>> fetch.
>>            */
>> -         if (brw->gen >= 7 &&
>> -             intel_tex->mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS &&
>> -             intel_tex->mt->num_samples > 1) {
>> +         if (intel_tex->mt->aux_usage == ISL_AUX_USAGE_MCS) {
>> +            assert(brw->gen >= 7);
>> +            assert(intel_tex->mt->num_samples > 1);
>> +            assert(intel_tex->mt->mcs_buf);
>> +            assert(intel_tex->mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS);
>>              key->compressed_multisample_layout_mask |= 1 << s;
>>
>>              if (intel_tex->mt->num_samples >= 16) {
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> index 69a9328d6c..bef544c0ae 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> @@ -62,6 +62,45 @@ static bool
>>  intel_miptree_alloc_aux(struct brw_context *brw,
>>                          struct intel_mipmap_tree *mt);
>>
>> +static bool
>> +is_mcs_supported(const struct brw_context *brw, mesa_format format,
>> +                 uint32_t layout_flags)
>> +{
>> +   /* Prior to Gen7, all MSAA surfaces used IMS layout. */
>> +   if (brw->gen < 7)
>> +      return false;
>> +
>> +   /* In Gen7, IMS layout is only used for depth and stencil buffers. */
>> +   switch (_mesa_get_format_base_format(format)) {
>> +   case GL_DEPTH_COMPONENT:
>> +   case GL_STENCIL_INDEX:
>> +   case GL_DEPTH_STENCIL:
>> +      return false;
>> +   default:
>> +      /* From the Ivy Bridge PRM, Vol4 Part1 p77 ("MCS Enable"):
>> +       *
>> +       *   This field must be set to 0 for all SINT MSRTs when all RT
>> channels
>> +       *   are not written
>> +       *
>> +       * In practice this means that we have to disable MCS for all
>> signed
>> +       * integer MSAA buffers.  The alternative, to disable MCS only
>> when one
>> +       * of the render target channels is disabled, is impractical
>> because it
>> +       * would require converting between CMS and UMS MSAA layouts on
>> the fly,
>> +       * which is expensive.
>> +       */
>> +      if (brw->gen == 7 && _mesa_get_format_datatype(format) == GL_INT)
>> {
>> +         return false;
>> +      } else if (layout_flags & MIPTREE_LAYOUT_DISABLE_AUX) {
>> +         /* We can't use the CMS layout because it uses an aux buffer,
>> the MCS
>> +          * buffer. So fallback to UMS, which is identical to CMS
>> without the
>> +          * MCS. */
>> +         return false;
>> +      } else {
>> +         return true;
>> +      }
>> +   }
>> +}
>> +
>>  /**
>>   * Determine which MSAA layout should be used by the MSAA surface being
>>   * created, based on the chip generation and the surface type.
>> @@ -566,7 +605,9 @@ intel_miptree_choose_aux_usage(struct brw_context
>> *brw,
>>  {
>>     assert(mt->aux_usage == ISL_AUX_USAGE_NONE);
>>
>> -   if (mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS) {
>> +   const unsigned no_flags = 0;
>> +   if (mt->num_samples > 1 && is_mcs_supported(brw, mt->format,
>> no_flags)) {
>> +      assert(mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS);
>>        mt->aux_usage = ISL_AUX_USAGE_MCS;
>>     } else if (intel_tiling_supports_ccs(brw, mt->tiling) &&
>>                intel_miptree_supports_ccs(brw, mt)) {
>> --
>> 2.11.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170717/aebdfd25/attachment.html>


More information about the mesa-dev mailing list