* [RFC] drm/i915: Add GuC timeout config options to Kconfig.debug
@ 2017-06-13 23:19 Kelvin Gardiner
2017-06-13 23:41 ` ✓ Fi.CI.BAT: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Kelvin Gardiner @ 2017-06-13 23:19 UTC (permalink / raw)
To: intel-gfx
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@intel.com>
---
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".
+
+config DRM_I915_TIMEOUT_GUC_LOAD
+ int "Set the value of the GuC load timeout"
+ depends on DRM_I915_OVERRIDE_TIMEOUTS
+ 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
+ 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
+
+
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);
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,
+ &status);
+
if (status != INTEL_GUC_STATUS_SUCCESS) {
/*
* Either the GuC explicitly returned an error (which
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Add GuC timeout config options to Kconfig.debug
2017-06-13 23:19 [RFC] drm/i915: Add GuC timeout config options to Kconfig.debug Kelvin Gardiner
@ 2017-06-13 23:41 ` Patchwork
2017-06-27 21:22 ` [RFC] " Kelvin Gardiner
2017-06-28 8:54 ` Tvrtko Ursulin
2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2017-06-13 23:41 UTC (permalink / raw)
To: Kelvin Gardiner; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Add GuC timeout config options to Kconfig.debug
URL : https://patchwork.freedesktop.org/series/25741/
State : success
== Summary ==
Series 25741v1 drm/i915: Add GuC timeout config options to Kconfig.debug
https://patchwork.freedesktop.org/api/1.0/series/25741/revisions/1/mbox/
Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass -> DMESG-WARN (fi-kbl-r) fdo#100125
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-c:
pass -> INCOMPLETE (fi-bsw-n3050) fdo#100651
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#100651 https://bugs.freedesktop.org/show_bug.cgi?id=100651
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:443s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:430s
fi-bsw-n3050 total:237 pass:204 dwarn:0 dfail:0 fail:0 skip:32
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:509s
fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:491s
fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:485s
fi-glk-2a total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:592s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:427s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:412s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:414s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:489s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:465s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:470s
fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 time:569s
fi-kbl-r total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time:575s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:468s
fi-skl-6700hq total:278 pass:228 dwarn:1 dfail:0 fail:27 skip:22 time:404s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:468s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:475s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:448s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:544s
fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time:411s
d4bedb8b0f9ba91df2e8cb136a489145a83e96a7 drm-tip: 2017y-06m-13d-14h-22m-46s UTC integration manifest
b37f219 drm/i915: Add GuC timeout config options to Kconfig.debug
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4946/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] drm/i915: Add GuC timeout config options to Kconfig.debug
2017-06-13 23:19 [RFC] drm/i915: Add GuC timeout config options to Kconfig.debug Kelvin Gardiner
2017-06-13 23:41 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-06-27 21:22 ` Kelvin Gardiner
2017-06-28 8:54 ` Tvrtko Ursulin
2 siblings, 0 replies; 5+ messages in thread
From: Kelvin Gardiner @ 2017-06-27 21:22 UTC (permalink / raw)
To: intel-gfx
Hi,
Any feedback on this?
Thanks,
Kelvin
On 13/06/17 16: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@intel.com>
> ---
> 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".
> +
> +config DRM_I915_TIMEOUT_GUC_LOAD
> + int "Set the value of the GuC load timeout"
> + depends on DRM_I915_OVERRIDE_TIMEOUTS
> + 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
> + 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
> +
> +
> 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);
>
> 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,
> + &status);
> +
> if (status != INTEL_GUC_STATUS_SUCCESS) {
> /*
> * Either the GuC explicitly returned an error (which
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] drm/i915: Add GuC timeout config options to Kconfig.debug
2017-06-13 23:19 [RFC] drm/i915: Add GuC timeout config options to Kconfig.debug Kelvin Gardiner
2017-06-13 23:41 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-06-27 21:22 ` [RFC] " Kelvin Gardiner
@ 2017-06-28 8:54 ` Tvrtko Ursulin
2017-07-14 20:57 ` Kelvin Gardiner
2 siblings, 1 reply; 5+ messages in thread
From: Tvrtko Ursulin @ 2017-06-28 8:54 UTC (permalink / raw)
To: Kelvin Gardiner, intel-gfx
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@intel.com>
> ---
> 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.
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] drm/i915: Add GuC timeout config options to Kconfig.debug
2017-06-28 8:54 ` Tvrtko Ursulin
@ 2017-07-14 20:57 ` Kelvin Gardiner
0 siblings, 0 replies; 5+ messages in thread
From: Kelvin Gardiner @ 2017-07-14 20:57 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
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@intel.com>
>> ---
>> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-07-14 20:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-13 23:19 [RFC] drm/i915: Add GuC timeout config options to Kconfig.debug Kelvin Gardiner
2017-06-13 23:41 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-06-27 21:22 ` [RFC] " Kelvin Gardiner
2017-06-28 8:54 ` Tvrtko Ursulin
2017-07-14 20:57 ` Kelvin Gardiner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.