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

[Lizhi Hou <lizhi.hou@xxxxxxx>]

On 2/11/25 03:14, Philipp Stanner wrote:
> drm_sched_init() has a great many parameters and upcoming new
> functionality for the scheduler might add even more. Generally, the
> great number of parameters reduces readability and has already caused
> one missnaming, addressed in:
>
> commit 6f1cacf4eba7 ("drm/nouveau: Improve variable name in
> nouveau_sched_init()").
>
> Introduce a new struct for the scheduler init parameters and port all
> users.
>
> Signed-off-by: Philipp Stanner<phasta@xxxxxxxxxx>
> Reviewed-by: Liviu Dudau<liviu.dudau@xxxxxxx>
> Acked-by: Matthew Brost<matthew.brost@xxxxxxxxx>  # for Xe
> Reviewed-by: Boris Brezillon<boris.brezillon@xxxxxxxxxxxxx>  # for Panfrost and Panthor
> Reviewed-by: Christian Gmeiner<cgmeiner@xxxxxxxxxx>  # for Etnaviv
> Reviewed-by: Frank Binns<frank.binns@xxxxxxxxxx>  # for Imagination
> Reviewed-by: Tvrtko Ursulin<tvrtko.ursulin@xxxxxxxxxx>  # for Sched
> Reviewed-by: Maíra Canal<mcanal@xxxxxxxxxx>  # for v3d
> ---
> Changes in v4:
>    - Add forgotten driver accel/amdxdna. (Me)
>    - Rephrase the "init to NULL" comments. (Tvrtko)
>    - Apply RBs by Tvrtko and Maira.
>    - Terminate the last struct members with a comma, so that future
>      fields can be added with a minimal patch diff. (Me)
>
> Changes in v3:
>    - Various formatting requirements.
>
> Changes in v2:
>    - Point out that the hang-limit is deprecated. (Christian)
>    - Initialize the structs to 0 at declaration. (Planet Earth)
>    - Don't set stuff explicitly to 0 / NULL. (Tvrtko)
>    - Make the structs const where possible. (Boris)
>    - v3d: Use just 1, universal, function for sched-init. (Maíra)
> ---
>   drivers/accel/amdxdna/aie2_ctx.c           | 12 +++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++++--
>   drivers/gpu/drm/etnaviv/etnaviv_sched.c    | 20 +++----
>   drivers/gpu/drm/imagination/pvr_queue.c    | 18 ++++--
>   drivers/gpu/drm/lima/lima_sched.c          | 16 ++++--
>   drivers/gpu/drm/msm/msm_ringbuffer.c       | 17 +++---
>   drivers/gpu/drm/nouveau/nouveau_sched.c    | 16 ++++--
>   drivers/gpu/drm/panfrost/panfrost_job.c    | 20 ++++---
>   drivers/gpu/drm/panthor/panthor_mmu.c      | 16 ++++--
>   drivers/gpu/drm/panthor/panthor_sched.c    | 28 ++++++----
>   drivers/gpu/drm/scheduler/sched_main.c     | 50 ++++++-----------
>   drivers/gpu/drm/v3d/v3d_sched.c            | 65 +++++++++-------------
>   drivers/gpu/drm/xe/xe_execlist.c           | 15 +++--
>   drivers/gpu/drm/xe/xe_gpu_scheduler.c      | 17 +++++-
>   include/drm/gpu_scheduler.h                | 36 ++++++++++--
>   15 files changed, 210 insertions(+), 154 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
> index 2799729801f6..8ac01bc2f0db 100644
> --- a/drivers/accel/amdxdna/aie2_ctx.c
> +++ b/drivers/accel/amdxdna/aie2_ctx.c
> @@ -516,6 +516,14 @@ int aie2_hwctx_init(struct amdxdna_hwctx *hwctx)
>   {
>   	struct amdxdna_client *client = hwctx->client;
>   	struct amdxdna_dev *xdna = client->xdna;
> +	const struct drm_sched_init_args args = {
> +		.ops = &sched_ops,
> +		.num_rqs = DRM_SCHED_PRIORITY_COUNT,
> +		.credit_limit = HWCTX_MAX_CMDS,
> +		.timeout = msecs_to_jiffies(HWCTX_MAX_TIMEOUT),
> +		.name = hwctx->name,
> +		.dev = xdna->ddev.dev,
> +	};
>   	struct drm_gpu_scheduler *sched;
>   	struct amdxdna_hwctx_priv *priv;
>   	struct amdxdna_gem_obj *heap;
> @@ -573,9 +581,7 @@ int aie2_hwctx_init(struct amdxdna_hwctx *hwctx)
>   	might_lock(&priv->io_lock);
>   	fs_reclaim_release(GFP_KERNEL);
>   
> -	ret = drm_sched_init(sched, &sched_ops, NULL, DRM_SCHED_PRIORITY_COUNT,
> -			     HWCTX_MAX_CMDS, 0, msecs_to_jiffies(HWCTX_MAX_TIMEOUT),
> -			     NULL, NULL, hwctx->name, xdna->ddev.dev);
> +	ret = drm_sched_init(sched, &args);
Reviewed-by: Lizhi Hou <lizhi.hou@xxxxxxx>
>   	if (ret) {
>   		XDNA_ERR(xdna, "Failed to init DRM scheduler. ret %d", ret);
>   		goto free_cmd_bufs;