On 30/05/2022 14:40, Christian König wrote:
Am 30.05.22 um 12:09 schrieb Lionel Landwerlin:
On 30/05/2022 12:52, Christian König wrote:
Am 25.05.22 um 23:59 schrieb Lucas De Marchi:
On Wed, May 25, 2022 at 12:38:51PM +0200, Christian König wrote:
Am 25.05.22 um 11:35 schrieb Lionel Landwerlin:
[SNIP]
Err... Let's double check with my colleagues.
It seems we're running into a test failure in IGT with this
patch, but now I have doubts that it's where the problem lies.
Yeah, exactly that's what I couldn't understand as well.
What you describe above should still work fine.
Thanks for taking a look into this,
Christian.
With some additional prints:
[ 210.742634] Console: switching to colour dummy device 80x25
[ 210.742686] [IGT] syncobj_timeline: executing
[ 210.756988] [IGT] syncobj_timeline: starting subtest
transfer-timeline-point
[ 210.757364] [drm:drm_syncobj_transfer_ioctl] *ERROR* adding
fence0 signaled=1
[ 210.764543] [drm:drm_syncobj_transfer_ioctl] *ERROR* resulting
array fence signaled=0
[ 210.800469] [IGT] syncobj_timeline: exiting, ret=98
[ 210.825426] Console: switching to colour frame buffer device 240x67
still learning this part of the code but AFAICS the problem is because
when we are creating the array, the 'signaled' doesn't propagate to
the
array.
Yeah, but that is intentionally. The array should only signal when
requested.
I still don't get what the test case here is checking.
There must be something I don't know about fence arrays.
You seem to say that creating an array of signaled fences will not
make the array signaled.
Exactly that, yes. The array delays it's signaling until somebody asks
for it.
In other words the fences inside the array are check only after
someone calls dma_fence_enable_sw_signaling() which in turn calls
dma_fence_array_enable_signaling().
It is certainly possible that nobody does that in the drm_syncobj and
because of this the array never signals.
Regards,
Christian.
Thanks,
Yeah I guess dma_fence_enable_sw_signaling() is never called for sw_sync.
Don't we also want to call it right at the end of
drm_syncobj_flatten_chain() ?
-Lionel
This is the situation with this IGT test.
We started with a syncobj with point 1 & 2 signaled.
We take point 2 and import it as a new point 3 on the same syncobj.
We expect point 3 to be signaled as well and it's not.
Thanks,
-Lionel
Regards,
Christian.
dma_fence_array_create() {
...
atomic_set(&array->num_pending, signal_on_any ? 1 : num_fences);
...
}
This is not considering the fact that some of the fences could already
have been signaled as is the case in the
igt@syncobj_timeline@transfer-timeline-point
test. See
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11693/shard-dg1-12/igt@syncobj_timeline@xxxxxxxxxxxxxxxxxxxxxxxxxxxx
Quick patch on this function fixes it for me:
---------8<----------------
Subject: [PATCH] dma-buf: Honor already signaled fences on array
creation
When creating an array, array->num_pending is marked with the
number of
fences. However the fences could alredy have been signaled. Propagate
num_pending to the array by looking at each individual fence the array
contains.
Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
---
drivers/dma-buf/dma-fence-array.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-fence-array.c
b/drivers/dma-buf/dma-fence-array.c
index 5c8a7084577b..32f491c32fa0 100644
--- a/drivers/dma-buf/dma-fence-array.c
+++ b/drivers/dma-buf/dma-fence-array.c
@@ -158,6 +158,8 @@ struct dma_fence_array
*dma_fence_array_create(int num_fences,
{
struct dma_fence_array *array;
size_t size = sizeof(*array);
+ unsigned num_pending = 0;
+ struct dma_fence **f;
WARN_ON(!num_fences || !fences);
@@ -173,7 +175,14 @@ struct dma_fence_array
*dma_fence_array_create(int num_fences,
init_irq_work(&array->work, irq_dma_fence_array_work);
array->num_fences = num_fences;
- atomic_set(&array->num_pending, signal_on_any ? 1 : num_fences);
+
+ for (f = fences; f < fences + num_fences; f++)
+ num_pending += !dma_fence_is_signaled(*f);
+
+ if (signal_on_any)
+ num_pending = !!num_pending;
+
+ atomic_set(&array->num_pending, num_pending);
array->fences = fences;
array->base.error = PENDING_ERROR;
