Re: [Intel-gfx] [PATCH] drm/i915: Propagate fence->error across semaphores
Quoting Chris Wilson (2020-05-05 17:13:02)
> Replacing an inter-engine fence with a semaphore reduced the HW
> execution latency, but that comes at a cost. For normal fences, we are
> able to propagate the metadata such as errors along with the signaling.
> For semaphores, we are missing this error propagation so add it in the
> back channel we use to monitor the semaphore overload.
>
> This raises a valid point on whether error propagation is sufficient in
> the semaphore case if it is coupled to a fatal error, such as EFAULT. It
> is not, and we should teach ourselves not to use a semaphore if we would
> chain up to an external fence whose error we must not ignore.
>
> Fixes: ef4688497512 ("drm/i915: Propagate fence errors")
> Signed-off-by: Chris Wilson
> Cc: Tvrtko Ursulin
> Cc: Matthew Auld
> ---
> drivers/gpu/drm/i915/i915_request.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_request.c
> b/drivers/gpu/drm/i915/i915_request.c
> index 9c5de07db47d..96a8c7a1be73 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -614,6 +614,9 @@ semaphore_notify(struct i915_sw_fence *fence, enum
> i915_sw_fence_notify state)
>
> switch (state) {
> case FENCE_COMPLETE:
> + if (unlikely(fence->error))
> + i915_request_set_error_once(rq, fence->error);
This is just horrible. I don't like it even as a hack.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Propagate fence->error across semaphores
Replacing an inter-engine fence with a semaphore reduced the HW
execution latency, but that comes at a cost. For normal fences, we are
able to propagate the metadata such as errors along with the signaling.
For semaphores, we are missing this error propagation so add it in the
back channel we use to monitor the semaphore overload.
This raises a valid point on whether error propagation is sufficient in
the semaphore case if it is coupled to a fatal error, such as EFAULT. It
is not, and we should teach ourselves not to use a semaphore if we would
chain up to an external fence whose error we must not ignore.
Fixes: ef4688497512 ("drm/i915: Propagate fence errors")
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Matthew Auld
---
drivers/gpu/drm/i915/i915_request.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index 9c5de07db47d..96a8c7a1be73 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -614,6 +614,9 @@ semaphore_notify(struct i915_sw_fence *fence, enum
i915_sw_fence_notify state)
switch (state) {
case FENCE_COMPLETE:
+ if (unlikely(fence->error))
+ i915_request_set_error_once(rq, fence->error);
+
if (!(READ_ONCE(rq->sched.attr.priority) &
I915_PRIORITY_NOSEMAPHORE)) {
i915_request_get(rq);
init_irq_work(>semaphore_work, irq_semaphore_cb);
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
