We'd love to just be using PAT, but even on chips with PAT it gets disabled
sometimes due to an errata. It would probably be better to have pat_enabled
exported and only bother with this when !pat_enabled.
---
drivers/gpu/drm/i915/i915_dma.c | 30 +++++++++++++++++++++++++-----
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 6 ------
3 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 439d271..c391568 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -968,10 +968,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto kfree_devname;
- dev_priv->mm.gtt_mapping =
- io_mapping_create_wc(dev->agp->base,
- dev->agp->agp_info.aper_size * 1024*1024);
-
/* Allow hardware batchbuffers unless told otherwise.
*/
dev_priv->allow_batchbuffer = 1;
@@ -1083,6 +1079,24 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
goto free_priv;
}
+ dev_priv->mm.gtt_mapping =
+ io_mapping_create_wc(dev->agp->base,
+ dev->agp->agp_info.aper_size * 1024*1024);
+ /* Set up a WC MTRR for non-PAT systems. This is more common than
+ * one would think, because the kernel disables PAT on first
+ * generation Core chips because WC PAT gets overridden by a UC
+ * MTRR if present. Even if a UC MTRR isn't present.
+ */
+ dev_priv->mm.gtt_mtrr = -1;
+ dev_priv->mm.gtt_mtrr = mtrr_add(dev->agp->base,
+ dev->agp->agp_info.aper_size *
+ 1024 * 1024,
+ MTRR_TYPE_WRCOMB, 1);
+ if (dev_priv->mm.gtt_mtrr < 0) {
+ DRM_INFO("MTRR allocation failed\n. Graphics "
+ "performance may suffer.\n");
+ }
+
#ifdef CONFIG_HIGHMEM64G
/* don't enable GEM on PAE - needs agp + set_memory_* interface fixes */
dev_priv->has_gem = 0;
@@ -1147,8 +1161,14 @@ int i915_driver_unload(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ io_mapping_free(dev_priv->mm.gtt_mapping);
+ if (dev_priv->mm.gtt_mtrr >= 0) {
+ mtrr_del(dev_priv->mm.gtt_mtrr, dev->agp->base,
+ dev->agp->agp_info.aper_size * 1024 * 1024);
+ dev_priv->mm.gtt_mtrr = -1;
+ }
+
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
- io_mapping_free(dev_priv->mm.gtt_mapping);
drm_irq_uninstall(dev);
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e135182..f471d21 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -284,6 +284,7 @@ typedef struct drm_i915_private {
struct drm_mm gtt_space;
struct io_mapping *gtt_mapping;
+ int gtt_mtrr;
/**
* List of objects currently involved in rendering from the
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3741101..66a2dc1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3229,10 +3229,6 @@ i915_gem_entervt_ioctl(struct drm_device *dev, void *data,
dev_priv->mm.wedged = 0;
}
- dev_priv->mm.gtt_mapping = io_mapping_create_wc(dev->agp->base,
- dev->agp->agp_info.aper_size
- * 1024 * 1024);
-
mutex_lock(&dev->struct_mutex);
dev_priv->mm.suspended = 0;
@@ -3255,7 +3251,6 @@ int
i915_gem_leavevt_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
- drm_i915_private_t *dev_priv = dev->dev_private;
int ret;
if (drm_core_check_feature(dev, DRIVER_MODESET))
@@ -3264,7 +3259,6 @@ i915_gem_leavevt_ioctl(struct drm_device *dev, void *data,
ret = i915_gem_idle(dev);
drm_irq_uninstall(dev);
- io_mapping_free(dev_priv->mm.gtt_mapping);
return ret;
}
--
1.5.6.5
In the absence of PAT, PAGE_KERNEL_WC ends up mapping to a memory type that
gets UC behavior even in the presence of a WC MTRR covering the area in
question. By swapping to PAGE_KERNEL_UC_MINUS, we can get the actual
behavior the caller wanted (WC if you can manage it, UC otherwise).
This is recovers the 40% performance improvement of using WC in the DRM
to upload vertex data.
---
arch/x86/mm/iomap_32.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/arch/x86/mm/iomap_32.c b/arch/x86/mm/iomap_32.c
index d0151d8..b5e3964 100644
--- a/arch/x86/mm/iomap_32.c
+++ b/arch/x86/mm/iomap_32.c
@@ -17,6 +17,7 @@
*/
#include <asm/iomap.h>
+#include <asm/pat.h>
#include <linux/module.h>
/* Map 'pfn' using fixed map 'type' and protections 'prot'
@@ -29,6 +30,14 @@ iomap_atomic_prot_pfn(unsigned long pfn, enum km_type type, pgprot_t prot)
pagefault_disable();
+ /* For non-PAT systems, promote PAGE_KERNEL_WC to PAGE_KERNEL_UC_MINUS.
+ * PAGE_KERNEL_WC maps to PWT, which translates to uncached if the
+ * MTRR is UC or WC. UC_MINUS gets the real intention, of the
+ * user, which is "WC if the MTRR is WC, UC if you can't do that."
+ */
+ if (!pat_enabled && pgprot_val(prot) == pgprot_val(PAGE_KERNEL_WC))
+ prot = PAGE_KERNEL_UC_MINUS;
+
idx = type + KM_TYPE_NR*smp_processor_id();
vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
set_pte(kmap_pte-idx, pfn_pte(pfn, prot));
--
1.5.6.5
> + dev_priv->mm.gtt_mtrr = -1;
> + dev_priv->mm.gtt_mtrr = mtrr_add(dev->agp->base,
> + dev->agp->agp_info.aper_size *
> + 1024 * 1024,
> + MTRR_TYPE_WRCOMB, 1);
Two assignments to gtt_mtrr in a row looks a bit odd?
- R.
> + if (!pat_enabled && pgprot_val(prot) == pgprot_val(PAGE_KERNEL_WC))
> + prot = PAGE_KERNEL_UC_MINUS;
indentation broke here (the "prot =" line is indented with a tab then
two spaces, instead of two tabs)
- R.
On Fri, 2009-01-23 at 15:15 -0800, Roland Dreier wrote:
> > + if (!pat_enabled && pgprot_val(prot) == pgprot_val(PAGE_KERNEL_WC))
> > + prot = PAGE_KERNEL_UC_MINUS;
>
> indentation broke here (the "prot =" line is indented with a tab then
> two spaces, instead of two tabs)
Bad path matching going on in my .emacs indentation setup. I don't get
out of the DRM driver often enough. Or rather, I'm in nasty X code too
often :)
Fixed this and the other nitpick. Thanks!
--
Eric Anholt
[email protected] [email protected]
On Fri, Jan 23, 2009 at 02:54:17PM -0800, Eric Anholt wrote:
> + /* Set up a WC MTRR for non-PAT systems. This is more common than
> + * one would think, because the kernel disables PAT on first
> + * generation Core chips because WC PAT gets overridden by a UC
> + * MTRR if present. Even if a UC MTRR isn't present.
As a non-native speaker, I don't understand what the last sentence
means. X gets overridden by Y even if there's no Y doesn't make
sense. Could you do a simpler version?
OG.