[PATCH] drm/nouveau: Remove surplus struct member

[PATCH] drm/nouveau: Remove surplus struct member

[Philipp Stanner]

struct nouveau_channel contains the member 'accel_done' and a forgotten
TODO which hints at that mechanism being removed in the "near future".
Since that variable is read nowhere anymore, this "near future" is now.

Remove the variable and the TODO.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/gpu/drm/nouveau/nouveau_chan.h | 2 --
 drivers/gpu/drm/nouveau/nouveau_dma.h  | 1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.h b/drivers/gpu/drm/nouveau/nouveau_chan.h
index 561877725aac..bb34b0a6082d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.h
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.h
@@ -31,8 +31,6 @@ struct nouveau_channel {
 		u64 addr;
 	} push;
 
-	/* TODO: this will be reworked in the near future */
-	bool accel_done;
 	void *fence;
 	struct {
 		int max;
diff --git a/drivers/gpu/drm/nouveau/nouveau_dma.h b/drivers/gpu/drm/nouveau/nouveau_dma.h
index 0e27b76d1e1c..c25ef9a54b9f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dma.h
+++ b/drivers/gpu/drm/nouveau/nouveau_dma.h
@@ -90,7 +90,6 @@ FIRE_RING(struct nouveau_channel *chan)
 {
 	if (chan->dma.cur == chan->dma.put)
 		return;
-	chan->accel_done = true;
 
 	WRITE_PUT(chan->dma.cur);
 
-- 
2.49.0

Re: [PATCH] drm/nouveau: Remove surplus struct member

[Timur Tabi]

Does mean that the TODO has been done, or that someone completely forgot and now your patch is
remove all reminders?

If it's the format, maybe add a fixes: tag for the commit that resolved the TODO?

On Fri, 2025-08-01 at 09:45 +0200, Philipp Stanner wrote:
> struct nouveau_channel contains the member 'accel_done' and a forgotten
> TODO which hints at that mechanism being removed in the "near future".
> Since that variable is read nowhere anymore, this "near future" is now.
> 
> Remove the variable and the TODO.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>  drivers/gpu/drm/nouveau/nouveau_chan.h | 2 --
>  drivers/gpu/drm/nouveau/nouveau_dma.h  | 1 -
>  2 files changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.h b/drivers/gpu/drm/nouveau/nouveau_chan.h
> index 561877725aac..bb34b0a6082d 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_chan.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_chan.h
> @@ -31,8 +31,6 @@ struct nouveau_channel {
>  		u64 addr;
>  	} push;
>  
> -	/* TODO: this will be reworked in the near future */
> -	bool accel_done;
>  	void *fence;
>  	struct {
>  		int max;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_dma.h b/drivers/gpu/drm/nouveau/nouveau_dma.h
> index 0e27b76d1e1c..c25ef9a54b9f 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_dma.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_dma.h
> @@ -90,7 +90,6 @@ FIRE_RING(struct nouveau_channel *chan)
>  {
>  	if (chan->dma.cur == chan->dma.put)
>  		return;
> -	chan->accel_done = true;
>  
>  	WRITE_PUT(chan->dma.cur);
>  

Re: [PATCH] drm/nouveau: Remove surplus struct member

[Danilo Krummrich]

On Fri Aug 1, 2025 at 4:50 PM CEST, Timur Tabi wrote:
> Does mean that the TODO has been done, or that someone completely forgot and now your patch is
> remove all reminders?
>
> If it's the format, maybe add a fixes: tag for the commit that resolved the TODO?

The TODO was introduced by commit ebb945a94bba ("drm/nouveau: port all engines
to new engine module format") from 2012.

It's a bit hard to know what exactly resolves "this will be reworked in the near
future" for a commit with the following diffstat. :)

	146 files changed, 14219 insertions(+), 11099 deletions(-)

The last remains of accel_done were removed with commit
4e2ec2500bfc ("drm/nouveau: Remove file nouveau_fbcon.c"), but I don't think we
should mention this commit, given that apparently no one knows what was intended
to be reworked here [1].

We could mention the above in the commit message, though it will also be
available through the lore link in the commit message once the patch is applied.

NIT: Please don't top post, use interleaved style [2] instead.

[1] https://lore.kernel.org/all/481a2808c235f95726d17803503b2b6dc2746dc3.camel@mailbox.org/
[2] https://docs.kernel.org/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions

Re: [PATCH] drm/nouveau: Remove surplus struct member

[Timur Tabi]

On Fri, 2025-08-01 at 17:12 +0200, Danilo Krummrich wrote:
> On Fri Aug 1, 2025 at 4:50 PM CEST, Timur Tabi wrote:
> > Does mean that the TODO has been done, or that someone completely forgot and now your patch
> > is
> > remove all reminders?
> > 
> > If it's the format, maybe add a fixes: tag for the commit that resolved the TODO?
> 
> The TODO was introduced by commit ebb945a94bba ("drm/nouveau: port all engines
> to new engine module format") from 2012.
> 
> It's a bit hard to know what exactly resolves "this will be reworked in the near
> future" for a commit with the following diffstat. :)
> 
> 	146 files changed, 14219 insertions(+), 11099 deletions(-)
> 
> The last remains of accel_done were removed with commit
> 4e2ec2500bfc ("drm/nouveau: Remove file nouveau_fbcon.c"), but I don't think we
> should mention this commit, given that apparently no one knows what was intended
> to be reworked here [1].
> 
> We could mention the above in the commit message, though it will also be
> available through the lore link in the commit message once the patch is applied.

It's your call, I'm just leery of removing a TODO without any mention of whether it's actually
done.

> NIT: Please don't top post, use interleaved style [2] instead.

Sorry, coffee didn't kick in yet.

Re: [PATCH] drm/nouveau: Remove surplus struct member

[Philipp Stanner]

On Fri, 2025-08-01 at 15:42 +0000, Timur Tabi wrote:
> On Fri, 2025-08-01 at 17:12 +0200, Danilo Krummrich wrote:
> > On Fri Aug 1, 2025 at 4:50 PM CEST, Timur Tabi wrote:
> > > Does mean that the TODO has been done, or that someone completely forgot and now your patch
> > > is
> > > remove all reminders?
> > > 
> > > If it's the format, maybe add a fixes: tag for the commit that resolved the TODO?
> > 
> > The TODO was introduced by commit ebb945a94bba ("drm/nouveau: port all engines
> > to new engine module format") from 2012.
> > 
> > It's a bit hard to know what exactly resolves "this will be reworked in the near
> > future" for a commit with the following diffstat. :)
> > 
> > 	146 files changed, 14219 insertions(+), 11099 deletions(-)
> > 
> > The last remains of accel_done were removed with commit
> > 4e2ec2500bfc ("drm/nouveau: Remove file nouveau_fbcon.c"), but I don't think we
> > should mention this commit, given that apparently no one knows what was intended
> > to be reworked here [1].
> > 
> > We could mention the above in the commit message, though it will also be
> > available through the lore link in the commit message once the patch is applied.
> 
> It's your call, I'm just leery of removing a TODO without any mention of whether it's actually
> done.

We can't really tell whether "it" is done or not, because the original
author didn't write down *what* exactly needs to be done, which makes
the TODO kind of useless.

Since that information is lost forever, removing it arguably is
alright.

P.

> 
> > NIT: Please don't top post, use interleaved style [2] instead.
> 
> Sorry, coffee didn't kick in yet.

Re: [PATCH] drm/nouveau: Remove surplus struct member

[Danilo Krummrich]

On 8/1/25 9:45 AM, Philipp Stanner wrote:
> struct nouveau_channel contains the member 'accel_done' and a forgotten
> TODO which hints at that mechanism being removed in the "near future".
> Since that variable is read nowhere anymore, this "near future" is now.
> 
> Remove the variable and the TODO.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

Applied to drm-misc-next, thanks!