[PATCH] [intel-gfx] drm/i915/intel_csr.c Fix DMC FW Loading issue on ICL.

[PATCH] [intel-gfx] drm/i915/intel_csr.c Fix DMC FW Loading issue on ICL.

[Jyoti Yadav]

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)

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

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 1ec4f09..3f78a3e 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_CNL_CSR_MAX_FW_SIZE		0x3FFF
+#define ICL_CSR_MAX_FW_SIZE		0x5FFF
 #define CSR_DEFAULT_FW_OFFSET		0xFFFFFFFF
 
 struct intel_css_header {
@@ -359,6 +361,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,9 +395,21 @@ 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;
+	if (IS_BROXTON(dev_priv)) {
+		if (nbytes > BXT_CSR_MAX_FW_SIZE) {
+			DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
+			return NULL;
+		}
+	} else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
+		if (nbytes > GLK_CNL_CSR_MAX_FW_SIZE) {
+			DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
+			return NULL;
+		}
+	} else {
+		if (nbytes > ICL_CSR_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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] [intel-gfx] drm/i915/intel_csr.c Fix DMC FW Loading issue on ICL.

[Imre Deak]

On Tue, Aug 28, 2018 at 12:54:13PM -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)
> 
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_csr.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 1ec4f09..3f78a3e 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_CNL_CSR_MAX_FW_SIZE		0x3FFF
> +#define ICL_CSR_MAX_FW_SIZE		0x5FFF
>  #define CSR_DEFAULT_FW_OFFSET		0xFFFFFFFF
>  
>  struct intel_css_header {
> @@ -359,6 +361,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,9 +395,21 @@ 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;
> +	if (IS_BROXTON(dev_priv)) {
> +		if (nbytes > BXT_CSR_MAX_FW_SIZE) {
> +			DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
> +			return NULL;
> +		}
> +	} else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> +		if (nbytes > GLK_CNL_CSR_MAX_FW_SIZE) {
> +			DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
> +			return NULL;
> +		}
> +	} else {
> +		if (nbytes > ICL_CSR_MAX_FW_SIZE) {
> +			DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
> +			return NULL;
> +		}

To account for the rest of GEN9 platforms and avoid some duplication we'd
need something like the following (just using the GLK_CSR prefix, it's
enough to mark the first relevant platform):

+	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 > CSR_MAX_FW_SIZE) {
+	if (nbytes > max_fw_size) {

>  	}
>  	csr->dmc_fw_size = dmc_header->fw_size;
>  
> -- 
> 1.9.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] [intel-gfx] drm/i915/intel_csr.c Fix DMC FW Loading issue on ICL.

[Srivatsa, Anusha]

>-----Original Message-----
>From: Deak, Imre
>Sent: Tuesday, August 28, 2018 10:16 AM
>To: Yadav, Jyoti R <jyoti.r.yadav@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Srivatsa, Anusha
><anusha.srivatsa@intel.com>; Saarinen, Jani <jani.saarinen@intel.com>
>Subject: Re: [PATCH] [intel-gfx] drm/i915/intel_csr.c Fix DMC FW Loading issue
>on ICL.
>
>On Tue, Aug 28, 2018 at 12:54:13PM -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
s/number/number	

>> correct DMC FW from correct place.
>>
>> v2 : Added check for DMC FW max size for various gen. (Imre Deak)
>>
>> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_csr.c | 24 ++++++++++++++++++++----
>>  1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_csr.c
>> b/drivers/gpu/drm/i915/intel_csr.c
>> index 1ec4f09..3f78a3e 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_CNL_CSR_MAX_FW_SIZE		0x3FFF
>> +#define ICL_CSR_MAX_FW_SIZE		0x5FFF
>>  #define CSR_DEFAULT_FW_OFFSET		0xFFFFFFFF
>>
>>  struct intel_css_header {
>> @@ -359,6 +361,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,9 +395,21 @@ 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;
>> +	if (IS_BROXTON(dev_priv)) {
>> +		if (nbytes > BXT_CSR_MAX_FW_SIZE) {
>> +			DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
>> +			return NULL;
>> +		}
>> +	} else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
>> +		if (nbytes > GLK_CNL_CSR_MAX_FW_SIZE) {
>> +			DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
>> +			return NULL;
>> +		}
>> +	} else {
>> +		if (nbytes > ICL_CSR_MAX_FW_SIZE) {
>> +			DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
>> +			return NULL;
>> +		}
>
>To account for the rest of GEN9 platforms and avoid some duplication we'd need
>something like the following (just using the GLK_CSR prefix, it's enough to mark
>the first relevant platform):
I agree with imre here.

Anusha
>
>+	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 > CSR_MAX_FW_SIZE) {
>+	if (nbytes > max_fw_size) {
>
>>  	}
>>  	csr->dmc_fw_size = dmc_header->fw_size;
>>
>> --
>> 1.9.1
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for drm/i915/intel_csr.c Fix DMC FW Loading issue on ICL. (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/intel_csr.c Fix DMC FW Loading issue on ICL. (rev2)
URL   : https://patchwork.freedesktop.org/series/48803/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
c6afa54697da drm/i915/intel_csr.c Fix DMC FW Loading issue on ICL.
-:10: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#10: 
So while reading the dmc_offset from package header, for 1st stepping offset

-:68: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Jyoti <jyoti.r.yadav@intel.com>'

total: 0 errors, 2 warnings, 0 checks, 42 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/intel_csr.c Fix DMC FW Loading issue on ICL. (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/intel_csr.c Fix DMC FW Loading issue on ICL. (rev2)
URL   : https://patchwork.freedesktop.org/series/48803/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4718 -> Patchwork_10034 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/48803/revisions/2/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10034 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_module_reload@basic-reload:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      {fi-cfl-8109u}:     DMESG-FAIL (fdo#106560) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    {igt@pm_rpm@module-reload}:
      fi-cnl-psr:         WARN (fdo#107708, fdo#107602) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#107602 https://bugs.freedesktop.org/show_bug.cgi?id=107602
  fdo#107708 https://bugs.freedesktop.org/show_bug.cgi?id=107708
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718


== Participating hosts (54 -> 47) ==

  Missing    (7): fi-hsw-4770r fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4718 -> Patchwork_10034

  CI_DRM_4718: c7398fd19cef9b11c79af7292109507b6be075c4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4611: b966dd93a30f41581fe1dbf9bc1c4a29b552ca05 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10034: c6afa54697daa0412b0c485c6633208e9e26d270 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

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

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10034/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] [intel-gfx] drm/i915/intel_csr.c Fix DMC FW Loading issue on ICL.

[Rodrigo Vivi]

On Tue, Aug 28, 2018 at 12:54:13PM -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

s/numaber/number

> 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)
> 
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_csr.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 1ec4f09..3f78a3e 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_CNL_CSR_MAX_FW_SIZE		0x3FFF
> +#define ICL_CSR_MAX_FW_SIZE		0x5FFF
>  #define CSR_DEFAULT_FW_OFFSET		0xFFFFFFFF
>  
>  struct intel_css_header {
> @@ -359,6 +361,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;

I'm kind of confused here. Is it true for all platforms and all steppings?

>  	readcount += dmc_offset;
>  
>  	/* Extract dmc_header information. */
> @@ -391,9 +395,21 @@ 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;
> +	if (IS_BROXTON(dev_priv)) {
> +		if (nbytes > BXT_CSR_MAX_FW_SIZE) {
> +			DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
> +			return NULL;
> +		}
> +	} else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> +		if (nbytes > GLK_CNL_CSR_MAX_FW_SIZE) {
> +			DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
> +			return NULL;
> +		}
> +	} else {
> +		if (nbytes > ICL_CSR_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
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx