[PATCH] drm/xe/xe2lpm: Fixup Wa_14020756599

[Lucas De Marchi]

On Thu, Jun 06, 2024 at 07:22:28PM GMT, Tejas Upadhyay wrote:
>In the current situation, Media 2000 is along with xe2_lpg,
>thus WA is already covered.

this needs to be improved to detail the issue the previous approach had.

>
>https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2016

you shouldn't just drop a url in the commit message without saying what
this is. AFAICS should be a Closes:, and in the trailer part

>
>Fixes: 131328aa5699 ("drm/xe/xe2lpm: Add permanent Wa_14020756599")
>Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
>---
> drivers/gpu/drm/xe/xe_rtp.c |  5 -----
> drivers/gpu/drm/xe/xe_rtp.h | 14 --------------
> drivers/gpu/drm/xe/xe_wa.c  | 13 ++++++-------
> 3 files changed, 6 insertions(+), 26 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c
>index 01c32a932780..eff1c9c2f5cc 100644
>--- a/drivers/gpu/drm/xe/xe_rtp.c
>+++ b/drivers/gpu/drm/xe/xe_rtp.c
>@@ -324,8 +324,3 @@ bool xe_rtp_match_first_gslice_fused_off(const struct xe_gt *gt,
> 	return dss >= dss_per_gslice;
> }
>
>-bool xe_rtp_match_when_media2000(const struct xe_gt *gt,
>-				 const struct xe_hw_engine *hwe)
>-{
>-	return (gt_to_xe(gt))->info.media_verx100 == 2000;
>-}
>diff --git a/drivers/gpu/drm/xe/xe_rtp.h b/drivers/gpu/drm/xe/xe_rtp.h
>index a32645f5f80b..337b1ef1959c 100644
>--- a/drivers/gpu/drm/xe/xe_rtp.h
>+++ b/drivers/gpu/drm/xe/xe_rtp.h
>@@ -427,18 +427,4 @@ bool xe_rtp_match_first_render_or_compute(const struct xe_gt *gt,
> bool xe_rtp_match_first_gslice_fused_off(const struct xe_gt *gt,
> 					 const struct xe_hw_engine *hwe);
>
>-/*
>- * xe_rtp_match_when_media2000 - Match when media GT version 2000
>- *
>- * @gt: GT structure
>- * @hwe: Engine instance
>- *
>- * Its one of the case where we need to apply workaround on primary GT
>- * based on if media GT version 2000 is present. Thus this API will help
>- * us to match media version 2000.
>- *
>- * Returns: true if media GT version 2000, false otherwise.
>- */
>-bool xe_rtp_match_when_media2000(const struct xe_gt *gt,
>-				 const struct xe_hw_engine *hwe);
> #endif
>diff --git a/drivers/gpu/drm/xe/xe_wa.c b/drivers/gpu/drm/xe/xe_wa.c
>index 26b170a0cdc7..ce6f1b0ca808 100644
>--- a/drivers/gpu/drm/xe/xe_wa.c
>+++ b/drivers/gpu/drm/xe/xe_wa.c
>@@ -677,6 +677,12 @@ static const struct xe_rtp_entry_sr lrc_was[] = {
> 		       ENGINE_CLASS(RENDER)),
> 	  XE_RTP_ACTIONS(SET(CHICKEN_RASTER_2, TBIMR_FAST_CLIP))
> 	},
>+	/* This WA is also needed on primary GT when the media version is 2000.

please balance the comment /* and */, this is drm, not net:
https://www.kernel.org/doc/html/v6.9/process/coding-style.html#commenting

>+	 * In the current situation, Media 2000 is along with xe2_lpg, thus WA
>+	 * is already covered below. In the future, Media version 2000 can be
>+	 * used with some other graphics version where WA still needs to be
>+	 * implemented.
>+	 */

Suggested reword:

	/*
	 * This WA is also needed on primary GT when the media version is 2000.
	 * Currently, media version 2000 is always paired with graphics version
	 * 2004, so just checking the latter is sufficient. In the future, media
	 * version 2000 can be used with some other graphics version where WA
	 * still needs to be implemented
	 */

I think the underlying issue with duplicate WAs should be solved when we
implement an OR operation in rtp.

Lucas De Marchi

> 	{ XE_RTP_NAME("14020756599"),
> 	  XE_RTP_RULES(GRAPHICS_VERSION(2004), ENGINE_CLASS(RENDER)),
> 	  XE_RTP_ACTIONS(SET(WM_CHICKEN3, HIZ_PLANE_COMPRESSION_DIS))
>@@ -705,13 +711,6 @@ static const struct xe_rtp_entry_sr lrc_was[] = {
> 			     DIS_AUTOSTRIP))
> 	},
>
>-	/* Xe2_LPM */
>-
>-	{ XE_RTP_NAME("14020756599"),
>-	  XE_RTP_RULES(ENGINE_CLASS(RENDER), FUNC(xe_rtp_match_when_media2000)),
>-	  XE_RTP_ACTIONS(SET(WM_CHICKEN3, HIZ_PLANE_COMPRESSION_DIS))
>-	},
>-
> 	{}
> };
>
>-- 
>2.25.1
>