Nouveau - Jan 2025 - [PATCH] drm/sched: Use struct for drm_sched_init() params

On Wed, 22 Jan 2025 15:08:20 +0100
Philipp Stanner <phasta at kernel.org> wrote:
>  int drm_sched_init(struct drm_gpu_scheduler *sched,
> -		   const struct drm_sched_backend_ops *ops,
> -		   struct workqueue_struct *submit_wq,
> -		   u32 num_rqs, u32 credit_limit, unsigned int hang_limit,
> -		   long timeout, struct workqueue_struct *timeout_wq,
> -		   atomic_t *score, const char *name, struct device *dev);
> +		const struct drm_sched_init_params *params);

Another nit: indenting is messed up here.

On Wed, 2025-01-22 at 18:16 +0100, Boris Brezillon
wrote:
> On Wed, 22 Jan 2025 15:08:20 +0100
> Philipp Stanner <phasta at kernel.org> wrote:
> 
> > ?int drm_sched_init(struct drm_gpu_scheduler *sched,
> > - ?? const struct drm_sched_backend_ops *ops,
> > - ?? struct workqueue_struct *submit_wq,
> > - ?? u32 num_rqs, u32 credit_limit, unsigned int hang_limit,
> > - ?? long timeout, struct workqueue_struct *timeout_wq,
> > - ?? atomic_t *score, const char *name, struct device *dev);
> > + const struct drm_sched_init_params *params);
> 
> 
> Another nit: indenting is messed up here.
That was done on purpose.

I never got why so many like to intend to the opening brackets,
because:
   1. The kernel coding guide line does not demand it
   2. It mixes tabs with spaces
   3. It doesn't create an identical level of intendation
   4. It wastes huge amount of space and does not solve the problem of
      long names, but might even make it worse:
      https://elixir.bootlin.com/linux/v6.13-
      rc3/source/drivers/gpu/drm/scheduler/sched_main.c#L1296

On Thu, 23 Jan 2025 08:33:01 +0100
Philipp Stanner <phasta at mailbox.org> wrote:
> On Wed, 2025-01-22 at 18:16 +0100, Boris Brezillon wrote:
> > On Wed, 22 Jan 2025 15:08:20 +0100
> > Philipp Stanner <phasta at kernel.org> wrote:
> >   
> > > ?int drm_sched_init(struct drm_gpu_scheduler *sched,
> > > - ?? const struct drm_sched_backend_ops *ops,
> > > - ?? struct workqueue_struct *submit_wq,
> > > - ?? u32 num_rqs, u32 credit_limit, unsigned int hang_limit,
> > > - ?? long timeout, struct workqueue_struct *timeout_wq,
> > > - ?? atomic_t *score, const char *name, struct device *dev);
> > > + const struct drm_sched_init_params *params);  
> > 
> > 
> > Another nit: indenting is messed up here.  
> 
> That was done on purpose.
> 
> I never got why so many like to intend to the opening brackets,
> because:
>    1. The kernel coding guide line does not demand it
>    2. It mixes tabs with spaces
>    3. It doesn't create an identical level of intendation
>    4. It wastes huge amount of space and does not solve the problem of
>       long names, but might even make it worse:
>       https://elixir.bootlin.com/linux/v6.13-
>       rc3/source/drivers/gpu/drm/scheduler/sched_main.c#L1296
It's mostly a matter of keeping things consistent in a code base. I
don't really have strong opinions when it comes to coding style, but I
always try to follow the rules in place in the file/subsystem/project
I'm contributing to, and clearly the pattern in this file is to align
the extra lines of arguments on the first argument...

On Thu, Jan 23, 2025 at 08:33:01AM +0100, Philipp Stanner
wrote:
> On Wed, 2025-01-22 at 18:16 +0100, Boris Brezillon wrote:
> > On Wed, 22 Jan 2025 15:08:20 +0100
> > Philipp Stanner <phasta at kernel.org> wrote:
> > 
> > > ?int drm_sched_init(struct drm_gpu_scheduler *sched,
> > > - ?? const struct drm_sched_backend_ops *ops,
> > > - ?? struct workqueue_struct *submit_wq,
> > > - ?? u32 num_rqs, u32 credit_limit, unsigned int hang_limit,
> > > - ?? long timeout, struct workqueue_struct *timeout_wq,
> > > - ?? atomic_t *score, const char *name, struct device *dev);
> > > + const struct drm_sched_init_params *params);
> > 
> > 
> > Another nit: indenting is messed up here.
> 
> That was done on purpose.
Let's not change this convention, it's used all over the kernel tree,
including
the GPU scheduler. People are used to read code that is formatted this way, plus
the attempt of changing it will make code formatting inconsistent.