Re: [PATCH] drm/xe/xelp: Fix Wa_18022495364

[Matt Roper <matthew.d.roper@intel.com>]

On Mon, Jan 19, 2026 at 01:34:02PM +0000, Tvrtko Ursulin wrote:
> 
> On 16/01/2026 16:46, Matt Roper wrote:
> > On Fri, Jan 16, 2026 at 09:50:40AM +0000, Tvrtko Ursulin wrote:
> > > It looks I mistyped CS_DEBUG_MODE2 as CS_DEBUG_MODE1 when adding the
> > > workaround. Fix it.
> > 
> > This matches the explanation of "option 1" for the workaround, but I'm
> > wondering if we want/need this workaround at all.  Option 1 is to write
> > the CS_DEBUG_MODE2 register (as you're doing here), but Option 2 is to
> > do a constant cache invalidation (PIPE_CONTROL[DW1][Bit3]) during
> > top-of-pipe invalidation and it looks like we already do that in general
> > in emit_pipe_invalidate(), so it seems like we're implementing both
> > options at the same time.  It looks like there's similar redundancy in
> > i915 as well...
> > 
> > Are you seeing the programming of the correct register here actually
> > change/fix anything?  If so, does just deleting the programming of the
> > wrong register without programming the right one also fix the issue?
> 
> So far the off-line reports from people doing the testing appear to suggest
> this fix indeed, well, fixes it.
> 
> If that is confirmed we will need to add:
> 
> Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/13630

I don't think we should mark an Xe change as fixing a general mesa
ticket since Xe is the wrong driver to be using on gen12 platforms. We'd
need an i915 fix instead for that (and i915 is already using the correct
CS_DEBUG_MODE2 register so I guess there must be some other factor
involved as well).

> 
> As to your wider conundrum - could it be that preemption is at play? When
> done from the indirect context the workaround will trigger after preemption,
> unlike when done from emit_pipe_invalidate(). So perhaps "Option 1" and
> "Option 2" you mention miss that angle?

Possibly.  I just read through the old internal history on this
workaround and it sounds like the expectation is that the underlying
issue this was originally added for would only happen when both RCS+POCS
(i.e., the POSH command streamer) are in use, which isn't something
that's relevant to Linux.  So most likely the problems you're stumbling
over some different issue than what Wa_18022495364 was initially
intended for, but it turns out that the register programming for Option
1 also solves this new issue, whereas Option 2 does not.

Anyway, your change to fix the register name is correct, so

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> 
> Regards,
> 
> Tvrtko
> 
> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> > > Fixes: ca33cd271ef9 ("drm/xe/xelp: Add Wa_18022495364")
> > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: <stable@vger.kernel.org> # v6.18+
> > > ---
> > >   drivers/gpu/drm/xe/xe_lrc.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
> > > index 70eae7d03a27..44f112df4eb2 100644
> > > --- a/drivers/gpu/drm/xe/xe_lrc.c
> > > +++ b/drivers/gpu/drm/xe/xe_lrc.c
> > > @@ -1200,7 +1200,7 @@ static ssize_t setup_invalidate_state_cache_wa(struct xe_lrc *lrc,
> > >   		return -ENOSPC;
> > >   	*cmd++ = MI_LOAD_REGISTER_IMM | MI_LRI_NUM_REGS(1);
> > > -	*cmd++ = CS_DEBUG_MODE1(0).addr;
> > > +	*cmd++ = CS_DEBUG_MODE2(0).addr;
> > >   	*cmd++ = _MASKED_BIT_ENABLE(INSTRUCTION_STATE_CACHE_INVALIDATE);
> > >   	return cmd - batch;
> > > -- 
> > > 2.52.0
> > > 
> > 
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation