[PATCH 0/4] drm/nouveau: Simplify nouveau_fence.c

[PATCH 0/4] drm/nouveau: Simplify nouveau_fence.c

[Philipp Stanner]

Just some minor attempts at improving the readability of
nouveau_fence.c

This series is based on this partially merged series: [1]

Feel free to drop single patches if they are not deemed worth the
effort.

P.

[1] https://lore.kernel.org/dri-devel/20250415121900.55719-3-phasta@kernel.org/

Philipp Stanner (4):
  drm/nouveau: nouveau_fence: Standardize list iterations
  drm/nouveau: Simplify calls to nvif_event_block()
  drm/nouveau: Simplify nouveau_fence_done()
  drm/nouveau: Check dma_fence in canonical way

 drivers/gpu/drm/nouveau/nouveau_fence.c | 72 +++++++++++--------------
 1 file changed, 30 insertions(+), 42 deletions(-)

-- 
2.48.1

[PATCH 1/4] drm/nouveau: nouveau_fence: Standardize list iterations

[Philipp Stanner]

nouveau_fence.c iterates over lists in a non-canonical way. Since the
operations done are just basic for-each-loops and list-empty checks,
they should be written in the standard form.

Use standard list operations.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/gpu/drm/nouveau/nouveau_fence.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c
index 6ded8c2b6d3b..761c174cb286 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -83,13 +83,11 @@ nouveau_local_fence(struct dma_fence *fence, struct nouveau_drm *drm)
 void
 nouveau_fence_context_kill(struct nouveau_fence_chan *fctx, int error)
 {
-	struct nouveau_fence *fence;
+	struct nouveau_fence *fence, *tmp;
 	unsigned long flags;
 
 	spin_lock_irqsave(&fctx->lock, flags);
-	while (!list_empty(&fctx->pending)) {
-		fence = list_entry(fctx->pending.next, typeof(*fence), head);
-
+	list_for_each_entry_safe(fence, tmp, &fctx->pending, head) {
 		if (error && !dma_fence_is_signaled_locked(&fence->base))
 			dma_fence_set_error(&fence->base, error);
 
@@ -130,13 +128,11 @@ nouveau_fence_context_free(struct nouveau_fence_chan *fctx)
 static int
 nouveau_fence_update(struct nouveau_channel *chan, struct nouveau_fence_chan *fctx)
 {
-	struct nouveau_fence *fence;
+	struct nouveau_fence *fence, *tmp;
 	int drop = 0;
 	u32 seq = fctx->read(chan);
 
-	while (!list_empty(&fctx->pending)) {
-		fence = list_entry(fctx->pending.next, typeof(*fence), head);
-
+	list_for_each_entry_safe(fence, tmp, &fctx->pending, head) {
 		if ((int)(seq - fence->base.seqno) < 0)
 			break;
 
@@ -151,15 +147,14 @@ nouveau_fence_uevent_work(struct work_struct *work)
 {
 	struct nouveau_fence_chan *fctx = container_of(work, struct nouveau_fence_chan,
 						       uevent_work);
+	struct nouveau_channel *chan;
+	struct nouveau_fence *fence;
 	unsigned long flags;
 	int drop = 0;
 
 	spin_lock_irqsave(&fctx->lock, flags);
-	if (!list_empty(&fctx->pending)) {
-		struct nouveau_fence *fence;
-		struct nouveau_channel *chan;
-
-		fence = list_entry(fctx->pending.next, typeof(*fence), head);
+	fence = list_first_entry_or_null(&fctx->pending, typeof(*fence), head);
+	if (fence) {
 		chan = rcu_dereference_protected(fence->channel, lockdep_is_held(&fctx->lock));
 		if (nouveau_fence_update(chan, fctx))
 			drop = 1;
-- 
2.48.1

[PATCH 2/4] drm/nouveau: Simplify calls to nvif_event_block()

[Philipp Stanner]

nouveau_fence_signal() returns a de-facto boolean to indicate when
nvif_event_block() shall be called.

The code can be made more compact and readable by calling
nvif_event_block() in nouveau_fence_update() directly.

Make those calls in nouveau_fence.c more canonical.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/gpu/drm/nouveau/nouveau_fence.c | 31 +++++++++++--------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c
index 761c174cb286..2b79bcb7da16 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -50,10 +50,10 @@ nouveau_fctx(struct nouveau_fence *fence)
 	return container_of(fence->base.lock, struct nouveau_fence_chan, lock);
 }
 
-static int
+static bool
 nouveau_fence_signal(struct nouveau_fence *fence)
 {
-	int drop = 0;
+	bool drop = false;
 
 	dma_fence_signal_locked(&fence->base);
 	list_del(&fence->head);
@@ -63,7 +63,7 @@ nouveau_fence_signal(struct nouveau_fence *fence)
 		struct nouveau_fence_chan *fctx = nouveau_fctx(fence);
 
 		if (!--fctx->notify_ref)
-			drop = 1;
+			drop = true;
 	}
 
 	dma_fence_put(&fence->base);
@@ -125,21 +125,23 @@ nouveau_fence_context_free(struct nouveau_fence_chan *fctx)
 	kref_put(&fctx->fence_ref, nouveau_fence_context_put);
 }
 
-static int
+static void
 nouveau_fence_update(struct nouveau_channel *chan, struct nouveau_fence_chan *fctx)
 {
 	struct nouveau_fence *fence, *tmp;
-	int drop = 0;
+	bool drop = false;
 	u32 seq = fctx->read(chan);
 
 	list_for_each_entry_safe(fence, tmp, &fctx->pending, head) {
 		if ((int)(seq - fence->base.seqno) < 0)
 			break;
 
-		drop |= nouveau_fence_signal(fence);
+		if (nouveau_fence_signal(fence))
+			drop = true;
 	}
 
-	return drop;
+	if (drop)
+		nvif_event_block(&fctx->event);
 }
 
 static void
@@ -150,18 +152,13 @@ nouveau_fence_uevent_work(struct work_struct *work)
 	struct nouveau_channel *chan;
 	struct nouveau_fence *fence;
 	unsigned long flags;
-	int drop = 0;
 
 	spin_lock_irqsave(&fctx->lock, flags);
 	fence = list_first_entry_or_null(&fctx->pending, typeof(*fence), head);
 	if (fence) {
 		chan = rcu_dereference_protected(fence->channel, lockdep_is_held(&fctx->lock));
-		if (nouveau_fence_update(chan, fctx))
-			drop = 1;
+		nouveau_fence_update(chan, fctx);
 	}
-	if (drop)
-		nvif_event_block(&fctx->event);
-
 	spin_unlock_irqrestore(&fctx->lock, flags);
 }
 
@@ -241,9 +238,7 @@ nouveau_fence_emit(struct nouveau_fence *fence)
 			return -ENODEV;
 		}
 
-		if (nouveau_fence_update(chan, fctx))
-			nvif_event_block(&fctx->event);
-
+		nouveau_fence_update(chan, fctx);
 		list_add_tail(&fence->head, &fctx->pending);
 		spin_unlock_irq(&fctx->lock);
 	}
@@ -265,8 +260,8 @@ nouveau_fence_done(struct nouveau_fence *fence)
 
 		spin_lock_irqsave(&fctx->lock, flags);
 		chan = rcu_dereference_protected(fence->channel, lockdep_is_held(&fctx->lock));
-		if (chan && nouveau_fence_update(chan, fctx))
-			nvif_event_block(&fctx->event);
+		if (chan)
+			nouveau_fence_update(chan, fctx);
 		spin_unlock_irqrestore(&fctx->lock, flags);
 	}
 	return dma_fence_is_signaled(&fence->base);
-- 
2.48.1

[PATCH 3/4] drm/nouveau: Simplify nouveau_fence_done()

[Philipp Stanner]

nouveau_fence_done() contains an if branch that checks whether a
nouveau_fence has either of the two existing nouveau_fence backend ops,
which will always evaluate to true.

Remove the surplus check.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/gpu/drm/nouveau/nouveau_fence.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c
index 2b79bcb7da16..fb9811938c82 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -249,21 +249,19 @@ nouveau_fence_emit(struct nouveau_fence *fence)
 bool
 nouveau_fence_done(struct nouveau_fence *fence)
 {
-	if (fence->base.ops == &nouveau_fence_ops_legacy ||
-	    fence->base.ops == &nouveau_fence_ops_uevent) {
-		struct nouveau_fence_chan *fctx = nouveau_fctx(fence);
-		struct nouveau_channel *chan;
-		unsigned long flags;
+	struct nouveau_fence_chan *fctx = nouveau_fctx(fence);
+	struct nouveau_channel *chan;
+	unsigned long flags;
 
-		if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->base.flags))
-			return true;
+	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->base.flags))
+		return true;
+
+	spin_lock_irqsave(&fctx->lock, flags);
+	chan = rcu_dereference_protected(fence->channel, lockdep_is_held(&fctx->lock));
+	if (chan)
+		nouveau_fence_update(chan, fctx);
+	spin_unlock_irqrestore(&fctx->lock, flags);
 
-		spin_lock_irqsave(&fctx->lock, flags);
-		chan = rcu_dereference_protected(fence->channel, lockdep_is_held(&fctx->lock));
-		if (chan)
-			nouveau_fence_update(chan, fctx);
-		spin_unlock_irqrestore(&fctx->lock, flags);
-	}
 	return dma_fence_is_signaled(&fence->base);
 }
 
-- 
2.48.1

Re: [PATCH 3/4] drm/nouveau: Simplify nouveau_fence_done()

[Christian König]

On 4/24/25 15:02, Philipp Stanner wrote:
> nouveau_fence_done() contains an if branch that checks whether a
> nouveau_fence has either of the two existing nouveau_fence backend ops,
> which will always evaluate to true.
> 
> Remove the surplus check.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>


Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/nouveau/nouveau_fence.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c
> index 2b79bcb7da16..fb9811938c82 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> @@ -249,21 +249,19 @@ nouveau_fence_emit(struct nouveau_fence *fence)
>  bool
>  nouveau_fence_done(struct nouveau_fence *fence)
>  {
> -	if (fence->base.ops == &nouveau_fence_ops_legacy ||
> -	    fence->base.ops == &nouveau_fence_ops_uevent) {
> -		struct nouveau_fence_chan *fctx = nouveau_fctx(fence);
> -		struct nouveau_channel *chan;
> -		unsigned long flags;
> +	struct nouveau_fence_chan *fctx = nouveau_fctx(fence);
> +	struct nouveau_channel *chan;
> +	unsigned long flags;
>  
> -		if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->base.flags))
> -			return true;
> +	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->base.flags))
> +		return true;
> +
> +	spin_lock_irqsave(&fctx->lock, flags);
> +	chan = rcu_dereference_protected(fence->channel, lockdep_is_held(&fctx->lock));
> +	if (chan)
> +		nouveau_fence_update(chan, fctx);
> +	spin_unlock_irqrestore(&fctx->lock, flags);
>  
> -		spin_lock_irqsave(&fctx->lock, flags);
> -		chan = rcu_dereference_protected(fence->channel, lockdep_is_held(&fctx->lock));
> -		if (chan)
> -			nouveau_fence_update(chan, fctx);
> -		spin_unlock_irqrestore(&fctx->lock, flags);
> -	}
>  	return dma_fence_is_signaled(&fence->base);
>  }
>  

[PATCH 4/4] drm/nouveau: Check dma_fence in canonical way

[Philipp Stanner]

In nouveau_fence_done(), a fence is checked for being signaled by
manually evaluating the base fence's bits. This can be done in a
canonical manner through dma_fence_is_signaled().

Replace the bit-check with dma_fence_is_signaled().

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c
index fb9811938c82..d5654e26d5bc 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -253,7 +253,7 @@ nouveau_fence_done(struct nouveau_fence *fence)
 	struct nouveau_channel *chan;
 	unsigned long flags;
 
-	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->base.flags))
+	if (dma_fence_is_signaled(&fence->base))
 		return true;
 
 	spin_lock_irqsave(&fctx->lock, flags);
-- 
2.48.1

Re: [PATCH 4/4] drm/nouveau: Check dma_fence in canonical way

[Danilo Krummrich]

On 4/24/25 3:02 PM, Philipp Stanner wrote:
> In nouveau_fence_done(), a fence is checked for being signaled by
> manually evaluating the base fence's bits. This can be done in a
> canonical manner through dma_fence_is_signaled().
> 
> Replace the bit-check with dma_fence_is_signaled().
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>   drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c
> index fb9811938c82..d5654e26d5bc 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> @@ -253,7 +253,7 @@ nouveau_fence_done(struct nouveau_fence *fence)
>   	struct nouveau_channel *chan;
>   	unsigned long flags;
>   
> -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->base.flags))
> +	if (dma_fence_is_signaled(&fence->base))

This is only correct with commit bbe5679f30d7 ("drm/nouveau: Fix WARN_ON in
nouveau_fence_context_kill()") from drm-misc-fixes, correct?

Re: [PATCH 4/4] drm/nouveau: Check dma_fence in canonical way

[Philipp Stanner]

On Thu, 2025-04-24 at 15:24 +0200, Danilo Krummrich wrote:
> On 4/24/25 3:02 PM, Philipp Stanner wrote:
> > In nouveau_fence_done(), a fence is checked for being signaled by
> > manually evaluating the base fence's bits. This can be done in a
> > canonical manner through dma_fence_is_signaled().
> > 
> > Replace the bit-check with dma_fence_is_signaled().
> > 
> > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > ---
> >   drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
> > b/drivers/gpu/drm/nouveau/nouveau_fence.c
> > index fb9811938c82..d5654e26d5bc 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> > @@ -253,7 +253,7 @@ nouveau_fence_done(struct nouveau_fence *fence)
> >   	struct nouveau_channel *chan;
> >   	unsigned long flags;
> >   
> > -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence-
> > >base.flags))
> > +	if (dma_fence_is_signaled(&fence->base))
> 
> This is only correct with commit bbe5679f30d7 ("drm/nouveau: Fix
> WARN_ON in
> nouveau_fence_context_kill()") from drm-misc-fixes, correct?

Yup. Otherwise, this series can't be merged anyways, because patch 1
depends on it.

The cover letter says so: "This series is based on this partially
merged series: [1]"

P.

Re: [PATCH 4/4] drm/nouveau: Check dma_fence in canonical way

[Danilo Krummrich]

(+ drm-misc maintainers)

On Thu, Apr 24, 2025 at 03:25:55PM +0200, Philipp Stanner wrote:
> On Thu, 2025-04-24 at 15:24 +0200, Danilo Krummrich wrote:
> > On 4/24/25 3:02 PM, Philipp Stanner wrote:
> > > In nouveau_fence_done(), a fence is checked for being signaled by
> > > manually evaluating the base fence's bits. This can be done in a
> > > canonical manner through dma_fence_is_signaled().
> > > 
> > > Replace the bit-check with dma_fence_is_signaled().
> > > 
> > > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > > ---
> > >   drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
> > > b/drivers/gpu/drm/nouveau/nouveau_fence.c
> > > index fb9811938c82..d5654e26d5bc 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> > > @@ -253,7 +253,7 @@ nouveau_fence_done(struct nouveau_fence *fence)
> > >   	struct nouveau_channel *chan;
> > >   	unsigned long flags;
> > >   
> > > -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence-
> > > >base.flags))
> > > +	if (dma_fence_is_signaled(&fence->base))
> > 
> > This is only correct with commit bbe5679f30d7 ("drm/nouveau: Fix
> > WARN_ON in
> > nouveau_fence_context_kill()") from drm-misc-fixes, correct?
> 
> Yup. Otherwise, this series can't be merged anyways, because patch 1
> depends on it.
> 
> The cover letter says so: "This series is based on this partially
> merged series: [1]"

Well, the series may be based on commit bbe5679f30d7, but all patches from the
series can still be applied independently.

Only patch 4 depends on this commit in terms of correctness.

But that's fine, I think we can get drm-misc-fixes (or the next -rc) backmerged
into drm-misc-next.

Re: [PATCH 4/4] drm/nouveau: Check dma_fence in canonical way

[Christian König]

On 4/24/25 15:02, Philipp Stanner wrote:
> In nouveau_fence_done(), a fence is checked for being signaled by
> manually evaluating the base fence's bits. This can be done in a
> canonical manner through dma_fence_is_signaled().
> 
> Replace the bit-check with dma_fence_is_signaled().
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>


I think the bit check was used here as fast path optimization because we later call dma_fence_is_signaled() anyway.

Feel free to add my acked-by, but honestly what nouveau does here looks rather suspicious to me.

Regards,
Christian.

> ---
>  drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c
> index fb9811938c82..d5654e26d5bc 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> @@ -253,7 +253,7 @@ nouveau_fence_done(struct nouveau_fence *fence)
>  	struct nouveau_channel *chan;
>  	unsigned long flags;
>  
> -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->base.flags))
> +	if (dma_fence_is_signaled(&fence->base))
>  		return true;
>  
>  	spin_lock_irqsave(&fctx->lock, flags);

Re: [PATCH 4/4] drm/nouveau: Check dma_fence in canonical way

[Philipp Stanner]

On Mon, 2025-04-28 at 16:45 +0200, Christian König wrote:
> On 4/24/25 15:02, Philipp Stanner wrote:
> > In nouveau_fence_done(), a fence is checked for being signaled by
> > manually evaluating the base fence's bits. This can be done in a
> > canonical manner through dma_fence_is_signaled().
> > 
> > Replace the bit-check with dma_fence_is_signaled().
> > 
> > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> 
> 
> I think the bit check was used here as fast path optimization because
> we later call dma_fence_is_signaled() anyway.

That fast path optimization effectively saves one JMP instruction to
the function.

I'm increasingly of the opinion that we shall work towards all DRM
users only ever using infrastructure through officially documented API
functions, without touching internal data structures.

> Feel free to add my acked-by, but honestly what nouveau does here
> looks rather suspicious to me.

:)


P.

> 
> Regards,
> Christian.
> 
> > ---
> >  drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
> > b/drivers/gpu/drm/nouveau/nouveau_fence.c
> > index fb9811938c82..d5654e26d5bc 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> > @@ -253,7 +253,7 @@ nouveau_fence_done(struct nouveau_fence *fence)
> >  	struct nouveau_channel *chan;
> >  	unsigned long flags;
> >  
> > -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence-
> > >base.flags))
> > +	if (dma_fence_is_signaled(&fence->base))
> >  		return true;
> >  
> >  	spin_lock_irqsave(&fctx->lock, flags);
> 

Re: [PATCH 4/4] drm/nouveau: Check dma_fence in canonical way

[Christian König]

On 5/8/25 11:13, Philipp Stanner wrote:
> On Mon, 2025-04-28 at 16:45 +0200, Christian König wrote:
>> On 4/24/25 15:02, Philipp Stanner wrote:
>>> In nouveau_fence_done(), a fence is checked for being signaled by
>>> manually evaluating the base fence's bits. This can be done in a
>>> canonical manner through dma_fence_is_signaled().
>>>
>>> Replace the bit-check with dma_fence_is_signaled().
>>>
>>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
>>
>>
>> I think the bit check was used here as fast path optimization because
>> we later call dma_fence_is_signaled() anyway.
> 
> That fast path optimization effectively saves one JMP instruction to
> the function.


What I meant was that we might completely drop that optimization. It looks like overkill and potentially hides bugs.

Regards,
Christian.

> 
> I'm increasingly of the opinion that we shall work towards all DRM
> users only ever using infrastructure through officially documented API
> functions, without touching internal data structures.
> 
>> Feel free to add my acked-by, but honestly what nouveau does here
>> looks rather suspicious to me.
> 
> :)
> 
> 
> P.
> 
>>
>> Regards,
>> Christian.
>>
>>> ---
>>>  drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
>>> b/drivers/gpu/drm/nouveau/nouveau_fence.c
>>> index fb9811938c82..d5654e26d5bc 100644
>>> --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
>>> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
>>> @@ -253,7 +253,7 @@ nouveau_fence_done(struct nouveau_fence *fence)
>>>  	struct nouveau_channel *chan;
>>>  	unsigned long flags;
>>>  
>>> -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence-
>>>> base.flags))
>>> +	if (dma_fence_is_signaled(&fence->base))
>>>  		return true;
>>>  
>>>  	spin_lock_irqsave(&fctx->lock, flags);
>>
> 

Re: [PATCH 0/4] drm/nouveau: Simplify nouveau_fence.c

[Danilo Krummrich]

On 4/24/25 3:02 PM, Philipp Stanner wrote:
> Just some minor attempts at improving the readability of
> nouveau_fence.c

Applied to drm-misc-next, thanks!