[Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Anusha Srivatsa]

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. Rename device_id to guc_branch_client_version,
make guc_sw_version a union. . Put UC_FW_TYPE_GUC
and UC_FW_TYPE_HUC into an enum.
v4: rebased on top of drm-tip.Update dev to dev_priv in
intel_uc_fw_fetch.
v5: rebased. Add INTEL_ prefix to an enum. Add fw_type declaration
from patch 1.Combine two different unions for huc and guc version,
reserved etc into one union with two structs.
v6: rebased. Change fw_type to enum.
v7: rebased. Rename the enum fw_type to intel_uc_fw_type.

Cc: Michal Wajdeczko 
Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
Reviewed-by: Arkadiusz Hiler 
Reviewed-by: Michal Wajdeczko 
---
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 23 ++
 drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
 drivers/gpu/drm/i915/intel_uc.h |  6 +
 3 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 3202b32..ed1ab40 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -145,7 +145,7 @@
  * The GuC firmware layout looks like this:
  *
  * +---+
- * |guc_css_header |
+ * | uc_css_header |
  * |   |
  * | contains major/minor version  |
  * +---+
@@ -172,9 +172,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. */
@@ -205,8 +212,16 @@ struct guc_css_header {
 
char username[8];
char buildnumber[12];
-   uint32_t device_id;
-   uint32_t guc_sw_version;
+   union {
+   struct {
+   uint32_t branch_client_version;
+   uint32_t sw_version;
+   } guc;
+   struct {
+   uint32_t sw_version;
+   uint32_t reserved;
+   } huc;
+   };
uint32_t prod_preprod_fw;
uint32_t reserved[12];
uint32_t header_info;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 5a2c86f..30801ee 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -595,7 +595,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
struct pci_dev *pdev = dev_priv->drm.pdev;
struct drm_i915_gem_object *obj;
const struct firmware *fw = NULL;
-   struct guc_css_header *css;
+   struct uc_css_header *css;
size_t size;
int err;
 
@@ -612,19 +612,19 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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_NOTE("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_NOTE("CSS header definition mismatch\n");
goto fail;
}
@@ -648,21 +648,36 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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(dev_priv)) {
-   DRM_NOTE("Firmware is too large to fit in WOPCM\n");
-   goto fail

[Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Anusha Srivatsa]

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. Rename device_id to guc_branch_client_version,
make guc_sw_version a union. . Put UC_FW_TYPE_GUC
and UC_FW_TYPE_HUC into an enum.
v4: rebased on top of drm-tip.Update dev to dev_priv in
intel_uc_fw_fetch.
v5: rebased. Add INTEL_ prefix to an enum. Add fw_type declaration
from patch 1.Combine two different unions for huc and guc version,
reserved etc into one union with two structs.
v6: rebased. Change fw_type to enum.
v7: rebased. Rename the enum fw_type to intel_uc_fw_type.

Cc: Michal Wajdeczko 
Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
Reviewed-by: Arkadiusz Hiler 
Reviewed-by: Michal Wajdeczko 
---
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 23 ++
 drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
 drivers/gpu/drm/i915/intel_uc.h |  6 +
 3 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 3202b32..ed1ab40 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -145,7 +145,7 @@
  * The GuC firmware layout looks like this:
  *
  * +---+
- * |guc_css_header |
+ * | uc_css_header |
  * |   |
  * | contains major/minor version  |
  * +---+
@@ -172,9 +172,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. */
@@ -205,8 +212,16 @@ struct guc_css_header {
 
char username[8];
char buildnumber[12];
-   uint32_t device_id;
-   uint32_t guc_sw_version;
+   union {
+   struct {
+   uint32_t branch_client_version;
+   uint32_t sw_version;
+   } guc;
+   struct {
+   uint32_t sw_version;
+   uint32_t reserved;
+   } huc;
+   };
uint32_t prod_preprod_fw;
uint32_t reserved[12];
uint32_t header_info;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 5a2c86f..30801ee 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -595,7 +595,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
struct pci_dev *pdev = dev_priv->drm.pdev;
struct drm_i915_gem_object *obj;
const struct firmware *fw = NULL;
-   struct guc_css_header *css;
+   struct uc_css_header *css;
size_t size;
int err;
 
@@ -612,19 +612,19 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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_NOTE("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_NOTE("CSS header definition mismatch\n");
goto fail;
}
@@ -648,21 +648,36 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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(dev_priv)) {
-   DRM_NOTE("Firmware is too large to fit in WOPCM\n");
-   goto fail

[Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Anusha Srivatsa]

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. Rename device_id to guc_branch_client_version,
make guc_sw_version a union. . Put UC_FW_TYPE_GUC
and UC_FW_TYPE_HUC into an enum.
v4: rebased on top of drm-tip.Update dev to dev_priv in
intel_uc_fw_fetch.
v5: rebased. Add INTEL_ prefix to an enum. Add fw_type declaration
from patch 1.Combine two different unions for huc and guc version,
reserved etc into one union with two structs.
v6: rebased. Change fw_type to enum.
v7: rebased. Rename the enum fw_type to intel_uc_fw_type.

Cc: Michal Wajdeczko 
Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
Reviewed-by: Arkadiusz Hiler 
Reviewed-by: Michal Wajdeczko 
---
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 23 ++
 drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
 drivers/gpu/drm/i915/intel_uc.h |  6 +
 3 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 3202b32..ed1ab40 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -145,7 +145,7 @@
  * The GuC firmware layout looks like this:
  *
  * +---+
- * |guc_css_header |
+ * | uc_css_header |
  * |   |
  * | contains major/minor version  |
  * +---+
@@ -172,9 +172,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. */
@@ -205,8 +212,16 @@ struct guc_css_header {
 
char username[8];
char buildnumber[12];
-   uint32_t device_id;
-   uint32_t guc_sw_version;
+   union {
+   struct {
+   uint32_t branch_client_version;
+   uint32_t sw_version;
+   } guc;
+   struct {
+   uint32_t sw_version;
+   uint32_t reserved;
+   } huc;
+   };
uint32_t prod_preprod_fw;
uint32_t reserved[12];
uint32_t header_info;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 9071575..8ca2f62 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -594,7 +594,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
struct pci_dev *pdev = dev_priv->drm.pdev;
struct drm_i915_gem_object *obj;
const struct firmware *fw = NULL;
-   struct guc_css_header *css;
+   struct uc_css_header *css;
size_t size;
int err;
 
@@ -611,19 +611,19 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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_NOTE("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_NOTE("CSS header definition mismatch\n");
goto fail;
}
@@ -647,21 +647,36 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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(dev_priv)) {
-   DRM_NOTE("Firmware is too large to fit in WOPCM\n");
-   goto fail

Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Arkadiusz Hiler]

On Wed, Jan 04, 2017 at 06:55:49AM -0800, Anusha Srivatsa wrote:
> From: Peter Antoine 
> 
> 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).
> v4: rebased + spaces.
> v7: rebased.
> v8: rebased.
> v9: rebased. Rename device_id to guc_branch_client_version,
> make guc_sw_version a union. . Put UC_FW_TYPE_GUC
> and UC_FW_TYPE_HUC into an enum.
> v10: rebased.
> v11: rebased.
> v12: rebased on top of drm-tip.
> v13: rebased.Update dev to dev_priv in intel_uc_fw_fetch
> v14: rebased. Add INTEL_ prefix to an enum. Add fw_type declaration
> from patch 1.Combine two different unions for huc and guc version,
> reserved etc into one union with two structs.
> v15: rebased.
> v16: rebased. Change fw_type to enum.
> 
> Tested-by: Xiang Haihao 
> Signed-off-by: Anusha Srivatsa 
> Signed-off-by: Alex Dai 
> Signed-off-by: Peter Antoine 
> ---
>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 23 ++
>  drivers/gpu/drm/i915/intel_guc_loader.c | 41 
> ++---
>  drivers/gpu/drm/i915/intel_uc.h |  6 +
>  3 files changed, 53 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 3202b32..ed1ab40 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -145,7 +145,7 @@
>   * The GuC firmware layout looks like this:
>   *
>   * +---+
> - * |guc_css_header |
> + * | uc_css_header |
>   * |   |
>   * | contains major/minor version  |
>   * +---+
> @@ -172,9 +172,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. */
> @@ -205,8 +212,16 @@ struct guc_css_header {
>  
>   char username[8];
>   char buildnumber[12];
> - uint32_t device_id;
> - uint32_t guc_sw_version;
> + union {
> + struct {
> + uint32_t branch_client_version;
> + uint32_t sw_version;
> + } guc;
> + struct {
> + uint32_t sw_version;
> + uint32_t reserved;
> + } huc;
> + };
>   uint32_t prod_preprod_fw;
>   uint32_t reserved[12];
>   uint32_t header_info;
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index a55cfa7..68af7ae 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
>   struct pci_dev *pdev = dev_priv->drm.pdev;
>   struct drm_i915_gem_object *obj;
>   const struct firmware *fw = NULL;
> - struct guc_css_header *css;
> + struct uc_css_header *css;
>   size_t size;
>   int err;
>  
> @@ -610,19 +610,19 @@ void intel_uc_fw_fetch(struct drm_i915_private 
> *dev_priv,
>   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_NOTE("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_NOTE("CSS header definition mismatch\n");
>   goto fail;
>   }
> @@ -646,21 +646,36 @@ void intel_uc_fw_fetch(struct drm_i915_private 
> *dev_priv,
>   goto fail;
>   

[Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Anusha Srivatsa]

From: Peter Antoine 

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).
v4: rebased + spaces.
v7: rebased.
v8: rebased.
v9: rebased. Rename device_id to guc_branch_client_version,
make guc_sw_version a union. . Put UC_FW_TYPE_GUC
and UC_FW_TYPE_HUC into an enum.
v10: rebased.
v11: rebased.
v12: rebased on top of drm-tip.
v13: rebased.Update dev to dev_priv in intel_uc_fw_fetch
v14: rebased. Add INTEL_ prefix to an enum. Add fw_type declaration
from patch 1.Combine two different unions for huc and guc version,
reserved etc into one union with two structs.
v15: rebased.
v16: rebased. Change fw_type to enum.

Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
---
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 23 ++
 drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
 drivers/gpu/drm/i915/intel_uc.h |  6 +
 3 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 3202b32..ed1ab40 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -145,7 +145,7 @@
  * The GuC firmware layout looks like this:
  *
  * +---+
- * |guc_css_header |
+ * | uc_css_header |
  * |   |
  * | contains major/minor version  |
  * +---+
@@ -172,9 +172,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. */
@@ -205,8 +212,16 @@ struct guc_css_header {
 
char username[8];
char buildnumber[12];
-   uint32_t device_id;
-   uint32_t guc_sw_version;
+   union {
+   struct {
+   uint32_t branch_client_version;
+   uint32_t sw_version;
+   } guc;
+   struct {
+   uint32_t sw_version;
+   uint32_t reserved;
+   } huc;
+   };
uint32_t prod_preprod_fw;
uint32_t reserved[12];
uint32_t header_info;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index a55cfa7..68af7ae 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
struct pci_dev *pdev = dev_priv->drm.pdev;
struct drm_i915_gem_object *obj;
const struct firmware *fw = NULL;
-   struct guc_css_header *css;
+   struct uc_css_header *css;
size_t size;
int err;
 
@@ -610,19 +610,19 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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_NOTE("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_NOTE("CSS header definition mismatch\n");
goto fail;
}
@@ -646,21 +646,36 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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(dev_priv)) {
-   DRM_NOTE("Firmware is too large to fit in 

[Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Anusha Srivatsa]

From: Peter Antoine 

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).
v4: rebased + spaces.
v7: rebased.
v8: rebased.
v9: rebased. Rename device_id to guc_branch_client_version,
make guc_sw_version a union. . Put UC_FW_TYPE_GUC
and UC_FW_TYPE_HUC into an enum.
v10: rebased.
v11: rebased.
v12: rebased on top of drm-tip.
v13: rebased.Update dev to dev_priv in intel_uc_fw_fetch
v14: rebased. Add INTEL_ prefix to an enum. Add fw_type declaration
from patch 1.Combine two different unions for huc and guc version,
reserved etc into one union with two structs.
v15: rebased.
v16: rebased. Change fw_type to enum.

Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
---
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 23 ++
 drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
 drivers/gpu/drm/i915/intel_uc.h |  6 +
 3 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 3202b32..ed1ab40 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -145,7 +145,7 @@
  * The GuC firmware layout looks like this:
  *
  * +---+
- * |guc_css_header |
+ * | uc_css_header |
  * |   |
  * | contains major/minor version  |
  * +---+
@@ -172,9 +172,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. */
@@ -205,8 +212,16 @@ struct guc_css_header {
 
char username[8];
char buildnumber[12];
-   uint32_t device_id;
-   uint32_t guc_sw_version;
+   union {
+   struct {
+   uint32_t branch_client_version;
+   uint32_t sw_version;
+   } guc;
+   struct {
+   uint32_t sw_version;
+   uint32_t reserved;
+   } huc;
+   };
uint32_t prod_preprod_fw;
uint32_t reserved[12];
uint32_t header_info;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 15215b7..a4196e2 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
struct pci_dev *pdev = dev_priv->drm.pdev;
struct drm_i915_gem_object *obj;
const struct firmware *fw = NULL;
-   struct guc_css_header *css;
+   struct uc_css_header *css;
size_t size;
int err;
 
@@ -610,19 +610,19 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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_NOTE("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_NOTE("CSS header definition mismatch\n");
goto fail;
}
@@ -646,21 +646,36 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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(dev_priv)) {
-   DRM_NOTE("Firmware is too large to f

Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Srivatsa, Anusha]

>-Original Message-
>From: Hiler, Arkadiusz
>Sent: Friday, December 23, 2016 6:22 AM
>To: Srivatsa, Anusha 
>Cc: intel-gfx@lists.freedesktop.org; Alex Dai ; Peter Antoine
>
>Subject: Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct 
>for
>GuC and HuC
>
>On Thu, Dec 22, 2016 at 03:12:18PM -0800, Anusha Srivatsa wrote:
>> From: Peter Antoine 
>>
>> 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).
>> v4: rebased + spaces.
>> v7: rebased.
>> v8: rebased.
>> v9: rebased. Rename device_id to guc_branch_client_version, make
>> guc_sw_version a union. . Put UC_FW_TYPE_GUC and
>> UC_FW_TYPE_HUC into an enum.
>> v10: rebased.
>> v11: rebased.
>> v12: rebased on top of drm-tip.
>> v13: rebased.Update dev to dev_priv in intel_uc_fw_fetch
>> v14: rebased. Add INTEL_ prefix to an enum. Add fw_type declaration
>> from patch 1.Combine two different unions for huc and guc version,
>> reserved etc into one union with two structs.
>> v15: rebased.
>>
>> Tested-by: Xiang Haihao 
>> Signed-off-by: Anusha Srivatsa 
>> Signed-off-by: Alex Dai 
>> Signed-off-by: Peter Antoine 
>> ---
>>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 23 ++
>>  drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
>
>>  drivers/gpu/drm/i915/intel_uc.h |  6 +
>>  3 files changed, 53 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
>> b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> index 3202b32..ed1ab40 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
>> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> @@ -145,7 +145,7 @@
>>   * The GuC firmware layout looks like this:
>>   *
>>   * +---+
>> - * |guc_css_header |
>> + * | uc_css_header |
>>   * |   |
>>   * | contains major/minor version  |
>>   * +---+
>> @@ -172,9 +172,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. */ @@ -205,8 +212,16 @@
>> struct guc_css_header {
>>
>>  char username[8];
>>  char buildnumber[12];
>> -uint32_t device_id;
>> -uint32_t guc_sw_version;
>> +union {
>> +struct {
>> +uint32_t branch_client_version;
>> +uint32_t sw_version;
>> +} guc;
>> +struct {
>> +uint32_t sw_version;
>> +uint32_t reserved;
>> +} huc;
>> +};
>>  uint32_t prod_preprod_fw;
>>  uint32_t reserved[12];
>>  uint32_t header_info;
>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
>> b/drivers/gpu/drm/i915/intel_guc_loader.c
>> index ffe53dd7..06e3e5c 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> @@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_i915_private
>*dev_priv,
>>  struct pci_dev *pdev = dev_priv->drm.pdev;
>>  struct drm_i915_gem_object *obj;
>>  const struct firmware *fw = NULL;
>> -struct guc_css_header *css;
>> +struct uc_css_header *css;
>>  size_t size;
>>  int err;
>>
>> @@ -610,19 +610,19 @@ void intel_uc_fw_fetch(struct drm_i915_private
>*dev_priv,
>>  uc_fw-

Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Arkadiusz Hiler]

On Thu, Dec 22, 2016 at 03:12:18PM -0800, Anusha Srivatsa wrote:
> From: Peter Antoine 
> 
> 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).
> v4: rebased + spaces.
> v7: rebased.
> v8: rebased.
> v9: rebased. Rename device_id to guc_branch_client_version,
> make guc_sw_version a union. . Put UC_FW_TYPE_GUC
> and UC_FW_TYPE_HUC into an enum.
> v10: rebased.
> v11: rebased.
> v12: rebased on top of drm-tip.
> v13: rebased.Update dev to dev_priv in intel_uc_fw_fetch
> v14: rebased. Add INTEL_ prefix to an enum. Add fw_type declaration
> from patch 1.Combine two different unions for huc and guc version,
> reserved etc into one union with two structs.
> v15: rebased.
> 
> Tested-by: Xiang Haihao 
> Signed-off-by: Anusha Srivatsa 
> Signed-off-by: Alex Dai 
> Signed-off-by: Peter Antoine 
> ---
>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 23 ++
>  drivers/gpu/drm/i915/intel_guc_loader.c | 41 
> ++---
>  drivers/gpu/drm/i915/intel_uc.h |  6 +
>  3 files changed, 53 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 3202b32..ed1ab40 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -145,7 +145,7 @@
>   * The GuC firmware layout looks like this:
>   *
>   * +---+
> - * |guc_css_header |
> + * | uc_css_header |
>   * |   |
>   * | contains major/minor version  |
>   * +---+
> @@ -172,9 +172,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. */
> @@ -205,8 +212,16 @@ struct guc_css_header {
>  
>   char username[8];
>   char buildnumber[12];
> - uint32_t device_id;
> - uint32_t guc_sw_version;
> + union {
> + struct {
> + uint32_t branch_client_version;
> + uint32_t sw_version;
> + } guc;
> + struct {
> + uint32_t sw_version;
> + uint32_t reserved;
> + } huc;
> + };
>   uint32_t prod_preprod_fw;
>   uint32_t reserved[12];
>   uint32_t header_info;
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index ffe53dd7..06e3e5c 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
>   struct pci_dev *pdev = dev_priv->drm.pdev;
>   struct drm_i915_gem_object *obj;
>   const struct firmware *fw = NULL;
> - struct guc_css_header *css;
> + struct uc_css_header *css;
>   size_t size;
>   int err;
>  
> @@ -610,19 +610,19 @@ void intel_uc_fw_fetch(struct drm_i915_private 
> *dev_priv,
>   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_NOTE("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_NOTE("CSS header definition mismatch\n");
>   goto fail;
>   }
> @@ -646,21 +646,36 @@ void intel_uc_fw_fetch(struct drm_i915_private 
> *dev_priv,
>   goto fail;
>   }
>  
> - /* Header and u

[Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Anusha Srivatsa]

From: Peter Antoine 

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).
v4: rebased + spaces.
v7: rebased.
v8: rebased.
v9: rebased. Rename device_id to guc_branch_client_version,
make guc_sw_version a union. . Put UC_FW_TYPE_GUC
and UC_FW_TYPE_HUC into an enum.
v10: rebased.
v11: rebased.
v12: rebased on top of drm-tip.
v13: rebased.Update dev to dev_priv in intel_uc_fw_fetch
v14: rebased. Add INTEL_ prefix to an enum. Add fw_type declaration
from patch 1.Combine two different unions for huc and guc version,
reserved etc into one union with two structs.
v15: rebased.

Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
---
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 23 ++
 drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
 drivers/gpu/drm/i915/intel_uc.h |  6 +
 3 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 3202b32..ed1ab40 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -145,7 +145,7 @@
  * The GuC firmware layout looks like this:
  *
  * +---+
- * |guc_css_header |
+ * | uc_css_header |
  * |   |
  * | contains major/minor version  |
  * +---+
@@ -172,9 +172,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. */
@@ -205,8 +212,16 @@ struct guc_css_header {
 
char username[8];
char buildnumber[12];
-   uint32_t device_id;
-   uint32_t guc_sw_version;
+   union {
+   struct {
+   uint32_t branch_client_version;
+   uint32_t sw_version;
+   } guc;
+   struct {
+   uint32_t sw_version;
+   uint32_t reserved;
+   } huc;
+   };
uint32_t prod_preprod_fw;
uint32_t reserved[12];
uint32_t header_info;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index ffe53dd7..06e3e5c 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
struct pci_dev *pdev = dev_priv->drm.pdev;
struct drm_i915_gem_object *obj;
const struct firmware *fw = NULL;
-   struct guc_css_header *css;
+   struct uc_css_header *css;
size_t size;
int err;
 
@@ -610,19 +610,19 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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_NOTE("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_NOTE("CSS header definition mismatch\n");
goto fail;
}
@@ -646,21 +646,36 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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(dev_priv)) {
-   DRM_NOTE("Firmware is too large to fit in WOPCM\n");
-   goto

[Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[anushasr]

From: Peter Antoine 

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).
v4: rebased + spaces.
v7: rebased.
v8: rebased.
v9: rebased. Rename device_id to guc_branch_client_version,
make guc_sw_version a union. . Put UC_FW_TYPE_GUC
and UC_FW_TYPE_HUC into an enum.
v10: rebased.
v11: rebased.
v12: rebased on top of drm-tip.
v13: rebased.Update dev to dev_priv in intel_uc_fw_fetch
v14: rebased. Add INTEL_ prefix to an enum. Add fw_type declaration
from patch 1.Combine two different unions for huc and guc version,
reserved etc into one union with two structs.

Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
---
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 23 ++
 drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
 drivers/gpu/drm/i915/intel_uc.h |  6 +
 3 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 3202b32..ed1ab40 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -145,7 +145,7 @@
  * The GuC firmware layout looks like this:
  *
  * +---+
- * |guc_css_header |
+ * | uc_css_header |
  * |   |
  * | contains major/minor version  |
  * +---+
@@ -172,9 +172,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. */
@@ -205,8 +212,16 @@ struct guc_css_header {
 
char username[8];
char buildnumber[12];
-   uint32_t device_id;
-   uint32_t guc_sw_version;
+   union {
+   struct {
+   uint32_t branch_client_version;
+   uint32_t sw_version;
+   } guc;
+   struct {
+   uint32_t sw_version;
+   uint32_t reserved;
+   } huc;
+   };
uint32_t prod_preprod_fw;
uint32_t reserved[12];
uint32_t header_info;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 9b3dbea..09e463b 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
struct pci_dev *pdev = dev_priv->drm.pdev;
struct drm_i915_gem_object *obj;
const struct firmware *fw = NULL;
-   struct guc_css_header *css;
+   struct uc_css_header *css;
size_t size;
int err;
 
@@ -610,19 +610,19 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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_NOTE("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_NOTE("CSS header definition mismatch\n");
goto fail;
}
@@ -646,21 +646,36 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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(dev_priv)) {
-   DRM_NOTE("Firmware is too large to fit in WOPCM\n");
-   goto fail;
-   

Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Arkadiusz Hiler]

On Fri, Dec 09, 2016 at 09:42:06PM +, Srivatsa, Anusha wrote:
> 
> 
> >-Original Message-
> >From: Michal Wajdeczko [mailto:michal.wajdec...@linux.intel.com]
> >Sent: Friday, December 9, 2016 3:56 AM
> >To: Srivatsa, Anusha 
> >Cc: intel-gfx@lists.freedesktop.org; Alex Dai ; Peter 
> >Antoine
> >
> >Subject: Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct 
> >for
> >GuC and HuC
> >
> >On Thu, Dec 08, 2016 at 03:02:13PM -0800, anushasr wrote:
> >> From: Peter Antoine 
> >>
> >> 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).
> >> v4: rebased + spaces.
> >> v7: rebased.
> >> v8: rebased.
> >> v9: rebased. Rename device_id to guc_branch_client_version, make
> >> guc_sw_version a union. . Put UC_FW_TYPE_GUC and
> >> UC_FW_TYPE_HUC into an enum.
> >> v10: rebased.
> >> v11: rebased.
> >> v12: rebased on top of drm-tip.
> >> v13: rebased.Update dev to dev_priv in intel_uc_fw_fetch
> >>
> >> Tested-by: Xiang Haihao 
> >> Signed-off-by: Anusha Srivatsa 
> >> Signed-off-by: Alex Dai 
> >> Signed-off-by: Peter Antoine 
> >> ---
> >>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 21 +
> >>  drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
> >
> >>  drivers/gpu/drm/i915/intel_uc.h |  5 
> >>  3 files changed, 50 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
> >> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> >> index 3202b32..c1e7faf 100644
> >> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> >> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> >> @@ -145,7 +145,7 @@
> >>   * The GuC firmware layout looks like this:
> >>   *
> >>   * +---+
> >> - * |guc_css_header |
> >> + * | uc_css_header |
> >>   * |   |
> >>   * | contains major/minor version  |
> >>   * +---+
> >> @@ -172,9 +172,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 {
> >
> >Hmm, I'm wondering why we don't use "intel_uc_" prefix for structs defined 
> >here.
> >It seems that only enums are defined with intel_ prefix.
> If we rename it to intel_uc_css_header, wont it be confused with 
> intel_css_header in intel_csr.c? Unles we want to combine them
> 
> >Also, it looks that this struct definition is very similar to the 
> >intel_css_header
> >defined in intel_csr.c. Are there any plans to unify them all ?
> >
> No idea about any plans regarding unifying the struct in intel_csr.c and this 
> struct. Arek, any idea?

Currently not, but that seems like idea worth exploring. I'll look into
it this week and if it turns sensible to do, I'll add it to my GuC
cleanup series.

> >>uint32_t module_type;
> >
> >What values are used here? Are they the same as used in fw_type?

module_type is just a part of HuC/GuC's firmware header (css). You can
read it from the blobs:

module_type for guc == 0006  00a1  (v6, v4)
module_type for huc == 0006  00a1  (v1)

It's kind of magic value to determine it's a firmware (I am not aware of
other values than the one above, but that might change in the future).
We do not check for that exact field though. We do not use 

Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Srivatsa, Anusha]

>-Original Message-
>From: Michal Wajdeczko [mailto:michal.wajdec...@linux.intel.com]
>Sent: Friday, December 9, 2016 3:56 AM
>To: Srivatsa, Anusha 
>Cc: intel-gfx@lists.freedesktop.org; Alex Dai ; Peter Antoine
>
>Subject: Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct 
>for
>GuC and HuC
>
>On Thu, Dec 08, 2016 at 03:02:13PM -0800, anushasr wrote:
>> From: Peter Antoine 
>>
>> 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).
>> v4: rebased + spaces.
>> v7: rebased.
>> v8: rebased.
>> v9: rebased. Rename device_id to guc_branch_client_version, make
>> guc_sw_version a union. . Put UC_FW_TYPE_GUC and
>> UC_FW_TYPE_HUC into an enum.
>> v10: rebased.
>> v11: rebased.
>> v12: rebased on top of drm-tip.
>> v13: rebased.Update dev to dev_priv in intel_uc_fw_fetch
>>
>> Tested-by: Xiang Haihao 
>> Signed-off-by: Anusha Srivatsa 
>> Signed-off-by: Alex Dai 
>> Signed-off-by: Peter Antoine 
>> ---
>>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 21 +
>>  drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
>
>>  drivers/gpu/drm/i915/intel_uc.h |  5 
>>  3 files changed, 50 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
>> b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> index 3202b32..c1e7faf 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
>> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> @@ -145,7 +145,7 @@
>>   * The GuC firmware layout looks like this:
>>   *
>>   * +---+
>> - * |guc_css_header |
>> + * | uc_css_header |
>>   * |   |
>>   * | contains major/minor version  |
>>   * +---+
>> @@ -172,9 +172,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 {
>
>Hmm, I'm wondering why we don't use "intel_uc_" prefix for structs defined 
>here.
>It seems that only enums are defined with intel_ prefix.
If we rename it to intel_uc_css_header, wont it be confused with 
intel_css_header in intel_csr.c? Unles we want to combine them

>Also, it looks that this struct definition is very similar to the 
>intel_css_header
>defined in intel_csr.c. Are there any plans to unify them all ?
>
No idea about any plans regarding unifying the struct in intel_csr.c and this 
struct. Arek, any idea?
>>  uint32_t module_type;
>
>What values are used here? Are they the same as used in fw_type?
>
>
>>  /* header_size includes all non-uCode bits, including css_header, rsa
>>   * key, modulus key and exponent data. */ @@ -205,8 +212,14 @@
>> struct guc_css_header {
>>
>>  char username[8];
>>  char buildnumber[12];
>> -uint32_t device_id;
>> -uint32_t guc_sw_version;
>> +union {
>> +uint32_t guc_branch_client_version;
>> +uint32_t huc_sw_version;
>> +};
>> +union {
>> +uint32_t guc_sw_version;
>> +uint32_t huc_reserved;
>> +};
>
>Maybe to make this a little easier to read we can use:
>
>union {
>  struct {
>uint32_t branch_client_version;
>uint32_t sw_version;
>  } guc;
>  struct {
>uint32_t sw_version;
>unit32_t reserved;
>  } huc;
>};
Yes. Will do.
>
>>  uint32_t prod_preprod_fw;
>>  uint32_t reserved[12];
>>  uint32_t header_info;
>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
>> b/drivers/gpu/drm/i915/intel_gu

Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Michal Wajdeczko]

On Thu, Dec 08, 2016 at 03:02:13PM -0800, anushasr wrote:
> From: Peter Antoine 
> 
> 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).
> v4: rebased + spaces.
> v7: rebased.
> v8: rebased.
> v9: rebased. Rename device_id to guc_branch_client_version,
> make guc_sw_version a union. . Put UC_FW_TYPE_GUC
> and UC_FW_TYPE_HUC into an enum.
> v10: rebased.
> v11: rebased.
> v12: rebased on top of drm-tip.
> v13: rebased.Update dev to dev_priv in intel_uc_fw_fetch
> 
> Tested-by: Xiang Haihao 
> Signed-off-by: Anusha Srivatsa 
> Signed-off-by: Alex Dai 
> Signed-off-by: Peter Antoine 
> ---
>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 21 +
>  drivers/gpu/drm/i915/intel_guc_loader.c | 41 
> ++---
>  drivers/gpu/drm/i915/intel_uc.h |  5 
>  3 files changed, 50 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 3202b32..c1e7faf 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -145,7 +145,7 @@
>   * The GuC firmware layout looks like this:
>   *
>   * +---+
> - * |guc_css_header |
> + * | uc_css_header |
>   * |   |
>   * | contains major/minor version  |
>   * +---+
> @@ -172,9 +172,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 {

Hmm, I'm wondering why we don't use "intel_uc_" prefix for structs defined here.
It seems that only enums are defined with intel_ prefix.

Also, it looks that this struct definition is very similar to the 
intel_css_header
defined in intel_csr.c. Are there any plans to unify them all ?


>   uint32_t module_type;

What values are used here? Are they the same as used in fw_type?


>   /* header_size includes all non-uCode bits, including css_header, rsa
>* key, modulus key and exponent data. */
> @@ -205,8 +212,14 @@ struct guc_css_header {
>  
>   char username[8];
>   char buildnumber[12];
> - uint32_t device_id;
> - uint32_t guc_sw_version;
> + union {
> + uint32_t guc_branch_client_version;
> + uint32_t huc_sw_version;
> + };
> + union {
> + uint32_t guc_sw_version;
> + uint32_t huc_reserved;
> + };

Maybe to make this a little easier to read we can use:

union {
  struct {
uint32_t branch_client_version;
uint32_t sw_version;
  } guc;
  struct {
uint32_t sw_version;
unit32_t reserved;
  } huc;
};


>   uint32_t prod_preprod_fw;
>   uint32_t reserved[12];
>   uint32_t header_info;
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 8f04f6e..26a184f 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
>   struct pci_dev *pdev = dev_priv->drm.pdev;
>   struct drm_i915_gem_object *obj;
>   const struct firmware *fw = NULL;
> - struct guc_css_header *css;
> + struct uc_css_header *css;
>   size_t size;
>   int err;
>  
> @@ -610,19 +610,19 @@ void intel_uc_fw_fetch(struct drm_i915_private 
> *dev_priv,
>   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_NOTE("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->

Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Arkadiusz Hiler]

On Thu, Dec 08, 2016 at 03:02:13PM -0800, anushasr wrote:
> From: Peter Antoine 
> 
> 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).
> v4: rebased + spaces.
> v7: rebased.
> v8: rebased.
> v9: rebased. Rename device_id to guc_branch_client_version,
> make guc_sw_version a union. . Put UC_FW_TYPE_GUC
> and UC_FW_TYPE_HUC into an enum.
> v10: rebased.
> v11: rebased.
> v12: rebased on top of drm-tip.
> v13: rebased.Update dev to dev_priv in intel_uc_fw_fetch
> 
> Tested-by: Xiang Haihao 
> Signed-off-by: Anusha Srivatsa 
> Signed-off-by: Alex Dai 
> Signed-off-by: Peter Antoine 
Reviewed-by: Arkadiusz Hiler 
> ---
>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 21 +
>  drivers/gpu/drm/i915/intel_guc_loader.c | 41 
> ++---
>  drivers/gpu/drm/i915/intel_uc.h |  5 
>  3 files changed, 50 insertions(+), 17 deletions(-)

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[anushasr]

From: Peter Antoine 

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).
v4: rebased + spaces.
v7: rebased.
v8: rebased.
v9: rebased. Rename device_id to guc_branch_client_version,
make guc_sw_version a union. . Put UC_FW_TYPE_GUC
and UC_FW_TYPE_HUC into an enum.
v10: rebased.
v11: rebased.
v12: rebased on top of drm-tip.
v13: rebased.Update dev to dev_priv in intel_uc_fw_fetch

Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
---
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 21 +
 drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
 drivers/gpu/drm/i915/intel_uc.h |  5 
 3 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 3202b32..c1e7faf 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -145,7 +145,7 @@
  * The GuC firmware layout looks like this:
  *
  * +---+
- * |guc_css_header |
+ * | uc_css_header |
  * |   |
  * | contains major/minor version  |
  * +---+
@@ -172,9 +172,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. */
@@ -205,8 +212,14 @@ struct guc_css_header {
 
char username[8];
char buildnumber[12];
-   uint32_t device_id;
-   uint32_t guc_sw_version;
+   union {
+   uint32_t guc_branch_client_version;
+   uint32_t huc_sw_version;
+   };
+   union {
+   uint32_t guc_sw_version;
+   uint32_t huc_reserved;
+   };
uint32_t prod_preprod_fw;
uint32_t reserved[12];
uint32_t header_info;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 8f04f6e..26a184f 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
struct pci_dev *pdev = dev_priv->drm.pdev;
struct drm_i915_gem_object *obj;
const struct firmware *fw = NULL;
-   struct guc_css_header *css;
+   struct uc_css_header *css;
size_t size;
int err;
 
@@ -610,19 +610,19 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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_NOTE("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_NOTE("CSS header definition mismatch\n");
goto fail;
}
@@ -646,21 +646,36 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
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(dev_priv)) {
-   DRM_NOTE("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 ar

Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Srivatsa, Anusha]

>-Original Message-
>From: Hiler, Arkadiusz
>Sent: Thursday, December 1, 2016 4:23 AM
>To: Srivatsa, Anusha 
>Cc: intel-gfx@lists.freedesktop.org; Mcgee, Jeff ;
>Kamble, Sagar A 
>Subject: Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct 
>for
>GuC and HuC
>
>On Wed, Nov 30, 2016 at 03:31:28PM -0800, Anusha Srivatsa wrote:
>> From: Peter Antoine 
>>
>> 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).
>> v4: rebased + spaces.
>> v7: rebased.
>> v8: rebased.
>> v9: rebased. Rename device_id to guc_branch_client_version, make
>> guc_sw_version a union. . Put UC_FW_TYPE_GUC and
>> UC_FW_TYPE_HUC into an enum.
>> v10: rebased.
>> v11: rebased.
>> v12: rebased on top of drm-tip.
>>
>> Tested-by: Xiang Haihao 
>> Signed-off-by: Anusha Srivatsa 
>> Signed-off-by: Alex Dai 
>> Signed-off-by: Peter Antoine 
>> Reviewed-by: Dave Gordon 
>> Reviewed-by: Jeff McGee 
>> ---
>>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 21 +
>>  drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
>
>>  drivers/gpu/drm/i915/intel_uc.h |  5 
>>  3 files changed, 50 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
>> b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> index 00ca0df..c07d9da 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  |
>>   * +---+
>> @@ -181,9 +181,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 {
>
>I think we should either move most of this stuff to intel_uc.{c,h} or rename 
>the
>file to intel_uc_fwif.h.
>
>Anyway, this file contains information on top that this is automatically 
>generated
>and your changes might be lost...
>
>The file was introduced and then *manually edited* *multiple times* by *many
>people*, without any signs of changes lost or file being regenerated.
>
>I think we can, at least, drop the warning. I am in favor of drooping the file
>completely though.
So, basically move all these struct and union declarations to intel_uc.h?
>If something would change, we can assume it would be done by hand anyway.
>
>Jeff, Sagar: any thought on that?
>
>>  uint32_t module_type;
>>  /* header_size includes all non-uCode bits, including css_header, rsa
>>   * key, modulus key and exponent data. */ @@ -214,8 +221,14 @@
>> struct guc_css_header {
>>
>>  char username[8];
>>  char buildnumber[12];
>> -uint32_t device_id;
>> -uint32_t guc_sw_version;
>> +union {
>> +uint32_t guc_branch_client_version;
>> +uint32_t huc_sw_version;
>> +};
>> +union {
>> +uint32_t guc_sw_version;
>> +uint32_t huc_reserved;
>> +};
>>  uint32_t prod_preprod_fw;
>>  uint32_t reserved[12];
>>  uint32_t header_info;
>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
>> b/drivers/gpu/drm/i915/intel_guc_loader.c
>> index e55ec2c..557d4b4 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> @@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct
>

Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Arkadiusz Hiler]

On Wed, Nov 30, 2016 at 03:31:28PM -0800, Anusha Srivatsa wrote:
> From: Peter Antoine 
> 
> 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).
> v4: rebased + spaces.
> v7: rebased.
> v8: rebased.
> v9: rebased. Rename device_id to guc_branch_client_version,
> make guc_sw_version a union. . Put UC_FW_TYPE_GUC
> and UC_FW_TYPE_HUC into an enum.
> v10: rebased.
> v11: rebased.
> v12: rebased on top of drm-tip.
> 
> Tested-by: Xiang Haihao 
> Signed-off-by: Anusha Srivatsa 
> Signed-off-by: Alex Dai 
> Signed-off-by: Peter Antoine 
> Reviewed-by: Dave Gordon 
> Reviewed-by: Jeff McGee 
> ---
>  drivers/gpu/drm/i915/intel_guc_fwif.h   | 21 +
>  drivers/gpu/drm/i915/intel_guc_loader.c | 41 
> ++---
>  drivers/gpu/drm/i915/intel_uc.h |  5 
>  3 files changed, 50 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 00ca0df..c07d9da 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  |
>   * +---+
> @@ -181,9 +181,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 {

I think we should either move most of this stuff to intel_uc.{c,h} or
rename the file to intel_uc_fwif.h.

Anyway, this file contains information on top that this is automatically
generated and your changes might be lost...

The file was introduced and then *manually edited* *multiple times* by
*many people*, without any signs of changes lost or file being
regenerated.

I think we can, at least, drop the warning. I am in favor of drooping
the file completely though.

If something would change, we can assume it would be done by hand anyway.

Jeff, Sagar: any thought on that?

>   uint32_t module_type;
>   /* header_size includes all non-uCode bits, including css_header, rsa
>* key, modulus key and exponent data. */
> @@ -214,8 +221,14 @@ struct guc_css_header {
>  
>   char username[8];
>   char buildnumber[12];
> - uint32_t device_id;
> - uint32_t guc_sw_version;
> + union {
> + uint32_t guc_branch_client_version;
> + uint32_t huc_sw_version;
> + };
> + union {
> + uint32_t guc_sw_version;
> + uint32_t huc_reserved;
> + };
>   uint32_t prod_preprod_fw;
>   uint32_t reserved[12];
>   uint32_t header_info;
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index e55ec2c..557d4b4 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct 
> intel_uc_fw *uc_fw)
>   struct pci_dev *pdev = dev->pdev;
>   struct drm_i915_gem_object *obj;
>   const struct firmware *fw = NULL;
> - struct guc_css_header *css;
> + struct uc_css_header *css;
>   size_t size;
>   int err;
>  
> @@ -610,19 +610,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_NOTE("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

[Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Anusha Srivatsa]

From: Peter Antoine 

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).
v4: rebased + spaces.
v7: rebased.
v8: rebased.
v9: rebased. Rename device_id to guc_branch_client_version,
make guc_sw_version a union. . Put UC_FW_TYPE_GUC
and UC_FW_TYPE_HUC into an enum.
v10: rebased.
v11: rebased.
v12: rebased on top of drm-tip.

Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
Reviewed-by: Dave Gordon 
Reviewed-by: Jeff McGee 
---
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 21 +
 drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
 drivers/gpu/drm/i915/intel_uc.h |  5 
 3 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 00ca0df..c07d9da 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  |
  * +---+
@@ -181,9 +181,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. */
@@ -214,8 +221,14 @@ struct guc_css_header {
 
char username[8];
char buildnumber[12];
-   uint32_t device_id;
-   uint32_t guc_sw_version;
+   union {
+   uint32_t guc_branch_client_version;
+   uint32_t huc_sw_version;
+   };
+   union {
+   uint32_t guc_sw_version;
+   uint32_t huc_reserved;
+   };
uint32_t prod_preprod_fw;
uint32_t reserved[12];
uint32_t header_info;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index e55ec2c..557d4b4 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct 
intel_uc_fw *uc_fw)
struct pci_dev *pdev = dev->pdev;
struct drm_i915_gem_object *obj;
const struct firmware *fw = NULL;
-   struct guc_css_header *css;
+   struct uc_css_header *css;
size_t size;
int err;
 
@@ -610,19 +610,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_NOTE("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_NOTE("CSS header definition mismatch\n");
goto fail;
}
@@ -646,21 +646,36 @@ 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_NOTE("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. u1

[Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Anusha Srivatsa]

From: Peter Antoine 

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).
v4: rebased + spaces.
v7: rebased.
v8: rebased.
v9: rebased. Rename device_id to guc_branch_client_version,
make guc_sw_version a union. . Put UC_FW_TYPE_GUC
and UC_FW_TYPE_HUC into an enum.
v10: rebased.
v11: rebased.

Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
Reviewed-by: Dave Gordon 
Reviewed-by: Jeff McGee 
---
 drivers/gpu/drm/i915/intel_guc.h|  6 +
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 21 +
 drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
 3 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 6dc328f..45dfa40 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -98,6 +98,11 @@ enum intel_uc_fw_status {
UC_FIRMWARE_SUCCESS
 };
 
+enum {
+   UC_FW_TYPE_GUC,
+   UC_FW_TYPE_HUC
+};
+
 /*
  * This structure encapsulates all the data needed during the process
  * of fetching, caching, and loading the firmware image into the GuC.
@@ -115,6 +120,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 324ea90..c2a7fdd 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  |
  * +---+
@@ -181,9 +181,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. */
@@ -214,8 +221,14 @@ struct guc_css_header {
 
char username[8];
char buildnumber[12];
-   uint32_t device_id;
-   uint32_t guc_sw_version;
+   union {
+   uint32_t guc_branch_client_version;
+   uint32_t huc_sw_version;
+   };
+   union {
+   uint32_t guc_sw_version;
+   uint32_t huc_reserved;
+   };
uint32_t prod_preprod_fw;
uint32_t reserved[12];
uint32_t header_info;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index c1b53a6..70b372b 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct 
intel_uc_fw *uc_fw)
struct pci_dev *pdev = dev->pdev;
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;
 
@@ -610,19 +610,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_NOTE("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_NOTE("CSS header definition mismatch\n");
 

[Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Anusha Srivatsa]

From: Peter Antoine 

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).
v4: rebased + spaces.
v7: rebased.
v8: rebased.
v9: rebased. Rename device_id to guc_branch_client_version,
make guc_sw_version a union. . Put UC_FW_TYPE_GUC
and UC_FW_TYPE_HUC into an enum.
v10: rebased.

Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
Reviewed-by: Dave Gordon 
Reviewed-by: Jeff McGee 
---
 drivers/gpu/drm/i915/intel_guc.h|  6 +
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 21 +
 drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
 3 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 6dc328f..45dfa40 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -98,6 +98,11 @@ enum intel_uc_fw_status {
UC_FIRMWARE_SUCCESS
 };
 
+enum {
+   UC_FW_TYPE_GUC,
+   UC_FW_TYPE_HUC
+};
+
 /*
  * This structure encapsulates all the data needed during the process
  * of fetching, caching, and loading the firmware image into the GuC.
@@ -115,6 +120,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 324ea90..c2a7fdd 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  |
  * +---+
@@ -181,9 +181,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. */
@@ -214,8 +221,14 @@ struct guc_css_header {
 
char username[8];
char buildnumber[12];
-   uint32_t device_id;
-   uint32_t guc_sw_version;
+   union {
+   uint32_t guc_branch_client_version;
+   uint32_t huc_sw_version;
+   };
+   union {
+   uint32_t guc_sw_version;
+   uint32_t huc_reserved;
+   };
uint32_t prod_preprod_fw;
uint32_t reserved[12];
uint32_t header_info;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 6683a88..ff26d2c 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct 
intel_uc_fw *uc_fw)
struct pci_dev *pdev = dev->pdev;
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;
 
@@ -610,19 +610,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_NOTE("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_NOTE("CSS header definition mismatch\n");
got

[Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Anusha Srivatsa]

From: Peter Antoine 

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).
v4: rebased + spaces.
v7: rebased.
v8: rebased.
v9: rebased. Rename device_id to guc_branch_client_version,
make guc_sw_version a union. . Put UC_FW_TYPE_GUC
and UC_FW_TYPE_HUC into an enum.
v10: rebased.

Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
Reviewed-by: Dave Gordon 
Reviewed-by: Jeff McGee 
---
 drivers/gpu/drm/i915/intel_guc.h|  6 +
 drivers/gpu/drm/i915/intel_guc_fwif.h   | 21 +
 drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
 3 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 6dc328f..45dfa40 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -98,6 +98,11 @@ enum intel_uc_fw_status {
UC_FIRMWARE_SUCCESS
 };
 
+enum {
+   UC_FW_TYPE_GUC,
+   UC_FW_TYPE_HUC
+};
+
 /*
  * This structure encapsulates all the data needed during the process
  * of fetching, caching, and loading the firmware image into the GuC.
@@ -115,6 +120,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 324ea90..c2a7fdd 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  |
  * +---+
@@ -181,9 +181,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. */
@@ -214,8 +221,14 @@ struct guc_css_header {
 
char username[8];
char buildnumber[12];
-   uint32_t device_id;
-   uint32_t guc_sw_version;
+   union {
+   uint32_t guc_branch_client_version;
+   uint32_t huc_sw_version;
+   };
+   union {
+   uint32_t guc_sw_version;
+   uint32_t huc_reserved;
+   };
uint32_t prod_preprod_fw;
uint32_t reserved[12];
uint32_t header_info;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 6683a88..ff26d2c 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct 
intel_uc_fw *uc_fw)
struct pci_dev *pdev = dev->pdev;
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;
 
@@ -610,19 +610,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_NOTE("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_NOTE("CSS header definition mismatch\n");
got

Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Carlos Santa]

On Mon, 2016-10-03 at 11:42 -0700, Anusha Srivatsa wrote:
> From: Peter Antoine 
> 
> 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).
> v4: rebased + spaces.
> v7: rebased.
> v8: rebased.
> 
> Tested-by: Xiang Haihao 
> Signed-off-by: Anusha Srivatsa 
> Signed-off-by: Alex Dai 
> Signed-off-by: Peter Antoine 
> Reviewed-by: Dave Gordon 
> ---
>  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 | 41 ++-
> --
>  3 files changed, 45 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.h
> b/drivers/gpu/drm/i915/intel_guc.h
> index b134a41..812e4ca 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -98,6 +98,9 @@ enum intel_uc_fw_status {
>   UC_FIRMWARE_SUCCESS
>  };
>  
> +#define UC_FW_TYPE_GUC   0
> +#define UC_FW_TYPE_HUC   1
> +

This can be changed to an enum as suggested earlier by Rodrigo.

>  /*
>   * This structure encapsulates all the data needed during the
> process
>   * of fetching, caching, and loading the firmware image into the
> GuC.
> @@ -115,6 +118,7 @@ struct intel_uc_fw {
>   uint16_t major_ver_found;
>   uint16_t minor_ver_found;
> 
> + uint32_t fw_type;

Maybe a comment can be added to this variable to explicitly describe
what it is ("GuC/HuC FW")?

Carlos

>   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 e40db2d..b38b6b4 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  |
>   * +---+
> @@ -181,9 +181,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. */
> @@ -214,7 +221,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 493295d..0b863a1 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -586,7 +586,7 @@ void intel_uc_fw_fetch(struct drm_device *dev,
> struct intel_uc_fw *uc_fw)
>   struct pci_dev *pdev = dev->pdev;
>   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;
>  
> @@ -603,19 +603,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_NOTE("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(struc

Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Jeff McGee]

On Thu, Oct 13, 2016 at 08:45:45AM -0700, Jeff McGee wrote:
> On Mon, Oct 03, 2016 at 11:42:56AM -0700, Anusha Srivatsa wrote:
> > From: Peter Antoine 
> > 
> > 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).
> > v4: rebased + spaces.
> > v7: rebased.
> > v8: rebased.
> > 
> > Tested-by: Xiang Haihao 
> > Signed-off-by: Anusha Srivatsa 
> > Signed-off-by: Alex Dai 
> > Signed-off-by: Peter Antoine 
> > Reviewed-by: Dave Gordon 
> > ---
> >  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 | 41 
> > ++---
> >  3 files changed, 45 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_guc.h 
> > b/drivers/gpu/drm/i915/intel_guc.h
> > index b134a41..812e4ca 100644
> > --- a/drivers/gpu/drm/i915/intel_guc.h
> > +++ b/drivers/gpu/drm/i915/intel_guc.h
> > @@ -98,6 +98,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.
> > @@ -115,6 +118,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 e40db2d..b38b6b4 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  |
> >   * +---+
> > @@ -181,9 +181,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. */
> > @@ -214,7 +221,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];
> 
> I propose renaming the device_id field in this union to
> 'guc_client_branch_version'. GuC uses this position to store a client
> version and branch version. I'm not sure where the 'device_id' term came
> from. We don't reference this currently but may need to in the future
> so might as well name it properly. At the very least we should probably
> make it guc_device_id, to help indicate that it applies to guc fw only.
> 
> In that same vein, can we make guc_sw_version into a union as below to
> reinforce the difference to huc?
> 
>   union {
>   uint32_t guc_sw_version;
>   uint32_t huc_reserved;
>   };
> 
> Jeff
> 

One correction. The branch version is considered the "major" and the
client version the "minor", so the better name for this field is
'guc_branch_client_version'.

> > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
> > b/drivers/gpu/drm/i915/intel_guc_loader.c
> > index 493295d..0b863a1 100644
> > --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> > @@ -586,7 +586,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct 
> > intel_uc_fw *uc_fw)
> > struct pci_dev *pdev = dev->pdev;
> > 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;
> >  

Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Jeff McGee]

On Mon, Oct 03, 2016 at 11:42:56AM -0700, Anusha Srivatsa wrote:
> From: Peter Antoine 
> 
> 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).
> v4: rebased + spaces.
> v7: rebased.
> v8: rebased.
> 
> Tested-by: Xiang Haihao 
> Signed-off-by: Anusha Srivatsa 
> Signed-off-by: Alex Dai 
> Signed-off-by: Peter Antoine 
> Reviewed-by: Dave Gordon 
> ---
>  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 | 41 
> ++---
>  3 files changed, 45 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.h 
> b/drivers/gpu/drm/i915/intel_guc.h
> index b134a41..812e4ca 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -98,6 +98,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.
> @@ -115,6 +118,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 e40db2d..b38b6b4 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  |
>   * +---+
> @@ -181,9 +181,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. */
> @@ -214,7 +221,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];

I propose renaming the device_id field in this union to
'guc_client_branch_version'. GuC uses this position to store a client
version and branch version. I'm not sure where the 'device_id' term came
from. We don't reference this currently but may need to in the future
so might as well name it properly. At the very least we should probably
make it guc_device_id, to help indicate that it applies to guc fw only.

In that same vein, can we make guc_sw_version into a union as below to
reinforce the difference to huc?

union {
uint32_t guc_sw_version;
uint32_t huc_reserved;
};

Jeff

> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 493295d..0b863a1 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -586,7 +586,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct 
> intel_uc_fw *uc_fw)
>   struct pci_dev *pdev = dev->pdev;
>   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;
>  
> @@ -603,19 +603,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_NOTE("Firmware header is missing\n");
>   goto fail;
> 

[Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Anusha Srivatsa]

From: Peter Antoine 

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).
v4: rebased + spaces.
v7: rebased.
v8: rebased.

Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
Reviewed-by: Dave Gordon 
---
 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 | 41 ++---
 3 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index b134a41..812e4ca 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -98,6 +98,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.
@@ -115,6 +118,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 e40db2d..b38b6b4 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  |
  * +---+
@@ -181,9 +181,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. */
@@ -214,7 +221,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 493295d..0b863a1 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -586,7 +586,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct 
intel_uc_fw *uc_fw)
struct pci_dev *pdev = dev->pdev;
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;
 
@@ -603,19 +603,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_NOTE("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_NOTE("CSS header definition mismatch\n");
goto fail;
}
@@ -639,21 +639,36 @@ 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))) {

[Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC

[Anusha Srivatsa]

From: Peter Antoine 

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).
v4: rebased + spaces.
v7: rebased.
v8: rebased.

Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
Reviewed-by: Dave Gordon 
---
 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 | 41 ++---
 3 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index b134a41..812e4ca 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -98,6 +98,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.
@@ -115,6 +118,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 e40db2d..b38b6b4 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  |
  * +---+
@@ -181,9 +181,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. */
@@ -214,7 +221,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 493295d..0b863a1 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -586,7 +586,7 @@ void intel_uc_fw_fetch(struct drm_device *dev, struct 
intel_uc_fw *uc_fw)
struct pci_dev *pdev = dev->pdev;
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;
 
@@ -603,19 +603,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_NOTE("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_NOTE("CSS header definition mismatch\n");
goto fail;
}
@@ -639,21 +639,36 @@ 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))) {