* [PATCH 0/9] Add support for new DMC headers
@ 2019-06-07 9:12 Lucas De Marchi
2019-06-07 9:12 ` [PATCH 1/9] drm/i915/dmc: use kernel types Lucas De Marchi
` (10 more replies)
0 siblings, 11 replies; 22+ messages in thread
From: Lucas De Marchi @ 2019-06-07 9:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
This is v2 of https://patchwork.freedesktop.org/series/61016/
Last patch as moved as the first and sent separately so it can be
backported. That one is already applied.
Rodrigo and Anusha gave their r-b on all these, but given the rebase, the
patches changed considerably (and the end result is not even the same).
So I didn't add them here, except for the first that is a trivial one.
Lucas De Marchi (9):
drm/i915/dmc: use kernel types
drm/i915/dmc: extract fw_info and table walk from intel_package_header
drm/i915/dmc: add support for package_header with version 2
drm/i915/dmc: extract function to parse css header
drm/i915/dmc: extract function to parse package_header
drm/i915/dmc: extract function to parse dmc_header
drm/i915/dmc: add support to load dmc_header version 3
drm/i915/dmc: remove redundant return in parse_csr_fw()
drm/i915/dmc: protect against loading wrong firmware
drivers/gpu/drm/i915/i915_drv.h | 4 +-
drivers/gpu/drm/i915/intel_csr.c | 407 ++++++++++++++++++++++---------
2 files changed, 290 insertions(+), 121 deletions(-)
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/9] drm/i915/dmc: use kernel types
2019-06-07 9:12 [PATCH 0/9] Add support for new DMC headers Lucas De Marchi
@ 2019-06-07 9:12 ` Lucas De Marchi
2019-06-07 9:12 ` [PATCH 2/9] drm/i915/dmc: extract fw_info and table walk from intel_package_header Lucas De Marchi
` (9 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Lucas De Marchi @ 2019-06-07 9:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
Change all fields in intel_package_header and intel_dmc_header whose
meaning are 1-byte numbers to use u8.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/intel_csr.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index bf0eebd385b9..3d2beecd8d0d 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -130,12 +130,12 @@ struct intel_fw_info {
struct intel_package_header {
/* DMC container header length in dwords */
- unsigned char header_len;
+ u8 header_len;
/* always value would be 0x01 */
- unsigned char header_ver;
+ u8 header_ver;
- unsigned char reserved[10];
+ u8 reserved[10];
/* Number of valid entries in the FWInfo array below */
u32 num_entries;
@@ -148,10 +148,10 @@ struct intel_dmc_header {
u32 signature;
/* DMC binary header length */
- unsigned char header_len;
+ u8 header_len;
/* 0x01 */
- unsigned char header_ver;
+ u8 header_ver;
/* Reserved */
u16 dmcc_ver;
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/9] drm/i915/dmc: extract fw_info and table walk from intel_package_header
2019-06-07 9:12 [PATCH 0/9] Add support for new DMC headers Lucas De Marchi
2019-06-07 9:12 ` [PATCH 1/9] drm/i915/dmc: use kernel types Lucas De Marchi
@ 2019-06-07 9:12 ` Lucas De Marchi
2019-06-10 18:16 ` Srivatsa, Anusha
2019-06-07 9:12 ` [PATCH 3/9] drm/i915/dmc: add support for package_header with version 2 Lucas De Marchi
` (8 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Lucas De Marchi @ 2019-06-07 9:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
Move fw_info out of struct intel_package_header to allow it to grow more
easily in future. To make a cleaner move, let's also extract a function to
search the header for the dmc_offset.
While reviewing this code I wondered why we continued the search even
after finding a suitable firmware. Add a comment to explain we will
continue to try to find a more specific firmware version, even if this
is not required by the spec.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/intel_csr.c | 72 ++++++++++++++++++++++++--------
1 file changed, 55 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 3d2beecd8d0d..99fa4db95e46 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -70,6 +70,7 @@ MODULE_FIRMWARE(SKL_CSR_PATH);
MODULE_FIRMWARE(BXT_CSR_PATH);
#define CSR_DEFAULT_FW_OFFSET 0xFFFFFFFF
+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
struct intel_css_header {
/* 0x09 for DMC */
@@ -139,8 +140,6 @@ struct intel_package_header {
/* Number of valid entries in the FWInfo array below */
u32 num_entries;
-
- struct intel_fw_info fw_info[20];
} __packed;
struct intel_dmc_header {
@@ -292,6 +291,46 @@ void intel_csr_load_program(struct drm_i915_private *dev_priv)
gen9_set_dc_state_debugmask(dev_priv);
}
+/*
+ * Search fw_info table for dmc_offset to find firmware binary: num_entries is
+ * already sanitized.
+ */
+static u32 find_dmc_fw_offset(const struct intel_fw_info *fw_info,
+ unsigned int num_entries,
+ const struct stepping_info *si)
+{
+ u32 dmc_offset = CSR_DEFAULT_FW_OFFSET;
+ unsigned int i;
+
+ for (i = 0; i < num_entries; i++) {
+ if (fw_info[i].substepping == '*' &&
+ si->stepping == fw_info[i].stepping) {
+ dmc_offset = fw_info[i].offset;
+ break;
+ }
+
+ if (si->stepping == fw_info[i].stepping &&
+ si->substepping == fw_info[i].substepping) {
+ dmc_offset = fw_info[i].offset;
+ break;
+ }
+
+ if (fw_info[i].stepping == '*' &&
+ fw_info[i].substepping == '*') {
+ /*
+ * In theory we should stop the search as generic
+ * entries should always come after the more specific
+ * ones, but let's continue to make sure to work even
+ * with "broken" firmwares. If we don't find a more
+ * specific one, then we use this entry
+ */
+ dmc_offset = fw_info[i].offset;
+ }
+ }
+
+ return dmc_offset;
+}
+
static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
const struct firmware *fw)
{
@@ -300,7 +339,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 = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
+ u32 dmc_offset, num_entries, readcount = 0, nbytes;
u32 i;
u32 *dmc_payload;
size_t fsize;
@@ -349,27 +388,26 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
(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;
- /* Search for dmc_offset to find firware binary. */
- for (i = 0; i < package_header->num_entries; i++) {
- if (package_header->fw_info[i].substepping == '*' &&
- si->stepping == package_header->fw_info[i].stepping) {
- dmc_offset = package_header->fw_info[i].offset;
- break;
- } else if (si->stepping == package_header->fw_info[i].stepping &&
- si->substepping == package_header->fw_info[i].substepping) {
- dmc_offset = package_header->fw_info[i].offset;
- break;
- } else if (package_header->fw_info[i].stepping == '*' &&
- package_header->fw_info[i].substepping == '*')
- dmc_offset = package_header->fw_info[i].offset;
- }
+ fsize += PACKAGE_MAX_FW_INFO_ENTRIES * sizeof(struct intel_fw_info);
+ if (fsize > fw->size)
+ goto error_truncated;
+
+ dmc_offset = find_dmc_fw_offset((struct intel_fw_info *)
+ &fw->data[readcount], num_entries, si);
if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
DRM_ERROR("DMC firmware not supported for %c stepping\n",
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);
+
/* Convert dmc_offset into number of bytes. By default it is in dwords*/
dmc_offset *= 4;
readcount += dmc_offset;
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/9] drm/i915/dmc: add support for package_header with version 2
2019-06-07 9:12 [PATCH 0/9] Add support for new DMC headers Lucas De Marchi
2019-06-07 9:12 ` [PATCH 1/9] drm/i915/dmc: use kernel types Lucas De Marchi
2019-06-07 9:12 ` [PATCH 2/9] drm/i915/dmc: extract fw_info and table walk from intel_package_header Lucas De Marchi
@ 2019-06-07 9:12 ` Lucas De Marchi
2019-06-10 18:22 ` Srivatsa, Anusha
2019-06-07 9:12 ` [PATCH 4/9] drm/i915/dmc: extract function to parse css header Lucas De Marchi
` (7 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Lucas De Marchi @ 2019-06-07 9:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
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 | 38 ++++++++++++++++++++++----------
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 99fa4db95e46..ba72c29acbcc 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;
size_t fsize;
@@ -381,20 +382,32 @@ 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;
- fsize += PACKAGE_MAX_FW_INFO_ENTRIES * sizeof(struct intel_fw_info);
+ fsize += max_entries * sizeof(struct intel_fw_info);
if (fsize > fw->size)
goto error_truncated;
@@ -405,8 +418,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;
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/9] drm/i915/dmc: extract function to parse css header
2019-06-07 9:12 [PATCH 0/9] Add support for new DMC headers Lucas De Marchi
` (2 preceding siblings ...)
2019-06-07 9:12 ` [PATCH 3/9] drm/i915/dmc: add support for package_header with version 2 Lucas De Marchi
@ 2019-06-07 9:12 ` Lucas De Marchi
2019-06-14 19:30 ` Srivatsa, Anusha
2019-06-07 9:12 ` [PATCH 5/9] drm/i915/dmc: extract function to parse package_header Lucas De Marchi
` (6 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Lucas De Marchi @ 2019-06-07 9:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
Let's start splitting the parse function, making all of them return the
number of bytes parsed - different versions of the firmware header may
require different sizes for the structures.
v2: rework remaining bytes calculation on new protection for amount of
bytes read
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/intel_csr.c | 66 ++++++++++++++++++++------------
1 file changed, 41 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index ba72c29acbcc..79732075f008 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -332,36 +332,22 @@ static u32 find_dmc_fw_offset(const struct intel_fw_info *fw_info,
return dmc_offset;
}
-static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
- const struct firmware *fw)
+/* Return number of bytes parsed or 0 on error */
+static u32 parse_csr_fw_css(struct intel_csr *csr,
+ struct intel_css_header *css_header,
+ size_t rem_size)
{
- struct intel_css_header *css_header;
- struct intel_package_header *package_header;
- 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, max_entries, readcount = 0, nbytes;
- u32 i;
- u32 *dmc_payload;
- size_t fsize;
-
- if (!fw)
- return NULL;
-
- fsize = sizeof(struct intel_css_header) +
- sizeof(struct intel_package_header) +
- sizeof(struct intel_dmc_header);
- if (fsize > fw->size)
- goto error_truncated;
+ if (rem_size < sizeof(struct intel_css_header)) {
+ DRM_ERROR("Truncated DMC firmware, refusing.\n");
+ return 0;
+ }
- /* Extract CSS Header information*/
- css_header = (struct intel_css_header *)fw->data;
if (sizeof(struct intel_css_header) !=
(css_header->header_len * 4)) {
DRM_ERROR("DMC firmware has wrong CSS header length "
"(%u bytes)\n",
(css_header->header_len * 4));
- return NULL;
+ return 0;
}
if (csr->required_version &&
@@ -372,12 +358,42 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
CSR_VERSION_MINOR(css_header->version),
CSR_VERSION_MAJOR(csr->required_version),
CSR_VERSION_MINOR(csr->required_version));
- return NULL;
+ return 0;
}
csr->version = css_header->version;
- readcount += sizeof(struct intel_css_header);
+ return sizeof(struct intel_css_header);
+}
+
+static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
+ const struct firmware *fw)
+{
+ struct intel_css_header *css_header;
+ struct intel_package_header *package_header;
+ 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, max_entries, readcount = 0, nbytes;
+ u32 i, r;
+ u32 *dmc_payload;
+ size_t fsize;
+
+ if (!fw)
+ return NULL;
+
+ /* Extract CSS Header information */
+ css_header = (struct intel_css_header *)fw->data;
+ r = parse_csr_fw_css(csr, css_header, fw->size);
+ if (!r)
+ return NULL;
+
+ readcount += r;
+ fsize = readcount +
+ sizeof(struct intel_package_header) +
+ sizeof(struct intel_dmc_header);
+ if (fsize > fw->size)
+ goto error_truncated;
/* Extract Package Header information*/
package_header = (struct intel_package_header *)
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 5/9] drm/i915/dmc: extract function to parse package_header
2019-06-07 9:12 [PATCH 0/9] Add support for new DMC headers Lucas De Marchi
` (3 preceding siblings ...)
2019-06-07 9:12 ` [PATCH 4/9] drm/i915/dmc: extract function to parse css header Lucas De Marchi
@ 2019-06-07 9:12 ` Lucas De Marchi
2019-06-14 20:09 ` Srivatsa, Anusha
2019-06-07 9:12 ` [PATCH 6/9] drm/i915/dmc: extract function to parse dmc_header Lucas De Marchi
` (5 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Lucas De Marchi @ 2019-06-07 9:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
Like parse_csr_fw_css() this parses the package_header from firmware and
saves the relevant fields in the csr struct. In this function we also
lookup the fw_info we are interested in.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/intel_csr.c | 117 +++++++++++++++++--------------
1 file changed, 66 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 79732075f008..db5772616a4f 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -332,6 +332,64 @@ static u32 find_dmc_fw_offset(const struct intel_fw_info *fw_info,
return dmc_offset;
}
+static u32
+parse_csr_fw_package(struct intel_csr *csr,
+ const struct intel_package_header *package_header,
+ const struct stepping_info *si,
+ size_t rem_size)
+{
+ u32 package_size = sizeof(struct intel_package_header);
+ u32 num_entries, max_entries, dmc_offset;
+ const struct intel_fw_info *fw_info;
+
+ if (rem_size < package_size)
+ goto error_truncated;
+
+ 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 0;
+ }
+
+ /*
+ * We should always have space for max_entries,
+ * even if not all are used
+ */
+ package_size += max_entries * sizeof(struct intel_fw_info);
+ if (rem_size < package_size)
+ goto error_truncated;
+
+ if (package_header->header_len * 4 != package_size) {
+ DRM_ERROR("DMC firmware has wrong package header length "
+ "(%u bytes)\n", package_size);
+ return 0;
+ }
+
+ num_entries = package_header->num_entries;
+ if (WARN_ON(package_header->num_entries > max_entries))
+ num_entries = max_entries;
+
+ fw_info = (const struct intel_fw_info *)
+ ((u8 *)package_header + sizeof(*package_header));
+ dmc_offset = find_dmc_fw_offset(fw_info, num_entries, si);
+ if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
+ DRM_ERROR("DMC firmware not supported for %c stepping\n",
+ si->stepping);
+ return 0;
+ }
+
+ /* dmc_offset is in dwords */
+ return package_size + dmc_offset * 4;
+
+error_truncated:
+ DRM_ERROR("Truncated DMC firmware, refusing.\n");
+ return 0;
+}
+
/* Return number of bytes parsed or 0 on error */
static u32 parse_csr_fw_css(struct intel_csr *csr,
struct intel_css_header *css_header,
@@ -374,7 +432,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, max_entries, readcount = 0, nbytes;
+ u32 readcount = 0, nbytes;
u32 i, r;
u32 *dmc_payload;
size_t fsize;
@@ -389,59 +447,16 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
return NULL;
readcount += r;
- fsize = readcount +
- sizeof(struct intel_package_header) +
- sizeof(struct intel_dmc_header);
- if (fsize > fw->size)
- goto error_truncated;
-
- /* Extract Package Header information*/
- package_header = (struct intel_package_header *)
- &fw->data[readcount];
-
- 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);
- return NULL;
- }
-
- num_entries = package_header->num_entries;
- if (WARN_ON(package_header->num_entries > max_entries))
- num_entries = max_entries;
-
- fsize += max_entries * sizeof(struct intel_fw_info);
- if (fsize > fw->size)
- goto error_truncated;
- dmc_offset = find_dmc_fw_offset((struct intel_fw_info *)
- &fw->data[readcount], num_entries, si);
- if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
- DRM_ERROR("DMC firmware not supported for %c stepping\n",
- si->stepping);
+ /* Extract Package Header information */
+ package_header = (struct intel_package_header *)&fw->data[readcount];
+ r = parse_csr_fw_package(csr, package_header, si, fw->size - readcount);
+ if (!r)
return NULL;
- }
- /* 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;
- fsize += dmc_offset;
+ readcount += r;
+ fsize = readcount +
+ sizeof(struct intel_dmc_header);
if (fsize > fw->size)
goto error_truncated;
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 6/9] drm/i915/dmc: extract function to parse dmc_header
2019-06-07 9:12 [PATCH 0/9] Add support for new DMC headers Lucas De Marchi
` (4 preceding siblings ...)
2019-06-07 9:12 ` [PATCH 5/9] drm/i915/dmc: extract function to parse package_header Lucas De Marchi
@ 2019-06-07 9:12 ` Lucas De Marchi
2019-06-14 20:24 ` Srivatsa, Anusha
2019-06-07 9:12 ` [PATCH 7/9] drm/i915/dmc: add support to load dmc_header version 3 Lucas De Marchi
` (4 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Lucas De Marchi @ 2019-06-07 9:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
Complete the extraction of functions to parse specific parts of the
firmware. The return of the function parse_csr_fw() is now redundant
since it already sets the dmc_payload field. Changing it is left for
later to avoid noise in the commit.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/intel_csr.c | 130 ++++++++++++++++++-------------
1 file changed, 74 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index db5772616a4f..1fb42fd44519 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -332,6 +332,74 @@ static u32 find_dmc_fw_offset(const struct intel_fw_info *fw_info,
return dmc_offset;
}
+static u32 parse_csr_fw_dmc(struct intel_csr *csr,
+ const struct intel_dmc_header *dmc_header,
+ size_t rem_size)
+{
+ unsigned int i, payload_size;
+ u32 r;
+ u8 *payload;
+
+ if (rem_size < sizeof(struct intel_dmc_header))
+ goto error_truncated;
+
+ if (sizeof(struct intel_dmc_header) != dmc_header->header_len) {
+ DRM_ERROR("DMC firmware has wrong dmc header length "
+ "(%u bytes)\n",
+ (dmc_header->header_len));
+ return 0;
+ }
+
+ /* Cache the dmc header info. */
+ if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) {
+ DRM_ERROR("DMC firmware has wrong mmio count %u\n",
+ dmc_header->mmio_count);
+ return 0;
+ }
+
+ csr->mmio_count = dmc_header->mmio_count;
+ for (i = 0; i < dmc_header->mmio_count; i++) {
+ if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE ||
+ dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
+ DRM_ERROR("DMC firmware has wrong mmio address 0x%x\n",
+ dmc_header->mmioaddr[i]);
+ return 0;
+ }
+ csr->mmioaddr[i] = _MMIO(dmc_header->mmioaddr[i]);
+ csr->mmiodata[i] = dmc_header->mmiodata[i];
+ }
+
+ rem_size -= dmc_header->header_len;
+
+ /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
+ payload_size = dmc_header->fw_size * 4;
+ if (rem_size < payload_size)
+ goto error_truncated;
+
+ if (payload_size > csr->max_fw_size) {
+ DRM_ERROR("DMC FW too big (%u bytes)\n", payload_size);
+ return 0;
+ }
+ csr->dmc_fw_size = dmc_header->fw_size;
+
+ csr->dmc_payload = kmalloc(payload_size, GFP_KERNEL);
+ if (!csr->dmc_payload) {
+ DRM_ERROR("Memory allocation failed for dmc payload\n");
+ return 0;
+ }
+
+ r = sizeof(struct intel_dmc_header);
+ payload = (u8 *)(dmc_header) + r;
+ memcpy(csr->dmc_payload, payload, payload_size);
+ r += payload_size;
+
+ return r;
+
+error_truncated:
+ DRM_ERROR("Truncated DMC firmware, refusing.\n");
+ return 0;
+}
+
static u32
parse_csr_fw_package(struct intel_csr *csr,
const struct intel_package_header *package_header,
@@ -432,10 +500,8 @@ 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 readcount = 0, nbytes;
- u32 i, r;
- u32 *dmc_payload;
- size_t fsize;
+ u32 readcount = 0;
+ u32 r;
if (!fw)
return NULL;
@@ -455,62 +521,14 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
return NULL;
readcount += r;
- fsize = readcount +
- sizeof(struct intel_dmc_header);
- if (fsize > fw->size)
- goto error_truncated;
- /* Extract dmc_header information. */
+ /* Extract dmc_header information */
dmc_header = (struct intel_dmc_header *)&fw->data[readcount];
- if (sizeof(struct intel_dmc_header) != (dmc_header->header_len)) {
- DRM_ERROR("DMC firmware has wrong dmc header length "
- "(%u bytes)\n",
- (dmc_header->header_len));
- return NULL;
- }
- readcount += sizeof(struct intel_dmc_header);
-
- /* Cache the dmc header info. */
- if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) {
- DRM_ERROR("DMC firmware has wrong mmio count %u\n",
- dmc_header->mmio_count);
- return NULL;
- }
- csr->mmio_count = dmc_header->mmio_count;
- for (i = 0; i < dmc_header->mmio_count; i++) {
- if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE ||
- dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
- DRM_ERROR("DMC firmware has wrong mmio address 0x%x\n",
- dmc_header->mmioaddr[i]);
- return NULL;
- }
- csr->mmioaddr[i] = _MMIO(dmc_header->mmioaddr[i]);
- csr->mmiodata[i] = dmc_header->mmiodata[i];
- }
-
- /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
- nbytes = dmc_header->fw_size * 4;
- fsize += nbytes;
- if (fsize > fw->size)
- goto error_truncated;
-
- if (nbytes > csr->max_fw_size) {
- DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
- return NULL;
- }
- csr->dmc_fw_size = dmc_header->fw_size;
-
- dmc_payload = kmalloc(nbytes, GFP_KERNEL);
- if (!dmc_payload) {
- DRM_ERROR("Memory allocation failed for dmc payload\n");
+ r = parse_csr_fw_dmc(csr, dmc_header, fw->size - readcount);
+ if (!r)
return NULL;
- }
- return memcpy(dmc_payload, &fw->data[readcount], nbytes);
-
-error_truncated:
- DRM_ERROR("Truncated DMC firmware, rejecting.\n");
- return NULL;
+ return csr->dmc_payload;
}
static void intel_csr_runtime_pm_get(struct drm_i915_private *dev_priv)
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 7/9] drm/i915/dmc: add support to load dmc_header version 3
2019-06-07 9:12 [PATCH 0/9] Add support for new DMC headers Lucas De Marchi
` (5 preceding siblings ...)
2019-06-07 9:12 ` [PATCH 6/9] drm/i915/dmc: extract function to parse dmc_header Lucas De Marchi
@ 2019-06-07 9:12 ` Lucas De Marchi
2019-06-14 20:03 ` Srivatsa, Anusha
2019-06-07 9:12 ` [PATCH 8/9] drm/i915/dmc: remove redundant return in parse_csr_fw() Lucas De Marchi
` (3 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Lucas De Marchi @ 2019-06-07 9:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
Main difference is that now there are up to 20 MMIOs that can be set and
a lot of noise due to the struct changing the fields in the middle.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 4 +-
drivers/gpu/drm/i915/intel_csr.c | 121 ++++++++++++++++++++++++-------
2 files changed, 95 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dfe4b11ee423..fc0a770fef15 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -352,8 +352,8 @@ struct intel_csr {
u32 dmc_fw_size; /* dwords */
u32 version;
u32 mmio_count;
- i915_reg_t mmioaddr[8];
- u32 mmiodata[8];
+ i915_reg_t mmioaddr[20];
+ u32 mmiodata[20];
u32 dc_state;
u32 allowed_dc_mask;
intel_wakeref_t wakeref;
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 1fb42fd44519..0b1978a2f87d 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -72,6 +72,8 @@ 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
+#define DMC_V1_MAX_MMIO_COUNT 8
+#define DMC_V3_MAX_MMIO_COUNT 20
struct intel_css_header {
/* 0x09 for DMC */
@@ -143,7 +145,7 @@ struct intel_package_header {
u32 num_entries;
} __packed;
-struct intel_dmc_header {
+struct intel_dmc_header_base {
/* always value would be 0x40403E3E */
u32 signature;
@@ -164,22 +166,47 @@ struct intel_dmc_header {
/* Major Minor version */
u32 fw_version;
+} __packed;
+
+struct intel_dmc_header_v1 {
+ struct intel_dmc_header_base base;
/* Number of valid MMIO cycles present. */
u32 mmio_count;
/* MMIO address */
- u32 mmioaddr[8];
+ u32 mmioaddr[DMC_V1_MAX_MMIO_COUNT];
/* MMIO data */
- u32 mmiodata[8];
+ u32 mmiodata[DMC_V1_MAX_MMIO_COUNT];
/* FW filename */
- unsigned char dfile[32];
+ char dfile[32];
u32 reserved1[2];
} __packed;
+struct intel_dmc_header_v3 {
+ struct intel_dmc_header_base base;
+
+ /* DMC RAM start MMIO address */
+ u32 start_mmioaddr;
+
+ u32 reserved[9];
+
+ /* FW filename */
+ char dfile[32];
+
+ /* Number of valid MMIO cycles present. */
+ u32 mmio_count;
+
+ /* MMIO address */
+ u32 mmioaddr[DMC_V3_MAX_MMIO_COUNT];
+
+ /* MMIO data */
+ u32 mmiodata[DMC_V3_MAX_MMIO_COUNT];
+} __packed;
+
struct stepping_info {
char stepping;
char substepping;
@@ -333,43 +360,83 @@ static u32 find_dmc_fw_offset(const struct intel_fw_info *fw_info,
}
static u32 parse_csr_fw_dmc(struct intel_csr *csr,
- const struct intel_dmc_header *dmc_header,
+ const struct intel_dmc_header_base *dmc_header,
size_t rem_size)
{
- unsigned int i, payload_size;
- u32 r;
+ unsigned int header_len_bytes, dmc_header_size, payload_size, i;
+ const u32 *mmioaddr, *mmiodata;
+ u32 mmio_count, mmio_count_max;
u8 *payload;
- if (rem_size < sizeof(struct intel_dmc_header))
+ BUILD_BUG_ON(ARRAY_SIZE(csr->mmioaddr) < DMC_V3_MAX_MMIO_COUNT ||
+ ARRAY_SIZE(csr->mmioaddr) < DMC_V1_MAX_MMIO_COUNT);
+
+ /*
+ * Check if we can access common fields, we will checkc again below
+ * after we have read the version
+ */
+ if (rem_size < sizeof(struct intel_dmc_header_base))
goto error_truncated;
- if (sizeof(struct intel_dmc_header) != dmc_header->header_len) {
+ /* Cope with small differences between v1 and v3 */
+ if (dmc_header->header_ver == 3) {
+ const struct intel_dmc_header_v3 *v3 =
+ (const struct intel_dmc_header_v3 *)dmc_header;
+
+ if (rem_size < sizeof(struct intel_dmc_header_v3))
+ goto error_truncated;
+
+ mmioaddr = v3->mmioaddr;
+ mmiodata = v3->mmiodata;
+ mmio_count = v3->mmio_count;
+ mmio_count_max = DMC_V3_MAX_MMIO_COUNT;
+ /* header_len is in dwords */
+ header_len_bytes = dmc_header->header_len * 4;
+ dmc_header_size = sizeof(*v3);
+ } else if (dmc_header->header_ver == 1) {
+ const struct intel_dmc_header_v1 *v1 =
+ (const struct intel_dmc_header_v1 *)dmc_header;
+
+ if (rem_size < sizeof(struct intel_dmc_header_v1))
+ goto error_truncated;
+
+ mmioaddr = v1->mmioaddr;
+ mmiodata = v1->mmiodata;
+ mmio_count = v1->mmio_count;
+ mmio_count_max = DMC_V1_MAX_MMIO_COUNT;
+ header_len_bytes = dmc_header->header_len;
+ dmc_header_size = sizeof(*v1);
+ } else {
+ DRM_ERROR("Unknown DMC fw header version: %u\n",
+ dmc_header->header_ver);
+ return 0;
+ }
+
+ if (header_len_bytes != dmc_header_size) {
DRM_ERROR("DMC firmware has wrong dmc header length "
- "(%u bytes)\n",
- (dmc_header->header_len));
+ "(%u bytes)\n", header_len_bytes);
return 0;
}
/* Cache the dmc header info. */
- if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) {
- DRM_ERROR("DMC firmware has wrong mmio count %u\n",
- dmc_header->mmio_count);
+ if (mmio_count > mmio_count_max) {
+ DRM_ERROR("DMC firmware has wrong mmio count %u\n", mmio_count);
return 0;
}
- csr->mmio_count = dmc_header->mmio_count;
- for (i = 0; i < dmc_header->mmio_count; i++) {
- if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE ||
- dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < CSR_MMIO_START_RANGE ||
+ mmioaddr[i] > CSR_MMIO_END_RANGE) {
DRM_ERROR("DMC firmware has wrong mmio address 0x%x\n",
- dmc_header->mmioaddr[i]);
+ mmioaddr[i]);
return 0;
}
- csr->mmioaddr[i] = _MMIO(dmc_header->mmioaddr[i]);
- csr->mmiodata[i] = dmc_header->mmiodata[i];
+ csr->mmioaddr[i] = _MMIO(mmioaddr[i]);
+ csr->mmiodata[i] = mmiodata[i];
}
+ csr->mmio_count = mmio_count;
- rem_size -= dmc_header->header_len;
+ rem_size -= header_len_bytes;
/* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
payload_size = dmc_header->fw_size * 4;
@@ -388,12 +455,10 @@ static u32 parse_csr_fw_dmc(struct intel_csr *csr,
return 0;
}
- r = sizeof(struct intel_dmc_header);
- payload = (u8 *)(dmc_header) + r;
+ payload = (u8 *)(dmc_header) + header_len_bytes;
memcpy(csr->dmc_payload, payload, payload_size);
- r += payload_size;
- return r;
+ return header_len_bytes + payload_size;
error_truncated:
DRM_ERROR("Truncated DMC firmware, refusing.\n");
@@ -497,7 +562,7 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
{
struct intel_css_header *css_header;
struct intel_package_header *package_header;
- struct intel_dmc_header *dmc_header;
+ struct intel_dmc_header_base *dmc_header;
struct intel_csr *csr = &dev_priv->csr;
const struct stepping_info *si = intel_get_stepping_info(dev_priv);
u32 readcount = 0;
@@ -523,7 +588,7 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
readcount += r;
/* Extract dmc_header information */
- dmc_header = (struct intel_dmc_header *)&fw->data[readcount];
+ dmc_header = (struct intel_dmc_header_base *)&fw->data[readcount];
r = parse_csr_fw_dmc(csr, dmc_header, fw->size - readcount);
if (!r)
return NULL;
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 8/9] drm/i915/dmc: remove redundant return in parse_csr_fw()
2019-06-07 9:12 [PATCH 0/9] Add support for new DMC headers Lucas De Marchi
` (6 preceding siblings ...)
2019-06-07 9:12 ` [PATCH 7/9] drm/i915/dmc: add support to load dmc_header version 3 Lucas De Marchi
@ 2019-06-07 9:12 ` Lucas De Marchi
2019-06-14 19:51 ` Srivatsa, Anusha
2019-06-07 9:12 ` [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware Lucas De Marchi
` (2 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Lucas De Marchi @ 2019-06-07 9:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
parse_csr_fw() is responsible to set up several fields in struct intel_csr,
including the payload. We don't need to assign it again.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/intel_csr.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 0b1978a2f87d..7608e4e2950d 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -557,7 +557,7 @@ static u32 parse_csr_fw_css(struct intel_csr *csr,
return sizeof(struct intel_css_header);
}
-static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
+static void parse_csr_fw(struct drm_i915_private *dev_priv,
const struct firmware *fw)
{
struct intel_css_header *css_header;
@@ -569,13 +569,13 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
u32 r;
if (!fw)
- return NULL;
+ return;
/* Extract CSS Header information */
css_header = (struct intel_css_header *)fw->data;
r = parse_csr_fw_css(csr, css_header, fw->size);
if (!r)
- return NULL;
+ return;
readcount += r;
@@ -583,17 +583,13 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
package_header = (struct intel_package_header *)&fw->data[readcount];
r = parse_csr_fw_package(csr, package_header, si, fw->size - readcount);
if (!r)
- return NULL;
+ return;
readcount += r;
/* Extract dmc_header information */
dmc_header = (struct intel_dmc_header_base *)&fw->data[readcount];
- r = parse_csr_fw_dmc(csr, dmc_header, fw->size - readcount);
- if (!r)
- return NULL;
-
- return csr->dmc_payload;
+ parse_csr_fw_dmc(csr, dmc_header, fw->size - readcount);
}
static void intel_csr_runtime_pm_get(struct drm_i915_private *dev_priv)
@@ -621,8 +617,7 @@ static void csr_load_work_fn(struct work_struct *work)
csr = &dev_priv->csr;
request_firmware(&fw, dev_priv->csr.fw_path, &dev_priv->drm.pdev->dev);
- if (fw)
- dev_priv->csr.dmc_payload = parse_csr_fw(dev_priv, fw);
+ parse_csr_fw(dev_priv, fw);
if (dev_priv->csr.dmc_payload) {
intel_csr_load_program(dev_priv);
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware
2019-06-07 9:12 [PATCH 0/9] Add support for new DMC headers Lucas De Marchi
` (7 preceding siblings ...)
2019-06-07 9:12 ` [PATCH 8/9] drm/i915/dmc: remove redundant return in parse_csr_fw() Lucas De Marchi
@ 2019-06-07 9:12 ` Lucas De Marchi
2019-06-14 19:44 ` Srivatsa, Anusha
2019-06-07 10:40 ` ✓ Fi.CI.BAT: success for Add support for new DMC headers Patchwork
2019-06-09 17:17 ` ✓ Fi.CI.IGT: " Patchwork
10 siblings, 1 reply; 22+ messages in thread
From: Lucas De Marchi @ 2019-06-07 9:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
In intel_package_header version 2 there's a new field in the
fw_info table that must be 0, otherwise it's not the correct DMC
firmware. Add a check for version 2 or later.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/intel_csr.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 7608e4e2950d..864531aae1a5 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -120,7 +120,10 @@ struct intel_css_header {
} __packed;
struct intel_fw_info {
- u16 reserved1;
+ u8 reserved1;
+
+ /* reserved on package_header version 1, must be 0 on version 2 */
+ u8 dmc_id;
/* Stepping (A, B, C, ..., *). * is a wildcard */
char stepping;
@@ -325,12 +328,16 @@ void intel_csr_load_program(struct drm_i915_private *dev_priv)
*/
static u32 find_dmc_fw_offset(const struct intel_fw_info *fw_info,
unsigned int num_entries,
- const struct stepping_info *si)
+ const struct stepping_info *si,
+ u8 package_ver)
{
u32 dmc_offset = CSR_DEFAULT_FW_OFFSET;
unsigned int i;
for (i = 0; i < num_entries; i++) {
+ if (package_ver > 1 && fw_info[i].dmc_id != 0)
+ continue;
+
if (fw_info[i].substepping == '*' &&
si->stepping == fw_info[i].stepping) {
dmc_offset = fw_info[i].offset;
@@ -508,7 +515,8 @@ parse_csr_fw_package(struct intel_csr *csr,
fw_info = (const struct intel_fw_info *)
((u8 *)package_header + sizeof(*package_header));
- dmc_offset = find_dmc_fw_offset(fw_info, num_entries, si);
+ dmc_offset = find_dmc_fw_offset(fw_info, num_entries, si,
+ package_header->header_ver);
if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
DRM_ERROR("DMC firmware not supported for %c stepping\n",
si->stepping);
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* ✓ Fi.CI.BAT: success for Add support for new DMC headers
2019-06-07 9:12 [PATCH 0/9] Add support for new DMC headers Lucas De Marchi
` (8 preceding siblings ...)
2019-06-07 9:12 ` [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware Lucas De Marchi
@ 2019-06-07 10:40 ` Patchwork
2019-06-09 17:17 ` ✓ Fi.CI.IGT: " Patchwork
10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-06-07 10:40 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: Add support for new DMC headers
URL : https://patchwork.freedesktop.org/series/61761/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6216 -> Patchwork_13201
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/
Known issues
------------
Here are the changes found in Patchwork_13201 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_close_race@basic-threads:
- fi-icl-dsi: [PASS][1] -> [INCOMPLETE][2] ([fdo#107713])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/fi-icl-dsi/igt@gem_close_race@basic-threads.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/fi-icl-dsi/igt@gem_close_race@basic-threads.html
* igt@gem_tiled_pread_basic:
- fi-icl-u3: [PASS][3] -> [DMESG-WARN][4] ([fdo#107724])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/fi-icl-u3/igt@gem_tiled_pread_basic.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/fi-icl-u3/igt@gem_tiled_pread_basic.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-icl-u3: [PASS][5] -> [INCOMPLETE][6] ([fdo#107713])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/fi-icl-u3/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/fi-icl-u3/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
#### Possible fixes ####
* igt@gem_close_race@basic-process:
- fi-icl-u3: [DMESG-WARN][7] ([fdo#107724]) -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/fi-icl-u3/igt@gem_close_race@basic-process.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/fi-icl-u3/igt@gem_close_race@basic-process.html
* igt@gem_ctx_create@basic-files:
- fi-icl-u2: [INCOMPLETE][9] ([fdo#107713] / [fdo#109100]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/fi-icl-u2/igt@gem_ctx_create@basic-files.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/fi-icl-u2/igt@gem_ctx_create@basic-files.html
- fi-icl-y: [INCOMPLETE][11] ([fdo#107713] / [fdo#109100]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/fi-icl-y/igt@gem_ctx_create@basic-files.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/fi-icl-y/igt@gem_ctx_create@basic-files.html
* igt@gem_exec_basic@basic-all:
- fi-cml-u2: [INCOMPLETE][13] ([fdo#110566]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/fi-cml-u2/igt@gem_exec_basic@basic-all.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/fi-cml-u2/igt@gem_exec_basic@basic-all.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#108044]: https://bugs.freedesktop.org/show_bug.cgi?id=108044
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
[fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566
Participating hosts (54 -> 47)
------------------------------
Missing (7): fi-ilk-m540 fi-hsw-4200u fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_6216 -> Patchwork_13201
CI_DRM_6216: e58a2b1f565ec5d77c69724d2f43a7de6f7953b3 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5047: 3f4b1a49ed5e1c77ea42970d4d3156c997f66050 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13201: 768c3b01e168330056b40713f46ea998e18b3a2c @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
768c3b01e168 drm/i915/dmc: protect against loading wrong firmware
d8baaeb55131 drm/i915/dmc: remove redundant return in parse_csr_fw()
912af5910248 drm/i915/dmc: add support to load dmc_header version 3
0c4b45196f4a drm/i915/dmc: extract function to parse dmc_header
11f96e9969b0 drm/i915/dmc: extract function to parse package_header
baa55de8db95 drm/i915/dmc: extract function to parse css header
71c4555f11ca drm/i915/dmc: add support for package_header with version 2
80a7692a85f3 drm/i915/dmc: extract fw_info and table walk from intel_package_header
455b1de4efc3 drm/i915/dmc: use kernel types
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* ✓ Fi.CI.IGT: success for Add support for new DMC headers
2019-06-07 9:12 [PATCH 0/9] Add support for new DMC headers Lucas De Marchi
` (9 preceding siblings ...)
2019-06-07 10:40 ` ✓ Fi.CI.BAT: success for Add support for new DMC headers Patchwork
@ 2019-06-09 17:17 ` Patchwork
10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-06-09 17:17 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: Add support for new DMC headers
URL : https://patchwork.freedesktop.org/series/61761/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6216_full -> Patchwork_13201_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Known issues
------------
Here are the changes found in Patchwork_13201_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_eio@in-flight-suspend:
- shard-kbl: [PASS][1] -> [DMESG-WARN][2] ([fdo#108566]) +1 similar issue
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-kbl3/igt@gem_eio@in-flight-suspend.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-kbl6/igt@gem_eio@in-flight-suspend.html
* igt@gem_persistent_relocs@forked-interruptible-thrash-inactive:
- shard-apl: [PASS][3] -> [INCOMPLETE][4] ([fdo#103927])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-apl4/igt@gem_persistent_relocs@forked-interruptible-thrash-inactive.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-apl8/igt@gem_persistent_relocs@forked-interruptible-thrash-inactive.html
* igt@gem_softpin@noreloc-s3:
- shard-skl: [PASS][5] -> [INCOMPLETE][6] ([fdo#104108])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-skl6/igt@gem_softpin@noreloc-s3.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-skl7/igt@gem_softpin@noreloc-s3.html
* igt@i915_suspend@debugfs-reader:
- shard-apl: [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +5 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-apl4/igt@i915_suspend@debugfs-reader.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-apl4/igt@i915_suspend@debugfs-reader.html
* igt@kms_cursor_crc@pipe-b-cursor-suspend:
- shard-skl: [PASS][9] -> [INCOMPLETE][10] ([fdo#110741])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-skl1/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-skl6/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw:
- shard-iclb: [PASS][11] -> [FAIL][12] ([fdo#103167]) +3 similar issues
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw.html
* igt@kms_frontbuffer_tracking@psr-suspend:
- shard-skl: [PASS][13] -> [INCOMPLETE][14] ([fdo#104108] / [fdo#106978])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-skl8/igt@kms_frontbuffer_tracking@psr-suspend.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-skl10/igt@kms_frontbuffer_tracking@psr-suspend.html
* igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
- shard-skl: [PASS][15] -> [FAIL][16] ([fdo#108145])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
* igt@kms_plane_lowres@pipe-a-tiling-x:
- shard-iclb: [PASS][17] -> [FAIL][18] ([fdo#103166])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-x.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-x.html
* igt@kms_psr@psr2_sprite_blt:
- shard-iclb: [PASS][19] -> [SKIP][20] ([fdo#109441])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-iclb2/igt@kms_psr@psr2_sprite_blt.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-iclb6/igt@kms_psr@psr2_sprite_blt.html
#### Possible fixes ####
* igt@gem_ctx_engines@execute-one:
- shard-skl: [DMESG-WARN][21] -> [PASS][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-skl1/igt@gem_ctx_engines@execute-one.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-skl6/igt@gem_ctx_engines@execute-one.html
* igt@gem_ctx_isolation@rcs0-s3:
- shard-iclb: [INCOMPLETE][23] ([fdo#107713] / [fdo#109100]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-iclb7/igt@gem_ctx_isolation@rcs0-s3.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-iclb7/igt@gem_ctx_isolation@rcs0-s3.html
* igt@gem_workarounds@suspend-resume-fd:
- shard-skl: [INCOMPLETE][25] ([fdo#104108]) -> [PASS][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-skl1/igt@gem_workarounds@suspend-resume-fd.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-skl1/igt@gem_workarounds@suspend-resume-fd.html
* igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding:
- shard-skl: [FAIL][27] ([fdo#103232]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-skl9/igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-skl8/igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding.html
* igt@kms_frontbuffer_tracking@fbc-stridechange:
- shard-iclb: [FAIL][29] ([fdo#103167]) -> [PASS][30] +5 similar issues
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-stridechange.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-stridechange.html
* igt@kms_frontbuffer_tracking@fbc-suspend:
- shard-apl: [DMESG-WARN][31] ([fdo#108566]) -> [PASS][32] +2 similar issues
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-apl1/igt@kms_frontbuffer_tracking@fbc-suspend.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-apl3/igt@kms_frontbuffer_tracking@fbc-suspend.html
* igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
- shard-skl: [FAIL][33] ([fdo#108145] / [fdo#110403]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-skl9/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
* igt@kms_psr@psr2_primary_blt:
- shard-iclb: [SKIP][35] ([fdo#109441]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-iclb6/igt@kms_psr@psr2_primary_blt.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-iclb2/igt@kms_psr@psr2_primary_blt.html
* igt@kms_setmode@basic:
- shard-kbl: [FAIL][37] ([fdo#99912]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-kbl7/igt@kms_setmode@basic.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-kbl3/igt@kms_setmode@basic.html
* igt@kms_vblank@pipe-c-query-idle-hang:
- shard-iclb: [INCOMPLETE][39] ([fdo#107713]) -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6216/shard-iclb1/igt@kms_vblank@pipe-c-query-idle-hang.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/shard-iclb7/igt@kms_vblank@pipe-c-query-idle-hang.html
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
[fdo#110741]: https://bugs.freedesktop.org/show_bug.cgi?id=110741
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (9 -> 9)
------------------------------
No changes in participating hosts
Build changes
-------------
* Linux: CI_DRM_6216 -> Patchwork_13201
CI_DRM_6216: e58a2b1f565ec5d77c69724d2f43a7de6f7953b3 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5047: 3f4b1a49ed5e1c77ea42970d4d3156c997f66050 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13201: 768c3b01e168330056b40713f46ea998e18b3a2c @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13201/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/9] drm/i915/dmc: extract fw_info and table walk from intel_package_header
2019-06-07 9:12 ` [PATCH 2/9] drm/i915/dmc: extract fw_info and table walk from intel_package_header Lucas De Marchi
@ 2019-06-10 18:16 ` Srivatsa, Anusha
0 siblings, 0 replies; 22+ messages in thread
From: Srivatsa, Anusha @ 2019-06-10 18:16 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx
>-----Original Message-----
>From: De Marchi, Lucas
>Sent: Friday, June 7, 2019 2:12 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Srivatsa, Anusha
><anusha.srivatsa@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>
>Subject: [PATCH 2/9] drm/i915/dmc: extract fw_info and table walk from
>intel_package_header
>
>Move fw_info out of struct intel_package_header to allow it to grow more easily
>in future. To make a cleaner move, let's also extract a function to search the
>header for the dmc_offset.
>
>While reviewing this code I wondered why we continued the search even after
>finding a suitable firmware. Add a comment to explain we will continue to try to
>find a more specific firmware version, even if this is not required by the spec.
>
>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>---
> drivers/gpu/drm/i915/intel_csr.c | 72 ++++++++++++++++++++++++--------
> 1 file changed, 55 insertions(+), 17 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>index 3d2beecd8d0d..99fa4db95e46 100644
>--- a/drivers/gpu/drm/i915/intel_csr.c
>+++ b/drivers/gpu/drm/i915/intel_csr.c
>@@ -70,6 +70,7 @@ MODULE_FIRMWARE(SKL_CSR_PATH);
>MODULE_FIRMWARE(BXT_CSR_PATH);
>
> #define CSR_DEFAULT_FW_OFFSET 0xFFFFFFFF
>+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>
> struct intel_css_header {
> /* 0x09 for DMC */
>@@ -139,8 +140,6 @@ struct intel_package_header {
>
> /* Number of valid entries in the FWInfo array below */
> u32 num_entries;
>-
>- struct intel_fw_info fw_info[20];
> } __packed;
>
> struct intel_dmc_header {
>@@ -292,6 +291,46 @@ void intel_csr_load_program(struct drm_i915_private
>*dev_priv)
> gen9_set_dc_state_debugmask(dev_priv);
> }
>
>+/*
>+ * Search fw_info table for dmc_offset to find firmware binary:
>+num_entries is
>+ * already sanitized.
>+ */
>+static u32 find_dmc_fw_offset(const struct intel_fw_info *fw_info,
>+ unsigned int num_entries,
>+ const struct stepping_info *si) {
>+ u32 dmc_offset = CSR_DEFAULT_FW_OFFSET;
>+ unsigned int i;
>+
>+ for (i = 0; i < num_entries; i++) {
>+ if (fw_info[i].substepping == '*' &&
>+ si->stepping == fw_info[i].stepping) {
>+ dmc_offset = fw_info[i].offset;
>+ break;
>+ }
>+
>+ if (si->stepping == fw_info[i].stepping &&
>+ si->substepping == fw_info[i].substepping) {
>+ dmc_offset = fw_info[i].offset;
>+ break;
>+ }
>+
>+ if (fw_info[i].stepping == '*' &&
>+ fw_info[i].substepping == '*') {
>+ /*
>+ * In theory we should stop the search as generic
>+ * entries should always come after the more specific
>+ * ones, but let's continue to make sure to work even
>+ * with "broken" firmwares. If we don't find a more
>+ * specific one, then we use this entry
>+ */
>+ dmc_offset = fw_info[i].offset;
>+ }
>+ }
>+
>+ return dmc_offset;
>+}
>+
> static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
> const struct firmware *fw)
> {
>@@ -300,7 +339,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 = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
>+ u32 dmc_offset, num_entries, readcount = 0, nbytes;
> u32 i;
> u32 *dmc_payload;
> size_t fsize;
>@@ -349,27 +388,26 @@ static u32 *parse_csr_fw(struct drm_i915_private
>*dev_priv,
> (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;
>
>- /* Search for dmc_offset to find firware binary. */
>- for (i = 0; i < package_header->num_entries; i++) {
>- if (package_header->fw_info[i].substepping == '*' &&
>- si->stepping == package_header->fw_info[i].stepping) {
>- dmc_offset = package_header->fw_info[i].offset;
>- break;
>- } else if (si->stepping == package_header->fw_info[i].stepping
>&&
>- si->substepping == package_header-
>>fw_info[i].substepping) {
>- dmc_offset = package_header->fw_info[i].offset;
>- break;
>- } else if (package_header->fw_info[i].stepping == '*' &&
>- package_header->fw_info[i].substepping == '*')
>- dmc_offset = package_header->fw_info[i].offset;
>- }
>+ fsize += PACKAGE_MAX_FW_INFO_ENTRIES * sizeof(struct
>intel_fw_info);
>+ if (fsize > fw->size)
>+ goto error_truncated;
>+
>+ dmc_offset = find_dmc_fw_offset((struct intel_fw_info *)
>+ &fw->data[readcount], num_entries, si);
> if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
> DRM_ERROR("DMC firmware not supported for %c stepping\n",
> 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);
>+
> /* Convert dmc_offset into number of bytes. By default it is in dwords*/
> dmc_offset *= 4;
> readcount += dmc_offset;
>--
>2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/9] drm/i915/dmc: add support for package_header with version 2
2019-06-07 9:12 ` [PATCH 3/9] drm/i915/dmc: add support for package_header with version 2 Lucas De Marchi
@ 2019-06-10 18:22 ` Srivatsa, Anusha
0 siblings, 0 replies; 22+ messages in thread
From: Srivatsa, Anusha @ 2019-06-10 18:22 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx
>-----Original Message-----
>From: De Marchi, Lucas
>Sent: Friday, June 7, 2019 2:12 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Srivatsa, Anusha
><anusha.srivatsa@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>
>Subject: [PATCH 3/9] 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>
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>---
> drivers/gpu/drm/i915/intel_csr.c | 38 ++++++++++++++++++++++----------
> 1 file changed, 26 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>index 99fa4db95e46..ba72c29acbcc 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;
> size_t fsize;
>@@ -381,20 +382,32 @@ 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;
>
>- fsize += PACKAGE_MAX_FW_INFO_ENTRIES * sizeof(struct
>intel_fw_info);
>+ fsize += max_entries * sizeof(struct intel_fw_info);
> if (fsize > fw->size)
> goto error_truncated;
>
>@@ -405,8 +418,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;
>--
>2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4/9] drm/i915/dmc: extract function to parse css header
2019-06-07 9:12 ` [PATCH 4/9] drm/i915/dmc: extract function to parse css header Lucas De Marchi
@ 2019-06-14 19:30 ` Srivatsa, Anusha
0 siblings, 0 replies; 22+ messages in thread
From: Srivatsa, Anusha @ 2019-06-14 19:30 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx
>-----Original Message-----
>From: De Marchi, Lucas
>Sent: Friday, June 7, 2019 2:12 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Srivatsa, Anusha
><anusha.srivatsa@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>
>Subject: [PATCH 4/9] drm/i915/dmc: extract function to parse css header
>
>Let's start splitting the parse function, making all of them return the number of
>bytes parsed - different versions of the firmware header may require different
>sizes for the structures.
>
>v2: rework remaining bytes calculation on new protection for amount of
> bytes read
Looks good.
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>---
> drivers/gpu/drm/i915/intel_csr.c | 66 ++++++++++++++++++++------------
> 1 file changed, 41 insertions(+), 25 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>index ba72c29acbcc..79732075f008 100644
>--- a/drivers/gpu/drm/i915/intel_csr.c
>+++ b/drivers/gpu/drm/i915/intel_csr.c
>@@ -332,36 +332,22 @@ static u32 find_dmc_fw_offset(const struct
>intel_fw_info *fw_info,
> return dmc_offset;
> }
>
>-static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
>- const struct firmware *fw)
>+/* Return number of bytes parsed or 0 on error */ static u32
>+parse_csr_fw_css(struct intel_csr *csr,
>+ struct intel_css_header *css_header,
>+ size_t rem_size)
> {
>- struct intel_css_header *css_header;
>- struct intel_package_header *package_header;
>- 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, max_entries, readcount = 0, nbytes;
>- u32 i;
>- u32 *dmc_payload;
>- size_t fsize;
>-
>- if (!fw)
>- return NULL;
>-
>- fsize = sizeof(struct intel_css_header) +
>- sizeof(struct intel_package_header) +
>- sizeof(struct intel_dmc_header);
>- if (fsize > fw->size)
>- goto error_truncated;
>+ if (rem_size < sizeof(struct intel_css_header)) {
>+ DRM_ERROR("Truncated DMC firmware, refusing.\n");
>+ return 0;
>+ }
>
>- /* Extract CSS Header information*/
>- css_header = (struct intel_css_header *)fw->data;
> if (sizeof(struct intel_css_header) !=
> (css_header->header_len * 4)) {
> DRM_ERROR("DMC firmware has wrong CSS header length "
> "(%u bytes)\n",
> (css_header->header_len * 4));
>- return NULL;
>+ return 0;
> }
>
> if (csr->required_version &&
>@@ -372,12 +358,42 @@ static u32 *parse_csr_fw(struct drm_i915_private
>*dev_priv,
> CSR_VERSION_MINOR(css_header->version),
> CSR_VERSION_MAJOR(csr->required_version),
> CSR_VERSION_MINOR(csr->required_version));
>- return NULL;
>+ return 0;
> }
>
> csr->version = css_header->version;
>
>- readcount += sizeof(struct intel_css_header);
>+ return sizeof(struct intel_css_header); }
>+
>+static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
>+ const struct firmware *fw)
>+{
>+ struct intel_css_header *css_header;
>+ struct intel_package_header *package_header;
>+ 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, max_entries, readcount = 0, nbytes;
>+ u32 i, r;
>+ u32 *dmc_payload;
>+ size_t fsize;
>+
>+ if (!fw)
>+ return NULL;
>+
>+ /* Extract CSS Header information */
>+ css_header = (struct intel_css_header *)fw->data;
>+ r = parse_csr_fw_css(csr, css_header, fw->size);
>+ if (!r)
>+ return NULL;
>+
>+ readcount += r;
>+ fsize = readcount +
>+ sizeof(struct intel_package_header) +
>+ sizeof(struct intel_dmc_header);
>+ if (fsize > fw->size)
>+ goto error_truncated;
>
> /* Extract Package Header information*/
> package_header = (struct intel_package_header *)
>--
>2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware
2019-06-07 9:12 ` [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware Lucas De Marchi
@ 2019-06-14 19:44 ` Srivatsa, Anusha
2019-06-14 21:37 ` Lucas De Marchi
0 siblings, 1 reply; 22+ messages in thread
From: Srivatsa, Anusha @ 2019-06-14 19:44 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx
>-----Original Message-----
>From: De Marchi, Lucas
>Sent: Friday, June 7, 2019 2:13 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Srivatsa, Anusha
><anusha.srivatsa@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>
>Subject: [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware
>
>In intel_package_header version 2 there's a new field in the fw_info table that
>must be 0, otherwise it's not the correct DMC firmware. Add a check for version 2
>or later.
>
>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>---
> drivers/gpu/drm/i915/intel_csr.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>index 7608e4e2950d..864531aae1a5 100644
>--- a/drivers/gpu/drm/i915/intel_csr.c
>+++ b/drivers/gpu/drm/i915/intel_csr.c
>@@ -120,7 +120,10 @@ struct intel_css_header { } __packed;
>
> struct intel_fw_info {
>- u16 reserved1;
>+ u8 reserved1;
>+
>+ /* reserved on package_header version 1, must be 0 on version 2 */
>+ u8 dmc_id;
>
> /* Stepping (A, B, C, ..., *). * is a wildcard */
> char stepping;
>@@ -325,12 +328,16 @@ void intel_csr_load_program(struct drm_i915_private
>*dev_priv)
> */
> static u32 find_dmc_fw_offset(const struct intel_fw_info *fw_info,
> unsigned int num_entries,
>- const struct stepping_info *si)
>+ const struct stepping_info *si,
>+ u8 package_ver)
> {
> u32 dmc_offset = CSR_DEFAULT_FW_OFFSET;
> unsigned int i;
>
> for (i = 0; i < num_entries; i++) {
>+ if (package_ver > 1 && fw_info[i].dmc_id != 0)
>+ continue;
Wont we need a message here? "Wrong DMC version, not loading v_x.0x" or something?.."
Anusha
> if (fw_info[i].substepping == '*' &&
> si->stepping == fw_info[i].stepping) {
> dmc_offset = fw_info[i].offset;
>@@ -508,7 +515,8 @@ parse_csr_fw_package(struct intel_csr *csr,
>
> fw_info = (const struct intel_fw_info *)
> ((u8 *)package_header + sizeof(*package_header));
>- dmc_offset = find_dmc_fw_offset(fw_info, num_entries, si);
>+ dmc_offset = find_dmc_fw_offset(fw_info, num_entries, si,
>+ package_header->header_ver);
> if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
> DRM_ERROR("DMC firmware not supported for %c stepping\n",
> si->stepping);
>--
>2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 8/9] drm/i915/dmc: remove redundant return in parse_csr_fw()
2019-06-07 9:12 ` [PATCH 8/9] drm/i915/dmc: remove redundant return in parse_csr_fw() Lucas De Marchi
@ 2019-06-14 19:51 ` Srivatsa, Anusha
0 siblings, 0 replies; 22+ messages in thread
From: Srivatsa, Anusha @ 2019-06-14 19:51 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx
>-----Original Message-----
>From: De Marchi, Lucas
>Sent: Friday, June 7, 2019 2:12 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Srivatsa, Anusha
><anusha.srivatsa@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>
>Subject: [PATCH 8/9] drm/i915/dmc: remove redundant return in parse_csr_fw()
>
>parse_csr_fw() is responsible to set up several fields in struct intel_csr, including
>the payload. We don't need to assign it again.
>
>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Looks good.
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>---
> drivers/gpu/drm/i915/intel_csr.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>index 0b1978a2f87d..7608e4e2950d 100644
>--- a/drivers/gpu/drm/i915/intel_csr.c
>+++ b/drivers/gpu/drm/i915/intel_csr.c
>@@ -557,7 +557,7 @@ static u32 parse_csr_fw_css(struct intel_csr *csr,
> return sizeof(struct intel_css_header); }
>
>-static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
>+static void parse_csr_fw(struct drm_i915_private *dev_priv,
> const struct firmware *fw)
> {
> struct intel_css_header *css_header;
>@@ -569,13 +569,13 @@ static u32 *parse_csr_fw(struct drm_i915_private
>*dev_priv,
> u32 r;
>
> if (!fw)
>- return NULL;
>+ return;
>
> /* Extract CSS Header information */
> css_header = (struct intel_css_header *)fw->data;
> r = parse_csr_fw_css(csr, css_header, fw->size);
> if (!r)
>- return NULL;
>+ return;
>
> readcount += r;
>
>@@ -583,17 +583,13 @@ static u32 *parse_csr_fw(struct drm_i915_private
>*dev_priv,
> package_header = (struct intel_package_header *)&fw-
>>data[readcount];
> r = parse_csr_fw_package(csr, package_header, si, fw->size - readcount);
> if (!r)
>- return NULL;
>+ return;
>
> readcount += r;
>
> /* Extract dmc_header information */
> dmc_header = (struct intel_dmc_header_base *)&fw->data[readcount];
>- r = parse_csr_fw_dmc(csr, dmc_header, fw->size - readcount);
>- if (!r)
>- return NULL;
>-
>- return csr->dmc_payload;
>+ parse_csr_fw_dmc(csr, dmc_header, fw->size - readcount);
> }
>
> static void intel_csr_runtime_pm_get(struct drm_i915_private *dev_priv) @@ -
>621,8 +617,7 @@ static void csr_load_work_fn(struct work_struct *work)
> csr = &dev_priv->csr;
>
> request_firmware(&fw, dev_priv->csr.fw_path, &dev_priv->drm.pdev-
>>dev);
>- if (fw)
>- dev_priv->csr.dmc_payload = parse_csr_fw(dev_priv, fw);
>+ parse_csr_fw(dev_priv, fw);
>
> if (dev_priv->csr.dmc_payload) {
> intel_csr_load_program(dev_priv);
>--
>2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 7/9] drm/i915/dmc: add support to load dmc_header version 3
2019-06-07 9:12 ` [PATCH 7/9] drm/i915/dmc: add support to load dmc_header version 3 Lucas De Marchi
@ 2019-06-14 20:03 ` Srivatsa, Anusha
0 siblings, 0 replies; 22+ messages in thread
From: Srivatsa, Anusha @ 2019-06-14 20:03 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx
>-----Original Message-----
>From: De Marchi, Lucas
>Sent: Friday, June 7, 2019 2:12 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Srivatsa, Anusha
><anusha.srivatsa@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>
>Subject: [PATCH 7/9] drm/i915/dmc: add support to load dmc_header version 3
>
>Main difference is that now there are up to 20 MMIOs that can be set and a lot of
>noise due to the struct changing the fields in the middle.
Changes look good.
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>---
> drivers/gpu/drm/i915/i915_drv.h | 4 +-
> drivers/gpu/drm/i915/intel_csr.c | 121 ++++++++++++++++++++++++-------
> 2 files changed, 95 insertions(+), 30 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index dfe4b11ee423..fc0a770fef15 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -352,8 +352,8 @@ struct intel_csr {
> u32 dmc_fw_size; /* dwords */
> u32 version;
> u32 mmio_count;
>- i915_reg_t mmioaddr[8];
>- u32 mmiodata[8];
>+ i915_reg_t mmioaddr[20];
>+ u32 mmiodata[20];
> u32 dc_state;
> u32 allowed_dc_mask;
> intel_wakeref_t wakeref;
>diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>index 1fb42fd44519..0b1978a2f87d 100644
>--- a/drivers/gpu/drm/i915/intel_csr.c
>+++ b/drivers/gpu/drm/i915/intel_csr.c
>@@ -72,6 +72,8 @@ 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
>+#define DMC_V1_MAX_MMIO_COUNT 8
>+#define DMC_V3_MAX_MMIO_COUNT 20
>
> struct intel_css_header {
> /* 0x09 for DMC */
>@@ -143,7 +145,7 @@ struct intel_package_header {
> u32 num_entries;
> } __packed;
>
>-struct intel_dmc_header {
>+struct intel_dmc_header_base {
> /* always value would be 0x40403E3E */
> u32 signature;
>
>@@ -164,22 +166,47 @@ struct intel_dmc_header {
>
> /* Major Minor version */
> u32 fw_version;
>+} __packed;
>+
>+struct intel_dmc_header_v1 {
>+ struct intel_dmc_header_base base;
>
> /* Number of valid MMIO cycles present. */
> u32 mmio_count;
>
> /* MMIO address */
>- u32 mmioaddr[8];
>+ u32 mmioaddr[DMC_V1_MAX_MMIO_COUNT];
>
> /* MMIO data */
>- u32 mmiodata[8];
>+ u32 mmiodata[DMC_V1_MAX_MMIO_COUNT];
>
> /* FW filename */
>- unsigned char dfile[32];
>+ char dfile[32];
>
> u32 reserved1[2];
> } __packed;
>
>+struct intel_dmc_header_v3 {
>+ struct intel_dmc_header_base base;
>+
>+ /* DMC RAM start MMIO address */
>+ u32 start_mmioaddr;
>+
>+ u32 reserved[9];
>+
>+ /* FW filename */
>+ char dfile[32];
>+
>+ /* Number of valid MMIO cycles present. */
>+ u32 mmio_count;
>+
>+ /* MMIO address */
>+ u32 mmioaddr[DMC_V3_MAX_MMIO_COUNT];
>+
>+ /* MMIO data */
>+ u32 mmiodata[DMC_V3_MAX_MMIO_COUNT];
>+} __packed;
>+
> struct stepping_info {
> char stepping;
> char substepping;
>@@ -333,43 +360,83 @@ static u32 find_dmc_fw_offset(const struct
>intel_fw_info *fw_info, }
>
> static u32 parse_csr_fw_dmc(struct intel_csr *csr,
>- const struct intel_dmc_header *dmc_header,
>+ const struct intel_dmc_header_base *dmc_header,
> size_t rem_size)
> {
>- unsigned int i, payload_size;
>- u32 r;
>+ unsigned int header_len_bytes, dmc_header_size, payload_size, i;
>+ const u32 *mmioaddr, *mmiodata;
>+ u32 mmio_count, mmio_count_max;
> u8 *payload;
>
>- if (rem_size < sizeof(struct intel_dmc_header))
>+ BUILD_BUG_ON(ARRAY_SIZE(csr->mmioaddr) <
>DMC_V3_MAX_MMIO_COUNT ||
>+ ARRAY_SIZE(csr->mmioaddr) <
>DMC_V1_MAX_MMIO_COUNT);
>+
>+ /*
>+ * Check if we can access common fields, we will checkc again below
>+ * after we have read the version
>+ */
>+ if (rem_size < sizeof(struct intel_dmc_header_base))
> goto error_truncated;
>
>- if (sizeof(struct intel_dmc_header) != dmc_header->header_len) {
>+ /* Cope with small differences between v1 and v3 */
>+ if (dmc_header->header_ver == 3) {
>+ const struct intel_dmc_header_v3 *v3 =
>+ (const struct intel_dmc_header_v3 *)dmc_header;
>+
>+ if (rem_size < sizeof(struct intel_dmc_header_v3))
>+ goto error_truncated;
>+
>+ mmioaddr = v3->mmioaddr;
>+ mmiodata = v3->mmiodata;
>+ mmio_count = v3->mmio_count;
>+ mmio_count_max = DMC_V3_MAX_MMIO_COUNT;
>+ /* header_len is in dwords */
>+ header_len_bytes = dmc_header->header_len * 4;
>+ dmc_header_size = sizeof(*v3);
>+ } else if (dmc_header->header_ver == 1) {
>+ const struct intel_dmc_header_v1 *v1 =
>+ (const struct intel_dmc_header_v1 *)dmc_header;
>+
>+ if (rem_size < sizeof(struct intel_dmc_header_v1))
>+ goto error_truncated;
>+
>+ mmioaddr = v1->mmioaddr;
>+ mmiodata = v1->mmiodata;
>+ mmio_count = v1->mmio_count;
>+ mmio_count_max = DMC_V1_MAX_MMIO_COUNT;
>+ header_len_bytes = dmc_header->header_len;
>+ dmc_header_size = sizeof(*v1);
>+ } else {
>+ DRM_ERROR("Unknown DMC fw header version: %u\n",
>+ dmc_header->header_ver);
>+ return 0;
>+ }
>+
>+ if (header_len_bytes != dmc_header_size) {
> DRM_ERROR("DMC firmware has wrong dmc header length "
>- "(%u bytes)\n",
>- (dmc_header->header_len));
>+ "(%u bytes)\n", header_len_bytes);
> return 0;
> }
>
> /* Cache the dmc header info. */
>- if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) {
>- DRM_ERROR("DMC firmware has wrong mmio count %u\n",
>- dmc_header->mmio_count);
>+ if (mmio_count > mmio_count_max) {
>+ DRM_ERROR("DMC firmware has wrong mmio count %u\n",
>mmio_count);
> return 0;
> }
>
>- csr->mmio_count = dmc_header->mmio_count;
>- for (i = 0; i < dmc_header->mmio_count; i++) {
>- if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE ||
>- dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < CSR_MMIO_START_RANGE ||
>+ mmioaddr[i] > CSR_MMIO_END_RANGE) {
> DRM_ERROR("DMC firmware has wrong mmio address
>0x%x\n",
>- dmc_header->mmioaddr[i]);
>+ mmioaddr[i]);
> return 0;
> }
>- csr->mmioaddr[i] = _MMIO(dmc_header->mmioaddr[i]);
>- csr->mmiodata[i] = dmc_header->mmiodata[i];
>+ csr->mmioaddr[i] = _MMIO(mmioaddr[i]);
>+ csr->mmiodata[i] = mmiodata[i];
> }
>+ csr->mmio_count = mmio_count;
>
>- rem_size -= dmc_header->header_len;
>+ rem_size -= header_len_bytes;
>
> /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
> payload_size = dmc_header->fw_size * 4; @@ -388,12 +455,10 @@
>static u32 parse_csr_fw_dmc(struct intel_csr *csr,
> return 0;
> }
>
>- r = sizeof(struct intel_dmc_header);
>- payload = (u8 *)(dmc_header) + r;
>+ payload = (u8 *)(dmc_header) + header_len_bytes;
> memcpy(csr->dmc_payload, payload, payload_size);
>- r += payload_size;
>
>- return r;
>+ return header_len_bytes + payload_size;
>
> error_truncated:
> DRM_ERROR("Truncated DMC firmware, refusing.\n"); @@ -497,7
>+562,7 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv, {
> struct intel_css_header *css_header;
> struct intel_package_header *package_header;
>- struct intel_dmc_header *dmc_header;
>+ struct intel_dmc_header_base *dmc_header;
> struct intel_csr *csr = &dev_priv->csr;
> const struct stepping_info *si = intel_get_stepping_info(dev_priv);
> u32 readcount = 0;
>@@ -523,7 +588,7 @@ static u32 *parse_csr_fw(struct drm_i915_private
>*dev_priv,
> readcount += r;
>
> /* Extract dmc_header information */
>- dmc_header = (struct intel_dmc_header *)&fw->data[readcount];
>+ dmc_header = (struct intel_dmc_header_base *)&fw->data[readcount];
> r = parse_csr_fw_dmc(csr, dmc_header, fw->size - readcount);
> if (!r)
> return NULL;
>--
>2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/9] drm/i915/dmc: extract function to parse package_header
2019-06-07 9:12 ` [PATCH 5/9] drm/i915/dmc: extract function to parse package_header Lucas De Marchi
@ 2019-06-14 20:09 ` Srivatsa, Anusha
0 siblings, 0 replies; 22+ messages in thread
From: Srivatsa, Anusha @ 2019-06-14 20:09 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx
>-----Original Message-----
>From: De Marchi, Lucas
>Sent: Friday, June 7, 2019 2:12 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Srivatsa, Anusha
><anusha.srivatsa@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>
>Subject: [PATCH 5/9] drm/i915/dmc: extract function to parse package_header
>
>Like parse_csr_fw_css() this parses the package_header from firmware and saves
>the relevant fields in the csr struct. In this function we also lookup the fw_info we
>are interested in.
>
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>---
> drivers/gpu/drm/i915/intel_csr.c | 117 +++++++++++++++++--------------
> 1 file changed, 66 insertions(+), 51 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>index 79732075f008..db5772616a4f 100644
>--- a/drivers/gpu/drm/i915/intel_csr.c
>+++ b/drivers/gpu/drm/i915/intel_csr.c
>@@ -332,6 +332,64 @@ static u32 find_dmc_fw_offset(const struct
>intel_fw_info *fw_info,
> return dmc_offset;
> }
>
>+static u32
>+parse_csr_fw_package(struct intel_csr *csr,
>+ const struct intel_package_header *package_header,
>+ const struct stepping_info *si,
>+ size_t rem_size)
>+{
>+ u32 package_size = sizeof(struct intel_package_header);
>+ u32 num_entries, max_entries, dmc_offset;
>+ const struct intel_fw_info *fw_info;
>+
>+ if (rem_size < package_size)
>+ goto error_truncated;
>+
>+ 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 0;
>+ }
>+
>+ /*
>+ * We should always have space for max_entries,
>+ * even if not all are used
>+ */
>+ package_size += max_entries * sizeof(struct intel_fw_info);
>+ if (rem_size < package_size)
>+ goto error_truncated;
>+
>+ if (package_header->header_len * 4 != package_size) {
>+ DRM_ERROR("DMC firmware has wrong package header length "
>+ "(%u bytes)\n", package_size);
>+ return 0;
>+ }
>+
>+ num_entries = package_header->num_entries;
>+ if (WARN_ON(package_header->num_entries > max_entries))
>+ num_entries = max_entries;
>+
>+ fw_info = (const struct intel_fw_info *)
>+ ((u8 *)package_header + sizeof(*package_header));
>+ dmc_offset = find_dmc_fw_offset(fw_info, num_entries, si);
>+ if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
>+ DRM_ERROR("DMC firmware not supported for %c stepping\n",
>+ si->stepping);
>+ return 0;
>+ }
>+
>+ /* dmc_offset is in dwords */
>+ return package_size + dmc_offset * 4;
>+
>+error_truncated:
>+ DRM_ERROR("Truncated DMC firmware, refusing.\n");
>+ return 0;
>+}
>+
> /* Return number of bytes parsed or 0 on error */ static u32
>parse_csr_fw_css(struct intel_csr *csr,
> struct intel_css_header *css_header, @@ -374,7
>+432,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, max_entries, readcount = 0, nbytes;
>+ u32 readcount = 0, nbytes;
> u32 i, r;
> u32 *dmc_payload;
> size_t fsize;
>@@ -389,59 +447,16 @@ static u32 *parse_csr_fw(struct drm_i915_private
>*dev_priv,
> return NULL;
>
> readcount += r;
>- fsize = readcount +
>- sizeof(struct intel_package_header) +
>- sizeof(struct intel_dmc_header);
>- if (fsize > fw->size)
>- goto error_truncated;
>-
>- /* Extract Package Header information*/
>- package_header = (struct intel_package_header *)
>- &fw->data[readcount];
>-
>- 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);
>- return NULL;
>- }
>-
>- num_entries = package_header->num_entries;
>- if (WARN_ON(package_header->num_entries > max_entries))
>- num_entries = max_entries;
>-
>- fsize += max_entries * sizeof(struct intel_fw_info);
>- if (fsize > fw->size)
>- goto error_truncated;
>
>- dmc_offset = find_dmc_fw_offset((struct intel_fw_info *)
>- &fw->data[readcount], num_entries, si);
>- if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
>- DRM_ERROR("DMC firmware not supported for %c stepping\n",
>- si->stepping);
>+ /* Extract Package Header information */
>+ package_header = (struct intel_package_header *)&fw-
>>data[readcount];
>+ r = parse_csr_fw_package(csr, package_header, si, fw->size - readcount);
>+ if (!r)
> return NULL;
>- }
>
>- /* 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;
>- fsize += dmc_offset;
>+ readcount += r;
>+ fsize = readcount +
>+ sizeof(struct intel_dmc_header);
> if (fsize > fw->size)
> goto error_truncated;
>
>--
>2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 6/9] drm/i915/dmc: extract function to parse dmc_header
2019-06-07 9:12 ` [PATCH 6/9] drm/i915/dmc: extract function to parse dmc_header Lucas De Marchi
@ 2019-06-14 20:24 ` Srivatsa, Anusha
0 siblings, 0 replies; 22+ messages in thread
From: Srivatsa, Anusha @ 2019-06-14 20:24 UTC (permalink / raw)
To: De Marchi, Lucas, intel-gfx
>-----Original Message-----
>From: De Marchi, Lucas
>Sent: Friday, June 7, 2019 2:12 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Srivatsa, Anusha
><anusha.srivatsa@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>
>Subject: [PATCH 6/9] drm/i915/dmc: extract function to parse dmc_header
>
>Complete the extraction of functions to parse specific parts of the firmware. The
>return of the function parse_csr_fw() is now redundant since it already sets the
>dmc_payload field. Changing it is left for later to avoid noise in the commit.
>
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>---
> drivers/gpu/drm/i915/intel_csr.c | 130 ++++++++++++++++++-------------
> 1 file changed, 74 insertions(+), 56 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>index db5772616a4f..1fb42fd44519 100644
>--- a/drivers/gpu/drm/i915/intel_csr.c
>+++ b/drivers/gpu/drm/i915/intel_csr.c
>@@ -332,6 +332,74 @@ static u32 find_dmc_fw_offset(const struct
>intel_fw_info *fw_info,
> return dmc_offset;
> }
>
>+static u32 parse_csr_fw_dmc(struct intel_csr *csr,
>+ const struct intel_dmc_header *dmc_header,
>+ size_t rem_size)
>+{
>+ unsigned int i, payload_size;
>+ u32 r;
>+ u8 *payload;
>+
>+ if (rem_size < sizeof(struct intel_dmc_header))
>+ goto error_truncated;
>+
>+ if (sizeof(struct intel_dmc_header) != dmc_header->header_len) {
>+ DRM_ERROR("DMC firmware has wrong dmc header length "
>+ "(%u bytes)\n",
>+ (dmc_header->header_len));
>+ return 0;
>+ }
>+
>+ /* Cache the dmc header info. */
>+ if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) {
>+ DRM_ERROR("DMC firmware has wrong mmio count %u\n",
>+ dmc_header->mmio_count);
>+ return 0;
>+ }
>+
>+ csr->mmio_count = dmc_header->mmio_count;
>+ for (i = 0; i < dmc_header->mmio_count; i++) {
>+ if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE ||
>+ dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
>+ DRM_ERROR("DMC firmware has wrong mmio address
>0x%x\n",
>+ dmc_header->mmioaddr[i]);
>+ return 0;
>+ }
>+ csr->mmioaddr[i] = _MMIO(dmc_header->mmioaddr[i]);
>+ csr->mmiodata[i] = dmc_header->mmiodata[i];
>+ }
>+
>+ rem_size -= dmc_header->header_len;
>+
>+ /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
>+ payload_size = dmc_header->fw_size * 4;
>+ if (rem_size < payload_size)
>+ goto error_truncated;
>+
>+ if (payload_size > csr->max_fw_size) {
>+ DRM_ERROR("DMC FW too big (%u bytes)\n", payload_size);
>+ return 0;
>+ }
>+ csr->dmc_fw_size = dmc_header->fw_size;
>+
>+ csr->dmc_payload = kmalloc(payload_size, GFP_KERNEL);
>+ if (!csr->dmc_payload) {
>+ DRM_ERROR("Memory allocation failed for dmc payload\n");
>+ return 0;
>+ }
>+
>+ r = sizeof(struct intel_dmc_header);
>+ payload = (u8 *)(dmc_header) + r;
>+ memcpy(csr->dmc_payload, payload, payload_size);
>+ r += payload_size;
>+
>+ return r;
>+
>+error_truncated:
>+ DRM_ERROR("Truncated DMC firmware, refusing.\n");
>+ return 0;
>+}
>+
> static u32
> parse_csr_fw_package(struct intel_csr *csr,
> const struct intel_package_header *package_header, @@ -
>432,10 +500,8 @@ 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 readcount = 0, nbytes;
>- u32 i, r;
>- u32 *dmc_payload;
>- size_t fsize;
>+ u32 readcount = 0;
>+ u32 r;
>
> if (!fw)
> return NULL;
>@@ -455,62 +521,14 @@ static u32 *parse_csr_fw(struct drm_i915_private
>*dev_priv,
> return NULL;
>
> readcount += r;
>- fsize = readcount +
>- sizeof(struct intel_dmc_header);
>- if (fsize > fw->size)
>- goto error_truncated;
>
>- /* Extract dmc_header information. */
>+ /* Extract dmc_header information */
> dmc_header = (struct intel_dmc_header *)&fw->data[readcount];
>- if (sizeof(struct intel_dmc_header) != (dmc_header->header_len)) {
>- DRM_ERROR("DMC firmware has wrong dmc header length "
>- "(%u bytes)\n",
>- (dmc_header->header_len));
>- return NULL;
>- }
>- readcount += sizeof(struct intel_dmc_header);
>-
>- /* Cache the dmc header info. */
>- if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) {
>- DRM_ERROR("DMC firmware has wrong mmio count %u\n",
>- dmc_header->mmio_count);
>- return NULL;
>- }
>- csr->mmio_count = dmc_header->mmio_count;
>- for (i = 0; i < dmc_header->mmio_count; i++) {
>- if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE ||
>- dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
>- DRM_ERROR("DMC firmware has wrong mmio address
>0x%x\n",
>- dmc_header->mmioaddr[i]);
>- return NULL;
>- }
>- csr->mmioaddr[i] = _MMIO(dmc_header->mmioaddr[i]);
>- csr->mmiodata[i] = dmc_header->mmiodata[i];
>- }
>-
>- /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
>- nbytes = dmc_header->fw_size * 4;
>- fsize += nbytes;
>- if (fsize > fw->size)
>- goto error_truncated;
>-
>- if (nbytes > csr->max_fw_size) {
>- DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
>- return NULL;
>- }
>- csr->dmc_fw_size = dmc_header->fw_size;
>-
>- dmc_payload = kmalloc(nbytes, GFP_KERNEL);
>- if (!dmc_payload) {
>- DRM_ERROR("Memory allocation failed for dmc payload\n");
>+ r = parse_csr_fw_dmc(csr, dmc_header, fw->size - readcount);
>+ if (!r)
> return NULL;
>- }
>
>- return memcpy(dmc_payload, &fw->data[readcount], nbytes);
>-
>-error_truncated:
>- DRM_ERROR("Truncated DMC firmware, rejecting.\n");
>- return NULL;
>+ return csr->dmc_payload;
> }
>
> static void intel_csr_runtime_pm_get(struct drm_i915_private *dev_priv)
>--
>2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware
2019-06-14 19:44 ` Srivatsa, Anusha
@ 2019-06-14 21:37 ` Lucas De Marchi
2019-06-17 17:46 ` Srivatsa, Anusha
0 siblings, 1 reply; 22+ messages in thread
From: Lucas De Marchi @ 2019-06-14 21:37 UTC (permalink / raw)
To: Srivatsa, Anusha; +Cc: intel-gfx
On Fri, Jun 14, 2019 at 12:44:50PM -0700, Anusha Srivatsa wrote:
>
>
>>-----Original Message-----
>>From: De Marchi, Lucas
>>Sent: Friday, June 7, 2019 2:13 AM
>>To: intel-gfx@lists.freedesktop.org
>>Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Srivatsa, Anusha
>><anusha.srivatsa@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>
>>Subject: [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware
>>
>>In intel_package_header version 2 there's a new field in the fw_info table that
>>must be 0, otherwise it's not the correct DMC firmware. Add a check for version 2
>>or later.
>>
>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>---
>> drivers/gpu/drm/i915/intel_csr.c | 14 +++++++++++---
>> 1 file changed, 11 insertions(+), 3 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>>index 7608e4e2950d..864531aae1a5 100644
>>--- a/drivers/gpu/drm/i915/intel_csr.c
>>+++ b/drivers/gpu/drm/i915/intel_csr.c
>>@@ -120,7 +120,10 @@ struct intel_css_header { } __packed;
>>
>> struct intel_fw_info {
>>- u16 reserved1;
>>+ u8 reserved1;
>>+
>>+ /* reserved on package_header version 1, must be 0 on version 2 */
>>+ u8 dmc_id;
>>
>> /* Stepping (A, B, C, ..., *). * is a wildcard */
>> char stepping;
>>@@ -325,12 +328,16 @@ void intel_csr_load_program(struct drm_i915_private
>>*dev_priv)
>> */
>> static u32 find_dmc_fw_offset(const struct intel_fw_info *fw_info,
>> unsigned int num_entries,
>>- const struct stepping_info *si)
>>+ const struct stepping_info *si,
>>+ u8 package_ver)
>> {
>> u32 dmc_offset = CSR_DEFAULT_FW_OFFSET;
>> unsigned int i;
>>
>> for (i = 0; i < num_entries; i++) {
>>+ if (package_ver > 1 && fw_info[i].dmc_id != 0)
>>+ continue;
>
>Wont we need a message here? "Wrong DMC version, not loading v_x.0x" or something?.."
I don't think so. It's normal to have other blobs inside the firmware
that we don't care about. At most I could put a debug, but I don't think
we really care.
As long as we find one we should be fine. And if we don't, then we will
complain loudly later in this function with a DRM_ERROR().
dmc_offset remaining as CSR_DEFAULT_FW_OFFSET has a double meaning here:
either we didn't find any entry or we found one that had the offset set
to this value.
Lucas De Marchi
>
>Anusha
>> if (fw_info[i].substepping == '*' &&
>> si->stepping == fw_info[i].stepping) {
>> dmc_offset = fw_info[i].offset;
>>@@ -508,7 +515,8 @@ parse_csr_fw_package(struct intel_csr *csr,
>>
>> fw_info = (const struct intel_fw_info *)
>> ((u8 *)package_header + sizeof(*package_header));
>>- dmc_offset = find_dmc_fw_offset(fw_info, num_entries, si);
>>+ dmc_offset = find_dmc_fw_offset(fw_info, num_entries, si,
>>+ package_header->header_ver);
>> if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
>> DRM_ERROR("DMC firmware not supported for %c stepping\n",
>> si->stepping);
>>--
>>2.21.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware
2019-06-14 21:37 ` Lucas De Marchi
@ 2019-06-17 17:46 ` Srivatsa, Anusha
0 siblings, 0 replies; 22+ messages in thread
From: Srivatsa, Anusha @ 2019-06-17 17:46 UTC (permalink / raw)
To: De Marchi, Lucas; +Cc: intel-gfx
>-----Original Message-----
>From: De Marchi, Lucas
>Sent: Friday, June 14, 2019 2:37 PM
>To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo <rodrigo.vivi@intel.com>
>Subject: Re: [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware
>
>On Fri, Jun 14, 2019 at 12:44:50PM -0700, Anusha Srivatsa wrote:
>>
>>
>>>-----Original Message-----
>>>From: De Marchi, Lucas
>>>Sent: Friday, June 7, 2019 2:13 AM
>>>To: intel-gfx@lists.freedesktop.org
>>>Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Srivatsa, Anusha
>>><anusha.srivatsa@intel.com>; De Marchi, Lucas
>>><lucas.demarchi@intel.com>
>>>Subject: [PATCH 9/9] drm/i915/dmc: protect against loading wrong
>>>firmware
>>>
>>>In intel_package_header version 2 there's a new field in the fw_info
>>>table that must be 0, otherwise it's not the correct DMC firmware. Add
>>>a check for version 2 or later.
>>>
>>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>>---
>>> drivers/gpu/drm/i915/intel_csr.c | 14 +++++++++++---
>>> 1 file changed, 11 insertions(+), 3 deletions(-)
>>>
>>>diff --git a/drivers/gpu/drm/i915/intel_csr.c
>>>b/drivers/gpu/drm/i915/intel_csr.c
>>>index 7608e4e2950d..864531aae1a5 100644
>>>--- a/drivers/gpu/drm/i915/intel_csr.c
>>>+++ b/drivers/gpu/drm/i915/intel_csr.c
>>>@@ -120,7 +120,10 @@ struct intel_css_header { } __packed;
>>>
>>> struct intel_fw_info {
>>>- u16 reserved1;
>>>+ u8 reserved1;
>>>+
>>>+ /* reserved on package_header version 1, must be 0 on version 2 */
>>>+ u8 dmc_id;
>>>
>>> /* Stepping (A, B, C, ..., *). * is a wildcard */
>>> char stepping;
>>>@@ -325,12 +328,16 @@ void intel_csr_load_program(struct
>>>drm_i915_private
>>>*dev_priv)
>>> */
>>> static u32 find_dmc_fw_offset(const struct intel_fw_info *fw_info,
>>> unsigned int num_entries,
>>>- const struct stepping_info *si)
>>>+ const struct stepping_info *si,
>>>+ u8 package_ver)
>>> {
>>> u32 dmc_offset = CSR_DEFAULT_FW_OFFSET;
>>> unsigned int i;
>>>
>>> for (i = 0; i < num_entries; i++) {
>>>+ if (package_ver > 1 && fw_info[i].dmc_id != 0)
>>>+ continue;
>>
>>Wont we need a message here? "Wrong DMC version, not loading v_x.0x" or
>something?.."
>
>I don't think so. It's normal to have other blobs inside the firmware that we don't
>care about. At most I could put a debug, but I don't think we really care.
>
>As long as we find one we should be fine. And if we don't, then we will complain
>loudly later in this function with a DRM_ERROR().
>dmc_offset remaining as CSR_DEFAULT_FW_OFFSET has a double meaning here:
>either we didn't find any entry or we found one that had the offset set to this
>value.
Ok. Makes sense.
Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>Lucas De Marchi
>
>>
>>Anusha
>>> if (fw_info[i].substepping == '*' &&
>>> si->stepping == fw_info[i].stepping) {
>>> dmc_offset = fw_info[i].offset;
>>>@@ -508,7 +515,8 @@ parse_csr_fw_package(struct intel_csr *csr,
>>>
>>> fw_info = (const struct intel_fw_info *)
>>> ((u8 *)package_header + sizeof(*package_header));
>>>- dmc_offset = find_dmc_fw_offset(fw_info, num_entries, si);
>>>+ dmc_offset = find_dmc_fw_offset(fw_info, num_entries, si,
>>>+ package_header->header_ver);
>>> if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
>>> DRM_ERROR("DMC firmware not supported for %c stepping\n",
>>> si->stepping);
>>>--
>>>2.21.0
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2019-06-17 17:46 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-07 9:12 [PATCH 0/9] Add support for new DMC headers Lucas De Marchi
2019-06-07 9:12 ` [PATCH 1/9] drm/i915/dmc: use kernel types Lucas De Marchi
2019-06-07 9:12 ` [PATCH 2/9] drm/i915/dmc: extract fw_info and table walk from intel_package_header Lucas De Marchi
2019-06-10 18:16 ` Srivatsa, Anusha
2019-06-07 9:12 ` [PATCH 3/9] drm/i915/dmc: add support for package_header with version 2 Lucas De Marchi
2019-06-10 18:22 ` Srivatsa, Anusha
2019-06-07 9:12 ` [PATCH 4/9] drm/i915/dmc: extract function to parse css header Lucas De Marchi
2019-06-14 19:30 ` Srivatsa, Anusha
2019-06-07 9:12 ` [PATCH 5/9] drm/i915/dmc: extract function to parse package_header Lucas De Marchi
2019-06-14 20:09 ` Srivatsa, Anusha
2019-06-07 9:12 ` [PATCH 6/9] drm/i915/dmc: extract function to parse dmc_header Lucas De Marchi
2019-06-14 20:24 ` Srivatsa, Anusha
2019-06-07 9:12 ` [PATCH 7/9] drm/i915/dmc: add support to load dmc_header version 3 Lucas De Marchi
2019-06-14 20:03 ` Srivatsa, Anusha
2019-06-07 9:12 ` [PATCH 8/9] drm/i915/dmc: remove redundant return in parse_csr_fw() Lucas De Marchi
2019-06-14 19:51 ` Srivatsa, Anusha
2019-06-07 9:12 ` [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware Lucas De Marchi
2019-06-14 19:44 ` Srivatsa, Anusha
2019-06-14 21:37 ` Lucas De Marchi
2019-06-17 17:46 ` Srivatsa, Anusha
2019-06-07 10:40 ` ✓ Fi.CI.BAT: success for Add support for new DMC headers Patchwork
2019-06-09 17:17 ` ✓ Fi.CI.IGT: " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.