diff mbox series

[v2] drm/syncobj: Fix sync syncobj issue

Message ID 20220707102953.769684-1-jesse.zhang@amd.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/syncobj: Fix sync syncobj issue | expand

Commit Message

Zhang, Jesse(Jie) July 7, 2022, 10:29 a.m. UTC
enable signaling after flatten dma_fence_chains on transfer

Signed-off-by: jie1zhan <jesse.zhang@amd.com>
---
 drivers/gpu/drm/drm_syncobj.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Christian König July 12, 2022, 10:26 a.m. UTC | #1
Ping to the Intel guys here. Especially Lucas/Nirmoy/Lionel.

IIRC you stumbled over that problem as well, have you found any solution?

Regards,
Christian.

Am 07.07.22 um 12:29 schrieb jie1zhan:
> enable signaling after flatten dma_fence_chains on transfer
>
> Signed-off-by: jie1zhan <jesse.zhang@amd.com>
> ---
>   drivers/gpu/drm/drm_syncobj.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 7e48dcd1bee4..0d9d3577325f 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -920,6 +920,7 @@ static int drm_syncobj_transfer_to_timeline(struct drm_file *file_private,
>   	if (ret)
>   		goto err_free_fence;
>   
> +	dma_fence_enable_sw_signaling(fence);
>   	chain = dma_fence_chain_alloc();
>   	if (!chain) {
>   		ret = -ENOMEM;
Lionel Landwerlin July 12, 2022, 2:22 p.m. UTC | #2
I'll let Lucas comment. I've only looked a little at it.
 From what I remember just enabling sw_signaling was enough to fix the 
issue.

-Lionel

On 12/07/2022 13:26, Christian König wrote:
> Ping to the Intel guys here. Especially Lucas/Nirmoy/Lionel.
>
> IIRC you stumbled over that problem as well, have you found any solution?
>
> Regards,
> Christian.
>
> Am 07.07.22 um 12:29 schrieb jie1zhan:
>> enable signaling after flatten dma_fence_chains on transfer
>>
>> Signed-off-by: jie1zhan <jesse.zhang@amd.com>
>> ---
>>   drivers/gpu/drm/drm_syncobj.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
>> b/drivers/gpu/drm/drm_syncobj.c
>> index 7e48dcd1bee4..0d9d3577325f 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -920,6 +920,7 @@ static int 
>> drm_syncobj_transfer_to_timeline(struct drm_file *file_private,
>>       if (ret)
>>           goto err_free_fence;
>>   +    dma_fence_enable_sw_signaling(fence);
>>       chain = dma_fence_chain_alloc();
>>       if (!chain) {
>>           ret = -ENOMEM;
>
Christian König July 12, 2022, 2:57 p.m. UTC | #3
Yeah, adding dma_fence_enable_sw_signaling() is the right thing to do.

The question is where to add that? Usually right before the fence is 
returned from the object or queried from userspace would probably be the 
right place.

Regards,
Christian.

Am 12.07.22 um 16:22 schrieb Lionel Landwerlin:
> I'll let Lucas comment. I've only looked a little at it.
> From what I remember just enabling sw_signaling was enough to fix the 
> issue.
>
> -Lionel
>
> On 12/07/2022 13:26, Christian König wrote:
>> Ping to the Intel guys here. Especially Lucas/Nirmoy/Lionel.
>>
>> IIRC you stumbled over that problem as well, have you found any 
>> solution?
>>
>> Regards,
>> Christian.
>>
>> Am 07.07.22 um 12:29 schrieb jie1zhan:
>>> enable signaling after flatten dma_fence_chains on transfer
>>>
>>> Signed-off-by: jie1zhan <jesse.zhang@amd.com>
>>> ---
>>>   drivers/gpu/drm/drm_syncobj.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
>>> b/drivers/gpu/drm/drm_syncobj.c
>>> index 7e48dcd1bee4..0d9d3577325f 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -920,6 +920,7 @@ static int 
>>> drm_syncobj_transfer_to_timeline(struct drm_file *file_private,
>>>       if (ret)
>>>           goto err_free_fence;
>>>   +    dma_fence_enable_sw_signaling(fence);
>>>       chain = dma_fence_chain_alloc();
>>>       if (!chain) {
>>>           ret = -ENOMEM;
>>
>
Nirmoy Das July 13, 2022, 8:42 a.m. UTC | #4
Hi Christian,

On 7/12/2022 12:26 PM, Christian König wrote:
> Ping to the Intel guys here. Especially Lucas/Nirmoy/Lionel.
>
> IIRC you stumbled over that problem as well, have you found any solution?

I might be wrong but  I think you are talking about 
igt@syncobj_timeline@transfer-timeline-point testcase which seems to be

green in CI now 
https://intel-gfx-ci.01.org/tree/drm-tip/igt@syncobj_timeline@transfer-timeline-point.html

Lucas found out that the issues got fixed after ec8d985ff26f ("drm: use 
dma_fence_unwrap_merge() in drm_syncobj")


Regards,

Nirmoy

>
> Regards,
> Christian.
>
> Am 07.07.22 um 12:29 schrieb jie1zhan:
>> enable signaling after flatten dma_fence_chains on transfer
>>
>> Signed-off-by: jie1zhan <jesse.zhang@amd.com>
>> ---
>>   drivers/gpu/drm/drm_syncobj.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
>> b/drivers/gpu/drm/drm_syncobj.c
>> index 7e48dcd1bee4..0d9d3577325f 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -920,6 +920,7 @@ static int 
>> drm_syncobj_transfer_to_timeline(struct drm_file *file_private,
>>       if (ret)
>>           goto err_free_fence;
>>   +    dma_fence_enable_sw_signaling(fence);
>>       chain = dma_fence_chain_alloc();
>>       if (!chain) {
>>           ret = -ENOMEM;
>
Christian König July 13, 2022, 9:15 a.m. UTC | #5
Am 13.07.22 um 10:42 schrieb Das, Nirmoy:
> Hi Christian,
>
> On 7/12/2022 12:26 PM, Christian König wrote:
>> Ping to the Intel guys here. Especially Lucas/Nirmoy/Lionel.
>>
>> IIRC you stumbled over that problem as well, have you found any 
>> solution?
>
> I might be wrong but  I think you are talking about 
> igt@syncobj_timeline@transfer-timeline-point testcase which seems to be
>
> green in CI now 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fintel-gfx-ci.01.org%2Ftree%2Fdrm-tip%2Figt%40syncobj_timeline%40transfer-timeline-point.html&amp;data=05%7C01%7Cchristian.koenig%40amd.com%7C722fc33842734ef5c05108da64abac60%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637932985747614383%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=q8JygSVZfGA8cEZn%2BdxVXX79pkpXRjZTS8kBQ6Lq%2Bmw%3D&amp;reserved=0
>
> Lucas found out that the issues got fixed after ec8d985ff26f ("drm: 
> use dma_fence_unwrap_merge() in drm_syncobj")

Yeah, but that's just coincident. The original bug that we fail to 
enable signaling in the syncobj is still there.

No that this is any major problem, but it would still be nice to have 
that fixed.

Regards,
Christian.

>
>
> Regards,
>
> Nirmoy
>
>>
>> Regards,
>> Christian.
>>
>> Am 07.07.22 um 12:29 schrieb jie1zhan:
>>> enable signaling after flatten dma_fence_chains on transfer
>>>
>>> Signed-off-by: jie1zhan <jesse.zhang@amd.com>
>>> ---
>>>   drivers/gpu/drm/drm_syncobj.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
>>> b/drivers/gpu/drm/drm_syncobj.c
>>> index 7e48dcd1bee4..0d9d3577325f 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -920,6 +920,7 @@ static int 
>>> drm_syncobj_transfer_to_timeline(struct drm_file *file_private,
>>>       if (ret)
>>>           goto err_free_fence;
>>>   +    dma_fence_enable_sw_signaling(fence);
>>>       chain = dma_fence_chain_alloc();
>>>       if (!chain) {
>>>           ret = -ENOMEM;
>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 7e48dcd1bee4..0d9d3577325f 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -920,6 +920,7 @@  static int drm_syncobj_transfer_to_timeline(struct drm_file *file_private,
 	if (ret)
 		goto err_free_fence;
 
+	dma_fence_enable_sw_signaling(fence);
 	chain = dma_fence_chain_alloc();
 	if (!chain) {
 		ret = -ENOMEM;