diff mbox

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

Message ID 1467815071-35665-2-git-send-email-peter.antoine@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Antoine July 6, 2016, 2:24 p.m. UTC
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(-)

Comments

Rodrigo Vivi July 6, 2016, 8:41 p.m. UTC | #1
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) {
Peter Antoine July 7, 2016, 10:09 a.m. UTC | #2
On Wed, 6 Jul 2016, Vivi, Rodrigo wrote:

> 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
>> +
Yes.
I'll change it.

Peter.

>
> 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) {

--
    Peter Antoine (Android Graphics Driver Software Engineer)
    ---------------------------------------------------------------------
    Intel Corporation (UK) Limited
    Registered No. 1134945 (England)
    Registered Office: Pipers Way, Swindon SN3 1RJ
    VAT No: 860 2173 47
diff mbox

Patch

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
+
 /*
  * 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) {