Re: [Intel-gfx] [PATCH] drm/syncobj: flatten dma_fence_chains on transfer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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;







[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux