| Message ID | 1460566637-19638-1-git-send-email-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13/04/16 17:57, Ben Widawsky wrote: > For debug and development purposes only. > > Cc: Mika Kuoppala <mika.kuoppala@intel.com> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 13 +++++++++++++ > drivers/gpu/drm/i915/i915_gpu_error.c | 3 +++ > drivers/gpu/drm/i915/i915_params.c | 6 ++++++ > drivers/gpu/drm/i915/i915_params.h | 1 + > drivers/gpu/drm/i915/intel_csr.c | 11 +++++++++++ > drivers/gpu/drm/i915/intel_guc_loader.c | 11 +++++++++++ > 6 files changed, 45 insertions(+) You can already disable GuC loading in the existing driver by turning off GuC submission, or if you have the previously-posted "add enable_guc_loading" patch, then you can separately control the loading of the GuC firmware and the use of the GuC for batch submission. .Dave.
On Wed, Apr 13, 2016 at 06:45:32PM +0100, Dave Gordon wrote: > On 13/04/16 17:57, Ben Widawsky wrote: > > For debug and development purposes only. > > > > Cc: Mika Kuoppala <mika.kuoppala@intel.com> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/i915_debugfs.c | 13 +++++++++++++ > > drivers/gpu/drm/i915/i915_gpu_error.c | 3 +++ > > drivers/gpu/drm/i915/i915_params.c | 6 ++++++ > > drivers/gpu/drm/i915/i915_params.h | 1 + > > drivers/gpu/drm/i915/intel_csr.c | 11 +++++++++++ > > drivers/gpu/drm/i915/intel_guc_loader.c | 11 +++++++++++ > > 6 files changed, 45 insertions(+) > > You can already disable GuC loading in the existing driver by turning off > GuC submission, I saw this. I believe that behavior is incorrect but I didn't bother to fix it. GuC submission and GuC loading are separate. > or if you have the previously-posted "add enable_guc_loading" patch, then you > can separately control the loading of the GuC firmware and the use of the GuC > for batch submission. Sounds to me like that patch should be merged, and we could roll that into this and give me what I want. > > .Dave.
On Wed, 13 Apr 2016, Ben Widawsky <ben@bwidawsk.net> wrote: > +module_param_named_unsafe(disable_firmware_loading, i915.disable_firmware_loading, uint, 0400); > +MODULE_PARM_DESC(disable_firmware_loading, > + "Bypass loading of firmware based on mask. This overrides all other firmware module parameters. (0:all firmwares enabled (default), 1<<0:disable GuC, 1<<1:disable DMC"); I'd like all parameters like this to read "enable", defaulting to enabled or disabled, possibly with platform specific defaults. I know there's the underlying assumption that firmware loading is enabled by default, therefore disabling is a special case, so you call this disable. But we've already got enable_whatnot, with all kinds of defaults. The disable_whatnot ones are confusing. I can easily imagine adding platform specific defaults to enable_firmware_loading, particularly during platform enabling. BR, Jani.
On Thu, Apr 14, 2016 at 11:27:25AM +0300, Jani Nikula wrote: > On Wed, 13 Apr 2016, Ben Widawsky <ben@bwidawsk.net> wrote: > > +module_param_named_unsafe(disable_firmware_loading, i915.disable_firmware_loading, uint, 0400); > > +MODULE_PARM_DESC(disable_firmware_loading, > > + "Bypass loading of firmware based on mask. This overrides all other firmware module parameters. (0:all firmwares enabled (default), 1<<0:disable GuC, 1<<1:disable DMC"); > > I'd like all parameters like this to read "enable", defaulting to > enabled or disabled, possibly with platform specific defaults. > > I know there's the underlying assumption that firmware loading is > enabled by default, therefore disabling is a special case, so you call > this disable. But we've already got enable_whatnot, with all kinds of > defaults. The disable_whatnot ones are confusing. > > I can easily imagine adding platform specific defaults to > enable_firmware_loading, particularly during platform enabling. > > BR, > Jani. I'm happy to change it, but it'd be nice to have some kind of ack that someone will merge it before I bother with that. > > > -- > Jani Nikula, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index c0b28fa..75551b5 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2445,6 +2445,9 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data) struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; u32 tmp, i; + if (i915.disable_firmware_loading & (1<<0)) + return 0; + if (!HAS_GUC_UCODE(dev_priv)) return 0; @@ -2519,6 +2522,11 @@ static int i915_guc_info(struct seq_file *m, void *data) struct intel_engine_cs *engine; u64 total = 0; + if (i915.disable_firmware_loading & (1<<0)) { + seq_printf(m, "GuC disabled by module parameter.\n"); + return 0; + } + if (!HAS_GUC_SCHED(dev_priv)) return 0; @@ -2784,6 +2792,11 @@ static int i915_dmc_info(struct seq_file *m, void *unused) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_csr *csr; + if (i915.disable_firmware_loading & (1<<1)) { + seq_puts(m, "disabled by module parameter\n"); + return 0; + } + if (!HAS_CSR(dev)) { seq_puts(m, "not supported\n"); return 0; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 8562866..543477e 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -369,6 +369,9 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m, dev->pdev->subsystem_vendor, dev->pdev->subsystem_device); err_printf(m, "IOMMU enabled?: %d\n", error->iommu); + err_printf(m, "Firmwares disabled: %s%s\n", + i915.disable_firmware_loading & (1<<0) ? "GuC " : "", + i915.disable_firmware_loading & (1<<1) ? "DMC " : ""); if (HAS_CSR(dev)) { struct intel_csr *csr = &dev_priv->csr; diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 1779f02..6a73716 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -58,6 +58,7 @@ struct i915_params i915 __read_mostly = { .guc_log_level = -1, .enable_dp_mst = true, .inject_load_failure = 0, + .disable_firmware_loading = 0, }; module_param_named(modeset, i915.modeset, int, 0400); @@ -207,6 +208,11 @@ MODULE_PARM_DESC(guc_log_level, module_param_named_unsafe(enable_dp_mst, i915.enable_dp_mst, bool, 0600); MODULE_PARM_DESC(enable_dp_mst, "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)"); + module_param_named_unsafe(inject_load_failure, i915.inject_load_failure, uint, 0400); MODULE_PARM_DESC(inject_load_failure, "Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)"); + +module_param_named_unsafe(disable_firmware_loading, i915.disable_firmware_loading, uint, 0400); +MODULE_PARM_DESC(disable_firmware_loading, + "Bypass loading of firmware based on mask. This overrides all other firmware module parameters. (0:all firmwares enabled (default), 1<<0:disable GuC, 1<<1:disable DMC"); diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 02bc278..c1b6c0f 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -50,6 +50,7 @@ struct i915_params { int mmio_debug; int edp_vswing; unsigned int inject_load_failure; + unsigned int disable_firmware_loading; /* leave bools at the end to not create holes */ bool enable_hangcheck; bool fastboot; diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index 3f57cb9..d6f553b 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -246,6 +246,9 @@ void intel_csr_load_program(struct drm_i915_private *dev_priv) u32 *payload = dev_priv->csr.dmc_payload; uint32_t i, fw_size; + if (i915.disable_firmware_loading & (1<<1)) + return; + if (!IS_GEN9(dev_priv)) { DRM_ERROR("No CSR support available for this platform\n"); return; @@ -433,6 +436,11 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) INIT_WORK(&dev_priv->csr.work, csr_load_work_fn); + if (i915.disable_firmware_loading & (1<<1)) { + DRM_INFO("CSR support disabled by module parameter\n"); + return; + } + if (!HAS_CSR(dev_priv)) return; @@ -465,6 +473,9 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) */ void intel_csr_ucode_fini(struct drm_i915_private *dev_priv) { + if (i915.disable_firmware_loading & (1<<1)) + return; + if (!HAS_CSR(dev_priv)) return; diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 876e5da..cdc2c8b 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -389,6 +389,9 @@ int intel_guc_ucode_load(struct drm_device *dev) struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; int retries, err = 0; + if (i915.disable_firmware_loading & (1<<0)) + return 0; + if (!i915.enable_guc_submission) return 0; @@ -631,6 +634,11 @@ void intel_guc_ucode_init(struct drm_device *dev) struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; const char *fw_path; + if (i915.disable_firmware_loading & (1<<0)) { + DRM_INFO("GuC support disabled by module parameter\n"); + return; + } + if (!HAS_GUC_SCHED(dev)) i915.enable_guc_submission = false; @@ -677,6 +685,9 @@ void intel_guc_ucode_fini(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; + if (i915.disable_firmware_loading & (1<<0)) + return; + mutex_lock(&dev->struct_mutex); direct_interrupts_to_host(dev_priv); i915_guc_submission_disable(dev);
For debug and development purposes only. Cc: Mika Kuoppala <mika.kuoppala@intel.com> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_debugfs.c | 13 +++++++++++++ drivers/gpu/drm/i915/i915_gpu_error.c | 3 +++ drivers/gpu/drm/i915/i915_params.c | 6 ++++++ drivers/gpu/drm/i915/i915_params.h | 1 + drivers/gpu/drm/i915/intel_csr.c | 11 +++++++++++ drivers/gpu/drm/i915/intel_guc_loader.c | 11 +++++++++++ 6 files changed, 45 insertions(+)