It is normal that binary syncobj replaces the underlying fence.
Signed-off-by: Chunming Zhou david1.zhou@amd.com --- drivers/gpu/drm/drm_syncobj.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 929f7c64f9a2..bc7ec1679e4d 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -151,9 +151,6 @@ void drm_syncobj_add_point(struct drm_syncobj *syncobj, spin_lock(&syncobj->lock);
prev = drm_syncobj_fence_get(syncobj); - /* You are adding an unorder point to timeline, which could cause payload returned from query_ioctl is 0! */ - if (prev && prev->seqno >= point) - DRM_ERROR("You are adding an unorder point to timeline!\n"); dma_fence_chain_init(chain, prev, fence, point); rcu_assign_pointer(syncobj->fence, &chain->base);
On Mon, Jul 29, 2019 at 04:20:39PM +0800, Chunming Zhou wrote:
It is normal that binary syncobj replaces the underlying fence.
Signed-off-by: Chunming Zhou david1.zhou@amd.com
Do we hit this with one of the syncobj igts? -Daniel
On Tue, Jul 30, 2019 at 05:31:26PM +0800, zhoucm1 wrote:
Looks like lionel has something, maybe help review that? -Daniel
Just had a few exchanges with Chris about this. Chris suggests that if we're about to add a point to the timeline in an unordered fashion, actually better not add it at all.
What's your take on this? I'm fine with this, but rather than add another variant of add_point() maybe we change change the existing.
Thanks,
-Lionel
On 29/07/2019 11:20, Chunming Zhou wrote:
Am 01.08.19 um 15:45 schrieb Lionel Landwerlin:
That is a clear NAK. See not adding a point at all means we lose some synchronization and that is not something we can do here.
In other words syncing to much if userspace does something nasty is ok and defensive programmed, but not syncing at all could have unforeseen consequences.
Another idea would be to add the fence, but also set an error flag and deny any further signaling on that syncobj.
Regards, Christian.
On Thu, Aug 1, 2019 at 9:05 AM Koenig, Christian Christian.Koenig@amd.com wrote:
So if process A signals 7, process B detects that and signals 3 and then process A tries to insert something which waits on 7 and signals 8, what happens? My understanding is that it "breaks" the timeline and so, from the perspective of process A, its signal operation on 7 is gone and it's attempt to wait on 7 will either -EINVAL because the kernel can't find the time point or else just sit there. Am I understanding this correctly? If so, it sounds more like an attack vector than defensive programming to me.
Yes, more syncornization is generally better than less. However, if you're screwing up your syncronization from userspace and getting wrong rendering results, that's your fault. If you're causing your compositor to suddenly start seeing -EINVAL which gets turned into VK_ERROR_DEVICE_LOST, that's a lot worse IMO. I'm not saying that we shouldn't try to be robust in this case; I'm just concerned that the suggest solution isn't.
--Jason
dri-devel@lists.freedesktop.org
-
Chunming Zhou -
Daniel Vetter -
Jason Ekstrand -
Koenig, Christian -
Lionel Landwerlin -
zhoucm1