From patchwork Wed Jul 5 17:21:22 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Ekstrand X-Patchwork-Id: 9826857 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0636F602F0 for ; Wed, 5 Jul 2017 17:21:32 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DD1B328565 for ; Wed, 5 Jul 2017 17:21:31 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D17982856D; Wed, 5 Jul 2017 17:21:31 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id E5F3828565 for ; Wed, 5 Jul 2017 17:21:29 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 70A286E4EA; Wed, 5 Jul 2017 17:21:29 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-pf0-x244.google.com (mail-pf0-x244.google.com [IPv6:2607:f8b0:400e:c00::244]) by gabe.freedesktop.org (Postfix) with ESMTPS id 78F286E4EA for ; Wed, 5 Jul 2017 17:21:28 +0000 (UTC) Received: by mail-pf0-x244.google.com with SMTP id z6so36838572pfk.3 for ; Wed, 05 Jul 2017 10:21:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jlekstrand-net.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id; bh=6bDuEVG5MV82Bw0m4nUEcyAYhJrjYcCwDROuISAFX6s=; b=LUH0h6POzIENGSpjCDu6nEDvI4y11MX+MgAMKRg7rDjb5Y4H4+eNRefi6miFYewc8Q MBbGweDSXE4FLTD3ZWqCAyhYVe87XoRY+K6bXQPu//cP3Jg34FPO8Za1V+8Q9GbzzGm3 5NSik2sBcpBBdbUBIpZerIOtOpvoTZOy6lKc+tYzReIJsJ7keijz9qTx7qPVA31lVM9u sCwt7H8XtruIC8mYbjeaDeoQZ++HBwtyLLDlDF1zbdVypcpLKY0fr9yQABcBR/0shalk 8Fkk9Mv5mIvR/63rqP81G6usgP721SJCvqNlNRDPCV7GxdhVc7GFMDHiP7xTgh4qNfxU 0+0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=6bDuEVG5MV82Bw0m4nUEcyAYhJrjYcCwDROuISAFX6s=; b=RQues8JLTGv2YbyR6a3fl5NRGau3y8JOD+I/UjndS4f23YMzieQf8a35aPLyto5vRK prv9DyfLS0tapggLieR35kZkzUuex0PAJ7RQip7I1NE3UMRKfkmZCcgJvCXIIImCn3r3 9eJyZarCJYsT6bIIL64jsh7hQY/uhSktABpzfLaU01Acb9ar6T2hrESWO6H+h/AzUkl+ QwCr7D5vVaw1fKeugJrqggemfBdDhH5OBZ7Q19+v+HbWKjzaOPFDItBTOGhivrb3s/kQ T7BMc0sG2D0jeCiTfIMd/AZk8/4Ptt2Z0RoH4TI9o7APUHkrXPA7Fr0oqW5Qp6cMy3TM 6WVw== X-Gm-Message-State: AIVw113/9A5rSWtfaOt82h8S9cXXTLzRMOoYBUP0KOiGb0IZE/x83F2N sGuQz8j5+KOzlf7qeDW6cg== X-Received: by 10.99.115.85 with SMTP id d21mr21801682pgn.8.1499275287633; Wed, 05 Jul 2017 10:21:27 -0700 (PDT) Received: from omlet.jf.intel.com ([134.134.139.83]) by smtp.gmail.com with ESMTPSA id 189sm1314639pfe.26.2017.07.05.10.21.26 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 05 Jul 2017 10:21:26 -0700 (PDT) From: Jason Ekstrand X-Google-Original-From: Jason Ekstrand To: dri-devel@lists.freedesktop.org Subject: [PATCH] i915: Add support for drm syncobjs Date: Wed, 5 Jul 2017 10:21:22 -0700 Message-Id: <1499275282-14249-1-git-send-email-jason.ekstrand@intel.com> X-Mailer: git-send-email 2.5.0.400.gff86faf Cc: Jason Ekstrand X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP This commit adds support for waiting on or signaling DRM syncobjs as part of execbuf. It does so by hijacking the currently unused cliprects pointer to instead point to an array of i915_gem_exec_fence structs which containe a DRM syncobj and a flags parameter which specifies whether to wait on it or to signal it. This implementation theoretically allows for both flags to be set in which case it waits on the dma_fence that was in the syncobj and then immediately replaces it with the dma_fence from the current execbuf. Cc: Chris Wilson --- Ideally, I'd like to get whatever kernel reviewing and/or merging is required to land the userspace bits ASAP. That said, I understand that this is my first kernel patch and it's liable to have a few rounds of review before going in. :-) The mesa branch for using this in Vulkan can be found here: https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=review/anv-syncobj drivers/gpu/drm/i915/i915_drv.c | 3 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 97 ++++++++++++++++++++++++++++-- include/uapi/drm/i915_drm.h | 30 ++++++++- 3 files changed, 121 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 9167a73..e6f7f49 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -384,6 +384,7 @@ static int i915_getparam(struct drm_device *dev, void *data, case I915_PARAM_HAS_EXEC_FENCE: case I915_PARAM_HAS_EXEC_CAPTURE: case I915_PARAM_HAS_EXEC_BATCH_FIRST: + case I915_PARAM_HAS_EXEC_FENCE_ARRAY: /* For the time being all of these are always true; * if some supported hardware does not have one of these * features this value needs to be provided from @@ -2734,7 +2735,7 @@ static struct drm_driver driver = { */ .driver_features = DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME | - DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC, + DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_SYNCOBJ, .release = i915_driver_release, .open = i915_driver_open, .lastclose = i915_driver_lastclose, diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 929f275..81b7b7b 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -33,6 +33,7 @@ #include #include +#include #include "i915_drv.h" #include "i915_gem_clflush.h" @@ -1882,8 +1883,10 @@ static bool i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec) return false; /* Kernel clipping was a DRI1 misfeature */ - if (exec->num_cliprects || exec->cliprects_ptr) - return false; + if (!(exec->flags & I915_EXEC_FENCE_ARRAY)) { + if (exec->num_cliprects || exec->cliprects_ptr) + return false; + } if (exec->DR4 == 0xffffffff) { DRM_DEBUG("UXA submitting garbage DR4, fixing up\n"); @@ -2118,12 +2121,15 @@ static int i915_gem_do_execbuffer(struct drm_device *dev, struct drm_file *file, struct drm_i915_gem_execbuffer2 *args, - struct drm_i915_gem_exec_object2 *exec) + struct drm_i915_gem_exec_object2 *exec, + struct drm_i915_gem_exec_fence *fences) { struct i915_execbuffer eb; struct dma_fence *in_fence = NULL; struct sync_file *out_fence = NULL; + struct drm_syncobj **syncobjs = NULL; int out_fence_fd = -1; + unsigned int i; int err; BUILD_BUG_ON(__EXEC_OBJECT_INTERNAL_FLAGS & @@ -2186,9 +2192,30 @@ i915_gem_do_execbuffer(struct drm_device *dev, } } + if (args->flags & I915_EXEC_FENCE_ARRAY) { + syncobjs = kvmalloc_array(args->num_cliprects, + sizeof(*syncobjs), + __GFP_NOWARN | GFP_TEMPORARY | + __GFP_ZERO); + if (syncobjs == NULL) { + DRM_DEBUG("Failed to allocate array of %d syncobjs\n", + args->num_cliprects); + err = -ENOMEM; + goto err_out_fence; + } + for (i = 0; i < args->num_cliprects; i++) { + syncobjs[i] = drm_syncobj_find(file, fences[i].handle); + if (!syncobjs[i]) { + DRM_DEBUG("Invalid syncobj handle provided\n"); + err = -EINVAL; + goto err_syncobjs; + } + } + } + err = eb_create(&eb); if (err) - goto err_out_fence; + goto err_syncobjs; GEM_BUG_ON(!eb.lut_size); @@ -2304,6 +2331,16 @@ i915_gem_do_execbuffer(struct drm_device *dev, goto err_request; } + if (args->flags & I915_EXEC_FENCE_ARRAY) { + for (i = 0; i < args->num_cliprects; i++) { + if (fences[i].flags & I915_EXEC_FENCE_WAIT) { + if (i915_gem_request_await_dma_fence(eb.request, + syncobjs[i]->fence)) + goto err_request; + } + } + } + if (out_fence_fd != -1) { out_fence = sync_file_create(&eb.request->fence); if (!out_fence) { @@ -2312,6 +2349,20 @@ i915_gem_do_execbuffer(struct drm_device *dev, } } + /* We need to add fences to syncobjs last because doing so mutates the + * syncobj and we have no good way to recover if the execbuf fails + * after this point. Fortunately, drm_syncobj_replace_fence can never + * fail. + */ + if (args->flags & I915_EXEC_FENCE_ARRAY) { + for (i = 0; i < args->num_cliprects; i++) { + if (fences[i].flags & I915_EXEC_FENCE_SIGNAL) { + drm_syncobj_replace_fence(file, syncobjs[i], + &eb.request->fence); + } + } + } + /* * Whilst this request exists, batch_obj will be on the * active_list, and so will hold the active reference. Only when this @@ -2350,6 +2401,12 @@ i915_gem_do_execbuffer(struct drm_device *dev, i915_gem_context_put(eb.ctx); err_destroy: eb_destroy(&eb); +err_syncobjs: + if (syncobjs) { + for (i = 0; i < args->num_cliprects; i++) + drm_syncobj_put(syncobjs[i]); + kvfree(syncobjs); + } err_out_fence: if (out_fence_fd != -1) put_unused_fd(out_fence_fd); @@ -2428,7 +2485,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, exec2_list[i].flags = 0; } - err = i915_gem_do_execbuffer(dev, file, &exec2, exec2_list); + err = i915_gem_do_execbuffer(dev, file, &exec2, exec2_list, NULL); if (exec2.flags & __EXEC_HAS_RELOC) { struct drm_i915_gem_exec_object __user *user_exec_list = u64_to_user_ptr(args->buffers_ptr); @@ -2460,6 +2517,7 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data, const size_t sz = sizeof(struct drm_i915_gem_exec_object2); struct drm_i915_gem_execbuffer2 *args = data; struct drm_i915_gem_exec_object2 *exec2_list; + struct drm_i915_gem_exec_fence *fence_list = NULL; int err; if (args->buffer_count < 1 || args->buffer_count > SIZE_MAX / sz - 1) { @@ -2486,7 +2544,33 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data, return -EFAULT; } - err = i915_gem_do_execbuffer(dev, file, args, exec2_list); + if (args->flags & I915_EXEC_FENCE_ARRAY) { + if (args->num_cliprects > UINT_MAX / sizeof(*exec2_list)) { + DRM_DEBUG("execbuf2 with %d fences\n", args->num_cliprects); + kvfree(exec2_list); + return -EINVAL; + } + fence_list = kvmalloc_array(args->num_cliprects, + sizeof(*fence_list), + __GFP_NOWARN | GFP_TEMPORARY); + if (fence_list == NULL) { + DRM_DEBUG("Failed to allocate fence list for %d fences\n", + args->num_cliprects); + kvfree(exec2_list); + return -ENOMEM; + } + if (copy_from_user(fence_list, + u64_to_user_ptr(args->cliprects_ptr), + sizeof(*fence_list) * args->num_cliprects)) { + DRM_DEBUG("copy %d fence entries failed\n", + args->num_cliprects); + kvfree(exec2_list); + kvfree(fence_list); + return -EFAULT; + } + } + + err = i915_gem_do_execbuffer(dev, file, args, exec2_list, fence_list); /* * Now that we have begun execution of the batchbuffer, we ignore @@ -2517,5 +2601,6 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data, args->flags &= ~__I915_EXEC_UNKNOWN_FLAGS; kvfree(exec2_list); + kvfree(fence_list); return err; } diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 7ccbd6a..5b5f033 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -431,6 +431,11 @@ typedef struct drm_i915_irq_wait { */ #define I915_PARAM_HAS_EXEC_BATCH_FIRST 48 +/* Query whether DRM_I915_GEM_EXECBUFFER2 supports supplying an array of + * drm_i915_gem_exec_fence structures. See I915_EXEC_FENCE_ARRAY. + */ +#define I915_PARAM_HAS_EXEC_FENCE_ARRAY 49 + typedef struct drm_i915_getparam { __s32 param; /* @@ -812,6 +817,17 @@ struct drm_i915_gem_exec_object2 { __u64 rsvd2; }; +struct drm_i915_gem_exec_fence { + /** + * User's handle for a dma-fence to wait on or signal. + */ + __u32 handle; + +#define I915_EXEC_FENCE_WAIT (1<<0) +#define I915_EXEC_FENCE_SIGNAL (1<<1) + __u32 flags; +}; + struct drm_i915_gem_execbuffer2 { /** * List of gem_exec_object2 structs @@ -826,7 +842,10 @@ struct drm_i915_gem_execbuffer2 { __u32 DR1; __u32 DR4; __u32 num_cliprects; - /** This is a struct drm_clip_rect *cliprects */ + /** This is a struct drm_clip_rect *cliprects if I915_EXEC_FENCE_ARRAY + * is not set. If I915_EXEC_FENCE_ARRAY is set, then this is a + * struct drm_i915_gem_exec_fence *fences. + */ __u64 cliprects_ptr; #define I915_EXEC_RING_MASK (7<<0) #define I915_EXEC_DEFAULT (0<<0) @@ -927,7 +946,14 @@ struct drm_i915_gem_execbuffer2 { * element). */ #define I915_EXEC_BATCH_FIRST (1<<18) -#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_BATCH_FIRST<<1)) + +/* Setting I915_FENCE_ARRAY implies that num_cliprects and cliprects_ptr + * define an array of i915_gem_exec_fence structures which specify a set of + * dma fences to wait upon or signal. + */ +#define I915_EXEC_FENCE_ARRAY (1<<19) + +#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_ARRAY<<1)) #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) #define i915_execbuffer2_set_context_id(eb2, context) \