Re: [PATCH i-g-t 6/6] lib/rendercopy: Enable clear color consistently

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Jun 25, 2024 at 08:40:32PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We are computing the clear color enable bit differently
> for the reloc vs. what we stuff into the surface state
> directly. Unify.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  lib/rendercopy_gen9.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/rendercopy_gen9.c b/lib/rendercopy_gen9.c
> index 04ec7ec99d9a..b4d34bc6864f 100644
> --- a/lib/rendercopy_gen9.c
> +++ b/lib/rendercopy_gen9.c
> @@ -146,6 +146,12 @@ static const uint32_t xe2_render_copy[][4] = {
>  	{ 0x8010c031, 0x00000004, 0x58000c24, 0x00c40000 },
>  };
>  
> +static bool cc_enable(struct intel_bb *ibb,
> +		      const struct intel_buf *buf, bool fast_clear)
> +{
> +	return fast_clear || (buf->cc.offset && !HAS_FLATCCS(ibb->devid));

This whole thing is actually wrong. We just want the
buf->cc.offset!=0 check and nothing else, regardless of
platform. Otherwise sampling from previously fast
cleared buffers will not work correctly on dg2/etc.

> +}
> +
>  /* Mostly copy+paste from gen6, except height, width, pitch moved */
>  static uint32_t
>  gen9_bind_buf(struct intel_bb *ibb, const struct intel_buf *buf, int is_dst,
> @@ -238,7 +244,7 @@ gen9_bind_buf(struct intel_bb *ibb, const struct intel_buf *buf, int is_dst,
>  
>  			address = intel_bb_offset_reloc_with_delta(ibb, buf->handle,
>  								   read_domain, write_domain,
> -								   (buf->cc.offset ? (1 << 10) : 0)
> +								   (cc_enable(ibb, buf, fast_clear) ? (1 << 10) : 0)
>  								   | buf->ccs[0].offset,
>  								   intel_bb_offset(ibb) + 4 * 10,
>  								   buf->addr.offset);
> @@ -246,7 +252,7 @@ gen9_bind_buf(struct intel_bb *ibb, const struct intel_buf *buf, int is_dst,
>  			ss->ss11.aux_base_addr_hi = (address + buf->ccs[0].offset) >> 32;
>  		}
>  
> -		if (fast_clear || (buf->cc.offset && !HAS_FLATCCS(ibb->devid))) {
> +		if (cc_enable(ibb, buf, fast_clear)) {
>  			igt_assert(buf->compression == I915_COMPRESSION_RENDER);
>  
>  			ss->ss10.clearvalue_addr_enable = 1;
> -- 
> 2.44.2

-- 
Ville Syrjälä
Intel