* [PATCH 1/2] drm/i915: Use non-atomic kmap for slow copy paths
@ 2010-05-27 13:15 Chris Wilson
2010-05-27 13:15 ` [PATCH 2/2] drm/i915: Fix up address spaces in slow_kernel_write() Chris Wilson
2010-05-28 18:06 ` [PATCH 1/2] drm/i915: Use non-atomic kmap for slow copy paths Eric Anholt
0 siblings, 2 replies; 5+ messages in thread
From: Chris Wilson @ 2010-05-27 13:15 UTC (permalink / raw)
To: intel-gfx
As we do not have a requirement to be atomic and avoid sleeping whilst
performing the slow copy for shmem based pread and pwrite, we can use
kmap instead, thus simplifying the code.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_gem.c | 82 ++++++++++++++------------------------
1 files changed, 30 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6694b44..6769fab 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -167,7 +167,7 @@ static int i915_gem_object_needs_bit17_swizzle(struct drm_gem_object *obj)
obj_priv->tiling_mode != I915_TILING_NONE;
}
-static inline int
+static inline void
slow_shmem_copy(struct page *dst_page,
int dst_offset,
struct page *src_page,
@@ -176,25 +176,16 @@ slow_shmem_copy(struct page *dst_page,
{
char *dst_vaddr, *src_vaddr;
- dst_vaddr = kmap_atomic(dst_page, KM_USER0);
- if (dst_vaddr == NULL)
- return -ENOMEM;
-
- src_vaddr = kmap_atomic(src_page, KM_USER1);
- if (src_vaddr == NULL) {
- kunmap_atomic(dst_vaddr, KM_USER0);
- return -ENOMEM;
- }
+ dst_vaddr = kmap(dst_page);
+ src_vaddr = kmap(src_page);
memcpy(dst_vaddr + dst_offset, src_vaddr + src_offset, length);
- kunmap_atomic(src_vaddr, KM_USER1);
- kunmap_atomic(dst_vaddr, KM_USER0);
-
- return 0;
+ kunmap(src_page);
+ kunmap(dst_page);
}
-static inline int
+static inline void
slow_shmem_bit17_copy(struct page *gpu_page,
int gpu_offset,
struct page *cpu_page,
@@ -214,15 +205,8 @@ slow_shmem_bit17_copy(struct page *gpu_page,
cpu_page, cpu_offset, length);
}
- gpu_vaddr = kmap_atomic(gpu_page, KM_USER0);
- if (gpu_vaddr == NULL)
- return -ENOMEM;
-
- cpu_vaddr = kmap_atomic(cpu_page, KM_USER1);
- if (cpu_vaddr == NULL) {
- kunmap_atomic(gpu_vaddr, KM_USER0);
- return -ENOMEM;
- }
+ gpu_vaddr = kmap(gpu_page);
+ cpu_vaddr = kmap(cpu_page);
/* Copy the data, XORing A6 with A17 (1). The user already knows he's
* XORing with the other bits (A9 for Y, A9 and A10 for X)
@@ -246,10 +230,8 @@ slow_shmem_bit17_copy(struct page *gpu_page,
length -= this_length;
}
- kunmap_atomic(cpu_vaddr, KM_USER1);
- kunmap_atomic(gpu_vaddr, KM_USER0);
-
- return 0;
+ kunmap(cpu_page);
+ kunmap(gpu_page);
}
/**
@@ -425,21 +407,19 @@ i915_gem_shmem_pread_slow(struct drm_device *dev, struct drm_gem_object *obj,
page_length = PAGE_SIZE - data_page_offset;
if (do_bit17_swizzling) {
- ret = slow_shmem_bit17_copy(obj_priv->pages[shmem_page_index],
- shmem_page_offset,
- user_pages[data_page_index],
- data_page_offset,
- page_length,
- 1);
- } else {
- ret = slow_shmem_copy(user_pages[data_page_index],
- data_page_offset,
- obj_priv->pages[shmem_page_index],
+ slow_shmem_bit17_copy(obj_priv->pages[shmem_page_index],
shmem_page_offset,
- page_length);
+ user_pages[data_page_index],
+ data_page_offset,
+ page_length,
+ 1);
+ } else {
+ slow_shmem_copy(user_pages[data_page_index],
+ data_page_offset,
+ obj_priv->pages[shmem_page_index],
+ shmem_page_offset,
+ page_length);
}
- if (ret)
- goto fail_put_pages;
remain -= page_length;
data_ptr += page_length;
@@ -900,21 +880,19 @@ i915_gem_shmem_pwrite_slow(struct drm_device *dev, struct drm_gem_object *obj,
page_length = PAGE_SIZE - data_page_offset;
if (do_bit17_swizzling) {
- ret = slow_shmem_bit17_copy(obj_priv->pages[shmem_page_index],
- shmem_page_offset,
- user_pages[data_page_index],
- data_page_offset,
- page_length,
- 0);
- } else {
- ret = slow_shmem_copy(obj_priv->pages[shmem_page_index],
+ slow_shmem_bit17_copy(obj_priv->pages[shmem_page_index],
shmem_page_offset,
user_pages[data_page_index],
data_page_offset,
- page_length);
+ page_length,
+ 0);
+ } else {
+ slow_shmem_copy(obj_priv->pages[shmem_page_index],
+ shmem_page_offset,
+ user_pages[data_page_index],
+ data_page_offset,
+ page_length);
}
- if (ret)
- goto fail_put_pages;
remain -= page_length;
data_ptr += page_length;
--
1.7.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] drm/i915: Fix up address spaces in slow_kernel_write()
2010-05-27 13:15 [PATCH 1/2] drm/i915: Use non-atomic kmap for slow copy paths Chris Wilson
@ 2010-05-27 13:15 ` Chris Wilson
2010-05-28 9:28 ` Daniel Vetter
2010-05-28 18:06 ` [PATCH 1/2] drm/i915: Use non-atomic kmap for slow copy paths Eric Anholt
1 sibling, 1 reply; 5+ messages in thread
From: Chris Wilson @ 2010-05-27 13:15 UTC (permalink / raw)
To: intel-gfx
Since we now get_user_pages() outside of the mutex prior to performing
the copy, we kmap() the page inside the copy routine and so need to
perform an ordinary memcpy() and not copy_from_user().
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_gem.c | 42 +++++++++++++++-----------------------
1 files changed, 17 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6769fab..528ff7d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -509,25 +509,24 @@ fast_user_write(struct io_mapping *mapping,
* page faults
*/
-static inline int
+static inline void
slow_kernel_write(struct io_mapping *mapping,
loff_t gtt_base, int gtt_offset,
struct page *user_page, int user_offset,
int length)
{
- char *src_vaddr, *dst_vaddr;
- unsigned long unwritten;
+ char __iomem *dst_vaddr;
+ char *src_vaddr;
- dst_vaddr = io_mapping_map_atomic_wc(mapping, gtt_base);
- src_vaddr = kmap_atomic(user_page, KM_USER1);
- unwritten = __copy_from_user_inatomic_nocache(dst_vaddr + gtt_offset,
- src_vaddr + user_offset,
- length);
- kunmap_atomic(src_vaddr, KM_USER1);
- io_mapping_unmap_atomic(dst_vaddr);
- if (unwritten)
- return -EFAULT;
- return 0;
+ dst_vaddr = io_mapping_map_wc(mapping, gtt_base);
+ src_vaddr = kmap(user_page);
+
+ memcpy_toio(dst_vaddr + gtt_offset,
+ src_vaddr + user_offset,
+ length);
+
+ kunmap(user_page);
+ io_mapping_unmap(dst_vaddr);
}
static inline int
@@ -700,18 +699,11 @@ i915_gem_gtt_pwrite_slow(struct drm_device *dev, struct drm_gem_object *obj,
if ((data_page_offset + page_length) > PAGE_SIZE)
page_length = PAGE_SIZE - data_page_offset;
- ret = slow_kernel_write(dev_priv->mm.gtt_mapping,
- gtt_page_base, gtt_page_offset,
- user_pages[data_page_index],
- data_page_offset,
- page_length);
-
- /* If we get a fault while copying data, then (presumably) our
- * source page isn't available. Return the error and we'll
- * retry in the slow path.
- */
- if (ret)
- goto out_unpin_object;
+ slow_kernel_write(dev_priv->mm.gtt_mapping,
+ gtt_page_base, gtt_page_offset,
+ user_pages[data_page_index],
+ data_page_offset,
+ page_length);
remain -= page_length;
offset += page_length;
--
1.7.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/i915: Fix up address spaces in slow_kernel_write()
2010-05-27 13:15 ` [PATCH 2/2] drm/i915: Fix up address spaces in slow_kernel_write() Chris Wilson
@ 2010-05-28 9:28 ` Daniel Vetter
2010-05-28 9:36 ` Chris Wilson
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Vetter @ 2010-05-28 9:28 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Thu, May 27, 2010 at 02:15:35PM +0100, Chris Wilson wrote:
> Since we now get_user_pages() outside of the mutex prior to performing
> the copy, we kmap() the page inside the copy routine and so need to
> perform an ordinary memcpy() and not copy_from_user().
Patches look sane and will definitely help in (someday) fixing up the racy
pwrite/pread locking. So for both patches:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/i915: Fix up address spaces in slow_kernel_write()
2010-05-28 9:28 ` Daniel Vetter
@ 2010-05-28 9:36 ` Chris Wilson
0 siblings, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2010-05-28 9:36 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Fri, 28 May 2010 11:28:50 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, May 27, 2010 at 02:15:35PM +0100, Chris Wilson wrote:
> > Since we now get_user_pages() outside of the mutex prior to performing
> > the copy, we kmap() the page inside the copy routine and so need to
> > perform an ordinary memcpy() and not copy_from_user().
>
> Patches look sane and will definitely help in (someday) fixing up the racy
> pwrite/pread locking.
Now who's being optimistic... ;-)
All I am trying to do is find some way to explain
https://bugs.freedesktop.org/show_bug.cgi?id=26645#c5
-ickle
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] drm/i915: Use non-atomic kmap for slow copy paths
2010-05-27 13:15 [PATCH 1/2] drm/i915: Use non-atomic kmap for slow copy paths Chris Wilson
2010-05-27 13:15 ` [PATCH 2/2] drm/i915: Fix up address spaces in slow_kernel_write() Chris Wilson
@ 2010-05-28 18:06 ` Eric Anholt
1 sibling, 0 replies; 5+ messages in thread
From: Eric Anholt @ 2010-05-28 18:06 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 361 bytes --]
On Thu, 27 May 2010 14:15:34 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> As we do not have a requirement to be atomic and avoid sleeping whilst
> performing the slow copy for shmem based pread and pwrite, we can use
> kmap instead, thus simplifying the code.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Applied this series.
[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-05-28 18:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-27 13:15 [PATCH 1/2] drm/i915: Use non-atomic kmap for slow copy paths Chris Wilson
2010-05-27 13:15 ` [PATCH 2/2] drm/i915: Fix up address spaces in slow_kernel_write() Chris Wilson
2010-05-28 9:28 ` Daniel Vetter
2010-05-28 9:36 ` Chris Wilson
2010-05-28 18:06 ` [PATCH 1/2] drm/i915: Use non-atomic kmap for slow copy paths Eric Anholt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).