[Intel-gfx] [PATCH] drm/i915/dmc: Accept symbolic link in firmware name

[Vivi, Rodrigo]

On Thu, 2016-07-07 at 17:57 +0300, Mika Kuoppala wrote:
> "Vivi, Rodrigo" <rodrigo.vivi at intel.com> writes:
> 
> > 
> > Nak.
> > 
> > I don't intend to update the symbolic links on linux-firmware.git
> > repository anymore so if we receive a new minor version update we
> > are
> > not going to load.
> > 
> > I was the one advocating in the favor for the symbolic link
> > flexibility
> > but I lost the discussions for the stability and validation etc.
> > 
> And I was one advocating in favor of getting rid of symlink. 

oh true! indeed funny! :)

> But the
> filename versioning is superfluous as the contents has the version
> info
> which we can solely rely to not run something we dont want.

indeed.

> 
> So I am not sure what we lose in stability and validation front
> with the strict version check.

But I'm not sure what we gain though.

Is the reason https://bugs.freedesktop.org/show_bug.cgi?id=96800 ?

If the reason is that CONFIG_EXTRA_FIRMWARE variable if we increase the
major version we also cause an issue anyways. 

But also, we don't know the version the users using the
CONFIG_EXTRA_FIRMWARE is compiling and what exact version of the
firmware that source in that specific moment is requiring. How would we
be sure the users has the symbolic link pointing to the right version
that code is requiring?



> -Mika
> 
> > 
> > So let's just move away from symbolic link for good.
> > 
> > 
> > On Tue, 2016-07-05 at 12:25 +0300, Mika Kuoppala wrote:
> > > 
> > > We need the ability to explicitly load only a specified firmware
> > > version. As the firmware blob contains the version, we use
> > > that to filter out the ones we don't want. The version encoded
> > > into the firmware name is superfluous and we should allow user
> > > to point into specific firmware through a symlink, and only do
> > > filtering based on the version stamp included in the blob.
> > > This allows user to conveniently point to a firmware blob and
> > > still gives us the control of what we decided to run on.
> > > 
> > > This is partial revert of
> > > 4aa7fb9c3c4f ("drm/i915/dmc: Step away from symbolic links")
> > > 
> > > Fixes: 4aa7fb9c3c4f ("drm/i915/dmc: Step away from symbolic
> > > links")
> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=96800
> > > Reported-by: Mike Lothian <mike at fireburn.co.uk>
> > > 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>
> > > Cc: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>
> > > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > > Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_csr.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > > b/drivers/gpu/drm/i915/intel_csr.c
> > > index ea047cd46b71..77d01a0b64b4 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_01.bin"
> > > +#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin"
> > >  MODULE_FIRMWARE(I915_CSR_KBL);
> > >  #define KBL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 1)
> > >  
> > > -#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin"
> > > +#define I915_CSR_SKL "i915/skl_dmc_ver1.bin"
> > >  MODULE_FIRMWARE(I915_CSR_SKL);
> > >  #define SKL_CSR_VERSION_REQUIRED	CSR_VERSION(1, 26)
> > >  
> > > -#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin"
> > > +#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin"
> > >  MODULE_FIRMWARE(I915_CSR_BXT);
> > >  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
> > >