[RFC] drm/xe/guc: Don't support GuC older GuC 70.x releases
Daniele Ceraolo Spurio
daniele.ceraolospurio at intel.com
Wed Feb 7 18:15:43 UTC 2024
On 2/6/2024 8:21 PM, Lucas De Marchi wrote:
> 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.
My aim here was to discuss putting a lower bound to the GuC FW version,
not necessarily merging this immediately (hence the RFC tag). If we
agree to proceed with this approach, I can then sort out the
requirements, like pushing a new PVC FW for testing.
Daniele
>
>> 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
>>
More information about the Intel-xe
mailing list