[RFC] drm/xe/guc: Don't support GuC older GuC 70.x releases

[Lucas De Marchi]

On Tue, Feb 06, 2024 at 03:41:03PM -0800, Daniele Ceraolo Spurio wrote:
>Supporting older GuC versions comes with baggage, both on the coding
>side (due to interfaces only being available from a certain version
>onwards) and on the testing side (due to having to make sure the driver
>works as expected with older GuCs).
>Since all of our Xe platform are still under force probe, we haven't
>committed to support any specific GuC version and we therefore don't
>need to support the older once, which means that we can force a bottom
>limit to what GuC we accept. This allows us to remove any conditional
>statements based on older GuC versions and also to approach newer
>additions knowing that we'll never attempt to load something older
>than our minimum requirement.
>
>RFC: this patch sets the minimum to the current GuC version (70.19),

we are still using PVC for development, even if not completely
supported. We can't update to 70.19 since PVC is not in that version.
Once we have a firmware in at least drm-firmware repo, then I think
we can think about the changes here.

>but that can be moved one way or the other. The main aim here is
>agreeing to stop supporting very old GuC releases on the newer driver.
>
>Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>Cc: John Harrison <John.C.Harrison at Intel.com>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Cc: Matt Roper <matthew.d.roper at intel.com>
>Cc: Matthew Brost <matthew.brost at intel.com>
>Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>---
> drivers/gpu/drm/xe/xe_guc.c   | 14 ++------------
> drivers/gpu/drm/xe/xe_uc_fw.c | 36 ++++++++++++++---------------------
> 2 files changed, 16 insertions(+), 34 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
>index 868208a39829..5e6b27aac495 100644
>--- a/drivers/gpu/drm/xe/xe_guc.c
>+++ b/drivers/gpu/drm/xe/xe_guc.c
>@@ -132,15 +132,10 @@ static u32 guc_ctl_ads_flags(struct xe_guc *guc)
> 	return flags;
> }
>
>-#define GUC_VER(maj, min, pat)	(((maj) << 16) | ((min) << 8) | (pat))
>-
> static u32 guc_ctl_wa_flags(struct xe_guc *guc)
> {
> 	struct xe_device *xe = guc_to_xe(guc);
> 	struct xe_gt *gt = guc_to_gt(guc);
>-	struct xe_uc_fw *uc_fw = &guc->fw;
>-	struct xe_uc_fw_version *version = &uc_fw->versions.found[XE_UC_FW_VER_RELEASE];
>-
> 	u32 flags = 0;
>
> 	if (XE_WA(gt, 22012773006))
>@@ -170,13 +165,8 @@ static u32 guc_ctl_wa_flags(struct xe_guc *guc)
> 	if (XE_WA(gt, 1509372804))
> 		flags |= GUC_WA_RENDER_RST_RC6_EXIT;
>
>-	if (XE_WA(gt, 14018913170)) {
>-		if (GUC_VER(version->major, version->minor, version->patch) >= GUC_VER(70, 7, 0))
>-			flags |= GUC_WA_ENABLE_TSC_CHECK_ON_RC6;
>-		else
>-			drm_dbg(&xe->drm, "Skip WA 14018913170: GUC version expected >= 70.7.0, found %u.%u.%u\n",
>-				version->major, version->minor, version->patch);
>-	}
>+	if (XE_WA(gt, 14018913170))
>+		flags |= GUC_WA_ENABLE_TSC_CHECK_ON_RC6;
>
> 	return flags;
> }
>diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
>index 4714f2c8d2ba..e5bf59616f3d 100644
>--- a/drivers/gpu/drm/xe/xe_uc_fw.c
>+++ b/drivers/gpu/drm/xe/xe_uc_fw.c
>@@ -296,36 +296,28 @@ static void uc_fw_fini(struct drm_device *drm, void *arg)
> 	xe_uc_fw_change_status(uc_fw, XE_UC_FIRMWARE_SELECTED);
> }
>
>-static void guc_read_css_info(struct xe_uc_fw *uc_fw, struct uc_css_header *css)
>+static int guc_read_css_info(struct xe_uc_fw *uc_fw, struct uc_css_header *css)
> {
> 	struct xe_gt *gt = uc_fw_to_gt(uc_fw);
> 	struct xe_uc_fw_version *release = &uc_fw->versions.found[XE_UC_FW_VER_RELEASE];
> 	struct xe_uc_fw_version *compatibility = &uc_fw->versions.found[XE_UC_FW_VER_COMPATIBILITY];
>
> 	xe_gt_assert(gt, uc_fw->type == XE_UC_FW_TYPE_GUC);
>-	xe_gt_assert(gt, release->major >= 70);
>-
>-	if (release->major > 70 || release->minor >= 6) {
>-		/* v70.6.0 adds CSS header support */
>-		compatibility->major = FIELD_GET(CSS_SW_VERSION_UC_MAJOR,
>-						 css->submission_version);
>-		compatibility->minor = FIELD_GET(CSS_SW_VERSION_UC_MINOR,
>-						 css->submission_version);
>-		compatibility->patch = FIELD_GET(CSS_SW_VERSION_UC_PATCH,
>-						 css->submission_version);
>-	} else if (release->minor >= 3) {
>-		/* v70.3.0 introduced v1.1.0 */
>-		compatibility->major = 1;
>-		compatibility->minor = 1;
>-		compatibility->patch = 0;
>-	} else {
>-		/* v70.0.0 introduced v1.0.0 */
>-		compatibility->major = 1;
>-		compatibility->minor = 0;
>-		compatibility->patch = 0;
>+
>+	/* We don't support GuC releases older than 70.19 */
>+	if (release->major < 70 || (release->major == 70 && release->minor < 19)) {
>+		xe_gt_err(gt, "Unsupported GuC v%u.%u! v70.19 or newer is required\n",
>+			  release->major, release->minor);
>+		return -EINVAL;
> 	}
>
>+	compatibility->major = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, css->submission_version);
>+	compatibility->minor = FIELD_GET(CSS_SW_VERSION_UC_MINOR, css->submission_version);
>+	compatibility->patch = FIELD_GET(CSS_SW_VERSION_UC_PATCH, css->submission_version);
>+
> 	uc_fw->private_data_size = css->private_data_size;
>+
>+	return 0;

my main concern is not about raising the version requirement, but that
this is too early. We don't have 70.19 for any platform in
linux-firmware yet. And we don't have it in drm-firmware for PVC at all.

 From the above changes, it doesn't seem we are reducing a lot of code:

	2 files changed, 16 insertions(+), 34 deletions(-)

maybe let's wait a little more before doing that?

Lucas De Marchi

> }
>
> int xe_uc_fw_check_version_requirements(struct xe_uc_fw *uc_fw)
>@@ -424,7 +416,7 @@ static int parse_css_header(struct xe_uc_fw *uc_fw, const void *fw_data, size_t
> 	release->patch = FIELD_GET(CSS_SW_VERSION_UC_PATCH, css->sw_version);
>
> 	if (uc_fw->type == XE_UC_FW_TYPE_GUC)
>-		guc_read_css_info(uc_fw, css);
>+		return guc_read_css_info(uc_fw, css);
>
> 	return 0;
> }
>-- 
>2.43.0
>