diff mbox

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

Message ID 1463391057-32350-1-git-send-email-patrik.jakobsson@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Patrik Jakobsson May 16, 2016, 9:30 a.m. UTC
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(-)

Comments

Jani Nikula May 16, 2016, 12:04 p.m. UTC | #1
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.
Mika Kuoppala May 18, 2016, 10:24 a.m. UTC | #2
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
Patrik Jakobsson May 23, 2016, 8:57 a.m. UTC | #3
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
Rodrigo Vivi June 15, 2016, 12:11 a.m. UTC | #4
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

>
Patrik Jakobsson June 27, 2016, 10:57 a.m. UTC | #5
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
> >
Imre Deak June 27, 2016, 11:20 a.m. UTC | #6
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
> > > 
>
Rodrigo Vivi June 27, 2016, 4:32 p.m. UTC | #7
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
Imre Deak June 27, 2016, 4:51 p.m. UTC | #8
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;
> > > > > > >  	}
> > > > > > >  
> > > > > > > --
Rodrigo Vivi June 27, 2016, 5:12 p.m. UTC | #9
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;

> > > > > > > >  	}

> > > > > > > >  

> > > > > > > > --
Patrik Jakobsson June 27, 2016, 6:45 p.m. UTC | #10
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
Rodrigo Vivi June 27, 2016, 9:29 p.m. UTC | #11
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
diff mbox

Patch

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;
 	}