[PATCH V2 2/2] drm/xe/xe2lpm: Add permanent wa_14020756599

[Matt Roper]

On Mon, May 20, 2024 at 11:23:12PM -0700, Upadhyay, Tejas wrote:
> 
> 
> > -----Original Message-----
> > From: Roper, Matthew D <matthew.d.roper at intel.com>
> > Sent: Wednesday, May 15, 2024 9:01 AM
> > To: Upadhyay, Tejas <tejas.upadhyay at intel.com>
> > Cc: intel-xe at lists.freedesktop.org; Ghimiray, Himal Prasad
> > <himal.prasad.ghimiray at intel.com>
> > Subject: Re: [PATCH V2 2/2] drm/xe/xe2lpm: Add permanent
> > wa_14020756599
> > 
> > On Tue, Apr 30, 2024 at 01:26:09PM +0530, Tejas Upadhyay wrote:
> > > For xe2_lpm Wa_14020756599 is applied to all steppings.
> > >
> > > V2:
> > >   - Remove engine filter video decode
> > >   - Fix typo GRAPHICS/MEDIA/s - Himal
> > >
> > > Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_wa.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/xe/xe_wa.c b/drivers/gpu/drm/xe/xe_wa.c
> > > index 7657aadf217c..76f7a7e3ee61 100644
> > > --- a/drivers/gpu/drm/xe/xe_wa.c
> > > +++ b/drivers/gpu/drm/xe/xe_wa.c
> > > @@ -701,6 +701,12 @@ static const struct xe_rtp_entry_sr lrc_was[] = {
> > >  			     DIS_AUTOSTRIP))
> > >  	},
> > >
> > > +	/* Xe2_LPM */
> > > +	{ XE_RTP_NAME("14020756599"),
> > > +	  XE_RTP_RULES(MEDIA_VERSION(2000)),
> > 
> > Even though this workaround needs to be applied on any platform with
> > media version 20.00 (regardless of the primary GT's version), there's still only
> > a single copy of this register and it resides within the primary GT as part of
> > the render engine's LRC (bspec 65182).  This is a pretty unusual case (turning
> > off something in a different hardware unit than what the workaround is
> > marked against) and we might need to do something special for this
> > workaround since we need to emit this workaround specifically as part of the
> > RCS engine's LRC (you don't have an engine filter here, so it's going to try to
> > put it in every engine's LRC), and the use of MEDIA_VERSION also implicitly
> > makes our RTP framework use the media GT to apply it rather than the
> > primary GT.
> > Since this register is within the GSI range, applying the workaround through
> > the media GT will make it try to write to offset
> > (0x380000+0x5588) which won't do what we want.
> > 
> > For now the only platforms that have media 20.00 also have primary GT
> > versions that match the Xe2_LPG version number, so we could probably get
> > away with just putting a comment in the table saying that we're not adding
> > the Xe2_LPM entry for now since the Xe2_LPG entry already covers the
> > necessary programming.  But if they ever pair media 20.00 with some other
> > primary GT version that doesn't need this workaround, we may have to think
> > about adding more flags to RTP to deal with this kind of situation.
> 
> Do you think, handling it this way would be good for future use well?

Yeah, I think logic like that should work (although you might want to
use MEDIA_VERx100 and match 2000 exactly).  It might also be worth
naming the function something more specific (e.g.,
"xe_rtp_match_rcs_when_media2000") that makes it more clear exactly 
what it's applying to.


Matt

> 
> diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c
> index fb44cc7521d8..54b99f31a538 100644
> --- a/drivers/gpu/drm/xe/xe_rtp.c
> +++ b/drivers/gpu/drm/xe/xe_rtp.c
> @@ -309,6 +309,14 @@ bool xe_rtp_match_first_render_or_compute(const struct xe_gt *gt,
>                 hwe->engine_id == __ffs(render_compute_mask);
>  }
> 
> +bool xe_rtp_match_media_need_wa(const struct xe_gt *gt,
> +                               const struct xe_hw_engine *hwe)
> +{
> +       if (!xe_gt_is_media_type(gt) && hwe->class == XE_ENGINE_CLASS_RENDER)
> +               return MEDIA_VER(gt_to_xe(gt)) == 20;
> +       return false;
> +}
> +
>  bool xe_rtp_match_first_gslice_fused_off(const struct xe_gt *gt,
>                                          const struct xe_hw_engine *hwe)
>  {
> diff --git a/drivers/gpu/drm/xe/xe_wa.c b/drivers/gpu/drm/xe/xe_wa.c
> index 05db53c1448c..8a374afb789f 100644
> --- a/drivers/gpu/drm/xe/xe_wa.c
> +++ b/drivers/gpu/drm/xe/xe_wa.c
> @@ -227,6 +227,11 @@ static const struct xe_rtp_entry_sr gt_was[] = {
>           XE_RTP_ACTIONS(SET(VDBOX_CGCTL3F08(0), CG3DDISHRS_CLKGATE_DIS)),
>           XE_RTP_ENTRY_FLAG(FOREACH_ENGINE),
>         },
> +       { XE_RTP_NAME("14020756599"),
> +         XE_RTP_RULES(FUNC(xe_rtp_match_media_need_wa)),
> +         XE_RTP_ACTIONS(SET(WM_CHICKEN3, HIZ_PLANE_COMPRESSION_DIS))
> +       },
> +
> 
>         /* Xe2_HPM */
> 
> Thanks,
> Tejas
> > 
> > 
> > Matt
> > 
> > > +	  XE_RTP_ACTIONS(SET(WM_CHICKEN3,
> > HIZ_PLANE_COMPRESSION_DIS))
> > > +	},
> > > +
> > >  	{}
> > >  };
> > >
> > > --
> > > 2.25.1
> > >
> > 
> > --
> > Matt Roper
> > Graphics Software Engineer
> > Linux GPU Platform Enablement
> > Intel Corporation

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