diff mbox series

[v2] drm/i915/selftests: Flush the old heartbeat more gently

Message ID 20201019142841.32273-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [v2] drm/i915/selftests: Flush the old heartbeat more gently | expand

Commit Message

Chris Wilson Oct. 19, 2020, 2:28 p.m. UTC
In order to test how fast the heartbeat can respond, we measure with the
interval set to its minimum. Before we measure though, we want to be
sure we start with a fresh pulse, and so wait until any old one is
complete. During that wait though, we were continually flushing the
work, and so continually re-evaluating to see if the pulse was complete,
and each attempt would count as an unresponsive system. If the engine
did not complete the request in the couple of busy-spins, it would flag
an error. This is unfortunate, so let's not busy-spin waiting for the
old heartbeat, but terminate it and start afresh.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Mika Kuoppala Oct. 21, 2020, 5:57 p.m. UTC | #1
Chris Wilson <chris@chris-wilson.co.uk> writes:

> In order to test how fast the heartbeat can respond, we measure with the
> interval set to its minimum. Before we measure though, we want to be
> sure we start with a fresh pulse, and so wait until any old one is
> complete. During that wait though, we were continually flushing the
> work, and so continually re-evaluating to see if the pulse was complete,
> and each attempt would count as an unresponsive system. If the engine
> did not complete the request in the couple of busy-spins, it would flag
> an error. This is unfortunate, so let's not busy-spin waiting for the
> old heartbeat, but terminate it and start afresh.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> index e73854dd2fe0..b88aa35ad75b 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> @@ -215,16 +215,17 @@ static int __live_heartbeat_fast(struct intel_engine_cs *engine)
>  		goto err_pm;
>  
>  	for (i = 0; i < ARRAY_SIZE(times); i++) {
> -		/* Manufacture a tick */
>  		do {
> -			while (READ_ONCE(engine->heartbeat.systole))
> -				flush_delayed_work(&engine->heartbeat.work);
> +			/* Manufacture a tick */
> +			intel_engine_park_heartbeat(engine);
> +			GEM_BUG_ON(engine->heartbeat.systole);
> +			engine->serial++; /*  pretend we are not idle! */
> +			intel_engine_unpark_heartbeat(engine);
>  
> -			engine->serial++; /* quick, pretend we are not idle! */
>  			flush_delayed_work(&engine->heartbeat.work);
>  			if (!delayed_work_pending(&engine->heartbeat.work)) {
> -				pr_err("%s: heartbeat did not start\n",
> -				       engine->name);
> +				pr_err("%s: heartbeat %d did not start\n",
> +				       engine->name, i);
>  				err = -EINVAL;
>  				goto err_pm;
>  			}
> -- 
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
index e73854dd2fe0..b88aa35ad75b 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
@@ -215,16 +215,17 @@  static int __live_heartbeat_fast(struct intel_engine_cs *engine)
 		goto err_pm;
 
 	for (i = 0; i < ARRAY_SIZE(times); i++) {
-		/* Manufacture a tick */
 		do {
-			while (READ_ONCE(engine->heartbeat.systole))
-				flush_delayed_work(&engine->heartbeat.work);
+			/* Manufacture a tick */
+			intel_engine_park_heartbeat(engine);
+			GEM_BUG_ON(engine->heartbeat.systole);
+			engine->serial++; /*  pretend we are not idle! */
+			intel_engine_unpark_heartbeat(engine);
 
-			engine->serial++; /* quick, pretend we are not idle! */
 			flush_delayed_work(&engine->heartbeat.work);
 			if (!delayed_work_pending(&engine->heartbeat.work)) {
-				pr_err("%s: heartbeat did not start\n",
-				       engine->name);
+				pr_err("%s: heartbeat %d did not start\n",
+				       engine->name, i);
 				err = -EINVAL;
 				goto err_pm;
 			}