Re: [Intel-gfx] [PULL] drm-intel-next-fixes

From: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
To: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: dim-tools@lists.freedesktop.org,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	intel-gfx@lists.freedesktop.org,
	Maxime Ripard <mripard@kernel.org>,
	dri-devel@lists.freedesktop.org,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Andi Shyti <andi.shyti@intel.com>,
	Dave Airlie <airlied@gmail.com>
Subject: Re: [Intel-gfx] [PULL] drm-intel-next-fixes
Date: Fri, 5 Aug 2022 17:25:43 +0200	[thread overview]
Message-ID: <20220805172543.1cdcd58a@maurocar-mobl2> (raw)
In-Reply-To: <Yu0rsGnxcFmvzSPi@intel.com>

On Fri, 5 Aug 2022 10:39:44 -0400
Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:

> On Fri, Aug 05, 2022 at 10:46:57AM +0200, Mauro Carvalho Chehab wrote:
> > Hi Rodrigo,
> > 
> > On Thu, 4 Aug 2022 13:33:06 -0400
> > Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> >   
> > > Hi Dave and Daniel,
> > > 
> > > Here goes drm-intel-next-fixes-2022-08-04:
> > > 
> > > - disable pci resize on 32-bit systems (Nirmoy)
> > > - don't leak the ccs state (Matt)
> > > - TLB invalidation fixes (Chris)
> > > 
> > > Thanks,
> > > Rodrigo.
> > > 
> > > The following changes since commit 2bc7ea71a73747a77e7f83bc085b0d2393235410:
> > > 
> > >   Merge tag 'topic/nouveau-misc-2022-07-27' of git://anongit.freedesktop.org/drm/drm into drm-next (2022-07-27 11:34:07 +1000)
> > > 
> > > are available in the Git repository at:
> > > 
> > >   git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-fixes-2022-08-04
> > > 
> > > for you to fetch changes up to e57b9369e0c6f60155027e120fafd4b57e385b71:
> > > 
> > >   drm/i915/gt: Batch TLB invalidations (2022-08-01 09:48:06 -0400)
> > > 
> > > ----------------------------------------------------------------
> > > - disable pci resize on 32-bit systems (Nirmoy)
> > > - don't leak the ccs state (Matt)
> > > - TLB invalidation fixes (Chris)
> > > 
> > > ----------------------------------------------------------------
> > > Chris Wilson (4):
> > >       drm/i915/gt: Ignore TLB invalidations on idle engines
> > >       drm/i915/gt: Invalidate TLB of the OA unit at TLB invalidations
> > >       drm/i915/gt: Skip TLB invalidations once wedged  
> >   
> > >       drm/i915/gt: Batch TLB invalidations  
> > This patch actually adds a regression due to a silly mistake. 
> > The fix is here:
> > 
> > 	https://patchwork.freedesktop.org/patch/496249/?series=106805&rev=4
> >  
> 
> Thank you for the heads up.
> 
> Since that patch is not merged yet, what are your recommendations here?
> Should I remove this from drm-intel-next-fixes now?

Those patches are OK to merge:

       drm/i915/gt: Ignore TLB invalidations on idle engines
       drm/i915/gt: Invalidate TLB of the OA unit at TLB invalidations
       drm/i915/gt: Skip TLB invalidations once wedged  

And helps reduce performance regressions due to TLB cache invalidation.
So, I would keep them.

With regards to the 4th patch, please don't merge:

	drm/i915/gt: Batch TLB invalidations

or merge it together with:

	drm/i915: pass a pointer for tlb seqno at vma_invalidate_tlb()

> Which regression is worst?

The regression caused by the batch patch is a lot worse, as it
effectively disables TLB cache invalidation. The fix was meant to
be merged at the patch, but somehow between several rebases and 3
machines involved on tests, internal development and upstream, the
branch actually sent upstream was not the right one. It got only
noticed after reviewing a newer patch. Yeah, that sucks.

> Also dim has trouble with fixes for fixes in the same round.
> Please ping me when you get that patch merged so I can pull that.

OK.

Regards,
Mauro