Re: [PATCH v5 5/7] drm/sched: Split free_job into own work item
On 2023-10-16 11:29, Luben Tuikov wrote: > On 2023-10-16 11:12, Matthew Brost wrote: >> On Sat, Oct 14, 2023 at 08:09:31PM -0400, Luben Tuikov wrote: >>> On 2023-10-13 22:49, Luben Tuikov wrote: On 2023-10-11 19:58, Matthew Brost wrote: > Rather than call free_job and run_job in same work item have a dedicated > work item for each. This aligns with the design and intended use of work > queues. > > v2: >- Test for DMA_FENCE_FLAG_TIMESTAMP_BIT before setting > timestamp in free_job() work item (Danilo) > v3: > - Drop forward dec of drm_sched_select_entity (Boris) > - Return in drm_sched_run_job_work if entity NULL (Boris) > v4: > - Replace dequeue with peek and invert logic (Luben) > - Wrap to 100 lines (Luben) > - Update comments for *_queue / *_queue_if_ready functions (Luben) > > Signed-off-by: Matthew Brost I wasn't able to apply this patch on top of drm-misc-next at a48e2cc92835fa. Create a branch off of a *clean* drm-misc-next and rebase/reapply/cherry-pick all 7 patches on top of that clean drm-misc-next branch. You should also run scripts/checkpatch.pl --strict on all your patches, or integrate it into the precommit hook, see githooks(5), so you don't have to run it manually. Set format.useAutobase to whenAble somewhere in your Git configs (global/local), or use --base=auto to git-format-patch when you format your patches before git-send-email-ing them. Repost your patches. The base commit will be added to the bottom of the cover letter. It should be an ancestor (or tip) of drm-misc-next (git branch --contains --list etc.). If it is not, your base tree wasn't clean, and you should redo this process. >>> >>> Poking around I found out that this patch set is based off of >>> drm-misc-fixes. >>> Had the base been included, it would've made this easier. >> >> I believe I based this off of drm-tip. Will follow the flow mentioned above >> in next rev. > > Sure. You can use drm-tip, drm-misc-next, drm-misc-fixes, just make sure > --base=auto > is set as outlined above, and if you're including a cover letter (a free > format text), > you can mention the base branch in there too, even though useAutobase has > included the SHA, > it makes it easier. Using -fixes or -tip is better as it includes Christian's dma_fence_timestamp() helper, and we want that in. -- Regards, Luben
Re: [PATCH v5 5/7] drm/sched: Split free_job into own work item
On 2023-10-16 11:12, Matthew Brost wrote: > On Sat, Oct 14, 2023 at 08:09:31PM -0400, Luben Tuikov wrote: >> On 2023-10-13 22:49, Luben Tuikov wrote: >>> On 2023-10-11 19:58, Matthew Brost wrote: Rather than call free_job and run_job in same work item have a dedicated work item for each. This aligns with the design and intended use of work queues. v2: - Test for DMA_FENCE_FLAG_TIMESTAMP_BIT before setting timestamp in free_job() work item (Danilo) v3: - Drop forward dec of drm_sched_select_entity (Boris) - Return in drm_sched_run_job_work if entity NULL (Boris) v4: - Replace dequeue with peek and invert logic (Luben) - Wrap to 100 lines (Luben) - Update comments for *_queue / *_queue_if_ready functions (Luben) Signed-off-by: Matthew Brost >>> >>> I wasn't able to apply this patch on top of drm-misc-next at a48e2cc92835fa. >>> >>> Create a branch off of a *clean* drm-misc-next and >>> rebase/reapply/cherry-pick all >>> 7 patches on top of that clean drm-misc-next branch. You should also run >>> scripts/checkpatch.pl --strict on all your patches, or integrate it into >>> the precommit >>> hook, see githooks(5), so you don't have to run it manually. >>> >>> Set format.useAutobase to whenAble somewhere in your Git configs >>> (global/local), >>> or use --base=auto to git-format-patch when you format your patches before >>> git-send-email-ing them. >>> >>> Repost your patches. >>> >>> The base commit will be added to the bottom of the cover letter. It should >>> be an ancestor (or tip) of drm-misc-next (git branch --contains >>> --list etc.). >>> If it is not, your base tree wasn't clean, and you should redo this process. >> >> Poking around I found out that this patch set is based off of drm-misc-fixes. >> Had the base been included, it would've made this easier. > > I believe I based this off of drm-tip. Will follow the flow mentioned above > in next rev. Sure. You can use drm-tip, drm-misc-next, drm-misc-fixes, just make sure --base=auto is set as outlined above, and if you're including a cover letter (a free format text), you can mention the base branch in there too, even though useAutobase has included the SHA, it makes it easier. -- Regards, Luben
Re: [PATCH v5 5/7] drm/sched: Split free_job into own work item
On Sat, Oct 14, 2023 at 08:09:31PM -0400, Luben Tuikov wrote: > On 2023-10-13 22:49, Luben Tuikov wrote: > > On 2023-10-11 19:58, Matthew Brost wrote: > >> Rather than call free_job and run_job in same work item have a dedicated > >> work item for each. This aligns with the design and intended use of work > >> queues. > >> > >> v2: > >>- Test for DMA_FENCE_FLAG_TIMESTAMP_BIT before setting > >> timestamp in free_job() work item (Danilo) > >> v3: > >> - Drop forward dec of drm_sched_select_entity (Boris) > >> - Return in drm_sched_run_job_work if entity NULL (Boris) > >> v4: > >> - Replace dequeue with peek and invert logic (Luben) > >> - Wrap to 100 lines (Luben) > >> - Update comments for *_queue / *_queue_if_ready functions (Luben) > >> > >> Signed-off-by: Matthew Brost > > > > I wasn't able to apply this patch on top of drm-misc-next at a48e2cc92835fa. > > > > Create a branch off of a *clean* drm-misc-next and > > rebase/reapply/cherry-pick all > > 7 patches on top of that clean drm-misc-next branch. You should also run > > scripts/checkpatch.pl --strict on all your patches, or integrate it into > > the precommit > > hook, see githooks(5), so you don't have to run it manually. > > > > Set format.useAutobase to whenAble somewhere in your Git configs > > (global/local), > > or use --base=auto to git-format-patch when you format your patches before > > git-send-email-ing them. > > > > Repost your patches. > > > > The base commit will be added to the bottom of the cover letter. It should > > be an ancestor (or tip) of drm-misc-next (git branch --contains > > --list etc.). > > If it is not, your base tree wasn't clean, and you should redo this process. > > Poking around I found out that this patch set is based off of drm-misc-fixes. > Had the base been included, it would've made this easier. I believe I based this off of drm-tip. Will follow the flow mentioned above in next rev. Matt > -- > Regards, > Luben >
Re: [PATCH v5 5/7] drm/sched: Split free_job into own work item
On 2023-10-13 22:49, Luben Tuikov wrote: > On 2023-10-11 19:58, Matthew Brost wrote: >> Rather than call free_job and run_job in same work item have a dedicated >> work item for each. This aligns with the design and intended use of work >> queues. >> >> v2: >>- Test for DMA_FENCE_FLAG_TIMESTAMP_BIT before setting >> timestamp in free_job() work item (Danilo) >> v3: >> - Drop forward dec of drm_sched_select_entity (Boris) >> - Return in drm_sched_run_job_work if entity NULL (Boris) >> v4: >> - Replace dequeue with peek and invert logic (Luben) >> - Wrap to 100 lines (Luben) >> - Update comments for *_queue / *_queue_if_ready functions (Luben) >> >> Signed-off-by: Matthew Brost > > I wasn't able to apply this patch on top of drm-misc-next at a48e2cc92835fa. > > Create a branch off of a *clean* drm-misc-next and rebase/reapply/cherry-pick > all > 7 patches on top of that clean drm-misc-next branch. You should also run > scripts/checkpatch.pl --strict on all your patches, or integrate it into the > precommit > hook, see githooks(5), so you don't have to run it manually. > > Set format.useAutobase to whenAble somewhere in your Git configs > (global/local), > or use --base=auto to git-format-patch when you format your patches before > git-send-email-ing them. > > Repost your patches. > > The base commit will be added to the bottom of the cover letter. It should > be an ancestor (or tip) of drm-misc-next (git branch --contains --list > etc.). > If it is not, your base tree wasn't clean, and you should redo this process. Poking around I found out that this patch set is based off of drm-misc-fixes. Had the base been included, it would've made this easier. -- Regards, Luben
Re: [PATCH v5 5/7] drm/sched: Split free_job into own work item
On 2023-10-11 19:58, Matthew Brost wrote:
> Rather than call free_job and run_job in same work item have a dedicated
> work item for each. This aligns with the design and intended use of work
> queues.
>
> v2:
>- Test for DMA_FENCE_FLAG_TIMESTAMP_BIT before setting
> timestamp in free_job() work item (Danilo)
> v3:
> - Drop forward dec of drm_sched_select_entity (Boris)
> - Return in drm_sched_run_job_work if entity NULL (Boris)
> v4:
> - Replace dequeue with peek and invert logic (Luben)
> - Wrap to 100 lines (Luben)
> - Update comments for *_queue / *_queue_if_ready functions (Luben)
>
> Signed-off-by: Matthew Brost
I wasn't able to apply this patch on top of drm-misc-next at a48e2cc92835fa.
Create a branch off of a *clean* drm-misc-next and rebase/reapply/cherry-pick
all
7 patches on top of that clean drm-misc-next branch. You should also run
scripts/checkpatch.pl --strict on all your patches, or integrate it into the
precommit
hook, see githooks(5), so you don't have to run it manually.
Set format.useAutobase to whenAble somewhere in your Git configs (global/local),
or use --base=auto to git-format-patch when you format your patches before
git-send-email-ing them.
Repost your patches.
The base commit will be added to the bottom of the cover letter. It should
be an ancestor (or tip) of drm-misc-next (git branch --contains --list
etc.).
If it is not, your base tree wasn't clean, and you should redo this process.
--
Regards,
Luben
> ---
> drivers/gpu/drm/scheduler/sched_main.c | 287 +++--
> include/drm/gpu_scheduler.h| 8 +-
> 2 files changed, 178 insertions(+), 117 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
> b/drivers/gpu/drm/scheduler/sched_main.c
> index 7dbb3392124d..cf4c23db7547 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -213,11 +213,12 @@ void drm_sched_rq_remove_entity(struct drm_sched_rq *rq,
> * drm_sched_rq_select_entity_rr - Select an entity which could provide a
> job to run
> *
> * @rq: scheduler run queue to check.
> + * @peek: Just find, don't set to current.
> *
> * Try to find a ready entity, returns NULL if none found.
> */
> static struct drm_sched_entity *
> -drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq)
> +drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq, bool peek)
> {
> struct drm_sched_entity *entity;
>
> @@ -227,8 +228,10 @@ drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq)
> if (entity) {
> list_for_each_entry_continue(entity, >entities, list) {
> if (drm_sched_entity_is_ready(entity)) {
> - rq->current_entity = entity;
> - reinit_completion(>entity_idle);
> + if (!peek) {
> + rq->current_entity = entity;
> + reinit_completion(>entity_idle);
> + }
> spin_unlock(>lock);
> return entity;
> }
> @@ -238,8 +241,10 @@ drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq)
> list_for_each_entry(entity, >entities, list) {
>
> if (drm_sched_entity_is_ready(entity)) {
> - rq->current_entity = entity;
> - reinit_completion(>entity_idle);
> + if (!peek) {
> + rq->current_entity = entity;
> + reinit_completion(>entity_idle);
> + }
> spin_unlock(>lock);
> return entity;
> }
> @@ -257,11 +262,12 @@ drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq)
> * drm_sched_rq_select_entity_fifo - Select an entity which provides a job
> to run
> *
> * @rq: scheduler run queue to check.
> + * @peek: Just find, don't set to current.
> *
> * Find oldest waiting ready entity, returns NULL if none found.
> */
> static struct drm_sched_entity *
> -drm_sched_rq_select_entity_fifo(struct drm_sched_rq *rq)
> +drm_sched_rq_select_entity_fifo(struct drm_sched_rq *rq, bool peek)
> {
> struct rb_node *rb;
>
> @@ -271,8 +277,10 @@ drm_sched_rq_select_entity_fifo(struct drm_sched_rq *rq)
>
> entity = rb_entry(rb, struct drm_sched_entity, rb_tree_node);
> if (drm_sched_entity_is_ready(entity)) {
> - rq->current_entity = entity;
> - reinit_completion(>entity_idle);
> + if (!peek) {
> + rq->current_entity = entity;
> + reinit_completion(>entity_idle);
> + }
> break;
> }
> }
> @@ -282,13 +290,98 @@ drm_sched_rq_select_entity_fifo(struct drm_sched_rq *rq)
> }
>
> /**
> - *
