[PATCH 1/6] drm/i915: BUG() if fences are used on unsupported platform

[PATCH 1/6] drm/i915: BUG() if fences are used on unsupported platform

[Ben Widawsky]

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_gem.c | 2 +-
 drivers/gpu/drm/i915/i915_irq.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e4b233d..bf3b0c7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2639,7 +2639,7 @@ static void i915_gem_write_fence(struct drm_device *dev, int reg,
 	case 4: i965_write_fence_reg(dev, reg, obj); break;
 	case 3: i915_write_fence_reg(dev, reg, obj); break;
 	case 2: i830_write_fence_reg(dev, reg, obj); break;
-	default: break;
+	default: BUG();
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 914ecf4..4ec8ff2 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1106,6 +1106,8 @@ static void i915_gem_record_fences(struct drm_device *dev,
 			error->fence[i] = I915_READ(FENCE_REG_830_0 + (i * 4));
 		break;
 
+	default:
+		BUG();
 	}
 }
 
-- 
1.8.0.1

[PATCH 2/6] drm/i915: Bug on unsupported swizzled platforms

[Ben Widawsky]

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_gem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index bf3b0c7..4c6f043 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3887,8 +3887,10 @@ void i915_gem_init_swizzling(struct drm_device *dev)
 	I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_SWZCTL);
 	if (IS_GEN6(dev))
 		I915_WRITE(ARB_MODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_SNB));
-	else
+	else if (IS_GEN7(dev))
 		I915_WRITE(ARB_MODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_IVB));
+	else
+		BUG();
 }
 
 static bool
-- 
1.8.0.1

[PATCH 3/6] drm/i915: Missed conversion to gtt_pte_t

[Ben Widawsky]

commit f61c0609073133375ace61f74b0e4e7cf631406b
Author: Ben Widawsky <ben@bwidawsk.net>
Date:   Mon Oct 22 11:44:43 2012 -0700

    drm/i915: introduce gtt_pte_t

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 9ae588a..b699a04 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -282,7 +282,7 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
 	uint32_t pd_offset;
 	struct intel_ring_buffer *ring;
 	struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
-	uint32_t __iomem *pd_addr;
+	gtt_pte_t __iomem *pd_addr;
 	uint32_t pd_entry;
 	int i;
 
@@ -290,7 +290,7 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
 		return;
 
 
-	pd_addr = dev_priv->mm.gtt->gtt + ppgtt->pd_offset/sizeof(uint32_t);
+	pd_addr = dev_priv->mm.gtt->gtt + ppgtt->pd_offset/sizeof(gtt_pte_t);
 	for (i = 0; i < ppgtt->num_pd_entries; i++) {
 		dma_addr_t pt_addr;
 
-- 
1.8.0.1

[PATCH 4/6] drm/i915: Move even more gtt code to i915_gem_gtt

[Ben Widawsky]

This really should have been part of the kill agp series.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.h     |  7 ++---
 drivers/gpu/drm/i915/i915_gem.c     | 51 ++------------------------------
 drivers/gpu/drm/i915/i915_gem_gtt.c | 59 ++++++++++++++++++++++++++++++++++---
 3 files changed, 61 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2ab476d..75003c3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1585,10 +1585,9 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
 				enum i915_cache_level cache_level);
 void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj);
 void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
-void i915_gem_init_global_gtt(struct drm_device *dev,
-			      unsigned long start,
-			      unsigned long mappable_end,
-			      unsigned long end);
+void i915_gem_init_global_gtt(struct drm_device *dev);
+void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
+			       unsigned long mappable_end, unsigned long end);
 int i915_gem_gtt_init(struct drm_device *dev);
 void i915_gem_gtt_fini(struct drm_device *dev);
 static inline void i915_gem_chipset_flush(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4c6f043..42b948b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -163,8 +163,8 @@ i915_gem_init_ioctl(struct drm_device *dev, void *data,
 		return -ENODEV;
 
 	mutex_lock(&dev->struct_mutex);
-	i915_gem_init_global_gtt(dev, args->gtt_start,
-				 args->gtt_end, args->gtt_end);
+	i915_gem_setup_global_gtt(dev, args->gtt_start, args->gtt_end,
+				  args->gtt_end);
 	mutex_unlock(&dev->struct_mutex);
 
 	return 0;
@@ -3959,58 +3959,13 @@ cleanup_render_ring:
 	return ret;
 }
 
-static bool
-intel_enable_ppgtt(struct drm_device *dev)
-{
-	if (i915_enable_ppgtt >= 0)
-		return i915_enable_ppgtt;
-
-#ifdef CONFIG_INTEL_IOMMU
-	/* Disable ppgtt on SNB if VT-d is on. */
-	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
-		return false;
-#endif
-
-	return true;
-}
-
 int i915_gem_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	unsigned long gtt_size, mappable_size;
 	int ret;
 
-	gtt_size = dev_priv->mm.gtt->gtt_total_entries << PAGE_SHIFT;
-	mappable_size = dev_priv->mm.gtt->gtt_mappable_entries << PAGE_SHIFT;
-
 	mutex_lock(&dev->struct_mutex);
-	if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
-		/* PPGTT pdes are stolen from global gtt ptes, so shrink the
-		 * aperture accordingly when using aliasing ppgtt. */
-		gtt_size -= I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
-
-		i915_gem_init_global_gtt(dev, 0, mappable_size, gtt_size);
-
-		ret = i915_gem_init_aliasing_ppgtt(dev);
-		if (ret) {
-			mutex_unlock(&dev->struct_mutex);
-			return ret;
-		}
-	} else {
-		/* Let GEM Manage all of the aperture.
-		 *
-		 * However, leave one page at the end still bound to the scratch
-		 * page.  There are a number of places where the hardware
-		 * apparently prefetches past the end of the object, and we've
-		 * seen multiple hangs with the GPU head pointer stuck in a
-		 * batchbuffer bound at the last page of the aperture.  One page
-		 * should be enough to keep any prefetching inside of the
-		 * aperture.
-		 */
-		i915_gem_init_global_gtt(dev, 0, mappable_size,
-					 gtt_size);
-	}
-
+	i915_gem_init_global_gtt(dev);
 	ret = i915_gem_init_hw(dev);
 	mutex_unlock(&dev->struct_mutex);
 	if (ret) {
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b699a04..fc3c08a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -525,10 +525,10 @@ static void i915_gtt_color_adjust(struct drm_mm_node *node,
 	}
 }
 
-void i915_gem_init_global_gtt(struct drm_device *dev,
-			      unsigned long start,
-			      unsigned long mappable_end,
-			      unsigned long end)
+void i915_gem_setup_global_gtt(struct drm_device *dev,
+			       unsigned long start,
+			       unsigned long mappable_end,
+			       unsigned long end)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct drm_mm_node *entry;
@@ -573,6 +573,57 @@ void i915_gem_init_global_gtt(struct drm_device *dev,
 	i915_ggtt_clear_range(dev, end / PAGE_SIZE - 1, 1);
 }
 
+static bool
+intel_enable_ppgtt(struct drm_device *dev)
+{
+	if (i915_enable_ppgtt >= 0)
+		return i915_enable_ppgtt;
+
+#ifdef CONFIG_INTEL_IOMMU
+	/* Disable ppgtt on SNB if VT-d is on. */
+	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
+		return false;
+#endif
+
+	return true;
+}
+
+void i915_gem_init_global_gtt(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	unsigned long gtt_size, mappable_size;
+	int ret;
+
+	gtt_size = dev_priv->mm.gtt->gtt_total_entries << PAGE_SHIFT;
+	mappable_size = dev_priv->mm.gtt->gtt_mappable_entries << PAGE_SHIFT;
+
+	if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
+		/* PPGTT pdes are stolen from global gtt ptes, so shrink the
+		 * aperture accordingly when using aliasing ppgtt. */
+		gtt_size -= I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
+
+		i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
+
+		ret = i915_gem_init_aliasing_ppgtt(dev);
+		if (ret) {
+			mutex_unlock(&dev->struct_mutex);
+			return;
+		}
+	} else {
+		/* Let GEM Manage all of the aperture.
+		 *
+		 * However, leave one page at the end still bound to the scratch
+		 * page.  There are a number of places where the hardware
+		 * apparently prefetches past the end of the object, and we've
+		 * seen multiple hangs with the GPU head pointer stuck in a
+		 * batchbuffer bound at the last page of the aperture.  One page
+		 * should be enough to keep any prefetching inside of the
+		 * aperture.
+		 */
+		i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
+	}
+}
+
 static int setup_scratch_page(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-- 
1.8.0.1

[PATCH 5/6] drm/i915: Move GSM mapping into dev_priv

[Ben Widawsky]

This removes an unused field from the AGP structure and moves it into
the dev_priv structure (with a slightly better name). This builds upon
the kill-agp series already merged.

GSM is a well defined term in the bspec:
GSM: Graphics Stolen Memory

GTT stolen space is defined for storage of the GFX GTT entries in
physical memory. IA can not access GSM directly , it can only access via
GTTMMADR. GT can access GSM directly or through GTTMMADR.

This is not the entire stolen space.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/char/agp/intel-gtt.c        |  1 -
 drivers/gpu/drm/i915/i915_drv.h     |  3 +++
 drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++-------
 include/drm/intel-gtt.h             |  2 --
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index dbd901e..c8d9dcb 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -602,7 +602,6 @@ static int intel_gtt_init(void)
 		iounmap(intel_private.registers);
 		return -ENOMEM;
 	}
-	intel_private.base.gtt = intel_private.gtt;
 
 	global_cache_flush();   /* FIXME: ? */
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 75003c3..49f465a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -799,6 +799,9 @@ typedef struct drm_i915_private {
 		unsigned long gtt_end;
 		unsigned long stolen_base; /* limited to low memory (32-bit) */
 
+		/** "Graphics Stolen Memory" holds the global PTEs */
+		uint32_t __iomem *gsm;
+
 		struct io_mapping *gtt_mapping;
 		phys_addr_t gtt_base_addr;
 		int gtt_mtrr;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index fc3c08a..b4c1e34 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -290,7 +290,7 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
 		return;
 
 
-	pd_addr = dev_priv->mm.gtt->gtt + ppgtt->pd_offset/sizeof(gtt_pte_t);
+	pd_addr = dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t);
 	for (i = 0; i < ppgtt->num_pd_entries; i++) {
 		dma_addr_t pt_addr;
 
@@ -367,7 +367,7 @@ static void i915_ggtt_clear_range(struct drm_device *dev,
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	gtt_pte_t scratch_pte;
-	gtt_pte_t __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry;
+	gtt_pte_t __iomem *gtt_base = (gtt_pte_t __iomem *) dev_priv->mm.gsm + first_entry;
 	const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
 	int i;
 
@@ -432,7 +432,7 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
 	struct scatterlist *sg = st->sgl;
 	const int first_entry = obj->gtt_space->start >> PAGE_SHIFT;
 	const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
-	gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gtt->gtt + first_entry;
+	gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gsm + first_entry;
 	int unused, i = 0;
 	unsigned int len, m = 0;
 	dma_addr_t addr;
@@ -751,9 +751,9 @@ int i915_gem_gtt_init(struct drm_device *dev)
 		goto err_out;
 	}
 
-	dev_priv->mm.gtt->gtt = ioremap_wc(gtt_bus_addr,
-					   dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
-	if (!dev_priv->mm.gtt->gtt) {
+	dev_priv->mm.gsm = ioremap_wc(gtt_bus_addr,
+				      dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
+	if (!dev_priv->mm.gsm) {
 		DRM_ERROR("Failed to map the gtt page table\n");
 		teardown_scratch_page(dev);
 		ret = -ENOMEM;
@@ -777,7 +777,7 @@ err_out:
 void i915_gem_gtt_fini(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	iounmap(dev_priv->mm.gtt->gtt);
+	iounmap(dev_priv->mm.gsm);
 	teardown_scratch_page(dev);
 	if (INTEL_INFO(dev)->gen < 6)
 		intel_gmch_remove();
diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
index 6eb76a1..3e3a166 100644
--- a/include/drm/intel-gtt.h
+++ b/include/drm/intel-gtt.h
@@ -18,8 +18,6 @@ struct intel_gtt {
 	/* Share the scratch page dma with ppgtts. */
 	dma_addr_t scratch_page_dma;
 	struct page *scratch_page;
-	/* for ppgtt PDE access */
-	u32 __iomem *gtt;
 	/* needed for ioremap in drm/i915 */
 	phys_addr_t gma_bus_addr;
 } *intel_gtt_get(void);
-- 
1.8.0.1

[PATCH 6/6 v2] drm/i915: Make GSM void

[Ben Widawsky]

The iomapping of the register region has historically been a uint32_t
for the obvious reason that our PTE size was always 4b. In the future
however, we cannot make this assumption.

By making the type void, it makes the upcoming pointer math we will do
much easier, and hopefully gives the compiler opportunities to warn us
when we do stupid things.

v2: Cast to __iomem, caught by Ville

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.h     | 2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 49f465a..90dfbd5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -800,7 +800,7 @@ typedef struct drm_i915_private {
 		unsigned long stolen_base; /* limited to low memory (32-bit) */
 
 		/** "Graphics Stolen Memory" holds the global PTEs */
-		uint32_t __iomem *gsm;
+		void __iomem *gsm;
 
 		struct io_mapping *gtt_mapping;
 		phys_addr_t gtt_base_addr;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b4c1e34..a52e784 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -290,7 +290,7 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
 		return;
 
 
-	pd_addr = dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t);
+	pd_addr = (gtt_pte_t *)dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t);
 	for (i = 0; i < ppgtt->num_pd_entries; i++) {
 		dma_addr_t pt_addr;
 
@@ -432,7 +432,8 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
 	struct scatterlist *sg = st->sgl;
 	const int first_entry = obj->gtt_space->start >> PAGE_SHIFT;
 	const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
-	gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gsm + first_entry;
+	gtt_pte_t __iomem *gtt_entries =
+		(gtt_pte_t __iomem *)dev_priv->mm.gsm + first_entry;
 	int unused, i = 0;
 	unsigned int len, m = 0;
 	dma_addr_t addr;
-- 
1.8.0.1

Re: [PATCH 3/6] drm/i915: Missed conversion to gtt_pte_t

[Daniel Vetter]

On Tue, Dec 18, 2012 at 10:31:24AM -0800, Ben Widawsky wrote:
> commit f61c0609073133375ace61f74b0e4e7cf631406b
> Author: Ben Widawsky <ben@bwidawsk.net>
> Date:   Mon Oct 22 11:44:43 2012 -0700
> 
>     drm/i915: introduce gtt_pte_t
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

Merged the first three to dinq, will look at the others tomorrow (or
volunteer a victim to do so). But I've eaten my bikeshed about gsm, you
obviously like the grepability of it very much ;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 6/6 v2] drm/i915: Make GSM void

[Ville Syrjälä]

On Tue, Dec 18, 2012 at 10:31:27AM -0800, Ben Widawsky wrote:
> The iomapping of the register region has historically been a uint32_t
> for the obvious reason that our PTE size was always 4b. In the future
> however, we cannot make this assumption.
> 
> By making the type void, it makes the upcoming pointer math we will do
> much easier, and hopefully gives the compiler opportunities to warn us
> when we do stupid things.
> 
> v2: Cast to __iomem, caught by Ville
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     | 2 +-
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 49f465a..90dfbd5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -800,7 +800,7 @@ typedef struct drm_i915_private {
>  		unsigned long stolen_base; /* limited to low memory (32-bit) */
>  
>  		/** "Graphics Stolen Memory" holds the global PTEs */
> -		uint32_t __iomem *gsm;
> +		void __iomem *gsm;
>  
>  		struct io_mapping *gtt_mapping;
>  		phys_addr_t gtt_base_addr;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index b4c1e34..a52e784 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -290,7 +290,7 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
>  		return;
>  
>  
> -	pd_addr = dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t);
> +	pd_addr = (gtt_pte_t *)dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t);

This cast is still missing __iomem.

>  	for (i = 0; i < ppgtt->num_pd_entries; i++) {
>  		dma_addr_t pt_addr;
>  
> @@ -432,7 +432,8 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
>  	struct scatterlist *sg = st->sgl;
>  	const int first_entry = obj->gtt_space->start >> PAGE_SHIFT;
>  	const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
> -	gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gsm + first_entry;
> +	gtt_pte_t __iomem *gtt_entries =
> +		(gtt_pte_t __iomem *)dev_priv->mm.gsm + first_entry;
>  	int unused, i = 0;
>  	unsigned int len, m = 0;
>  	dma_addr_t addr;
> -- 
> 1.8.0.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 6/6 v2] drm/i915: Make GSM void

[Ben Widawsky]

On Wed, Dec 19, 2012 at 01:45:23PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 18, 2012 at 10:31:27AM -0800, Ben Widawsky wrote:
> > The iomapping of the register region has historically been a uint32_t
> > for the obvious reason that our PTE size was always 4b. In the future
> > however, we cannot make this assumption.
> > 
> > By making the type void, it makes the upcoming pointer math we will do
> > much easier, and hopefully gives the compiler opportunities to warn us
> > when we do stupid things.
> > 
> > v2: Cast to __iomem, caught by Ville
> > 
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h     | 2 +-
> >  drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
> >  2 files changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 49f465a..90dfbd5 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -800,7 +800,7 @@ typedef struct drm_i915_private {
> >  		unsigned long stolen_base; /* limited to low memory (32-bit) */
> >  
> >  		/** "Graphics Stolen Memory" holds the global PTEs */
> > -		uint32_t __iomem *gsm;
> > +		void __iomem *gsm;
> >  
> >  		struct io_mapping *gtt_mapping;
> >  		phys_addr_t gtt_base_addr;
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index b4c1e34..a52e784 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -290,7 +290,7 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
> >  		return;
> >  
> >  
> > -	pd_addr = dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t);
> > +	pd_addr = (gtt_pte_t *)dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t);
> 
> This cast is still missing __iomem.

That looks to be correct. But it wasn't introduced by me, right? Ie.
separate patch to fix it before this one?

> 
> >  	for (i = 0; i < ppgtt->num_pd_entries; i++) {
> >  		dma_addr_t pt_addr;
> >  
> > @@ -432,7 +432,8 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
> >  	struct scatterlist *sg = st->sgl;
> >  	const int first_entry = obj->gtt_space->start >> PAGE_SHIFT;
> >  	const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
> > -	gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gsm + first_entry;
> > +	gtt_pte_t __iomem *gtt_entries =
> > +		(gtt_pte_t __iomem *)dev_priv->mm.gsm + first_entry;
> >  	int unused, i = 0;
> >  	unsigned int len, m = 0;
> >  	dma_addr_t addr;
> > -- 
> > 1.8.0.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: Move even more gtt code to i915_gem_gtt

[Mika Kuoppala]

Ben Widawsky <benjamin.widawsky@intel.com> writes:

> This really should have been part of the kill agp series.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     |  7 ++---
>  drivers/gpu/drm/i915/i915_gem.c     | 51 ++------------------------------
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 59 ++++++++++++++++++++++++++++++++++---
>  3 files changed, 61 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2ab476d..75003c3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1585,10 +1585,9 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
>  				enum i915_cache_level cache_level);
>  void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj);
>  void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
> -void i915_gem_init_global_gtt(struct drm_device *dev,
> -			      unsigned long start,
> -			      unsigned long mappable_end,
> -			      unsigned long end);
> +void i915_gem_init_global_gtt(struct drm_device *dev);
> +void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
> +			       unsigned long mappable_end, unsigned long end);
>  int i915_gem_gtt_init(struct drm_device *dev);
>  void i915_gem_gtt_fini(struct drm_device *dev);
>  static inline void i915_gem_chipset_flush(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 4c6f043..42b948b 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -163,8 +163,8 @@ i915_gem_init_ioctl(struct drm_device *dev, void *data,
>  		return -ENODEV;
>  
>  	mutex_lock(&dev->struct_mutex);
> -	i915_gem_init_global_gtt(dev, args->gtt_start,
> -				 args->gtt_end, args->gtt_end);
> +	i915_gem_setup_global_gtt(dev, args->gtt_start, args->gtt_end,
> +				  args->gtt_end);
>  	mutex_unlock(&dev->struct_mutex);
>  
>  	return 0;
> @@ -3959,58 +3959,13 @@ cleanup_render_ring:
>  	return ret;
>  }
>  
> -static bool
> -intel_enable_ppgtt(struct drm_device *dev)
> -{
> -	if (i915_enable_ppgtt >= 0)
> -		return i915_enable_ppgtt;
> -
> -#ifdef CONFIG_INTEL_IOMMU
> -	/* Disable ppgtt on SNB if VT-d is on. */
> -	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
> -		return false;
> -#endif
> -
> -	return true;
> -}
> -
>  int i915_gem_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	unsigned long gtt_size, mappable_size;
>  	int ret;
>  
> -	gtt_size = dev_priv->mm.gtt->gtt_total_entries << PAGE_SHIFT;
> -	mappable_size = dev_priv->mm.gtt->gtt_mappable_entries << PAGE_SHIFT;
> -
>  	mutex_lock(&dev->struct_mutex);
> -	if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
> -		/* PPGTT pdes are stolen from global gtt ptes, so shrink the
> -		 * aperture accordingly when using aliasing ppgtt. */
> -		gtt_size -= I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
> -
> -		i915_gem_init_global_gtt(dev, 0, mappable_size, gtt_size);
> -
> -		ret = i915_gem_init_aliasing_ppgtt(dev);
> -		if (ret) {
> -			mutex_unlock(&dev->struct_mutex);
> -			return ret;
> -		}
> -	} else {
> -		/* Let GEM Manage all of the aperture.
> -		 *
> -		 * However, leave one page at the end still bound to the scratch
> -		 * page.  There are a number of places where the hardware
> -		 * apparently prefetches past the end of the object, and we've
> -		 * seen multiple hangs with the GPU head pointer stuck in a
> -		 * batchbuffer bound at the last page of the aperture.  One page
> -		 * should be enough to keep any prefetching inside of the
> -		 * aperture.
> -		 */
> -		i915_gem_init_global_gtt(dev, 0, mappable_size,
> -					 gtt_size);
> -	}
> -
> +	i915_gem_init_global_gtt(dev);
>  	ret = i915_gem_init_hw(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  	if (ret) {
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index b699a04..fc3c08a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -525,10 +525,10 @@ static void i915_gtt_color_adjust(struct drm_mm_node *node,
>  	}
>  }
>  
> -void i915_gem_init_global_gtt(struct drm_device *dev,
> -			      unsigned long start,
> -			      unsigned long mappable_end,
> -			      unsigned long end)
> +void i915_gem_setup_global_gtt(struct drm_device *dev,
> +			       unsigned long start,
> +			       unsigned long mappable_end,
> +			       unsigned long end)
>  {
>  	drm_i915_private_t *dev_priv = dev->dev_private;
>  	struct drm_mm_node *entry;
> @@ -573,6 +573,57 @@ void i915_gem_init_global_gtt(struct drm_device *dev,
>  	i915_ggtt_clear_range(dev, end / PAGE_SIZE - 1, 1);
>  }
>  
> +static bool
> +intel_enable_ppgtt(struct drm_device *dev)
> +{
> +	if (i915_enable_ppgtt >= 0)
> +		return i915_enable_ppgtt;
> +
> +#ifdef CONFIG_INTEL_IOMMU
> +	/* Disable ppgtt on SNB if VT-d is on. */
> +	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
> +		return false;
> +#endif
> +
> +	return true;
> +}
> +
> +void i915_gem_init_global_gtt(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	unsigned long gtt_size, mappable_size;
> +	int ret;
> +
> +	gtt_size = dev_priv->mm.gtt->gtt_total_entries << PAGE_SHIFT;
> +	mappable_size = dev_priv->mm.gtt->gtt_mappable_entries << PAGE_SHIFT;
> +
> +	if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
> +		/* PPGTT pdes are stolen from global gtt ptes, so shrink the
> +		 * aperture accordingly when using aliasing ppgtt. */
> +		gtt_size -= I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
> +
> +		i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
> +
> +		ret = i915_gem_init_aliasing_ppgtt(dev);
> +		if (ret) {
> +			mutex_unlock(&dev->struct_mutex);
> +			return;
> +		}
> +	} else {
> +		/* Let GEM Manage all of the aperture.
> +		 *
> +		 * However, leave one page at the end still bound to the scratch
> +		 * page.  There are a number of places where the hardware
> +		 * apparently prefetches past the end of the object, and we've
> +		 * seen multiple hangs with the GPU head pointer stuck in a
> +		 * batchbuffer bound at the last page of the aperture.  One page
> +		 * should be enough to keep any prefetching inside of the
> +		 * aperture.
> +		 */
> +		i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
> +	}
> +}
> +
>  static int setup_scratch_page(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -- 
> 1.8.0.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/6] drm/i915: Move GSM mapping into dev_priv

[Mika Kuoppala]

Ben Widawsky <benjamin.widawsky@intel.com> writes:

> This removes an unused field from the AGP structure and moves it into
> the dev_priv structure (with a slightly better name). This builds upon
> the kill-agp series already merged.
>
> GSM is a well defined term in the bspec:
> GSM: Graphics Stolen Memory
>
> GTT stolen space is defined for storage of the GFX GTT entries in
> physical memory. IA can not access GSM directly , it can only access via
> GTTMMADR. GT can access GSM directly or through GTTMMADR.
>
> This is not the entire stolen space.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/char/agp/intel-gtt.c        |  1 -
>  drivers/gpu/drm/i915/i915_drv.h     |  3 +++
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++-------
>  include/drm/intel-gtt.h             |  2 --
>  4 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> index dbd901e..c8d9dcb 100644
> --- a/drivers/char/agp/intel-gtt.c
> +++ b/drivers/char/agp/intel-gtt.c
> @@ -602,7 +602,6 @@ static int intel_gtt_init(void)
>  		iounmap(intel_private.registers);
>  		return -ENOMEM;
>  	}
> -	intel_private.base.gtt = intel_private.gtt;
>  
>  	global_cache_flush();   /* FIXME: ? */
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 75003c3..49f465a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -799,6 +799,9 @@ typedef struct drm_i915_private {
>  		unsigned long gtt_end;
>  		unsigned long stolen_base; /* limited to low memory (32-bit) */
>  
> +		/** "Graphics Stolen Memory" holds the global PTEs */
> +		uint32_t __iomem *gsm;
> +
>  		struct io_mapping *gtt_mapping;
>  		phys_addr_t gtt_base_addr;
>  		int gtt_mtrr;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index fc3c08a..b4c1e34 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -290,7 +290,7 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
>  		return;
>  
>  
> -	pd_addr = dev_priv->mm.gtt->gtt + ppgtt->pd_offset/sizeof(gtt_pte_t);
> +	pd_addr = dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t);
>  	for (i = 0; i < ppgtt->num_pd_entries; i++) {
>  		dma_addr_t pt_addr;
>  
> @@ -367,7 +367,7 @@ static void i915_ggtt_clear_range(struct drm_device *dev,
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	gtt_pte_t scratch_pte;
> -	gtt_pte_t __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry;
> +	gtt_pte_t __iomem *gtt_base = (gtt_pte_t __iomem *) dev_priv->mm.gsm + first_entry;
>  	const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
>  	int i;
>  
> @@ -432,7 +432,7 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
>  	struct scatterlist *sg = st->sgl;
>  	const int first_entry = obj->gtt_space->start >> PAGE_SHIFT;
>  	const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
> -	gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gtt->gtt + first_entry;
> +	gtt_pte_t __iomem *gtt_entries = dev_priv->mm.gsm + first_entry;
>  	int unused, i = 0;
>  	unsigned int len, m = 0;
>  	dma_addr_t addr;
> @@ -751,9 +751,9 @@ int i915_gem_gtt_init(struct drm_device *dev)
>  		goto err_out;
>  	}
>  
> -	dev_priv->mm.gtt->gtt = ioremap_wc(gtt_bus_addr,
> -					   dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
> -	if (!dev_priv->mm.gtt->gtt) {
> +	dev_priv->mm.gsm = ioremap_wc(gtt_bus_addr,
> +				      dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
> +	if (!dev_priv->mm.gsm) {
>  		DRM_ERROR("Failed to map the gtt page table\n");
>  		teardown_scratch_page(dev);
>  		ret = -ENOMEM;
> @@ -777,7 +777,7 @@ err_out:
>  void i915_gem_gtt_fini(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	iounmap(dev_priv->mm.gtt->gtt);
> +	iounmap(dev_priv->mm.gsm);
>  	teardown_scratch_page(dev);
>  	if (INTEL_INFO(dev)->gen < 6)
>  		intel_gmch_remove();
> diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
> index 6eb76a1..3e3a166 100644
> --- a/include/drm/intel-gtt.h
> +++ b/include/drm/intel-gtt.h
> @@ -18,8 +18,6 @@ struct intel_gtt {
>  	/* Share the scratch page dma with ppgtts. */
>  	dma_addr_t scratch_page_dma;
>  	struct page *scratch_page;
> -	/* for ppgtt PDE access */
> -	u32 __iomem *gtt;
>  	/* needed for ioremap in drm/i915 */
>  	phys_addr_t gma_bus_addr;
>  } *intel_gtt_get(void);
> -- 
> 1.8.0.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6 v2] drm/i915: Make GSM void

[Daniel Vetter]

On Wed, Dec 19, 2012 at 11:20:15AM -0800, Ben Widawsky wrote:
> On Wed, Dec 19, 2012 at 01:45:23PM +0200, Ville Syrjälä wrote:
> > On Tue, Dec 18, 2012 at 10:31:27AM -0800, Ben Widawsky wrote:
> > > The iomapping of the register region has historically been a uint32_t
> > > for the obvious reason that our PTE size was always 4b. In the future
> > > however, we cannot make this assumption.
> > > 
> > > By making the type void, it makes the upcoming pointer math we will do
> > > much easier, and hopefully gives the compiler opportunities to warn us
> > > when we do stupid things.
> > > 
> > > v2: Cast to __iomem, caught by Ville
> > > 
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h     | 2 +-
> > >  drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
> > >  2 files changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 49f465a..90dfbd5 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -800,7 +800,7 @@ typedef struct drm_i915_private {
> > >  		unsigned long stolen_base; /* limited to low memory (32-bit) */
> > >  
> > >  		/** "Graphics Stolen Memory" holds the global PTEs */
> > > -		uint32_t __iomem *gsm;
> > > +		void __iomem *gsm;
> > >  
> > >  		struct io_mapping *gtt_mapping;
> > >  		phys_addr_t gtt_base_addr;
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index b4c1e34..a52e784 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -290,7 +290,7 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
> > >  		return;
> > >  
> > >  
> > > -	pd_addr = dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t);
> > > +	pd_addr = (gtt_pte_t *)dev_priv->mm.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t);
> > 
> > This cast is still missing __iomem.
> 
> That looks to be correct. But it wasn't introduced by me, right? Ie.
> separate patch to fix it before this one?

Very much, and right in this patch - pd_addr and mm.gsm are both __iomem,
so you need to keep that annotation in the cast from void* to gtt_pte_t *.
Fixed up and the remaining three patches applied.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch