[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f905c70e-b93d-5a26-cba9-c826a1fc21fe@amd.com>
Date: Wed, 7 Sep 2022 12:18:37 -0400
From: Andrey Grodzovsky <andrey.grodzovsky@....com>
To: Christian König <christian.koenig@....com>,
Arvind Yadav <Arvind.Yadav@....com>, shashank.sharma@....com,
amaranath.somalapuram@....com, Arunpravin.PaneerSelvam@....com,
sumit.semwal@...aro.org, gustavo@...ovan.org, airlied@...ux.ie,
daniel@...ll.ch, linux-media@...r.kernel.org,
dri-devel@...ts.freedesktop.org, linaro-mm-sig@...ts.linaro.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/4] drm/sched: Enable signaling for finished fence
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@....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)
>>>
>
Powered by blists - more mailing lists