Hi Philipp,
On 28/01/25 11:29, 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 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>
---
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/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 | 15 +++--
drivers/gpu/drm/panfrost/panfrost_job.c | 20 ++++---
drivers/gpu/drm/panthor/panthor_mmu.c | 16 +++--
drivers/gpu/drm/panthor/panthor_sched.c | 29 +++++----
drivers/gpu/drm/scheduler/sched_main.c | 50 ++++++----------
drivers/gpu/drm/v3d/v3d_sched.c | 68 +++++++++--------
-----
drivers/gpu/drm/xe/xe_execlist.c | 16 +++--
drivers/gpu/drm/xe/xe_gpu_scheduler.c | 17 +++++-
include/drm/gpu_scheduler.h | 37 ++++++++++--
14 files changed, 206 insertions(+), 151 deletions(-)
[...]
diff --git a/drivers/gpu/drm/panthor/panthor_sched.c
b/drivers/gpu/drm/panthor/panthor_sched.c
index 5844a7f639e0..44713cfdcd74 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -3284,6 +3284,22 @@ static struct panthor_queue *
group_create_queue(struct panthor_group *group,
const struct drm_panthor_queue_create *args)
{
+ const struct drm_sched_init_args sched_args = {
+ .ops = &panthor_queue_sched_ops,
+ .submit_wq = group->ptdev->scheduler->wq,
+ .num_rqs = 1,
+ /*
+ * 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.
+ */
+ .credit_limit = args->ringbuf_size / sizeof(u64),
+ .timeout = msecs_to_jiffies(JOB_TIMEOUT_MS),
+ .timeout_wq = group->ptdev->reset.wq,
+ .name = "panthor-queue",
+ .dev = group->ptdev->base.dev
+ };
struct drm_gpu_scheduler *drm_sched;
struct panthor_queue *queue;
int ret;
@@ -3354,17 +3370,8 @@ group_create_queue(struct panthor_group
*group,
if (ret)
goto err_free_queue;
- /*
- * 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);
+
Please don't use multiple blank lines.
+ ret = drm_sched_init(&queue->scheduler, &sched_args);
if (ret)
goto err_free_queue;
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index a48be16ab84f..6295b2654a7c 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -1244,40 +1244,24 @@ static void drm_sched_run_job_work(struct
work_struct *w)
* drm_sched_init - Init a gpu scheduler instance
*
* @sched: scheduler instance
- * @ops: backend operations for this scheduler
- * @submit_wq: workqueue to use for submission. If NULL, an
ordered wq is
- * allocated and used
- * @num_rqs: number of runqueues, one for each priority, up to
DRM_SCHED_PRIORITY_COUNT
- * @credit_limit: the number of credits this scheduler can hold
from all jobs
- * @hang_limit: number of times to allow a job to hang before
dropping it
- * @timeout: timeout value in jiffies for the scheduler
- * @timeout_wq: workqueue to use for timeout work. If NULL, the
system_wq is
- * used
- * @score: optional score atomic shared with other schedulers
- * @name: name used for debugging
- * @dev: target &struct device
+ * @args: scheduler initialization arguments
*
* Return 0 on success, otherwise error code.
*/
-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)
+int drm_sched_init(struct drm_gpu_scheduler *sched, const struct
drm_sched_init_args *args)
{
int i;
- sched->ops = ops;
- sched->credit_limit = credit_limit;
- sched->name = name;
- sched->timeout = timeout;
- sched->timeout_wq = timeout_wq ? : system_wq;
- sched->hang_limit = hang_limit;
- sched->score = score ? score : &sched->_score;
- sched->dev = dev;
+ sched->ops = args->ops;
+ sched->credit_limit = args->credit_limit;
+ sched->name = args->name;
+ sched->timeout = args->timeout;
+ sched->timeout_wq = args->timeout_wq ? : system_wq;
+ sched->hang_limit = args->hang_limit;
+ sched->score = args->score ? args->score : &sched->_score;
Could we keep it consistent and use the Elvis Operator here as well?
Just like `sched->timeout_wq`.
