* [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" @ 2020-06-25 12:34 ` Lionel Landwerlin 0 siblings, 0 replies; 41+ messages in thread From: Lionel Landwerlin @ 2020-06-25 12:34 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx, christian.koenig, chris This reverts commit 5de376bb434f80a13138f0ebedc8351ab73d8b0d. This change breaks synchronization of a timeline. dma_fence_chain_find_seqno() might be a bit of a confusing name but this function is not trying to find a particular seqno, is supposed to give a fence to wait on for a particular point in the timeline. In a timeline, a particular value is reached when all the points up to and including that value have signaled. Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/dma-buf/dma-fence-chain.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c index c435bbba851c..3d123502ff12 100644 --- a/drivers/dma-buf/dma-fence-chain.c +++ b/drivers/dma-buf/dma-fence-chain.c @@ -99,12 +99,6 @@ int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno) return -EINVAL; dma_fence_chain_for_each(*pfence, &chain->base) { - if ((*pfence)->seqno < seqno) { /* already signaled */ - dma_fence_put(*pfence); - *pfence = NULL; - break; - } - if ((*pfence)->context != chain->base.context || to_dma_fence_chain(*pfence)->prev_seqno < seqno) break; @@ -228,7 +222,6 @@ EXPORT_SYMBOL(dma_fence_chain_ops); * @chain: the chain node to initialize * @prev: the previous fence * @fence: the current fence - * @seqno: the sequence number (syncpt) of the fence within the chain * * Initialize a new chain node and either start a new chain or add the node to * the existing chain of the previous fence. -- 2.27.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" @ 2020-06-25 12:34 ` Lionel Landwerlin 0 siblings, 0 replies; 41+ messages in thread From: Lionel Landwerlin @ 2020-06-25 12:34 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx, christian.koenig, chris This reverts commit 5de376bb434f80a13138f0ebedc8351ab73d8b0d. This change breaks synchronization of a timeline. dma_fence_chain_find_seqno() might be a bit of a confusing name but this function is not trying to find a particular seqno, is supposed to give a fence to wait on for a particular point in the timeline. In a timeline, a particular value is reached when all the points up to and including that value have signaled. Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/dma-buf/dma-fence-chain.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c index c435bbba851c..3d123502ff12 100644 --- a/drivers/dma-buf/dma-fence-chain.c +++ b/drivers/dma-buf/dma-fence-chain.c @@ -99,12 +99,6 @@ int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno) return -EINVAL; dma_fence_chain_for_each(*pfence, &chain->base) { - if ((*pfence)->seqno < seqno) { /* already signaled */ - dma_fence_put(*pfence); - *pfence = NULL; - break; - } - if ((*pfence)->context != chain->base.context || to_dma_fence_chain(*pfence)->prev_seqno < seqno) break; @@ -228,7 +222,6 @@ EXPORT_SYMBOL(dma_fence_chain_ops); * @chain: the chain node to initialize * @prev: the previous fence * @fence: the current fence - * @seqno: the sequence number (syncpt) of the fence within the chain * * Initialize a new chain node and either start a new chain or add the node to * the existing chain of the previous fence. -- 2.27.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test 2020-06-25 12:34 ` [Intel-gfx] " Lionel Landwerlin @ 2020-06-25 12:34 ` Lionel Landwerlin -1 siblings, 0 replies; 41+ messages in thread From: Lionel Landwerlin @ 2020-06-25 12:34 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx, christian.koenig, chris There was probably a misunderstand on how the dma-fence-chain is supposed to work or what dma_fence_chain_find_seqno() is supposed to return. dma_fence_chain_find_seqno() is here to give us the fence to wait upon for a particular point in the timeline. The timeline progresses only when all the points prior to a given number have completed. Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Fixes: dc2f7e67a28a5c ("dma-buf: Exercise dma-fence-chain under selftests") --- drivers/dma-buf/st-dma-fence-chain.c | 43 ++++++++++++++-------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/drivers/dma-buf/st-dma-fence-chain.c b/drivers/dma-buf/st-dma-fence-chain.c index 5d45ba7ba3cd..9525f7f56119 100644 --- a/drivers/dma-buf/st-dma-fence-chain.c +++ b/drivers/dma-buf/st-dma-fence-chain.c @@ -318,15 +318,16 @@ static int find_out_of_order(void *arg) goto err; } - if (fence && fence != fc.chains[1]) { + /* + * We signaled the middle fence (2) of the 1-2-3 chain. The behavior + * of the dma-fence-chain is to make us wait for all the fences up to + * the point we want. Since fence 1 is still not signaled, this what + * we should get as fence to wait upon (fence 2 being garbage + * collected during the traversal of the chain). + */ + if (fence != fc.chains[0]) { pr_err("Incorrect chain-fence.seqno:%lld reported for completed seqno:2\n", - fence->seqno); - - dma_fence_get(fence); - err = dma_fence_chain_find_seqno(&fence, 2); - dma_fence_put(fence); - if (err) - pr_err("Reported %d for finding self!\n", err); + fence ? fence->seqno : 0); err = -EINVAL; } @@ -415,20 +416,18 @@ static int __find_race(void *arg) if (!fence) goto signal; - err = dma_fence_chain_find_seqno(&fence, seqno); - if (err) { - pr_err("Reported an invalid fence for find-self:%d\n", - seqno); - dma_fence_put(fence); - break; - } - - if (fence->seqno < seqno) { - pr_err("Reported an earlier fence.seqno:%lld for seqno:%d\n", - fence->seqno, seqno); - err = -EINVAL; - dma_fence_put(fence); - break; + /* + * We can only find ourselves if we are on fence we were + * looking for. + */ + if (fence->seqno == seqno) { + err = dma_fence_chain_find_seqno(&fence, seqno); + if (err) { + pr_err("Reported an invalid fence for find-self:%d\n", + seqno); + dma_fence_put(fence); + break; + } } dma_fence_put(fence); -- 2.27.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Intel-gfx] [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test @ 2020-06-25 12:34 ` Lionel Landwerlin 0 siblings, 0 replies; 41+ messages in thread From: Lionel Landwerlin @ 2020-06-25 12:34 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx, christian.koenig, chris There was probably a misunderstand on how the dma-fence-chain is supposed to work or what dma_fence_chain_find_seqno() is supposed to return. dma_fence_chain_find_seqno() is here to give us the fence to wait upon for a particular point in the timeline. The timeline progresses only when all the points prior to a given number have completed. Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Fixes: dc2f7e67a28a5c ("dma-buf: Exercise dma-fence-chain under selftests") --- drivers/dma-buf/st-dma-fence-chain.c | 43 ++++++++++++++-------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/drivers/dma-buf/st-dma-fence-chain.c b/drivers/dma-buf/st-dma-fence-chain.c index 5d45ba7ba3cd..9525f7f56119 100644 --- a/drivers/dma-buf/st-dma-fence-chain.c +++ b/drivers/dma-buf/st-dma-fence-chain.c @@ -318,15 +318,16 @@ static int find_out_of_order(void *arg) goto err; } - if (fence && fence != fc.chains[1]) { + /* + * We signaled the middle fence (2) of the 1-2-3 chain. The behavior + * of the dma-fence-chain is to make us wait for all the fences up to + * the point we want. Since fence 1 is still not signaled, this what + * we should get as fence to wait upon (fence 2 being garbage + * collected during the traversal of the chain). + */ + if (fence != fc.chains[0]) { pr_err("Incorrect chain-fence.seqno:%lld reported for completed seqno:2\n", - fence->seqno); - - dma_fence_get(fence); - err = dma_fence_chain_find_seqno(&fence, 2); - dma_fence_put(fence); - if (err) - pr_err("Reported %d for finding self!\n", err); + fence ? fence->seqno : 0); err = -EINVAL; } @@ -415,20 +416,18 @@ static int __find_race(void *arg) if (!fence) goto signal; - err = dma_fence_chain_find_seqno(&fence, seqno); - if (err) { - pr_err("Reported an invalid fence for find-self:%d\n", - seqno); - dma_fence_put(fence); - break; - } - - if (fence->seqno < seqno) { - pr_err("Reported an earlier fence.seqno:%lld for seqno:%d\n", - fence->seqno, seqno); - err = -EINVAL; - dma_fence_put(fence); - break; + /* + * We can only find ourselves if we are on fence we were + * looking for. + */ + if (fence->seqno == seqno) { + err = dma_fence_chain_find_seqno(&fence, seqno); + if (err) { + pr_err("Reported an invalid fence for find-self:%d\n", + seqno); + dma_fence_put(fence); + break; + } } dma_fence_put(fence); -- 2.27.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test 2020-06-25 12:34 ` [Intel-gfx] " Lionel Landwerlin @ 2020-06-25 12:44 ` Christian König -1 siblings, 0 replies; 41+ messages in thread From: Christian König @ 2020-06-25 12:44 UTC (permalink / raw) To: Lionel Landwerlin, dri-devel; +Cc: intel-gfx, chris Am 25.06.20 um 14:34 schrieb Lionel Landwerlin: > There was probably a misunderstand on how the dma-fence-chain is > supposed to work or what dma_fence_chain_find_seqno() is supposed to > return. > > dma_fence_chain_find_seqno() is here to give us the fence to wait upon > for a particular point in the timeline. The timeline progresses only > when all the points prior to a given number have completed. > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Fixes: dc2f7e67a28a5c ("dma-buf: Exercise dma-fence-chain under selftests") Acked-by: Christian König <christian.koenig@amd.com> > --- > drivers/dma-buf/st-dma-fence-chain.c | 43 ++++++++++++++-------------- > 1 file changed, 21 insertions(+), 22 deletions(-) > > diff --git a/drivers/dma-buf/st-dma-fence-chain.c b/drivers/dma-buf/st-dma-fence-chain.c > index 5d45ba7ba3cd..9525f7f56119 100644 > --- a/drivers/dma-buf/st-dma-fence-chain.c > +++ b/drivers/dma-buf/st-dma-fence-chain.c > @@ -318,15 +318,16 @@ static int find_out_of_order(void *arg) > goto err; > } > > - if (fence && fence != fc.chains[1]) { > + /* > + * We signaled the middle fence (2) of the 1-2-3 chain. The behavior > + * of the dma-fence-chain is to make us wait for all the fences up to > + * the point we want. Since fence 1 is still not signaled, this what > + * we should get as fence to wait upon (fence 2 being garbage > + * collected during the traversal of the chain). > + */ > + if (fence != fc.chains[0]) { > pr_err("Incorrect chain-fence.seqno:%lld reported for completed seqno:2\n", > - fence->seqno); > - > - dma_fence_get(fence); > - err = dma_fence_chain_find_seqno(&fence, 2); > - dma_fence_put(fence); > - if (err) > - pr_err("Reported %d for finding self!\n", err); > + fence ? fence->seqno : 0); > > err = -EINVAL; > } > @@ -415,20 +416,18 @@ static int __find_race(void *arg) > if (!fence) > goto signal; > > - err = dma_fence_chain_find_seqno(&fence, seqno); > - if (err) { > - pr_err("Reported an invalid fence for find-self:%d\n", > - seqno); > - dma_fence_put(fence); > - break; > - } > - > - if (fence->seqno < seqno) { > - pr_err("Reported an earlier fence.seqno:%lld for seqno:%d\n", > - fence->seqno, seqno); > - err = -EINVAL; > - dma_fence_put(fence); > - break; > + /* > + * We can only find ourselves if we are on fence we were > + * looking for. > + */ > + if (fence->seqno == seqno) { > + err = dma_fence_chain_find_seqno(&fence, seqno); > + if (err) { > + pr_err("Reported an invalid fence for find-self:%d\n", > + seqno); > + dma_fence_put(fence); > + break; > + } > } > > dma_fence_put(fence); _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test @ 2020-06-25 12:44 ` Christian König 0 siblings, 0 replies; 41+ messages in thread From: Christian König @ 2020-06-25 12:44 UTC (permalink / raw) To: Lionel Landwerlin, dri-devel; +Cc: intel-gfx, chris Am 25.06.20 um 14:34 schrieb Lionel Landwerlin: > There was probably a misunderstand on how the dma-fence-chain is > supposed to work or what dma_fence_chain_find_seqno() is supposed to > return. > > dma_fence_chain_find_seqno() is here to give us the fence to wait upon > for a particular point in the timeline. The timeline progresses only > when all the points prior to a given number have completed. > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Fixes: dc2f7e67a28a5c ("dma-buf: Exercise dma-fence-chain under selftests") Acked-by: Christian König <christian.koenig@amd.com> > --- > drivers/dma-buf/st-dma-fence-chain.c | 43 ++++++++++++++-------------- > 1 file changed, 21 insertions(+), 22 deletions(-) > > diff --git a/drivers/dma-buf/st-dma-fence-chain.c b/drivers/dma-buf/st-dma-fence-chain.c > index 5d45ba7ba3cd..9525f7f56119 100644 > --- a/drivers/dma-buf/st-dma-fence-chain.c > +++ b/drivers/dma-buf/st-dma-fence-chain.c > @@ -318,15 +318,16 @@ static int find_out_of_order(void *arg) > goto err; > } > > - if (fence && fence != fc.chains[1]) { > + /* > + * We signaled the middle fence (2) of the 1-2-3 chain. The behavior > + * of the dma-fence-chain is to make us wait for all the fences up to > + * the point we want. Since fence 1 is still not signaled, this what > + * we should get as fence to wait upon (fence 2 being garbage > + * collected during the traversal of the chain). > + */ > + if (fence != fc.chains[0]) { > pr_err("Incorrect chain-fence.seqno:%lld reported for completed seqno:2\n", > - fence->seqno); > - > - dma_fence_get(fence); > - err = dma_fence_chain_find_seqno(&fence, 2); > - dma_fence_put(fence); > - if (err) > - pr_err("Reported %d for finding self!\n", err); > + fence ? fence->seqno : 0); > > err = -EINVAL; > } > @@ -415,20 +416,18 @@ static int __find_race(void *arg) > if (!fence) > goto signal; > > - err = dma_fence_chain_find_seqno(&fence, seqno); > - if (err) { > - pr_err("Reported an invalid fence for find-self:%d\n", > - seqno); > - dma_fence_put(fence); > - break; > - } > - > - if (fence->seqno < seqno) { > - pr_err("Reported an earlier fence.seqno:%lld for seqno:%d\n", > - fence->seqno, seqno); > - err = -EINVAL; > - dma_fence_put(fence); > - break; > + /* > + * We can only find ourselves if we are on fence we were > + * looking for. > + */ > + if (fence->seqno == seqno) { > + err = dma_fence_chain_find_seqno(&fence, seqno); > + if (err) { > + pr_err("Reported an invalid fence for find-self:%d\n", > + seqno); > + dma_fence_put(fence); > + break; > + } > } > > dma_fence_put(fence); _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test 2020-06-25 12:34 ` [Intel-gfx] " Lionel Landwerlin @ 2020-06-25 13:18 ` Chris Wilson -1 siblings, 0 replies; 41+ messages in thread From: Chris Wilson @ 2020-06-25 13:18 UTC (permalink / raw) To: Lionel Landwerlin, dri-devel; +Cc: intel-gfx, christian.koenig Quoting Lionel Landwerlin (2020-06-25 13:34:43) > There was probably a misunderstand on how the dma-fence-chain is > supposed to work or what dma_fence_chain_find_seqno() is supposed to > return. > > dma_fence_chain_find_seqno() is here to give us the fence to wait upon > for a particular point in the timeline. The timeline progresses only > when all the points prior to a given number have completed. Hmm, the question was what point is it supposed to wait for. For the simple chain of [1, 3], does 1 being signaled imply that all points up to 3 are signaled, or does 3 not being signaled imply that all points after 1 are not. If that's mentioned already somewhere, my bad. If not, could you put the answer somewhere. -Chris _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test @ 2020-06-25 13:18 ` Chris Wilson 0 siblings, 0 replies; 41+ messages in thread From: Chris Wilson @ 2020-06-25 13:18 UTC (permalink / raw) To: Lionel Landwerlin, dri-devel; +Cc: intel-gfx, christian.koenig Quoting Lionel Landwerlin (2020-06-25 13:34:43) > There was probably a misunderstand on how the dma-fence-chain is > supposed to work or what dma_fence_chain_find_seqno() is supposed to > return. > > dma_fence_chain_find_seqno() is here to give us the fence to wait upon > for a particular point in the timeline. The timeline progresses only > when all the points prior to a given number have completed. Hmm, the question was what point is it supposed to wait for. For the simple chain of [1, 3], does 1 being signaled imply that all points up to 3 are signaled, or does 3 not being signaled imply that all points after 1 are not. If that's mentioned already somewhere, my bad. If not, could you put the answer somewhere. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test 2020-06-25 13:18 ` Chris Wilson @ 2020-06-25 13:23 ` Lionel Landwerlin -1 siblings, 0 replies; 41+ messages in thread From: Lionel Landwerlin @ 2020-06-25 13:23 UTC (permalink / raw) To: Chris Wilson, dri-devel; +Cc: intel-gfx, christian.koenig On 25/06/2020 16:18, Chris Wilson wrote: > Quoting Lionel Landwerlin (2020-06-25 13:34:43) >> There was probably a misunderstand on how the dma-fence-chain is >> supposed to work or what dma_fence_chain_find_seqno() is supposed to >> return. >> >> dma_fence_chain_find_seqno() is here to give us the fence to wait upon >> for a particular point in the timeline. The timeline progresses only >> when all the points prior to a given number have completed. > Hmm, the question was what point is it supposed to wait for. > > For the simple chain of [1, 3], does 1 being signaled imply that all > points up to 3 are signaled, or does 3 not being signaled imply that all > points after 1 are not. If that's mentioned already somewhere, my bad. > If not, could you put the answer somewhere. > -Chris In [1, 3], if 1 is signaled, the timeline value is 1. And find_seqno(2) should return NULL. In the out_of_order selftest the chain was [1, 2, 3], 2 was signaled and the test was expecting no fence to be returned by find_seqno(2). But we still have to wait on 1 to complete before find_seqno(2) can return NULL (as in you don't have to wait on anything). Hope that answer the question. -Lionel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test @ 2020-06-25 13:23 ` Lionel Landwerlin 0 siblings, 0 replies; 41+ messages in thread From: Lionel Landwerlin @ 2020-06-25 13:23 UTC (permalink / raw) To: Chris Wilson, dri-devel; +Cc: intel-gfx, christian.koenig On 25/06/2020 16:18, Chris Wilson wrote: > Quoting Lionel Landwerlin (2020-06-25 13:34:43) >> There was probably a misunderstand on how the dma-fence-chain is >> supposed to work or what dma_fence_chain_find_seqno() is supposed to >> return. >> >> dma_fence_chain_find_seqno() is here to give us the fence to wait upon >> for a particular point in the timeline. The timeline progresses only >> when all the points prior to a given number have completed. > Hmm, the question was what point is it supposed to wait for. > > For the simple chain of [1, 3], does 1 being signaled imply that all > points up to 3 are signaled, or does 3 not being signaled imply that all > points after 1 are not. If that's mentioned already somewhere, my bad. > If not, could you put the answer somewhere. > -Chris In [1, 3], if 1 is signaled, the timeline value is 1. And find_seqno(2) should return NULL. In the out_of_order selftest the chain was [1, 2, 3], 2 was signaled and the test was expecting no fence to be returned by find_seqno(2). But we still have to wait on 1 to complete before find_seqno(2) can return NULL (as in you don't have to wait on anything). Hope that answer the question. -Lionel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test 2020-06-25 13:23 ` Lionel Landwerlin @ 2020-06-25 13:47 ` Chris Wilson -1 siblings, 0 replies; 41+ messages in thread From: Chris Wilson @ 2020-06-25 13:47 UTC (permalink / raw) To: Lionel Landwerlin, dri-devel; +Cc: intel-gfx, christian.koenig Quoting Lionel Landwerlin (2020-06-25 14:23:25) > On 25/06/2020 16:18, Chris Wilson wrote: > > Quoting Lionel Landwerlin (2020-06-25 13:34:43) > >> There was probably a misunderstand on how the dma-fence-chain is > >> supposed to work or what dma_fence_chain_find_seqno() is supposed to > >> return. > >> > >> dma_fence_chain_find_seqno() is here to give us the fence to wait upon > >> for a particular point in the timeline. The timeline progresses only > >> when all the points prior to a given number have completed. > > Hmm, the question was what point is it supposed to wait for. > > > > For the simple chain of [1, 3], does 1 being signaled imply that all > > points up to 3 are signaled, or does 3 not being signaled imply that all > > points after 1 are not. If that's mentioned already somewhere, my bad. > > If not, could you put the answer somewhere. > > -Chris > > In [1, 3], if 1 is signaled, the timeline value is 1. And find_seqno(2) > should return NULL. > > > In the out_of_order selftest the chain was [1, 2, 3], 2 was signaled and > the test was expecting no fence to be returned by find_seqno(2). > > But we still have to wait on 1 to complete before find_seqno(2) can > return NULL (as in you don't have to wait on anything). * scratches head I thought it was meant to be expecting fc.chain[1] to still be present as the chain at that point was not yet signaled. Oh well, a mistake compounded. :| -Chris _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test @ 2020-06-25 13:47 ` Chris Wilson 0 siblings, 0 replies; 41+ messages in thread From: Chris Wilson @ 2020-06-25 13:47 UTC (permalink / raw) To: Lionel Landwerlin, dri-devel; +Cc: intel-gfx, christian.koenig Quoting Lionel Landwerlin (2020-06-25 14:23:25) > On 25/06/2020 16:18, Chris Wilson wrote: > > Quoting Lionel Landwerlin (2020-06-25 13:34:43) > >> There was probably a misunderstand on how the dma-fence-chain is > >> supposed to work or what dma_fence_chain_find_seqno() is supposed to > >> return. > >> > >> dma_fence_chain_find_seqno() is here to give us the fence to wait upon > >> for a particular point in the timeline. The timeline progresses only > >> when all the points prior to a given number have completed. > > Hmm, the question was what point is it supposed to wait for. > > > > For the simple chain of [1, 3], does 1 being signaled imply that all > > points up to 3 are signaled, or does 3 not being signaled imply that all > > points after 1 are not. If that's mentioned already somewhere, my bad. > > If not, could you put the answer somewhere. > > -Chris > > In [1, 3], if 1 is signaled, the timeline value is 1. And find_seqno(2) > should return NULL. > > > In the out_of_order selftest the chain was [1, 2, 3], 2 was signaled and > the test was expecting no fence to be returned by find_seqno(2). > > But we still have to wait on 1 to complete before find_seqno(2) can > return NULL (as in you don't have to wait on anything). * scratches head I thought it was meant to be expecting fc.chain[1] to still be present as the chain at that point was not yet signaled. Oh well, a mistake compounded. :| -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test 2020-06-25 13:47 ` Chris Wilson @ 2020-06-25 13:56 ` Lionel Landwerlin -1 siblings, 0 replies; 41+ messages in thread From: Lionel Landwerlin @ 2020-06-25 13:56 UTC (permalink / raw) To: Chris Wilson, dri-devel; +Cc: intel-gfx, christian.koenig On 25/06/2020 16:47, Chris Wilson wrote: > Quoting Lionel Landwerlin (2020-06-25 14:23:25) >> On 25/06/2020 16:18, Chris Wilson wrote: >>> Quoting Lionel Landwerlin (2020-06-25 13:34:43) >>>> There was probably a misunderstand on how the dma-fence-chain is >>>> supposed to work or what dma_fence_chain_find_seqno() is supposed to >>>> return. >>>> >>>> dma_fence_chain_find_seqno() is here to give us the fence to wait upon >>>> for a particular point in the timeline. The timeline progresses only >>>> when all the points prior to a given number have completed. >>> Hmm, the question was what point is it supposed to wait for. >>> >>> For the simple chain of [1, 3], does 1 being signaled imply that all >>> points up to 3 are signaled, or does 3 not being signaled imply that all >>> points after 1 are not. If that's mentioned already somewhere, my bad. >>> If not, could you put the answer somewhere. >>> -Chris >> In [1, 3], if 1 is signaled, the timeline value is 1. And find_seqno(2) >> should return NULL. >> >> >> In the out_of_order selftest the chain was [1, 2, 3], 2 was signaled and >> the test was expecting no fence to be returned by find_seqno(2). >> >> But we still have to wait on 1 to complete before find_seqno(2) can >> return NULL (as in you don't have to wait on anything). > * scratches head > > I thought it was meant to be expecting fc.chain[1] to still be present > as the chain at that point was not yet signaled. You're right that the point is not yet signaled. But it doesn't need to stay in the chain if you can wait on a previous point. chain[1] gets removed as we walk the chain backward in dma_fence_chain_walk. -Lionel > > Oh well, a mistake compounded. :| > -Chris _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test @ 2020-06-25 13:56 ` Lionel Landwerlin 0 siblings, 0 replies; 41+ messages in thread From: Lionel Landwerlin @ 2020-06-25 13:56 UTC (permalink / raw) To: Chris Wilson, dri-devel; +Cc: intel-gfx, christian.koenig On 25/06/2020 16:47, Chris Wilson wrote: > Quoting Lionel Landwerlin (2020-06-25 14:23:25) >> On 25/06/2020 16:18, Chris Wilson wrote: >>> Quoting Lionel Landwerlin (2020-06-25 13:34:43) >>>> There was probably a misunderstand on how the dma-fence-chain is >>>> supposed to work or what dma_fence_chain_find_seqno() is supposed to >>>> return. >>>> >>>> dma_fence_chain_find_seqno() is here to give us the fence to wait upon >>>> for a particular point in the timeline. The timeline progresses only >>>> when all the points prior to a given number have completed. >>> Hmm, the question was what point is it supposed to wait for. >>> >>> For the simple chain of [1, 3], does 1 being signaled imply that all >>> points up to 3 are signaled, or does 3 not being signaled imply that all >>> points after 1 are not. If that's mentioned already somewhere, my bad. >>> If not, could you put the answer somewhere. >>> -Chris >> In [1, 3], if 1 is signaled, the timeline value is 1. And find_seqno(2) >> should return NULL. >> >> >> In the out_of_order selftest the chain was [1, 2, 3], 2 was signaled and >> the test was expecting no fence to be returned by find_seqno(2). >> >> But we still have to wait on 1 to complete before find_seqno(2) can >> return NULL (as in you don't have to wait on anything). > * scratches head > > I thought it was meant to be expecting fc.chain[1] to still be present > as the chain at that point was not yet signaled. You're right that the point is not yet signaled. But it doesn't need to stay in the chain if you can wait on a previous point. chain[1] gets removed as we walk the chain backward in dma_fence_chain_walk. -Lionel > > Oh well, a mistake compounded. :| > -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test 2020-06-25 13:23 ` Lionel Landwerlin @ 2020-06-25 13:59 ` Daniel Vetter -1 siblings, 0 replies; 41+ messages in thread From: Daniel Vetter @ 2020-06-25 13:59 UTC (permalink / raw) To: Lionel Landwerlin Cc: intel-gfx, Christian König, dri-devel, Chris Wilson On Thu, Jun 25, 2020 at 3:23 PM Lionel Landwerlin <lionel.g.landwerlin@intel.com> wrote: > > On 25/06/2020 16:18, Chris Wilson wrote: > > Quoting Lionel Landwerlin (2020-06-25 13:34:43) > >> There was probably a misunderstand on how the dma-fence-chain is > >> supposed to work or what dma_fence_chain_find_seqno() is supposed to > >> return. > >> > >> dma_fence_chain_find_seqno() is here to give us the fence to wait upon > >> for a particular point in the timeline. The timeline progresses only > >> when all the points prior to a given number have completed. > > Hmm, the question was what point is it supposed to wait for. > > > > For the simple chain of [1, 3], does 1 being signaled imply that all > > points up to 3 are signaled, or does 3 not being signaled imply that all > > points after 1 are not. If that's mentioned already somewhere, my bad. > > If not, could you put the answer somewhere. > > -Chris > > In [1, 3], if 1 is signaled, the timeline value is 1. And find_seqno(2) > should return NULL. > > > In the out_of_order selftest the chain was [1, 2, 3], 2 was signaled and > the test was expecting no fence to be returned by find_seqno(2). > > But we still have to wait on 1 to complete before find_seqno(2) can > return NULL (as in you don't have to wait on anything). > > > Hope that answer the question. I asked Christian to document why timeline works like this, but I can't find it in the kerneldoc right now. If it's missing I think we should fix that and add the explanation, iirc it was around gpu reset creating too much havoc otherwise. -Daniel > > > -Lionel > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test @ 2020-06-25 13:59 ` Daniel Vetter 0 siblings, 0 replies; 41+ messages in thread From: Daniel Vetter @ 2020-06-25 13:59 UTC (permalink / raw) To: Lionel Landwerlin Cc: intel-gfx, Christian König, dri-devel, Chris Wilson On Thu, Jun 25, 2020 at 3:23 PM Lionel Landwerlin <lionel.g.landwerlin@intel.com> wrote: > > On 25/06/2020 16:18, Chris Wilson wrote: > > Quoting Lionel Landwerlin (2020-06-25 13:34:43) > >> There was probably a misunderstand on how the dma-fence-chain is > >> supposed to work or what dma_fence_chain_find_seqno() is supposed to > >> return. > >> > >> dma_fence_chain_find_seqno() is here to give us the fence to wait upon > >> for a particular point in the timeline. The timeline progresses only > >> when all the points prior to a given number have completed. > > Hmm, the question was what point is it supposed to wait for. > > > > For the simple chain of [1, 3], does 1 being signaled imply that all > > points up to 3 are signaled, or does 3 not being signaled imply that all > > points after 1 are not. If that's mentioned already somewhere, my bad. > > If not, could you put the answer somewhere. > > -Chris > > In [1, 3], if 1 is signaled, the timeline value is 1. And find_seqno(2) > should return NULL. > > > In the out_of_order selftest the chain was [1, 2, 3], 2 was signaled and > the test was expecting no fence to be returned by find_seqno(2). > > But we still have to wait on 1 to complete before find_seqno(2) can > return NULL (as in you don't have to wait on anything). > > > Hope that answer the question. I asked Christian to document why timeline works like this, but I can't find it in the kerneldoc right now. If it's missing I think we should fix that and add the explanation, iirc it was around gpu reset creating too much havoc otherwise. -Daniel > > > -Lionel > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test 2020-06-25 13:59 ` Daniel Vetter @ 2020-07-02 12:57 ` Christian König -1 siblings, 0 replies; 41+ messages in thread From: Christian König @ 2020-07-02 12:57 UTC (permalink / raw) To: Daniel Vetter, Lionel Landwerlin; +Cc: intel-gfx, dri-devel, Chris Wilson Am 25.06.20 um 15:59 schrieb Daniel Vetter: > On Thu, Jun 25, 2020 at 3:23 PM Lionel Landwerlin > <lionel.g.landwerlin@intel.com> wrote: >> On 25/06/2020 16:18, Chris Wilson wrote: >>> Quoting Lionel Landwerlin (2020-06-25 13:34:43) >>>> There was probably a misunderstand on how the dma-fence-chain is >>>> supposed to work or what dma_fence_chain_find_seqno() is supposed to >>>> return. >>>> >>>> dma_fence_chain_find_seqno() is here to give us the fence to wait upon >>>> for a particular point in the timeline. The timeline progresses only >>>> when all the points prior to a given number have completed. >>> Hmm, the question was what point is it supposed to wait for. >>> >>> For the simple chain of [1, 3], does 1 being signaled imply that all >>> points up to 3 are signaled, or does 3 not being signaled imply that all >>> points after 1 are not. If that's mentioned already somewhere, my bad. >>> If not, could you put the answer somewhere. >>> -Chris >> In [1, 3], if 1 is signaled, the timeline value is 1. And find_seqno(2) >> should return NULL. >> >> >> In the out_of_order selftest the chain was [1, 2, 3], 2 was signaled and >> the test was expecting no fence to be returned by find_seqno(2). >> >> But we still have to wait on 1 to complete before find_seqno(2) can >> return NULL (as in you don't have to wait on anything). >> >> >> Hope that answer the question. > I asked Christian to document why timeline works like this, but I > can't find it in the kerneldoc right now. If it's missing I think we > should fix that and add the explanation, iirc it was around gpu reset > creating too much havoc otherwise. I do remember that I wrote a patch to improve the kerneldoc for timeline semaphores, but then somebody else came along with an even better description. Unfortunately it looks like neither was ever merged. Need to dig through my mails, Christian. > -Daniel > >> >> -Lionel >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fintel-gfx&data=02%7C01%7Cchristian.koenig%40amd.com%7Cfd87640cd9bd422971bf08d8191004d2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637286903879074805&sdata=M3WGWbuyQKZeGC0J3wEKtgQ1oKYo6GOAMvKU2mU3r%2FM%3D&reserved=0 > > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test @ 2020-07-02 12:57 ` Christian König 0 siblings, 0 replies; 41+ messages in thread From: Christian König @ 2020-07-02 12:57 UTC (permalink / raw) To: Daniel Vetter, Lionel Landwerlin; +Cc: intel-gfx, dri-devel, Chris Wilson Am 25.06.20 um 15:59 schrieb Daniel Vetter: > On Thu, Jun 25, 2020 at 3:23 PM Lionel Landwerlin > <lionel.g.landwerlin@intel.com> wrote: >> On 25/06/2020 16:18, Chris Wilson wrote: >>> Quoting Lionel Landwerlin (2020-06-25 13:34:43) >>>> There was probably a misunderstand on how the dma-fence-chain is >>>> supposed to work or what dma_fence_chain_find_seqno() is supposed to >>>> return. >>>> >>>> dma_fence_chain_find_seqno() is here to give us the fence to wait upon >>>> for a particular point in the timeline. The timeline progresses only >>>> when all the points prior to a given number have completed. >>> Hmm, the question was what point is it supposed to wait for. >>> >>> For the simple chain of [1, 3], does 1 being signaled imply that all >>> points up to 3 are signaled, or does 3 not being signaled imply that all >>> points after 1 are not. If that's mentioned already somewhere, my bad. >>> If not, could you put the answer somewhere. >>> -Chris >> In [1, 3], if 1 is signaled, the timeline value is 1. And find_seqno(2) >> should return NULL. >> >> >> In the out_of_order selftest the chain was [1, 2, 3], 2 was signaled and >> the test was expecting no fence to be returned by find_seqno(2). >> >> But we still have to wait on 1 to complete before find_seqno(2) can >> return NULL (as in you don't have to wait on anything). >> >> >> Hope that answer the question. > I asked Christian to document why timeline works like this, but I > can't find it in the kerneldoc right now. If it's missing I think we > should fix that and add the explanation, iirc it was around gpu reset > creating too much havoc otherwise. I do remember that I wrote a patch to improve the kerneldoc for timeline semaphores, but then somebody else came along with an even better description. Unfortunately it looks like neither was ever merged. Need to dig through my mails, Christian. > -Daniel > >> >> -Lionel >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fintel-gfx&data=02%7C01%7Cchristian.koenig%40amd.com%7Cfd87640cd9bd422971bf08d8191004d2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637286903879074805&sdata=M3WGWbuyQKZeGC0J3wEKtgQ1oKYo6GOAMvKU2mU3r%2FM%3D&reserved=0 > > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" 2020-06-25 12:34 ` [Intel-gfx] " Lionel Landwerlin @ 2020-06-25 12:43 ` Christian König -1 siblings, 0 replies; 41+ messages in thread From: Christian König @ 2020-06-25 12:43 UTC (permalink / raw) To: Lionel Landwerlin, dri-devel; +Cc: intel-gfx, chris Am 25.06.20 um 14:34 schrieb Lionel Landwerlin: > This reverts commit 5de376bb434f80a13138f0ebedc8351ab73d8b0d. > > This change breaks synchronization of a timeline. > dma_fence_chain_find_seqno() might be a bit of a confusing name but > this function is not trying to find a particular seqno, is supposed to > give a fence to wait on for a particular point in the timeline. > > In a timeline, a particular value is reached when all the points up to > and including that value have signaled. > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Reviewed-by: Christian König <christian.koenig@amd.com> > --- > drivers/dma-buf/dma-fence-chain.c | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c > index c435bbba851c..3d123502ff12 100644 > --- a/drivers/dma-buf/dma-fence-chain.c > +++ b/drivers/dma-buf/dma-fence-chain.c > @@ -99,12 +99,6 @@ int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno) > return -EINVAL; > > dma_fence_chain_for_each(*pfence, &chain->base) { > - if ((*pfence)->seqno < seqno) { /* already signaled */ > - dma_fence_put(*pfence); > - *pfence = NULL; > - break; > - } > - > if ((*pfence)->context != chain->base.context || > to_dma_fence_chain(*pfence)->prev_seqno < seqno) > break; > @@ -228,7 +222,6 @@ EXPORT_SYMBOL(dma_fence_chain_ops); > * @chain: the chain node to initialize > * @prev: the previous fence > * @fence: the current fence > - * @seqno: the sequence number (syncpt) of the fence within the chain > * > * Initialize a new chain node and either start a new chain or add the node to > * the existing chain of the previous fence. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" @ 2020-06-25 12:43 ` Christian König 0 siblings, 0 replies; 41+ messages in thread From: Christian König @ 2020-06-25 12:43 UTC (permalink / raw) To: Lionel Landwerlin, dri-devel; +Cc: intel-gfx, chris Am 25.06.20 um 14:34 schrieb Lionel Landwerlin: > This reverts commit 5de376bb434f80a13138f0ebedc8351ab73d8b0d. > > This change breaks synchronization of a timeline. > dma_fence_chain_find_seqno() might be a bit of a confusing name but > this function is not trying to find a particular seqno, is supposed to > give a fence to wait on for a particular point in the timeline. > > In a timeline, a particular value is reached when all the points up to > and including that value have signaled. > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Reviewed-by: Christian König <christian.koenig@amd.com> > --- > drivers/dma-buf/dma-fence-chain.c | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c > index c435bbba851c..3d123502ff12 100644 > --- a/drivers/dma-buf/dma-fence-chain.c > +++ b/drivers/dma-buf/dma-fence-chain.c > @@ -99,12 +99,6 @@ int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno) > return -EINVAL; > > dma_fence_chain_for_each(*pfence, &chain->base) { > - if ((*pfence)->seqno < seqno) { /* already signaled */ > - dma_fence_put(*pfence); > - *pfence = NULL; > - break; > - } > - > if ((*pfence)->context != chain->base.context || > to_dma_fence_chain(*pfence)->prev_seqno < seqno) > break; > @@ -228,7 +222,6 @@ EXPORT_SYMBOL(dma_fence_chain_ops); > * @chain: the chain node to initialize > * @prev: the previous fence > * @fence: the current fence > - * @seqno: the sequence number (syncpt) of the fence within the chain > * > * Initialize a new chain node and either start a new chain or add the node to > * the existing chain of the previous fence. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" 2020-06-25 12:43 ` [Intel-gfx] " Christian König @ 2020-06-25 17:13 ` Dave Airlie -1 siblings, 0 replies; 41+ messages in thread From: Dave Airlie @ 2020-06-25 17:13 UTC (permalink / raw) To: Christian König Cc: Intel Graphics Development, Chris Wilson, venkata.s.dhanalakota, dri-devel WTUF? How did this ever land in my tree, there is no ACK on this from anyone in core dma-buf, Intel team, clean your house up here, I'm going to have to ask you to stop Chris merging stuff without oversight, if this sort of thing happens, this is totally unacceptable. Dave. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Tested-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> Reviewed-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> On Thu, 25 Jun 2020 at 22:43, Christian König <christian.koenig@amd.com> wrote: > > Am 25.06.20 um 14:34 schrieb Lionel Landwerlin: > > This reverts commit 5de376bb434f80a13138f0ebedc8351ab73d8b0d. > > > > This change breaks synchronization of a timeline. > > dma_fence_chain_find_seqno() might be a bit of a confusing name but > > this function is not trying to find a particular seqno, is supposed to > > give a fence to wait on for a particular point in the timeline. > > > > In a timeline, a particular value is reached when all the points up to > > and including that value have signaled. > > > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > > Reviewed-by: Christian König <christian.koenig@amd.com> > > > --- > > drivers/dma-buf/dma-fence-chain.c | 7 ------- > > 1 file changed, 7 deletions(-) > > > > diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c > > index c435bbba851c..3d123502ff12 100644 > > --- a/drivers/dma-buf/dma-fence-chain.c > > +++ b/drivers/dma-buf/dma-fence-chain.c > > @@ -99,12 +99,6 @@ int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno) > > return -EINVAL; > > > > dma_fence_chain_for_each(*pfence, &chain->base) { > > - if ((*pfence)->seqno < seqno) { /* already signaled */ > > - dma_fence_put(*pfence); > > - *pfence = NULL; > > - break; > > - } > > - > > if ((*pfence)->context != chain->base.context || > > to_dma_fence_chain(*pfence)->prev_seqno < seqno) > > break; > > @@ -228,7 +222,6 @@ EXPORT_SYMBOL(dma_fence_chain_ops); > > * @chain: the chain node to initialize > > * @prev: the previous fence > > * @fence: the current fence > > - * @seqno: the sequence number (syncpt) of the fence within the chain > > * > > * Initialize a new chain node and either start a new chain or add the node to > > * the existing chain of the previous fence. > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" @ 2020-06-25 17:13 ` Dave Airlie 0 siblings, 0 replies; 41+ messages in thread From: Dave Airlie @ 2020-06-25 17:13 UTC (permalink / raw) To: Christian König; +Cc: Intel Graphics Development, Chris Wilson, dri-devel WTUF? How did this ever land in my tree, there is no ACK on this from anyone in core dma-buf, Intel team, clean your house up here, I'm going to have to ask you to stop Chris merging stuff without oversight, if this sort of thing happens, this is totally unacceptable. Dave. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Tested-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> Reviewed-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> On Thu, 25 Jun 2020 at 22:43, Christian König <christian.koenig@amd.com> wrote: > > Am 25.06.20 um 14:34 schrieb Lionel Landwerlin: > > This reverts commit 5de376bb434f80a13138f0ebedc8351ab73d8b0d. > > > > This change breaks synchronization of a timeline. > > dma_fence_chain_find_seqno() might be a bit of a confusing name but > > this function is not trying to find a particular seqno, is supposed to > > give a fence to wait on for a particular point in the timeline. > > > > In a timeline, a particular value is reached when all the points up to > > and including that value have signaled. > > > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > > Reviewed-by: Christian König <christian.koenig@amd.com> > > > --- > > drivers/dma-buf/dma-fence-chain.c | 7 ------- > > 1 file changed, 7 deletions(-) > > > > diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c > > index c435bbba851c..3d123502ff12 100644 > > --- a/drivers/dma-buf/dma-fence-chain.c > > +++ b/drivers/dma-buf/dma-fence-chain.c > > @@ -99,12 +99,6 @@ int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno) > > return -EINVAL; > > > > dma_fence_chain_for_each(*pfence, &chain->base) { > > - if ((*pfence)->seqno < seqno) { /* already signaled */ > > - dma_fence_put(*pfence); > > - *pfence = NULL; > > - break; > > - } > > - > > if ((*pfence)->context != chain->base.context || > > to_dma_fence_chain(*pfence)->prev_seqno < seqno) > > break; > > @@ -228,7 +222,6 @@ EXPORT_SYMBOL(dma_fence_chain_ops); > > * @chain: the chain node to initialize > > * @prev: the previous fence > > * @fence: the current fence > > - * @seqno: the sequence number (syncpt) of the fence within the chain > > * > > * Initialize a new chain node and either start a new chain or add the node to > > * the existing chain of the previous fence. > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" 2020-06-25 17:13 ` Dave Airlie @ 2020-06-25 19:27 ` Jani Nikula -1 siblings, 0 replies; 41+ messages in thread From: Jani Nikula @ 2020-06-25 19:27 UTC (permalink / raw) To: Dave Airlie, Christian König Cc: Intel Graphics Development, dri-devel, Chris Wilson, venkata.s.dhanalakota, Thomas Zimmermann On Fri, 26 Jun 2020, Dave Airlie <airlied@gmail.com> wrote: > WTUF? > > How did this ever land in my tree, there is no ACK on this from anyone > in core dma-buf, > > Intel team, clean your house up here, I'm going to have to ask you to > stop Chris merging stuff without oversight, if this sort of thing > happens, this is totally unacceptable. There's no argument, an ack is required. In fairness to the i915 maintainers, though, this particular commit was merged via drm-misc-next [1]. As a side note, there seem to be extra checks in place for acks when applying non-i915 patches to drm-intel; there are no such checks for drm-misc. BR, Jani. [1] http://lore.kernel.org/r/20200414090738.GA16827@linux-uq9g > > Dave. > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Tested-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> > Reviewed-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> > > > On Thu, 25 Jun 2020 at 22:43, Christian König <christian.koenig@amd.com> wrote: >> >> Am 25.06.20 um 14:34 schrieb Lionel Landwerlin: >> > This reverts commit 5de376bb434f80a13138f0ebedc8351ab73d8b0d. >> > >> > This change breaks synchronization of a timeline. >> > dma_fence_chain_find_seqno() might be a bit of a confusing name but >> > this function is not trying to find a particular seqno, is supposed to >> > give a fence to wait on for a particular point in the timeline. >> > >> > In a timeline, a particular value is reached when all the points up to >> > and including that value have signaled. >> > >> > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> >> Reviewed-by: Christian König <christian.koenig@amd.com> >> >> > --- >> > drivers/dma-buf/dma-fence-chain.c | 7 ------- >> > 1 file changed, 7 deletions(-) >> > >> > diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c >> > index c435bbba851c..3d123502ff12 100644 >> > --- a/drivers/dma-buf/dma-fence-chain.c >> > +++ b/drivers/dma-buf/dma-fence-chain.c >> > @@ -99,12 +99,6 @@ int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno) >> > return -EINVAL; >> > >> > dma_fence_chain_for_each(*pfence, &chain->base) { >> > - if ((*pfence)->seqno < seqno) { /* already signaled */ >> > - dma_fence_put(*pfence); >> > - *pfence = NULL; >> > - break; >> > - } >> > - >> > if ((*pfence)->context != chain->base.context || >> > to_dma_fence_chain(*pfence)->prev_seqno < seqno) >> > break; >> > @@ -228,7 +222,6 @@ EXPORT_SYMBOL(dma_fence_chain_ops); >> > * @chain: the chain node to initialize >> > * @prev: the previous fence >> > * @fence: the current fence >> > - * @seqno: the sequence number (syncpt) of the fence within the chain >> > * >> > * Initialize a new chain node and either start a new chain or add the node to >> > * the existing chain of the previous fence. >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" @ 2020-06-25 19:27 ` Jani Nikula 0 siblings, 0 replies; 41+ messages in thread From: Jani Nikula @ 2020-06-25 19:27 UTC (permalink / raw) To: Dave Airlie, Christian König Cc: Intel Graphics Development, dri-devel, Chris Wilson, Thomas Zimmermann On Fri, 26 Jun 2020, Dave Airlie <airlied@gmail.com> wrote: > WTUF? > > How did this ever land in my tree, there is no ACK on this from anyone > in core dma-buf, > > Intel team, clean your house up here, I'm going to have to ask you to > stop Chris merging stuff without oversight, if this sort of thing > happens, this is totally unacceptable. There's no argument, an ack is required. In fairness to the i915 maintainers, though, this particular commit was merged via drm-misc-next [1]. As a side note, there seem to be extra checks in place for acks when applying non-i915 patches to drm-intel; there are no such checks for drm-misc. BR, Jani. [1] http://lore.kernel.org/r/20200414090738.GA16827@linux-uq9g > > Dave. > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Tested-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> > Reviewed-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> > > > On Thu, 25 Jun 2020 at 22:43, Christian König <christian.koenig@amd.com> wrote: >> >> Am 25.06.20 um 14:34 schrieb Lionel Landwerlin: >> > This reverts commit 5de376bb434f80a13138f0ebedc8351ab73d8b0d. >> > >> > This change breaks synchronization of a timeline. >> > dma_fence_chain_find_seqno() might be a bit of a confusing name but >> > this function is not trying to find a particular seqno, is supposed to >> > give a fence to wait on for a particular point in the timeline. >> > >> > In a timeline, a particular value is reached when all the points up to >> > and including that value have signaled. >> > >> > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> >> Reviewed-by: Christian König <christian.koenig@amd.com> >> >> > --- >> > drivers/dma-buf/dma-fence-chain.c | 7 ------- >> > 1 file changed, 7 deletions(-) >> > >> > diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c >> > index c435bbba851c..3d123502ff12 100644 >> > --- a/drivers/dma-buf/dma-fence-chain.c >> > +++ b/drivers/dma-buf/dma-fence-chain.c >> > @@ -99,12 +99,6 @@ int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno) >> > return -EINVAL; >> > >> > dma_fence_chain_for_each(*pfence, &chain->base) { >> > - if ((*pfence)->seqno < seqno) { /* already signaled */ >> > - dma_fence_put(*pfence); >> > - *pfence = NULL; >> > - break; >> > - } >> > - >> > if ((*pfence)->context != chain->base.context || >> > to_dma_fence_chain(*pfence)->prev_seqno < seqno) >> > break; >> > @@ -228,7 +222,6 @@ EXPORT_SYMBOL(dma_fence_chain_ops); >> > * @chain: the chain node to initialize >> > * @prev: the previous fence >> > * @fence: the current fence >> > - * @seqno: the sequence number (syncpt) of the fence within the chain >> > * >> > * Initialize a new chain node and either start a new chain or add the node to >> > * the existing chain of the previous fence. >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" 2020-06-25 19:27 ` Jani Nikula @ 2020-06-25 19:54 ` Daniel Vetter -1 siblings, 0 replies; 41+ messages in thread From: Daniel Vetter @ 2020-06-25 19:54 UTC (permalink / raw) To: Jani Nikula Cc: Intel Graphics Development, dri-devel, Chris Wilson, Thomas Zimmermann, Christian König Ignoring everything else ... On Thu, Jun 25, 2020 at 9:28 PM Jani Nikula <jani.nikula@linux.intel.com> wrote: > As a side note, there seem to be extra checks in place for acks when > applying non-i915 patches to drm-intel; there are no such checks for > drm-misc. One option to generalize that that I pondered is to consult get_maintainers.pl asking for git repo link, and if that returns something else, then insist that there's an ack from a relevant maintainer. It's a bit of typing, but I think the bigger problem is that there's a ton of false positives. But maybe that's a good thing, would give some motivation to keep MAINTAINERS updated. The other issue is though that drm-misc is plenty used to merge patches even when the respective maintainers are absent for weeks, or unresponsive. If we just blindly implement that rule, then the only possible Ack for these would be Dave&me as subsystem maintainers, and I don't want to be in the business of stamping approvals for all this stuff. Much better if people just collaborate. So I think an ack check would be nice, but probably not practical. Plus in this situation here drm-misc.git actually is the main repo, and we wont ever be able to teach a script to make a judgement call of whether that patch has the right amount of review on it. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" @ 2020-06-25 19:54 ` Daniel Vetter 0 siblings, 0 replies; 41+ messages in thread From: Daniel Vetter @ 2020-06-25 19:54 UTC (permalink / raw) To: Jani Nikula Cc: Intel Graphics Development, dri-devel, Chris Wilson, Thomas Zimmermann, Christian König Ignoring everything else ... On Thu, Jun 25, 2020 at 9:28 PM Jani Nikula <jani.nikula@linux.intel.com> wrote: > As a side note, there seem to be extra checks in place for acks when > applying non-i915 patches to drm-intel; there are no such checks for > drm-misc. One option to generalize that that I pondered is to consult get_maintainers.pl asking for git repo link, and if that returns something else, then insist that there's an ack from a relevant maintainer. It's a bit of typing, but I think the bigger problem is that there's a ton of false positives. But maybe that's a good thing, would give some motivation to keep MAINTAINERS updated. The other issue is though that drm-misc is plenty used to merge patches even when the respective maintainers are absent for weeks, or unresponsive. If we just blindly implement that rule, then the only possible Ack for these would be Dave&me as subsystem maintainers, and I don't want to be in the business of stamping approvals for all this stuff. Much better if people just collaborate. So I think an ack check would be nice, but probably not practical. Plus in this situation here drm-misc.git actually is the main repo, and we wont ever be able to teach a script to make a judgement call of whether that patch has the right amount of review on it. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" 2020-06-25 19:54 ` Daniel Vetter @ 2020-06-26 4:43 ` Sumit Semwal -1 siblings, 0 replies; 41+ messages in thread From: Sumit Semwal @ 2020-06-26 4:43 UTC (permalink / raw) To: Daniel Vetter Cc: Intel Graphics Development, dri-devel, Chris Wilson, Thomas Zimmermann, Christian König On Fri, 26 Jun 2020 at 01:24, Daniel Vetter <daniel@ffwll.ch> wrote: > > Ignoring everything else ... > > On Thu, Jun 25, 2020 at 9:28 PM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > As a side note, there seem to be extra checks in place for acks when > > applying non-i915 patches to drm-intel; there are no such checks for > > drm-misc. > > One option to generalize that that I pondered is to consult > get_maintainers.pl asking for git repo link, and if that returns > something else, then insist that there's an ack from a relevant > maintainer. It's a bit of typing, but I think the bigger problem is > that there's a ton of false positives. Right; for the particular patch, I wasn't even in the to: or cc: field and that made it slip from my radar. I would definitely ask any one sending patches for dma-buf directory to follow the get_maintainers.pl religiously. > > But maybe that's a good thing, would give some motivation to keep > MAINTAINERS updated. > > The other issue is though that drm-misc is plenty used to merge > patches even when the respective maintainers are absent for weeks, or > unresponsive. If we just blindly implement that rule, then the only > possible Ack for these would be Dave&me as subsystem maintainers, and > I don't want to be in the business of stamping approvals for all this > stuff. Much better if people just collaborate. > > So I think an ack check would be nice, but probably not practical. > > Plus in this situation here drm-misc.git actually is the main repo, > and we wont ever be able to teach a script to make a judgement call of > whether that patch has the right amount of review on it. > -Daniel Best, Sumit. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" @ 2020-06-26 4:43 ` Sumit Semwal 0 siblings, 0 replies; 41+ messages in thread From: Sumit Semwal @ 2020-06-26 4:43 UTC (permalink / raw) To: Daniel Vetter Cc: Intel Graphics Development, dri-devel, Chris Wilson, Thomas Zimmermann, Christian König On Fri, 26 Jun 2020 at 01:24, Daniel Vetter <daniel@ffwll.ch> wrote: > > Ignoring everything else ... > > On Thu, Jun 25, 2020 at 9:28 PM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > As a side note, there seem to be extra checks in place for acks when > > applying non-i915 patches to drm-intel; there are no such checks for > > drm-misc. > > One option to generalize that that I pondered is to consult > get_maintainers.pl asking for git repo link, and if that returns > something else, then insist that there's an ack from a relevant > maintainer. It's a bit of typing, but I think the bigger problem is > that there's a ton of false positives. Right; for the particular patch, I wasn't even in the to: or cc: field and that made it slip from my radar. I would definitely ask any one sending patches for dma-buf directory to follow the get_maintainers.pl religiously. > > But maybe that's a good thing, would give some motivation to keep > MAINTAINERS updated. > > The other issue is though that drm-misc is plenty used to merge > patches even when the respective maintainers are absent for weeks, or > unresponsive. If we just blindly implement that rule, then the only > possible Ack for these would be Dave&me as subsystem maintainers, and > I don't want to be in the business of stamping approvals for all this > stuff. Much better if people just collaborate. > > So I think an ack check would be nice, but probably not practical. > > Plus in this situation here drm-misc.git actually is the main repo, > and we wont ever be able to teach a script to make a judgement call of > whether that patch has the right amount of review on it. > -Daniel Best, Sumit. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" 2020-06-26 4:43 ` Sumit Semwal @ 2020-06-26 7:03 ` Christian König -1 siblings, 0 replies; 41+ messages in thread From: Christian König @ 2020-06-26 7:03 UTC (permalink / raw) To: Sumit Semwal, Daniel Vetter Cc: Thomas Zimmermann, Intel Graphics Development, dri-devel, Chris Wilson Am 26.06.20 um 06:43 schrieb Sumit Semwal: > On Fri, 26 Jun 2020 at 01:24, Daniel Vetter <daniel@ffwll.ch> wrote: >> Ignoring everything else ... >> >> On Thu, Jun 25, 2020 at 9:28 PM Jani Nikula <jani.nikula@linux.intel.com> wrote: >>> As a side note, there seem to be extra checks in place for acks when >>> applying non-i915 patches to drm-intel; there are no such checks for >>> drm-misc. >> One option to generalize that that I pondered is to consult >> get_maintainers.pl asking for git repo link, and if that returns >> something else, then insist that there's an ack from a relevant >> maintainer. It's a bit of typing, but I think the bigger problem is >> that there's a ton of false positives. > Right; for the particular patch, I wasn't even in the to: or cc: field > and that made it slip from my radar. I would definitely ask any one > sending patches for dma-buf directory to follow the get_maintainers.pl > religiously. >> But maybe that's a good thing, would give some motivation to keep >> MAINTAINERS updated. Should I maybe add myself as maintainer as well? I've written enough stuff in there to know the code quite a bit. Christian. >> >> The other issue is though that drm-misc is plenty used to merge >> patches even when the respective maintainers are absent for weeks, or >> unresponsive. If we just blindly implement that rule, then the only >> possible Ack for these would be Dave&me as subsystem maintainers, and >> I don't want to be in the business of stamping approvals for all this >> stuff. Much better if people just collaborate. >> >> So I think an ack check would be nice, but probably not practical. >> >> Plus in this situation here drm-misc.git actually is the main repo, >> and we wont ever be able to teach a script to make a judgement call of >> whether that patch has the right amount of review on it. >> -Daniel > Best, > Sumit. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" @ 2020-06-26 7:03 ` Christian König 0 siblings, 0 replies; 41+ messages in thread From: Christian König @ 2020-06-26 7:03 UTC (permalink / raw) To: Sumit Semwal, Daniel Vetter Cc: Thomas Zimmermann, Intel Graphics Development, dri-devel, Chris Wilson Am 26.06.20 um 06:43 schrieb Sumit Semwal: > On Fri, 26 Jun 2020 at 01:24, Daniel Vetter <daniel@ffwll.ch> wrote: >> Ignoring everything else ... >> >> On Thu, Jun 25, 2020 at 9:28 PM Jani Nikula <jani.nikula@linux.intel.com> wrote: >>> As a side note, there seem to be extra checks in place for acks when >>> applying non-i915 patches to drm-intel; there are no such checks for >>> drm-misc. >> One option to generalize that that I pondered is to consult >> get_maintainers.pl asking for git repo link, and if that returns >> something else, then insist that there's an ack from a relevant >> maintainer. It's a bit of typing, but I think the bigger problem is >> that there's a ton of false positives. > Right; for the particular patch, I wasn't even in the to: or cc: field > and that made it slip from my radar. I would definitely ask any one > sending patches for dma-buf directory to follow the get_maintainers.pl > religiously. >> But maybe that's a good thing, would give some motivation to keep >> MAINTAINERS updated. Should I maybe add myself as maintainer as well? I've written enough stuff in there to know the code quite a bit. Christian. >> >> The other issue is though that drm-misc is plenty used to merge >> patches even when the respective maintainers are absent for weeks, or >> unresponsive. If we just blindly implement that rule, then the only >> possible Ack for these would be Dave&me as subsystem maintainers, and >> I don't want to be in the business of stamping approvals for all this >> stuff. Much better if people just collaborate. >> >> So I think an ack check would be nice, but probably not practical. >> >> Plus in this situation here drm-misc.git actually is the main repo, >> and we wont ever be able to teach a script to make a judgement call of >> whether that patch has the right amount of review on it. >> -Daniel > Best, > Sumit. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" 2020-06-26 7:03 ` Christian König @ 2020-06-26 12:39 ` Daniel Vetter -1 siblings, 0 replies; 41+ messages in thread From: Daniel Vetter @ 2020-06-26 12:39 UTC (permalink / raw) To: Christian König Cc: Intel Graphics Development, dri-devel, Chris Wilson, Thomas Zimmermann On Fri, Jun 26, 2020 at 9:03 AM Christian König <christian.koenig@amd.com> wrote: > > Am 26.06.20 um 06:43 schrieb Sumit Semwal: > > On Fri, 26 Jun 2020 at 01:24, Daniel Vetter <daniel@ffwll.ch> wrote: > >> Ignoring everything else ... > >> > >> On Thu, Jun 25, 2020 at 9:28 PM Jani Nikula <jani.nikula@linux.intel.com> wrote: > >>> As a side note, there seem to be extra checks in place for acks when > >>> applying non-i915 patches to drm-intel; there are no such checks for > >>> drm-misc. > >> One option to generalize that that I pondered is to consult > >> get_maintainers.pl asking for git repo link, and if that returns > >> something else, then insist that there's an ack from a relevant > >> maintainer. It's a bit of typing, but I think the bigger problem is > >> that there's a ton of false positives. > > Right; for the particular patch, I wasn't even in the to: or cc: field > > and that made it slip from my radar. I would definitely ask any one > > sending patches for dma-buf directory to follow the get_maintainers.pl > > religiously. > >> But maybe that's a good thing, would give some motivation to keep > >> MAINTAINERS updated. > > Should I maybe add myself as maintainer as well? I've written enough > stuff in there to know the code quite a bit. I think that makes lots of sense, since defacto you already are :-) If you feel like bikeshed, get_maintainers.pl also supports R: for reviewer, but given that you also push patches to drm-misc M: for maintainer feels more accurate. -Daniel > > Christian. > > >> > >> The other issue is though that drm-misc is plenty used to merge > >> patches even when the respective maintainers are absent for weeks, or > >> unresponsive. If we just blindly implement that rule, then the only > >> possible Ack for these would be Dave&me as subsystem maintainers, and > >> I don't want to be in the business of stamping approvals for all this > >> stuff. Much better if people just collaborate. > >> > >> So I think an ack check would be nice, but probably not practical. > >> > >> Plus in this situation here drm-misc.git actually is the main repo, > >> and we wont ever be able to teach a script to make a judgement call of > >> whether that patch has the right amount of review on it. > >> -Daniel > > Best, > > Sumit. > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" @ 2020-06-26 12:39 ` Daniel Vetter 0 siblings, 0 replies; 41+ messages in thread From: Daniel Vetter @ 2020-06-26 12:39 UTC (permalink / raw) To: Christian König Cc: Intel Graphics Development, dri-devel, Chris Wilson, Thomas Zimmermann, Sumit Semwal On Fri, Jun 26, 2020 at 9:03 AM Christian König <christian.koenig@amd.com> wrote: > > Am 26.06.20 um 06:43 schrieb Sumit Semwal: > > On Fri, 26 Jun 2020 at 01:24, Daniel Vetter <daniel@ffwll.ch> wrote: > >> Ignoring everything else ... > >> > >> On Thu, Jun 25, 2020 at 9:28 PM Jani Nikula <jani.nikula@linux.intel.com> wrote: > >>> As a side note, there seem to be extra checks in place for acks when > >>> applying non-i915 patches to drm-intel; there are no such checks for > >>> drm-misc. > >> One option to generalize that that I pondered is to consult > >> get_maintainers.pl asking for git repo link, and if that returns > >> something else, then insist that there's an ack from a relevant > >> maintainer. It's a bit of typing, but I think the bigger problem is > >> that there's a ton of false positives. > > Right; for the particular patch, I wasn't even in the to: or cc: field > > and that made it slip from my radar. I would definitely ask any one > > sending patches for dma-buf directory to follow the get_maintainers.pl > > religiously. > >> But maybe that's a good thing, would give some motivation to keep > >> MAINTAINERS updated. > > Should I maybe add myself as maintainer as well? I've written enough > stuff in there to know the code quite a bit. I think that makes lots of sense, since defacto you already are :-) If you feel like bikeshed, get_maintainers.pl also supports R: for reviewer, but given that you also push patches to drm-misc M: for maintainer feels more accurate. -Daniel > > Christian. > > >> > >> The other issue is though that drm-misc is plenty used to merge > >> patches even when the respective maintainers are absent for weeks, or > >> unresponsive. If we just blindly implement that rule, then the only > >> possible Ack for these would be Dave&me as subsystem maintainers, and > >> I don't want to be in the business of stamping approvals for all this > >> stuff. Much better if people just collaborate. > >> > >> So I think an ack check would be nice, but probably not practical. > >> > >> Plus in this situation here drm-misc.git actually is the main repo, > >> and we wont ever be able to teach a script to make a judgement call of > >> whether that patch has the right amount of review on it. > >> -Daniel > > Best, > > Sumit. > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" 2020-06-26 12:39 ` Daniel Vetter @ 2020-06-26 15:34 ` Sumit Semwal -1 siblings, 0 replies; 41+ messages in thread From: Sumit Semwal @ 2020-06-26 15:34 UTC (permalink / raw) To: Daniel Vetter Cc: Intel Graphics Development, dri-devel, Chris Wilson, Thomas Zimmermann, Christian König [-- Attachment #1.1: Type: text/plain, Size: 1872 bytes --] Hi Christian, On Fri, 26 Jun 2020, 18:10 Daniel Vetter, <daniel@ffwll.ch> wrote: > On Fri, Jun 26, 2020 at 9:03 AM Christian König > <christian.koenig@amd.com> wrote: > > > > Am 26.06.20 um 06:43 schrieb Sumit Semwal: > > > On Fri, 26 Jun 2020 at 01:24, Daniel Vetter <daniel@ffwll.ch> wrote: > > >> Ignoring everything else ... > > >> > > >> On Thu, Jun 25, 2020 at 9:28 PM Jani Nikula < > jani.nikula@linux.intel.com> wrote: > > >>> As a side note, there seem to be extra checks in place for acks when > > >>> applying non-i915 patches to drm-intel; there are no such checks for > > >>> drm-misc. > > >> One option to generalize that that I pondered is to consult > > >> get_maintainers.pl asking for git repo link, and if that returns > > >> something else, then insist that there's an ack from a relevant > > >> maintainer. It's a bit of typing, but I think the bigger problem is > > >> that there's a ton of false positives. > > > Right; for the particular patch, I wasn't even in the to: or cc: field > > > and that made it slip from my radar. I would definitely ask any one > > > sending patches for dma-buf directory to follow the get_maintainers.pl > > > religiously. > > >> But maybe that's a good thing, would give some motivation to keep > > >> MAINTAINERS updated. > > > > Should I maybe add myself as maintainer as well? I've written enough > > stuff in there to know the code quite a bit. > > I think that makes lots of sense, since defacto you already are :-) > > If you feel like bikeshed, get_maintainers.pl also supports R: for > reviewer, but given that you also push patches to drm-misc M: for > maintainer feels more accurate. > I think given you've been reviewing and changing most of the code around dma-fences, it should be ok to add you as the maintainer for those bits? -Daniel > Best, Sumit. [-- Attachment #1.2: Type: text/html, Size: 3195 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" @ 2020-06-26 15:34 ` Sumit Semwal 0 siblings, 0 replies; 41+ messages in thread From: Sumit Semwal @ 2020-06-26 15:34 UTC (permalink / raw) To: Daniel Vetter Cc: Intel Graphics Development, dri-devel, Chris Wilson, Thomas Zimmermann, Christian König [-- Attachment #1.1: Type: text/plain, Size: 1872 bytes --] Hi Christian, On Fri, 26 Jun 2020, 18:10 Daniel Vetter, <daniel@ffwll.ch> wrote: > On Fri, Jun 26, 2020 at 9:03 AM Christian König > <christian.koenig@amd.com> wrote: > > > > Am 26.06.20 um 06:43 schrieb Sumit Semwal: > > > On Fri, 26 Jun 2020 at 01:24, Daniel Vetter <daniel@ffwll.ch> wrote: > > >> Ignoring everything else ... > > >> > > >> On Thu, Jun 25, 2020 at 9:28 PM Jani Nikula < > jani.nikula@linux.intel.com> wrote: > > >>> As a side note, there seem to be extra checks in place for acks when > > >>> applying non-i915 patches to drm-intel; there are no such checks for > > >>> drm-misc. > > >> One option to generalize that that I pondered is to consult > > >> get_maintainers.pl asking for git repo link, and if that returns > > >> something else, then insist that there's an ack from a relevant > > >> maintainer. It's a bit of typing, but I think the bigger problem is > > >> that there's a ton of false positives. > > > Right; for the particular patch, I wasn't even in the to: or cc: field > > > and that made it slip from my radar. I would definitely ask any one > > > sending patches for dma-buf directory to follow the get_maintainers.pl > > > religiously. > > >> But maybe that's a good thing, would give some motivation to keep > > >> MAINTAINERS updated. > > > > Should I maybe add myself as maintainer as well? I've written enough > > stuff in there to know the code quite a bit. > > I think that makes lots of sense, since defacto you already are :-) > > If you feel like bikeshed, get_maintainers.pl also supports R: for > reviewer, but given that you also push patches to drm-misc M: for > maintainer feels more accurate. > I think given you've been reviewing and changing most of the code around dma-fences, it should be ok to add you as the maintainer for those bits? -Daniel > Best, Sumit. [-- Attachment #1.2: Type: text/html, Size: 3195 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" 2020-06-25 19:27 ` Jani Nikula @ 2020-06-26 1:36 ` Dave Airlie -1 siblings, 0 replies; 41+ messages in thread From: Dave Airlie @ 2020-06-26 1:36 UTC (permalink / raw) To: Jani Nikula Cc: Intel Graphics Development, dri-devel, Chris Wilson, venkata.s.dhanalakota, Thomas Zimmermann, Christian König On Fri, 26 Jun 2020 at 05:27, Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Fri, 26 Jun 2020, Dave Airlie <airlied@gmail.com> wrote: > > WTUF? > > > > How did this ever land in my tree, there is no ACK on this from anyone > > in core dma-buf, > > > > Intel team, clean your house up here, I'm going to have to ask you to > > stop Chris merging stuff without oversight, if this sort of thing > > happens, this is totally unacceptable. > > There's no argument, an ack is required. > > In fairness to the i915 maintainers, though, this particular commit was > merged via drm-misc-next [1]. > > As a side note, there seem to be extra checks in place for acks when > applying non-i915 patches to drm-intel; there are no such checks for > drm-misc. Sorry Jani, thanks for chasing that down. drm-misc we need to oversight a bit more, I don't think we should be landing things that affect core code with single company acks. Dave. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" @ 2020-06-26 1:36 ` Dave Airlie 0 siblings, 0 replies; 41+ messages in thread From: Dave Airlie @ 2020-06-26 1:36 UTC (permalink / raw) To: Jani Nikula Cc: Intel Graphics Development, dri-devel, Chris Wilson, Thomas Zimmermann, Christian König On Fri, 26 Jun 2020 at 05:27, Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Fri, 26 Jun 2020, Dave Airlie <airlied@gmail.com> wrote: > > WTUF? > > > > How did this ever land in my tree, there is no ACK on this from anyone > > in core dma-buf, > > > > Intel team, clean your house up here, I'm going to have to ask you to > > stop Chris merging stuff without oversight, if this sort of thing > > happens, this is totally unacceptable. > > There's no argument, an ack is required. > > In fairness to the i915 maintainers, though, this particular commit was > merged via drm-misc-next [1]. > > As a side note, there seem to be extra checks in place for acks when > applying non-i915 patches to drm-intel; there are no such checks for > drm-misc. Sorry Jani, thanks for chasing that down. drm-misc we need to oversight a bit more, I don't think we should be landing things that affect core code with single company acks. Dave. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" 2020-06-25 12:43 ` [Intel-gfx] " Christian König @ 2020-07-02 8:26 ` Lionel Landwerlin -1 siblings, 0 replies; 41+ messages in thread From: Lionel Landwerlin @ 2020-07-02 8:26 UTC (permalink / raw) To: Christian König, dri-devel; +Cc: intel-gfx, chris On 25/06/2020 15:43, Christian König wrote: > Am 25.06.20 um 14:34 schrieb Lionel Landwerlin: >> This reverts commit 5de376bb434f80a13138f0ebedc8351ab73d8b0d. >> >> This change breaks synchronization of a timeline. >> dma_fence_chain_find_seqno() might be a bit of a confusing name but >> this function is not trying to find a particular seqno, is supposed to >> give a fence to wait on for a particular point in the timeline. >> >> In a timeline, a particular value is reached when all the points up to >> and including that value have signaled. >> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > > Reviewed-by: Christian König <christian.koenig@amd.com> Now that you are a maintainer, feel free to merge this and the test changes. Thanks, -Lionel > >> --- >> drivers/dma-buf/dma-fence-chain.c | 7 ------- >> 1 file changed, 7 deletions(-) >> >> diff --git a/drivers/dma-buf/dma-fence-chain.c >> b/drivers/dma-buf/dma-fence-chain.c >> index c435bbba851c..3d123502ff12 100644 >> --- a/drivers/dma-buf/dma-fence-chain.c >> +++ b/drivers/dma-buf/dma-fence-chain.c >> @@ -99,12 +99,6 @@ int dma_fence_chain_find_seqno(struct dma_fence >> **pfence, uint64_t seqno) >> return -EINVAL; >> dma_fence_chain_for_each(*pfence, &chain->base) { >> - if ((*pfence)->seqno < seqno) { /* already signaled */ >> - dma_fence_put(*pfence); >> - *pfence = NULL; >> - break; >> - } >> - >> if ((*pfence)->context != chain->base.context || >> to_dma_fence_chain(*pfence)->prev_seqno < seqno) >> break; >> @@ -228,7 +222,6 @@ EXPORT_SYMBOL(dma_fence_chain_ops); >> * @chain: the chain node to initialize >> * @prev: the previous fence >> * @fence: the current fence >> - * @seqno: the sequence number (syncpt) of the fence within the chain >> * >> * Initialize a new chain node and either start a new chain or add >> the node to >> * the existing chain of the previous fence. > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" @ 2020-07-02 8:26 ` Lionel Landwerlin 0 siblings, 0 replies; 41+ messages in thread From: Lionel Landwerlin @ 2020-07-02 8:26 UTC (permalink / raw) To: Christian König, dri-devel; +Cc: intel-gfx, chris On 25/06/2020 15:43, Christian König wrote: > Am 25.06.20 um 14:34 schrieb Lionel Landwerlin: >> This reverts commit 5de376bb434f80a13138f0ebedc8351ab73d8b0d. >> >> This change breaks synchronization of a timeline. >> dma_fence_chain_find_seqno() might be a bit of a confusing name but >> this function is not trying to find a particular seqno, is supposed to >> give a fence to wait on for a particular point in the timeline. >> >> In a timeline, a particular value is reached when all the points up to >> and including that value have signaled. >> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > > Reviewed-by: Christian König <christian.koenig@amd.com> Now that you are a maintainer, feel free to merge this and the test changes. Thanks, -Lionel > >> --- >> drivers/dma-buf/dma-fence-chain.c | 7 ------- >> 1 file changed, 7 deletions(-) >> >> diff --git a/drivers/dma-buf/dma-fence-chain.c >> b/drivers/dma-buf/dma-fence-chain.c >> index c435bbba851c..3d123502ff12 100644 >> --- a/drivers/dma-buf/dma-fence-chain.c >> +++ b/drivers/dma-buf/dma-fence-chain.c >> @@ -99,12 +99,6 @@ int dma_fence_chain_find_seqno(struct dma_fence >> **pfence, uint64_t seqno) >> return -EINVAL; >> dma_fence_chain_for_each(*pfence, &chain->base) { >> - if ((*pfence)->seqno < seqno) { /* already signaled */ >> - dma_fence_put(*pfence); >> - *pfence = NULL; >> - break; >> - } >> - >> if ((*pfence)->context != chain->base.context || >> to_dma_fence_chain(*pfence)->prev_seqno < seqno) >> break; >> @@ -228,7 +222,6 @@ EXPORT_SYMBOL(dma_fence_chain_ops); >> * @chain: the chain node to initialize >> * @prev: the previous fence >> * @fence: the current fence >> - * @seqno: the sequence number (syncpt) of the fence within the chain >> * >> * Initialize a new chain node and either start a new chain or add >> the node to >> * the existing chain of the previous fence. > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" 2020-07-02 8:26 ` [Intel-gfx] " Lionel Landwerlin @ 2020-07-02 8:28 ` Christian König -1 siblings, 0 replies; 41+ messages in thread From: Christian König @ 2020-07-02 8:28 UTC (permalink / raw) To: Lionel Landwerlin, dri-devel; +Cc: intel-gfx, chris Am 02.07.20 um 10:26 schrieb Lionel Landwerlin: > On 25/06/2020 15:43, Christian König wrote: >> Am 25.06.20 um 14:34 schrieb Lionel Landwerlin: >>> This reverts commit 5de376bb434f80a13138f0ebedc8351ab73d8b0d. >>> >>> This change breaks synchronization of a timeline. >>> dma_fence_chain_find_seqno() might be a bit of a confusing name but >>> this function is not trying to find a particular seqno, is supposed to >>> give a fence to wait on for a particular point in the timeline. >>> >>> In a timeline, a particular value is reached when all the points up to >>> and including that value have signaled. >>> >>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> >> Reviewed-by: Christian König <christian.koenig@amd.com> > > > Now that you are a maintainer, feel free to merge this and the test > changes. Sure, I will pick that up later today :) Christian. > > > Thanks, > > > -Lionel > > >> >>> --- >>> drivers/dma-buf/dma-fence-chain.c | 7 ------- >>> 1 file changed, 7 deletions(-) >>> >>> diff --git a/drivers/dma-buf/dma-fence-chain.c >>> b/drivers/dma-buf/dma-fence-chain.c >>> index c435bbba851c..3d123502ff12 100644 >>> --- a/drivers/dma-buf/dma-fence-chain.c >>> +++ b/drivers/dma-buf/dma-fence-chain.c >>> @@ -99,12 +99,6 @@ int dma_fence_chain_find_seqno(struct dma_fence >>> **pfence, uint64_t seqno) >>> return -EINVAL; >>> dma_fence_chain_for_each(*pfence, &chain->base) { >>> - if ((*pfence)->seqno < seqno) { /* already signaled */ >>> - dma_fence_put(*pfence); >>> - *pfence = NULL; >>> - break; >>> - } >>> - >>> if ((*pfence)->context != chain->base.context || >>> to_dma_fence_chain(*pfence)->prev_seqno < seqno) >>> break; >>> @@ -228,7 +222,6 @@ EXPORT_SYMBOL(dma_fence_chain_ops); >>> * @chain: the chain node to initialize >>> * @prev: the previous fence >>> * @fence: the current fence >>> - * @seqno: the sequence number (syncpt) of the fence within the chain >>> * >>> * Initialize a new chain node and either start a new chain or add >>> the node to >>> * the existing chain of the previous fence. >> > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" @ 2020-07-02 8:28 ` Christian König 0 siblings, 0 replies; 41+ messages in thread From: Christian König @ 2020-07-02 8:28 UTC (permalink / raw) To: Lionel Landwerlin, dri-devel; +Cc: intel-gfx, chris Am 02.07.20 um 10:26 schrieb Lionel Landwerlin: > On 25/06/2020 15:43, Christian König wrote: >> Am 25.06.20 um 14:34 schrieb Lionel Landwerlin: >>> This reverts commit 5de376bb434f80a13138f0ebedc8351ab73d8b0d. >>> >>> This change breaks synchronization of a timeline. >>> dma_fence_chain_find_seqno() might be a bit of a confusing name but >>> this function is not trying to find a particular seqno, is supposed to >>> give a fence to wait on for a particular point in the timeline. >>> >>> In a timeline, a particular value is reached when all the points up to >>> and including that value have signaled. >>> >>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> >> Reviewed-by: Christian König <christian.koenig@amd.com> > > > Now that you are a maintainer, feel free to merge this and the test > changes. Sure, I will pick that up later today :) Christian. > > > Thanks, > > > -Lionel > > >> >>> --- >>> drivers/dma-buf/dma-fence-chain.c | 7 ------- >>> 1 file changed, 7 deletions(-) >>> >>> diff --git a/drivers/dma-buf/dma-fence-chain.c >>> b/drivers/dma-buf/dma-fence-chain.c >>> index c435bbba851c..3d123502ff12 100644 >>> --- a/drivers/dma-buf/dma-fence-chain.c >>> +++ b/drivers/dma-buf/dma-fence-chain.c >>> @@ -99,12 +99,6 @@ int dma_fence_chain_find_seqno(struct dma_fence >>> **pfence, uint64_t seqno) >>> return -EINVAL; >>> dma_fence_chain_for_each(*pfence, &chain->base) { >>> - if ((*pfence)->seqno < seqno) { /* already signaled */ >>> - dma_fence_put(*pfence); >>> - *pfence = NULL; >>> - break; >>> - } >>> - >>> if ((*pfence)->context != chain->base.context || >>> to_dma_fence_chain(*pfence)->prev_seqno < seqno) >>> break; >>> @@ -228,7 +222,6 @@ EXPORT_SYMBOL(dma_fence_chain_ops); >>> * @chain: the chain node to initialize >>> * @prev: the previous fence >>> * @fence: the current fence >>> - * @seqno: the sequence number (syncpt) of the fence within the chain >>> * >>> * Initialize a new chain node and either start a new chain or add >>> the node to >>> * the existing chain of the previous fence. >> > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" 2020-06-25 12:34 ` [Intel-gfx] " Lionel Landwerlin ` (2 preceding siblings ...) (?) @ 2020-06-26 14:15 ` Patchwork -1 siblings, 0 replies; 41+ messages in thread From: Patchwork @ 2020-06-26 14:15 UTC (permalink / raw) To: Lionel Landwerlin; +Cc: intel-gfx == Series Details == Series: series starting with [1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" URL : https://patchwork.freedesktop.org/series/78819/ State : success == Summary == CI Bug Log - changes from CI_DRM_8667 -> Patchwork_18025 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18025/index.html Known issues ------------ Here are the changes found in Patchwork_18025 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@core_auth@basic-auth: - fi-byt-n2820: [PASS][1] -> [DMESG-WARN][2] ([i915#1982]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8667/fi-byt-n2820/igt@core_auth@basic-auth.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18025/fi-byt-n2820/igt@core_auth@basic-auth.html * igt@gem_exec_suspend@basic-s0: - fi-apl-guc: [PASS][3] -> [INCOMPLETE][4] ([i915#1242]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8667/fi-apl-guc/igt@gem_exec_suspend@basic-s0.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18025/fi-apl-guc/igt@gem_exec_suspend@basic-s0.html * igt@i915_hangman@error-state-basic: - fi-tgl-y: [PASS][5] -> [DMESG-WARN][6] ([i915#402]) +1 similar issue [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8667/fi-tgl-y/igt@i915_hangman@error-state-basic.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18025/fi-tgl-y/igt@i915_hangman@error-state-basic.html * igt@i915_module_load@reload: - fi-icl-y: [PASS][7] -> [DMESG-WARN][8] ([i915#1982]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8667/fi-icl-y/igt@i915_module_load@reload.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18025/fi-icl-y/igt@i915_module_load@reload.html * igt@i915_pm_rpm@basic-pci-d3-state: - fi-bsw-kefka: [PASS][9] -> [DMESG-WARN][10] ([i915#1982]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8667/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18025/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html #### Possible fixes #### * igt@prime_self_import@basic-with_two_bos: - fi-tgl-y: [DMESG-WARN][11] ([i915#402]) -> [PASS][12] +2 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8667/fi-tgl-y/igt@prime_self_import@basic-with_two_bos.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18025/fi-tgl-y/igt@prime_self_import@basic-with_two_bos.html #### Warnings #### * igt@kms_flip@basic-flip-vs-dpms@a-dp1: - fi-kbl-x1275: [DMESG-WARN][13] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][14] ([i915#62] / [i915#92]) +2 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8667/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-dpms@a-dp1.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18025/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-dpms@a-dp1.html * igt@kms_force_connector_basic@force-edid: - fi-kbl-x1275: [DMESG-WARN][15] ([i915#62] / [i915#92]) -> [DMESG-WARN][16] ([i915#62] / [i915#92] / [i915#95]) +5 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8667/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18025/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: - fi-kbl-x1275: [DMESG-WARN][17] ([i915#1982] / [i915#62] / [i915#92]) -> [DMESG-WARN][18] ([i915#62] / [i915#92]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8667/fi-kbl-x1275/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18025/fi-kbl-x1275/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [i915#1242]: https://gitlab.freedesktop.org/drm/intel/issues/1242 [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402 [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62 [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92 [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95 Participating hosts (46 -> 39) ------------------------------ Missing (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus Build changes ------------- * Linux: CI_DRM_8667 -> Patchwork_18025 CI-20190529: 20190529 CI_DRM_8667: 57a1fc457c260002189382a406e920465d540d53 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5718: af1ef32bfae90bcdbaf1b5d84c61ff4e04368505 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_18025: 9d22b3c5b08feeea9b043fbb36c1868b78ffa7ab @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 9d22b3c5b08f dma-buf: fix dma-fence-chain out of order test 3a63b08bb2b4 Revert "dma-buf: Report signaled links inside dma-fence-chain" == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18025/index.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2020-07-02 12:57 UTC | newest] Thread overview: 41+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-06-25 12:34 [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" Lionel Landwerlin 2020-06-25 12:34 ` [Intel-gfx] " Lionel Landwerlin 2020-06-25 12:34 ` [PATCH 2/2] dma-buf: fix dma-fence-chain out of order test Lionel Landwerlin 2020-06-25 12:34 ` [Intel-gfx] " Lionel Landwerlin 2020-06-25 12:44 ` Christian König 2020-06-25 12:44 ` [Intel-gfx] " Christian König 2020-06-25 13:18 ` Chris Wilson 2020-06-25 13:18 ` Chris Wilson 2020-06-25 13:23 ` Lionel Landwerlin 2020-06-25 13:23 ` Lionel Landwerlin 2020-06-25 13:47 ` Chris Wilson 2020-06-25 13:47 ` Chris Wilson 2020-06-25 13:56 ` Lionel Landwerlin 2020-06-25 13:56 ` Lionel Landwerlin 2020-06-25 13:59 ` Daniel Vetter 2020-06-25 13:59 ` Daniel Vetter 2020-07-02 12:57 ` Christian König 2020-07-02 12:57 ` Christian König 2020-06-25 12:43 ` [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain" Christian König 2020-06-25 12:43 ` [Intel-gfx] " Christian König 2020-06-25 17:13 ` Dave Airlie 2020-06-25 17:13 ` Dave Airlie 2020-06-25 19:27 ` Jani Nikula 2020-06-25 19:27 ` Jani Nikula 2020-06-25 19:54 ` Daniel Vetter 2020-06-25 19:54 ` Daniel Vetter 2020-06-26 4:43 ` Sumit Semwal 2020-06-26 4:43 ` Sumit Semwal 2020-06-26 7:03 ` Christian König 2020-06-26 7:03 ` Christian König 2020-06-26 12:39 ` Daniel Vetter 2020-06-26 12:39 ` Daniel Vetter 2020-06-26 15:34 ` Sumit Semwal 2020-06-26 15:34 ` Sumit Semwal 2020-06-26 1:36 ` Dave Airlie 2020-06-26 1:36 ` Dave Airlie 2020-07-02 8:26 ` Lionel Landwerlin 2020-07-02 8:26 ` [Intel-gfx] " Lionel Landwerlin 2020-07-02 8:28 ` Christian König 2020-07-02 8:28 ` [Intel-gfx] " Christian König 2020-06-26 14:15 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.