[PATCH i-g-t 05/23] lib/rendercopy: Always setup clear color for TGL

[Ville Syrjälä]

On Fri, Sep 06, 2024 at 04:09:19PM +0300, Juha-Pekka Heikkila wrote:
> On 2.9.2024 17.37, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > TGL apparently automagically converts regular output to fast
> > clears when the output matches the configured clear color.
> > And if we don't enable the clear color packet at all then we
> > just get some rainbow gibberish on all black parts of the
> > output.
> > 
> > To avoid always set up the clear color packet when using
> > using a non-clear color modifier. We'll just stick a bunch
> > of NaNs into the clear value so it'll never match any
> > legitimate output, and thus automagic fast clear should not
> > happen.
> > 
> > TODO: Hide this better inside rendercopy_gen9.c without
> >        requring extra allocation in the FB BO
> > TODO: Figure out if other platforms need this sort stuff
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >   lib/igt_fb.c          | 24 ++++++++++++++++++++++++
> >   lib/intel_bufops.h    |  1 +
> >   lib/rendercopy_gen9.c | 14 +++++++++++---
> >   3 files changed, 36 insertions(+), 3 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 21c56a454c5a..0eb3897d0f8b 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -965,6 +965,16 @@ void igt_calc_fb_size(struct igt_fb *fb)
> >   		size += calc_plane_size(fb, plane);
> >   	}
> >   
> > +	/*
> > +	 * We always need a clear color on TGL, make some extra
> > +	 * room for one it if it's not explicit in the modifier.
> > +	 *
> > +	 * TODO: probably better to allocate this as part of the
> > +	 * batch instead so the fb size doesn't need to change...
> > +	 */
> > +	if (fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS)
> > +		size = ALIGN(size + 64, 64);
> > +
> >   	if (is_xe_device(fb->fd)) {
> >   		size = ALIGN(size, xe_get_default_alignment(fb->fd));
> >   		if (fb->modifier == I915_FORMAT_MOD_4_TILED_BMG_CCS)
> > @@ -2670,6 +2680,20 @@ igt_fb_create_intel_buf(int fd, struct buf_ops *bops,
> >   	if (fb->modifier == I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC)
> >   		buf->cc.offset = fb->offsets[1];
> >   
> > +	/*
> > +	 * TGL appears to do automagic fast clear when rendering
> > +	 * black and the clear color isn't specified, or when the
> > +	 * output matches the specified clear color. Force a
> > +	 * non-sensical clear color to prevent it from doing this
> > +	 * when using a non-clear color modifier.
> > +	 *
> > +	 * TODO: figure out if other platforms are affected...
> > +	 */
> > +	if (fb->modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS) {
> 
> What about adlp on this? I believe it will now get same treatment as tgl 
> and ci doesn't seem to have adlp on shards so there's no good results(?)

I *think* adl wasn't affected, but I'll need to double check that...

Although apart from allocating a bit of extra memory this should
be completely harmeless on unaffected platforms.

> 
> /Juha-Pekka
> 
> > +		buf->cc.disable = true;
> > +		buf->cc.offset = fb->size - 64;
> > +	}
> > +
> >   	return buf;
> >   }
> >   
> > diff --git a/lib/intel_bufops.h b/lib/intel_bufops.h
> > index 84e71d41a2c2..06e72ba4ba93 100644
> > --- a/lib/intel_bufops.h
> > +++ b/lib/intel_bufops.h
> > @@ -38,6 +38,7 @@ struct intel_buf {
> >   	} ccs[2];
> >   	struct {
> >   		uint32_t offset;
> > +		bool disable;
> >   	} cc;
> >   	struct {
> >   		uint64_t offset;
> > diff --git a/lib/rendercopy_gen9.c b/lib/rendercopy_gen9.c
> > index 5ee4c89f5cdb..f0757a8e6b78 100644
> > --- a/lib/rendercopy_gen9.c
> > +++ b/lib/rendercopy_gen9.c
> > @@ -1133,15 +1133,23 @@ void _gen9_render_op(struct intel_bb *ibb,
> >   
> >   	gen12_emit_aux_pgtable_state(ibb, aux_pgtable_state, true);
> >   
> > -	if (fast_clear) {
> > +	if (fast_clear || dst->cc.disable) {
> >   		for (int i = 0; i < 4; i++) {
> >   			intel_bb_out(ibb, MI_STORE_DWORD_IMM_GEN4);
> >   			intel_bb_emit_reloc(ibb, dst->handle,
> >   					    I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
> >                                               dst->cc.offset + i*sizeof(float),
> >   					    dst->addr.offset);
> > -			intel_bb_out(ibb, *(uint32_t*)&clear_color[i]);
> > -               }
> > +			if (fast_clear) {
> > +				intel_bb_out(ibb, *(uint32_t*)&clear_color[i]);
> > +			} else {
> > +				/*
> > +				 * Emit NaN so it'll match nothing and thus prevent
> > +				 * TGL from doing its automagic fast clear tricks.
> > +				 */
> > +				intel_bb_out(ibb, 0xffffffff);
> > +			}
> > +		}
> >          }
> >   
> >   

-- 
Ville Syrjälä
Intel