Re: [PATCH 2/2] drm/sched: Rename to drm_sched_wakeup_if_canqueue()
Ignore this series--I'll repost without the duplication. Regards, Luben On 2023-05-17 19:08, Luben Tuikov wrote: > Rename drm_sched_wakeup() to drm_sched_wakeup_if_canqueue() since the former > is misleading, as it wakes up the GPU scheduler _only if_ more jobs can be > queued to the underlying hardware. > > This distinction is important to make, since the wake conditional in the GPU > scheduler thread wakes up when other conditions are also true, e.g. when there > are jobs to be cleaned. For instance, a user might want to wake up the > scheduler only because there are more jobs to clean, but whether we can queue > more jobs is irrelevant. > > Cc: Christian König > Cc: Alex Deucher > Signed-off-by: Luben Tuikov > --- > drivers/gpu/drm/scheduler/sched_entity.c | 4 ++-- > drivers/gpu/drm/scheduler/sched_main.c | 6 +++--- > include/drm/gpu_scheduler.h | 2 +- > 3 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/scheduler/sched_entity.c > b/drivers/gpu/drm/scheduler/sched_entity.c > index cfb433e9200586..371b44fb4bacf4 100644 > --- a/drivers/gpu/drm/scheduler/sched_entity.c > +++ b/drivers/gpu/drm/scheduler/sched_entity.c > @@ -342,7 +342,7 @@ static void drm_sched_entity_wakeup(struct dma_fence *f, > container_of(cb, struct drm_sched_entity, cb); > > drm_sched_entity_clear_dep(f, cb); > - drm_sched_wakeup(entity->rq->sched); > + drm_sched_wakeup_if_canqueue(entity->rq->sched); > } > > /** > @@ -565,7 +565,7 @@ void drm_sched_entity_push_job(struct drm_sched_job > *sched_job) > if (drm_sched_policy == DRM_SCHED_POLICY_FIFO) > drm_sched_rq_update_fifo(entity, submit_ts); > > - drm_sched_wakeup(entity->rq->sched); > + drm_sched_wakeup_if_canqueue(entity->rq->sched); > } > } > EXPORT_SYMBOL(drm_sched_entity_push_job); > diff --git a/drivers/gpu/drm/scheduler/sched_main.c > b/drivers/gpu/drm/scheduler/sched_main.c > index 8739322c30321b..f437dbe62112cc 100644 > --- a/drivers/gpu/drm/scheduler/sched_main.c > +++ b/drivers/gpu/drm/scheduler/sched_main.c > @@ -860,12 +860,12 @@ static bool drm_sched_can_queue(struct > drm_gpu_scheduler *sched) > } > > /** > - * drm_sched_wakeup - Wake up the scheduler when it is ready > - * > + * drm_sched_wakeup_if_canqueue - Wake up the scheduler > * @sched: scheduler instance > * > + * Wake up the scheduler if we can queue jobs. > */ > -void drm_sched_wakeup(struct drm_gpu_scheduler *sched) > +void drm_sched_wakeup_if_canqueue(struct drm_gpu_scheduler *sched) > { > if (drm_sched_can_queue(sched)) > wake_up_interruptible(>wake_up_worker); > diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h > index 31d1f5166c79fe..a05dd01e264842 100644 > --- a/include/drm/gpu_scheduler.h > +++ b/include/drm/gpu_scheduler.h > @@ -549,7 +549,7 @@ void drm_sched_entity_modify_sched(struct > drm_sched_entity *entity, > unsigned int num_sched_list); > > void drm_sched_job_cleanup(struct drm_sched_job *job); > -void drm_sched_wakeup(struct drm_gpu_scheduler *sched); > +void drm_sched_wakeup_if_canqueue(struct drm_gpu_scheduler *sched); > void drm_sched_stop(struct drm_gpu_scheduler *sched, struct drm_sched_job > *bad); > void drm_sched_start(struct drm_gpu_scheduler *sched, bool full_recovery); > void drm_sched_resubmit_jobs(struct drm_gpu_scheduler *sched);
Re: [PATCH 2/2] drm/sched: Rename to drm_sched_wakeup_if_canqueue()
On 2023-05-17 16:43, Alex Deucher wrote: > On Wed, May 17, 2023 at 3:04 PM Luben Tuikov wrote: >> >> Rename drm_sched_wakeup() to drm_sched_wakeup_if_canqueue() since the former > > I think drm_sched_wakeup_if_can_queue() looks cleaner. Yeah, I can change it to this--I was concerned of too many underscores in the name. (I was also considering drm_sched_wakeup_queue_cond() to mean "wake up if we can queue conditionally...) Regards, Luben > > Alex > >> is misleading, as it wakes up the GPU scheduler _only if_ more jobs can be >> queued to the underlying hardware. >> >> This distinction is important to make, since the wake conditional in the GPU >> scheduler thread wakes up when other conditions are also true, e.g. when >> there >> are jobs to be cleaned. For instance, a user might want to wake up the >> scheduler only because there are more jobs to clean, but whether we can queue >> more jobs is irrelevant. >> >> Cc: Christian König >> Cc: Alex Deucher >> Signed-off-by: Luben Tuikov >> --- >> drivers/gpu/drm/scheduler/sched_entity.c | 4 ++-- >> drivers/gpu/drm/scheduler/sched_main.c | 6 +++--- >> include/drm/gpu_scheduler.h | 2 +- >> 3 files changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c >> b/drivers/gpu/drm/scheduler/sched_entity.c >> index cfb433e9200586..371b44fb4bacf4 100644 >> --- a/drivers/gpu/drm/scheduler/sched_entity.c >> +++ b/drivers/gpu/drm/scheduler/sched_entity.c >> @@ -342,7 +342,7 @@ static void drm_sched_entity_wakeup(struct dma_fence *f, >> container_of(cb, struct drm_sched_entity, cb); >> >> drm_sched_entity_clear_dep(f, cb); >> - drm_sched_wakeup(entity->rq->sched); >> + drm_sched_wakeup_if_canqueue(entity->rq->sched); >> } >> >> /** >> @@ -565,7 +565,7 @@ void drm_sched_entity_push_job(struct drm_sched_job >> *sched_job) >> if (drm_sched_policy == DRM_SCHED_POLICY_FIFO) >> drm_sched_rq_update_fifo(entity, submit_ts); >> >> - drm_sched_wakeup(entity->rq->sched); >> + drm_sched_wakeup_if_canqueue(entity->rq->sched); >> } >> } >> EXPORT_SYMBOL(drm_sched_entity_push_job); >> diff --git a/drivers/gpu/drm/scheduler/sched_main.c >> b/drivers/gpu/drm/scheduler/sched_main.c >> index 8739322c30321b..f437dbe62112cc 100644 >> --- a/drivers/gpu/drm/scheduler/sched_main.c >> +++ b/drivers/gpu/drm/scheduler/sched_main.c >> @@ -860,12 +860,12 @@ static bool drm_sched_can_queue(struct >> drm_gpu_scheduler *sched) >> } >> >> /** >> - * drm_sched_wakeup - Wake up the scheduler when it is ready >> - * >> + * drm_sched_wakeup_if_canqueue - Wake up the scheduler >> * @sched: scheduler instance >> * >> + * Wake up the scheduler if we can queue jobs. >> */ >> -void drm_sched_wakeup(struct drm_gpu_scheduler *sched) >> +void drm_sched_wakeup_if_canqueue(struct drm_gpu_scheduler *sched) >> { >> if (drm_sched_can_queue(sched)) >> wake_up_interruptible(>wake_up_worker); >> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h >> index 31d1f5166c79fe..a05dd01e264842 100644 >> --- a/include/drm/gpu_scheduler.h >> +++ b/include/drm/gpu_scheduler.h >> @@ -549,7 +549,7 @@ void drm_sched_entity_modify_sched(struct >> drm_sched_entity *entity, >> unsigned int num_sched_list); >> >> void drm_sched_job_cleanup(struct drm_sched_job *job); >> -void drm_sched_wakeup(struct drm_gpu_scheduler *sched); >> +void drm_sched_wakeup_if_canqueue(struct drm_gpu_scheduler *sched); >> void drm_sched_stop(struct drm_gpu_scheduler *sched, struct drm_sched_job >> *bad); >> void drm_sched_start(struct drm_gpu_scheduler *sched, bool full_recovery); >> void drm_sched_resubmit_jobs(struct drm_gpu_scheduler *sched); >> -- >> 2.40.1 >>
Re: [PATCH 2/2] drm/sched: Rename to drm_sched_wakeup_if_canqueue()
On Wed, May 17, 2023 at 3:04 PM Luben Tuikov wrote: > > Rename drm_sched_wakeup() to drm_sched_wakeup_if_canqueue() since the former I think drm_sched_wakeup_if_can_queue() looks cleaner. Alex > is misleading, as it wakes up the GPU scheduler _only if_ more jobs can be > queued to the underlying hardware. > > This distinction is important to make, since the wake conditional in the GPU > scheduler thread wakes up when other conditions are also true, e.g. when there > are jobs to be cleaned. For instance, a user might want to wake up the > scheduler only because there are more jobs to clean, but whether we can queue > more jobs is irrelevant. > > Cc: Christian König > Cc: Alex Deucher > Signed-off-by: Luben Tuikov > --- > drivers/gpu/drm/scheduler/sched_entity.c | 4 ++-- > drivers/gpu/drm/scheduler/sched_main.c | 6 +++--- > include/drm/gpu_scheduler.h | 2 +- > 3 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/scheduler/sched_entity.c > b/drivers/gpu/drm/scheduler/sched_entity.c > index cfb433e9200586..371b44fb4bacf4 100644 > --- a/drivers/gpu/drm/scheduler/sched_entity.c > +++ b/drivers/gpu/drm/scheduler/sched_entity.c > @@ -342,7 +342,7 @@ static void drm_sched_entity_wakeup(struct dma_fence *f, > container_of(cb, struct drm_sched_entity, cb); > > drm_sched_entity_clear_dep(f, cb); > - drm_sched_wakeup(entity->rq->sched); > + drm_sched_wakeup_if_canqueue(entity->rq->sched); > } > > /** > @@ -565,7 +565,7 @@ void drm_sched_entity_push_job(struct drm_sched_job > *sched_job) > if (drm_sched_policy == DRM_SCHED_POLICY_FIFO) > drm_sched_rq_update_fifo(entity, submit_ts); > > - drm_sched_wakeup(entity->rq->sched); > + drm_sched_wakeup_if_canqueue(entity->rq->sched); > } > } > EXPORT_SYMBOL(drm_sched_entity_push_job); > diff --git a/drivers/gpu/drm/scheduler/sched_main.c > b/drivers/gpu/drm/scheduler/sched_main.c > index 8739322c30321b..f437dbe62112cc 100644 > --- a/drivers/gpu/drm/scheduler/sched_main.c > +++ b/drivers/gpu/drm/scheduler/sched_main.c > @@ -860,12 +860,12 @@ static bool drm_sched_can_queue(struct > drm_gpu_scheduler *sched) > } > > /** > - * drm_sched_wakeup - Wake up the scheduler when it is ready > - * > + * drm_sched_wakeup_if_canqueue - Wake up the scheduler > * @sched: scheduler instance > * > + * Wake up the scheduler if we can queue jobs. > */ > -void drm_sched_wakeup(struct drm_gpu_scheduler *sched) > +void drm_sched_wakeup_if_canqueue(struct drm_gpu_scheduler *sched) > { > if (drm_sched_can_queue(sched)) > wake_up_interruptible(>wake_up_worker); > diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h > index 31d1f5166c79fe..a05dd01e264842 100644 > --- a/include/drm/gpu_scheduler.h > +++ b/include/drm/gpu_scheduler.h > @@ -549,7 +549,7 @@ void drm_sched_entity_modify_sched(struct > drm_sched_entity *entity, > unsigned int num_sched_list); > > void drm_sched_job_cleanup(struct drm_sched_job *job); > -void drm_sched_wakeup(struct drm_gpu_scheduler *sched); > +void drm_sched_wakeup_if_canqueue(struct drm_gpu_scheduler *sched); > void drm_sched_stop(struct drm_gpu_scheduler *sched, struct drm_sched_job > *bad); > void drm_sched_start(struct drm_gpu_scheduler *sched, bool full_recovery); > void drm_sched_resubmit_jobs(struct drm_gpu_scheduler *sched); > -- > 2.40.1 >