Re: [PATCH 10/10] drm/i915/uc: Add params for specifying firmware

[Jani Nikula <jani.nikula@intel.com>]

On Mon, 27 Feb 2017, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote:
> On pe, 2017-02-24 at 16:40 +0100, Arkadiusz Hiler wrote:
>> `guc_firmware_path` and `huc_firmware_path` module parameters are added.
>> 
>> Using the parameter disabled version checks and loads desired firmware
>> instead of the default one.
>> 
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Michal Winiarski <michal.winiarski@intel.com>
>> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>
> <SNIP>
>
>> @@ -230,6 +232,14 @@ module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
>>  MODULE_PARM_DESC(guc_log_level,
>>  	"GuC firmware logging level (-1:disabled (default), 0-3:enabled)");
>>  
>> +module_param_named(guc_firmware_path, i915.guc_firmware_path, charp, 0400);
>
> I'm pretty sure this should be _unsafe, because it overrides the
> version checks. Cc'd Jani for this.

Yes, please. I replied the same thing to Chris' patches adding the same
thing.

BR,
Jani.

>
>> @@ -479,7 +479,11 @@ void intel_guc_select_fw(struct intel_guc *guc)
>>  	guc->fw.load_status = INTEL_UC_FIRMWARE_NONE;
>>  	guc->fw.fw = INTEL_UC_FW_TYPE_GUC;
>>  
>> -	if (IS_SKYLAKE(dev_priv)) {
>> +	if (i915.guc_firmware_path) {
>> +		guc->fw.path = i915.guc_firmware_path;
>> +		guc->fw.major_ver_wanted = 0;
>> +		guc->fw.minor_ver_wanted = 0;
>
> Or, we could keep the wanted version number, only replace the path, and
> spit out WARN/taint kernel if some other version was detected?
>
> But I guess the main purpose is to override version (not provide
> request_firmware workarounds), so my vote is to make the param _unsafe.
>
> Regards, Joonas

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