Re: [PATCH i-g-t v2 05/18] lib/rendercopy: Skip AUX surface setup in TGL+

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

On Mon, Sep 23, 2024 at 12:51:53PM +0300, Juha-Pekka Heikkila wrote:
> Took a while to realize why this didn't break anything but since hw 
> agree it's all ok. I wonder if on the changed line this check
> "buf->ccs[0].stride" is really needed, I think that happen only with 
> flatccs which doesn't exist on <gen12

Hmm. The code populating that seems quite dodgy. I can't even
figure out what it ends up doing. I'll see if I can fix it to
make sense.

I think the correct rules should be:
- clear color should not be part of ccs[], currently 
  flat ccs platforms seem to violate that
- flat ccs platforms don't need ccs[], but again we
  seem to put nonsense there for those

> 
> Anyway,
> Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> 
> On 18.9.2024 15.05, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > On TGL+ the AUX stuff is handled via the magic AUX pagetables.
> > Mesa code tells me that the hardware only uses that the
> > SURFACE_STATE AUX stuff for MCS, which is of no interest to us.
> > So just skip the AUX setup in SURFACE_STATE.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   lib/rendercopy_gen9.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/lib/rendercopy_gen9.c b/lib/rendercopy_gen9.c
> > index b33e7fc9feba..719307e6f45b 100644
> > --- a/lib/rendercopy_gen9.c
> > +++ b/lib/rendercopy_gen9.c
> > @@ -261,8 +261,7 @@ gen9_bind_buf(struct intel_bb *ibb, const struct intel_buf *buf, int is_dst,
> >   		else
> >   			ss->ss6.aux_mode = 0x5; /* AUX_CCS_E */
> >   
> > -		if (buf->ccs[0].stride) {
> > -
> > +		if (intel_gen(ibb->devid) < 12 && buf->ccs[0].stride) {
> >   			ss->ss6.aux_pitch = (buf->ccs[0].stride / 128) - 1;
> >   
> >   			address = intel_bb_offset_reloc_with_delta(ibb, buf->handle,

-- 
Ville Syrjälä
Intel