[Mesa-dev] [PATCH 2/2] i965/miptree: Create a disable CCS flag

[Ben Widawsky]

On 17-01-03 08:21:06, Chad Versace wrote:
>On Sun 01 Jan 2017, Ben Widawsky wrote:
>> Cc: Topi Pohjolainen <topi.pohjolainen at intel.com>
>> Cc: Chad Versace <chadversary at chromium.org>
>> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
>> ---
>>  src/mesa/drivers/dri/i965/brw_blorp.c         |  2 +-
>>  src/mesa/drivers/dri/i965/brw_draw.c          |  3 ++-
>>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 21 ++++++++++-----------
>>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 10 +++-------
>>  4 files changed, 16 insertions(+), 20 deletions(-)
>
>
>> @@ -2334,19 +2334,18 @@ intel_miptree_make_shareable(struct brw_context *brw,
>>
>>     if (mt->mcs_buf) {
>>        intel_miptree_all_slices_resolve_color(brw, mt, 0);
>> -      mt->no_ccs = true;
>> +      mt->aux_disable |= (INTEL_AUX_DISABLE_CCS | INTEL_AUX_DISABLE_MCS);
>>        drm_intel_bo_unreference(mt->mcs_buf->bo);
>>        free(mt->mcs_buf);
>>        mt->mcs_buf = NULL;
>>     }
>>
>>     if (mt->hiz_buf) {
>> +      mt->aux_disable |= INTEL_AUX_DISABLE_HIZ;
>>        intel_miptree_all_slices_resolve_depth(brw, mt);
>>        intel_miptree_hiz_buffer_free(mt->hiz_buf);
>>        mt->hiz_buf = NULL;
>>     }
>> -
>> -   mt->aux_disable = INTEL_AUX_DISABLE_ALL;
>>  }
>
>All look goods to me except the above hunk. As written, in some
>instances the driver will allocate (and use) the aux surface *after*
>intel_miptree_make_shareable() unshares the miptree. To fix that, the
>mt->aux_disable bits must be set unconditionally, outside the if's.
>

Getting unconditional disable is exactly what the patches are trying to get rid
of (specifically for the CCS case). So I really hope there is some solution
other than putting it outside of the if's.

>The problem is that the driver sometimes allocates the aux surfaces
>lazily. For evidence, grep for all instances of
>intel_miptree_alloc_hiz() and intel_miptree_alloc_non_msrt_mcs() outside
>of intel_mipmap_tree.c. If the code calls intel_miptree_make_shareable()
>on a miptree for which the aux surface has not yet been allocated yet,
>then intel_miptree_make_shareable() fails to set the disable bit.

Hmm. I wasn't able to find a single regression with this patch, and everything
seemed pretty okay to me with the code. Can you find a specific failing case,
because I cannot?