[Intel-gfx] [PATCH 0/4] gtt cache coherency checker for i830 class hw
Hi all, This is part of my try-to-fix-i855-cache-coherency patch. It's essentially everything save the actual hacks to fix the gtt chipset flush (that simply not yet ready). The most important part is the cache coherency checker. I think merging this without any other fixes is worth it for a few reasons: - easier i8xx bug triaging: random crashes without any other applicable known issues but with some WARN_ONs due to failed chipset flushes suddenly get a decent explanation. - better assessment of how widespread this problem is: Given that my Thinkpad X40 happily spits out these traces when under use but is otherwise rather stable, we'll likely see a surge of bug reports. Perhaps even a top-ten spot on kerneloops.org :( ... - and one purely selfish reason: The patch I'm carrying around gets smaller ;) I've thought about whether to include a bz link in the dmesg output, but couldn't really decide one way or another. I fear that bz entry will get swamped under by totally useless me-too-and-all-intel-devs-suck reports. But on the other hand it's not really nice to waste testers time (creating nice bug reports) by not telling them that this is a know issue and where progress is tracked. Comments highly welcome. And when testing, don't get scared - at least on i855 getting the first failed flush before gdm finished drawing the login screen is kinda expected ;) Patches rebased against my two outstanding agp/gtt split-up patches, but should work on top of latest drm-intel-next, too. Yours, Daniel Daniel Vetter (4): agp/intel-gtt: steal the last gtt page drm/i915: add locking around chipset flush agp/intel-gtt: check cache-coherency on i830 class chipsets agp/intel-gtt: extract mch buffer flush in i830 chipset flush drivers/char/agp/intel-gtt.c| 163 +++ drivers/gpu/drm/i915/i915_dma.c |4 +- drivers/gpu/drm/i915/i915_gem.c |3 + include/drm/intel-gtt.h |8 ++ 4 files changed, 146 insertions(+), 32 deletions(-) create mode 100644 include/drm/intel-gtt.h ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] agp/intel-gtt: steal the last gtt page
This page will be used to check cache coherency on i8xx chips.
Furthermore gem in drm/i915 doesn't use the last page in the gtt
already to prevent pagefaults due to the gpu prefetcher crossing
into unmapped memory. So this page is useless, anyway.
This introduces include/drm/intel-gtt.h. Atm it only contains this
single define. But I've already noticed quite some code duplication
between the intel-agp and the i915 drm module. The idea is that this
new header file can be used to share some code between these two
modules.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/char/agp/intel-gtt.c| 13 +++--
drivers/gpu/drm/i915/i915_dma.c |4 +++-
include/drm/intel-gtt.h |6 ++
3 files changed, 16 insertions(+), 7 deletions(-)
create mode 100644 include/drm/intel-gtt.h
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index f7793ec..eec043b 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -24,6 +24,7 @@
#include asm/smp.h
#include agp.h
#include intel-agp.h
+#include drm/intel-gtt.h
/*
* If we have Intel graphics, we're not going to have anything other than
@@ -37,9 +38,9 @@
static const struct aper_size_info_fixed intel_i810_sizes[] =
{
- {64, 16384, 4},
+ {64, 16384 - I830_CC_DANCE_PAGES, 4},
/* The 32M mode still requires a 64k gatt */
- {32, 8192, 4}
+ {32, 8192 - I830_CC_DANCE_PAGES, 4}
};
#define AGP_DCACHE_MEMORY 1
@@ -489,11 +490,11 @@ static unsigned long intel_i810_mask_memory(struct
agp_bridge_data *bridge,
static struct aper_size_info_fixed intel_i830_sizes[] =
{
- {128, 32768, 5},
+ {128, 32768 - I830_CC_DANCE_PAGES, 5},
/* The 64M mode still requires a 128k gatt */
- {64, 16384, 5},
- {256, 65536, 6},
- {512, 131072, 7},
+ {64, 16384 - I830_CC_DANCE_PAGES, 5},
+ {256, 65536 - I830_CC_DANCE_PAGES, 6},
+ {512, 131072 - I830_CC_DANCE_PAGES, 7},
};
static void intel_i830_init_gtt_entries(void)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 851a2f8..3e16938 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -39,6 +39,7 @@
#include linux/pnp.h
#include linux/vga_switcheroo.h
#include linux/slab.h
+#include drm/intel-gtt.h
/* Really want an OS-independent resettable timer. Would like to have
* this loop run for (eg) 3 sec, but have the timer reset every time
@@ -1448,7 +1449,8 @@ static int i915_load_modeset_init(struct drm_device *dev,
* at the last page of the aperture. One page should be enough to
* keep any prefetching inside of the aperture.
*/
- i915_gem_do_init(dev, prealloc_size, agp_size - 4096);
+ i915_gem_do_init(dev, prealloc_size,
+agp_size - I830_CC_DANCE_PAGES*4096);
mutex_lock(dev-struct_mutex);
ret = i915_gem_init_ringbuffer(dev);
diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
new file mode 100644
index 000..6cec6d2
--- /dev/null
+++ b/include/drm/intel-gtt.h
@@ -0,0 +1,6 @@
+/* Header file to share declarations between the intel-agp module and the i915
+ * drm module
+ */
+
+/* This denotes how many pages intel-gtt steals at the end of the gart. */
+#define I830_CC_DANCE_PAGES 1
--
1.7.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: add locking around chipset flush
My cache coherency checker for i8xx chipsets will make cache flushes stateful. Therefore add some locking around the only caller that had none. This is not a fast-path, anyway, so it won't hurt for the other chipsets. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_gem.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index f04612f..04fe7f5 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -5234,7 +5234,10 @@ i915_gem_phys_pwrite(struct drm_device *dev, struct drm_gem_object *obj, if (ret) return -EFAULT; + mutex_lock(dev-struct_mutex); drm_agp_chipset_flush(dev); + mutex_unlock(dev-struct_mutex); + return 0; } -- 1.7.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] agp/intel-gtt: extract mch buffer flush in i830 chipset flush
Just a small clean up. The real fix will add tons of code here,
so it's nice to shrink the function a tad bit, first.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/char/agp/intel-gtt.c | 24
1 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index bed0ed6..0277314 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -808,6 +808,20 @@ setup:
intel_private.i8xx_cache_flush_num = 0;
}
+static void intel_flush_mch_write_buffer(void)
+{
+ memset(intel_private.i8xx_cpu_flush_page, 0,
+ I830_MCH_WRITE_BUFFER_SIZE);
+
+ mb();
+ if (cpu_has_clflush) {
+ clflush_cache_range(intel_private.i8xx_cpu_flush_page,
+ I830_MCH_WRITE_BUFFER_SIZE);
+ } else if (wbinvd_on_all_cpus() != 0)
+ printk(KERN_ERR Timed out waiting for cache flush.\n);
+ mb();
+}
+
/* The chipset_flush interface needs to get data that has already been
* flushed out of the CPU all the way out to main memory, because the GPU
* doesn't snoop those buffers.
@@ -846,14 +860,8 @@ static void intel_i830_chipset_flush(struct
agp_bridge_data *bridge)
intel_private.i8xx_gtt_cc_pages + offset1);
mb();
- memset(intel_private.i8xx_cpu_flush_page, 0,
- I830_MCH_WRITE_BUFFER_SIZE);
-
- if (cpu_has_clflush) {
- clflush_cache_range(intel_private.i8xx_cpu_flush_page,
- I830_MCH_WRITE_BUFFER_SIZE);
- } else if (wbinvd_on_all_cpus() != 0)
- printk(KERN_ERR Timed out waiting for cache flush.\n);
+ /* start chipset flush */
+ intel_flush_mch_write_buffer();
/* read check values */
mb();
--
1.7.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Mark the object as dirty when setting to the CPU write domain.
Or else we may not write back the written pages upon unbind. For
example the contents of a batch buffer written using a simple mmap or
using shmmem pwrite may be discarded if we are forced to evict
everything whilst pinning the objects for execbuffer.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: sta...@kernel.org
---
drivers/gpu/drm/i915/i915_gem.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f04612f..5d60c3b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3076,6 +3076,7 @@ i915_gem_object_set_to_cpu_domain(struct drm_gem_object
*obj, int write)
if (write) {
obj-read_domains = I915_GEM_DOMAIN_CPU;
obj-write_domain = I915_GEM_DOMAIN_CPU;
+ obj_priv-dirty = 1;
}
trace_i915_gem_object_change_domain(obj,
--
1.7.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
