On 10/13/2023 6:15 PM, Daniel Vetter wrote:
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.

Yes, I should. It took me a while to find it. Going to add that and resend the patch.


Thanks,

Nirmoy

-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 <rodrigo.v...@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursu...@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
Cc: Jani Nikula <jani.nik...@linux.intel.com>
Cc: Jonathan Cavitt <jonathan.cav...@intel.com>
Cc: John Harrison <john.c.harri...@intel.com>
Cc: Andi Shyti <andi.sh...@linux.intel.com>
Cc: <sta...@vger.kernel.org> # v6.2+
Suggested-by: Matt Roper <matthew.d.ro...@intel.com>
Signed-off-by: Nirmoy Das <nirmoy....@intel.com>
---
   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

Reply via email to