diff mbox series

drm/i915/intel_csr.c Fix DMC FW Loading issue on ICL.

Message ID 1535484259-17550-1-git-send-email-jyoti.r.yadav@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/intel_csr.c Fix DMC FW Loading issue on ICL. | expand

Commit Message

Yadav, Jyoti R Aug. 28, 2018, 7:24 p.m. UTC
From: Jyoti <jyoti.r.yadav@intel.com>

This patch resolves the DMC FW loading issue.
Earlier DMC FW package have only one DMC FW for one stepping. But as such
there is no such restriction from Package side.
For ICL icl_dmc_ver1_07.bin binary package has DMC FW for 2 steppings.
So while reading the dmc_offset from package header, for 1st stepping offset
used to come 0x0 and was working fine till now.
But for second stepping and other steppings, offset is non zero numaber and is
in dwords. So we need to convert into bytes to fetch correct DMC FW from
correct place.

v2 : Added check for DMC FW max size for various gen. (Imre Deak)
v3 : Corrected naming convention for various gen. (Imre Deak)
v4 : Initalized max_fw_size to 0

Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
---
 drivers/gpu/drm/i915/intel_csr.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

Comments

Imre Deak Aug. 29, 2018, 9:21 a.m. UTC | #1
On Tue, Aug 28, 2018 at 03:24:19PM -0400, Jyoti Yadav wrote:
> From: Jyoti <jyoti.r.yadav@intel.com>
> 
> This patch resolves the DMC FW loading issue.
> Earlier DMC FW package have only one DMC FW for one stepping. But as such
> there is no such restriction from Package side.
> For ICL icl_dmc_ver1_07.bin binary package has DMC FW for 2 steppings.
> So while reading the dmc_offset from package header, for 1st stepping offset
> used to come 0x0 and was working fine till now.
> But for second stepping and other steppings, offset is non zero numaber and is
> in dwords. So we need to convert into bytes to fetch correct DMC FW from
> correct place.
> 
> v2 : Added check for DMC FW max size for various gen. (Imre Deak)
> v3 : Corrected naming convention for various gen. (Imre Deak)
> v4 : Initalized max_fw_size to 0
> 
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_csr.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 1ec4f09..d12051b 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -55,7 +55,9 @@
>  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
>  
>  
> -#define CSR_MAX_FW_SIZE			0x2FFF
> +#define BXT_CSR_MAX_FW_SIZE		0x2FFF
> +#define GLK_CSR_MAX_FW_SIZE		0x3FFF
> +#define ICL_CSR_MAX_FW_SIZE		0x5FFF

Err, just noticed that both the original and the new ones have
an off-by-one error, could you still fix them while at it?
(Correctly 0x3000, 0x4000, 0x6000 see 'Display CSR Program' in
BSpec). With that and asssuming the description for the fw offset
field will get clarified in BSpec:

Reviewed-by: Imre Deak <imre.deak@intel.com>

>  #define CSR_DEFAULT_FW_OFFSET		0xFFFFFFFF
>  
>  struct intel_css_header {
> @@ -279,6 +281,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>  	struct intel_csr *csr = &dev_priv->csr;
>  	const struct stepping_info *si = intel_get_stepping_info(dev_priv);
>  	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
> +	uint32_t max_fw_size = 0;
>  	uint32_t i;
>  	uint32_t *dmc_payload;
>  	uint32_t required_version;
> @@ -359,6 +362,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>  			  si->stepping);
>  		return NULL;
>  	}
> +	/* Convert dmc_offset into number of bytes. By default it is in dwords*/
> +	dmc_offset *= 4;
>  	readcount += dmc_offset;
>  
>  	/* Extract dmc_header information. */
> @@ -391,8 +396,16 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>  
>  	/* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
>  	nbytes = dmc_header->fw_size * 4;
> -	if (nbytes > CSR_MAX_FW_SIZE) {
> -		DRM_ERROR("DMC firmware too big (%u bytes)\n", nbytes);
> +	if (INTEL_GEN(dev_priv) >= 11)
> +		max_fw_size = ICL_CSR_MAX_FW_SIZE;
> +	else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> +		max_fw_size = GLK_CSR_MAX_FW_SIZE;
> +	else if (IS_GEN9(dev_priv)) 
> +		max_fw_size = BXT_CSR_MAX_FW_SIZE;
> +	else
> +		MISSING_CASE(INTEL_REVID(dev_priv));
> +	if (nbytes > max_fw_size) {
> +		DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
>  		return NULL;
>  	}
>  	csr->dmc_fw_size = dmc_header->fw_size;
> -- 
> 1.9.1
>
Yadav, Jyoti R Aug. 29, 2018, 10:18 a.m. UTC | #2
Hi Imre,

I also checked in Bspec. Good catch:)

FW size = (End address - Start Address) + 1

Will update the patch also. Thanks for quickly reviewing the patch.

Regards

Jyoti
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 1ec4f09..d12051b 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -55,7 +55,9 @@ 
 #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
 
 
-#define CSR_MAX_FW_SIZE			0x2FFF
+#define BXT_CSR_MAX_FW_SIZE		0x2FFF
+#define GLK_CSR_MAX_FW_SIZE		0x3FFF
+#define ICL_CSR_MAX_FW_SIZE		0x5FFF
 #define CSR_DEFAULT_FW_OFFSET		0xFFFFFFFF
 
 struct intel_css_header {
@@ -279,6 +281,7 @@  static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
 	struct intel_csr *csr = &dev_priv->csr;
 	const struct stepping_info *si = intel_get_stepping_info(dev_priv);
 	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
+	uint32_t max_fw_size = 0;
 	uint32_t i;
 	uint32_t *dmc_payload;
 	uint32_t required_version;
@@ -359,6 +362,8 @@  static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
 			  si->stepping);
 		return NULL;
 	}
+	/* Convert dmc_offset into number of bytes. By default it is in dwords*/
+	dmc_offset *= 4;
 	readcount += dmc_offset;
 
 	/* Extract dmc_header information. */
@@ -391,8 +396,16 @@  static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
 
 	/* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
 	nbytes = dmc_header->fw_size * 4;
-	if (nbytes > CSR_MAX_FW_SIZE) {
-		DRM_ERROR("DMC firmware too big (%u bytes)\n", nbytes);
+	if (INTEL_GEN(dev_priv) >= 11)
+		max_fw_size = ICL_CSR_MAX_FW_SIZE;
+	else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+		max_fw_size = GLK_CSR_MAX_FW_SIZE;
+	else if (IS_GEN9(dev_priv)) 
+		max_fw_size = BXT_CSR_MAX_FW_SIZE;
+	else
+		MISSING_CASE(INTEL_REVID(dev_priv));
+	if (nbytes > max_fw_size) {
+		DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
 		return NULL;
 	}
 	csr->dmc_fw_size = dmc_header->fw_size;