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

Mon Jun 27 16:32:10 UTC 2016

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 at 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 at intel.com>
> > > > > > Cc: Imre Deak <imre.deak at intel.com>
> > > > > > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > > > > > Signed-off-by: Patrik Jakobsson 
> > > > > > > 
> > > > > > > 
> > > > > Reviewed-by: Mika Kuoppala <mika.kuoppala at 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 at 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