Re: [PATCH] drm/sched: Use struct for drm_sched_init() params

[Boris Brezillon]

On Wed, 22 Jan 2025 15:08:20 +0100
Philipp Stanner <pha...@kernel.org> wrote:

> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -3272,6 +3272,7 @@ group_create_queue(struct panthor_group *group,
>                  const struct drm_panthor_queue_create *args)
>  {
>       struct drm_gpu_scheduler *drm_sched;
> +     struct drm_sched_init_params sched_params;

nit: Could we use a struct initializer instead of a
memset(0)+field-assignment?

        struct drm_sched_init_params sched_params = {
                .ops = &panthor_queue_sched_ops,
                .submit_wq = group->ptdev->scheduler->wq,
                .num_rqs = 1,
                .credit_limit = args->ringbuf_size / sizeof(u64),
                .hang_limit = 0,
                .timeout = msecs_to_jiffies(JOB_TIMEOUT_MS),
                .timeout_wq = group->ptdev->reset.wq,
                .name = "panthor-queue",
                .dev = group->ptdev->base.dev,
        };

The same comment applies the panfrost changes BTW.

>       struct panthor_queue *queue;
>       int ret;
>  
> @@ -3289,6 +3290,8 @@ group_create_queue(struct panthor_group *group,
>       if (!queue)
>               return ERR_PTR(-ENOMEM);
>  
> +     memset(&sched_params, 0, sizeof(struct drm_sched_init_params));
> +
>       queue->fence_ctx.id = dma_fence_context_alloc(1);
>       spin_lock_init(&queue->fence_ctx.lock);
>       INIT_LIST_HEAD(&queue->fence_ctx.in_flight_jobs);
> @@ -3341,17 +3344,23 @@ group_create_queue(struct panthor_group *group,
>       if (ret)
>               goto err_free_queue;
>  
> +     sched_params.ops = &panthor_queue_sched_ops;
> +     sched_params.submit_wq = group->ptdev->scheduler->wq;
> +     sched_params.num_rqs = 1;
>       /*
> -      * Credit limit argument tells us the total number of instructions
> +      * The credit limit argument tells us the total number of instructions
>        * across all CS slots in the ringbuffer, with some jobs requiring
>        * twice as many as others, depending on their profiling status.
>        */
> -     ret = drm_sched_init(&queue->scheduler, &panthor_queue_sched_ops,
> -                          group->ptdev->scheduler->wq, 1,
> -                          args->ringbuf_size / sizeof(u64),
> -                          0, msecs_to_jiffies(JOB_TIMEOUT_MS),
> -                          group->ptdev->reset.wq,
> -                          NULL, "panthor-queue", group->ptdev->base.dev);
> +     sched_params.credit_limit = args->ringbuf_size / sizeof(u64);
> +     sched_params.hang_limit = 0;
> +     sched_params.timeout = msecs_to_jiffies(JOB_TIMEOUT_MS);
> +     sched_params.timeout_wq = group->ptdev->reset.wq;
> +     sched_params.score = NULL;
> +     sched_params.name = "panthor-queue";
> +     sched_params.dev = group->ptdev->base.dev;
> +
> +     ret = drm_sched_init(&queue->scheduler, &sched_params);
>       if (ret)
>               goto err_free_queue;