[PATCH 0/5 v2] [RFC] GTT vtable

[PATCH 0/5 v2] [RFC] GTT vtable

[Ben Widawsky]

Elaborating a bit more on what we can do with the vtable... still hoping
to get feedback.

Ben Widawsky (5):
  drm/i915: Create a vtable for i915 gtt
  drm/i915: Resume dissecting intel_gtt
  drm/i915: Extract gtt stolen calculations
  drm/i915: Extract clear_range to gtt_ops
  drm/i915: Extract bind object

 drivers/char/agp/intel-gtt.c               |  36 ++--
 drivers/gpu/drm/i915/i915_dma.c            |   2 +-
 drivers/gpu/drm/i915/i915_drv.h            |  34 +++-
 drivers/gpu/drm/i915/i915_gem.c            |  20 ++-
 drivers/gpu/drm/i915/i915_gem_context.c    |   4 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  10 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c        | 270 ++++++++++++++++-------------
 drivers/gpu/drm/i915/i915_gem_stolen.c     |   8 +-
 include/drm/intel-gtt.h                    |  10 +-
 9 files changed, 223 insertions(+), 171 deletions(-)

-- 
1.8.1.1

[PATCH 1/5] drm/i915: Create a vtable for i915 gtt

[Ben Widawsky]

The GTT handling has changed from generation to generation. There was a
particularly large shift around the gen5 time frame. The GTT itself
those provides a few core functions which make it a nice candidate for a
dispatch table. A similar concepts exists in the AGP layer, and were it
not for design constraints in those functions and code duplication in
i915, that would have been sufficient. In any case, it's the same idea.

One immediately obvious thing to implement is our gmch probing. The init
function was getting massively bloated. Fundamentally, all that's needed
from GMCH probing is the GTT size, and the stolen size. It makes design
sense to put the mappable calculation in there as well, but the code
turns out a bit nicer without it (IMO). As some examples though of
things which are nice for this table which will come up: PTE encoding,
range clearing, binding objects, entry updates, etc.

The intel_gtt bridge thing is still here, but the subsequent patches
will finish ripping that out.

A couple things worth note:
+ There are some additional cleanups in this patch to gem_gtt_init.
Those can be extracted if requested.

+ The DRM_INFO and DRM_DEBUG_DRIVER displaying the GTT info wasn't done
on pre-GEN6 before hand. Now is is.

- There is a pretty ugly hack regarding the kfree of the gtt struct.
It will be gone soon, I promise. I only left it because it shows
reviewers what the patch is striving to achieve with regard to the
cleanup of the gtt init function. I can fix it up if requested.

v2:
* Pass in the gtt struct instead of drm_device. This doesn't quite match
the typical dispatch table style, but it's much easier to get at the
things we care about this way, and a gtt->gtt_ops will always be 1:1.

* Drop unnecessary BUG_ON for new platforms... It's silly.

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

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 11c7aa8..fdfd6be 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1665,7 +1665,7 @@ out_mtrrfree:
 out_rmmap:
 	pci_iounmap(dev->pdev, dev_priv->regs);
 put_gmch:
-	i915_gem_gtt_fini(dev);
+	dev_priv->gtt.gtt_ops->gmch_remove(&dev_priv->gtt);
 put_bridge:
 	pci_dev_put(dev_priv->bridge_dev);
 free_priv:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c84743b..006caaa 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -364,6 +364,13 @@ struct intel_device_info {
 	u8 has_llc:1;
 };
 
+struct i915_gtt;
+struct i915_gtt_operations {
+	int (*gmch_probe)(struct i915_gtt *gtt,
+			  size_t *gtt_total, size_t *stolen);
+	void (*gmch_remove)(struct i915_gtt *gtt);
+};
+
 /* The Graphics Translation Table is the way in which GEN hardware translates a
  * Graphics Virtual Address into a Physical Address. In addition to the normal
  * collateral associated with any va->pa translations GEN hardware also has a
@@ -374,6 +381,7 @@ struct intel_device_info {
 struct i915_gtt {
 	unsigned long start;		/* Start offset of used GTT */
 	size_t total;			/* Total size GTT can map */
+	size_t stolen_size;		/* Total size of stolen memory */
 
 	unsigned long mappable_end;	/* End offset that we can CPU map */
 	struct io_mapping *mappable;	/* Mapping to our CPU mappable region */
@@ -385,6 +393,8 @@ struct i915_gtt {
 	bool do_idle_maps;
 	dma_addr_t scratch_page_dma;
 	struct page *scratch_page;
+
+	const struct i915_gtt_operations *gtt_ops;
 };
 
 #define I915_PPGTT_PD_ENTRIES 512
@@ -1663,7 +1673,6 @@ 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)
 {
 	if (INTEL_INFO(dev)->gen < 6)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a0ba4a9..11fb82c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -673,14 +673,14 @@ static inline unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl)
 	return snb_gmch_ctl << 20;
 }
 
-static inline unsigned int gen6_get_stolen_size(u16 snb_gmch_ctl)
+static inline size_t gen6_get_stolen_size(u16 snb_gmch_ctl)
 {
 	snb_gmch_ctl >>= SNB_GMCH_GMS_SHIFT;
 	snb_gmch_ctl &= SNB_GMCH_GMS_MASK;
 	return snb_gmch_ctl << 25; /* 32 MB units */
 }
 
-static inline unsigned int gen7_get_stolen_size(u16 snb_gmch_ctl)
+static inline size_t gen7_get_stolen_size(u16 snb_gmch_ctl)
 {
 	static const int stolen_decoder[] = {
 		0, 0, 0, 0, 0, 32, 48, 64, 128, 256, 96, 160, 224, 352};
@@ -689,103 +689,145 @@ static inline unsigned int gen7_get_stolen_size(u16 snb_gmch_ctl)
 	return stolen_decoder[snb_gmch_ctl] << 20;
 }
 
-int i915_gem_gtt_init(struct drm_device *dev)
+static int gen6_gmch_probe(struct i915_gtt *gtt,
+			   size_t *gtt_total,
+			   size_t *stolen)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv =
+		container_of(gtt, struct drm_i915_private, gtt);
+	struct drm_device *dev = dev_priv->dev;
 	phys_addr_t gtt_bus_addr;
+	unsigned int gtt_size;
 	u16 snb_gmch_ctl;
 	int ret;
 
-	dev_priv->gtt.mappable_base = pci_resource_start(dev->pdev, 2);
-	dev_priv->gtt.mappable_end = pci_resource_len(dev->pdev, 2);
+	if (!pci_set_dma_mask(dev->pdev, DMA_BIT_MASK(40)))
+		pci_set_consistent_dma_mask(dev->pdev, DMA_BIT_MASK(40));
+	pci_read_config_word(dev->pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
+	gtt_size = gen6_get_total_gtt_size(snb_gmch_ctl);
 
-	/* On modern platforms we need not worry ourself with the legacy
-	 * hostbridge query stuff. Skip it entirely
-	 */
-	if (INTEL_INFO(dev)->gen < 6) {
-		ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL);
-		if (!ret) {
-			DRM_ERROR("failed to set up gmch\n");
-			return -EIO;
-		}
+	if (IS_GEN7(dev))
+		*stolen = gen7_get_stolen_size(snb_gmch_ctl);
+	else
+		*stolen = gen6_get_stolen_size(snb_gmch_ctl);
 
-		dev_priv->mm.gtt = intel_gtt_get();
-		if (!dev_priv->mm.gtt) {
-			DRM_ERROR("Failed to initialize GTT\n");
-			intel_gmch_remove();
-			return -ENODEV;
-		}
+	*gtt_total = (gtt_size / sizeof(gtt_pte_t)) << PAGE_SHIFT;
 
-		dev_priv->gtt.do_idle_maps = needs_idle_maps(dev);
+	/* For GEN6+ the PTEs for the ggtt live at 2MB + BAR0 */
+	gtt_bus_addr = pci_resource_start(dev->pdev, 0) + (2<<20);
+	dev_priv->gtt.gsm = ioremap_wc(gtt_bus_addr, gtt_size);
+	if (!dev_priv->gtt.gsm) {
+		DRM_ERROR("Failed to map the gtt page table\n");
+		return -ENOMEM;
+	}
 
-		return 0;
+	ret = setup_scratch_page(dev);
+	if (ret)
+		DRM_ERROR("Scratch setup failed\n");
+
+	return ret;
+}
+
+void gen6_gmch_remove(struct i915_gtt *gtt)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(gtt, struct drm_i915_private, gtt);
+	iounmap(gtt->gsm);
+	teardown_scratch_page(dev_priv->dev);
+	kfree(dev_priv->mm.gtt);
+}
+
+static const struct i915_gtt_operations gen6_gtt_ops = {
+	.gmch_probe = gen6_gmch_probe,
+	.gmch_remove = gen6_gmch_remove,
+};
+
+static int i915_gmch_probe(struct i915_gtt *gtt,
+			   size_t *gtt_total,
+			   size_t *stolen)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(gtt, struct drm_i915_private, gtt);
+	int ret;
+
+	/* This is a temporary hack to make the code cleaner in
+	 * i915_gem_gtt_init. I promise it will go away very shortly. */
+	kfree(dev_priv->mm.gtt);
+
+	ret = intel_gmch_probe(dev_priv->bridge_dev, dev_priv->dev->pdev, NULL);
+	if (!ret) {
+		DRM_ERROR("failed to set up gmch\n");
+		return -EIO;
 	}
 
-	if (!pci_set_dma_mask(dev->pdev, DMA_BIT_MASK(40)))
-		pci_set_consistent_dma_mask(dev->pdev, DMA_BIT_MASK(40));
+	dev_priv->mm.gtt = intel_gtt_get();
+	if (!dev_priv->mm.gtt) {
+		DRM_ERROR("Failed to initialize GTT\n");
+		intel_gmch_remove();
+		return -ENODEV;
+	}
+
+	dev_priv->gtt.do_idle_maps = needs_idle_maps(dev_priv->dev);
+
+	*gtt_total = dev_priv->mm.gtt->gtt_total_entries << PAGE_SHIFT;
+	*stolen = dev_priv->mm.gtt->stolen_size << PAGE_SHIFT;
+
+	return 0;
+}
+
+static void i915_gmch_remove(struct i915_gtt *gtt)
+{
+	intel_gmch_remove();
+}
+
+static const struct i915_gtt_operations legacy_gtt_ops = {
+	.gmch_probe = i915_gmch_probe,
+	.gmch_remove = i915_gmch_remove,
+};
+
+int i915_gem_gtt_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct i915_gtt *gtt = &dev_priv->gtt;
+	unsigned long gtt_size;
+	int ret;
 
 	dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), GFP_KERNEL);
 	if (!dev_priv->mm.gtt)
 		return -ENOMEM;
 
-	/* For GEN6+ the PTEs for the ggtt live at 2MB + BAR0 */
-	gtt_bus_addr = pci_resource_start(dev->pdev, 0) + (2<<20);
-
-	/* i9xx_setup */
-	pci_read_config_word(dev->pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
-	dev_priv->mm.gtt->gtt_total_entries =
-		gen6_get_total_gtt_size(snb_gmch_ctl) / sizeof(gtt_pte_t);
-	if (INTEL_INFO(dev)->gen < 7)
-		dev_priv->mm.gtt->stolen_size = gen6_get_stolen_size(snb_gmch_ctl);
-	else
-		dev_priv->mm.gtt->stolen_size = gen7_get_stolen_size(snb_gmch_ctl);
+	gtt->mappable_base = pci_resource_start(dev->pdev, 2);
+	gtt->mappable_end = pci_resource_len(dev->pdev, 2);
 
 	/* 64/512MB is the current min/max we actually know of, but this is just a
 	 * coarse sanity check.
 	 */
-	if ((dev_priv->gtt.mappable_end < (64<<20) ||
-	    (dev_priv->gtt.mappable_end > (512<<20)))) {
-		DRM_ERROR("Unknown GMADR size (%lx)\n",
-			  dev_priv->gtt.mappable_end);
-		ret = -ENXIO;
-		goto err_out;
+	if ((gtt->mappable_end < (64<<20) || (gtt->mappable_end > (512<<20)))) {
+		DRM_ERROR("Unknown GMADR size (%lx)\n", gtt->mappable_end);
+		return -ENXIO;
 	}
 
-	ret = setup_scratch_page(dev);
+	if (INTEL_INFO(dev)->gen <= 5)
+		gtt->gtt_ops = &legacy_gtt_ops;
+	else
+		gtt->gtt_ops = &gen6_gtt_ops;
+
+	ret = gtt->gtt_ops->gmch_probe(gtt, &dev_priv->gtt.total,
+				       &dev_priv->gtt.stolen_size);
 	if (ret) {
-		DRM_ERROR("Scratch setup failed\n");
-		goto err_out;
+		kfree(dev_priv->mm.gtt);
+		return ret;
 	}
 
-	dev_priv->gtt.gsm = ioremap_wc(gtt_bus_addr,
-				       dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
-	if (!dev_priv->gtt.gsm) {
-		DRM_ERROR("Failed to map the gtt page table\n");
-		teardown_scratch_page(dev);
-		ret = -ENOMEM;
-		goto err_out;
-	}
+	dev_priv->mm.gtt->gtt_total_entries = dev_priv->gtt.total >> PAGE_SHIFT;
+	dev_priv->mm.gtt->stolen_size = dev_priv->gtt.stolen_size;
+
+	gtt_size = (dev_priv->gtt.total >> PAGE_SHIFT) * sizeof(gtt_pte_t);
 
 	/* GMADR is the PCI aperture used by SW to access tiled GFX surfaces in a linear fashion. */
-	DRM_INFO("Memory usable by graphics device = %dM\n", dev_priv->mm.gtt->gtt_total_entries >> 8);
+	DRM_INFO("Memory usable by graphics device = %zdM\n", dev_priv->gtt.total >> 20);
 	DRM_DEBUG_DRIVER("GMADR size = %ldM\n", dev_priv->gtt.mappable_end >> 20);
-	DRM_DEBUG_DRIVER("GTT stolen size = %dM\n", dev_priv->mm.gtt->stolen_size >> 20);
+	DRM_DEBUG_DRIVER("GTT stolen size = %zdM\n", dev_priv->gtt.stolen_size >> 20);
 
 	return 0;
-
-err_out:
-	kfree(dev_priv->mm.gtt);
-	if (INTEL_INFO(dev)->gen < 6)
-		intel_gmch_remove();
-	return ret;
-}
-
-void i915_gem_gtt_fini(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	iounmap(dev_priv->gtt.gsm);
-	teardown_scratch_page(dev);
-	if (INTEL_INFO(dev)->gen < 6)
-		intel_gmch_remove();
-	kfree(dev_priv->mm.gtt);
 }
-- 
1.8.1.1

[PATCH 2/5] drm/i915: Resume dissecting intel_gtt

[Ben Widawsky]

With the GMCH call in our dispatch table, we can now cut away two of the
remaining members in the intel_gtt shared struct.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/char/agp/intel-gtt.c           | 36 ++++++++++++++++++++--------------
 drivers/gpu/drm/i915/i915_drv.h        |  3 +--
 drivers/gpu/drm/i915/i915_gem_gtt.c    | 36 +++++++---------------------------
 drivers/gpu/drm/i915/i915_gem_stolen.c |  8 ++++----
 include/drm/intel-gtt.h                | 10 +---------
 5 files changed, 34 insertions(+), 59 deletions(-)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index ff5f348..d8e7e6c 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -60,7 +60,6 @@ struct intel_gtt_driver {
 };
 
 static struct _intel_private {
-	struct intel_gtt base;
 	const struct intel_gtt_driver *driver;
 	struct pci_dev *pcidev;	/* device one */
 	struct pci_dev *bridge_dev;
@@ -80,6 +79,13 @@ static struct _intel_private {
 	/* Whether i915 needs to use the dmar apis or not. */
 	unsigned int needs_dmar : 1;
 	phys_addr_t gma_bus_addr;
+	/*  Size of memory reserved for graphics by the BIOS */
+	unsigned int stolen_size;
+	/* Total number of gtt entries. */
+	unsigned int gtt_total_entries;
+	/* Part of the gtt that is mappable by the cpu, for those chips where
+	 * this is not the full gtt. */
+	unsigned int gtt_mappable_entries;
 } intel_private;
 
 #define INTEL_GTT_GEN	intel_private.driver->gen
@@ -510,7 +516,7 @@ static unsigned int intel_gtt_total_entries(void)
 		/* On previous hardware, the GTT size was just what was
 		 * required to map the aperture.
 		 */
-		return intel_private.base.gtt_mappable_entries;
+		return intel_private.gtt_mappable_entries;
 	}
 }
 
@@ -576,8 +582,8 @@ static int intel_gtt_init(void)
 	if (ret != 0)
 		return ret;
 
-	intel_private.base.gtt_mappable_entries = intel_gtt_mappable_entries();
-	intel_private.base.gtt_total_entries = intel_gtt_total_entries();
+	intel_private.gtt_mappable_entries = intel_gtt_mappable_entries();
+	intel_private.gtt_total_entries = intel_gtt_total_entries();
 
 	/* save the PGETBL reg for resume */
 	intel_private.PGETBL_save =
@@ -589,10 +595,10 @@ static int intel_gtt_init(void)
 
 	dev_info(&intel_private.bridge_dev->dev,
 			"detected gtt size: %dK total, %dK mappable\n",
-			intel_private.base.gtt_total_entries * 4,
-			intel_private.base.gtt_mappable_entries * 4);
+			intel_private.gtt_total_entries * 4,
+			intel_private.gtt_mappable_entries * 4);
 
-	gtt_map_size = intel_private.base.gtt_total_entries * 4;
+	gtt_map_size = intel_private.gtt_total_entries * 4;
 
 	intel_private.gtt = NULL;
 	if (INTEL_GTT_GEN < 6 && INTEL_GTT_GEN > 2)
@@ -609,7 +615,7 @@ static int intel_gtt_init(void)
 
 	global_cache_flush();   /* FIXME: ? */
 
-	intel_private.base.stolen_size = intel_gtt_stolen_size();
+	intel_private.stolen_size = intel_gtt_stolen_size();
 
 	intel_private.needs_dmar = USE_PCI_DMA_API && INTEL_GTT_GEN > 2;
 
@@ -637,8 +643,7 @@ static int intel_fake_agp_fetch_size(void)
 	unsigned int aper_size;
 	int i;
 
-	aper_size = (intel_private.base.gtt_mappable_entries << PAGE_SHIFT)
-		    / MB(1);
+	aper_size = (intel_private.gtt_mappable_entries << PAGE_SHIFT) / MB(1);
 
 	for (i = 0; i < num_sizes; i++) {
 		if (aper_size == intel_fake_agp_sizes[i].size) {
@@ -845,8 +850,8 @@ static int intel_fake_agp_insert_entries(struct agp_memory *mem,
 	int ret = -EINVAL;
 
 	if (intel_private.clear_fake_agp) {
-		int start = intel_private.base.stolen_size / PAGE_SIZE;
-		int end = intel_private.base.gtt_mappable_entries;
+		int start = intel_private.stolen_size / PAGE_SIZE;
+		int end = intel_private.gtt_mappable_entries;
 		intel_gtt_clear_range(start, end - start);
 		intel_private.clear_fake_agp = false;
 	}
@@ -857,7 +862,7 @@ static int intel_fake_agp_insert_entries(struct agp_memory *mem,
 	if (mem->page_count == 0)
 		goto out;
 
-	if (pg_start + mem->page_count > intel_private.base.gtt_total_entries)
+	if (pg_start + mem->page_count > intel_private.gtt_total_entries)
 		goto out_err;
 
 	if (type != mem->type)
@@ -1366,9 +1371,10 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
 }
 EXPORT_SYMBOL(intel_gmch_probe);
 
-struct intel_gtt *intel_gtt_get(void)
+void intel_gtt_get(size_t *gtt_total, size_t *stolen_size)
 {
-	return &intel_private.base;
+	*gtt_total = intel_private.gtt_total_entries << PAGE_SHIFT;
+	*stolen_size = intel_private.stolen_size;
 }
 EXPORT_SYMBOL(intel_gtt_get);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 006caaa..4c0e670 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -396,6 +396,7 @@ struct i915_gtt {
 
 	const struct i915_gtt_operations *gtt_ops;
 };
+#define gtt_total_entries(gtt) ((gtt).total >> PAGE_SHIFT)
 
 #define I915_PPGTT_PD_ENTRIES 512
 #define I915_PPGTT_PT_ENTRIES 1024
@@ -676,8 +677,6 @@ struct intel_l3_parity {
 };
 
 struct i915_gem_mm {
-	/** Bridge to intel-gtt-ko */
-	struct intel_gtt *gtt;
 	/** Memory allocator for GTT stolen memory */
 	struct drm_mm stolen;
 	/** Memory allocator for GTT */
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 11fb82c..d383add 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -119,7 +119,8 @@ int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
 	/* ppgtt PDEs reside in the global gtt pagetable, which has 512*1024
 	 * entries. For aliasing ppgtt support we just steal them at the end for
 	 * now. */
-	first_pd_entry_in_global_pt = dev_priv->mm.gtt->gtt_total_entries - I915_PPGTT_PD_ENTRIES;
+	first_pd_entry_in_global_pt =
+		gtt_total_entries(dev_priv->gtt) - I915_PPGTT_PD_ENTRIES;
 
 	ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
 	if (!ppgtt)
@@ -374,7 +375,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 = (gtt_pte_t __iomem *) dev_priv->gtt.gsm + first_entry;
-	const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
+	const int max_entries = gtt_total_entries(dev_priv->gtt) - first_entry;
 	int i;
 
 	if (INTEL_INFO(dev)->gen < 6) {
@@ -437,7 +438,7 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
 	struct sg_table *st = obj->pages;
 	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;
+	const int max_entries = gtt_total_entries(dev_priv->gtt) - first_entry;
 	gtt_pte_t __iomem *gtt_entries =
 		(gtt_pte_t __iomem *)dev_priv->gtt.gsm + first_entry;
 	int unused, i = 0;
@@ -600,7 +601,7 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
 	unsigned long gtt_size, mappable_size;
 	int ret;
 
-	gtt_size = dev_priv->mm.gtt->gtt_total_entries << PAGE_SHIFT;
+	gtt_size = dev_priv->gtt.total;
 	mappable_size = dev_priv->gtt.mappable_end;
 
 	if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
@@ -734,7 +735,6 @@ void gen6_gmch_remove(struct i915_gtt *gtt)
 		container_of(gtt, struct drm_i915_private, gtt);
 	iounmap(gtt->gsm);
 	teardown_scratch_page(dev_priv->dev);
-	kfree(dev_priv->mm.gtt);
 }
 
 static const struct i915_gtt_operations gen6_gtt_ops = {
@@ -750,28 +750,15 @@ static int i915_gmch_probe(struct i915_gtt *gtt,
 		container_of(gtt, struct drm_i915_private, gtt);
 	int ret;
 
-	/* This is a temporary hack to make the code cleaner in
-	 * i915_gem_gtt_init. I promise it will go away very shortly. */
-	kfree(dev_priv->mm.gtt);
-
 	ret = intel_gmch_probe(dev_priv->bridge_dev, dev_priv->dev->pdev, NULL);
 	if (!ret) {
 		DRM_ERROR("failed to set up gmch\n");
 		return -EIO;
 	}
 
-	dev_priv->mm.gtt = intel_gtt_get();
-	if (!dev_priv->mm.gtt) {
-		DRM_ERROR("Failed to initialize GTT\n");
-		intel_gmch_remove();
-		return -ENODEV;
-	}
-
+	intel_gtt_get(gtt_total, stolen);
 	dev_priv->gtt.do_idle_maps = needs_idle_maps(dev_priv->dev);
 
-	*gtt_total = dev_priv->mm.gtt->gtt_total_entries << PAGE_SHIFT;
-	*stolen = dev_priv->mm.gtt->stolen_size << PAGE_SHIFT;
-
 	return 0;
 }
 
@@ -792,10 +779,6 @@ int i915_gem_gtt_init(struct drm_device *dev)
 	unsigned long gtt_size;
 	int ret;
 
-	dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), GFP_KERNEL);
-	if (!dev_priv->mm.gtt)
-		return -ENOMEM;
-
 	gtt->mappable_base = pci_resource_start(dev->pdev, 2);
 	gtt->mappable_end = pci_resource_len(dev->pdev, 2);
 
@@ -814,13 +797,8 @@ int i915_gem_gtt_init(struct drm_device *dev)
 
 	ret = gtt->gtt_ops->gmch_probe(gtt, &dev_priv->gtt.total,
 				       &dev_priv->gtt.stolen_size);
-	if (ret) {
-		kfree(dev_priv->mm.gtt);
+	if (ret)
 		return ret;
-	}
-
-	dev_priv->mm.gtt->gtt_total_entries = dev_priv->gtt.total >> PAGE_SHIFT;
-	dev_priv->mm.gtt->stolen_size = dev_priv->gtt.stolen_size;
 
 	gtt_size = (dev_priv->gtt.total >> PAGE_SHIFT) * sizeof(gtt_pte_t);
 
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index f21ae17..69d97cb 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -187,11 +187,11 @@ int i915_gem_init_stolen(struct drm_device *dev)
 	if (dev_priv->mm.stolen_base == 0)
 		return 0;
 
-	DRM_DEBUG_KMS("found %d bytes of stolen memory at %08lx\n",
-		      dev_priv->mm.gtt->stolen_size, dev_priv->mm.stolen_base);
+	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
+		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
 
 	/* Basic memrange allocator for stolen space */
-	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->mm.gtt->stolen_size);
+	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size);
 
 	return 0;
 }
@@ -205,7 +205,7 @@ i915_pages_create_for_stolen(struct drm_device *dev,
 	struct scatterlist *sg;
 
 	DRM_DEBUG_DRIVER("offset=0x%x, size=%d\n", offset, size);
-	BUG_ON(offset > dev_priv->mm.gtt->stolen_size - size);
+	BUG_ON(offset > dev_priv->gtt.stolen_size - size);
 
 	/* We hide that we have no struct page backing our stolen object
 	 * by wrapping the contiguous physical allocation with a fake
diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
index 769b6c7..cf10555 100644
--- a/include/drm/intel-gtt.h
+++ b/include/drm/intel-gtt.h
@@ -3,15 +3,7 @@
 #ifndef _DRM_INTEL_GTT_H
 #define	_DRM_INTEL_GTT_H
 
-struct intel_gtt {
-	/* Size of memory reserved for graphics by the BIOS */
-	unsigned int stolen_size;
-	/* Total number of gtt entries. */
-	unsigned int gtt_total_entries;
-	/* Part of the gtt that is mappable by the cpu, for those chips where
-	 * this is not the full gtt. */
-	unsigned int gtt_mappable_entries;
-} *intel_gtt_get(void);
+void intel_gtt_get(size_t *gtt_total, size_t *stolen_size);
 
 int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
 		     struct agp_bridge_data *bridge);
-- 
1.8.1.1

[PATCH 3/5] drm/i915: Extract gtt stolen calculations

[Ben Widawsky]

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

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4c0e670..d462875 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -369,6 +369,9 @@ struct i915_gtt_operations {
 	int (*gmch_probe)(struct i915_gtt *gtt,
 			  size_t *gtt_total, size_t *stolen);
 	void (*gmch_remove)(struct i915_gtt *gtt);
+
+	/* GEN6+ helpers */
+	size_t (*get_stolen_size)(u16 gmch_ctl);
 };
 
 /* The Graphics Translation Table is the way in which GEN hardware translates a
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d383add..2f2b1ba 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -707,11 +707,7 @@ static int gen6_gmch_probe(struct i915_gtt *gtt,
 	pci_read_config_word(dev->pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
 	gtt_size = gen6_get_total_gtt_size(snb_gmch_ctl);
 
-	if (IS_GEN7(dev))
-		*stolen = gen7_get_stolen_size(snb_gmch_ctl);
-	else
-		*stolen = gen6_get_stolen_size(snb_gmch_ctl);
-
+	*stolen = gtt->gtt_ops->get_stolen_size(snb_gmch_ctl);
 	*gtt_total = (gtt_size / sizeof(gtt_pte_t)) << PAGE_SHIFT;
 
 	/* For GEN6+ the PTEs for the ggtt live at 2MB + BAR0 */
@@ -740,6 +736,13 @@ void gen6_gmch_remove(struct i915_gtt *gtt)
 static const struct i915_gtt_operations gen6_gtt_ops = {
 	.gmch_probe = gen6_gmch_probe,
 	.gmch_remove = gen6_gmch_remove,
+	.get_stolen_size = gen6_get_stolen_size,
+};
+
+static const struct i915_gtt_operations gen7_gtt_ops = {
+	.gmch_probe = gen6_gmch_probe,
+	.gmch_remove = gen6_gmch_remove,
+	.get_stolen_size = gen7_get_stolen_size,
 };
 
 static int i915_gmch_probe(struct i915_gtt *gtt,
@@ -792,9 +795,12 @@ int i915_gem_gtt_init(struct drm_device *dev)
 
 	if (INTEL_INFO(dev)->gen <= 5)
 		gtt->gtt_ops = &legacy_gtt_ops;
-	else
+	else if(IS_GEN6(dev))
 		gtt->gtt_ops = &gen6_gtt_ops;
-
+	else if(IS_GEN7(dev))
+		gtt->gtt_ops = &gen7_gtt_ops;
+	else
+		BUG();
 	ret = gtt->gtt_ops->gmch_probe(gtt, &dev_priv->gtt.total,
 				       &dev_priv->gtt.stolen_size);
 	if (ret)
-- 
1.8.1.1

[PATCH 4/5] drm/i915: Extract clear_range to gtt_ops

[Ben Widawsky]

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

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d462875..4d7990f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -369,6 +369,11 @@ struct i915_gtt_operations {
 	int (*gmch_probe)(struct i915_gtt *gtt,
 			  size_t *gtt_total, size_t *stolen);
 	void (*gmch_remove)(struct i915_gtt *gtt);
+	void (*clear_range)(struct i915_gtt *gtt,
+			    unsigned int first_entry,
+			    size_t num_entries);
+#define unbind_object(gtt, obj) \
+	clear_range(gtt, obj->gtt_space->start >> PAGE_SHIFT, obj->base.size >> PAGE_SHIFT)
 
 	/* GEN6+ helpers */
 	size_t (*get_stolen_size)(u16 gmch_ctl);
@@ -1669,7 +1674,6 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev);
 int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj);
 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);
 void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5bb370f..66362b4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2461,8 +2461,10 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj)
 
 	trace_i915_gem_object_unbind(obj);
 
-	if (obj->has_global_gtt_mapping)
-		i915_gem_gtt_unbind_object(obj);
+	if (obj->has_global_gtt_mapping) {
+		dev_priv->gtt.gtt_ops->unbind_object(&dev_priv->gtt, obj);
+		obj->has_global_gtt_mapping = 0;
+	}
 	if (obj->has_aliasing_ppgtt_mapping) {
 		i915_ppgtt_unbind_object(dev_priv->mm.aliasing_ppgtt, obj);
 		obj->has_aliasing_ppgtt_mapping = 0;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 2f2b1ba..5ae1d13 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -368,23 +368,20 @@ static void undo_idling(struct drm_i915_private *dev_priv, bool interruptible)
 		dev_priv->mm.interruptible = interruptible;
 }
 
-static void i915_ggtt_clear_range(struct drm_device *dev,
-				 unsigned first_entry,
-				 unsigned num_entries)
+static void gen6_gtt_clear_range(struct i915_gtt *gtt,
+				 unsigned int first_entry,
+				 size_t num_entries)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv =
+		container_of(gtt, struct drm_i915_private, gtt);
+	struct drm_device *dev = dev_priv->dev;
 	gtt_pte_t scratch_pte;
 	gtt_pte_t __iomem *gtt_base = (gtt_pte_t __iomem *) dev_priv->gtt.gsm + first_entry;
 	const int max_entries = gtt_total_entries(dev_priv->gtt) - first_entry;
 	int i;
 
-	if (INTEL_INFO(dev)->gen < 6) {
-		intel_gtt_clear_range(first_entry, num_entries);
-		return;
-	}
-
 	if (WARN(num_entries > max_entries,
-		 "First entry = %d; Num entries = %d (max=%d)\n",
+		 "First entry = %d; Num entries = %zd (max=%d)\n",
 		 first_entry, num_entries, max_entries))
 		num_entries = max_entries;
 
@@ -397,11 +394,12 @@ static void i915_ggtt_clear_range(struct drm_device *dev,
 void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct i915_gtt *gtt = &dev_priv->gtt;
 	struct drm_i915_gem_object *obj;
 
-	/* First fill our portion of the GTT with scratch pages */
-	i915_ggtt_clear_range(dev, dev_priv->gtt.start / PAGE_SIZE,
-			      dev_priv->gtt.total / PAGE_SIZE);
+	gtt->gtt_ops->clear_range(gtt,
+				  gtt->start >> PAGE_SHIFT,
+				  gtt->total >> PAGE_SHIFT);
 
 	list_for_each_entry(obj, &dev_priv->mm.bound_list, gtt_list) {
 		i915_gem_clflush_object(obj);
@@ -491,15 +489,6 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
 	obj->has_global_gtt_mapping = 1;
 }
 
-void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj)
-{
-	i915_ggtt_clear_range(obj->base.dev,
-			      obj->gtt_space->start >> PAGE_SHIFT,
-			      obj->base.size >> PAGE_SHIFT);
-
-	obj->has_global_gtt_mapping = 0;
-}
-
 void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj)
 {
 	struct drm_device *dev = obj->base.dev;
@@ -542,6 +531,7 @@ void i915_gem_setup_global_gtt(struct drm_device *dev,
 	struct drm_mm_node *entry;
 	struct drm_i915_gem_object *obj;
 	unsigned long hole_start, hole_end;
+	struct i915_gtt *gtt = &dev_priv->gtt;
 
 	BUG_ON(mappable_end > end);
 
@@ -563,21 +553,21 @@ void i915_gem_setup_global_gtt(struct drm_device *dev,
 		obj->has_global_gtt_mapping = 1;
 	}
 
-	dev_priv->gtt.start = start;
-	dev_priv->gtt.total = end - start;
+	gtt->start = start;
+	gtt->total = end - start;
 
 	/* Clear any non-preallocated blocks */
 	drm_mm_for_each_hole(entry, &dev_priv->mm.gtt_space,
 			     hole_start, hole_end) {
 		DRM_DEBUG_KMS("clearing unused GTT space: [%lx, %lx]\n",
 			      hole_start, hole_end);
-		i915_ggtt_clear_range(dev,
-				      hole_start / PAGE_SIZE,
-				      (hole_end-hole_start) / PAGE_SIZE);
+		gtt->gtt_ops->clear_range(gtt,
+					  hole_start / PAGE_SIZE,
+					  (hole_end-hole_start) / PAGE_SIZE);
 	}
 
 	/* And finally clear the reserved guard page */
-	i915_ggtt_clear_range(dev, end / PAGE_SIZE - 1, 1);
+	gtt->gtt_ops->clear_range(gtt, end / PAGE_SIZE - 1, 1);
 }
 
 static bool
@@ -736,12 +726,14 @@ void gen6_gmch_remove(struct i915_gtt *gtt)
 static const struct i915_gtt_operations gen6_gtt_ops = {
 	.gmch_probe = gen6_gmch_probe,
 	.gmch_remove = gen6_gmch_remove,
+	.clear_range = gen6_gtt_clear_range,
 	.get_stolen_size = gen6_get_stolen_size,
 };
 
 static const struct i915_gtt_operations gen7_gtt_ops = {
 	.gmch_probe = gen6_gmch_probe,
 	.gmch_remove = gen6_gmch_remove,
+	.clear_range = gen6_gtt_clear_range,
 	.get_stolen_size = gen7_get_stolen_size,
 };
 
@@ -770,9 +762,17 @@ static void i915_gmch_remove(struct i915_gtt *gtt)
 	intel_gmch_remove();
 }
 
+static void i915_gtt_clear_range(struct i915_gtt *gtt,
+				 unsigned int first_entry,
+				 size_t num_entries)
+{
+	intel_gtt_clear_range(first_entry, num_entries);
+}
+
 static const struct i915_gtt_operations legacy_gtt_ops = {
 	.gmch_probe = i915_gmch_probe,
 	.gmch_remove = i915_gmch_remove,
+	.clear_range = i915_gtt_clear_range,
 };
 
 int i915_gem_gtt_init(struct drm_device *dev)
-- 
1.8.1.1

[PATCH 5/5] drm/i915: Extract bind object

[Ben Widawsky]

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.h            | 11 +++++++++--
 drivers/gpu/drm/i915/i915_gem.c            | 14 ++++++++------
 drivers/gpu/drm/i915/i915_gem_context.c    |  4 +++-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 10 ++++++----
 drivers/gpu/drm/i915/i915_gem_gtt.c        | 30 ++++++++++++++----------------
 5 files changed, 40 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4d7990f..62559b5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -365,6 +365,7 @@ struct intel_device_info {
 };
 
 struct i915_gtt;
+enum i915_cache_level;
 struct i915_gtt_operations {
 	int (*gmch_probe)(struct i915_gtt *gtt,
 			  size_t *gtt_total, size_t *stolen);
@@ -372,6 +373,9 @@ struct i915_gtt_operations {
 	void (*clear_range)(struct i915_gtt *gtt,
 			    unsigned int first_entry,
 			    size_t num_entries);
+	void (*bind_object)(struct i915_gtt *gtt,
+			    struct drm_i915_gem_object *obj,
+			    enum i915_cache_level cache_level);
 #define unbind_object(gtt, obj) \
 	clear_range(gtt, obj->gtt_space->start >> PAGE_SHIFT, obj->base.size >> PAGE_SHIFT)
 
@@ -1672,8 +1676,11 @@ void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
 
 void i915_gem_restore_gtt_mappings(struct drm_device *dev);
 int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj);
-void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
-				enum i915_cache_level cache_level);
+#define i915_gem_gtt_bind_object(dev_priv, obj, cache_level) \
+	do { \
+		dev_priv->gtt.gtt_ops->bind_object(&dev_priv->gtt, \
+						   obj, cache_level); \
+	} while(0)
 void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
 void i915_gem_init_global_gtt(struct drm_device *dev);
 void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 66362b4..97c19d2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3153,8 +3153,10 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 				return ret;
 		}
 
-		if (obj->has_global_gtt_mapping)
-			i915_gem_gtt_bind_object(obj, cache_level);
+		if (obj->has_global_gtt_mapping) {
+			i915_gem_gtt_bind_object(dev_priv, obj, cache_level);
+			obj->has_global_gtt_mapping = 1;
+		}
 		if (obj->has_aliasing_ppgtt_mapping)
 			i915_ppgtt_bind_object(dev_priv->mm.aliasing_ppgtt,
 					       obj, cache_level);
@@ -3433,6 +3435,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
 		    bool map_and_fenceable,
 		    bool nonblocking)
 {
+	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
 	int ret;
 
 	if (WARN_ON(obj->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT))
@@ -3455,8 +3458,6 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
 	}
 
 	if (obj->gtt_space == NULL) {
-		struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
-
 		ret = i915_gem_object_bind_to_gtt(obj, alignment,
 						  map_and_fenceable,
 						  nonblocking);
@@ -3464,11 +3465,12 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
 			return ret;
 
 		if (!dev_priv->mm.aliasing_ppgtt)
-			i915_gem_gtt_bind_object(obj, obj->cache_level);
+			i915_gem_gtt_bind_object(dev_priv, obj,
+						 obj->cache_level);
 	}
 
 	if (!obj->has_global_gtt_mapping && map_and_fenceable)
-		i915_gem_gtt_bind_object(obj, obj->cache_level);
+		i915_gem_gtt_bind_object(dev_priv, obj, obj->cache_level);
 
 	obj->pin_count++;
 	obj->pin_mappable |= map_and_fenceable;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index a3f06bc..b3304c3 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -365,6 +365,7 @@ mi_set_context(struct intel_ring_buffer *ring,
 static int do_switch(struct i915_hw_context *to)
 {
 	struct intel_ring_buffer *ring = to->ring;
+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	struct drm_i915_gem_object *from_obj = ring->last_context_obj;
 	u32 hw_flags = 0;
 	int ret;
@@ -390,7 +391,8 @@ static int do_switch(struct i915_hw_context *to)
 	}
 
 	if (!to->obj->has_global_gtt_mapping)
-		i915_gem_gtt_bind_object(to->obj, to->obj->cache_level);
+		i915_gem_gtt_bind_object(dev_priv, to->obj,
+					 to->obj->cache_level);
 
 	if (!to->is_initialized || is_default_context(to))
 		hw_flags |= MI_RESTORE_INHIBIT;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 2726910..496f7f2 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -177,6 +177,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
 				   struct drm_i915_gem_relocation_entry *reloc)
 {
 	struct drm_device *dev = obj->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_gem_object *target_obj;
 	struct drm_i915_gem_object *target_i915_obj;
 	uint32_t target_offset;
@@ -196,7 +197,8 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
 	if (unlikely(IS_GEN6(dev) &&
 	    reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION &&
 	    !target_i915_obj->has_global_gtt_mapping)) {
-		i915_gem_gtt_bind_object(target_i915_obj,
+		i915_gem_gtt_bind_object(dev_priv,
+					 target_i915_obj,
 					 target_i915_obj->cache_level);
 	}
 
@@ -267,7 +269,6 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
 		*(uint32_t *)(vaddr + page_offset) = reloc->delta;
 		kunmap_atomic(vaddr);
 	} else {
-		struct drm_i915_private *dev_priv = dev->dev_private;
 		uint32_t __iomem *reloc_entry;
 		void __iomem *reloc_page;
 
@@ -449,7 +450,7 @@ i915_gem_execbuffer_reserve_object(struct drm_i915_gem_object *obj,
 
 	if (entry->flags & EXEC_OBJECT_NEEDS_GTT &&
 	    !obj->has_global_gtt_mapping)
-		i915_gem_gtt_bind_object(obj, obj->cache_level);
+		i915_gem_gtt_bind_object(dev_priv, obj, obj->cache_level);
 
 	return 0;
 }
@@ -990,7 +991,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	 * hsw should have this fixed, but let's be paranoid and do it
 	 * unconditionally for now. */
 	if (flags & I915_DISPATCH_SECURE && !batch_obj->has_global_gtt_mapping)
-		i915_gem_gtt_bind_object(batch_obj, batch_obj->cache_level);
+		i915_gem_gtt_bind_object(dev_priv, batch_obj,
+					 batch_obj->cache_level);
 
 	ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->objects);
 	if (ret)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5ae1d13..c4e47a0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -403,7 +403,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 
 	list_for_each_entry(obj, &dev_priv->mm.bound_list, gtt_list) {
 		i915_gem_clflush_object(obj);
-		i915_gem_gtt_bind_object(obj, obj->cache_level);
+		i915_gem_gtt_bind_object(dev_priv, obj, obj->cache_level);
 	}
 
 	i915_gem_chipset_flush(dev);
@@ -428,8 +428,9 @@ int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
  * within the global GTT as well as accessible by the GPU through the GMADR
  * mapped BAR (dev_priv->mm.gtt->gtt).
  */
-static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
-				  enum i915_cache_level level)
+static void gen6_gtt_bind_object(struct i915_gtt *gtt,
+				 struct drm_i915_gem_object *obj,
+				 enum i915_cache_level level)
 {
 	struct drm_device *dev = obj->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -472,21 +473,15 @@ static void gen6_ggtt_bind_object(struct drm_i915_gem_object *obj,
 	POSTING_READ(GFX_FLSH_CNTL_GEN6);
 }
 
-void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
-			      enum i915_cache_level cache_level)
+void i915_gtt_bind_object(struct i915_gtt *gtt,
+			  struct drm_i915_gem_object *obj,
+			  enum i915_cache_level cache_level)
 {
-	struct drm_device *dev = obj->base.dev;
-	if (INTEL_INFO(dev)->gen < 6) {
-		unsigned int flags = (cache_level == I915_CACHE_NONE) ?
+	unsigned int flags = (cache_level == I915_CACHE_NONE) ?
 			AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY;
-		intel_gtt_insert_sg_entries(obj->pages,
-					    obj->gtt_space->start >> PAGE_SHIFT,
-					    flags);
-	} else {
-		gen6_ggtt_bind_object(obj, cache_level);
-	}
-
-	obj->has_global_gtt_mapping = 1;
+	intel_gtt_insert_sg_entries(obj->pages,
+				    obj->gtt_space->start >> PAGE_SHIFT,
+				    flags);
 }
 
 void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj)
@@ -727,6 +722,7 @@ static const struct i915_gtt_operations gen6_gtt_ops = {
 	.gmch_probe = gen6_gmch_probe,
 	.gmch_remove = gen6_gmch_remove,
 	.clear_range = gen6_gtt_clear_range,
+	.bind_object = gen6_gtt_bind_object,
 	.get_stolen_size = gen6_get_stolen_size,
 };
 
@@ -734,6 +730,7 @@ static const struct i915_gtt_operations gen7_gtt_ops = {
 	.gmch_probe = gen6_gmch_probe,
 	.gmch_remove = gen6_gmch_remove,
 	.clear_range = gen6_gtt_clear_range,
+	.bind_object = gen6_gtt_bind_object,
 	.get_stolen_size = gen7_get_stolen_size,
 };
 
@@ -773,6 +770,7 @@ static const struct i915_gtt_operations legacy_gtt_ops = {
 	.gmch_probe = i915_gmch_probe,
 	.gmch_remove = i915_gmch_remove,
 	.clear_range = i915_gtt_clear_range,
+	.bind_object = i915_gtt_bind_object,
 };
 
 int i915_gem_gtt_init(struct drm_device *dev)
-- 
1.8.1.1

Re: [PATCH 0/5 v2] [RFC] GTT vtable

[Chris Wilson]

On Mon, Jan 21, 2013 at 02:10:31PM -0800, Ben Widawsky wrote:
> Elaborating a bit more on what we can do with the vtable... still hoping
> to get feedback.

Multiple layers of identical abstraction, just what we like!

One thing that we lost in this switch is a small string identifying the
chipset. When reading through a dmesg it is very handy to the GPU name
in there, and we may as well print the pci-id.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 0/5 v2] [RFC] GTT vtable

[Daniel Vetter]

On Wed, Jan 23, 2013 at 12:03 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Mon, Jan 21, 2013 at 02:10:31PM -0800, Ben Widawsky wrote:
>> Elaborating a bit more on what we can do with the vtable... still hoping
>> to get feedback.
>
> Multiple layers of identical abstraction, just what we like!
>
> One thing that we lost in this switch is a small string identifying the
> chipset. When reading through a dmesg it is very handy to the GPU name
> in there, and we may as well print the pci-id.

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

Re: [PATCH 0/5 v2] [RFC] GTT vtable

[Chris Wilson]

On Wed, Jan 23, 2013 at 12:39:01PM +0100, Daniel Vetter wrote:
> On Wed, Jan 23, 2013 at 12:03 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Mon, Jan 21, 2013 at 02:10:31PM -0800, Ben Widawsky wrote:
> >> Elaborating a bit more on what we can do with the vtable... still hoping
> >> to get feedback.
> >
> > Multiple layers of identical abstraction, just what we like!
> >
> > One thing that we lost in this switch is a small string identifying the
> > chipset. When reading through a dmesg it is very handy to the GPU name
> > in there, and we may as well print the pci-id.
> 
> i915_dump_device_info not good enough?

No, it's a keyword I use to familarise myself and sanity check random
dmesgs attached to bug reports.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre