| Message ID | 20190523082420.10352-3-lucas.demarchi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/10] drm/i915/dmc: use kernel types | expand |
On Thu, May 23, 2019 at 01:24:13AM -0700, Lucas De Marchi wrote: > The only meaninful change is that it supports up to 32 fw_info entries > rather than the previous max=20. > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/intel_csr.c | 35 ++++++++++++++++++++++---------- > 1 file changed, 24 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > index 01ae356e69cc..b9651fbe4c25 100644 > --- a/drivers/gpu/drm/i915/intel_csr.c > +++ b/drivers/gpu/drm/i915/intel_csr.c > @@ -71,6 +71,7 @@ MODULE_FIRMWARE(BXT_CSR_PATH); > > #define CSR_DEFAULT_FW_OFFSET 0xFFFFFFFF > #define PACKAGE_MAX_FW_INFO_ENTRIES 20 > +#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32 Looking to spec I could only find "} FWInfo[20];" Where did you get the 32? > > struct intel_css_header { > /* 0x09 for DMC */ > @@ -133,7 +134,7 @@ struct intel_package_header { > /* DMC container header length in dwords */ > u8 header_len; > > - /* always value would be 0x01 */ > + /* 0x01, 0x02 */ > u8 header_ver; > > u8 reserved[10]; > @@ -339,7 +340,7 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv, > struct intel_dmc_header *dmc_header; > struct intel_csr *csr = &dev_priv->csr; > const struct stepping_info *si = intel_get_stepping_info(dev_priv); > - u32 dmc_offset, num_entries, readcount = 0, nbytes; > + u32 dmc_offset, num_entries, max_entries, readcount = 0, nbytes; > u32 i; > u32 *dmc_payload; > > @@ -374,18 +375,30 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv, > /* Extract Package Header information*/ > package_header = (struct intel_package_header *) > &fw->data[readcount]; > - if (sizeof(struct intel_package_header) != > - (package_header->header_len * 4)) { > + > + readcount += sizeof(struct intel_package_header); > + > + if (package_header->header_ver == 1) { > + max_entries = PACKAGE_MAX_FW_INFO_ENTRIES; > + } else if (package_header->header_ver == 2) { > + max_entries = PACKAGE_V2_MAX_FW_INFO_ENTRIES; > + } else { > + DRM_ERROR("DMC firmware has unknown header version %u\n", > + package_header->header_ver); > + return NULL; > + } > + > + if (package_header->header_len * 4 != > + sizeof(struct intel_package_header) + > + max_entries * sizeof(struct intel_fw_info)) { > DRM_ERROR("DMC firmware has wrong package header length " > - "(%u bytes)\n", > - (package_header->header_len * 4)); > + "(%u bytes)\n", package_header->header_len * 4); > return NULL; > } > > - readcount += sizeof(struct intel_package_header); > num_entries = package_header->num_entries; > - if (WARN_ON(package_header->num_entries > PACKAGE_MAX_FW_INFO_ENTRIES)) > - num_entries = PACKAGE_MAX_FW_INFO_ENTRIES; > + if (WARN_ON(package_header->num_entries > max_entries)) > + num_entries = max_entries; > > dmc_offset = find_dmc_fw_offset((struct intel_fw_info *) > &fw->data[readcount], num_entries, si); > @@ -394,9 +407,9 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv, > si->stepping); > return NULL; > } > - /* we always have space for PACKAGE_MAX_FW_INFO_ENTRIES */ > - readcount += PACKAGE_MAX_FW_INFO_ENTRIES * sizeof(struct intel_fw_info); > > + /* we always have space for max_entries, even if not all are used */ > + readcount += max_entries * sizeof(struct intel_fw_info); > /* Convert dmc_offset into number of bytes. By default it is in dwords*/ > dmc_offset *= 4; > readcount += dmc_offset; > -- > 2.21.0 >
>-----Original Message----- >From: De Marchi, Lucas >Sent: Thursday, May 23, 2019 1:24 AM >To: intel-gfx@lists.freedesktop.org >Cc: Jani Nikula <jani.nikula@linux.intel.com>; Srivatsa, Anusha ><anusha.srivatsa@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; De >Marchi, Lucas <lucas.demarchi@intel.com> >Subject: [PATCH 03/10] drm/i915/dmc: add support for package_header with >version 2 > >The only meaninful change is that it supports up to 32 fw_info entries rather than >the previous max=20. > >Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> Confirmed with the spec. Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com> >--- > drivers/gpu/drm/i915/intel_csr.c | 35 ++++++++++++++++++++++---------- > 1 file changed, 24 insertions(+), 11 deletions(-) > >diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c >index 01ae356e69cc..b9651fbe4c25 100644 >--- a/drivers/gpu/drm/i915/intel_csr.c >+++ b/drivers/gpu/drm/i915/intel_csr.c >@@ -71,6 +71,7 @@ MODULE_FIRMWARE(BXT_CSR_PATH); > > #define CSR_DEFAULT_FW_OFFSET 0xFFFFFFFF > #define PACKAGE_MAX_FW_INFO_ENTRIES 20 >+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32 > > struct intel_css_header { > /* 0x09 for DMC */ >@@ -133,7 +134,7 @@ struct intel_package_header { > /* DMC container header length in dwords */ > u8 header_len; > >- /* always value would be 0x01 */ >+ /* 0x01, 0x02 */ > u8 header_ver; > > u8 reserved[10]; >@@ -339,7 +340,7 @@ static u32 *parse_csr_fw(struct drm_i915_private >*dev_priv, > struct intel_dmc_header *dmc_header; > struct intel_csr *csr = &dev_priv->csr; > const struct stepping_info *si = intel_get_stepping_info(dev_priv); >- u32 dmc_offset, num_entries, readcount = 0, nbytes; >+ u32 dmc_offset, num_entries, max_entries, readcount = 0, nbytes; > u32 i; > u32 *dmc_payload; > >@@ -374,18 +375,30 @@ static u32 *parse_csr_fw(struct drm_i915_private >*dev_priv, > /* Extract Package Header information*/ > package_header = (struct intel_package_header *) > &fw->data[readcount]; >- if (sizeof(struct intel_package_header) != >- (package_header->header_len * 4)) { >+ >+ readcount += sizeof(struct intel_package_header); >+ >+ if (package_header->header_ver == 1) { >+ max_entries = PACKAGE_MAX_FW_INFO_ENTRIES; >+ } else if (package_header->header_ver == 2) { >+ max_entries = PACKAGE_V2_MAX_FW_INFO_ENTRIES; >+ } else { >+ DRM_ERROR("DMC firmware has unknown header version >%u\n", >+ package_header->header_ver); >+ return NULL; >+ } >+ >+ if (package_header->header_len * 4 != >+ sizeof(struct intel_package_header) + >+ max_entries * sizeof(struct intel_fw_info)) { > DRM_ERROR("DMC firmware has wrong package header length " >- "(%u bytes)\n", >- (package_header->header_len * 4)); >+ "(%u bytes)\n", package_header->header_len * 4); > return NULL; > } > >- readcount += sizeof(struct intel_package_header); > num_entries = package_header->num_entries; >- if (WARN_ON(package_header->num_entries > >PACKAGE_MAX_FW_INFO_ENTRIES)) >- num_entries = PACKAGE_MAX_FW_INFO_ENTRIES; >+ if (WARN_ON(package_header->num_entries > max_entries)) >+ num_entries = max_entries; > > dmc_offset = find_dmc_fw_offset((struct intel_fw_info *) > &fw->data[readcount], num_entries, si); >@@ -394,9 +407,9 @@ static u32 *parse_csr_fw(struct drm_i915_private >*dev_priv, > si->stepping); > return NULL; > } >- /* we always have space for PACKAGE_MAX_FW_INFO_ENTRIES */ >- readcount += PACKAGE_MAX_FW_INFO_ENTRIES * sizeof(struct >intel_fw_info); > >+ /* we always have space for max_entries, even if not all are used */ >+ readcount += max_entries * sizeof(struct intel_fw_info); > /* Convert dmc_offset into number of bytes. By default it is in dwords*/ > dmc_offset *= 4; > readcount += dmc_offset; >-- >2.21.0
On Thu, May 23, 2019 at 10:43:39AM -0700, Rodrigo Vivi wrote: >On Thu, May 23, 2019 at 01:24:13AM -0700, Lucas De Marchi wrote: >> The only meaninful change is that it supports up to 32 fw_info entries >> rather than the previous max=20. >> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >> --- >> drivers/gpu/drm/i915/intel_csr.c | 35 ++++++++++++++++++++++---------- >> 1 file changed, 24 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c >> index 01ae356e69cc..b9651fbe4c25 100644 >> --- a/drivers/gpu/drm/i915/intel_csr.c >> +++ b/drivers/gpu/drm/i915/intel_csr.c >> @@ -71,6 +71,7 @@ MODULE_FIRMWARE(BXT_CSR_PATH); >> >> #define CSR_DEFAULT_FW_OFFSET 0xFFFFFFFF >> #define PACKAGE_MAX_FW_INFO_ENTRIES 20 >> +#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32 > >Looking to spec I could only find >"} FWInfo[20];" > >Where did you get the 32? you are looking to the struct for header_ver == 1. Look to the one with header_ver == 2. Lucas De Marchi
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index 01ae356e69cc..b9651fbe4c25 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -71,6 +71,7 @@ MODULE_FIRMWARE(BXT_CSR_PATH); #define CSR_DEFAULT_FW_OFFSET 0xFFFFFFFF #define PACKAGE_MAX_FW_INFO_ENTRIES 20 +#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32 struct intel_css_header { /* 0x09 for DMC */ @@ -133,7 +134,7 @@ struct intel_package_header { /* DMC container header length in dwords */ u8 header_len; - /* always value would be 0x01 */ + /* 0x01, 0x02 */ u8 header_ver; u8 reserved[10]; @@ -339,7 +340,7 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv, struct intel_dmc_header *dmc_header; struct intel_csr *csr = &dev_priv->csr; const struct stepping_info *si = intel_get_stepping_info(dev_priv); - u32 dmc_offset, num_entries, readcount = 0, nbytes; + u32 dmc_offset, num_entries, max_entries, readcount = 0, nbytes; u32 i; u32 *dmc_payload; @@ -374,18 +375,30 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv, /* Extract Package Header information*/ package_header = (struct intel_package_header *) &fw->data[readcount]; - if (sizeof(struct intel_package_header) != - (package_header->header_len * 4)) { + + readcount += sizeof(struct intel_package_header); + + if (package_header->header_ver == 1) { + max_entries = PACKAGE_MAX_FW_INFO_ENTRIES; + } else if (package_header->header_ver == 2) { + max_entries = PACKAGE_V2_MAX_FW_INFO_ENTRIES; + } else { + DRM_ERROR("DMC firmware has unknown header version %u\n", + package_header->header_ver); + return NULL; + } + + if (package_header->header_len * 4 != + sizeof(struct intel_package_header) + + max_entries * sizeof(struct intel_fw_info)) { DRM_ERROR("DMC firmware has wrong package header length " - "(%u bytes)\n", - (package_header->header_len * 4)); + "(%u bytes)\n", package_header->header_len * 4); return NULL; } - readcount += sizeof(struct intel_package_header); num_entries = package_header->num_entries; - if (WARN_ON(package_header->num_entries > PACKAGE_MAX_FW_INFO_ENTRIES)) - num_entries = PACKAGE_MAX_FW_INFO_ENTRIES; + if (WARN_ON(package_header->num_entries > max_entries)) + num_entries = max_entries; dmc_offset = find_dmc_fw_offset((struct intel_fw_info *) &fw->data[readcount], num_entries, si); @@ -394,9 +407,9 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv, si->stepping); return NULL; } - /* we always have space for PACKAGE_MAX_FW_INFO_ENTRIES */ - readcount += PACKAGE_MAX_FW_INFO_ENTRIES * sizeof(struct intel_fw_info); + /* we always have space for max_entries, even if not all are used */ + readcount += max_entries * sizeof(struct intel_fw_info); /* Convert dmc_offset into number of bytes. By default it is in dwords*/ dmc_offset *= 4; readcount += dmc_offset;
The only meaninful change is that it supports up to 32 fw_info entries rather than the previous max=20. Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/intel_csr.c | 35 ++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 11 deletions(-)