[4/9] drm/i915: Tidy awaiting on dma-fences
diff mbox series

Message ID 20200508092933.738-4-chris@chris-wilson.co.uk
State New
Headers show
Series
  • [1/9] drm/i915: Ignore submit-fences on the same timeline
Related show

Commit Message

Chris Wilson May 8, 2020, 9:29 a.m. UTC
Just tidy up the return handling for completed dma-fences. While it may
return errors for invalid fence, we already know that we have a good
fence and the only error will be an already signaled fence.

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

Comments

Mika Kuoppala May 8, 2020, 3:50 p.m. UTC | #1
Chris Wilson <chris@chris-wilson.co.uk> writes:

> Just tidy up the return handling for completed dma-fences. While it may
> return errors for invalid fence, we already know that we have a good
> fence and the only error will be an already signaled fence.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_sw_fence.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
> index 7daf81f55c90..295b9829e2da 100644
> --- a/drivers/gpu/drm/i915/i915_sw_fence.c
> +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
> @@ -546,13 +546,11 @@ int __i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence,
>  	cb->fence = fence;
>  	i915_sw_fence_await(fence);
>  
> -	ret = dma_fence_add_callback(dma, &cb->base, __dma_i915_sw_fence_wake);
> -	if (ret == 0) {
> -		ret = 1;
> -	} else {
> +	ret = 1;
> +	if (dma_fence_add_callback(dma, &cb->base, __dma_i915_sw_fence_wake)) {
> +		/* fence already signaled */

This seems to hold water now. Perhaps for eternity.

But how about if (dma_fence_add_callback() == -ENOENT) ret = 0; else
GEM_BUG_ON()?
-Mika

>  		__dma_i915_sw_fence_wake(dma, &cb->base);
> -		if (ret == -ENOENT) /* fence already signaled */
> -			ret = 0;
> +		ret = 0;
>  	}
>  
>  	return ret;
> -- 
> 2.20.1
Chris Wilson May 8, 2020, 3:58 p.m. UTC | #2
Quoting Mika Kuoppala (2020-05-08 16:50:22)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Just tidy up the return handling for completed dma-fences. While it may
> > return errors for invalid fence, we already know that we have a good
> > fence and the only error will be an already signaled fence.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_sw_fence.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
> > index 7daf81f55c90..295b9829e2da 100644
> > --- a/drivers/gpu/drm/i915/i915_sw_fence.c
> > +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
> > @@ -546,13 +546,11 @@ int __i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence,
> >       cb->fence = fence;
> >       i915_sw_fence_await(fence);
> >  
> > -     ret = dma_fence_add_callback(dma, &cb->base, __dma_i915_sw_fence_wake);
> > -     if (ret == 0) {
> > -             ret = 1;
> > -     } else {
> > +     ret = 1;
> > +     if (dma_fence_add_callback(dma, &cb->base, __dma_i915_sw_fence_wake)) {
> > +             /* fence already signaled */
> 
> This seems to hold water now. Perhaps for eternity.
> 
> But how about if (dma_fence_add_callback() == -ENOENT) ret = 0; else
> GEM_BUG_ON()?

Because that's just ugly. If we do not install the callback, we need to
signal the callback. The only question is whether or not an error there
is moot -- we either have the fence, or we would have exploded. The
fence callback will be propagating errors along the fence.
-Chris

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index 7daf81f55c90..295b9829e2da 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -546,13 +546,11 @@  int __i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence,
 	cb->fence = fence;
 	i915_sw_fence_await(fence);
 
-	ret = dma_fence_add_callback(dma, &cb->base, __dma_i915_sw_fence_wake);
-	if (ret == 0) {
-		ret = 1;
-	} else {
+	ret = 1;
+	if (dma_fence_add_callback(dma, &cb->base, __dma_i915_sw_fence_wake)) {
+		/* fence already signaled */
 		__dma_i915_sw_fence_wake(dma, &cb->base);
-		if (ret == -ENOENT) /* fence already signaled */
-			ret = 0;
+		ret = 0;
 	}
 
 	return ret;