public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Allow the user to override default firmware file names
@ 2017-02-06 13:05 Chris Wilson
  2017-02-06 13:52 ` Michal Wajdeczko
  2017-02-06 14:41 ` Jani Nikula
  0 siblings, 2 replies; 4+ messages in thread
From: Chris Wilson @ 2017-02-06 13:05 UTC (permalink / raw)
  To: intel-gfx

Make the actual path we attempt to load for each of (DMC, GuC, HuC)
overidable by the user. This helps testing new firmware releases.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_params.c      | 12 ++++++++++++
 drivers/gpu/drm/i915/i915_params.h      |  3 +++
 drivers/gpu/drm/i915/intel_csr.c        |  9 ++++++++-
 drivers/gpu/drm/i915/intel_guc_loader.c |  4 ++++
 drivers/gpu/drm/i915/intel_huc.c        |  8 ++++++++
 5 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index f1fa51190dc6..d7001c0effa5 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -243,3 +243,15 @@ MODULE_PARM_DESC(enable_dpcd_backlight,
 module_param_named(enable_gvt, i915.enable_gvt, bool, 0400);
 MODULE_PARM_DESC(enable_gvt,
 	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
+
+module_param_named(dmc_firmware, i915.dmc_firmware_path, charp, 0400);
+MODULE_PARM_DESC(dmc_firmare,
+	"Provide an alternate name for the DMC firmware file");
+
+module_param_named(guc_firmware, i915.guc_firmware_path, charp, 0400);
+MODULE_PARM_DESC(guc_firmare,
+	"Provide an alternate name for the GuC firmware file");
+
+module_param_named(huc_firmware, i915.huc_firmware_path, charp, 0400);
+MODULE_PARM_DESC(huc_firmare,
+	"Provide an alternate name for the HuC firmware file");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 55d47eea172e..d35d6ff600c1 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -28,6 +28,9 @@
 #include <linux/cache.h> /* for __read_mostly */
 
 #define I915_PARAMS_FOR_EACH(func) \
+	func(char *, dmc_firmware_path); \
+	func(char *, guc_firmware_path); \
+	func(char *, huc_firmware_path); \
 	func(int, modeset); \
 	func(int, panel_ignore_lid); \
 	func(int, semaphores); \
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 9dcc434d3b74..49932f85a786 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -447,7 +447,14 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv)
 		csr->fw_path = I915_CSR_SKL;
 	else if (IS_BROXTON(dev_priv))
 		csr->fw_path = I915_CSR_BXT;
-	else {
+	else
+		csr->fw_path = "";
+
+	/* Allow the user to override the default path via a module parameter */
+	if (csr->fw_path && i915.dmc_firmware_path)
+		csr->fw_path = i915.dmc_firmware_path;
+
+	if (!*csr->fw_path) {
 		DRM_ERROR("Unexpected: no known CSR firmware for platform\n");
 		return;
 	}
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 70637902c30d..e21b5c21d463 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -764,6 +764,10 @@ void intel_guc_init(struct drm_i915_private *dev_priv)
 		fw_path = "";	/* unknown device */
 	}
 
+	/* Allow the user to override the default path via a module parameter */
+	if (fw_path && i915.guc_firmware_path)
+		fw_path = i915.guc_firmware_path;
+
 	guc_fw->path = fw_path;
 	guc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE;
 	guc_fw->load_status = INTEL_UC_FIRMWARE_NONE;
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index c28543d220a2..a1d748085ded 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -178,8 +178,16 @@ void intel_huc_init(struct drm_i915_private *dev_priv)
 		fw_path = I915_KBL_HUC_UCODE;
 		huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR;
 		huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR;
+	} else {
+		fw_path = "";
 	}
 
+	/* Allow the user to override the default path via a module parameter */
+	if (fw_path && i915.huc_firmware_path)
+		fw_path = i915.huc_firmware_path;
+	if (!*fw_path)
+		return;
+
 	huc_fw->path = fw_path;
 	huc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
 
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Allow the user to override default firmware file names
  2017-02-06 13:05 [PATCH] drm/i915: Allow the user to override default firmware file names Chris Wilson
@ 2017-02-06 13:52 ` Michal Wajdeczko
  2017-02-06 14:18   ` Chris Wilson
  2017-02-06 14:41 ` Jani Nikula
  1 sibling, 1 reply; 4+ messages in thread
From: Michal Wajdeczko @ 2017-02-06 13:52 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Mon, Feb 06, 2017 at 01:05:29PM +0000, Chris Wilson wrote:
> Make the actual path we attempt to load for each of (DMC, GuC, HuC)
> overidable by the user. This helps testing new firmware releases.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_params.c      | 12 ++++++++++++
>  drivers/gpu/drm/i915/i915_params.h      |  3 +++
>  drivers/gpu/drm/i915/intel_csr.c        |  9 ++++++++-
>  drivers/gpu/drm/i915/intel_guc_loader.c |  4 ++++
>  drivers/gpu/drm/i915/intel_huc.c        |  8 ++++++++
>  5 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index f1fa51190dc6..d7001c0effa5 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -243,3 +243,15 @@ MODULE_PARM_DESC(enable_dpcd_backlight,
>  module_param_named(enable_gvt, i915.enable_gvt, bool, 0400);
>  MODULE_PARM_DESC(enable_gvt,
>  	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
> +
> +module_param_named(dmc_firmware, i915.dmc_firmware_path, charp, 0400);
> +MODULE_PARM_DESC(dmc_firmare,
> +	"Provide an alternate name for the DMC firmware file");
> +
> +module_param_named(guc_firmware, i915.guc_firmware_path, charp, 0400);
> +MODULE_PARM_DESC(guc_firmare,
> +	"Provide an alternate name for the GuC firmware file");
> +
> +module_param_named(huc_firmware, i915.huc_firmware_path, charp, 0400);
> +MODULE_PARM_DESC(huc_firmare,
> +	"Provide an alternate name for the HuC firmware file");
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index 55d47eea172e..d35d6ff600c1 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -28,6 +28,9 @@
>  #include <linux/cache.h> /* for __read_mostly */
>  
>  #define I915_PARAMS_FOR_EACH(func) \
> +	func(char *, dmc_firmware_path); \
> +	func(char *, guc_firmware_path); \
> +	func(char *, huc_firmware_path); \
>  	func(int, modeset); \
>  	func(int, panel_ignore_lid); \
>  	func(int, semaphores); \
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 9dcc434d3b74..49932f85a786 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -447,7 +447,14 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv)
>  		csr->fw_path = I915_CSR_SKL;
>  	else if (IS_BROXTON(dev_priv))
>  		csr->fw_path = I915_CSR_BXT;
> -	else {
> +	else
> +		csr->fw_path = "";
> +
> +	/* Allow the user to override the default path via a module parameter */
> +	if (csr->fw_path && i915.dmc_firmware_path)

First check for null fw_path is redundant here, as it looks that at least "" will be used.

> +		csr->fw_path = i915.dmc_firmware_path;
> +
> +	if (!*csr->fw_path) {
>  		DRM_ERROR("Unexpected: no known CSR firmware for platform\n");

This error message will be incorrect when user provides empty path.
Maybe we should just check for null fw_path and let firmware api to report failure?

Same comments for Guc&huc...

Regards,
Michal

>  		return;
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 70637902c30d..e21b5c21d463 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -764,6 +764,10 @@ void intel_guc_init(struct drm_i915_private *dev_priv)
>  		fw_path = "";	/* unknown device */
>  	}
>  
> +	/* Allow the user to override the default path via a module parameter */
> +	if (fw_path && i915.guc_firmware_path)
> +		fw_path = i915.guc_firmware_path;
> +
>  	guc_fw->path = fw_path;
>  	guc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE;
>  	guc_fw->load_status = INTEL_UC_FIRMWARE_NONE;
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index c28543d220a2..a1d748085ded 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -178,8 +178,16 @@ void intel_huc_init(struct drm_i915_private *dev_priv)
>  		fw_path = I915_KBL_HUC_UCODE;
>  		huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR;
>  		huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR;
> +	} else {
> +		fw_path = "";
>  	}
>  
> +	/* Allow the user to override the default path via a module parameter */
> +	if (fw_path && i915.huc_firmware_path)
> +		fw_path = i915.huc_firmware_path;
> +	if (!*fw_path)
> +		return;
> +
>  	huc_fw->path = fw_path;
>  	huc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
>  
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Allow the user to override default firmware file names
  2017-02-06 13:52 ` Michal Wajdeczko
@ 2017-02-06 14:18   ` Chris Wilson
  0 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2017-02-06 14:18 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

On Mon, Feb 06, 2017 at 02:52:14PM +0100, Michal Wajdeczko wrote:
> On Mon, Feb 06, 2017 at 01:05:29PM +0000, Chris Wilson wrote:
> > Make the actual path we attempt to load for each of (DMC, GuC, HuC)
> > overidable by the user. This helps testing new firmware releases.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_params.c      | 12 ++++++++++++
> >  drivers/gpu/drm/i915/i915_params.h      |  3 +++
> >  drivers/gpu/drm/i915/intel_csr.c        |  9 ++++++++-
> >  drivers/gpu/drm/i915/intel_guc_loader.c |  4 ++++
> >  drivers/gpu/drm/i915/intel_huc.c        |  8 ++++++++
> >  5 files changed, 35 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> > index f1fa51190dc6..d7001c0effa5 100644
> > --- a/drivers/gpu/drm/i915/i915_params.c
> > +++ b/drivers/gpu/drm/i915/i915_params.c
> > @@ -243,3 +243,15 @@ MODULE_PARM_DESC(enable_dpcd_backlight,
> >  module_param_named(enable_gvt, i915.enable_gvt, bool, 0400);
> >  MODULE_PARM_DESC(enable_gvt,
> >  	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
> > +
> > +module_param_named(dmc_firmware, i915.dmc_firmware_path, charp, 0400);
> > +MODULE_PARM_DESC(dmc_firmare,
> > +	"Provide an alternate name for the DMC firmware file");
> > +
> > +module_param_named(guc_firmware, i915.guc_firmware_path, charp, 0400);
> > +MODULE_PARM_DESC(guc_firmare,
> > +	"Provide an alternate name for the GuC firmware file");
> > +
> > +module_param_named(huc_firmware, i915.huc_firmware_path, charp, 0400);
> > +MODULE_PARM_DESC(huc_firmare,
> > +	"Provide an alternate name for the HuC firmware file");
> > diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> > index 55d47eea172e..d35d6ff600c1 100644
> > --- a/drivers/gpu/drm/i915/i915_params.h
> > +++ b/drivers/gpu/drm/i915/i915_params.h
> > @@ -28,6 +28,9 @@
> >  #include <linux/cache.h> /* for __read_mostly */
> >  
> >  #define I915_PARAMS_FOR_EACH(func) \
> > +	func(char *, dmc_firmware_path); \
> > +	func(char *, guc_firmware_path); \
> > +	func(char *, huc_firmware_path); \
> >  	func(int, modeset); \
> >  	func(int, panel_ignore_lid); \
> >  	func(int, semaphores); \
> > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> > index 9dcc434d3b74..49932f85a786 100644
> > --- a/drivers/gpu/drm/i915/intel_csr.c
> > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > @@ -447,7 +447,14 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv)
> >  		csr->fw_path = I915_CSR_SKL;
> >  	else if (IS_BROXTON(dev_priv))
> >  		csr->fw_path = I915_CSR_BXT;
> > -	else {
> > +	else
> > +		csr->fw_path = "";
> > +
> > +	/* Allow the user to override the default path via a module parameter */
> > +	if (csr->fw_path && i915.dmc_firmware_path)
> 
> First check for null fw_path is redundant here, as it looks that at least "" will be used.

I was preferring to use a consistent method for all fw paths, since one
path was using that method, I'd prefer all to use the same and some
bright soul to refactor.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Allow the user to override default firmware file names
  2017-02-06 13:05 [PATCH] drm/i915: Allow the user to override default firmware file names Chris Wilson
  2017-02-06 13:52 ` Michal Wajdeczko
@ 2017-02-06 14:41 ` Jani Nikula
  1 sibling, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2017-02-06 14:41 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On Mon, 06 Feb 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Make the actual path we attempt to load for each of (DMC, GuC, HuC)
> overidable by the user. This helps testing new firmware releases.

And old firmware releases, and potentially helps bisect the kernel by
eliminating firmware changes.

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_params.c      | 12 ++++++++++++
>  drivers/gpu/drm/i915/i915_params.h      |  3 +++
>  drivers/gpu/drm/i915/intel_csr.c        |  9 ++++++++-
>  drivers/gpu/drm/i915/intel_guc_loader.c |  4 ++++
>  drivers/gpu/drm/i915/intel_huc.c        |  8 ++++++++
>  5 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index f1fa51190dc6..d7001c0effa5 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -243,3 +243,15 @@ MODULE_PARM_DESC(enable_dpcd_backlight,
>  module_param_named(enable_gvt, i915.enable_gvt, bool, 0400);
>  MODULE_PARM_DESC(enable_gvt,
>  	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
> +
> +module_param_named(dmc_firmware, i915.dmc_firmware_path, charp, 0400);
> +MODULE_PARM_DESC(dmc_firmare,
> +	"Provide an alternate name for the DMC firmware file");
> +
> +module_param_named(guc_firmware, i915.guc_firmware_path, charp, 0400);
> +MODULE_PARM_DESC(guc_firmare,
> +	"Provide an alternate name for the GuC firmware file");
> +
> +module_param_named(huc_firmware, i915.huc_firmware_path, charp, 0400);
> +MODULE_PARM_DESC(huc_firmare,
> +	"Provide an alternate name for the HuC firmware file");

Please make them _unsafe. With that,

Acked-by: Jani Nikula <jani.nikula@intel.com>


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

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-02-06 14:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-06 13:05 [PATCH] drm/i915: Allow the user to override default firmware file names Chris Wilson
2017-02-06 13:52 ` Michal Wajdeczko
2017-02-06 14:18   ` Chris Wilson
2017-02-06 14:41 ` Jani Nikula

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox