Re: [PATCH] drm/i915/dmc: Accept symbolic link in firmware name

Mika Kuoppala
2016-07-07T14:58:04Z

"Vivi, Rodrigo" <rodrigo.vivi@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. 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.

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

-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@fireburn.co.uk>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@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)
>>