diff mbox series

[2/6] drm/i915/guc: Fix issues with live_preempt_cancel

Message ID 20220728024225.2363663-3-John.C.Harrison@Intel.com (mailing list archive)
State New, archived
Headers show
Series Random assortment of (mostly) GuC related patches | expand

Commit Message

John Harrison July 28, 2022, 2:42 a.m. UTC
From: Matthew Brost <matthew.brost@intel.com>

Having semaphores results in different behavior when a dependent request
is cancelled. In the case of semaphores the request could be on the HW
and complete successfully while without the request is held in the
driver and the error from the dependent request is propagated. Fix
live_preempt_cancel to take this behavior into account.

Also update live_preempt_cancel to use new function intel_context_ban
rather than intel_context_set_banned.

Signed-off-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_execlists.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Comments

John Harrison July 28, 2022, 2:45 a.m. UTC | #1
On 7/27/2022 19:42, John.C.Harrison@Intel.com wrote:
> From: Matthew Brost <matthew.brost@intel.com>
>
> Having semaphores results in different behavior when a dependent request
> is cancelled. In the case of semaphores the request could be on the HW
> and complete successfully while without the request is held in the
> driver and the error from the dependent request is propagated. Fix
> live_preempt_cancel to take this behavior into account.
>
> Also update live_preempt_cancel to use new function intel_context_ban
> rather than intel_context_set_banned.
>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: John Harrison <John.C.Harrison@Intel.com>

> ---
>   drivers/gpu/drm/i915/gt/selftest_execlists.c | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c
> index 02fc97a0ab502..015f8cd3463e2 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
> @@ -2087,7 +2087,7 @@ static int __cancel_active0(struct live_preempt_cancel *arg)
>   		goto out;
>   	}
>   
> -	intel_context_set_banned(rq->context);
> +	intel_context_ban(rq->context, rq);
>   	err = intel_engine_pulse(arg->engine);
>   	if (err)
>   		goto out;
> @@ -2146,7 +2146,7 @@ static int __cancel_active1(struct live_preempt_cancel *arg)
>   	if (err)
>   		goto out;
>   
> -	intel_context_set_banned(rq[1]->context);
> +	intel_context_ban(rq[1]->context, rq[1]);
>   	err = intel_engine_pulse(arg->engine);
>   	if (err)
>   		goto out;
> @@ -2229,7 +2229,7 @@ static int __cancel_queued(struct live_preempt_cancel *arg)
>   	if (err)
>   		goto out;
>   
> -	intel_context_set_banned(rq[2]->context);
> +	intel_context_ban(rq[2]->context, rq[2]);
>   	err = intel_engine_pulse(arg->engine);
>   	if (err)
>   		goto out;
> @@ -2244,7 +2244,13 @@ static int __cancel_queued(struct live_preempt_cancel *arg)
>   		goto out;
>   	}
>   
> -	if (rq[1]->fence.error != 0) {
> +	/*
> +	 * The behavior between having semaphores and not is different. With
> +	 * semaphores the subsequent request is on the hardware and not cancelled
> +	 * while without the request is held in the driver and cancelled.
> +	 */
> +	if (intel_engine_has_semaphores(rq[1]->engine) &&
> +	    rq[1]->fence.error != 0) {
>   		pr_err("Normal inflight1 request did not complete\n");
>   		err = -EINVAL;
>   		goto out;
> @@ -2292,7 +2298,7 @@ static int __cancel_hostile(struct live_preempt_cancel *arg)
>   		goto out;
>   	}
>   
> -	intel_context_set_banned(rq->context);
> +	intel_context_ban(rq->context, rq);
>   	err = intel_engine_pulse(arg->engine); /* force reset */
>   	if (err)
>   		goto out;
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c
index 02fc97a0ab502..015f8cd3463e2 100644
--- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
+++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
@@ -2087,7 +2087,7 @@  static int __cancel_active0(struct live_preempt_cancel *arg)
 		goto out;
 	}
 
-	intel_context_set_banned(rq->context);
+	intel_context_ban(rq->context, rq);
 	err = intel_engine_pulse(arg->engine);
 	if (err)
 		goto out;
@@ -2146,7 +2146,7 @@  static int __cancel_active1(struct live_preempt_cancel *arg)
 	if (err)
 		goto out;
 
-	intel_context_set_banned(rq[1]->context);
+	intel_context_ban(rq[1]->context, rq[1]);
 	err = intel_engine_pulse(arg->engine);
 	if (err)
 		goto out;
@@ -2229,7 +2229,7 @@  static int __cancel_queued(struct live_preempt_cancel *arg)
 	if (err)
 		goto out;
 
-	intel_context_set_banned(rq[2]->context);
+	intel_context_ban(rq[2]->context, rq[2]);
 	err = intel_engine_pulse(arg->engine);
 	if (err)
 		goto out;
@@ -2244,7 +2244,13 @@  static int __cancel_queued(struct live_preempt_cancel *arg)
 		goto out;
 	}
 
-	if (rq[1]->fence.error != 0) {
+	/*
+	 * The behavior between having semaphores and not is different. With
+	 * semaphores the subsequent request is on the hardware and not cancelled
+	 * while without the request is held in the driver and cancelled.
+	 */
+	if (intel_engine_has_semaphores(rq[1]->engine) &&
+	    rq[1]->fence.error != 0) {
 		pr_err("Normal inflight1 request did not complete\n");
 		err = -EINVAL;
 		goto out;
@@ -2292,7 +2298,7 @@  static int __cancel_hostile(struct live_preempt_cancel *arg)
 		goto out;
 	}
 
-	intel_context_set_banned(rq->context);
+	intel_context_ban(rq->context, rq);
 	err = intel_engine_pulse(arg->engine); /* force reset */
 	if (err)
 		goto out;