Re: [PATCH] drm/xe: Mark ComputeCS read mode as UC on iGPU

["Summers, Stuart" <stuart.summers@intel.com>]

On Mon, 2025-01-13 at 23:57 +0000, Summers, Stuart wrote:
> On Thu, 2024-12-12 at 15:46 -0800, Matthew Brost wrote:
> > RING_CMD_CCTL read index should be UC on iGPU parts due to L3
> > caching
> > structure. Having this as WB blocks ULLS from being enabled. Change
> > to
> > UC to unblock ULLS on iGPU.
> > 
> > Cc: Balasubramani Vivekanandan
> > <balasubramani.vivekanandan@intel.com>
> > Cc: Michal Mrozek <michal.mrozek@intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: stable@vger.kernel.org
> > Fixes: 328e089bfb37 ("drm/xe: Leverage ComputeCS read L3 caching")
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_hw_engine.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c
> > b/drivers/gpu/drm/xe/xe_hw_engine.c
> > index b19366744148..cc258b2a77c9 100644
> > --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> > +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> > @@ -417,9 +417,14 @@ hw_engine_setup_default_state(struct
> > xe_hw_engine *hwe)
> >          * entry is recommended by the spec in certain
> > circumstances
> > on
> >          * specific platforms.
> >          * Bspec: 72161
> > +        *
> > +        * XXX: According to internal communications bspec is wrong
> > and
> 
> Can you remove this reference to "internal communications"? It looks
> like from the latest in bspec this is pretty clear.
> 
> I do see one test regressing on LNL, can you double check that this
> isn't related to the caching change before we merge this?
> 
> Otherwise lgtm

Reviewed-by: Stuart Summers <stuart.summers@intel.com>

Before merging, can you get a CI re-run of this since it's now been a
month or so and we have that LNL failure? It doesn't seem related here,
but I don't see anything in ci-buglog right now that looks specifically
like that signature.

Thanks,
Stuart

> 
> Thanks,
> Stuart
> 
> > +        * RING_CMD_CCTL read index should be UC on iGPU parts due
> > to
> > L3 caching
> > +        * structure. Having this as WB blocks ULLS from being
> > enabled. Add
> > +        * bspec link or delete this comment once bspec is updated.
> >          */
> >         const u8 mocs_write_idx = gt->mocs.uc_index;
> > -       const u8 mocs_read_idx = hwe->class ==
> > XE_ENGINE_CLASS_COMPUTE &&
> > +       const u8 mocs_read_idx = hwe->class ==
> > XE_ENGINE_CLASS_COMPUTE && IS_DGFX(xe) &&
> >                                  (GRAPHICS_VER(xe) >= 20 || xe-
> > > info.platform == XE_PVC) ?
> >                                  gt->mocs.wb_index : gt-
> > > mocs.uc_index;
> >         u32 ring_cmd_cctl_val =
> > REG_FIELD_PREP(CMD_CCTL_WRITE_OVERRIDE_MASK, mocs_write_idx) |
>