在 2019/8/8 22:01, Lionel Landwerlin 写道: > On 08/08/2019 16:42, Chunming Zhou wrote: >> No, I just see your comment "The next vkQueueSubmit() >> waiting on a the syncobj will read the sideband payload and wait for a >> fence chain element with a seqno superior or equal to the sideband >> payload value to be added into the fence chain and use that fence to >> trigger the submission on the kernel driver. ". > > > That was meant to say wait on the chain fence to reach the sideband > payload value. > > It a bit confusing but I can't see any other way to word it. > > >> In that, you mentioned wait for sideband. >> So I want to know we how to use that, maybe I miss something in previous >> discussing thread. > > > In QueueSubmit(), we start by reading the sideband payloads : > https://gitlab.freedesktop.org/llandwerlin/mesa/blob/review/anv-timeline_semaphore_prep/src/intel/vulkan/anv_queue.c#L655 > > Then build everything for the submission and hand it over to the > submission thread. > > Instead of the just waiting on the timeline semaphore values, we also > wait on the binary semaphore sideband payload values. Waiting on timeline values is when finding fence in kernel. But I don't see how to wait/block in kernel when finding fence for binary sideband payload values. -David > > Finally before exiting the QueueSubmit() call, we bump the sideband > payloads of all the binary semaphores have have been signaled : > https://gitlab.freedesktop.org/llandwerlin/mesa/blob/review/anv-timeline_semaphore_prep/src/intel/vulkan/anv_queue.c#L806 > > > Whoever calls QueueSubmit() after that will pickup the new sideband > payload values to wait on. > > > -Lionel > > > >> >> -DAvid >> >> >> 在 2019/8/8 21:38, Lionel Landwerlin 写道: >>> Interesting question :) >>> >>> I didn't see any usecase for that. >>> This sideband payload value is used for a wait so waiting on the wait >>> value for another wait is bit meta :) >>> >>> Do you have a use case for that? >>> >>> -Lionel >>> >>> On 08/08/2019 16:23, Chunming Zhou wrote: >>>> The propursal is fine with me. >>>> >>>> one question: >>>> >>>> how to wait sideband payload? Following patch will show that? >>>> >>>> -David >>>> >>>> 在 2019/8/8 21:14, Lionel Landwerlin 写道: >>>>> The Vulkan timeline semaphores allow signaling to happen on the point >>>>> of the timeline without all of the its dependencies to be created. >>>>> >>>>> The current 2 implementations (AMD/Intel) of the Vulkan spec on >>>>> top of >>>>> the Linux kernel are using a thread to wait on the dependencies of a >>>>> given point to materialize and delay actual submission to the kernel >>>>> driver until the wait completes. >>>>> >>>>> If a binary semaphore is submitted for signaling along the side of a >>>>> timeline semaphore waiting for completion that means that the drm >>>>> syncobj associated with that binary semaphore will not have a DMA >>>>> fence associated with it by the time vkQueueSubmit() returns. This >>>>> and >>>>> the fact that a binary semaphore can be signaled and unsignaled as >>>>> before its DMA fences materialize mean that we cannot just rely on >>>>> the >>>>> fence within the syncobj but we also need a sideband payload >>>>> verifying >>>>> that the fence in the syncobj matches the last submission from the >>>>> Vulkan API point of view. >>>>> >>>>> This change adds a sideband payload that is incremented with signaled >>>>> syncobj when vkQueueSubmit() is called. The next vkQueueSubmit() >>>>> waiting on a the syncobj will read the sideband payload and wait >>>>> for a >>>>> fence chain element with a seqno superior or equal to the sideband >>>>> payload value to be added into the fence chain and use that fence to >>>>> trigger the submission on the kernel driver. >>>>> >>>>> v2: Use a separate ioctl to get/set the sideband value (Christian) >>>>> >>>>> v3: Use 2 ioctls for get/set (Christian) >>>>> >>>>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx> >>>>> Cc: Christian Koenig <Christian.Koenig@xxxxxxx> >>>>> Cc: Jason Ekstrand <jason@xxxxxxxxxxxxxx> >>>>> Cc: David(ChunMing) Zhou <David1.Zhou@xxxxxxx> >>>>> --- >>>>> drivers/gpu/drm/drm_internal.h | 4 ++ >>>>> drivers/gpu/drm/drm_ioctl.c | 5 +++ >>>>> drivers/gpu/drm/drm_syncobj.c | 79 >>>>> +++++++++++++++++++++++++++++++++- >>>>> include/drm/drm_syncobj.h | 9 ++++ >>>>> include/uapi/drm/drm.h | 2 + >>>>> 5 files changed, 98 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/drm_internal.h >>>>> b/drivers/gpu/drm/drm_internal.h >>>>> index 51a2055c8f18..0c9736199df0 100644 >>>>> --- a/drivers/gpu/drm/drm_internal.h >>>>> +++ b/drivers/gpu/drm/drm_internal.h >>>>> @@ -208,6 +208,10 @@ int drm_syncobj_timeline_signal_ioctl(struct >>>>> drm_device *dev, void *data, >>>>> struct drm_file *file_private); >>>>> int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, >>>>> struct drm_file *file_private); >>>>> +int drm_syncobj_get_sideband_ioctl(struct drm_device *dev, void >>>>> *data, >>>>> + struct drm_file *file_private); >>>>> +int drm_syncobj_set_sideband_ioctl(struct drm_device *dev, void >>>>> *data, >>>>> + struct drm_file *file_private); >>>>> /* drm_framebuffer.c */ >>>>> void drm_framebuffer_print_info(struct drm_printer *p, unsigned >>>>> int indent, >>>>> diff --git a/drivers/gpu/drm/drm_ioctl.c >>>>> b/drivers/gpu/drm/drm_ioctl.c >>>>> index f675a3bb2c88..48500bf62801 100644 >>>>> --- a/drivers/gpu/drm/drm_ioctl.c >>>>> +++ b/drivers/gpu/drm/drm_ioctl.c >>>>> @@ -703,6 +703,11 @@ static const struct drm_ioctl_desc drm_ioctls[] >>>>> = { >>>>> DRM_RENDER_ALLOW), >>>>> DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, >>>>> drm_syncobj_query_ioctl, >>>>> DRM_RENDER_ALLOW), >>>>> + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_GET_SIDEBAND, >>>>> drm_syncobj_get_sideband_ioctl, >>>>> + DRM_RENDER_ALLOW), >>>>> + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SET_SIDEBAND, >>>>> drm_syncobj_set_sideband_ioctl, >>>>> + DRM_RENDER_ALLOW), >>>>> + >>>>> DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, >>>>> drm_crtc_get_sequence_ioctl, 0), >>>>> DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, >>>>> drm_crtc_queue_sequence_ioctl, 0), >>>>> DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, >>>>> drm_mode_create_lease_ioctl, DRM_MASTER), >>>>> diff --git a/drivers/gpu/drm/drm_syncobj.c >>>>> b/drivers/gpu/drm/drm_syncobj.c >>>>> index b927e482e554..c90ef20b9242 100644 >>>>> --- a/drivers/gpu/drm/drm_syncobj.c >>>>> +++ b/drivers/gpu/drm/drm_syncobj.c >>>>> @@ -1150,8 +1150,10 @@ drm_syncobj_reset_ioctl(struct drm_device >>>>> *dev, void *data, >>>>> if (ret < 0) >>>>> return ret; >>>>> - for (i = 0; i < args->count_handles; i++) >>>>> + for (i = 0; i < args->count_handles; i++) { >>>>> drm_syncobj_replace_fence(syncobjs[i], NULL); >>>>> + syncobjs[i]->sideband_payload = 0; >>>>> + } >>>>> drm_syncobj_array_free(syncobjs, args->count_handles); >>>>> @@ -1321,6 +1323,81 @@ int drm_syncobj_query_ioctl(struct >>>>> drm_device *dev, void *data, >>>>> if (ret) >>>>> break; >>>>> } >>>>> + >>>>> + drm_syncobj_array_free(syncobjs, args->count_handles); >>>>> + >>>>> + return ret; >>>>> +} >>>>> + >>>>> +int drm_syncobj_get_sideband_ioctl(struct drm_device *dev, void >>>>> *data, >>>>> + struct drm_file *file_private) >>>>> +{ >>>>> + struct drm_syncobj_timeline_array *args = data; >>>>> + struct drm_syncobj **syncobjs; >>>>> + uint64_t __user *points = u64_to_user_ptr(args->points); >>>>> + uint32_t i; >>>>> + int ret; >>>>> + >>>>> + if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE)) >>>>> + return -EOPNOTSUPP; >>>>> + >>>>> + if (args->pad != 0) >>>>> + return -EINVAL; >>>>> + >>>>> + if (args->count_handles == 0) >>>>> + return -EINVAL; >>>>> + >>>>> + ret = drm_syncobj_array_find(file_private, >>>>> + u64_to_user_ptr(args->handles), >>>>> + args->count_handles, >>>>> + &syncobjs); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + for (i = 0; i < args->count_handles; i++) { >>>>> + copy_to_user(&points[i], &syncobjs[i]->sideband_payload, >>>>> sizeof(uint64_t)); >>>>> + ret = ret ? -EFAULT : 0; >>>>> + if (ret) >>>>> + break; >>>>> + } >>>>> + >>>>> + drm_syncobj_array_free(syncobjs, args->count_handles); >>>>> + >>>>> + return ret; >>>>> +} >>>>> + >>>>> +int drm_syncobj_set_sideband_ioctl(struct drm_device *dev, void >>>>> *data, >>>>> + struct drm_file *file_private) >>>>> +{ >>>>> + struct drm_syncobj_timeline_array *args = data; >>>>> + struct drm_syncobj **syncobjs; >>>>> + uint64_t __user *points = u64_to_user_ptr(args->points); >>>>> + uint32_t i; >>>>> + int ret; >>>>> + >>>>> + if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE)) >>>>> + return -EOPNOTSUPP; >>>>> + >>>>> + if (args->pad != 0) >>>>> + return -EINVAL; >>>>> + >>>>> + if (args->count_handles == 0) >>>>> + return -EINVAL; >>>>> + >>>>> + ret = drm_syncobj_array_find(file_private, >>>>> + u64_to_user_ptr(args->handles), >>>>> + args->count_handles, >>>>> + &syncobjs); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + for (i = 0; i < args->count_handles; i++) { >>>>> + copy_from_user(&syncobjs[i]->sideband_payload, &points[i], >>>>> sizeof(uint64_t)); >>>>> + ret = ret ? -EFAULT : 0; >>>>> + if (ret) >>>>> + break; >>>>> + } >>>>> + >>>>> drm_syncobj_array_free(syncobjs, args->count_handles); >>>>> return ret; >>>>> diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h >>>>> index 6cf7243a1dc5..b587b8e07547 100644 >>>>> --- a/include/drm/drm_syncobj.h >>>>> +++ b/include/drm/drm_syncobj.h >>>>> @@ -61,6 +61,15 @@ struct drm_syncobj { >>>>> * @file: A file backing for this syncobj. >>>>> */ >>>>> struct file *file; >>>>> + /** >>>>> + * @sideband_payload: A 64bit side band payload. >>>>> + * >>>>> + * We use the sideband payload value to wait on binary syncobj >>>>> fences >>>>> + * to materialize. It is a reservation mechanism for the >>>>> signaler to >>>>> + * express that at some point in the future a dma fence with >>>>> the same >>>>> + * seqno will be put into the syncobj. >>>>> + */ >>>>> + u64 sideband_payload; >>>>> }; >>>>> void drm_syncobj_free(struct kref *kref); >>>>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h >>>>> index 8a5b2f8f8eb9..cffdc6c9772c 100644 >>>>> --- a/include/uapi/drm/drm.h >>>>> +++ b/include/uapi/drm/drm.h >>>>> @@ -946,6 +946,8 @@ extern "C" { >>>>> #define DRM_IOCTL_SYNCOBJ_QUERY DRM_IOWR(0xCB, struct >>>>> drm_syncobj_timeline_array) >>>>> #define DRM_IOCTL_SYNCOBJ_TRANSFER DRM_IOWR(0xCC, struct >>>>> drm_syncobj_transfer) >>>>> #define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL DRM_IOWR(0xCD, >>>>> struct drm_syncobj_timeline_array) >>>>> +#define DRM_IOCTL_SYNCOBJ_GET_SIDEBAND DRM_IOR(0xCE, struct >>>>> drm_syncobj_timeline_array) >>>>> +#define DRM_IOCTL_SYNCOBJ_SET_SIDEBAND DRM_IOWR(0xCF, struct >>>>> drm_syncobj_timeline_array) >>>>> /** >>>>> * Device specific ioctls should only be in their respective >>>>> headers >>> > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel