Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 7 Sep 2022 12:18:37 -0400 | Subject | Re: [PATCH v2 1/4] drm/sched: Enable signaling for finished fence | From | Andrey Grodzovsky <> |
| |
On 2022-09-07 02:37, Christian König wrote: > Am 06.09.22 um 21:55 schrieb Andrey Grodzovsky: >> >> On 2022-09-06 02:34, Christian König wrote: >>> Am 05.09.22 um 18:34 schrieb Arvind Yadav: >>>> Here's enabling software signaling for finished fence. >>>> >>>> Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com> >>>> --- >>>> >>>> Changes in v1 : >>>> 1- Addressing Christian's comment to remove CONFIG_DEBUG_FS check from >>>> this patch. >>>> 2- The version of this patch is also changed and previously >>>> it was [PATCH 2/4] >>>> >>>> --- >>>> drivers/gpu/drm/scheduler/sched_main.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c >>>> b/drivers/gpu/drm/scheduler/sched_main.c >>>> index e0ab14e0fb6b..fe72de0e2911 100644 >>>> --- a/drivers/gpu/drm/scheduler/sched_main.c >>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c >>>> @@ -962,6 +962,8 @@ static int drm_sched_main(void *param) >>>> /* Drop for original kref_init of the fence */ >>>> dma_fence_put(fence); >>>> + dma_fence_enable_sw_signaling(&s_fence->finished); >>> >>> Ok, this makes it a lot clearer. Previously I though that we have >>> some bug in dma_fence_add_callback(). >>> >>> This is essentially the wrong place to call this, the finished fence >>> should be enabled by the caller and not here. >>> >>> There is also another problem in dma_fence_enable_sw_signaling(), it >>> returns early when the fence is already signaled: >>> >>> if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) >>> return; >>> >>> Please remove that one first. >> >> >> Why we even need this explicit call if dma_fence_add_callback calls >> __dma_fence_enable_signaling anyway ? > > Two different fence objects. > > The dma_fence_add_callback() is done on the hw fence we get in return > of submitting the job. > > The dma_fence_enable_sw_signaling() here is done on the finished fence > we use to signal the completion externally. > > Key point is the finished fence should be used by the frontend drivers > which uses the scheduler and not by the scheduler itself. > > Christian.
Oh, so we need to explicitly call this because dma_fence_add_callback is not always used for finished fence right ?
Yea, then it makes sense that the client needs to manage this since each one has his own logic what to do with it.
Andrey
> >> >> Andrey >> >> >>> >>> Thanks, >>> Christian. >>> >>> >>>> + >>>> r = dma_fence_add_callback(fence, &sched_job->cb, >>>> drm_sched_job_done_cb); >>>> if (r == -ENOENT) >>> >
| ![\](/images/icornerr.gif) |