diff mbox series

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

Message ID 1535467496-2889-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, 2:44 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.

Also there is no such restriction on DMC FW size. So removing that check also.
---
 drivers/gpu/drm/i915/intel_csr.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Imre Deak Aug. 28, 2018, 3:26 p.m. UTC | #1
On Tue, Aug 28, 2018 at 10:44:56AM -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.
> 
> Also there is no such restriction on DMC FW size. So removing that check also.
> ---
>  drivers/gpu/drm/i915/intel_csr.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 1ec4f09..4590796 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -359,6 +359,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;

Yes, this looks correct based on the latest ICL binary. Not sure if
we ever had a binary with multiple FWs in it. BSpec says about this
offset:

"FW offset within the package"

while for other length fields it states explicitly if they are in
dwords. So this would need to be clarified in BSpec, could you file a
request for update there?

>  	readcount += dmc_offset;
>  
>  	/* Extract dmc_header information. */
> @@ -391,10 +393,6 @@ 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);
> -		return NULL;
> -	}

There is a maximum size defined, we just didn't update it past GEN9:

GLK, CNL: 0x3FFF
GEN11+  : 0x5FFF

could you please update the check accordingly?

>  	csr->dmc_fw_size = dmc_header->fw_size;
>  
>  	dmc_payload = kmalloc(nbytes, GFP_KERNEL);
> -- 
> 1.9.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 1ec4f09..4590796 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -359,6 +359,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,10 +393,6 @@  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);
-		return NULL;
-	}
 	csr->dmc_fw_size = dmc_header->fw_size;
 
 	dmc_payload = kmalloc(nbytes, GFP_KERNEL);