From: Andi Shyti <andi.shyti@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/i915/selftests: Provide a mock GPU reset routine
Date: Fri, 27 Sep 2019 23:41:19 +0300 [thread overview]
Message-ID: <20190927204119.GA2902@intel.intel> (raw)
In-Reply-To: <20190927191443.14126-1-chris@chris-wilson.co.uk>
Hi Chris,
On Fri, Sep 27, 2019 at 08:14:42PM +0100, Chris Wilson wrote:
> For those mock tests that may wish to pretend triggering a GPU reset and
> processing the cleanup.
The patch is OK, per se, but I think it should be split in two
parts:
- the i915 to gt conversion (that is the biggest part of the
patch)
- the mock-reset part (baskically the function)
right?
Andi
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Andi Shyti <andi.shyti@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_reset.c | 32 +++++++++++++------
> drivers/gpu/drm/i915/gt/intel_reset.h | 5 ++-
> drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 12 +++----
> drivers/gpu/drm/i915/gt/selftest_lrc.c | 2 +-
> drivers/gpu/drm/i915/gt/selftest_reset.c | 4 +--
> .../gpu/drm/i915/gt/selftest_workarounds.c | 8 ++---
> drivers/gpu/drm/i915/i915_getparam.c | 4 +--
> 8 files changed, 41 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 8f125f1624bd..7758a3744626 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4273,7 +4273,7 @@ __intel_display_resume(struct drm_device *dev,
> static bool gpu_reset_clobbers_display(struct drm_i915_private *dev_priv)
> {
> return (INTEL_INFO(dev_priv)->gpu_reset_clobbers_display &&
> - intel_has_gpu_reset(dev_priv));
> + intel_has_gpu_reset(&dev_priv->gt));
> }
>
> void intel_prepare_reset(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index d08226f5bea5..76938fa3a1b9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -542,13 +542,24 @@ static int gen8_reset_engines(struct intel_gt *gt,
> return ret;
> }
>
> +static int mock_reset(struct intel_gt *gt,
> + intel_engine_mask_t mask,
> + unsigned int retry)
> +{
> + return 0;
> +}
> +
> typedef int (*reset_func)(struct intel_gt *,
> intel_engine_mask_t engine_mask,
> unsigned int retry);
>
> -static reset_func intel_get_gpu_reset(struct drm_i915_private *i915)
> +static reset_func intel_get_gpu_reset(const struct intel_gt *gt)
> {
> - if (INTEL_GEN(i915) >= 8)
> + struct drm_i915_private *i915 = gt->i915;
> +
> + if (is_mock_gt(gt))
> + return mock_reset;
> + else if (INTEL_GEN(i915) >= 8)
> return gen8_reset_engines;
> else if (INTEL_GEN(i915) >= 6)
> return gen6_reset_engines;
> @@ -571,7 +582,7 @@ int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask)
> int ret = -ETIMEDOUT;
> int retry;
>
> - reset = intel_get_gpu_reset(gt->i915);
> + reset = intel_get_gpu_reset(gt);
> if (!reset)
> return -ENODEV;
>
> @@ -591,17 +602,20 @@ int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask)
> return ret;
> }
>
> -bool intel_has_gpu_reset(struct drm_i915_private *i915)
> +bool intel_has_gpu_reset(const struct intel_gt *gt)
> {
> if (!i915_modparams.reset)
> return NULL;
>
> - return intel_get_gpu_reset(i915);
> + return intel_get_gpu_reset(gt);
> }
>
> -bool intel_has_reset_engine(struct drm_i915_private *i915)
> +bool intel_has_reset_engine(const struct intel_gt *gt)
> {
> - return INTEL_INFO(i915)->has_reset_engine && i915_modparams.reset >= 2;
> + if (i915_modparams.reset < 2)
> + return false;
> +
> + return INTEL_INFO(gt->i915)->has_reset_engine;
> }
>
> int intel_reset_guc(struct intel_gt *gt)
> @@ -958,7 +972,7 @@ void intel_gt_reset(struct intel_gt *gt,
>
> awake = reset_prepare(gt);
>
> - if (!intel_has_gpu_reset(gt->i915)) {
> + if (!intel_has_gpu_reset(gt)) {
> if (i915_modparams.reset)
> dev_err(gt->i915->drm.dev, "GPU reset not supported\n");
> else
> @@ -1179,7 +1193,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
> * Try engine reset when available. We fall back to full reset if
> * single reset fails.
> */
> - if (intel_has_reset_engine(gt->i915) && !intel_gt_is_wedged(gt)) {
> + if (intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) {
> for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
> BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE);
> if (test_and_set_bit(I915_RESET_ENGINE + engine->id,
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.h b/drivers/gpu/drm/i915/gt/intel_reset.h
> index 0b6ff1ee7f06..8e8d5f761166 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.h
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.h
> @@ -14,7 +14,6 @@
> #include "intel_engine_types.h"
> #include "intel_reset_types.h"
>
> -struct drm_i915_private;
> struct i915_request;
> struct intel_engine_cs;
> struct intel_gt;
> @@ -80,7 +79,7 @@ static inline bool __intel_reset_failed(const struct intel_reset *reset)
> return unlikely(test_bit(I915_WEDGED, &reset->flags));
> }
>
> -bool intel_has_gpu_reset(struct drm_i915_private *i915);
> -bool intel_has_reset_engine(struct drm_i915_private *i915);
> +bool intel_has_gpu_reset(const struct intel_gt *gt);
> +bool intel_has_reset_engine(const struct intel_gt *gt);
>
> #endif /* I915_RESET_H */
> diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> index a0098fc35921..9c0c8441c22a 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> @@ -458,7 +458,7 @@ static int igt_reset_nop_engine(void *arg)
>
> /* Check that we can engine-reset during non-user portions */
>
> - if (!intel_has_reset_engine(gt->i915))
> + if (!intel_has_reset_engine(gt))
> return 0;
>
> file = mock_file(gt->i915);
> @@ -559,7 +559,7 @@ static int __igt_reset_engine(struct intel_gt *gt, bool active)
>
> /* Check that we can issue an engine reset on an idle engine (no-op) */
>
> - if (!intel_has_reset_engine(gt->i915))
> + if (!intel_has_reset_engine(gt))
> return 0;
>
> if (active) {
> @@ -791,7 +791,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
> * with any other engine.
> */
>
> - if (!intel_has_reset_engine(gt->i915))
> + if (!intel_has_reset_engine(gt))
> return 0;
>
> if (flags & TEST_ACTIVE) {
> @@ -1547,7 +1547,7 @@ static int igt_handle_error(void *arg)
>
> /* Check that we can issue a global GPU and engine reset */
>
> - if (!intel_has_reset_engine(gt->i915))
> + if (!intel_has_reset_engine(gt))
> return 0;
>
> if (!engine || !intel_engine_can_store_dword(engine))
> @@ -1689,7 +1689,7 @@ static int igt_reset_engines_atomic(void *arg)
>
> /* Check that the engines resets are usable from atomic context */
>
> - if (!intel_has_reset_engine(gt->i915))
> + if (!intel_has_reset_engine(gt))
> return 0;
>
> if (USES_GUC_SUBMISSION(gt->i915))
> @@ -1746,7 +1746,7 @@ int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
> bool saved_hangcheck;
> int err;
>
> - if (!intel_has_gpu_reset(gt->i915))
> + if (!intel_has_gpu_reset(gt))
> return 0;
>
> if (intel_gt_is_wedged(gt))
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index 22ea2e747064..93f2fcdc49bf 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -1310,7 +1310,7 @@ static int live_preempt_hang(void *arg)
> if (!HAS_LOGICAL_RING_PREEMPTION(i915))
> return 0;
>
> - if (!intel_has_reset_engine(i915))
> + if (!intel_has_reset_engine(&i915->gt))
> return 0;
>
> mutex_lock(&i915->drm.struct_mutex);
> diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
> index 00a4f60cdfd5..d79482db7fe8 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_reset.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_reset.c
> @@ -112,7 +112,7 @@ static int igt_atomic_engine_reset(void *arg)
>
> /* Check that the resets are usable from atomic context */
>
> - if (!intel_has_reset_engine(gt->i915))
> + if (!intel_has_reset_engine(gt))
> return 0;
>
> if (USES_GUC_SUBMISSION(gt->i915))
> @@ -170,7 +170,7 @@ int intel_reset_live_selftests(struct drm_i915_private *i915)
> };
> struct intel_gt *gt = &i915->gt;
>
> - if (!intel_has_gpu_reset(gt->i915))
> + if (!intel_has_gpu_reset(gt))
> return 0;
>
> if (intel_gt_is_wedged(gt))
> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> index 999a98f00494..d40ce0709bff 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -747,7 +747,7 @@ static int live_reset_whitelist(void *arg)
>
> igt_global_reset_lock(&i915->gt);
>
> - if (intel_has_reset_engine(i915)) {
> + if (intel_has_reset_engine(&i915->gt)) {
> err = check_whitelist_across_reset(engine,
> do_engine_reset,
> "engine");
> @@ -755,7 +755,7 @@ static int live_reset_whitelist(void *arg)
> goto out;
> }
>
> - if (intel_has_gpu_reset(i915)) {
> + if (intel_has_gpu_reset(&i915->gt)) {
> err = check_whitelist_across_reset(engine,
> do_device_reset,
> "device");
> @@ -1131,7 +1131,7 @@ live_gpu_reset_workarounds(void *arg)
> struct wa_lists lists;
> bool ok;
>
> - if (!intel_has_gpu_reset(i915))
> + if (!intel_has_gpu_reset(&i915->gt))
> return 0;
>
> ctx = kernel_context(i915);
> @@ -1178,7 +1178,7 @@ live_engine_reset_workarounds(void *arg)
> struct wa_lists lists;
> int ret = 0;
>
> - if (!intel_has_reset_engine(i915))
> + if (!intel_has_reset_engine(&i915->gt))
> return 0;
>
> ctx = kernel_context(i915);
> diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
> index 5d9101376a3d..f4b3cbb1adce 100644
> --- a/drivers/gpu/drm/i915/i915_getparam.c
> +++ b/drivers/gpu/drm/i915/i915_getparam.c
> @@ -79,8 +79,8 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
> break;
> case I915_PARAM_HAS_GPU_RESET:
> value = i915_modparams.enable_hangcheck &&
> - intel_has_gpu_reset(i915);
> - if (value && intel_has_reset_engine(i915))
> + intel_has_gpu_reset(&i915->gt);
> + if (value && intel_has_reset_engine(&i915->gt))
> value = 2;
> break;
> case I915_PARAM_HAS_RESOURCE_STREAMER:
> --
> 2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-09-27 20:41 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-27 19:14 [PATCH 1/2] drm/i915/selftests: Provide a mock GPU reset routine Chris Wilson
2019-09-27 19:14 ` [PATCH 2/2] drm/i915/selftests; Do not try to sanitize mock HW Chris Wilson
2019-09-27 20:45 ` Andi Shyti
2019-09-27 20:12 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/selftests: Provide a mock GPU reset routine Patchwork
2019-09-27 20:41 ` Andi Shyti [this message]
2019-09-27 20:54 ` [PATCH 1/2] " Chris Wilson
2019-09-27 21:09 ` Chris Wilson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190927204119.GA2902@intel.intel \
--to=andi.shyti@intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.