[Intel-gfx] [PATCH v2 05/22] drm/i915/guc: Update GuC firmware CSS header

[Daniele Ceraolo Spurio]

On 4/11/19 1:44 AM, Michal Wajdeczko wrote:
> There are few minor changes in the CSS header related to the version
> numbering in new GuC firmwares. Update our definition and start using
> common tools for extracting bitfields.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: John Spotswood <john.a.spotswood at intel.com>
> Cc: Jeff Mcgee <jeff.mcgee at intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc_fwif.h | 23 ++++++++---------------
>   drivers/gpu/drm/i915/intel_uc_fw.c    | 20 ++++++++++----------
>   2 files changed, 18 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index b2f5148f4f17..1cb4fad2d539 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -168,11 +168,7 @@
>    *    in fw. So driver will load a truncated firmware in this case.
>    *
>    * HuC firmware layout is same as GuC firmware.
> - *
> - * HuC firmware css header is different. However, the only difference is where
> - * the version information is saved. The uc_css_header is unified to support
> - * both. Driver should get HuC version from uc_css_header.huc_sw_version, while
> - * uc_css_header.guc_sw_version for GuC.
> + * Only HuC version information is saved in a different way.
>    */
>   
>   struct uc_css_header {
> @@ -206,16 +202,13 @@ struct uc_css_header {
>   
>   	char username[8];
>   	char buildnumber[12];
> -	union {
> -		struct {
> -			u32 branch_client_version;
> -			u32 sw_version;
> -	} guc;
> -		struct {
> -			u32 sw_version;
> -			u32 reserved;
> -	} huc;
> -	};
> +	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)
> +	u32 sw_reserved;
>   	u32 prod_preprod_fw;
>   	u32 reserved[12];
>   	u32 header_info;

This doesn't match guc FW headers. Over there I see

	char username[8];
	char buildnumber[12];
	u32 sw_version;
	u32 reserved[13];
	u32 private_data_size;
	u32 header_info;

While HuC headers (hoping I got the right ones since it was my first 
time looking for the HuC CSS) have:

	char username[8];
	char buildnumber[12];
	u32 sw_version;
	u32 reserved[14];
	u32 header_info;

Patch LGTM apart from this.
Daniele

> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
> index becf05ebae4d..957c1feb30d3 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.c
> @@ -22,6 +22,7 @@
>    *
>    */
>   
> +#include <linux/bitfield.h>
>   #include <linux/firmware.h>
>   #include <drm/drm_print.h>
>   
> @@ -119,21 +120,20 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
>   		goto fail;
>   	}
>   
> -	/*
> -	 * The GuC firmware image has the version number embedded at a
> -	 * well-known offset within the firmware blob; note that major / minor
> -	 * version are TWO bytes each (i.e. u16), although all pointers and
> -	 * offsets are defined in terms of bytes (u8).
> -	 */
> +	/* Get version numbers from the CSS header */
>   	switch (uc_fw->type) {
>   	case INTEL_UC_FW_TYPE_GUC:
> -		uc_fw->major_ver_found = css->guc.sw_version >> 16;
> -		uc_fw->minor_ver_found = css->guc.sw_version & 0xFFFF;
> +		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 = css->huc.sw_version >> 16;
> -		uc_fw->minor_ver_found = css->huc.sw_version & 0xFFFF;
> +		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:
>