Re: [PATCH v3 2/6] drm/i915/huc: Unified css_header struct for GuC and HuC

["Vivi, Rodrigo" <rodrigo.vivi@intel.com>]

On Wed, 2016-07-06 at 15:24 +0100, Peter Antoine wrote:
> HuC firmware css header has almost exactly same definition as GuC
> firmware except for the sw_version. Also, add a new member fw_type
> into intel_uc_fw to indicate what kind of fw it is. So, the loader
> will pull right sw_version from header.
> 
> v2: rebased on-top of drm-intel-nightly
> v3: rebased on-top of drm-intel-nightly (again).
> 
> Signed-off-by: Alex Dai <yu.dai@intel.com>
> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc.h        |  4 ++++
>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 16 ++++++++++---
>  drivers/gpu/drm/i915/intel_guc_loader.c | 40 ++++++++++++++++++++++-
> ----------
>  3 files changed, 44 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.h
> b/drivers/gpu/drm/i915/intel_guc.h
> index 02adcfc..ebf9c8d 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -97,6 +97,9 @@ enum intel_uc_fw_status {
>  	UC_FIRMWARE_SUCCESS
>  };
>  
> +#define UC_FW_TYPE_GUC		0
> +#define UC_FW_TYPE_HUC		1
> +

couldn't this be an enum?

>  /*
>   * This structure encapsulates all the data needed during the
> process
>   * of fetching, caching, and loading the firmware image into the
> GuC.
> @@ -114,6 +117,7 @@ struct intel_uc_fw {
>  	uint16_t major_ver_found;
>  	uint16_t minor_ver_found;
>  
> +	uint32_t fw_type;
>  	uint32_t header_size;
>  	uint32_t header_offset;
>  	uint32_t rsa_size;
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 944786d..a69ee36 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -154,7 +154,7 @@
>   * The GuC firmware layout looks like this:
>   *
>   *     +-------------------------------+
> - *     |        guc_css_header         |
> + *     |         uc_css_header         |
>   *     | contains major/minor version  |
>   *     +-------------------------------+
>   *     |             uCode             |
> @@ -180,9 +180,16 @@
>   * 3. Length info of each component can be found in header, in
> dwords.
>   * 4. Modulus and exponent key are not required by driver. They may
> not appear
>   * 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.
>   */
>  
> -struct guc_css_header {
> +struct uc_css_header {
>  	uint32_t module_type;
>  	/* header_size includes all non-uCode bits, including
> css_header, rsa
>  	 * key, modulus key and exponent data. */
> @@ -213,7 +220,10 @@ struct guc_css_header {
>  
>  	char username[8];
>  	char buildnumber[12];
> -	uint32_t device_id;
> +	union {
> +		uint32_t device_id;
> +		uint32_t huc_sw_version;
> +	};
>  	uint32_t guc_sw_version;
>  	uint32_t prod_preprod_fw;
>  	uint32_t reserved[12];
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 1afa49b..42b6509 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -553,7 +553,7 @@ void intel_uc_fw_fetch(struct drm_device *dev,
> struct intel_uc_fw *uc_fw)
>  {
>  	struct drm_i915_gem_object *obj;
>  	const struct firmware *fw;
> -	struct guc_css_header *css;
> +	struct uc_css_header *css;
>  	size_t size;
>  	int err;
>  
> @@ -570,19 +570,19 @@ void intel_uc_fw_fetch(struct drm_device *dev,
> struct intel_uc_fw *uc_fw)
>  		uc_fw->uc_fw_path, fw);
>  
>  	/* Check the size of the blob before examining buffer
> contents */
> -	if (fw->size < sizeof(struct guc_css_header)) {
> +	if (fw->size < sizeof(struct uc_css_header)) {
>  		DRM_ERROR("Firmware header is missing\n");
>  		goto fail;
>  	}
>  
> -	css = (struct guc_css_header *)fw->data;
> +	css = (struct uc_css_header *)fw->data;
>  
>  	/* Firmware bits always start from header */
>  	uc_fw->header_offset = 0;
>  	uc_fw->header_size = (css->header_size_dw - css-
> >modulus_size_dw -
>  		css->key_size_dw - css->exponent_size_dw) *
> sizeof(u32);
>  
> -	if (uc_fw->header_size != sizeof(struct guc_css_header)) {
> +	if (uc_fw->header_size != sizeof(struct uc_css_header)) {
>  		DRM_ERROR("CSS header definition mismatch\n");
>  		goto fail;
>  	}
> @@ -606,21 +606,35 @@ void intel_uc_fw_fetch(struct drm_device *dev,
> struct intel_uc_fw *uc_fw)
>  		goto fail;
>  	}
>  
> -	/* Header and uCode will be loaded to WOPCM. Size of the
> two. */
> -	size = uc_fw->header_size + uc_fw->ucode_size;
> -	if (size > guc_wopcm_size(to_i915(dev))) {
> -		DRM_ERROR("Firmware is too large to fit in
> WOPCM\n");
> -		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).
>  	 */
> -	uc_fw->major_ver_found = css->guc_sw_version >> 16;
> -	uc_fw->minor_ver_found = css->guc_sw_version & 0xFFFF;
> +	switch (uc_fw->fw_type) {
> +	case UC_FW_TYPE_GUC:
> +		/* Header and uCode will be loaded to WOPCM. Size of
> the two. */
> +		size = uc_fw->header_size + uc_fw->ucode_size;
> +
> +		/* Top 32k of WOPCM is reserved (8K stack + 24k RC6
> context). */
> +		if (size > guc_wopcm_size(to_i915(dev))) {
> +			DRM_ERROR("Firmware is too large to fit in
> WOPCM\n");
> +			goto fail;
> +		}
> +
> +		uc_fw->major_ver_found = css->guc_sw_version >> 16;
> +		uc_fw->minor_ver_found = css->guc_sw_version &
> 0xFFFF;
> +		break;
> +	case UC_FW_TYPE_HUC:
> +		uc_fw->major_ver_found = css->huc_sw_version >> 16;
> +		uc_fw->minor_ver_found = css->huc_sw_version &
> 0xFFFF;
> +		break;
> +	default:
> +		DRM_ERROR("Unknown firmware type %d\n", uc_fw-
> >fw_type);
> +		err = -ENOEXEC;
> +		goto fail;
> +	}
>  
>  	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
>  	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx