From 2943ef32c97666a6556c98496722c4b72420ad37 Mon Sep 17 00:00:00 2001 From: Andrzej Hajda Date: Tue, 15 Mar 2016 13:46:00 +0100 Subject: [PATCH 1/6] drm/atomic: use helper to get crtc state DRM core provide helper to access crtc state. Signed-off-by: Andrzej Hajda Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1458045960-25193-1-git-send-email-a.hajda@samsung.com --- drivers/gpu/drm/drm_atomic_helper.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index e93bb0bbdd82..91351111d0f9 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -67,7 +67,8 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state, struct drm_crtc_state *crtc_state; if (plane->state->crtc) { - crtc_state = state->crtc_states[drm_crtc_index(plane->state->crtc)]; + crtc_state = drm_atomic_get_existing_crtc_state(state, + plane->state->crtc); if (WARN_ON(!crtc_state)) return; @@ -76,8 +77,8 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state, } if (plane_state->crtc) { - crtc_state = - state->crtc_states[drm_crtc_index(plane_state->crtc)]; + crtc_state = drm_atomic_get_existing_crtc_state(state, + plane_state->crtc); if (WARN_ON(!crtc_state)) return; @@ -374,8 +375,8 @@ mode_fixup(struct drm_atomic_state *state) if (!conn_state->crtc || !conn_state->best_encoder) continue; - crtc_state = - state->crtc_states[drm_crtc_index(conn_state->crtc)]; + crtc_state = drm_atomic_get_existing_crtc_state(state, + conn_state->crtc); /* * Each encoder has at most one connector (since we always steal @@ -679,7 +680,8 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) if (!old_conn_state->crtc) continue; - old_crtc_state = old_state->crtc_states[drm_crtc_index(old_conn_state->crtc)]; + old_crtc_state = drm_atomic_get_existing_crtc_state(old_state, + old_conn_state->crtc); if (!old_crtc_state->active || !drm_atomic_crtc_needs_modeset(old_conn_state->crtc->state)) From b47ff7e6a534bc285eb98f90eadd2a56e2e54056 Mon Sep 17 00:00:00 2001 From: Andrzej Hajda Date: Tue, 15 Mar 2016 13:46:28 +0100 Subject: [PATCH 2/6] drm/atmel-hlcdc: use helper to get crtc state DRM core provide helper to access crtc state. Signed-off-by: Andrzej Hajda Acked-by: Boris Brezillon Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1458045988-25246-1-git-send-email-a.hajda@samsung.com --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c index 1ffe9c329c46..d65dcaee3832 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c @@ -558,7 +558,7 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p, if (!state->base.crtc || !fb) return 0; - crtc_state = s->state->crtc_states[drm_crtc_index(s->crtc)]; + crtc_state = drm_atomic_get_existing_crtc_state(s->state, s->crtc); mode = &crtc_state->adjusted_mode; state->src_x = s->src_x; From 18b862dcd57a3e23e34c8cd1e939f68548c1209a Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 18 Mar 2016 20:02:39 +0000 Subject: [PATCH 3/6] dma-buf, drm, ion: Propagate error code from dma_buf_start_cpu_access() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drivers, especially i915.ko, can fail during the initial migration of a dma-buf for CPU access. However, the error code from the driver was not being propagated back to ioctl and so userspace was blissfully ignorant of the failure. Rendering corruption ensues. Whilst fixing the ioctl to return the error code from dma_buf_start_cpu_access(), also do the same for dma_buf_end_cpu_access(). For most drivers, dma_buf_end_cpu_access() cannot fail. i915.ko however, as most drivers would, wants to avoid being uninterruptible (as would be required to guarrantee no failure when flushing the buffer to the device). As userspace already has to handle errors from the SYNC_IOCTL, take advantage of this to be able to restart the syscall across signals. This fixes a coherency issue for i915.ko as well as reducing the uninterruptible hold upon its BKL, the struct_mutex. Fixes commit c11e391da2a8fe973c3c2398452000bed505851e Author: Daniel Vetter Date: Thu Feb 11 20:04:51 2016 -0200 dma-buf: Add ioctls to allow userspace to flush Testcase: igt/gem_concurrent_blit/*dmabuf*interruptible Testcase: igt/prime_mmap_coherency/ioctl-errors Signed-off-by: Chris Wilson Cc: Tiago Vignatti Cc: Stéphane Marchesin Cc: David Herrmann Cc: Sumit Semwal Cc: Daniel Vetter CC: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linaro-mm-sig@lists.linaro.org Cc: intel-gfx@lists.freedesktop.org Cc: devel@driverdev.osuosl.org Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1458331359-2634-1-git-send-email-chris@chris-wilson.co.uk --- drivers/dma-buf/dma-buf.c | 17 +++++++++++------ drivers/gpu/drm/i915/i915_gem_dmabuf.c | 15 +++++---------- drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 5 +++-- drivers/gpu/drm/udl/udl_fb.c | 4 ++-- drivers/staging/android/ion/ion.c | 6 ++++-- include/linux/dma-buf.h | 6 +++--- 6 files changed, 28 insertions(+), 25 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 9810d1df0691..774a60f4309a 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -259,6 +259,7 @@ static long dma_buf_ioctl(struct file *file, struct dma_buf *dmabuf; struct dma_buf_sync sync; enum dma_data_direction direction; + int ret; dmabuf = file->private_data; @@ -285,11 +286,11 @@ static long dma_buf_ioctl(struct file *file, } if (sync.flags & DMA_BUF_SYNC_END) - dma_buf_end_cpu_access(dmabuf, direction); + ret = dma_buf_end_cpu_access(dmabuf, direction); else - dma_buf_begin_cpu_access(dmabuf, direction); + ret = dma_buf_begin_cpu_access(dmabuf, direction); - return 0; + return ret; default: return -ENOTTY; } @@ -613,13 +614,17 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access); * * This call must always succeed. */ -void dma_buf_end_cpu_access(struct dma_buf *dmabuf, - enum dma_data_direction direction) +int dma_buf_end_cpu_access(struct dma_buf *dmabuf, + enum dma_data_direction direction) { + int ret = 0; + WARN_ON(!dmabuf); if (dmabuf->ops->end_cpu_access) - dmabuf->ops->end_cpu_access(dmabuf, direction); + ret = dmabuf->ops->end_cpu_access(dmabuf, direction); + + return ret; } EXPORT_SYMBOL_GPL(dma_buf_end_cpu_access); diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c index 1f3eef6fb345..0506016e18e0 100644 --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c @@ -228,25 +228,20 @@ static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, enum dma_data_dire return ret; } -static void i915_gem_end_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction direction) +static int i915_gem_end_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction direction) { struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf); struct drm_device *dev = obj->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); - bool was_interruptible; int ret; - mutex_lock(&dev->struct_mutex); - was_interruptible = dev_priv->mm.interruptible; - dev_priv->mm.interruptible = false; + ret = i915_mutex_lock_interruptible(dev); + if (ret) + return ret; ret = i915_gem_object_set_to_gtt_domain(obj, false); - - dev_priv->mm.interruptible = was_interruptible; mutex_unlock(&dev->struct_mutex); - if (unlikely(ret)) - DRM_ERROR("unable to flush buffer following CPU access; rendering may be corrupt\n"); + return ret; } static const struct dma_buf_ops i915_dmabuf_ops = { diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c index aebae1c2dab2..c75249de2984 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c +++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c @@ -93,11 +93,12 @@ static int omap_gem_dmabuf_begin_cpu_access(struct dma_buf *buffer, return omap_gem_get_pages(obj, &pages, true); } -static void omap_gem_dmabuf_end_cpu_access(struct dma_buf *buffer, - enum dma_data_direction dir) +static int omap_gem_dmabuf_end_cpu_access(struct dma_buf *buffer, + enum dma_data_direction dir) { struct drm_gem_object *obj = buffer->priv; omap_gem_put_pages(obj); + return 0; } diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index c427499133d6..33239a2b264a 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -423,8 +423,8 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb, } if (ufb->obj->base.import_attach) { - dma_buf_end_cpu_access(ufb->obj->base.import_attach->dmabuf, - DMA_FROM_DEVICE); + ret = dma_buf_end_cpu_access(ufb->obj->base.import_attach->dmabuf, + DMA_FROM_DEVICE); } unlock: diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 0754a37c9674..49436b4510f4 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -1075,14 +1075,16 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, return PTR_ERR_OR_ZERO(vaddr); } -static void ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf, - enum dma_data_direction direction) +static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf, + enum dma_data_direction direction) { struct ion_buffer *buffer = dmabuf->priv; mutex_lock(&buffer->lock); ion_buffer_kmap_put(buffer); mutex_unlock(&buffer->lock); + + return 0; } static struct dma_buf_ops dma_buf_ops = { diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 532108ea0c1c..3fe90d494edb 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -94,7 +94,7 @@ struct dma_buf_ops { void (*release)(struct dma_buf *); int (*begin_cpu_access)(struct dma_buf *, enum dma_data_direction); - void (*end_cpu_access)(struct dma_buf *, enum dma_data_direction); + int (*end_cpu_access)(struct dma_buf *, enum dma_data_direction); void *(*kmap_atomic)(struct dma_buf *, unsigned long); void (*kunmap_atomic)(struct dma_buf *, unsigned long, void *); void *(*kmap)(struct dma_buf *, unsigned long); @@ -224,8 +224,8 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *, enum dma_data_direction); int dma_buf_begin_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction dir); -void dma_buf_end_cpu_access(struct dma_buf *dma_buf, - enum dma_data_direction dir); +int dma_buf_end_cpu_access(struct dma_buf *dma_buf, + enum dma_data_direction dir); void *dma_buf_kmap_atomic(struct dma_buf *, unsigned long); void dma_buf_kunmap_atomic(struct dma_buf *, unsigned long, void *); void *dma_buf_kmap(struct dma_buf *, unsigned long); From 3c66f4aa7f3cb6579dfe977c05fcb5db25b41cd7 Mon Sep 17 00:00:00 2001 From: Luis de Bethencourt Date: Sat, 19 Mar 2016 17:22:36 +0000 Subject: [PATCH 4/6] drm: remove excess description Description of expected_size doesn't match any parameter of the function drm_atomic_replace_property_blob. Removing it. Signed-off-by: Luis de Bethencourt Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1458408156-15990-1-git-send-email-luisbg@osg.samsung.com --- drivers/gpu/drm/drm_atomic.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index c7e7a955bb1e..d78f8ea5167e 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -392,7 +392,6 @@ EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc); * drm_atomic_replace_property_blob - replace a blob property * @blob: a pointer to the member blob to be replaced * @new_blob: the new blob to replace with - * @expected_size: the expected size of the new blob * @replaced: whether the blob has been replaced * * RETURNS: From 87e332d56b2c7dc6733f690c9069f4fe75f21c9d Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 21 Mar 2016 08:24:22 +0100 Subject: [PATCH 5/6] dma-buf: Update docs for SYNC ioctl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Just a bit of wording polish plus mentioning that it can fail and must be restarted. Requested by Sumit. v2: Fix them typos (Hans). Cc: Chris Wilson Cc: Tiago Vignatti Cc: Stéphane Marchesin Cc: David Herrmann Cc: Sumit Semwal Cc: Daniel Vetter CC: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linaro-mm-sig@lists.linaro.org Cc: intel-gfx@lists.freedesktop.org Cc: devel@driverdev.osuosl.org Cc: Hans Verkuil Acked-by: Sumit Semwal Acked-by: Hans Verkuil Signed-off-by: Daniel Vetter --- Documentation/dma-buf-sharing.txt | 11 ++++++----- drivers/dma-buf/dma-buf.c | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Documentation/dma-buf-sharing.txt b/Documentation/dma-buf-sharing.txt index 32ac32e773e1..ca44c5820585 100644 --- a/Documentation/dma-buf-sharing.txt +++ b/Documentation/dma-buf-sharing.txt @@ -352,7 +352,8 @@ Being able to mmap an export dma-buf buffer object has 2 main use-cases: No special interfaces, userspace simply calls mmap on the dma-buf fd, making sure that the cache synchronization ioctl (DMA_BUF_IOCTL_SYNC) is *always* - used when the access happens. This is discussed next paragraphs. + used when the access happens. Note that DMA_BUF_IOCTL_SYNC can fail with + -EAGAIN or -EINTR, in which case it must be restarted. Some systems might need some sort of cache coherency management e.g. when CPU and GPU domains are being accessed through dma-buf at the same time. To @@ -366,10 +367,10 @@ Being able to mmap an export dma-buf buffer object has 2 main use-cases: want (with the new data being consumed by the GPU or say scanout device) - munmap once you don't need the buffer any more - Therefore, for correctness and optimal performance, systems with the memory - cache shared by the GPU and CPU i.e. the "coherent" and also the - "incoherent" are always required to use SYNC_START and SYNC_END before and - after, respectively, when accessing the mapped address. + For correctness and optimal performance, it is always required to use + SYNC_START and SYNC_END before and after, respectively, when accessing the + mapped address. Userspace cannot rely on coherent access, even when there + are systems where it just works without calling these ioctls. 2. Supporting existing mmap interfaces in importers diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 774a60f4309a..4a2c07ee6677 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -612,7 +612,7 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access); * @dmabuf: [in] buffer to complete cpu access for. * @direction: [in] length of range for cpu access. * - * This call must always succeed. + * Can return negative error values, returns 0 on success. */ int dma_buf_end_cpu_access(struct dma_buf *dmabuf, enum dma_data_direction direction) From b47bcb93bbf201e9c5af698945755efeb60c0bc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Mon, 29 Feb 2016 10:53:11 -0500 Subject: [PATCH 6/6] dma-buf/fence: fix fence_is_later v2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fence is never later than itself. This caused a bunch of overhead for AMDGPU. v2: simplify check as suggested by Michel. Signed-off-by: Christian König Reviewed-by: Michel Dänzer Reviewed-by: Alex Deucher Reviewed-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst Signed-off-by: Alex Deucher Signed-off-by: Sumit Semwal Signed-off-by: Daniel Vetter --- include/linux/fence.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/fence.h b/include/linux/fence.h index bb522011383b..5aa95eb886f7 100644 --- a/include/linux/fence.h +++ b/include/linux/fence.h @@ -292,7 +292,7 @@ static inline bool fence_is_later(struct fence *f1, struct fence *f2) if (WARN_ON(f1->context != f2->context)) return false; - return f1->seqno - f2->seqno < INT_MAX; + return (int)(f1->seqno - f2->seqno) > 0; } /**