From: Dave Airlie airlied@redhat.com
In order for udl vmap to work properly, we need to push the object into the CPU domain before we start copying the data to the USB device.
question: what is direction here in terms of read/write to the device.
This along with the udl change avoids userspace explicit mapping to be used.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/i915/i915_gem_dmabuf.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c index cf6bdec..ee51c63 100644 --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c @@ -147,6 +147,22 @@ static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct * return -EINVAL; }
+static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start, size_t length, enum dma_data_direction direction) +{ + struct drm_i915_gem_object *obj = dma_buf->priv; + struct drm_device *dev = obj->base.dev; + int ret; + bool write = (direction == DMA_BIDIRECTIONAL || direction == DMA_TO_DEVICE); + + ret = i915_mutex_lock_interruptible(dev); + if (ret) + return ret; + + ret = i915_gem_object_set_to_cpu_domain(obj, write); + mutex_unlock(&dev->struct_mutex); + return ret; +} + static const struct dma_buf_ops i915_dmabuf_ops = { .map_dma_buf = i915_gem_map_dma_buf, .unmap_dma_buf = i915_gem_unmap_dma_buf, @@ -158,6 +174,7 @@ static const struct dma_buf_ops i915_dmabuf_ops = { .mmap = i915_gem_dmabuf_mmap, .vmap = i915_gem_dmabuf_vmap, .vunmap = i915_gem_dmabuf_vunmap, + .begin_cpu_access = i915_gem_begin_cpu_access, };
struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
From: Dave Airlie airlied@redhat.com
We need to call these before we transfer the damaged areas to the device not before/after we setup the long lived vmaps.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/udl/udl_fb.c | 22 ++++++++++++++++++++-- drivers/gpu/drm/udl/udl_gem.c | 7 ------- 2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index ce9a611..b8c00ed 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -13,6 +13,7 @@ #include <linux/module.h> #include <linux/slab.h> #include <linux/fb.h> +#include <linux/dma-buf.h>
#include "drmP.h" #include "drm.h" @@ -377,16 +378,33 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb, { struct udl_framebuffer *ufb = to_udl_fb(fb); int i; + int ret = 0;
if (!ufb->active_16) return 0;
+ if (ufb->obj->base.import_attach) { + ret = dma_buf_begin_cpu_access(ufb->obj->base.import_attach->dmabuf, + 0, ufb->obj->base.size, + DMA_FROM_DEVICE); + if (ret) + return ret; + } + for (i = 0; i < num_clips; i++) { - udl_handle_damage(ufb, clips[i].x1, clips[i].y1, + ret = udl_handle_damage(ufb, clips[i].x1, clips[i].y1, clips[i].x2 - clips[i].x1, clips[i].y2 - clips[i].y1); + if (ret) + break; } - return 0; + + if (ufb->obj->base.import_attach) { + dma_buf_end_cpu_access(ufb->obj->base.import_attach->dmabuf, + 0, ufb->obj->base.size, + DMA_FROM_DEVICE); + } + return ret; }
static void udl_user_framebuffer_destroy(struct drm_framebuffer *fb) diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c index 7bd65bd..2a49678 100644 --- a/drivers/gpu/drm/udl/udl_gem.c +++ b/drivers/gpu/drm/udl/udl_gem.c @@ -181,11 +181,6 @@ int udl_gem_vmap(struct udl_gem_object *obj) int ret;
if (obj->base.import_attach) { - ret = dma_buf_begin_cpu_access(obj->base.import_attach->dmabuf, - 0, obj->base.size, DMA_BIDIRECTIONAL); - if (ret) - return -EINVAL; - obj->vmapping = dma_buf_vmap(obj->base.import_attach->dmabuf); if (!obj->vmapping) return -ENOMEM; @@ -206,8 +201,6 @@ void udl_gem_vunmap(struct udl_gem_object *obj) { if (obj->base.import_attach) { dma_buf_vunmap(obj->base.import_attach->dmabuf, obj->vmapping); - dma_buf_end_cpu_access(obj->base.import_attach->dmabuf, 0, - obj->base.size, DMA_BIDIRECTIONAL); return; }
On Mon, Jul 30, 2012 at 02:06:55PM +1000, Dave Airlie wrote:
From: Dave Airlie airlied@redhat.com
We need to call these before we transfer the damaged areas to the device not before/after we setup the long lived vmaps.
Signed-off-by: Dave Airlie airlied@redhat.com
As discussed on irc, I've applied this patch here and the corresponding fix in i915 to make the begin/end_cpu_access stuff work to drm-intel-next. I've left patch 3 of this series for you to handle. -Daniel
From: Dave Airlie airlied@redhat.com
This checks the return from the hline renderer, and flags EAGAIN if we jump out here.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/udl/udl_fb.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index b8c00ed..dfd27e9 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -145,7 +145,7 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, { struct drm_device *dev = fb->base.dev; struct udl_device *udl = dev->dev_private; - int i, ret; + int i, ret = 0; char *cmd; cycles_t start_cycles, end_cycles; int bytes_sent = 0; @@ -189,11 +189,14 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, const int line_offset = fb->base.pitches[0] * i; const int byte_offset = line_offset + (x * bpp);
- if (udl_render_hline(dev, bpp, &urb, + ret = udl_render_hline(dev, bpp, &urb, (char *) fb->obj->vmapping, &cmd, byte_offset, width * bpp, - &bytes_identical, &bytes_sent)) + &bytes_identical, &bytes_sent); + if (ret == 1) { + ret = -EAGAIN; goto error; + } }
if (cmd > (char *) urb->transfer_buffer) { @@ -213,7 +216,7 @@ error: >> 10)), /* Kcycles */ &udl->cpu_kcycles_used);
- return 0; + return ret; }
static int udl_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
On Mon, Jul 30, 2012 at 02:06:54PM +1000, Dave Airlie wrote:
From: Dave Airlie airlied@redhat.com
In order for udl vmap to work properly, we need to push the object into the CPU domain before we start copying the data to the USB device.
question: what is direction here in terms of read/write to the device.
This along with the udl change avoids userspace explicit mapping to be used.
Signed-off-by: Dave Airlie airlied@redhat.com
In my understanding TO_DEVICE means cpu writes, devices reads. FROM is devices writes, cpu reads. So your patch looks correct.
One thing I wonder is how much lockdep likes this one here ... But I guess it's not the first one ;-) Also, do we have a simple testcase that clears the bo with the intel igd and then tells udl the updated damage, just to exercise the code a bit? -Daniel
On Mon, Aug 6, 2012 at 6:52 AM, Daniel Vetter daniel@ffwll.ch wrote:
On Mon, Jul 30, 2012 at 02:06:54PM +1000, Dave Airlie wrote:
From: Dave Airlie airlied@redhat.com
In order for udl vmap to work properly, we need to push the object into the CPU domain before we start copying the data to the USB device.
question: what is direction here in terms of read/write to the device.
This along with the udl change avoids userspace explicit mapping to be used.
Signed-off-by: Dave Airlie airlied@redhat.com
In my understanding TO_DEVICE means cpu writes, devices reads. FROM is devices writes, cpu reads. So your patch looks correct.
One thing I wonder is how much lockdep likes this one here ... But I guess it's not the first one ;-) Also, do we have a simple testcase that clears the bo with the intel igd and then tells udl the updated damage, just to exercise the code a bit?
no more lockdep that previously, which is to say one big splat.
I have real code to exercise the code a bit :-), but I'll try and add another test to my intel-gpu-tools branches this week,
Dave.
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Dave Airlie