[PATCH] drm/xe/xe2: Use XE_WA() check

[Upadhyay, Tejas]

> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi at intel.com>
> Sent: Wednesday, July 17, 2024 10:09 PM
> To: Upadhyay, Tejas <tejas.upadhyay at intel.com>
> Cc: intel-xe at lists.freedesktop.org; Roper, Matthew D
> <matthew.d.roper at intel.com>
> Subject: Re: [PATCH] drm/xe/xe2: Use XE_WA() check
> 
> On Wed, Jul 17, 2024 at 08:11:17AM GMT, Upadhyay, Tejas wrote:
> >
> >
> >> -----Original Message-----
> >> From: De Marchi, Lucas <lucas.demarchi at intel.com>
> >> Sent: Tuesday, July 16, 2024 8:52 PM
> >> To: Upadhyay, Tejas <tejas.upadhyay at intel.com>
> >> Cc: intel-xe at lists.freedesktop.org; Roper, Matthew D
> >> <matthew.d.roper at intel.com>
> >> Subject: Re: [PATCH] drm/xe/xe2: Use XE_WA() check
> >>
> >> On Tue, Jul 16, 2024 at 03:16:35PM GMT, Tejas Upadhyay wrote:
> >> >Use XE_WA as used elsewhere.
> >> >
> >> >Fixes: 86c5b70a9c0c ("drm/xe/xe2: Add Wa_15015404425")
> >> >Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> >> >---
> >> > drivers/gpu/drm/xe/xe_mmio.c       | 5 ++++-
> >> > drivers/gpu/drm/xe/xe_wa_oob.rules | 1 +
> >> > 2 files changed, 5 insertions(+), 1 deletion(-)
> >> >
> >> >diff --git a/drivers/gpu/drm/xe/xe_mmio.c
> >> >b/drivers/gpu/drm/xe/xe_mmio.c index ea3c37d3e13f..1138934cf969
> >> 100644
> >> >--- a/drivers/gpu/drm/xe/xe_mmio.c
> >> >+++ b/drivers/gpu/drm/xe/xe_mmio.c
> >> >@@ -13,6 +13,8 @@
> >> > #include <drm/drm_managed.h>
> >> > #include <drm/drm_print.h>
> >> >
> >> >+#include <generated/xe_wa_oob.h>
> >> >+
> >> > #include "regs/xe_bars.h"
> >> > #include "regs/xe_regs.h"
> >> > #include "xe_device.h"
> >> >@@ -22,6 +24,7 @@
> >> > #include "xe_macros.h"
> >> > #include "xe_sriov.h"
> >> > #include "xe_trace.h"
> >> >+#include "xe_wa.h"
> >> >
> >> > static void tiles_fini(void *arg)
> >> > {
> >> >@@ -127,7 +130,7 @@ static void mmio_flush_pending_writes(struct
> >> >xe_gt
> >> *gt)
> >> > 	struct xe_tile *tile = gt_to_tile(gt);
> >> > 	int i;
> >> >
> >> >-	if (tile->xe->info.platform != XE_LUNARLAKE)
> >> >+	if (!XE_WA(gt, 15015404425))
> >>
> >> I'm not sure we can actually call it that early as we have the
> >> following call chain on init:
> >>
> >> xe_device_probe_early()
> >>    xe_mmio_init()
> >> ...
> >> xe_device_probe()
> >>    xe_sriov_init()
> >>    xe_mmio_probe_tiles() <<<<
> >>    xe_gt_init_early()
> >>      xe_wa_process_oob()
> >>
> >> xe_mmio_probe_tiles() does call xe_mmio_read64_2x32() on
> >> xe_root_mmio_gt(). From a quick grep I'm not sure there are other cases.
> >> Can you rebase on https://patchwork.freedesktop.org/series/136146/
> >> to make sure the assert doesn't trigger?
> >>
> >> So... although xe_wa_process_oob() is called very early on gt init,
> >> there are still cases in which it may not be safe. At least for that
> >> mtcfg reg read, I think it's odd to call xe_mmio_* before actually setting up
> the mmio regs per tile.
> >>
> >> And just after writting the above I noticed the
> >>
> >> 	if (tile_count == 1)
> >> 		goto add_mmio_ext;
> >>
> >> uggh, that neeeds to be properly refactored, although doesn't
> >> invalidate the reasoning above.
> >
> >I got this, better to have check for future as well. With rebase do we want to
> get your patch merged first and then mine. Or you want me to send both as 1
> series after rebase?
> 
> I did that patch because I was not sure if it was fine to call XE_WA() where
> you're calling. So first thing is to rebase this patch on top of that check and
> test to make sure it's safe. If it's safe, then we can merge both, otherwise I
> think we need to postpone it until we figure out a way to move the oob wa
> initialization earlier.

It asserts,
[  778.229096] xe 0000:00:02.0: [drm] [drm] Assertion `(gt)->wa_active.oob_initialized` failed!
               platform: LUNARLAKE subplatform: 1
               graphics: (null) 0.00 step **
               media: (null) 0.00 step **
               tile: 0 VRAM 0 B
               GT: 0 type 0
[  778.229121] WARNING: CPU: 1 PID: 1567 at drivers/gpu/drm/xe/xe_mmio.c:133 mmio_flush_pending_writes+0x33d/0x350 [xe]

Looks like need to manage init well. Any suggestions!

Thanks,
Tejas
> 
> Lucas De Marchi
> 
> >
> >Thanks,
> >Tejas
> >>
> >> Lucas De Marchi
> >>
> >> > 		return;
> >> >
> >> > 	/* 4 dummy writes */
> >> >diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules
> >> >b/drivers/gpu/drm/xe/xe_wa_oob.rules
> >> >index 08f7336881e3..3d0e0779a833 100644
> >> >--- a/drivers/gpu/drm/xe/xe_wa_oob.rules
> >> >+++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
> >> >@@ -30,3 +30,4 @@
> >> > 22019338487	MEDIA_VERSION(2000)
> >> > 		GRAPHICS_VERSION(2001)
> >> > 16023588340	GRAPHICS_VERSION(2001)
> >> >+15015404425	GRAPHICS_VERSION(2004)
> >> >--
> >> >2.25.1
> >> >