On Wed, 2019-09-25 at 15:21 -0700, Daniele Ceraolo Spurio wrote:
> The HuC FW has silently switched to encoding the version the same way
> as
> the GuC FW does, i.e. major.minor.patch instead of just major.minor.
> All
> the current blobs follow the new scheme, but since minor and patch
> are
> both zero there is no difference in the end results and we happily
> load
> them. New binaries, however, will have non-zero values in there, so
> we
> need to make sure to parse them correctly.
> 
> Signed-off-by: Daniele Ceraolo Spurio <
> daniele.ceraolospurio@intel.com>

I don't have insight into the HuC change, so just taking your word
here. The code below looks sane and is an obvious improvement.

It might be interesting to get a look from someone a little closer to
this for a HuC perspective. With that disclaimer:
Reviewed-by: Stuart Summers <stuart.summers@intel.com>

> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c     | 23 ++++------------
> ----
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h |  8 +++----
>  2 files changed, 7 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index ea9a807abd4f..bb878119f06c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -339,25 +339,10 @@ int intel_uc_fw_fetch(struct intel_uc_fw
> *uc_fw, struct drm_i915_private *i915)
>  	}
>  
>  	/* Get version numbers from the CSS header */
> -	switch (uc_fw->type) {
> -	case INTEL_UC_FW_TYPE_GUC:
> -		uc_fw->major_ver_found =
> FIELD_GET(CSS_SW_VERSION_GUC_MAJOR,
> -						   css->sw_version);
> -		uc_fw->minor_ver_found =
> FIELD_GET(CSS_SW_VERSION_GUC_MINOR,
> -						   css->sw_version);
> -		break;
> -
> -	case INTEL_UC_FW_TYPE_HUC:
> -		uc_fw->major_ver_found =
> FIELD_GET(CSS_SW_VERSION_HUC_MAJOR,
> -						   css->sw_version);
> -		uc_fw->minor_ver_found =
> FIELD_GET(CSS_SW_VERSION_HUC_MINOR,
> -						   css->sw_version);
> -		break;
> -
> -	default:
> -		MISSING_CASE(uc_fw->type);
> -		break;
> -	}
> +	uc_fw->major_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MAJOR,
> +					   css->sw_version);
> +	uc_fw->minor_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MINOR,
> +					   css->sw_version);
>  
>  	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
>  	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> index ae58e8a8c53b..f8f6c91a0df6 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> @@ -69,11 +69,9 @@ struct uc_css_header {
>  	char username[8];
>  	char buildnumber[12];
>  	u32 sw_version;
> -#define CSS_SW_VERSION_GUC_MAJOR	(0xFF << 16)
> -#define CSS_SW_VERSION_GUC_MINOR	(0xFF << 8)
> -#define CSS_SW_VERSION_GUC_PATCH	(0xFF << 0)
> -#define CSS_SW_VERSION_HUC_MAJOR	(0xFFFF << 16)
> -#define CSS_SW_VERSION_HUC_MINOR	(0xFFFF << 0)
> +#define CSS_SW_VERSION_UC_MAJOR		(0xFF << 16)
> +#define CSS_SW_VERSION_UC_MINOR		(0xFF << 8)
> +#define CSS_SW_VERSION_UC_PATCH		(0xFF << 0)
>  	u32 reserved[14];
>  	u32 header_info;
>  } __packed;