On Wed, Nov 30, 2016 at 03:31:28PM -0800, Anusha Srivatsa wrote:
> From: Peter Antoine <peter.anto...@intel.com>
>
> 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. <Jeff Mcgee>. 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 <haihao.xi...@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.sriva...@intel.com>
> Signed-off-by: Alex Dai <yu....@intel.com>
> Signed-off-by: Peter Antoine <peter.anto...@intel.com>
> Reviewed-by: Dave Gordon <david.s.gor...@intel.com>
> Reviewed-by: Jeff McGee <jeff.mc...@intel.com>
> ---
> 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_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. u16), although all pointers and offsets are
> defined
> * in terms of bytes (u8).
> */
> - uc_fw->major_ver_found = css->guc_sw_version >> 16;
> - uc_fw->minor_ver_found = css->guc_sw_version & 0xFFFF;
> + switch (uc_fw->fw_type) {
> + case UC_FW_TYPE_GUC:
> + /* Header and uCode will be loaded to WOPCM. Size of the two. */
> + size = uc_fw->header_size + uc_fw->ucode_size;
> +
> + /* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
> + if (size > guc_wopcm_size(to_i915(dev))) {
> + DRM_ERROR("Firmware is too large to fit in WOPCM\n");
> + goto fail;
> + }
> + uc_fw->major_ver_found = css->guc_sw_version >> 16;
> + uc_fw->minor_ver_found = css->guc_sw_version & 0xFFFF;
> + break;
> +
> + case UC_FW_TYPE_HUC:
> + uc_fw->major_ver_found = css->huc_sw_version >> 16;
> + uc_fw->minor_ver_found = css->huc_sw_version & 0xFFFF;
> + break;
> +
> + default:
> + DRM_ERROR("Unknown firmware type %d\n", uc_fw->fw_type);
> + err = -ENOEXEC;
> + goto fail;
> + }
>
> if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
> uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index a1c771f..1616cac 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.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.
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
