[Mesa-dev] [Mesa-stable] [PATCH] amd/common/gfx9: workaround DCC corruption more conservatively

[Andres Gomez]

Nicolai, this patch depends on 759526813be which didn't make it for
17.2

Hence, I could recover that if you want to nominate it or we would need
a backport. Otherwise, I'm dropping this patch.

Let me know what you think.

On Thu, 2017-10-12 at 11:24 +0200, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
> 
> Fixes KHR-GL45.texture_swizzle.smoke and others on Vega.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102809
> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/amd/common/ac_surface.c | 32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/src/amd/common/ac_surface.c b/src/amd/common/ac_surface.c
> index 22c653f0c4f..ea348211787 100644
> --- a/src/amd/common/ac_surface.c
> +++ b/src/amd/common/ac_surface.c
> @@ -917,23 +917,25 @@ static int gfx9_compute_miptree(ADDR_HANDLE addrlib,
>  	} else {
>  		/* DCC */
>  		if (!(surf->flags & RADEON_SURF_DISABLE_DCC) &&
>  		    !(surf->flags & RADEON_SURF_SCANOUT) &&
>  		    !compressed &&
>  		    in->swizzleMode != ADDR_SW_LINEAR &&
>  		    /* TODO: We could support DCC with MSAA. */
>  		    in->numSamples == 1) {
>  			ADDR2_COMPUTE_DCCINFO_INPUT din = {0};
>  			ADDR2_COMPUTE_DCCINFO_OUTPUT dout = {0};
> +			ADDR2_META_MIP_INFO meta_mip_info[RADEON_SURF_MAX_LEVELS] = {};
>  
>  			din.size = sizeof(ADDR2_COMPUTE_DCCINFO_INPUT);
>  			dout.size = sizeof(ADDR2_COMPUTE_DCCINFO_OUTPUT);
> +			dout.pMipInfo = meta_mip_info;
>  
>  			din.dccKeyFlags.pipeAligned = 1;
>  			din.dccKeyFlags.rbAligned = 1;
>  			din.colorFlags = in->flags;
>  			din.resourceType = in->resourceType;
>  			din.swizzleMode = in->swizzleMode;
>  			din.bpp = in->bpp;
>  			din.unalignedWidth = in->width;
>  			din.unalignedHeight = in->height;
>  			din.numSlices = in->numSlices;
> @@ -945,35 +947,51 @@ static int gfx9_compute_miptree(ADDR_HANDLE addrlib,
>  			if (ret != ADDR_OK)
>  				return ret;
>  
>  			surf->u.gfx9.dcc.rb_aligned = din.dccKeyFlags.rbAligned;
>  			surf->u.gfx9.dcc.pipe_aligned = din.dccKeyFlags.pipeAligned;
>  			surf->u.gfx9.dcc_pitch_max = dout.pitch - 1;
>  			surf->dcc_size = dout.dccRamSize;
>  			surf->dcc_alignment = dout.dccRamBaseAlign;
>  			surf->num_dcc_levels = in->numMipLevels;
>  
> -			/* Disable DCC for the smallest levels. It seems to be
> -			 * required for DCC readability between CB and shaders
> -			 * when TC L2 isn't flushed. This was guessed.
> +			/* Disable DCC for levels that are in the mip tail.
> +			 *
> +			 * There are two issues that this is intended to
> +			 * address:
> +			 *
> +			 * 1. Multiple mip levels may share a cache line. This
> +			 *    can lead to corruption when switching between
> +			 *    rendering to different mip levels because the
> +			 *    RBs don't maintain coherency.
> +			 *
> +			 * 2. Texturing with metadata after rendering sometimes
> +			 *    fails with corruption, probably for a similar
> +			 *    reason.
> +			 *
> +			 * Working around these issues for all levels in the
> +			 * mip tail may be overly conservative, but it's what
> +			 * Vulkan does.
>  			 *
>  			 * Alternative solutions that also work but are worse:
> -			 * - Disable DCC.
> +			 * - Disable DCC entirely.
>  			 * - Flush TC L2 after rendering.
>  			 */
> -			for (unsigned i = 1; i < in->numMipLevels; i++) {
> -				if (mip_info[i].pitch *
> -				    mip_info[i].height * surf->bpe < 1024) {
> +			for (unsigned i = 0; i < in->numMipLevels; i++) {
> +				if (meta_mip_info[i].inMiptail) {
>  					surf->num_dcc_levels = i;
>  					break;
>  				}
>  			}
> +
> +			if (!surf->num_dcc_levels)
> +				surf->dcc_size = 0;
>  		}
>  
>  		/* FMASK */
>  		if (in->numSamples > 1) {
>  			ADDR2_COMPUTE_FMASK_INFO_INPUT fin = {0};
>  			ADDR2_COMPUTE_FMASK_INFO_OUTPUT fout = {0};
>  
>  			fin.size = sizeof(ADDR2_COMPUTE_FMASK_INFO_INPUT);
>  			fout.size = sizeof(ADDR2_COMPUTE_FMASK_INFO_OUTPUT);
>  
-- 
Br,

Andres