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

[Lucas De Marchi]

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.

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
>> >