Re: [PATCH 3/9] drm/i915/uc: Unify uC FW selection

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Daniele Ceraolo Spurio (2019-07-23 00:20:42)
> +/* must be ordered base on platform + revid, from newer to older */
> +static const struct intel_uc_platform_requirement guc_fw_blobs[] = {
> +       { INTEL_ICELAKE,        0,      &icl_guc_fw_blob },
> +       { INTEL_COFFEELAKE,     0,      &kbl_guc_fw_blob },
> +       { INTEL_GEMINILAKE,     0,      &glk_guc_fw_blob },
> +       { INTEL_KABYLAKE,       0,      &kbl_guc_fw_blob },
> +       { INTEL_BROXTON,        0,      &bxt_guc_fw_blob },
> +       { INTEL_SKYLAKE,        0,      &skl_guc_fw_blob },
> +};

> +/**
> + * intel_uc_fw_select - select the uC firmware to fetch & load
> + * @i915: device private
> + * @uc_fw: uC firmware
> + * @list: list of required firmware for each platform
> + * @length: number of entries in the list
> + *
> + * Select the uC firmware from the provided list based on platform and revid of
> + * the device we're on. If the firmware_path modparam override is set, it takes
> + * precedence over the entries in the list.
> + */
> +void intel_uc_fw_select(struct drm_i915_private *i915,
> +                       struct intel_uc_fw *uc_fw,
> +                       const struct intel_uc_platform_requirement *list,
> +                       unsigned int length)

If this is a list, do we need a length? Is a sentinel not good enough?

> +{
> +       GEM_BUG_ON(uc_fw->fetch_status != INTEL_UC_FIRMWARE_UNINITIALIZED);
> +
> +       if (!HAS_UC(i915)) {
> +               uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
> +               return;
> +       }

if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST)) {
	for (i = 1; i < length; i++)
		if (list[i].first_rev <= list[i-1].first_rev) {
			pr_err("...");
			uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
			return;
		}
}

> +
> +       uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED;
> +
> +       if (unlikely(i915_modparams.guc_firmware_path &&
> +                    uc_fw->type == INTEL_UC_FW_TYPE_GUC)) {
> +               uc_fw->path = i915_modparams.guc_firmware_path;
> +       } else if (unlikely(i915_modparams.huc_firmware_path &&
> +                           uc_fw->type == INTEL_UC_FW_TYPE_HUC)) {
> +               uc_fw->path = i915_modparams.huc_firmware_path;
> +       } else {
> +               enum intel_platform p = INTEL_INFO(i915)->platform;
> +               u8 rev = INTEL_REVID(i915);
> +               int i;
> +
> +               for (i = 0; i < length && p <= list[i].p; i++) {
> +                       if (p == list[i].p && rev >= list[i].first_rev) {
> +                               uc_fw->path = list[i].blob->path;
> +                               uc_fw->major_ver_wanted = list[i].blob->major;
> +                               uc_fw->minor_ver_wanted = list[i].blob->minor;
> +                               break;
> +                       }
> +               }
> +       }
> +}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx