From patchwork Mon Feb 29 11:13:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Gordon X-Patchwork-Id: 8451511 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 2A6929F38C for ; Mon, 29 Feb 2016 11:13:43 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0C25520265 for ; Mon, 29 Feb 2016 11:13:42 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id D9B54202A1 for ; Mon, 29 Feb 2016 11:13:40 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 067846E270; Mon, 29 Feb 2016 11:13:40 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTP id 3E2D26E272 for ; Mon, 29 Feb 2016 11:13:37 +0000 (UTC) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 29 Feb 2016 03:13:37 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,520,1449561600"; d="scan'208";a="923337556" Received: from dsgordon-linux2.isw.intel.com ([10.102.226.88]) by orsmga002.jf.intel.com with ESMTP; 29 Feb 2016 03:13:35 -0800 From: Dave Gordon To: intel-gfx@lists.freedesktop.org Date: Mon, 29 Feb 2016 11:13:14 +0000 Message-Id: <1456744394-29831-8-git-send-email-david.s.gordon@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1456744394-29831-1-git-send-email-david.s.gordon@intel.com> References: <1456744394-29831-1-git-send-email-david.s.gordon@intel.com> Organization: Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ Subject: [Intel-gfx] [PATCH v5 7/7] drm/i915: refactor duplicate object vmap functions (reworked some more) X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This is essentially Chris Wilson's patch of a similar name, reworked on top of Alex Dai's recent patch: > drm/i915: Add i915_gem_object_vmap to map GEM object to virtual space Chris' original commentary said: > We now have two implementations for vmapping a whole object, one for > dma-buf and one for the ringbuffer. If we couple the vmapping into > the obj->pages lifetime, then we can reuse an obj->vmapping for both > and at the same time couple it into the shrinker. > > v2: Mark the failable kmalloc() as __GFP_NOWARN (vsyrjala) > v3: Call unpin_vmap from the right dmabuf unmapper v4: reimplements the same functionality, but now as wrappers round the recently-introduced i915_gem_object_vmap_range() from Alex's patch mentioned above. v5: separated from two minor but unrelated changes [Tvrtko Ursulin]; this is the third and most substantial portion. Decided not to hold onto vmappings after the pin count goes to zero. This may reduce the benefit of Chris' scheme a bit, but does avoid any increased risk of exhausting kernel vmap space on 32-bit kernels [Tvrtko Ursulin]. Potentially, the vunmap() could be deferred until the put_pages() stage if a suitable notifier were written, but we're not doing that here. Nonetheless, the simplification of both dmabuf and ringbuffer code makes it worthwhile in its own right. With this change, we have only one vmap() in the whole driver :) Signed-off-by: Dave Gordon Cc: Chris Wilson Cc: Alex Dai Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_drv.h | 22 +++++++++++++++----- drivers/gpu/drm/i915/i915_gem.c | 36 ++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_gem_dmabuf.c | 37 ++++----------------------------- drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++++----- 4 files changed, 62 insertions(+), 43 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 918b0bb..7facdb5 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2172,10 +2172,7 @@ struct drm_i915_gem_object { struct scatterlist *sg; int last; } get_page; - - /* prime dma-buf support */ - void *dma_buf_vmapping; - int vmapping_count; + void *vmapping; /** Breadcrumb of last rendering to the buffer. * There can only be one writer, but we allow for multiple readers. @@ -2980,7 +2977,22 @@ static inline void i915_gem_object_pin_pages(struct drm_i915_gem_object *obj) static inline void i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj) { BUG_ON(obj->pages_pin_count == 0); - obj->pages_pin_count--; + if (--obj->pages_pin_count == 0 && obj->vmapping) { + /* + * Releasing the vmapping here may yield less benefit than + * if we kept it until put_pages(), but on the other hand + * avoids issues of exhausting kernel vmappable address + * space on 32-bit kernels. + */ + vunmap(obj->vmapping); + obj->vmapping = NULL; + } +} + +void *__must_check i915_gem_object_pin_vmap(struct drm_i915_gem_object *obj); +static inline void i915_gem_object_unpin_vmap(struct drm_i915_gem_object *obj) +{ + i915_gem_object_unpin_pages(obj); } void *__must_check i915_gem_object_vmap_range(struct drm_i915_gem_object *obj, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c126211..79d3d5b 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2235,6 +2235,8 @@ static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj) ops->put_pages(obj); obj->pages = NULL; + BUG_ON(obj->vmapping); + i915_gem_object_invalidate(obj); return 0; @@ -2452,6 +2454,40 @@ void *i915_gem_object_vmap_range(struct drm_i915_gem_object *obj, return addr; } +/** + * i915_gem_object_pin_vmap - pin a GEM object and map it into kernel space + * @obj: the GEM object to be mapped + * + * Combines the functions of get_pages(), pin_pages() and vmap_range() on + * the whole object. The caller should release the mapping by calling + * i915_gem_object_unpin_vmap() when it is no longer required. + * + * Returns the address at which the object has been mapped, or an ERR_PTR + * on failure. + */ +void *i915_gem_object_pin_vmap(struct drm_i915_gem_object *obj) +{ + int ret; + + ret = i915_gem_object_get_pages(obj); + if (ret) + return ERR_PTR(ret); + + i915_gem_object_pin_pages(obj); + + if (obj->vmapping == NULL) { + obj->vmapping = i915_gem_object_vmap_range(obj, 0, + obj->base.size >> PAGE_SHIFT); + + if (obj->vmapping == NULL) { + i915_gem_object_unpin_pages(obj); + return ERR_PTR(-ENOMEM); + } + } + + return obj->vmapping; +} + void i915_vma_move_to_active(struct i915_vma *vma, struct drm_i915_gem_request *req) { diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c index 68e21d1..adc7b5e 100644 --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c @@ -108,41 +108,17 @@ static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf) { struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf); struct drm_device *dev = obj->base.dev; + void *addr; int ret; ret = i915_mutex_lock_interruptible(dev); if (ret) return ERR_PTR(ret); - if (obj->dma_buf_vmapping) { - obj->vmapping_count++; - goto out_unlock; - } - - ret = i915_gem_object_get_pages(obj); - if (ret) - goto err; - - i915_gem_object_pin_pages(obj); - - ret = -ENOMEM; - - obj->dma_buf_vmapping = i915_gem_object_vmap_range(obj, 0, - dma_buf->size >> PAGE_SHIFT); - - if (!obj->dma_buf_vmapping) - goto err_unpin; - - obj->vmapping_count = 1; -out_unlock: + addr = i915_gem_object_pin_vmap(obj); mutex_unlock(&dev->struct_mutex); - return obj->dma_buf_vmapping; -err_unpin: - i915_gem_object_unpin_pages(obj); -err: - mutex_unlock(&dev->struct_mutex); - return ERR_PTR(ret); + return addr; } static void i915_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr) @@ -151,12 +127,7 @@ static void i915_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr) struct drm_device *dev = obj->base.dev; mutex_lock(&dev->struct_mutex); - if (--obj->vmapping_count == 0) { - vunmap(obj->dma_buf_vmapping); - obj->dma_buf_vmapping = NULL; - - i915_gem_object_unpin_pages(obj); - } + i915_gem_object_unpin_vmap(obj); mutex_unlock(&dev->struct_mutex); } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 15e2d29..47f186e 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2056,7 +2056,7 @@ static int init_phys_status_page(struct intel_engine_cs *ring) void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf) { if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen) - vunmap(ringbuf->virtual_start); + i915_gem_object_unpin_vmap(ringbuf->obj); else iounmap(ringbuf->virtual_start); ringbuf->virtual_start = NULL; @@ -2080,10 +2080,10 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev, if (ret) goto unpin; - ringbuf->virtual_start = i915_gem_object_vmap_range(obj, 0, - ringbuf->size >> PAGE_SHIFT); - if (ringbuf->virtual_start == NULL) { - ret = -ENOMEM; + ringbuf->virtual_start = i915_gem_object_pin_vmap(obj); + if (IS_ERR(ringbuf->virtual_start)) { + ret = PTR_ERR(ringbuf->virtual_start); + ringbuf->virtual_start = NULL; goto unpin; } } else {