[PATCH] drm/i915/dmc: Step away from symbolic links

[PATCH] drm/i915/dmc: Step away from symbolic links

[Patrik Jakobsson]

Load specific firmware versions for the DMC instead of using symbolic
links. The currently recommended versions are: SKL 1.26, KBL 1.01 and
BXT 1.07.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 2b3b428..ea047cd 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -41,15 +41,15 @@
  * be moved to FW_FAILED.
  */
 
-#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
+#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
 MODULE_FIRMWARE(I915_CSR_KBL);
 #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 1)
 
-#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
+#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
 MODULE_FIRMWARE(I915_CSR_SKL);
-#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 23)
+#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 26)
 
-#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
+#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
 MODULE_FIRMWARE(I915_CSR_BXT);
 #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
 
@@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
 	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
 	uint32_t i;
 	uint32_t *dmc_payload;
-	uint32_t required_min_version;
+	uint32_t required_version;
 
 	if (!fw)
 		return NULL;
@@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
 	csr->version = css_header->version;
 
 	if (IS_KABYLAKE(dev_priv)) {
-		required_min_version = KBL_CSR_VERSION_REQUIRED;
+		required_version = KBL_CSR_VERSION_REQUIRED;
 	} else if (IS_SKYLAKE(dev_priv)) {
-		required_min_version = SKL_CSR_VERSION_REQUIRED;
+		required_version = SKL_CSR_VERSION_REQUIRED;
 	} else if (IS_BROXTON(dev_priv)) {
-		required_min_version = BXT_CSR_VERSION_REQUIRED;
+		required_version = BXT_CSR_VERSION_REQUIRED;
 	} else {
 		MISSING_CASE(INTEL_REVID(dev_priv));
-		required_min_version = 0;
+		required_version = 0;
 	}
 
-	if (csr->version < required_min_version) {
-		DRM_INFO("Refusing to load old DMC firmware v%u.%u,"
-			 " please upgrade to v%u.%u or later"
-			   " [" FIRMWARE_URL "].\n",
+	if (csr->version != required_version) {
+		DRM_INFO("Refusing to load DMC firmware v%u.%u,"
+			 " please use v%u.%u [" FIRMWARE_URL "].\n",
 			 CSR_VERSION_MAJOR(csr->version),
 			 CSR_VERSION_MINOR(csr->version),
-			 CSR_VERSION_MAJOR(required_min_version),
-			 CSR_VERSION_MINOR(required_min_version));
+			 CSR_VERSION_MAJOR(required_version),
+			 CSR_VERSION_MINOR(required_version));
 		return NULL;
 	}
 
-- 
2.5.0

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

✗ Ro.CI.BAT: failure for drm/i915/dmc: Step away from symbolic links

[Patchwork]

== Series Details ==

Series: drm/i915/dmc: Step away from symbolic links
URL   : https://patchwork.freedesktop.org/series/6966/
State : failure

== Summary ==

Series 6966v1 drm/i915/dmc: Step away from symbolic links
http://patchwork.freedesktop.org/api/1.0/series/6966/revisions/1/mbox

Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> FAIL       (ro-ilk1-i5-650)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                fail       -> PASS       (ro-byt-n2820)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (ro-ivb2-i7-3770)
Test kms_sink_crc_basic:
                pass       -> SKIP       (ro-skl-i7-6700hq)

ro-bdw-i5-5250u  total:219  pass:202  dwarn:4   dfail:0   fail:0   skip:13 
ro-bdw-i7-5600u  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-bsw-n3050     total:219  pass:175  dwarn:0   dfail:0   fail:2   skip:42 
ro-byt-n2820     total:218  pass:175  dwarn:0   dfail:0   fail:2   skip:41 
ro-hsw-i3-4010u  total:218  pass:193  dwarn:0   dfail:0   fail:0   skip:25 
ro-hsw-i7-4770r  total:219  pass:194  dwarn:0   dfail:0   fail:0   skip:25 
ro-ilk-i7-620lm  total:219  pass:151  dwarn:0   dfail:0   fail:1   skip:67 
ro-ilk1-i5-650   total:214  pass:151  dwarn:0   dfail:0   fail:2   skip:61 
ro-ivb-i7-3770   total:219  pass:183  dwarn:0   dfail:0   fail:0   skip:36 
ro-ivb2-i7-3770  total:219  pass:186  dwarn:0   dfail:0   fail:1   skip:32 
ro-skl-i7-6700hq total:214  pass:189  dwarn:0   dfail:0   fail:0   skip:25 
ro-snb-i7-2620M  total:219  pass:177  dwarn:0   dfail:0   fail:1   skip:41 
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_845/

2d4abf3 drm-intel-nightly: 2016y-05m-10d-09h-36m-54s UTC integration manifest
5254ec7 drm/i915/dmc: Step away from symbolic links

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

Re: [PATCH] drm/i915/dmc: Step away from symbolic links

[Mika Kuoppala]

Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes:

> [ text/plain ]
> Load specific firmware versions for the DMC instead of using symbolic
> links. The currently recommended versions are: SKL 1.26, KBL 1.01 and
> BXT 1.07.
>

We should augment the commit message to answer the 'why' part.

Otherwise, looks good.

-Mika


> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 2b3b428..ea047cd 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -41,15 +41,15 @@
>   * be moved to FW_FAILED.
>   */
>  
> -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
> +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
>  MODULE_FIRMWARE(I915_CSR_KBL);
>  #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 1)
>  
> -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
> +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
>  MODULE_FIRMWARE(I915_CSR_SKL);
> -#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 23)
> +#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 26)
>  
> -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
> +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
>  MODULE_FIRMWARE(I915_CSR_BXT);
>  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
>  
> @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>  	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
>  	uint32_t i;
>  	uint32_t *dmc_payload;
> -	uint32_t required_min_version;
> +	uint32_t required_version;
>  
>  	if (!fw)
>  		return NULL;
> @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>  	csr->version = css_header->version;
>  
>  	if (IS_KABYLAKE(dev_priv)) {
> -		required_min_version = KBL_CSR_VERSION_REQUIRED;
> +		required_version = KBL_CSR_VERSION_REQUIRED;
>  	} else if (IS_SKYLAKE(dev_priv)) {
> -		required_min_version = SKL_CSR_VERSION_REQUIRED;
> +		required_version = SKL_CSR_VERSION_REQUIRED;
>  	} else if (IS_BROXTON(dev_priv)) {
> -		required_min_version = BXT_CSR_VERSION_REQUIRED;
> +		required_version = BXT_CSR_VERSION_REQUIRED;
>  	} else {
>  		MISSING_CASE(INTEL_REVID(dev_priv));
> -		required_min_version = 0;
> +		required_version = 0;
>  	}
>  
> -	if (csr->version < required_min_version) {
> -		DRM_INFO("Refusing to load old DMC firmware v%u.%u,"
> -			 " please upgrade to v%u.%u or later"
> -			   " [" FIRMWARE_URL "].\n",
> +	if (csr->version != required_version) {
> +		DRM_INFO("Refusing to load DMC firmware v%u.%u,"
> +			 " please use v%u.%u [" FIRMWARE_URL "].\n",
>  			 CSR_VERSION_MAJOR(csr->version),
>  			 CSR_VERSION_MINOR(csr->version),
> -			 CSR_VERSION_MAJOR(required_min_version),
> -			 CSR_VERSION_MINOR(required_min_version));
> +			 CSR_VERSION_MAJOR(required_version),
> +			 CSR_VERSION_MINOR(required_version));
>  		return NULL;
>  	}
>  
> -- 
> 2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dmc: Step away from symbolic links

[Patrik Jakobsson]

On Tue, May 10, 2016 at 03:52:02PM +0300, Mika Kuoppala wrote:
> Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes:
> 
> > [ text/plain ]
> > Load specific firmware versions for the DMC instead of using symbolic
> > links. The currently recommended versions are: SKL 1.26, KBL 1.01 and
> > BXT 1.07.
> >
> 
> We should augment the commit message to answer the 'why' part.
> 
> Otherwise, looks good.

Yes I agree. Where did this discussion take place? I don't believe I was
part of it. Rodrigo told me we've reached consensus on the decision but
that's all I know.

-Patrik

> 
> -Mika
> 
> 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++---------------
> >  1 file changed, 14 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> > index 2b3b428..ea047cd 100644
> > --- a/drivers/gpu/drm/i915/intel_csr.c
> > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > @@ -41,15 +41,15 @@
> >   * be moved to FW_FAILED.
> >   */
> >  
> > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
> > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
> >  MODULE_FIRMWARE(I915_CSR_KBL);
> >  #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 1)
> >  
> > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
> > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
> >  MODULE_FIRMWARE(I915_CSR_SKL);
> > -#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 23)
> > +#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 26)
> >  
> > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
> > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
> >  MODULE_FIRMWARE(I915_CSR_BXT);
> >  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
> >  
> > @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
> >  	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
> >  	uint32_t i;
> >  	uint32_t *dmc_payload;
> > -	uint32_t required_min_version;
> > +	uint32_t required_version;
> >  
> >  	if (!fw)
> >  		return NULL;
> > @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
> >  	csr->version = css_header->version;
> >  
> >  	if (IS_KABYLAKE(dev_priv)) {
> > -		required_min_version = KBL_CSR_VERSION_REQUIRED;
> > +		required_version = KBL_CSR_VERSION_REQUIRED;
> >  	} else if (IS_SKYLAKE(dev_priv)) {
> > -		required_min_version = SKL_CSR_VERSION_REQUIRED;
> > +		required_version = SKL_CSR_VERSION_REQUIRED;
> >  	} else if (IS_BROXTON(dev_priv)) {
> > -		required_min_version = BXT_CSR_VERSION_REQUIRED;
> > +		required_version = BXT_CSR_VERSION_REQUIRED;
> >  	} else {
> >  		MISSING_CASE(INTEL_REVID(dev_priv));
> > -		required_min_version = 0;
> > +		required_version = 0;
> >  	}
> >  
> > -	if (csr->version < required_min_version) {
> > -		DRM_INFO("Refusing to load old DMC firmware v%u.%u,"
> > -			 " please upgrade to v%u.%u or later"
> > -			   " [" FIRMWARE_URL "].\n",
> > +	if (csr->version != required_version) {
> > +		DRM_INFO("Refusing to load DMC firmware v%u.%u,"
> > +			 " please use v%u.%u [" FIRMWARE_URL "].\n",
> >  			 CSR_VERSION_MAJOR(csr->version),
> >  			 CSR_VERSION_MINOR(csr->version),
> > -			 CSR_VERSION_MAJOR(required_min_version),
> > -			 CSR_VERSION_MINOR(required_min_version));
> > +			 CSR_VERSION_MAJOR(required_version),
> > +			 CSR_VERSION_MINOR(required_version));
> >  		return NULL;
> >  	}
> >  
> > -- 
> > 2.5.0

-- 
Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dmc: Step away from symbolic links

[Jani Nikula]

On Tue, 10 May 2016, Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> On Tue, May 10, 2016 at 03:52:02PM +0300, Mika Kuoppala wrote:
>> Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes:
>> 
>> > [ text/plain ]
>> > Load specific firmware versions for the DMC instead of using symbolic
>> > links. The currently recommended versions are: SKL 1.26, KBL 1.01 and
>> > BXT 1.07.
>> >
>> 
>> We should augment the commit message to answer the 'why' part.
>> 
>> Otherwise, looks good.
>
> Yes I agree. Where did this discussion take place? I don't believe I was
> part of it. Rodrigo told me we've reached consensus on the decision but
> that's all I know.

This captures most of it I think
http://mid.gmane.org/87fuu6i0zb.fsf@intel.com


BR,
Jani.


>
> -Patrik
>
>> 
>> -Mika
>> 
>> 
>> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > Cc: Imre Deak <imre.deak@intel.com>
>> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++---------------
>> >  1 file changed, 14 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>> > index 2b3b428..ea047cd 100644
>> > --- a/drivers/gpu/drm/i915/intel_csr.c
>> > +++ b/drivers/gpu/drm/i915/intel_csr.c
>> > @@ -41,15 +41,15 @@
>> >   * be moved to FW_FAILED.
>> >   */
>> >  
>> > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
>> > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
>> >  MODULE_FIRMWARE(I915_CSR_KBL);
>> >  #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 1)
>> >  
>> > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
>> > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
>> >  MODULE_FIRMWARE(I915_CSR_SKL);
>> > -#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 23)
>> > +#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 26)
>> >  
>> > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
>> > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
>> >  MODULE_FIRMWARE(I915_CSR_BXT);
>> >  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
>> >  
>> > @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>> >  	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
>> >  	uint32_t i;
>> >  	uint32_t *dmc_payload;
>> > -	uint32_t required_min_version;
>> > +	uint32_t required_version;
>> >  
>> >  	if (!fw)
>> >  		return NULL;
>> > @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>> >  	csr->version = css_header->version;
>> >  
>> >  	if (IS_KABYLAKE(dev_priv)) {
>> > -		required_min_version = KBL_CSR_VERSION_REQUIRED;
>> > +		required_version = KBL_CSR_VERSION_REQUIRED;
>> >  	} else if (IS_SKYLAKE(dev_priv)) {
>> > -		required_min_version = SKL_CSR_VERSION_REQUIRED;
>> > +		required_version = SKL_CSR_VERSION_REQUIRED;
>> >  	} else if (IS_BROXTON(dev_priv)) {
>> > -		required_min_version = BXT_CSR_VERSION_REQUIRED;
>> > +		required_version = BXT_CSR_VERSION_REQUIRED;
>> >  	} else {
>> >  		MISSING_CASE(INTEL_REVID(dev_priv));
>> > -		required_min_version = 0;
>> > +		required_version = 0;
>> >  	}
>> >  
>> > -	if (csr->version < required_min_version) {
>> > -		DRM_INFO("Refusing to load old DMC firmware v%u.%u,"
>> > -			 " please upgrade to v%u.%u or later"
>> > -			   " [" FIRMWARE_URL "].\n",
>> > +	if (csr->version != required_version) {
>> > +		DRM_INFO("Refusing to load DMC firmware v%u.%u,"
>> > +			 " please use v%u.%u [" FIRMWARE_URL "].\n",
>> >  			 CSR_VERSION_MAJOR(csr->version),
>> >  			 CSR_VERSION_MINOR(csr->version),
>> > -			 CSR_VERSION_MAJOR(required_min_version),
>> > -			 CSR_VERSION_MINOR(required_min_version));
>> > +			 CSR_VERSION_MAJOR(required_version),
>> > +			 CSR_VERSION_MINOR(required_version));
>> >  		return NULL;
>> >  	}
>> >  
>> > -- 
>> > 2.5.0

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dmc: Step away from symbolic links

[Mika Kuoppala]

Jani Nikula <jani.nikula@linux.intel.com> writes:

> [ text/plain ]
> On Tue, 10 May 2016, Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
>> On Tue, May 10, 2016 at 03:52:02PM +0300, Mika Kuoppala wrote:
>>> Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes:
>>> 
>>> > [ text/plain ]
>>> > Load specific firmware versions for the DMC instead of using symbolic
>>> > links. The currently recommended versions are: SKL 1.26, KBL 1.01 and
>>> > BXT 1.07.
>>> >
>>> 
>>> We should augment the commit message to answer the 'why' part.
>>> 
>>> Otherwise, looks good.
>>
>> Yes I agree. Where did this discussion take place? I don't believe I was
>> part of it. Rodrigo told me we've reached consensus on the decision but
>> that's all I know.
>
> This captures most of it I think
> http://mid.gmane.org/87fuu6i0zb.fsf@intel.com
>

Pretty much. In essence we want to offer a combination
with somekind of proof (read tested) that it works. And lock
that combination. Think about it as a colored seal paint on screw
heads on some products: these were tested and delivered together.
You can unscrew and dissassemble the parts still with minimal effort
and mix the pieces, but they may or may not work as planned if you do so.

And if you broke the seal paint and get a black screen, you know
who to blame.

-Mika

>
> BR,
> Jani.
>
>
>>
>> -Patrik
>>
>>> 
>>> -Mika
>>> 
>>> 
>>> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> > Cc: Imre Deak <imre.deak@intel.com>
>>> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>>> > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
>>> > ---
>>> >  drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++---------------
>>> >  1 file changed, 14 insertions(+), 15 deletions(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>>> > index 2b3b428..ea047cd 100644
>>> > --- a/drivers/gpu/drm/i915/intel_csr.c
>>> > +++ b/drivers/gpu/drm/i915/intel_csr.c
>>> > @@ -41,15 +41,15 @@
>>> >   * be moved to FW_FAILED.
>>> >   */
>>> >  
>>> > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
>>> > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
>>> >  MODULE_FIRMWARE(I915_CSR_KBL);
>>> >  #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 1)
>>> >  
>>> > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
>>> > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
>>> >  MODULE_FIRMWARE(I915_CSR_SKL);
>>> > -#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 23)
>>> > +#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 26)
>>> >  
>>> > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
>>> > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
>>> >  MODULE_FIRMWARE(I915_CSR_BXT);
>>> >  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
>>> >  
>>> > @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>>> >  	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
>>> >  	uint32_t i;
>>> >  	uint32_t *dmc_payload;
>>> > -	uint32_t required_min_version;
>>> > +	uint32_t required_version;
>>> >  
>>> >  	if (!fw)
>>> >  		return NULL;
>>> > @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>>> >  	csr->version = css_header->version;
>>> >  
>>> >  	if (IS_KABYLAKE(dev_priv)) {
>>> > -		required_min_version = KBL_CSR_VERSION_REQUIRED;
>>> > +		required_version = KBL_CSR_VERSION_REQUIRED;
>>> >  	} else if (IS_SKYLAKE(dev_priv)) {
>>> > -		required_min_version = SKL_CSR_VERSION_REQUIRED;
>>> > +		required_version = SKL_CSR_VERSION_REQUIRED;
>>> >  	} else if (IS_BROXTON(dev_priv)) {
>>> > -		required_min_version = BXT_CSR_VERSION_REQUIRED;
>>> > +		required_version = BXT_CSR_VERSION_REQUIRED;
>>> >  	} else {
>>> >  		MISSING_CASE(INTEL_REVID(dev_priv));
>>> > -		required_min_version = 0;
>>> > +		required_version = 0;
>>> >  	}
>>> >  
>>> > -	if (csr->version < required_min_version) {
>>> > -		DRM_INFO("Refusing to load old DMC firmware v%u.%u,"
>>> > -			 " please upgrade to v%u.%u or later"
>>> > -			   " [" FIRMWARE_URL "].\n",
>>> > +	if (csr->version != required_version) {
>>> > +		DRM_INFO("Refusing to load DMC firmware v%u.%u,"
>>> > +			 " please use v%u.%u [" FIRMWARE_URL "].\n",
>>> >  			 CSR_VERSION_MAJOR(csr->version),
>>> >  			 CSR_VERSION_MINOR(csr->version),
>>> > -			 CSR_VERSION_MAJOR(required_min_version),
>>> > -			 CSR_VERSION_MINOR(required_min_version));
>>> > +			 CSR_VERSION_MAJOR(required_version),
>>> > +			 CSR_VERSION_MINOR(required_version));
>>> >  		return NULL;
>>> >  	}
>>> >  
>>> > -- 
>>> > 2.5.0
>
> -- 
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915/dmc: Step away from symbolic links

[Patrik Jakobsson]

Load specific firmware versions for the DMC instead of using symbolic
links. The currently recommended versions are: SKL 1.26, KBL 1.01 and
BXT 1.07.

Certain DMC versions need workarounds in the driver which forces us to
have a tight dependency between firmware and driver. In order to be able
to provide a tested and known working configuration we must lock down on
a specific DMC firmware version.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 2b3b428..ea047cd 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -41,15 +41,15 @@
  * be moved to FW_FAILED.
  */
 
-#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
+#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
 MODULE_FIRMWARE(I915_CSR_KBL);
 #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 1)
 
-#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
+#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
 MODULE_FIRMWARE(I915_CSR_SKL);
-#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 23)
+#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 26)
 
-#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
+#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
 MODULE_FIRMWARE(I915_CSR_BXT);
 #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
 
@@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
 	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
 	uint32_t i;
 	uint32_t *dmc_payload;
-	uint32_t required_min_version;
+	uint32_t required_version;
 
 	if (!fw)
 		return NULL;
@@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
 	csr->version = css_header->version;
 
 	if (IS_KABYLAKE(dev_priv)) {
-		required_min_version = KBL_CSR_VERSION_REQUIRED;
+		required_version = KBL_CSR_VERSION_REQUIRED;
 	} else if (IS_SKYLAKE(dev_priv)) {
-		required_min_version = SKL_CSR_VERSION_REQUIRED;
+		required_version = SKL_CSR_VERSION_REQUIRED;
 	} else if (IS_BROXTON(dev_priv)) {
-		required_min_version = BXT_CSR_VERSION_REQUIRED;
+		required_version = BXT_CSR_VERSION_REQUIRED;
 	} else {
 		MISSING_CASE(INTEL_REVID(dev_priv));
-		required_min_version = 0;
+		required_version = 0;
 	}
 
-	if (csr->version < required_min_version) {
-		DRM_INFO("Refusing to load old DMC firmware v%u.%u,"
-			 " please upgrade to v%u.%u or later"
-			   " [" FIRMWARE_URL "].\n",
+	if (csr->version != required_version) {
+		DRM_INFO("Refusing to load DMC firmware v%u.%u,"
+			 " please use v%u.%u [" FIRMWARE_URL "].\n",
 			 CSR_VERSION_MAJOR(csr->version),
 			 CSR_VERSION_MINOR(csr->version),
-			 CSR_VERSION_MAJOR(required_min_version),
-			 CSR_VERSION_MINOR(required_min_version));
+			 CSR_VERSION_MAJOR(required_version),
+			 CSR_VERSION_MINOR(required_version));
 		return NULL;
 	}
 
-- 
2.5.0

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

Re: [PATCH v2] drm/i915/dmc: Step away from symbolic links

[Jani Nikula]

On Mon, 16 May 2016, Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> Load specific firmware versions for the DMC instead of using symbolic
> links. The currently recommended versions are: SKL 1.26, KBL 1.01 and
> BXT 1.07.
>
> Certain DMC versions need workarounds in the driver which forces us to
> have a tight dependency between firmware and driver. In order to be able
> to provide a tested and known working configuration we must lock down on
> a specific DMC firmware version.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

On the general idea,

Acked-by: Jani Nikula <jani.nikula@intel.com>

I guess we'll need to keep the symbolic links around in linux-firmware,
pointing to known good firmware versions, and *not* update the links
anymore.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/dmc: Step away from symbolic links

[Mika Kuoppala]

Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes:

> [ text/plain ]
> Load specific firmware versions for the DMC instead of using symbolic
> links. The currently recommended versions are: SKL 1.26, KBL 1.01 and
> BXT 1.07.
>
> Certain DMC versions need workarounds in the driver which forces us to
> have a tight dependency between firmware and driver. In order to be able
> to provide a tested and known working configuration we must lock down on
> a specific DMC firmware version.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

We need ack from Rodrigo and/or whomever is handling
the fw releasing side.

-Mika

> ---
>  drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 2b3b428..ea047cd 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -41,15 +41,15 @@
>   * be moved to FW_FAILED.
>   */
>  
> -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
> +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
>  MODULE_FIRMWARE(I915_CSR_KBL);
>  #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 1)
>  
> -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
> +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
>  MODULE_FIRMWARE(I915_CSR_SKL);
> -#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 23)
> +#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 26)
>  
> -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
> +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
>  MODULE_FIRMWARE(I915_CSR_BXT);
>  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
>  
> @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>  	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
>  	uint32_t i;
>  	uint32_t *dmc_payload;
> -	uint32_t required_min_version;
> +	uint32_t required_version;
>  
>  	if (!fw)
>  		return NULL;
> @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>  	csr->version = css_header->version;
>  
>  	if (IS_KABYLAKE(dev_priv)) {
> -		required_min_version = KBL_CSR_VERSION_REQUIRED;
> +		required_version = KBL_CSR_VERSION_REQUIRED;
>  	} else if (IS_SKYLAKE(dev_priv)) {
> -		required_min_version = SKL_CSR_VERSION_REQUIRED;
> +		required_version = SKL_CSR_VERSION_REQUIRED;
>  	} else if (IS_BROXTON(dev_priv)) {
> -		required_min_version = BXT_CSR_VERSION_REQUIRED;
> +		required_version = BXT_CSR_VERSION_REQUIRED;
>  	} else {
>  		MISSING_CASE(INTEL_REVID(dev_priv));
> -		required_min_version = 0;
> +		required_version = 0;
>  	}
>  
> -	if (csr->version < required_min_version) {
> -		DRM_INFO("Refusing to load old DMC firmware v%u.%u,"
> -			 " please upgrade to v%u.%u or later"
> -			   " [" FIRMWARE_URL "].\n",
> +	if (csr->version != required_version) {
> +		DRM_INFO("Refusing to load DMC firmware v%u.%u,"
> +			 " please use v%u.%u [" FIRMWARE_URL "].\n",
>  			 CSR_VERSION_MAJOR(csr->version),
>  			 CSR_VERSION_MINOR(csr->version),
> -			 CSR_VERSION_MAJOR(required_min_version),
> -			 CSR_VERSION_MINOR(required_min_version));
> +			 CSR_VERSION_MAJOR(required_version),
> +			 CSR_VERSION_MINOR(required_version));
>  		return NULL;
>  	}
>  
> -- 
> 2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/dmc: Step away from symbolic links

[Patrik Jakobsson]

On Wed, May 18, 2016 at 01:24:12PM +0300, Mika Kuoppala wrote:
> Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes:
> 
> > [ text/plain ]
> > Load specific firmware versions for the DMC instead of using symbolic
> > links. The currently recommended versions are: SKL 1.26, KBL 1.01 and
> > BXT 1.07.
> >
> > Certain DMC versions need workarounds in the driver which forces us to
> > have a tight dependency between firmware and driver. In order to be able
> > to provide a tested and known working configuration we must lock down on
> > a specific DMC firmware version.
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> We need ack from Rodrigo and/or whomever is handling
> the fw releasing side.
> 
> -Mika
> 

As discussed on IRC, Rodrigo is currently away but since he requested this
feature we indirectly have his ACK.

-Patrik

> > ---
> >  drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++---------------
> >  1 file changed, 14 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> > index 2b3b428..ea047cd 100644
> > --- a/drivers/gpu/drm/i915/intel_csr.c
> > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > @@ -41,15 +41,15 @@
> >   * be moved to FW_FAILED.
> >   */
> >  
> > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
> > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
> >  MODULE_FIRMWARE(I915_CSR_KBL);
> >  #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 1)
> >  
> > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
> > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
> >  MODULE_FIRMWARE(I915_CSR_SKL);
> > -#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 23)
> > +#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 26)
> >  
> > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
> > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
> >  MODULE_FIRMWARE(I915_CSR_BXT);
> >  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
> >  
> > @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
> >  	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
> >  	uint32_t i;
> >  	uint32_t *dmc_payload;
> > -	uint32_t required_min_version;
> > +	uint32_t required_version;
> >  
> >  	if (!fw)
> >  		return NULL;
> > @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
> >  	csr->version = css_header->version;
> >  
> >  	if (IS_KABYLAKE(dev_priv)) {
> > -		required_min_version = KBL_CSR_VERSION_REQUIRED;
> > +		required_version = KBL_CSR_VERSION_REQUIRED;
> >  	} else if (IS_SKYLAKE(dev_priv)) {
> > -		required_min_version = SKL_CSR_VERSION_REQUIRED;
> > +		required_version = SKL_CSR_VERSION_REQUIRED;
> >  	} else if (IS_BROXTON(dev_priv)) {
> > -		required_min_version = BXT_CSR_VERSION_REQUIRED;
> > +		required_version = BXT_CSR_VERSION_REQUIRED;
> >  	} else {
> >  		MISSING_CASE(INTEL_REVID(dev_priv));
> > -		required_min_version = 0;
> > +		required_version = 0;
> >  	}
> >  
> > -	if (csr->version < required_min_version) {
> > -		DRM_INFO("Refusing to load old DMC firmware v%u.%u,"
> > -			 " please upgrade to v%u.%u or later"
> > -			   " [" FIRMWARE_URL "].\n",
> > +	if (csr->version != required_version) {
> > +		DRM_INFO("Refusing to load DMC firmware v%u.%u,"
> > +			 " please use v%u.%u [" FIRMWARE_URL "].\n",
> >  			 CSR_VERSION_MAJOR(csr->version),
> >  			 CSR_VERSION_MINOR(csr->version),
> > -			 CSR_VERSION_MAJOR(required_min_version),
> > -			 CSR_VERSION_MINOR(required_min_version));
> > +			 CSR_VERSION_MAJOR(required_version),
> > +			 CSR_VERSION_MINOR(required_version));
> >  		return NULL;
> >  	}
> >  
> > -- 
> > 2.5.0

-- 
Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/dmc: Step away from symbolic links

[Vivi, Rodrigo]

On Mon, 2016-05-23 at 10:57 +0200, Patrik Jakobsson wrote:
> On Wed, May 18, 2016 at 01:24:12PM +0300, Mika Kuoppala wrote:
> > Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes:
> > 
> > > [ text/plain ]
> > > Load specific firmware versions for the DMC instead of using
> > > symbolic
> > > links. The currently recommended versions are: SKL 1.26, KBL 1.01
> > > and
> > > BXT 1.07.
> > > 
> > > Certain DMC versions need workarounds in the driver which forces
> > > us to
> > > have a tight dependency between firmware and driver. In order to
> > > be able
> > > to provide a tested and known working configuration we must lock
> > > down on
> > > a specific DMC firmware version.
> > > 
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com
> > > >
> > 
> > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> > 
> > We need ack from Rodrigo and/or whomever is handling
> > the fw releasing side.
> > 
> > -Mika
> > 
> 
> As discussed on IRC, Rodrigo is currently away but since he requested
> this
> feature we indirectly have his ACK.

indeed! ;)


Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> -Patrik
> 
> > > ---
> > >  drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++------------
> > > ---
> > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > > b/drivers/gpu/drm/i915/intel_csr.c
> > > index 2b3b428..ea047cd 100644
> > > --- a/drivers/gpu/drm/i915/intel_csr.c
> > > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > > @@ -41,15 +41,15 @@
> > >   * be moved to FW_FAILED.
> > >   */
> > >  
> > > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
> > > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
> > >  MODULE_FIRMWARE(I915_CSR_KBL);
> > >  #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 1)
> > >  
> > > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
> > > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
> > >  MODULE_FIRMWARE(I915_CSR_SKL);
> > > -#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 23)
> > > +#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 26)
> > >  
> > > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
> > > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
> > >  MODULE_FIRMWARE(I915_CSR_BXT);
> > >  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
> > >  
> > > @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct
> > > drm_i915_private *dev_priv,
> > >  	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount =
> > > 0, nbytes;
> > >  	uint32_t i;
> > >  	uint32_t *dmc_payload;
> > > -	uint32_t required_min_version;
> > > +	uint32_t required_version;
> > >  
> > >  	if (!fw)
> > >  		return NULL;
> > > @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct
> > > drm_i915_private *dev_priv,
> > >  	csr->version = css_header->version;
> > >  
> > >  	if (IS_KABYLAKE(dev_priv)) {
> > > -		required_min_version = KBL_CSR_VERSION_REQUIRED;
> > > +		required_version = KBL_CSR_VERSION_REQUIRED;
> > >  	} else if (IS_SKYLAKE(dev_priv)) {
> > > -		required_min_version = SKL_CSR_VERSION_REQUIRED;
> > > +		required_version = SKL_CSR_VERSION_REQUIRED;
> > >  	} else if (IS_BROXTON(dev_priv)) {
> > > -		required_min_version = BXT_CSR_VERSION_REQUIRED;
> > > +		required_version = BXT_CSR_VERSION_REQUIRED;
> > >  	} else {
> > >  		MISSING_CASE(INTEL_REVID(dev_priv));
> > > -		required_min_version = 0;
> > > +		required_version = 0;
> > >  	}
> > >  
> > > -	if (csr->version < required_min_version) {
> > > -		DRM_INFO("Refusing to load old DMC firmware
> > > v%u.%u,"
> > > -			 " please upgrade to v%u.%u or later"
> > > -			   " [" FIRMWARE_URL "].\n",
> > > +	if (csr->version != required_version) {
> > > +		DRM_INFO("Refusing to load DMC firmware v%u.%u,"
> > > +			 " please use v%u.%u [" FIRMWARE_URL
> > > "].\n",
> > >  			 CSR_VERSION_MAJOR(csr->version),
> > >  			 CSR_VERSION_MINOR(csr->version),
> > > -			 CSR_VERSION_MAJOR(required_min_version)
> > > ,
> > > -			 CSR_VERSION_MINOR(required_min_version)
> > > );
> > > +			 CSR_VERSION_MAJOR(required_version),
> > > +			 CSR_VERSION_MINOR(required_version));
> > >  		return NULL;
> > >  	}
> > >  
> > > -- 
> > > 2.5.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/dmc: Step away from symbolic links

[Patrik Jakobsson]

On Wed, Jun 15, 2016 at 12:11:55AM +0000, Vivi, Rodrigo wrote:
> On Mon, 2016-05-23 at 10:57 +0200, Patrik Jakobsson wrote:
> > On Wed, May 18, 2016 at 01:24:12PM +0300, Mika Kuoppala wrote:
> > > Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes:
> > > 
> > > > [ text/plain ]
> > > > Load specific firmware versions for the DMC instead of using
> > > > symbolic
> > > > links. The currently recommended versions are: SKL 1.26, KBL 1.01
> > > > and
> > > > BXT 1.07.
> > > > 
> > > > Certain DMC versions need workarounds in the driver which forces
> > > > us to
> > > > have a tight dependency between firmware and driver. In order to
> > > > be able
> > > > to provide a tested and known working configuration we must lock
> > > > down on
> > > > a specific DMC firmware version.
> > > > 
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com
> > > > >
> > > 
> > > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> > > 
> > > We need ack from Rodrigo and/or whomever is handling
> > > the fw releasing side.
> > > 
> > > -Mika
> > > 
> > 
> > As discussed on IRC, Rodrigo is currently away but since he requested
> > this
> > feature we indirectly have his ACK.
> 
> indeed! ;)

I assume we need BXT 1.07 released on 01.org before merging this. Any status on
that?

-Patrik

> 
> 
> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> > -Patrik
> > 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++------------
> > > > ---
> > > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > > > b/drivers/gpu/drm/i915/intel_csr.c
> > > > index 2b3b428..ea047cd 100644
> > > > --- a/drivers/gpu/drm/i915/intel_csr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > > > @@ -41,15 +41,15 @@
> > > >   * be moved to FW_FAILED.
> > > >   */
> > > >  
> > > > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
> > > > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
> > > >  MODULE_FIRMWARE(I915_CSR_KBL);
> > > >  #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 1)
> > > >  
> > > > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
> > > > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
> > > >  MODULE_FIRMWARE(I915_CSR_SKL);
> > > > -#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 23)
> > > > +#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 26)
> > > >  
> > > > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
> > > > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
> > > >  MODULE_FIRMWARE(I915_CSR_BXT);
> > > >  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
> > > >  
> > > > @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct
> > > > drm_i915_private *dev_priv,
> > > >  	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount =
> > > > 0, nbytes;
> > > >  	uint32_t i;
> > > >  	uint32_t *dmc_payload;
> > > > -	uint32_t required_min_version;
> > > > +	uint32_t required_version;
> > > >  
> > > >  	if (!fw)
> > > >  		return NULL;
> > > > @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct
> > > > drm_i915_private *dev_priv,
> > > >  	csr->version = css_header->version;
> > > >  
> > > >  	if (IS_KABYLAKE(dev_priv)) {
> > > > -		required_min_version = KBL_CSR_VERSION_REQUIRED;
> > > > +		required_version = KBL_CSR_VERSION_REQUIRED;
> > > >  	} else if (IS_SKYLAKE(dev_priv)) {
> > > > -		required_min_version = SKL_CSR_VERSION_REQUIRED;
> > > > +		required_version = SKL_CSR_VERSION_REQUIRED;
> > > >  	} else if (IS_BROXTON(dev_priv)) {
> > > > -		required_min_version = BXT_CSR_VERSION_REQUIRED;
> > > > +		required_version = BXT_CSR_VERSION_REQUIRED;
> > > >  	} else {
> > > >  		MISSING_CASE(INTEL_REVID(dev_priv));
> > > > -		required_min_version = 0;
> > > > +		required_version = 0;
> > > >  	}
> > > >  
> > > > -	if (csr->version < required_min_version) {
> > > > -		DRM_INFO("Refusing to load old DMC firmware
> > > > v%u.%u,"
> > > > -			 " please upgrade to v%u.%u or later"
> > > > -			   " [" FIRMWARE_URL "].\n",
> > > > +	if (csr->version != required_version) {
> > > > +		DRM_INFO("Refusing to load DMC firmware v%u.%u,"
> > > > +			 " please use v%u.%u [" FIRMWARE_URL
> > > > "].\n",
> > > >  			 CSR_VERSION_MAJOR(csr->version),
> > > >  			 CSR_VERSION_MINOR(csr->version),
> > > > -			 CSR_VERSION_MAJOR(required_min_version)
> > > > ,
> > > > -			 CSR_VERSION_MINOR(required_min_version)
> > > > );
> > > > +			 CSR_VERSION_MAJOR(required_version),
> > > > +			 CSR_VERSION_MINOR(required_version));
> > > >  		return NULL;
> > > >  	}
> > > >  
> > > > -- 
> > > > 2.5.0
> > 

-- 
Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/dmc: Step away from symbolic links

[Imre Deak]

Adding Christophe, he was supposed to make the release after
validation. I don't think it prevents merging this patch though, the
result is failure to load the firmware in either case.

--Imre

On ma, 2016-06-27 at 12:57 +0200, Patrik Jakobsson wrote:
> On Wed, Jun 15, 2016 at 12:11:55AM +0000, Vivi, Rodrigo wrote:
> > On Mon, 2016-05-23 at 10:57 +0200, Patrik Jakobsson wrote:
> > > On Wed, May 18, 2016 at 01:24:12PM +0300, Mika Kuoppala wrote:
> > > > Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes:
> > > > 
> > > > > [ text/plain ]
> > > > > Load specific firmware versions for the DMC instead of using
> > > > > symbolic
> > > > > links. The currently recommended versions are: SKL 1.26, KBL 1.01
> > > > > and
> > > > > BXT 1.07.
> > > > > 
> > > > > Certain DMC versions need workarounds in the driver which forces
> > > > > us to
> > > > > have a tight dependency between firmware and driver. In order to
> > > > > be able
> > > > > to provide a tested and known working configuration we must lock
> > > > > down on
> > > > > a specific DMC firmware version.
> > > > > 
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > > > Signed-off-by: Patrik Jakobsson 
> > > > > > 
> > > > 
> > > > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> > > > 
> > > > We need ack from Rodrigo and/or whomever is handling
> > > > the fw releasing side.
> > > > 
> > > > -Mika
> > > > 
> > > 
> > > As discussed on IRC, Rodrigo is currently away but since he requested
> > > this
> > > feature we indirectly have his ACK.
> > 
> > indeed! ;)
> 
> I assume we need BXT 1.07 released on 01.org before merging this. Any status on
> that?



> 
> -Patrik
> 
> > 
> > 
> > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > > -Patrik
> > > 
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++------------
> > > > > ---
> > > > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > > > > b/drivers/gpu/drm/i915/intel_csr.c
> > > > > index 2b3b428..ea047cd 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_csr.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > > > > @@ -41,15 +41,15 @@
> > > > >   * be moved to FW_FAILED.
> > > > >   */
> > > > >  
> > > > > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
> > > > > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
> > > > >  MODULE_FIRMWARE(I915_CSR_KBL);
> > > > >  #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 1)
> > > > >  
> > > > > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
> > > > > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
> > > > >  MODULE_FIRMWARE(I915_CSR_SKL);
> > > > > -#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 23)
> > > > > +#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 26)
> > > > >  
> > > > > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
> > > > > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
> > > > >  MODULE_FIRMWARE(I915_CSR_BXT);
> > > > >  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
> > > > >  
> > > > > @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct
> > > > > drm_i915_private *dev_priv,
> > > > >  	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount =
> > > > > 0, nbytes;
> > > > >  	uint32_t i;
> > > > >  	uint32_t *dmc_payload;
> > > > > -	uint32_t required_min_version;
> > > > > +	uint32_t required_version;
> > > > >  
> > > > >  	if (!fw)
> > > > >  		return NULL;
> > > > > @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct
> > > > > drm_i915_private *dev_priv,
> > > > >  	csr->version = css_header->version;
> > > > >  
> > > > >  	if (IS_KABYLAKE(dev_priv)) {
> > > > > -		required_min_version = KBL_CSR_VERSION_REQUIRED;
> > > > > +		required_version = KBL_CSR_VERSION_REQUIRED;
> > > > >  	} else if (IS_SKYLAKE(dev_priv)) {
> > > > > -		required_min_version = SKL_CSR_VERSION_REQUIRED;
> > > > > +		required_version = SKL_CSR_VERSION_REQUIRED;
> > > > >  	} else if (IS_BROXTON(dev_priv)) {
> > > > > -		required_min_version = BXT_CSR_VERSION_REQUIRED;
> > > > > +		required_version = BXT_CSR_VERSION_REQUIRED;
> > > > >  	} else {
> > > > >  		MISSING_CASE(INTEL_REVID(dev_priv));
> > > > > -		required_min_version = 0;
> > > > > +		required_version = 0;
> > > > >  	}
> > > > >  
> > > > > -	if (csr->version < required_min_version) {
> > > > > -		DRM_INFO("Refusing to load old DMC firmware
> > > > > v%u.%u,"
> > > > > -			 " please upgrade to v%u.%u or later"
> > > > > -			   " [" FIRMWARE_URL "].\n",
> > > > > +	if (csr->version != required_version) {
> > > > > +		DRM_INFO("Refusing to load DMC firmware v%u.%u,"
> > > > > +			 " please use v%u.%u [" FIRMWARE_URL
> > > > > "].\n",
> > > > >  			 CSR_VERSION_MAJOR(csr->version),
> > > > >  			 CSR_VERSION_MINOR(csr->version),
> > > > > -			 CSR_VERSION_MAJOR(required_min_version)
> > > > > ,
> > > > > -			 CSR_VERSION_MINOR(required_min_version)
> > > > > );
> > > > > +			 CSR_VERSION_MAJOR(required_version),
> > > > > +			 CSR_VERSION_MINOR(required_version));
> > > > >  		return NULL;
> > > > >  	}
> > > > >  
> > > > > -- 
> > > > > 2.5.0
> > > 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/dmc: Step away from symbolic links

[Vivi, Rodrigo]

On Mon, 2016-06-27 at 14:20 +0300, Imre Deak wrote:
> Adding Christophe, he was supposed to make the release after
> validation.

Apparently we are almost ready to release and one latest round of final
validation was pending.

Christophe, any news on this front?

>  I don't think it prevents merging this patch though, the
> result is failure to load the firmware in either case.

I was going to say that I agree, but I believe Patrik might be right.
Without this patch we load the 1.06 while with this patch we start
loading only the 1.07 that is not available. 
Although 1.06 might have issues the failures would be different. So or
we blacklist 1.06 with a separated patch and then merge this one or we
release the 1.07 before.

> 
> --Imre
> 
> On ma, 2016-06-27 at 12:57 +0200, Patrik Jakobsson wrote:
> > 
> > On Wed, Jun 15, 2016 at 12:11:55AM +0000, Vivi, Rodrigo wrote:
> > > 
> > > On Mon, 2016-05-23 at 10:57 +0200, Patrik Jakobsson wrote:
> > > > 
> > > > On Wed, May 18, 2016 at 01:24:12PM +0300, Mika Kuoppala wrote:
> > > > > 
> > > > > Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes:
> > > > > 
> > > > > > 
> > > > > > [ text/plain ]
> > > > > > Load specific firmware versions for the DMC instead of
> > > > > > using
> > > > > > symbolic
> > > > > > links. The currently recommended versions are: SKL 1.26,
> > > > > > KBL 1.01
> > > > > > and
> > > > > > BXT 1.07.
> > > > > > 
> > > > > > Certain DMC versions need workarounds in the driver which
> > > > > > forces
> > > > > > us to
> > > > > > have a tight dependency between firmware and driver. In
> > > > > > order to
> > > > > > be able
> > > > > > to provide a tested and known working configuration we must
> > > > > > lock
> > > > > > down on
> > > > > > a specific DMC firmware version.
> > > > > > 
> > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > > > > Signed-off-by: Patrik Jakobsson 
> > > > > > > 
> > > > > > > 
> > > > > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> > > > > 
> > > > > We need ack from Rodrigo and/or whomever is handling
> > > > > the fw releasing side.
> > > > > 
> > > > > -Mika
> > > > > 
> > > > As discussed on IRC, Rodrigo is currently away but since he
> > > > requested
> > > > this
> > > > feature we indirectly have his ACK.
> > > indeed! ;)
> > I assume we need BXT 1.07 released on 01.org before merging this.
> > Any status on
> > that?
> 
> 
> > 
> > 
> > -Patrik
> > 
> > > 
> > > 
> > > 
> > > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > 
> > > > 
> > > > -Patrik
> > > > 
> > > > > 
> > > > > > 
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++----
> > > > > > --------
> > > > > > ---
> > > > > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > > > > > b/drivers/gpu/drm/i915/intel_csr.c
> > > > > > index 2b3b428..ea047cd 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_csr.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > > > > > @@ -41,15 +41,15 @@
> > > > > >   * be moved to FW_FAILED.
> > > > > >   */
> > > > > >  
> > > > > > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
> > > > > > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
> > > > > >  MODULE_FIRMWARE(I915_CSR_KBL);
> > > > > >  #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 1)
> > > > > >  
> > > > > > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
> > > > > > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
> > > > > >  MODULE_FIRMWARE(I915_CSR_SKL);
> > > > > > -#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 23)
> > > > > > +#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 26)
> > > > > >  
> > > > > > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
> > > > > > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
> > > > > >  MODULE_FIRMWARE(I915_CSR_BXT);
> > > > > >  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
> > > > > >  
> > > > > > @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct
> > > > > > drm_i915_private *dev_priv,
> > > > > >  	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET,
> > > > > > readcount =
> > > > > > 0, nbytes;
> > > > > >  	uint32_t i;
> > > > > >  	uint32_t *dmc_payload;
> > > > > > -	uint32_t required_min_version;
> > > > > > +	uint32_t required_version;
> > > > > >  
> > > > > >  	if (!fw)
> > > > > >  		return NULL;
> > > > > > @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct
> > > > > > drm_i915_private *dev_priv,
> > > > > >  	csr->version = css_header->version;
> > > > > >  
> > > > > >  	if (IS_KABYLAKE(dev_priv)) {
> > > > > > -		required_min_version =
> > > > > > KBL_CSR_VERSION_REQUIRED;
> > > > > > +		required_version =
> > > > > > KBL_CSR_VERSION_REQUIRED;
> > > > > >  	} else if (IS_SKYLAKE(dev_priv)) {
> > > > > > -		required_min_version =
> > > > > > SKL_CSR_VERSION_REQUIRED;
> > > > > > +		required_version =
> > > > > > SKL_CSR_VERSION_REQUIRED;
> > > > > >  	} else if (IS_BROXTON(dev_priv)) {
> > > > > > -		required_min_version =
> > > > > > BXT_CSR_VERSION_REQUIRED;
> > > > > > +		required_version =
> > > > > > BXT_CSR_VERSION_REQUIRED;
> > > > > >  	} else {
> > > > > >  		MISSING_CASE(INTEL_REVID(dev_priv));
> > > > > > -		required_min_version = 0;
> > > > > > +		required_version = 0;
> > > > > >  	}
> > > > > >  
> > > > > > -	if (csr->version < required_min_version) {
> > > > > > -		DRM_INFO("Refusing to load old DMC
> > > > > > firmware
> > > > > > v%u.%u,"
> > > > > > -			 " please upgrade to v%u.%u or
> > > > > > later"
> > > > > > -			   " [" FIRMWARE_URL "].\n",
> > > > > > +	if (csr->version != required_version) {
> > > > > > +		DRM_INFO("Refusing to load DMC firmware
> > > > > > v%u.%u,"
> > > > > > +			 " please use v%u.%u ["
> > > > > > FIRMWARE_URL
> > > > > > "].\n",
> > > > > >  			 CSR_VERSION_MAJOR(csr->version),
> > > > > >  			 CSR_VERSION_MINOR(csr->version),
> > > > > > -			 CSR_VERSION_MAJOR(required_min_ve
> > > > > > rsion)
> > > > > > ,
> > > > > > -			 CSR_VERSION_MINOR(required_min_ve
> > > > > > rsion)
> > > > > > );
> > > > > > +			 CSR_VERSION_MAJOR(required_versio
> > > > > > n),
> > > > > > +			 CSR_VERSION_MINOR(required_versio
> > > > > > n));
> > > > > >  		return NULL;
> > > > > >  	}
> > > > > >  
> > > > > > -- 
> > > > > > 2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/dmc: Step away from symbolic links

[Imre Deak]

On Mon, 2016-06-27 at 19:32 +0300, Vivi, Rodrigo wrote:
> On Mon, 2016-06-27 at 14:20 +0300, Imre Deak wrote:
> > Adding Christophe, he was supposed to make the release after
> > validation.
> 
> Apparently we are almost ready to release and one latest round of
> final
> validation was pending.
> 
> Christophe, any news on this front?
> 
> >  I don't think it prevents merging this patch though, the
> > result is failure to load the firmware in either case.
> 
> I was going to say that I agree, but I believe Patrik might be right.
> Without this patch we load the 1.06 while with this patch we start
> loading only the 1.07 that is not available. 
> Although 1.06 might have issues the failures would be different. So
> or
> we blacklist 1.06 with a separated patch and then merge this one or
> we
> release the 1.07 before.

1.06 is already blacklisted, it has known problems.

--Imre

> > On ma, 2016-06-27 at 12:57 +0200, Patrik Jakobsson wrote:
> > > 
> > > On Wed, Jun 15, 2016 at 12:11:55AM +0000, Vivi, Rodrigo wrote:
> > > > 
> > > > On Mon, 2016-05-23 at 10:57 +0200, Patrik Jakobsson wrote:
> > > > > 
> > > > > On Wed, May 18, 2016 at 01:24:12PM +0300, Mika Kuoppala
> > > > > wrote:
> > > > > > 
> > > > > > Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes:
> > > > > > 
> > > > > > > 
> > > > > > > [ text/plain ]
> > > > > > > Load specific firmware versions for the DMC instead of
> > > > > > > using
> > > > > > > symbolic
> > > > > > > links. The currently recommended versions are: SKL 1.26,
> > > > > > > KBL 1.01
> > > > > > > and
> > > > > > > BXT 1.07.
> > > > > > > 
> > > > > > > Certain DMC versions need workarounds in the driver which
> > > > > > > forces
> > > > > > > us to
> > > > > > > have a tight dependency between firmware and driver. In
> > > > > > > order to
> > > > > > > be able
> > > > > > > to provide a tested and known working configuration we
> > > > > > > must
> > > > > > > lock
> > > > > > > down on
> > > > > > > a specific DMC firmware version.
> > > > > > > 
> > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > > > > > Signed-off-by: Patrik Jakobsson 
> > > > > > > > 
> > > > > > > > 
> > > > > > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> > > > > > 
> > > > > > We need ack from Rodrigo and/or whomever is handling
> > > > > > the fw releasing side.
> > > > > > 
> > > > > > -Mika
> > > > > > 
> > > > > As discussed on IRC, Rodrigo is currently away but since he
> > > > > requested
> > > > > this
> > > > > feature we indirectly have his ACK.
> > > > indeed! ;)
> > > I assume we need BXT 1.07 released on 01.org before merging this.
> > > Any status on
> > > that?
> > 
> > 
> > > 
> > > 
> > > -Patrik
> > > 
> > > > 
> > > > 
> > > > 
> > > > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > 
> > > > > 
> > > > > -Patrik
> > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++----
> > > > > > > --------
> > > > > > > ---
> > > > > > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > > > > > > b/drivers/gpu/drm/i915/intel_csr.c
> > > > > > > index 2b3b428..ea047cd 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_csr.c
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > > > > > > @@ -41,15 +41,15 @@
> > > > > > >   * be moved to FW_FAILED.
> > > > > > >   */
> > > > > > >  
> > > > > > > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
> > > > > > > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
> > > > > > >  MODULE_FIRMWARE(I915_CSR_KBL);
> > > > > > >  #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1,
> > > > > > > 1)
> > > > > > >  
> > > > > > > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
> > > > > > > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
> > > > > > >  MODULE_FIRMWARE(I915_CSR_SKL);
> > > > > > > -#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1,
> > > > > > > 23)
> > > > > > > +#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1,
> > > > > > > 26)
> > > > > > >  
> > > > > > > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
> > > > > > > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
> > > > > > >  MODULE_FIRMWARE(I915_CSR_BXT);
> > > > > > >  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1,
> > > > > > > 7)
> > > > > > >  
> > > > > > > @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct
> > > > > > > drm_i915_private *dev_priv,
> > > > > > >  	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET,
> > > > > > > readcount =
> > > > > > > 0, nbytes;
> > > > > > >  	uint32_t i;
> > > > > > >  	uint32_t *dmc_payload;
> > > > > > > -	uint32_t required_min_version;
> > > > > > > +	uint32_t required_version;
> > > > > > >  
> > > > > > >  	if (!fw)
> > > > > > >  		return NULL;
> > > > > > > @@ -303,24 +303,23 @@ static uint32_t
> > > > > > > *parse_csr_fw(struct
> > > > > > > drm_i915_private *dev_priv,
> > > > > > >  	csr->version = css_header->version;
> > > > > > >  
> > > > > > >  	if (IS_KABYLAKE(dev_priv)) {
> > > > > > > -		required_min_version =
> > > > > > > KBL_CSR_VERSION_REQUIRED;
> > > > > > > +		required_version =
> > > > > > > KBL_CSR_VERSION_REQUIRED;
> > > > > > >  	} else if (IS_SKYLAKE(dev_priv)) {
> > > > > > > -		required_min_version =
> > > > > > > SKL_CSR_VERSION_REQUIRED;
> > > > > > > +		required_version =
> > > > > > > SKL_CSR_VERSION_REQUIRED;
> > > > > > >  	} else if (IS_BROXTON(dev_priv)) {
> > > > > > > -		required_min_version =
> > > > > > > BXT_CSR_VERSION_REQUIRED;
> > > > > > > +		required_version =
> > > > > > > BXT_CSR_VERSION_REQUIRED;
> > > > > > >  	} else {
> > > > > > >  		MISSING_CASE(INTEL_REVID(dev_priv));
> > > > > > > -		required_min_version = 0;
> > > > > > > +		required_version = 0;
> > > > > > >  	}
> > > > > > >  
> > > > > > > -	if (csr->version < required_min_version) {
> > > > > > > -		DRM_INFO("Refusing to load old DMC
> > > > > > > firmware
> > > > > > > v%u.%u,"
> > > > > > > -			 " please upgrade to v%u.%u or
> > > > > > > later"
> > > > > > > -			   " [" FIRMWARE_URL "].\n",
> > > > > > > +	if (csr->version != required_version) {
> > > > > > > +		DRM_INFO("Refusing to load DMC firmware
> > > > > > > v%u.%u,"
> > > > > > > +			 " please use v%u.%u ["
> > > > > > > FIRMWARE_URL
> > > > > > > "].\n",
> > > > > > >  			 CSR_VERSION_MAJOR(csr-
> > > > > > > >version),
> > > > > > >  			 CSR_VERSION_MINOR(csr-
> > > > > > > >version),
> > > > > > > -			 CSR_VERSION_MAJOR(required_min_
> > > > > > > ve
> > > > > > > rsion)
> > > > > > > ,
> > > > > > > -			 CSR_VERSION_MINOR(required_min_
> > > > > > > ve
> > > > > > > rsion)
> > > > > > > );
> > > > > > > +			 CSR_VERSION_MAJOR(required_vers
> > > > > > > io
> > > > > > > n),
> > > > > > > +			 CSR_VERSION_MINOR(required_vers
> > > > > > > io
> > > > > > > n));
> > > > > > >  		return NULL;
> > > > > > >  	}
> > > > > > >  
> > > > > > > -- 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/dmc: Step away from symbolic links

[Vivi, Rodrigo]

On Mon, 2016-06-27 at 19:51 +0300, Imre Deak wrote:
> On Mon, 2016-06-27 at 19:32 +0300, Vivi, Rodrigo wrote:
> > 
> > On Mon, 2016-06-27 at 14:20 +0300, Imre Deak wrote:
> > > 
> > > Adding Christophe, he was supposed to make the release after
> > > validation.
> > Apparently we are almost ready to release and one latest round of
> > final
> > validation was pending.
> > 
> > Christophe, any news on this front?
> > 
> > > 
> > >  I don't think it prevents merging this patch though, the
> > > result is failure to load the firmware in either case.
> > I was going to say that I agree, but I believe Patrik might be
> > right.
> > Without this patch we load the 1.06 while with this patch we start
> > loading only the 1.07 that is not available. 
> > Although 1.06 might have issues the failures would be different. So
> > or
> > we blacklist 1.06 with a separated patch and then merge this one or
> > we
> > release the 1.07 before.
> 1.06 is already blacklisted, it has known problems.

Oh! So I agree with the first statement. Let's merge this patch ;)

> 
> --Imre
> 
> > 
> > > 
> > > On ma, 2016-06-27 at 12:57 +0200, Patrik Jakobsson wrote:
> > > > 
> > > > 
> > > > On Wed, Jun 15, 2016 at 12:11:55AM +0000, Vivi, Rodrigo wrote:
> > > > > 
> > > > > 
> > > > > On Mon, 2016-05-23 at 10:57 +0200, Patrik Jakobsson wrote:
> > > > > > 
> > > > > > 
> > > > > > On Wed, May 18, 2016 at 01:24:12PM +0300, Mika Kuoppala
> > > > > > wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> > > > > > > writes:
> > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > [ text/plain ]
> > > > > > > > Load specific firmware versions for the DMC instead of
> > > > > > > > using
> > > > > > > > symbolic
> > > > > > > > links. The currently recommended versions are: SKL
> > > > > > > > 1.26,
> > > > > > > > KBL 1.01
> > > > > > > > and
> > > > > > > > BXT 1.07.
> > > > > > > > 
> > > > > > > > Certain DMC versions need workarounds in the driver
> > > > > > > > which
> > > > > > > > forces
> > > > > > > > us to
> > > > > > > > have a tight dependency between firmware and driver. In
> > > > > > > > order to
> > > > > > > > be able
> > > > > > > > to provide a tested and known working configuration we
> > > > > > > > must
> > > > > > > > lock
> > > > > > > > down on
> > > > > > > > a specific DMC firmware version.
> > > > > > > > 
> > > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > > > > > > Signed-off-by: Patrik Jakobsson 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> > > > > > > 
> > > > > > > We need ack from Rodrigo and/or whomever is handling
> > > > > > > the fw releasing side.
> > > > > > > 
> > > > > > > -Mika
> > > > > > > 
> > > > > > As discussed on IRC, Rodrigo is currently away but since he
> > > > > > requested
> > > > > > this
> > > > > > feature we indirectly have his ACK.
> > > > > indeed! ;)
> > > > I assume we need BXT 1.07 released on 01.org before merging
> > > > this.
> > > > Any status on
> > > > that?
> > > 
> > > > 
> > > > 
> > > > 
> > > > -Patrik
> > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > -Patrik
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > ---
> > > > > > > >  drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++
> > > > > > > > ----
> > > > > > > > --------
> > > > > > > > ---
> > > > > > > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > > > > > > > b/drivers/gpu/drm/i915/intel_csr.c
> > > > > > > > index 2b3b428..ea047cd 100644
> > > > > > > > --- a/drivers/gpu/drm/i915/intel_csr.c
> > > > > > > > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > > > > > > > @@ -41,15 +41,15 @@
> > > > > > > >   * be moved to FW_FAILED.
> > > > > > > >   */
> > > > > > > >  
> > > > > > > > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
> > > > > > > > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
> > > > > > > >  MODULE_FIRMWARE(I915_CSR_KBL);
> > > > > > > >  #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1,
> > > > > > > > 1)
> > > > > > > >  
> > > > > > > > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
> > > > > > > > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
> > > > > > > >  MODULE_FIRMWARE(I915_CSR_SKL);
> > > > > > > > -#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1,
> > > > > > > > 23)
> > > > > > > > +#define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1,
> > > > > > > > 26)
> > > > > > > >  
> > > > > > > > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
> > > > > > > > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
> > > > > > > >  MODULE_FIRMWARE(I915_CSR_BXT);
> > > > > > > >  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1,
> > > > > > > > 7)
> > > > > > > >  
> > > > > > > > @@ -286,7 +286,7 @@ static uint32_t
> > > > > > > > *parse_csr_fw(struct
> > > > > > > > drm_i915_private *dev_priv,
> > > > > > > >  	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET,
> > > > > > > > readcount =
> > > > > > > > 0, nbytes;
> > > > > > > >  	uint32_t i;
> > > > > > > >  	uint32_t *dmc_payload;
> > > > > > > > -	uint32_t required_min_version;
> > > > > > > > +	uint32_t required_version;
> > > > > > > >  
> > > > > > > >  	if (!fw)
> > > > > > > >  		return NULL;
> > > > > > > > @@ -303,24 +303,23 @@ static uint32_t
> > > > > > > > *parse_csr_fw(struct
> > > > > > > > drm_i915_private *dev_priv,
> > > > > > > >  	csr->version = css_header->version;
> > > > > > > >  
> > > > > > > >  	if (IS_KABYLAKE(dev_priv)) {
> > > > > > > > -		required_min_version =
> > > > > > > > KBL_CSR_VERSION_REQUIRED;
> > > > > > > > +		required_version =
> > > > > > > > KBL_CSR_VERSION_REQUIRED;
> > > > > > > >  	} else if (IS_SKYLAKE(dev_priv)) {
> > > > > > > > -		required_min_version =
> > > > > > > > SKL_CSR_VERSION_REQUIRED;
> > > > > > > > +		required_version =
> > > > > > > > SKL_CSR_VERSION_REQUIRED;
> > > > > > > >  	} else if (IS_BROXTON(dev_priv)) {
> > > > > > > > -		required_min_version =
> > > > > > > > BXT_CSR_VERSION_REQUIRED;
> > > > > > > > +		required_version =
> > > > > > > > BXT_CSR_VERSION_REQUIRED;
> > > > > > > >  	} else {
> > > > > > > >  		MISSING_CASE(INTEL_REVID(dev_priv));
> > > > > > > > -		required_min_version = 0;
> > > > > > > > +		required_version = 0;
> > > > > > > >  	}
> > > > > > > >  
> > > > > > > > -	if (csr->version < required_min_version) {
> > > > > > > > -		DRM_INFO("Refusing to load old DMC
> > > > > > > > firmware
> > > > > > > > v%u.%u,"
> > > > > > > > -			 " please upgrade to v%u.%u or
> > > > > > > > later"
> > > > > > > > -			   " [" FIRMWARE_URL "].\n",
> > > > > > > > +	if (csr->version != required_version) {
> > > > > > > > +		DRM_INFO("Refusing to load DMC
> > > > > > > > firmware
> > > > > > > > v%u.%u,"
> > > > > > > > +			 " please use v%u.%u ["
> > > > > > > > FIRMWARE_URL
> > > > > > > > "].\n",
> > > > > > > >  			 CSR_VERSION_MAJOR(csr-
> > > > > > > > > 
> > > > > > > > > version),
> > > > > > > >  			 CSR_VERSION_MINOR(csr-
> > > > > > > > > 
> > > > > > > > > version),
> > > > > > > > -			 CSR_VERSION_MAJOR(required_mi
> > > > > > > > n_
> > > > > > > > ve
> > > > > > > > rsion)
> > > > > > > > ,
> > > > > > > > -			 CSR_VERSION_MINOR(required_mi
> > > > > > > > n_
> > > > > > > > ve
> > > > > > > > rsion)
> > > > > > > > );
> > > > > > > > +			 CSR_VERSION_MAJOR(required_ve
> > > > > > > > rs
> > > > > > > > io
> > > > > > > > n),
> > > > > > > > +			 CSR_VERSION_MINOR(required_ve
> > > > > > > > rs
> > > > > > > > io
> > > > > > > > n));
> > > > > > > >  		return NULL;
> > > > > > > >  	}
> > > > > > > >  
> > > > > > > > -- 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/dmc: Step away from symbolic links

[Patrik Jakobsson]

On Mon, Jun 27, 2016 at 7:12 PM, Vivi, Rodrigo <rodrigo.vivi@intel.com> wrote:
> On Mon, 2016-06-27 at 19:51 +0300, Imre Deak wrote:
>> On Mon, 2016-06-27 at 19:32 +0300, Vivi, Rodrigo wrote:
>> >
>> > On Mon, 2016-06-27 at 14:20 +0300, Imre Deak wrote:
>> > >
>> > > Adding Christophe, he was supposed to make the release after
>> > > validation.
>> > Apparently we are almost ready to release and one latest round of
>> > final
>> > validation was pending.
>> >
>> > Christophe, any news on this front?
>> >
>> > >
>> > >  I don't think it prevents merging this patch though, the
>> > > result is failure to load the firmware in either case.
>> > I was going to say that I agree, but I believe Patrik might be
>> > right.
>> > Without this patch we load the 1.06 while with this patch we start
>> > loading only the 1.07 that is not available.
>> > Although 1.06 might have issues the failures would be different. So
>> > or
>> > we blacklist 1.06 with a separated patch and then merge this one or
>> > we
>> > release the 1.07 before.
>> 1.06 is already blacklisted, it has known problems.
>
> Oh! So I agree with the first statement. Let's merge this patch ;)

That was new info for me as well. I don't have commit access so anyone
who can, feel free to merge.

-Patrik

>
>>
>> --Imre
>>
>> >
>> > >
>> > > On ma, 2016-06-27 at 12:57 +0200, Patrik Jakobsson wrote:
>> > > >
>> > > >
>> > > > On Wed, Jun 15, 2016 at 12:11:55AM +0000, Vivi, Rodrigo wrote:
>> > > > >
>> > > > >
>> > > > > On Mon, 2016-05-23 at 10:57 +0200, Patrik Jakobsson wrote:
>> > > > > >
>> > > > > >
>> > > > > > On Wed, May 18, 2016 at 01:24:12PM +0300, Mika Kuoppala
>> > > > > > wrote:
>> > > > > > >
>> > > > > > >
>> > > > > > > Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
>> > > > > > > writes:
>> > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > [ text/plain ]
>> > > > > > > > Load specific firmware versions for the DMC instead of
>> > > > > > > > using
>> > > > > > > > symbolic
>> > > > > > > > links. The currently recommended versions are: SKL
>> > > > > > > > 1.26,
>> > > > > > > > KBL 1.01
>> > > > > > > > and
>> > > > > > > > BXT 1.07.
>> > > > > > > >
>> > > > > > > > Certain DMC versions need workarounds in the driver
>> > > > > > > > which
>> > > > > > > > forces
>> > > > > > > > us to
>> > > > > > > > have a tight dependency between firmware and driver. In
>> > > > > > > > order to
>> > > > > > > > be able
>> > > > > > > > to provide a tested and known working configuration we
>> > > > > > > > must
>> > > > > > > > lock
>> > > > > > > > down on
>> > > > > > > > a specific DMC firmware version.
>> > > > > > > >
>> > > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > > > > > > > Cc: Imre Deak <imre.deak@intel.com>
>> > > > > > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> > > > > > > > Signed-off-by: Patrik Jakobsson
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> > > > > > >
>> > > > > > > We need ack from Rodrigo and/or whomever is handling
>> > > > > > > the fw releasing side.
>> > > > > > >
>> > > > > > > -Mika
>> > > > > > >
>> > > > > > As discussed on IRC, Rodrigo is currently away but since he
>> > > > > > requested
>> > > > > > this
>> > > > > > feature we indirectly have his ACK.
>> > > > > indeed! ;)
>> > > > I assume we need BXT 1.07 released on 01.org before merging
>> > > > this.
>> > > > Any status on
>> > > > that?
>> > >
>> > > >
>> > > >
>> > > >
>> > > > -Patrik
>> > > >
>> > > > >
>> > > > >
>> > > > >
>> > > > >
>> > > > > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > > > >
>> > > > > >
>> > > > > >
>> > > > > > -Patrik
>> > > > > >
>> > > > > > >
>> > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > ---
>> > > > > > > >  drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++
>> > > > > > > > ----
>> > > > > > > > --------
>> > > > > > > > ---
>> > > > > > > >  1 file changed, 14 insertions(+), 15 deletions(-)
>> > > > > > > >
>> > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c
>> > > > > > > > b/drivers/gpu/drm/i915/intel_csr.c
>> > > > > > > > index 2b3b428..ea047cd 100644
>> > > > > > > > --- a/drivers/gpu/drm/i915/intel_csr.c
>> > > > > > > > +++ b/drivers/gpu/drm/i915/intel_csr.c
>> > > > > > > > @@ -41,15 +41,15 @@
>> > > > > > > >   * be moved to FW_FAILED.
>> > > > > > > >   */
>> > > > > > > >
>> > > > > > > > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
>> > > > > > > > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
>> > > > > > > >  MODULE_FIRMWARE(I915_CSR_KBL);
>> > > > > > > >  #define KBL_CSR_VERSION_REQUIRED       CSR_VERSION(1,
>> > > > > > > > 1)
>> > > > > > > >
>> > > > > > > > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
>> > > > > > > > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
>> > > > > > > >  MODULE_FIRMWARE(I915_CSR_SKL);
>> > > > > > > > -#define SKL_CSR_VERSION_REQUIRED       CSR_VERSION(1,
>> > > > > > > > 23)
>> > > > > > > > +#define SKL_CSR_VERSION_REQUIRED       CSR_VERSION(1,
>> > > > > > > > 26)
>> > > > > > > >
>> > > > > > > > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
>> > > > > > > > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
>> > > > > > > >  MODULE_FIRMWARE(I915_CSR_BXT);
>> > > > > > > >  #define BXT_CSR_VERSION_REQUIRED       CSR_VERSION(1,
>> > > > > > > > 7)
>> > > > > > > >
>> > > > > > > > @@ -286,7 +286,7 @@ static uint32_t
>> > > > > > > > *parse_csr_fw(struct
>> > > > > > > > drm_i915_private *dev_priv,
>> > > > > > > >         uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET,
>> > > > > > > > readcount =
>> > > > > > > > 0, nbytes;
>> > > > > > > >         uint32_t i;
>> > > > > > > >         uint32_t *dmc_payload;
>> > > > > > > > -       uint32_t required_min_version;
>> > > > > > > > +       uint32_t required_version;
>> > > > > > > >
>> > > > > > > >         if (!fw)
>> > > > > > > >                 return NULL;
>> > > > > > > > @@ -303,24 +303,23 @@ static uint32_t
>> > > > > > > > *parse_csr_fw(struct
>> > > > > > > > drm_i915_private *dev_priv,
>> > > > > > > >         csr->version = css_header->version;
>> > > > > > > >
>> > > > > > > >         if (IS_KABYLAKE(dev_priv)) {
>> > > > > > > > -               required_min_version =
>> > > > > > > > KBL_CSR_VERSION_REQUIRED;
>> > > > > > > > +               required_version =
>> > > > > > > > KBL_CSR_VERSION_REQUIRED;
>> > > > > > > >         } else if (IS_SKYLAKE(dev_priv)) {
>> > > > > > > > -               required_min_version =
>> > > > > > > > SKL_CSR_VERSION_REQUIRED;
>> > > > > > > > +               required_version =
>> > > > > > > > SKL_CSR_VERSION_REQUIRED;
>> > > > > > > >         } else if (IS_BROXTON(dev_priv)) {
>> > > > > > > > -               required_min_version =
>> > > > > > > > BXT_CSR_VERSION_REQUIRED;
>> > > > > > > > +               required_version =
>> > > > > > > > BXT_CSR_VERSION_REQUIRED;
>> > > > > > > >         } else {
>> > > > > > > >                 MISSING_CASE(INTEL_REVID(dev_priv));
>> > > > > > > > -               required_min_version = 0;
>> > > > > > > > +               required_version = 0;
>> > > > > > > >         }
>> > > > > > > >
>> > > > > > > > -       if (csr->version < required_min_version) {
>> > > > > > > > -               DRM_INFO("Refusing to load old DMC
>> > > > > > > > firmware
>> > > > > > > > v%u.%u,"
>> > > > > > > > -                        " please upgrade to v%u.%u or
>> > > > > > > > later"
>> > > > > > > > -                          " [" FIRMWARE_URL "].\n",
>> > > > > > > > +       if (csr->version != required_version) {
>> > > > > > > > +               DRM_INFO("Refusing to load DMC
>> > > > > > > > firmware
>> > > > > > > > v%u.%u,"
>> > > > > > > > +                        " please use v%u.%u ["
>> > > > > > > > FIRMWARE_URL
>> > > > > > > > "].\n",
>> > > > > > > >                          CSR_VERSION_MAJOR(csr-
>> > > > > > > > >
>> > > > > > > > > version),
>> > > > > > > >                          CSR_VERSION_MINOR(csr-
>> > > > > > > > >
>> > > > > > > > > version),
>> > > > > > > > -                        CSR_VERSION_MAJOR(required_mi
>> > > > > > > > n_
>> > > > > > > > ve
>> > > > > > > > rsion)
>> > > > > > > > ,
>> > > > > > > > -                        CSR_VERSION_MINOR(required_mi
>> > > > > > > > n_
>> > > > > > > > ve
>> > > > > > > > rsion)
>> > > > > > > > );
>> > > > > > > > +                        CSR_VERSION_MAJOR(required_ve
>> > > > > > > > rs
>> > > > > > > > io
>> > > > > > > > n),
>> > > > > > > > +                        CSR_VERSION_MINOR(required_ve
>> > > > > > > > rs
>> > > > > > > > io
>> > > > > > > > n));
>> > > > > > > >                 return NULL;
>> > > > > > > >         }
>> > > > > > > >
>> > > > > > > > --
> _______________________________________________
> 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

Re: [PATCH v2] drm/i915/dmc: Step away from symbolic links

[Vivi, Rodrigo]

On Mon, 2016-06-27 at 20:45 +0200, Patrik Jakobsson wrote:
> On Mon, Jun 27, 2016 at 7:12 PM, Vivi, Rodrigo <rodrigo.vivi@intel.co
> m> wrote:
> > 
> > On Mon, 2016-06-27 at 19:51 +0300, Imre Deak wrote:
> > > 
> > > On Mon, 2016-06-27 at 19:32 +0300, Vivi, Rodrigo wrote:
> > > > 
> > > > 
> > > > On Mon, 2016-06-27 at 14:20 +0300, Imre Deak wrote:
> > > > > 
> > > > > 
> > > > > Adding Christophe, he was supposed to make the release after
> > > > > validation.
> > > > Apparently we are almost ready to release and one latest round
> > > > of
> > > > final
> > > > validation was pending.
> > > > 
> > > > Christophe, any news on this front?
> > > > 
> > > > > 
> > > > > 
> > > > >  I don't think it prevents merging this patch though, the
> > > > > result is failure to load the firmware in either case.
> > > > I was going to say that I agree, but I believe Patrik might be
> > > > right.
> > > > Without this patch we load the 1.06 while with this patch we
> > > > start
> > > > loading only the 1.07 that is not available.
> > > > Although 1.06 might have issues the failures would be
> > > > different. So
> > > > or
> > > > we blacklist 1.06 with a separated patch and then merge this
> > > > one or
> > > > we
> > > > release the 1.07 before.
> > > 1.06 is already blacklisted, it has known problems.
> > Oh! So I agree with the first statement. Let's merge this patch ;)
> That was new info for me as well. I don't have commit access so
> anyone
> who can, feel free to merge.

I just did.

Thanks,
Rodrigo.

> 
> -Patrik
> 
> > 
> > 
> > > 
> > > 
> > > --Imre
> > > 
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > On ma, 2016-06-27 at 12:57 +0200, Patrik Jakobsson wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Wed, Jun 15, 2016 at 12:11:55AM +0000, Vivi, Rodrigo
> > > > > > wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On Mon, 2016-05-23 at 10:57 +0200, Patrik Jakobsson
> > > > > > > wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On Wed, May 18, 2016 at 01:24:12PM +0300, Mika Kuoppala
> > > > > > > > wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> > > > > > > > > writes:
> > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > [ text/plain ]
> > > > > > > > > > Load specific firmware versions for the DMC instead
> > > > > > > > > > of
> > > > > > > > > > using
> > > > > > > > > > symbolic
> > > > > > > > > > links. The currently recommended versions are: SKL
> > > > > > > > > > 1.26,
> > > > > > > > > > KBL 1.01
> > > > > > > > > > and
> > > > > > > > > > BXT 1.07.
> > > > > > > > > > 
> > > > > > > > > > Certain DMC versions need workarounds in the driver
> > > > > > > > > > which
> > > > > > > > > > forces
> > > > > > > > > > us to
> > > > > > > > > > have a tight dependency between firmware and
> > > > > > > > > > driver. In
> > > > > > > > > > order to
> > > > > > > > > > be able
> > > > > > > > > > to provide a tested and known working configuration
> > > > > > > > > > we
> > > > > > > > > > must
> > > > > > > > > > lock
> > > > > > > > > > down on
> > > > > > > > > > a specific DMC firmware version.
> > > > > > > > > > 
> > > > > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > > > > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > > > > > > > > Signed-off-by: Patrik Jakobsson
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> > > > > > > > > 
> > > > > > > > > We need ack from Rodrigo and/or whomever is handling
> > > > > > > > > the fw releasing side.
> > > > > > > > > 
> > > > > > > > > -Mika
> > > > > > > > > 
> > > > > > > > As discussed on IRC, Rodrigo is currently away but
> > > > > > > > since he
> > > > > > > > requested
> > > > > > > > this
> > > > > > > > feature we indirectly have his ACK.
> > > > > > > indeed! ;)
> > > > > > I assume we need BXT 1.07 released on 01.org before merging
> > > > > > this.
> > > > > > Any status on
> > > > > > that?
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > -Patrik
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > -Patrik
> > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > ---
> > > > > > > > > >  drivers/gpu/drm/i915/intel_csr.c | 29
> > > > > > > > > > ++++++++++++++
> > > > > > > > > > ----
> > > > > > > > > > --------
> > > > > > > > > > ---
> > > > > > > > > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > > > > > > > > > b/drivers/gpu/drm/i915/intel_csr.c
> > > > > > > > > > index 2b3b428..ea047cd 100644
> > > > > > > > > > --- a/drivers/gpu/drm/i915/intel_csr.c
> > > > > > > > > > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > > > > > > > > > @@ -41,15 +41,15 @@
> > > > > > > > > >   * be moved to FW_FAILED.
> > > > > > > > > >   */
> > > > > > > > > > 
> > > > > > > > > > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
> > > > > > > > > > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
> > > > > > > > > >  MODULE_FIRMWARE(I915_CSR_KBL);
> > > > > > > > > >  #define
> > > > > > > > > > KBL_CSR_VERSION_REQUIRED       CSR_VERSION(1,
> > > > > > > > > > 1)
> > > > > > > > > > 
> > > > > > > > > > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
> > > > > > > > > > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
> > > > > > > > > >  MODULE_FIRMWARE(I915_CSR_SKL);
> > > > > > > > > > -#define
> > > > > > > > > > SKL_CSR_VERSION_REQUIRED       CSR_VERSION(1,
> > > > > > > > > > 23)
> > > > > > > > > > +#define
> > > > > > > > > > SKL_CSR_VERSION_REQUIRED       CSR_VERSION(1,
> > > > > > > > > > 26)
> > > > > > > > > > 
> > > > > > > > > > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
> > > > > > > > > > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
> > > > > > > > > >  MODULE_FIRMWARE(I915_CSR_BXT);
> > > > > > > > > >  #define
> > > > > > > > > > BXT_CSR_VERSION_REQUIRED       CSR_VERSION(1,
> > > > > > > > > > 7)
> > > > > > > > > > 
> > > > > > > > > > @@ -286,7 +286,7 @@ static uint32_t
> > > > > > > > > > *parse_csr_fw(struct
> > > > > > > > > > drm_i915_private *dev_priv,
> > > > > > > > > >         uint32_t dmc_offset =
> > > > > > > > > > CSR_DEFAULT_FW_OFFSET,
> > > > > > > > > > readcount =
> > > > > > > > > > 0, nbytes;
> > > > > > > > > >         uint32_t i;
> > > > > > > > > >         uint32_t *dmc_payload;
> > > > > > > > > > -       uint32_t required_min_version;
> > > > > > > > > > +       uint32_t required_version;
> > > > > > > > > > 
> > > > > > > > > >         if (!fw)
> > > > > > > > > >                 return NULL;
> > > > > > > > > > @@ -303,24 +303,23 @@ static uint32_t
> > > > > > > > > > *parse_csr_fw(struct
> > > > > > > > > > drm_i915_private *dev_priv,
> > > > > > > > > >         csr->version = css_header->version;
> > > > > > > > > > 
> > > > > > > > > >         if (IS_KABYLAKE(dev_priv)) {
> > > > > > > > > > -               required_min_version =
> > > > > > > > > > KBL_CSR_VERSION_REQUIRED;
> > > > > > > > > > +               required_version =
> > > > > > > > > > KBL_CSR_VERSION_REQUIRED;
> > > > > > > > > >         } else if (IS_SKYLAKE(dev_priv)) {
> > > > > > > > > > -               required_min_version =
> > > > > > > > > > SKL_CSR_VERSION_REQUIRED;
> > > > > > > > > > +               required_version =
> > > > > > > > > > SKL_CSR_VERSION_REQUIRED;
> > > > > > > > > >         } else if (IS_BROXTON(dev_priv)) {
> > > > > > > > > > -               required_min_version =
> > > > > > > > > > BXT_CSR_VERSION_REQUIRED;
> > > > > > > > > > +               required_version =
> > > > > > > > > > BXT_CSR_VERSION_REQUIRED;
> > > > > > > > > >         } else {
> > > > > > > > > >                 MISSING_CASE(INTEL_REVID(dev_priv))
> > > > > > > > > > ;
> > > > > > > > > > -               required_min_version = 0;
> > > > > > > > > > +               required_version = 0;
> > > > > > > > > >         }
> > > > > > > > > > 
> > > > > > > > > > -       if (csr->version < required_min_version) {
> > > > > > > > > > -               DRM_INFO("Refusing to load old DMC
> > > > > > > > > > firmware
> > > > > > > > > > v%u.%u,"
> > > > > > > > > > -                        " please upgrade to v%u.%u
> > > > > > > > > > or
> > > > > > > > > > later"
> > > > > > > > > > -                          " [" FIRMWARE_URL
> > > > > > > > > > "].\n",
> > > > > > > > > > +       if (csr->version != required_version) {
> > > > > > > > > > +               DRM_INFO("Refusing to load DMC
> > > > > > > > > > firmware
> > > > > > > > > > v%u.%u,"
> > > > > > > > > > +                        " please use v%u.%u ["
> > > > > > > > > > FIRMWARE_URL
> > > > > > > > > > "].\n",
> > > > > > > > > >                          CSR_VERSION_MAJOR(csr-
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > version),
> > > > > > > > > >                          CSR_VERSION_MINOR(csr-
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > version),
> > > > > > > > > > -                        CSR_VERSION_MAJOR(required
> > > > > > > > > > _mi
> > > > > > > > > > n_
> > > > > > > > > > ve
> > > > > > > > > > rsion)
> > > > > > > > > > ,
> > > > > > > > > > -                        CSR_VERSION_MINOR(required
> > > > > > > > > > _mi
> > > > > > > > > > n_
> > > > > > > > > > ve
> > > > > > > > > > rsion)
> > > > > > > > > > );
> > > > > > > > > > +                        CSR_VERSION_MAJOR(required
> > > > > > > > > > _ve
> > > > > > > > > > rs
> > > > > > > > > > io
> > > > > > > > > > n),
> > > > > > > > > > +                        CSR_VERSION_MINOR(required
> > > > > > > > > > _ve
> > > > > > > > > > rs
> > > > > > > > > > io
> > > > > > > > > > n));
> > > > > > > > > >                 return NULL;
> > > > > > > > > >         }
> > > > > > > > > > 
> > > > > > > > > > --
> > _______________________________________________
> > 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

✓ Ro.CI.BAT: success for drm/i915/dmc: Step away from symbolic links (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/dmc: Step away from symbolic links (rev2)
URL   : https://patchwork.freedesktop.org/series/6966/
State : success

== Summary ==

Series 6966v2 drm/i915/dmc: Step away from symbolic links
http://patchwork.freedesktop.org/api/1.0/series/6966/revisions/2/mbox

Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (ro-ivb2-i7-3770)

ro-bdw-i5-5250u  total:219  pass:181  dwarn:0   dfail:0   fail:0   skip:38 
ro-bdw-i7-5557U  total:219  pass:206  dwarn:0   dfail:0   fail:0   skip:13 
ro-bdw-i7-5600u  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-bsw-n3050     total:219  pass:175  dwarn:0   dfail:0   fail:2   skip:42 
ro-byt-n2820     total:218  pass:174  dwarn:0   dfail:0   fail:3   skip:41 
ro-hsw-i3-4010u  total:218  pass:193  dwarn:0   dfail:0   fail:0   skip:25 
ro-hsw-i7-4770r  total:219  pass:194  dwarn:0   dfail:0   fail:0   skip:25 
ro-ilk-i7-620lm  total:219  pass:151  dwarn:0   dfail:0   fail:1   skip:67 
ro-ilk1-i5-650   total:214  pass:152  dwarn:0   dfail:0   fail:1   skip:61 
ro-ivb-i7-3770   total:219  pass:183  dwarn:0   dfail:0   fail:0   skip:36 
ro-ivb2-i7-3770  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-skl-i7-6700hq total:214  pass:189  dwarn:0   dfail:0   fail:0   skip:25 
ro-snb-i7-2620M  total:219  pass:177  dwarn:0   dfail:0   fail:1   skip:41 

Results at /archive/results/CI_IGT_test/RO_Patchwork_905/

7c7c832 drm-intel-nightly: 2016y-05m-16d-07h-51m-48s UTC integration manifest
0a92540 drm/i915/dmc: Step away from symbolic links

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