Re: [PATCH] drm/i915: Allow tiling of objects with bit 17 swizzling by the CPU.

2009-04-04 Thread Eric Anholt
On Fri, 2009-04-03 at 18:08 -0700, Keith Packard wrote:
 On Fri, 2009-04-03 at 17:03 -0700, Eric Anholt wrote:
 
  +   obj_priv-bit_17 = kmalloc(page_count, GFP_KERNEL);
 
 seems like you could save a bit of memory here by using 1 bit per page
 instead of a byte.

Before, I was figuring it was better to have easy code than memory
optimal code in my infrequently-tested workaround.  Since recent notes
about kmalloc limits, I'll switch to using bitmap.h.

-- 
Eric Anholt
e...@anholt.net eric.anh...@intel.com




signature.asc
Description: This is a digitally signed message part
--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


Re: [PATCH] drm/i915: Allow tiling of objects with bit 17 swizzling by the CPU.

2009-04-04 Thread Keith Packard
On Sat, 2009-04-04 at 10:25 -0700, Eric Anholt wrote:

 Before, I was figuring it was better to have easy code than memory
 optimal code in my infrequently-tested workaround.  Since recent notes
 about kmalloc limits, I'll switch to using bitmap.h.

Yeah, a single page of bits will cover 32k of pages this way. Not sure
bitmap.h is the best plan though; it looks designed to deal with
fixed-size bitmaps (there are warnings about using variable lengths in
the comments).

-- 
keith.pack...@intel.com


signature.asc
Description: This is a digitally signed message part
--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


[PATCH] drm/i915: Allow tiling of objects with bit 17 swizzling by the CPU.

2009-04-03 Thread Eric Anholt
Save the bit 17 state of the pages when freeing the page list, and
reswizzle them if necessary when rebinding the pages.  This exposes two new
swizzle enums to userland, which would have to be respected for correct
operation of software fallbacks on these objects.  Given that users
complain that 3D acceleration is slower than software rendering without
tiling, it seems that reenabling things even if some operations aren't correct
with old userland is worth the cost.

Userland won't be able to respect the two new tiling enums with
drm_intel_bo_map or pread/pwrite.  However, GTT maps are available and should
prove sufficient.

Signed-off-by: Eric Anholt e...@anholt.net
---
I've tested this, and it does take swapping of tiled objects on an
A17-affected machine from failing to working.  I haven't written the
Mesa patch to move to GTT mapping yet, so this would regress Mesa and I
won't submit it for inclusion until I do that.

Or, if there's objection to breaking backwards compat in this way, I
could write pread/pwrite compatibility paths that do the swizzling as
well and just tell the user that it's got the swizzling without A17,
which would cover Mesa SW fallbacks.  But either way we'll want to
change Mesa's behavior on at least A17 machines.

 drivers/gpu/drm/i915/i915_drv.h|5 ++
 drivers/gpu/drm/i915/i915_gem.c|8 +++
 drivers/gpu/drm/i915/i915_gem_tiling.c |   85 +++-
 include/drm/i915_drm.h |3 +
 4 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index efcd610..f968fe8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -446,6 +446,9 @@ struct drm_i915_gem_object {
uint32_t tiling_mode;
uint32_t stride;
 
+   /** Record of address bit 17 of each page at last unbind. */
+   char *bit_17;
+
/** AGP mapping type (AGP_USER_MEMORY or AGP_USER_CACHED_MEMORY */
uint32_t agp_type;
 
@@ -640,6 +643,8 @@ void i915_gem_object_put_pages(struct drm_gem_object *obj);
 
 /* i915_gem_tiling.c */
 void i915_gem_detect_bit_6_swizzle(struct drm_device *dev);
+void i915_gem_object_do_bit_17_swizzle(struct drm_gem_object *obj);
+void i915_gem_object_save_bit_17_swizzle(struct drm_gem_object *obj);
 
 /* i915_gem_debug.c */
 void i915_gem_dump_object(struct drm_gem_object *obj, int len,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 33ab07b..14db0e4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1295,6 +1295,9 @@ i915_gem_object_put_pages(struct drm_gem_object *obj)
if (--obj_priv-pages_refcount != 0)
return;
 
+   if (obj_priv-tiling_mode != I915_TILING_NONE)
+   i915_gem_object_save_bit_17_swizzle(obj);
+
for (i = 0; i  page_count; i++)
if (obj_priv-pages[i] != NULL) {
if (obj_priv-dirty)
@@ -1920,6 +1923,10 @@ i915_gem_object_get_pages(struct drm_gem_object *obj)
}
obj_priv-pages[i] = page;
}
+
+   if (obj_priv-tiling_mode != I915_TILING_NONE)
+   i915_gem_object_do_bit_17_swizzle(obj);
+
return 0;
 }
 
@@ -3591,6 +3598,7 @@ void i915_gem_free_object(struct drm_gem_object *obj)
i915_gem_free_mmap_offset(obj);
 
drm_free(obj_priv-page_cpu_valid, 1, DRM_MEM_DRIVER);
+   kfree(obj_priv-bit_17);
drm_free(obj-driver_private, 1, DRM_MEM_DRIVER);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c 
b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 6be3f92..5544102 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -25,6 +25,7 @@
  *
  */
 
+#include linux/string.h
 #include drmP.h
 #include drm.h
 #include i915_drm.h
@@ -127,8 +128,8 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
swizzle_y = I915_BIT_6_SWIZZLE_9_11;
} else {
/* Bit 17 swizzling by the CPU in addition. */
-   swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN;
-   swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN;
+   swizzle_x = I915_BIT_6_SWIZZLE_9_10_17;
+   swizzle_y = I915_BIT_6_SWIZZLE_9_17;
}
break;
}
@@ -359,3 +360,83 @@ i915_gem_get_tiling(struct drm_device *dev, void *data,
 
return 0;
 }
+
+/**
+ * Swap every 64 bytes of this page around, to account for it having a new
+ * bit 17 of its physical address and therefore being interpreted differently
+ * by the GPU.
+ */
+static int
+i915_gem_swizzle_page(struct page *page)
+{
+   char *vaddr;
+   int i;
+   char temp[64];
+
+   vaddr = kmap(page);
+   if (vaddr == NULL)
+   return -ENOMEM;
+
+  

Re: [PATCH] drm/i915: Allow tiling of objects with bit 17 swizzling by the CPU.

2009-04-03 Thread Keith Packard
On Fri, 2009-04-03 at 17:03 -0700, Eric Anholt wrote:

 + obj_priv-bit_17 = kmalloc(page_count, GFP_KERNEL);

seems like you could save a bit of memory here by using 1 bit per page
instead of a byte.

-- 
keith.pack...@intel.com


signature.asc
Description: This is a digitally signed message part
--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


Re: [PATCH] drm/i915: Allow tiling of objects with bit 17 swizzling by the CPU.

2009-04-03 Thread Jesse Barnes
On Fri, 03 Apr 2009 18:08:14 -0700
Keith Packard kei...@keithp.com wrote:

 On Fri, 2009-04-03 at 17:03 -0700, Eric Anholt wrote:
 
  +   obj_priv-bit_17 = kmalloc(page_count, GFP_KERNEL);
 
 seems like you could save a bit of memory here by using 1 bit per page
 instead of a byte.
 

If you choose to go that route there are a bunch of bitmap handling
routines in linux/bitmap.h.

-- 
Jesse Barnes, Intel Open Source Technology Center

--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel