* [RFC] i915: Add fence fds to execbuffer2 uapi @ 2016-06-27 17:51 Chad Versace 2016-06-27 20:18 ` Chad Versace 2016-06-28 5:41 ` ✓ Ro.CI.BAT: success for " Patchwork 0 siblings, 2 replies; 7+ messages in thread From: Chad Versace @ 2016-06-27 17:51 UTC (permalink / raw) To: intel-gfx; +Cc: Chad Versace Let the hight 32 bits of drm_i915_gem_execbuffer2::rsvd1 contain an input and/or output fence fd, whose presence is controlled by flags. Also add I915_PARAM_HAS_FENCE_FD. Signed-off-by: Chad Versace <chad.versace@intel.com> --- include/uapi/drm/i915_drm.h | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index c17d63d..6f26b79 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -361,6 +361,7 @@ typedef struct drm_i915_irq_wait { #define I915_PARAM_HAS_GPU_RESET 35 #define I915_PARAM_HAS_RESOURCE_STREAMER 36 #define I915_PARAM_HAS_EXEC_SOFTPIN 37 +#define I915_PARAM_HAS_FENCE_FD 38 typedef struct drm_i915_getparam { __s32 param; @@ -742,7 +743,17 @@ struct drm_i915_gem_execbuffer2 { #define I915_EXEC_CONSTANTS_ABSOLUTE (1<<6) #define I915_EXEC_CONSTANTS_REL_SURFACE (2<<6) /* gen4/5 only */ __u64 flags; - __u64 rsvd1; /* now used for context info */ + + /* The low word (bits 0:31) contains the context id. + * + * The high word (bits 32:63) contains an optional fence fd. If flag + * I915_EXEC_FENCE_FD_IN is set, then the high word is an input fence + * fd. The batch will not begin execution before the input fence + * signals. If flag I915_EXEC_FENCE_FD_OUT is set, then an output + * fence fd is returned in the high word. The output fence will signal + * after the batch completes execution. It is legal to set both flags. + */ + __u64 rsvd1; __u64 rsvd2; }; @@ -788,7 +799,10 @@ struct drm_i915_gem_execbuffer2 { */ #define I915_EXEC_RESOURCE_STREAMER (1<<15) -#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_RESOURCE_STREAMER<<1) +#define I915_EXEC_FENCE_FD_IN (1<<16) +#define I915_EXEC_FENCE_FD_OUT (1<<17) + +#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_FENCE_FD_OUT<<1) #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) #define i915_execbuffer2_set_context_id(eb2, context) \ @@ -796,6 +810,12 @@ struct drm_i915_gem_execbuffer2 { #define i915_execbuffer2_get_context_id(eb2) \ ((eb2).rsvd1 & I915_EXEC_CONTEXT_ID_MASK) +#define I915_EXEC_FENCE_FD_MASK (0xffffffff00000000) +#define i915_execbuffer2_set_fence_fd(eb2, fence_fd) \ + ((eb2).rsvd2 = (fence_fd) & I915_EXEC_FENCE_MASK) +#define i915_execbuffer2_get_fence_fd(eb2, fence_fd) \ + ((eb2).rsvd2 & I915_EXEC_FENCE_FD_MASK) + struct drm_i915_gem_pin { /** Handle of the buffer to be pinned. */ __u32 handle; -- 2.9.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC] i915: Add fence fds to execbuffer2 uapi 2016-06-27 17:51 [RFC] i915: Add fence fds to execbuffer2 uapi Chad Versace @ 2016-06-27 20:18 ` Chad Versace 2016-06-27 22:06 ` Chris Wilson 2016-06-28 17:06 ` John Harrison 2016-06-28 5:41 ` ✓ Ro.CI.BAT: success for " Patchwork 1 sibling, 2 replies; 7+ messages in thread From: Chad Versace @ 2016-06-27 20:18 UTC (permalink / raw) To: intel-gfx On Mon 27 Jun 2016, Chad Versace wrote: > Let the hight 32 bits of drm_i915_gem_execbuffer2::rsvd1 contain an > input and/or output fence fd, whose presence is controlled by flags. > Also add I915_PARAM_HAS_FENCE_FD. > > Signed-off-by: Chad Versace <chad.versace@intel.com> > --- > include/uapi/drm/i915_drm.h | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) Oops. git-send-email stripped the notes to the patch. Here's the notes: This RFC proposes a uapi that integrates execbuf with Android sync fds. Of course, this is *only* an RFC because other devs are working on the i915 internals, and this patch depends on that work. Why am I sending an RFC this early? I will soon begin prototyping Intel's Mesa implementation of EGL_ANDROID_native_fence_sync, and that prototype will be easier to write if I have a rough expectation of i915's eventual fence fd uapi. Please provide feedback: Does this roughly look like the uapi that the i915 devs expect? (Pardon any stupidity in the patch. It's my first non-trivial kernel patch). _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] i915: Add fence fds to execbuffer2 uapi 2016-06-27 20:18 ` Chad Versace @ 2016-06-27 22:06 ` Chris Wilson 2016-06-29 18:28 ` Chad Versace 2016-06-28 17:06 ` John Harrison 1 sibling, 1 reply; 7+ messages in thread From: Chris Wilson @ 2016-06-27 22:06 UTC (permalink / raw) To: Chad Versace, intel-gfx On Mon, Jun 27, 2016 at 01:18:59PM -0700, Chad Versace wrote: > On Mon 27 Jun 2016, Chad Versace wrote: > > Let the hight 32 bits of drm_i915_gem_execbuffer2::rsvd1 contain an > > input and/or output fence fd, whose presence is controlled by flags. > > Also add I915_PARAM_HAS_FENCE_FD. > > > > Signed-off-by: Chad Versace <chad.versace@intel.com> > > --- > > include/uapi/drm/i915_drm.h | 24 ++++++++++++++++++++++-- > > 1 file changed, 22 insertions(+), 2 deletions(-) > > Oops. git-send-email stripped the notes to the patch. Here's the notes: > > This RFC proposes a uapi that integrates execbuf with Android sync fds. Of > course, this is *only* an RFC because other devs are working on the i915 > internals, and this patch depends on that work. Why not just use the earlier patches for the uAPI as well? > Why am I sending an RFC this early? I will soon begin prototyping Intel's > Mesa implementation of EGL_ANDROID_native_fence_sync, and that prototype will > be easier to write if I have a rough expectation of i915's eventual fence fd > uapi. > > Please provide feedback: Does this roughly look like the uapi that the i915 > devs expect? Not quite. You have to use separate in/out dwords (i.e. rsvd2) in order to ensure that we don't overwite the in-fence when dealing with error paths (i.e. so that userspace can feed in the same execbuf parameters following EINTR, and you don't have confusion between in/out parameters). You have to also mark the ioctl as writing the new structures which is an ABI break and so requires a new identifier (otherwise you break userspace passing in the args from read-only memory). Playind devil's advocate, an alternative to every driver implementing their own fence extension for execbuf/cmdsubmit would be to add support for explicit sync_fences to be added via dmabuf. (Instead of setting the fence on the execbuf, you would set the fence on the batch buffer obj, or surface of interest - though for CreateSync, it would have to be the batch. Extracting the fence is then supplied by querying the batch buffer dmabuf. It's not as explicit, but I suspect such uABI will be added to dmabuf and will be required to be supported in the driver to handle implicit fencing between PRIME anyway.) -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] i915: Add fence fds to execbuffer2 uapi 2016-06-27 22:06 ` Chris Wilson @ 2016-06-29 18:28 ` Chad Versace 0 siblings, 0 replies; 7+ messages in thread From: Chad Versace @ 2016-06-29 18:28 UTC (permalink / raw) To: Chris Wilson, intel-gfx On Mon 27 Jun 2016, Chris Wilson wrote: > On Mon, Jun 27, 2016 at 01:18:59PM -0700, Chad Versace wrote: > > On Mon 27 Jun 2016, Chad Versace wrote: > > > Let the hight 32 bits of drm_i915_gem_execbuffer2::rsvd1 contain an > > > input and/or output fence fd, whose presence is controlled by flags. > > > Also add I915_PARAM_HAS_FENCE_FD. > > > > > > Signed-off-by: Chad Versace <chad.versace@intel.com> > > > --- > > > include/uapi/drm/i915_drm.h | 24 ++++++++++++++++++++++-- > > > 1 file changed, 22 insertions(+), 2 deletions(-) > > > > Oops. git-send-email stripped the notes to the patch. Here's the notes: > > > > This RFC proposes a uapi that integrates execbuf with Android sync fds. Of > > course, this is *only* an RFC because other devs are working on the i915 > > internals, and this patch depends on that work. > > Why not just use the earlier patches for the uAPI as well? I examined all the patches that John Harrison sent to the list, and they contained no uapi. Is there another patchset, from someone else (possibly you), that proposes a uapi? > > Why am I sending an RFC this early? I will soon begin prototyping Intel's > > Mesa implementation of EGL_ANDROID_native_fence_sync, and that prototype will > > be easier to write if I have a rough expectation of i915's eventual fence fd > > uapi. > > > > Please provide feedback: Does this roughly look like the uapi that the i915 > > devs expect? > > Not quite. You have to use separate in/out dwords (i.e. rsvd2) in order > to ensure that we don't overwite the in-fence when dealing with error > paths (i.e. so that userspace can feed in the same execbuf parameters > following EINTR, and you don't have confusion between in/out parameters). Right. I forgot about resubmission on EINTR. > You have to also mark the ioctl as writing the new structures which is an > ABI break and so requires a new identifier (otherwise you break userspace > passing in the args from read-only memory). Thanks for explaining the obvious ABI break to this kernel noob. > Playind devil's advocate, an alternative to every driver implementing > their own fence extension for execbuf/cmdsubmit would be to add support > for explicit sync_fences to be added via dmabuf. (Instead of setting the > fence on the execbuf, you would set the fence on the batch buffer obj, > or surface of interest - though for CreateSync, it would have to be the > batch. Extracting the fence is then supplied by querying the batch buffer > dmabuf. It's not as explicit, but I suspect such uABI will be added to > dmabuf and will be required to be supported in the driver to handle > implicit fencing between PRIME anyway.) If I were arguing with the devil, I would claim that uapi that attached fence fds to dma_bufs seems more elegant, but API that attaches fence fds to batch bos prevents an optimized use case in Vulkan's submission model. In Vulkan, the user submits work by compiling a VkCommandBuffer (which is closely related to Intel's batch bo) and then submitting it to a VkQueue (which is related to a GPU ring). For repetive rendering tasks, the Vulkan API encourages the user to re-use the compiled VkCommandBuffer by re-submitting it to the VkQueue. When the user resubmits a VkCommandBuffer, the Vulkan spec doesn't *require* the driver to resubmit the same exact batch buffer; but that's the spec's *intent*. And Mesa does that today; when the user compiles a VkCommandBuffer, we compile the batch buffer immediately, and resubmit that exact batch buffer each time the user submits the VkCommandBuffer. Vulkan doesn't use fence fds today, but it will someday. If the kernel doesn't add fence fds to the execbuffer ioctl, but instead requires that the fences be associated with a batch buffer, then that would prevents the natural batch buffer re-use in Vulkan. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] i915: Add fence fds to execbuffer2 uapi 2016-06-27 20:18 ` Chad Versace 2016-06-27 22:06 ` Chris Wilson @ 2016-06-28 17:06 ` John Harrison 2016-06-29 18:29 ` Chad Versace 1 sibling, 1 reply; 7+ messages in thread From: John Harrison @ 2016-06-28 17:06 UTC (permalink / raw) To: Chad Versace, intel-gfx On 27/06/2016 21:18, Chad Versace wrote: > On Mon 27 Jun 2016, Chad Versace wrote: >> Let the hight 32 bits of drm_i915_gem_execbuffer2::rsvd1 contain an >> input and/or output fence fd, whose presence is controlled by flags. >> Also add I915_PARAM_HAS_FENCE_FD. >> >> Signed-off-by: Chad Versace <chad.versace@intel.com> >> --- >> include/uapi/drm/i915_drm.h | 24 ++++++++++++++++++++++-- >> 1 file changed, 22 insertions(+), 2 deletions(-) > Oops. git-send-email stripped the notes to the patch. Here's the notes: > > This RFC proposes a uapi that integrates execbuf with Android sync fds. Of > course, this is *only* an RFC because other devs are working on the i915 > internals, and this patch depends on that work. > > Why am I sending an RFC this early? I will soon begin prototyping Intel's > Mesa implementation of EGL_ANDROID_native_fence_sync, and that prototype will > be easier to write if I have a rough expectation of i915's eventual fence fd > uapi. > > Please provide feedback: Does this roughly look like the uapi that the i915 > devs expect? > > (Pardon any stupidity in the patch. It's my first non-trivial kernel patch). > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx Not sure who you mean by 'the devs' but I am the one who implemented all of this for our Android releases. I have a patch series to add full native sync support to the i915 driver. It is currently used in our Android trees so has been tested quite extensively. It was also going through code review on the mailing list a while ago until it was decided that the de-staging of the Android code should be done by external people (currently in progress). The latest set of patches (including changes from feedback about rsvd fields) never actually got posted to the mailing list due to the above issue with de-staging. I have just updated my FDO git account with them instead. The kernel patch is: https://cgit.freedesktop.org/~johnharr/scheduler/commit/?h=all&id=b7cd5e85edce4af9d7d4c34bb640cd49e31236a8 The userland LibDRM patch is: https://cgit.freedesktop.org/~johnharr/scheduler/commit/?h=LibDRM&id=f11b2d577904b1a096d5b36384a9cc83ba51cbb8 John. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] i915: Add fence fds to execbuffer2 uapi 2016-06-28 17:06 ` John Harrison @ 2016-06-29 18:29 ` Chad Versace 0 siblings, 0 replies; 7+ messages in thread From: Chad Versace @ 2016-06-29 18:29 UTC (permalink / raw) To: John Harrison; +Cc: intel-gfx On Tue 28 Jun 2016, John Harrison wrote: > The latest set of patches (including changes from feedback about rsvd > fields) never actually got posted to the mailing list due to the above issue > with de-staging. I have just updated my FDO git account with them instead. > The kernel patch is: > https://cgit.freedesktop.org/~johnharr/scheduler/commit/?h=all&id=b7cd5e85edce4af9d7d4c34bb640cd49e31236a8 > > The userland LibDRM patch is: > https://cgit.freedesktop.org/~johnharr/scheduler/commit/?h=LibDRM&id=f11b2d577904b1a096d5b36384a9cc83ba51cbb8 Thanks for the sharing the code. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* ✓ Ro.CI.BAT: success for i915: Add fence fds to execbuffer2 uapi 2016-06-27 17:51 [RFC] i915: Add fence fds to execbuffer2 uapi Chad Versace 2016-06-27 20:18 ` Chad Versace @ 2016-06-28 5:41 ` Patchwork 1 sibling, 0 replies; 7+ messages in thread From: Patchwork @ 2016-06-28 5:41 UTC (permalink / raw) To: Chad Versace; +Cc: intel-gfx == Series Details == Series: i915: Add fence fds to execbuffer2 uapi URL : https://patchwork.freedesktop.org/series/9196/ State : success == Summary == Series 9196v1 i915: Add fence fds to execbuffer2 uapi http://patchwork.freedesktop.org/api/1.0/series/9196/revisions/1/mbox Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-c: dmesg-warn -> SKIP (ro-bdw-i5-5250u) fi-hsw-i7-4770k total:229 pass:194 dwarn:0 dfail:0 fail:2 skip:33 fi-kbl-qkkr total:229 pass:160 dwarn:29 dfail:0 fail:0 skip:40 fi-skl-i5-6260u total:229 pass:202 dwarn:0 dfail:0 fail:2 skip:25 fi-skl-i7-6700k total:229 pass:188 dwarn:0 dfail:0 fail:2 skip:39 fi-snb-i7-2600 total:229 pass:174 dwarn:0 dfail:0 fail:2 skip:53 ro-bdw-i5-5250u total:229 pass:202 dwarn:3 dfail:1 fail:2 skip:21 ro-bdw-i7-5557U total:229 pass:202 dwarn:1 dfail:1 fail:2 skip:23 ro-bdw-i7-5600u total:229 pass:190 dwarn:0 dfail:1 fail:0 skip:38 ro-bsw-n3050 total:229 pass:177 dwarn:0 dfail:1 fail:2 skip:49 ro-byt-n2820 total:229 pass:178 dwarn:0 dfail:1 fail:5 skip:45 ro-hsw-i3-4010u total:229 pass:195 dwarn:0 dfail:1 fail:2 skip:31 ro-hsw-i7-4770r total:229 pass:195 dwarn:0 dfail:1 fail:2 skip:31 ro-ilk-i7-620lm total:229 pass:155 dwarn:0 dfail:1 fail:3 skip:70 ro-ilk1-i5-650 total:224 pass:155 dwarn:0 dfail:1 fail:3 skip:65 ro-ivb-i7-3770 total:229 pass:186 dwarn:0 dfail:1 fail:2 skip:40 ro-ivb2-i7-3770 total:229 pass:190 dwarn:0 dfail:1 fail:2 skip:36 ro-skl3-i5-6260u total:229 pass:206 dwarn:1 dfail:1 fail:2 skip:19 ro-snb-i7-2620M total:229 pass:179 dwarn:0 dfail:1 fail:1 skip:48 Results at /archive/results/CI_IGT_test/RO_Patchwork_1315/ 13bc84e drm-intel-nightly: 2016y-06m-27d-21h-27m-04s UTC integration manifest 13ba40e i915: Add fence fds to execbuffer2 uapi _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-06-29 18:30 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-06-27 17:51 [RFC] i915: Add fence fds to execbuffer2 uapi Chad Versace 2016-06-27 20:18 ` Chad Versace 2016-06-27 22:06 ` Chris Wilson 2016-06-29 18:28 ` Chad Versace 2016-06-28 17:06 ` John Harrison 2016-06-29 18:29 ` Chad Versace 2016-06-28 5:41 ` ✓ Ro.CI.BAT: success for " 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.