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

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

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.

> @@ -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
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx