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

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Feb 06, 2024 at 05:18:50PM -0800, John Harrison wrote:
> On 2/6/2024 15:41, 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),
> > but that can be moved one way or the other. The main aim here is
> Ideally, this would be bumped every time we update Xe to a newer firmware
> version right up to the point when force probe is lifted. At that point it
> becomes fixed and we have to add the version check support back in for
> future w/a's and features.
> 
> Get's my vote :).
> 

Mine too.

With that:
Acked-by: Matthew Brost <matthew.brost@intel.com>

> John.
> 
> > agreeing to stop supporting very old GuC releases on the newer driver.
> > 
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: John Harrison <John.C.Harrison@Intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: Matthew Brost <matthew.brost@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@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;
> >   }
> >   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;
> >   }
>