| Message ID | 1482448344-6505-3-git-send-email-anusha.srivatsa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Dec 22, 2016 at 03:12:18PM -0800, Anusha Srivatsa wrote: > From: Peter Antoine <peter.antoine@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. > 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 <haihao.xiang@intel.com> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > Signed-off-by: Alex Dai <yu.dai@intel.com> > Signed-off-by: Peter Antoine <peter.antoine@intel.com> > --- > 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 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 INTEL_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(dev_priv)) { > + 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 INTEL_UC_FW_TYPE_HUC: > + uc_fw->major_ver_found = css->huc.sw_version >> 16; > + uc_fw->minor_ver_found = css->huc.sw_version & 0xFFFF; > + 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 893bcec..ad140e2 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 { > INTEL_UC_FIRMWARE_SUCCESS > }; > > +enum { > + INTEL_UC_FW_TYPE_GUC, > + INTEL_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. > @@ -114,6 +119,7 @@ struct intel_uc_fw { > uint16_t major_ver_found; > uint16_t minor_ver_found; > > + uint32_t fw_type; Any reason why we use uint32_t instead of a named enum type here? > uint32_t header_size; > uint32_t header_offset; > uint32_t rsa_size; > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>-----Original Message----- >From: Hiler, Arkadiusz >Sent: Friday, December 23, 2016 6:22 AM >To: Srivatsa, Anusha <anusha.srivatsa@intel.com> >Cc: intel-gfx@lists.freedesktop.org; Alex Dai <yu.dai@intel.com>; Peter Antoine ><peter.antoine@intel.com> >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 <peter.antoine@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. >> 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 <haihao.xiang@intel.com> >> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> >> Signed-off-by: Alex Dai <yu.dai@intel.com> >> Signed-off-by: Peter Antoine <peter.antoine@intel.com> >> --- >> 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 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 INTEL_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(dev_priv)) { >> + 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 INTEL_UC_FW_TYPE_HUC: >> + uc_fw->major_ver_found = css->huc.sw_version >> 16; >> + uc_fw->minor_ver_found = css->huc.sw_version & 0xFFFF; >> + 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 893bcec..ad140e2 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 { >> INTEL_UC_FIRMWARE_SUCCESS >> }; >> >> +enum { >> + INTEL_UC_FW_TYPE_GUC, >> + INTEL_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. >> @@ -114,6 +119,7 @@ struct intel_uc_fw { >> uint16_t major_ver_found; >> uint16_t minor_ver_found; >> >> + uint32_t fw_type; > >Any reason why we use uint32_t instead of a named enum type here? Maintains the uniformity of the struct intel_uc_fw...... every field is either uint32_t or uint16_t. Anusha >> uint32_t header_size; >> uint32_t header_offset; >> uint32_t rsa_size; >> -- >> 2.7.4 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >Cheers, >Arek
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 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 INTEL_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(dev_priv)) { + 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 INTEL_UC_FW_TYPE_HUC: + uc_fw->major_ver_found = css->huc.sw_version >> 16; + uc_fw->minor_ver_found = css->huc.sw_version & 0xFFFF; + 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 893bcec..ad140e2 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 { INTEL_UC_FIRMWARE_SUCCESS }; +enum { + INTEL_UC_FW_TYPE_GUC, + INTEL_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. @@ -114,6 +119,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;