On 28/06/17 01:54, Tvrtko Ursulin wrote:
On 14/06/2017 00:19, Kelvin Gardiner wrote:
It is sometimes useful for debug purposes to be able to set GuC timeout
lengths.
This patch adds GuC load and request timeouts values to Kconfig.debug,
which can then be optionally set as required for debug cases. A default
value equal to the current hard-coded values are provided.
In the case when a Kconfig option has not been set, a default value is
provide using a define.
Signed-off-by: Kelvin Gardiner <kelvin.gardiner@xxxxxxxxx>
---
drivers/gpu/drm/i915/Kconfig.debug | 40
+++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_drv.h | 13 +++++++++++
drivers/gpu/drm/i915/intel_guc_loader.c | 3 ++-
drivers/gpu/drm/i915/intel_uc.c | 5 ++++-
4 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/Kconfig.debug
b/drivers/gpu/drm/i915/Kconfig.debug
index 78c5c04..6a0767d 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -112,3 +112,43 @@ config DRM_I915_DEBUG_VBLANK_EVADE
the vblank.
If in doubt, say "N".
+
+config DRM_I915_OVERRIDE_TIMEOUTS
+ bool "Enable timeout overrides"
+ depends on DRM_I915
+ default n
+ help
+ Enable this option to allow overriding of selected timeouts
in the
+ driver i915.
+
+ Timeouts should only be overridden for debug and not in normal
use.
+
+ If in doubt, say "N".
If you drop this config...
+
+config DRM_I915_TIMEOUT_GUC_LOAD
+ int "Set the value of the GuC load timeout"
+ depends on DRM_I915_OVERRIDE_TIMEOUTS
... and this depends line...
+ default 100
+ range 0 10000
+ help
+ Set this option to select the value of the timeout in ms for
how long
+ the GuC FW load should take.
+
+ The valid range is 0 to 10000
+
+ The default timeout will work in normal use. This option is
provided
+ for debug.
+
+config DRM_I915_TIMEOUT_GUC_REQUEST
+ int "Set the value of the GuC request timeout"
+ depends on DRM_I915_OVERRIDE_TIMEOUTS
... and this one...
+ default 10
+ range 0 1000
+ help
+ Set this option to select the value of the timeout in ms
for how long
+ a request to the GuC should take.
+
+ The valid range is 0 to 1000
+
+ The default timeout will work in normal use. This option is
provided
+ for debug.
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 38ef734..efc56d2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1488,6 +1488,19 @@ struct drm_i915_error_state_buf {
#define I915_ENGINE_DEAD_TIMEOUT (4 * HZ) /* Seqno, head and
subunits dead */
#define I915_SEQNO_DEAD_TIMEOUT (12 * HZ) /* Seqno dead with
active head */
+#ifndef CONFIG_DRM_I915_TIMEOUT_GUC_LOAD
+#define DRM_I915_TIMEOUT_GUC_LOAD 100
+#else
+#define DRM_I915_TIMEOUT_GUC_LOAD CONFIG_DRM_I915_TIMEOUT_GUC_LOAD
+#endif
+
+#ifndef CONFIG_DRM_I915_TIMEOUT_GUC_REQUEST
+#define DRM_I915_TIMEOUT_GUC_REQUEST 10
+#else
+#define DRM_I915_TIMEOUT_GUC_REQUEST CONFIG_DRM_I915_TIMEOUT_GUC_REQUEST
+#endif
+
+
... and this whole block ...
struct i915_gpu_error {
/* For hangcheck timer */
#define DRM_I915_HANGCHECK_PERIOD 1500 /* in ms */
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 8b0ae7f..0d7abad 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -235,7 +235,8 @@ static int guc_ucode_xfer_dma(struct
drm_i915_private *dev_priv,
* (Higher levels of the driver will attempt to fall back to
* execlist mode if this happens.)
*/
- ret = wait_for(guc_ucode_response(dev_priv, &status), 100);
+ ret = wait_for(guc_ucode_response(dev_priv, &status),
+ DRM_I915_TIMEOUT_GUC_LOAD);
... and use CONFIG_DRM_I915_TIMEOUT_GUC_LOAD directly here ...
DRM_DEBUG_DRIVER("DMA status 0x%x, GuC status 0x%x\n",
I915_READ(DMA_CTRL), status);
diff --git a/drivers/gpu/drm/i915/intel_uc.c
b/drivers/gpu/drm/i915/intel_uc.c
index 27e072c..de34119 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -501,7 +501,10 @@ int intel_guc_send_mmio(struct intel_guc *guc,
const u32 *action, u32 len)
guc_send_reg(guc, 0),
INTEL_GUC_RECV_MASK,
INTEL_GUC_RECV_MASK,
- 10, 10, &status);
+ 10,
+ DRM_I915_TIMEOUT_GUC_REQUEST,
... and the same here respectively, then the patch becomes somewhat
shorter.
Agreed. I went with this implementation after some discussion with others.
I'll send a version with the changes you suggest, and we can see which
seems cleaner.
Question is whether that would be worth it, or having an explicit master
toggle is more desirable?
Since both timeout options provide defaults, and they are under the
debug submenu already, I thought we could get away with less. Especially
the #ifdef block in i915_drv.h I think is desirable to avoid if we can.
Regards,
Tvrtko
+ &status);
+
if (status != INTEL_GUC_STATUS_SUCCESS) {
/*
* Either the GuC explicitly returned an error (which
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
