Re: [PATCH 1/4] drm/sched: returns struct drm_gpu_scheduler ** for drm_sched_pick_best
Please send everything together because otherwise it's not clear why we need this. Andrey On 2022-09-08 11:09, James Zhu wrote: Yes, it is for NPI design. I will send out patches for review soon. Thanks! James On 2022-09-08 11:05 a.m., Andrey Grodzovsky wrote: So this is the real need of this patch-set, but this explanation doesn't appear anywhere in the description. It's always good to add a short 0 RFC patch which describes the intention of the patchset if the code is not self explanatory. And I still don't understand the need - i don't see anything in amdgpu_ctx_fini_entity regarding rings tracking ? Is it a new code you plan to add and not included in this patcheset ? Did i miss an earlier patch maybe ? Andrey On 2022-09-08 10:45, James Zhu wrote: To save lines is not the purpose. Also I want to use entity->sched_list to track ring which is used in this ctx in amdgpu_ctx_fini_entity Best Regards! James On 2022-09-08 10:38 a.m., Andrey Grodzovsky wrote: I guess it's an option but i don't really see what's the added value ? You saved a few lines in this patch but added a few lines in another. In total seems to me no to much difference ? Andrey On 2022-09-08 10:17, James Zhu wrote: Hi Andrey Basically this entire patch set are derived from patch [3/4]: entity->sched_list = num_sched_list > 1 ? sched_list : NULL; I think no special reason to treat single and multiple schedule list here. Best Regards! James On 2022-09-08 10:08 a.m., Andrey Grodzovsky wrote: What's the reason for this entire patch set ? Andrey On 2022-09-07 16:57, James Zhu wrote: drm_sched_pick_best returns struct drm_gpu_scheduler ** instead of struct drm_gpu_scheduler * Signed-off-by: James Zhu --- include/drm/gpu_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..011f70a43397 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -529,7 +529,7 @@ void drm_sched_fence_finished(struct drm_sched_fence *fence); unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched); void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, unsigned long remaining); -struct drm_gpu_scheduler * +struct drm_gpu_scheduler ** drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, unsigned int num_sched_list);
Re: [PATCH 1/4] drm/sched: returns struct drm_gpu_scheduler ** for drm_sched_pick_best
Yes, it is for NPI design. I will send out patches for review soon. Thanks! James On 2022-09-08 11:05 a.m., Andrey Grodzovsky wrote: So this is the real need of this patch-set, but this explanation doesn't appear anywhere in the description. It's always good to add a short 0 RFC patch which describes the intention of the patchset if the code is not self explanatory. And I still don't understand the need - i don't see anything in amdgpu_ctx_fini_entity regarding rings tracking ? Is it a new code you plan to add and not included in this patcheset ? Did i miss an earlier patch maybe ? Andrey On 2022-09-08 10:45, James Zhu wrote: To save lines is not the purpose. Also I want to use entity->sched_list to track ring which is used in this ctx in amdgpu_ctx_fini_entity Best Regards! James On 2022-09-08 10:38 a.m., Andrey Grodzovsky wrote: I guess it's an option but i don't really see what's the added value ? You saved a few lines in this patch but added a few lines in another. In total seems to me no to much difference ? Andrey On 2022-09-08 10:17, James Zhu wrote: Hi Andrey Basically this entire patch set are derived from patch [3/4]: entity->sched_list = num_sched_list > 1 ? sched_list : NULL; I think no special reason to treat single and multiple schedule list here. Best Regards! James On 2022-09-08 10:08 a.m., Andrey Grodzovsky wrote: What's the reason for this entire patch set ? Andrey On 2022-09-07 16:57, James Zhu wrote: drm_sched_pick_best returns struct drm_gpu_scheduler ** instead of struct drm_gpu_scheduler * Signed-off-by: James Zhu --- include/drm/gpu_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..011f70a43397 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -529,7 +529,7 @@ void drm_sched_fence_finished(struct drm_sched_fence *fence); unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched); void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, unsigned long remaining); -struct drm_gpu_scheduler * +struct drm_gpu_scheduler ** drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, unsigned int num_sched_list);
Re: [PATCH 1/4] drm/sched: returns struct drm_gpu_scheduler ** for drm_sched_pick_best
So this is the real need of this patch-set, but this explanation doesn't appear anywhere in the description. It's always good to add a short 0 RFC patch which describes the intention of the patchset if the code is not self explanatory. And I still don't understand the need - i don't see anything in amdgpu_ctx_fini_entity regarding rings tracking ? Is it a new code you plan to add and not included in this patcheset ? Did i miss an earlier patch maybe ? Andrey On 2022-09-08 10:45, James Zhu wrote: To save lines is not the purpose. Also I want to use entity->sched_list to track ring which is used in this ctx in amdgpu_ctx_fini_entity Best Regards! James On 2022-09-08 10:38 a.m., Andrey Grodzovsky wrote: I guess it's an option but i don't really see what's the added value ? You saved a few lines in this patch but added a few lines in another. In total seems to me no to much difference ? Andrey On 2022-09-08 10:17, James Zhu wrote: Hi Andrey Basically this entire patch set are derived from patch [3/4]: entity->sched_list = num_sched_list > 1 ? sched_list : NULL; I think no special reason to treat single and multiple schedule list here. Best Regards! James On 2022-09-08 10:08 a.m., Andrey Grodzovsky wrote: What's the reason for this entire patch set ? Andrey On 2022-09-07 16:57, James Zhu wrote: drm_sched_pick_best returns struct drm_gpu_scheduler ** instead of struct drm_gpu_scheduler * Signed-off-by: James Zhu --- include/drm/gpu_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..011f70a43397 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -529,7 +529,7 @@ void drm_sched_fence_finished(struct drm_sched_fence *fence); unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched); void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, unsigned long remaining); -struct drm_gpu_scheduler * +struct drm_gpu_scheduler ** drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, unsigned int num_sched_list);
Re: [PATCH 1/4] drm/sched: returns struct drm_gpu_scheduler ** for drm_sched_pick_best
To save lines is not the purpose. Also I want to use entity->sched_list to track ring which is used in this ctx in amdgpu_ctx_fini_entity Best Regards! James On 2022-09-08 10:38 a.m., Andrey Grodzovsky wrote: I guess it's an option but i don't really see what's the added value ? You saved a few lines in this patch but added a few lines in another. In total seems to me no to much difference ? Andrey On 2022-09-08 10:17, James Zhu wrote: Hi Andrey Basically this entire patch set are derived from patch [3/4]: entity->sched_list = num_sched_list > 1 ? sched_list : NULL; I think no special reason to treat single and multiple schedule list here. Best Regards! James On 2022-09-08 10:08 a.m., Andrey Grodzovsky wrote: What's the reason for this entire patch set ? Andrey On 2022-09-07 16:57, James Zhu wrote: drm_sched_pick_best returns struct drm_gpu_scheduler ** instead of struct drm_gpu_scheduler * Signed-off-by: James Zhu --- include/drm/gpu_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..011f70a43397 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -529,7 +529,7 @@ void drm_sched_fence_finished(struct drm_sched_fence *fence); unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched); void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, unsigned long remaining); -struct drm_gpu_scheduler * +struct drm_gpu_scheduler ** drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, unsigned int num_sched_list);
Re: [PATCH 1/4] drm/sched: returns struct drm_gpu_scheduler ** for drm_sched_pick_best
I guess it's an option but i don't really see what's the added value ? You saved a few lines in this patch but added a few lines in another. In total seems to me no to much difference ? Andrey On 2022-09-08 10:17, James Zhu wrote: Hi Andrey Basically this entire patch set are derived from patch [3/4]: entity->sched_list = num_sched_list > 1 ? sched_list : NULL; I think no special reason to treat single and multiple schedule list here. Best Regards! James On 2022-09-08 10:08 a.m., Andrey Grodzovsky wrote: What's the reason for this entire patch set ? Andrey On 2022-09-07 16:57, James Zhu wrote: drm_sched_pick_best returns struct drm_gpu_scheduler ** instead of struct drm_gpu_scheduler * Signed-off-by: James Zhu --- include/drm/gpu_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..011f70a43397 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -529,7 +529,7 @@ void drm_sched_fence_finished(struct drm_sched_fence *fence); unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched); void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, unsigned long remaining); -struct drm_gpu_scheduler * +struct drm_gpu_scheduler ** drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, unsigned int num_sched_list);
Re: [PATCH 1/4] drm/sched: returns struct drm_gpu_scheduler ** for drm_sched_pick_best
Hi Andrey Basically this entire patch set are derived from patch [3/4]: entity->sched_list = num_sched_list > 1 ? sched_list : NULL; I think no special reason to treat single and multiple schedule list here. Best Regards! James On 2022-09-08 10:08 a.m., Andrey Grodzovsky wrote: What's the reason for this entire patch set ? Andrey On 2022-09-07 16:57, James Zhu wrote: drm_sched_pick_best returns struct drm_gpu_scheduler ** instead of struct drm_gpu_scheduler * Signed-off-by: James Zhu --- include/drm/gpu_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..011f70a43397 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -529,7 +529,7 @@ void drm_sched_fence_finished(struct drm_sched_fence *fence); unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched); void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, unsigned long remaining); -struct drm_gpu_scheduler * +struct drm_gpu_scheduler ** drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, unsigned int num_sched_list);
Re: [PATCH 1/4] drm/sched: returns struct drm_gpu_scheduler ** for drm_sched_pick_best
What's the reason for this entire patch set ? Andrey On 2022-09-07 16:57, James Zhu wrote: drm_sched_pick_best returns struct drm_gpu_scheduler ** instead of struct drm_gpu_scheduler * Signed-off-by: James Zhu --- include/drm/gpu_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..011f70a43397 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -529,7 +529,7 @@ void drm_sched_fence_finished(struct drm_sched_fence *fence); unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched); void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, unsigned long remaining); -struct drm_gpu_scheduler * +struct drm_gpu_scheduler ** drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, unsigned int num_sched_list);
Re: [PATCH 1/4] drm/sched: returns struct drm_gpu_scheduler ** for drm_sched_pick_best
Hui? That's certainly not correct. Christian. Am 07.09.22 um 22:57 schrieb James Zhu: drm_sched_pick_best returns struct drm_gpu_scheduler ** instead of struct drm_gpu_scheduler * Signed-off-by: James Zhu --- include/drm/gpu_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..011f70a43397 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -529,7 +529,7 @@ void drm_sched_fence_finished(struct drm_sched_fence *fence); unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched); void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, unsigned long remaining); -struct drm_gpu_scheduler * +struct drm_gpu_scheduler ** drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, unsigned int num_sched_list);
[PATCH 1/4] drm/sched: returns struct drm_gpu_scheduler ** for drm_sched_pick_best
drm_sched_pick_best returns struct drm_gpu_scheduler ** instead of struct drm_gpu_scheduler * Signed-off-by: James Zhu --- include/drm/gpu_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..011f70a43397 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -529,7 +529,7 @@ void drm_sched_fence_finished(struct drm_sched_fence *fence); unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched); void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, unsigned long remaining); -struct drm_gpu_scheduler * +struct drm_gpu_scheduler ** drm_sched_pick_best(struct drm_gpu_scheduler **sched_list, unsigned int num_sched_list); -- 2.25.1
