[PATCH 2/9] drm/i915/dmc: extract fw_info and table walk from intel_package_header

[Lucas De Marchi <lucas.demarchi@intel.com>]

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