On Fri, Oct 13, 2023 at 02:28:21PM +0200, Nirmoy Das wrote:
> Hi Ville,
>
> On 10/13/2023 12:50 PM, Ville Syrjälä wrote:
> > On Fri, Oct 13, 2023 at 12:31:40PM +0200, Nirmoy Das wrote:
> > > gen8_ggtt_invalidate() is only needed for limitted set of platforms
> > > where GGTT is mapped as WC
> > I know there is supposed to be some kind hw snooping of the ggtt
> > pte writes to invalidate the tlb, but are we sure GFX_FLSH_CNTL
> > has no other side effects we depend on?
>
> I spent some time searching through the gfxspec. This GFX_FLSH_CNTL register
> only seems to be for
>
> invalidating TLB for GUnit and (from git log ) we started to do that to
> enable WC based GGTT updates.
Might be good to cite the relevant git commits in the commit message to
make this clear.
-Sima
>
>
> So if I am not missing anything obvious then this should be safe.
>
>
> Regards,
>
> Nirmoy
>
> >
> > > otherwise this can cause unwanted
> > > side-effects on XE_HP platforms where GFX_FLSH_CNTL_GEN6 is not
> > > valid.
> > >
> > > Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
> > > Cc: Rodrigo Vivi <[email protected]>
> > > Cc: Tvrtko Ursulin <[email protected]>
> > > Cc: Joonas Lahtinen <[email protected]>
> > > Cc: Jani Nikula <[email protected]>
> > > Cc: Jonathan Cavitt <[email protected]>
> > > Cc: John Harrison <[email protected]>
> > > Cc: Andi Shyti <[email protected]>
> > > Cc: <[email protected]> # v6.2+
> > > Suggested-by: Matt Roper <[email protected]>
> > > Signed-off-by: Nirmoy Das <[email protected]>
> > > ---
> > > drivers/gpu/drm/i915/gt/intel_ggtt.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > index 4d7d88b92632..c2858d434bce 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > @@ -197,13 +197,17 @@ void gen6_ggtt_invalidate(struct i915_ggtt *ggtt)
> > > static void gen8_ggtt_invalidate(struct i915_ggtt *ggtt)
> > > {
> > > + struct drm_i915_private *i915 = ggtt->vm.i915;
> > > struct intel_uncore *uncore = ggtt->vm.gt->uncore;
> > > /*
> > > * Note that as an uncached mmio write, this will flush the
> > > * WCB of the writes into the GGTT before it triggers the
> > > invalidate.
> > > + *
> > > + * Only perform this when GGTT is mapped as WC, see ggtt_probe_common().
> > > */
> > > - intel_uncore_write_fw(uncore, GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> > > + if (!IS_GEN9_LP(i915) && GRAPHICS_VER(i915) < 11)
> > > + intel_uncore_write_fw(uncore, GFX_FLSH_CNTL_GEN6,
> > > GFX_FLSH_CNTL_EN);
> > > }
> > > static void guc_ggtt_invalidate(struct i915_ggtt *ggtt)
> > > --
> > > 2.41.0
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
