Re: [PATCH v2 2/9] drm/sched: Move schedule policy to scheduler / entity
On Fri, Aug 11, 2023 at 06:43:22PM -0300, Maira Canal wrote:
> Hi Matthew,
>
> I'm not sure in which tree you plan to apply this series, but if you
> plan to apply it on drm-misc-next, it would be nice to rebase on top of
> it. It would make it easier for driver maintainers to review it.
>
I rebased this on drm-tip but forgot the first patch in the series.
Let me make sure I get this correct and will send a rev3 early next week.
> Apart from the small nit below it, I tested the Xe tree on v3d and things
> seems to be running smoothly.
>
> On 8/10/23 23:31, Matthew Brost wrote:
> > Rather than a global modparam for scheduling policy, move the scheduling
> > policy to scheduler / entity so user can control each scheduler / entity
> > policy.
> >
> > v2:
> >- s/DRM_SCHED_POLICY_MAX/DRM_SCHED_POLICY_COUNT (Luben)
> >- Only include policy in scheduler (Luben)
> >
> > Signed-off-by: Matthew Brost
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
> > drivers/gpu/drm/etnaviv/etnaviv_sched.c| 3 ++-
> > drivers/gpu/drm/lima/lima_sched.c | 3 ++-
> > drivers/gpu/drm/msm/msm_ringbuffer.c | 3 ++-
> > drivers/gpu/drm/nouveau/nouveau_sched.c| 3 ++-
> > drivers/gpu/drm/panfrost/panfrost_job.c| 3 ++-
> > drivers/gpu/drm/scheduler/sched_entity.c | 24 ++
> > drivers/gpu/drm/scheduler/sched_main.c | 23 +++--
> > drivers/gpu/drm/v3d/v3d_sched.c| 15 +-
> > include/drm/gpu_scheduler.h| 20 --
> > 10 files changed, 72 insertions(+), 26 deletions(-)
> >
>
> [...]
>
> > diff --git a/drivers/gpu/drm/v3d/v3d_sched.c
> > b/drivers/gpu/drm/v3d/v3d_sched.c
> > index 38e092ea41e6..5e3fe77fa991 100644
> > --- a/drivers/gpu/drm/v3d/v3d_sched.c
> > +++ b/drivers/gpu/drm/v3d/v3d_sched.c
> > @@ -391,7 +391,8 @@ v3d_sched_init(struct v3d_dev *v3d)
> > _bin_sched_ops, NULL,
> > hw_jobs_limit, job_hang_limit,
> > msecs_to_jiffies(hang_limit_ms), NULL,
> > -NULL, "v3d_bin", v3d->drm.dev);
> > +NULL, "v3d_bin", DRM_SCHED_POLICY_DEFAULT,
> > +v3d->drm.dev);
> > if (ret)
> > return ret;
> > @@ -399,7 +400,8 @@ v3d_sched_init(struct v3d_dev *v3d)
> > _render_sched_ops, NULL,
> > hw_jobs_limit, job_hang_limit,
> > msecs_to_jiffies(hang_limit_ms), NULL,
> > -NULL, "v3d_render", v3d->drm.dev);
> > +ULL, "v3d_render", DRM_SCHED_POLICY_DEFAULT,
>
> Small nit: s/ULL/NULL
>
Yep, will fix.
Matt
> Best Regards,
> - Maíra
>
> > +v3d->drm.dev);
> > if (ret)
> > goto fail;
> > @@ -407,7 +409,8 @@ v3d_sched_init(struct v3d_dev *v3d)
> > _tfu_sched_ops, NULL,
> > hw_jobs_limit, job_hang_limit,
> > msecs_to_jiffies(hang_limit_ms), NULL,
> > -NULL, "v3d_tfu", v3d->drm.dev);
> > +NULL, "v3d_tfu", DRM_SCHED_POLICY_DEFAULT,
> > +v3d->drm.dev);
> > if (ret)
> > goto fail;
> > @@ -416,7 +419,8 @@ v3d_sched_init(struct v3d_dev *v3d)
> > _csd_sched_ops, NULL,
> > hw_jobs_limit, job_hang_limit,
> > msecs_to_jiffies(hang_limit_ms), NULL,
> > -NULL, "v3d_csd", v3d->drm.dev);
> > +NULL, "v3d_csd", DRM_SCHED_POLICY_DEFAULT,
> > +v3d->drm.dev);
> > if (ret)
> > goto fail;
> > @@ -424,7 +428,8 @@ v3d_sched_init(struct v3d_dev *v3d)
> > _cache_clean_sched_ops, NULL,
> > hw_jobs_limit, job_hang_limit,
> > msecs_to_jiffies(hang_limit_ms), NULL,
> > -NULL, "v3d_cache_clean", v3d->drm.dev);
> > +NULL, "v3d_cache_clean",
> > +DRM_SCHED_POLICY_DEFAULT, v3d->drm.dev);
> > if (ret)
> > goto fail;
> > }
> > diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> > index 278106e358a8..897d52a4ff4f 100644
> > --- a/include/drm/gpu_scheduler.h
> > +++ b/include/drm/gpu_scheduler.h
> > @@ -72,11 +72,15 @@ enum drm_sched_priority {
> > DRM_SCHED_PRIORITY_UNSET = -2
> > };
> > -/* Used to chose between FIFO and RR jobs scheduling */
> > -extern int drm_sched_policy;
> > -
> > -#define DRM_SCHED_POLICY_RR0
> > -#define DRM_SCHED_POLICY_FIFO 1
> > +/* Used to chose default scheduling policy*/
> > +extern int default_drm_sched_policy;
> > +
> > +enum
Re: [PATCH v2 2/9] drm/sched: Move schedule policy to scheduler / entity
Hi Matthew,
I'm not sure in which tree you plan to apply this series, but if you
plan to apply it on drm-misc-next, it would be nice to rebase on top of
it. It would make it easier for driver maintainers to review it.
Apart from the small nit below it, I tested the Xe tree on v3d and
things seems to be running smoothly.
On 8/10/23 23:31, Matthew Brost wrote:
Rather than a global modparam for scheduling policy, move the scheduling
policy to scheduler / entity so user can control each scheduler / entity
policy.
v2:
- s/DRM_SCHED_POLICY_MAX/DRM_SCHED_POLICY_COUNT (Luben)
- Only include policy in scheduler (Luben)
Signed-off-by: Matthew Brost
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
drivers/gpu/drm/etnaviv/etnaviv_sched.c| 3 ++-
drivers/gpu/drm/lima/lima_sched.c | 3 ++-
drivers/gpu/drm/msm/msm_ringbuffer.c | 3 ++-
drivers/gpu/drm/nouveau/nouveau_sched.c| 3 ++-
drivers/gpu/drm/panfrost/panfrost_job.c| 3 ++-
drivers/gpu/drm/scheduler/sched_entity.c | 24 ++
drivers/gpu/drm/scheduler/sched_main.c | 23 +++--
drivers/gpu/drm/v3d/v3d_sched.c| 15 +-
include/drm/gpu_scheduler.h| 20 --
10 files changed, 72 insertions(+), 26 deletions(-)
[...]
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 38e092ea41e6..5e3fe77fa991 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -391,7 +391,8 @@ v3d_sched_init(struct v3d_dev *v3d)
_bin_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_bin", v3d->drm.dev);
+NULL, "v3d_bin", DRM_SCHED_POLICY_DEFAULT,
+v3d->drm.dev);
if (ret)
return ret;
@@ -399,7 +400,8 @@ v3d_sched_init(struct v3d_dev *v3d)
_render_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_render", v3d->drm.dev);
+ULL, "v3d_render", DRM_SCHED_POLICY_DEFAULT,
Small nit: s/ULL/NULL
Best Regards,
- Maíra
+v3d->drm.dev);
if (ret)
goto fail;
@@ -407,7 +409,8 @@ v3d_sched_init(struct v3d_dev *v3d)
_tfu_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_tfu", v3d->drm.dev);
+NULL, "v3d_tfu", DRM_SCHED_POLICY_DEFAULT,
+v3d->drm.dev);
if (ret)
goto fail;
@@ -416,7 +419,8 @@ v3d_sched_init(struct v3d_dev *v3d)
_csd_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_csd", v3d->drm.dev);
+NULL, "v3d_csd", DRM_SCHED_POLICY_DEFAULT,
+v3d->drm.dev);
if (ret)
goto fail;
@@ -424,7 +428,8 @@ v3d_sched_init(struct v3d_dev *v3d)
_cache_clean_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_cache_clean", v3d->drm.dev);
+NULL, "v3d_cache_clean",
+DRM_SCHED_POLICY_DEFAULT, v3d->drm.dev);
if (ret)
goto fail;
}
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 278106e358a8..897d52a4ff4f 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -72,11 +72,15 @@ enum drm_sched_priority {
DRM_SCHED_PRIORITY_UNSET = -2
};
-/* Used to chose between FIFO and RR jobs scheduling */
-extern int drm_sched_policy;
-
-#define DRM_SCHED_POLICY_RR0
-#define DRM_SCHED_POLICY_FIFO 1
+/* Used to chose default scheduling policy*/
+extern int default_drm_sched_policy;
+
+enum drm_sched_policy {
+ DRM_SCHED_POLICY_DEFAULT,
+ DRM_SCHED_POLICY_RR,
+ DRM_SCHED_POLICY_FIFO,
+ DRM_SCHED_POLICY_COUNT,
+};
/**
* struct drm_sched_entity - A wrapper around a job queue (typically
@@ -489,6 +493,7 @@ struct drm_sched_backend_ops {
* guilty and it will no longer be considered for scheduling.
* @score: score to help loadbalancer pick a idle sched
* @_score:
[PATCH v2 2/9] drm/sched: Move schedule policy to scheduler / entity
Rather than a global modparam for scheduling policy, move the scheduling
policy to scheduler / entity so user can control each scheduler / entity
policy.
v2:
- s/DRM_SCHED_POLICY_MAX/DRM_SCHED_POLICY_COUNT (Luben)
- Only include policy in scheduler (Luben)
Signed-off-by: Matthew Brost
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
drivers/gpu/drm/etnaviv/etnaviv_sched.c| 3 ++-
drivers/gpu/drm/lima/lima_sched.c | 3 ++-
drivers/gpu/drm/msm/msm_ringbuffer.c | 3 ++-
drivers/gpu/drm/nouveau/nouveau_sched.c| 3 ++-
drivers/gpu/drm/panfrost/panfrost_job.c| 3 ++-
drivers/gpu/drm/scheduler/sched_entity.c | 24 ++
drivers/gpu/drm/scheduler/sched_main.c | 23 +++--
drivers/gpu/drm/v3d/v3d_sched.c| 15 +-
include/drm/gpu_scheduler.h| 20 --
10 files changed, 72 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 3ebf9882edba..993a637dca0a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2335,6 +2335,7 @@ static int amdgpu_device_init_schedulers(struct
amdgpu_device *adev)
ring->num_hw_submission, 0,
timeout, adev->reset_domain->wq,
ring->sched_score, ring->name,
+ DRM_SCHED_POLICY_DEFAULT,
adev->dev);
if (r) {
DRM_ERROR("Failed to create scheduler on ring %s.\n",
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index 8486a2923f1b..61204a3f8b0b 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -136,7 +136,8 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu)
ret = drm_sched_init(>sched, _sched_ops, NULL,
etnaviv_hw_jobs_limit, etnaviv_job_hang_limit,
msecs_to_jiffies(500), NULL, NULL,
-dev_name(gpu->dev), gpu->dev);
+dev_name(gpu->dev), DRM_SCHED_POLICY_DEFAULT,
+gpu->dev);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/lima/lima_sched.c
b/drivers/gpu/drm/lima/lima_sched.c
index 8d858aed0e56..465d4bf3882b 100644
--- a/drivers/gpu/drm/lima/lima_sched.c
+++ b/drivers/gpu/drm/lima/lima_sched.c
@@ -491,7 +491,8 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe,
const char *name)
return drm_sched_init(>base, _sched_ops, NULL, 1,
lima_job_hang_limit,
msecs_to_jiffies(timeout), NULL,
- NULL, name, pipe->ldev->dev);
+ NULL, name, DRM_SCHED_POLICY_DEFAULT,
+ pipe->ldev->dev);
}
void lima_sched_pipe_fini(struct lima_sched_pipe *pipe)
diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c
b/drivers/gpu/drm/msm/msm_ringbuffer.c
index 79aa112118da..78f05be89b61 100644
--- a/drivers/gpu/drm/msm/msm_ringbuffer.c
+++ b/drivers/gpu/drm/msm/msm_ringbuffer.c
@@ -95,7 +95,8 @@ struct msm_ringbuffer *msm_ringbuffer_new(struct msm_gpu
*gpu, int id,
ret = drm_sched_init(>sched, _sched_ops, NULL,
num_hw_submissions, 0, sched_timeout,
- NULL, NULL, to_msm_bo(ring->bo)->name, gpu->dev->dev);
+ NULL, NULL, to_msm_bo(ring->bo)->name,
+ DRM_SCHED_POLICY_DEFAULT, gpu->dev->dev);
if (ret) {
goto fail;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.c
b/drivers/gpu/drm/nouveau/nouveau_sched.c
index 2d607de5d393..59c263acf347 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sched.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sched.c
@@ -409,7 +409,8 @@ int nouveau_sched_init(struct nouveau_drm *drm)
return drm_sched_init(sched, _sched_ops, NULL,
NOUVEAU_SCHED_HW_SUBMISSIONS, 0, job_hang_limit,
- NULL, NULL, "nouveau_sched", drm->dev->dev);
+ NULL, NULL, "nouveau_sched",
+ DRM_SCHED_POLICY_DEFAULT, drm->dev->dev);
}
void nouveau_sched_fini(struct nouveau_drm *drm)
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c
b/drivers/gpu/drm/panfrost/panfrost_job.c
index 4efbc8aa3c2f..c051aa01bb92 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -835,7 +835,8 @@ int panfrost_job_init(struct panfrost_device *pfdev)
nentries, 0,
msecs_to_jiffies(JOB_TIMEOUT_MS),
pfdev->reset.wq,
-
