[07/13] drm/i915: Stop using gtt_total_entries

Commit Message

Ben Widawsky Jan. 17, 2013, 8:45 p.m. UTC

Similar to gtt_mappable_entries we don't usually want the entries,
it's easy enough to calculate it when you need.

v2: Move relevant fields above pre-gen6 init

Reviewed-by: Rodrigo Vivi <[e-mail]>
Signed-off-by: Ben Widawsky <[e-mail]>
---
 drivers/gpu/drm/i915/i915_drv.h     |  3 +++
 drivers/gpu/drm/i915/i915_gem_gtt.c | 33 ++++++++++++++++++---------------
 2 files changed, 21 insertions(+), 15 deletions(-)

Comments

Daniel Vetter Jan. 17, 2013, 11:04 p.m. UTC | #1

On Thu, Jan 17, 2013 at 12:45:18PM -0800, Ben Widawsky wrote:
> Similar to gtt_mappable_entries we don't usually want the entries,
> it's easy enough to calculate it when you need.
> 
> v2: Move relevant fields above pre-gen6 init
> 
> Reviewed-by: Rodrigo Vivi <[e-mail]>
> Signed-off-by: Ben Widawsky <[e-mail]>

[snip]

> @@ -687,11 +690,18 @@ int i915_gem_gtt_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	phys_addr_t gtt_bus_addr;
> +	u32 gtt_size;
>  	u16 snb_gmch_ctl;
>  	int ret;
>  
> +	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);
>  	dev_priv->gtt.mappable_base = pci_resource_start(dev->pdev, 2);
>  	dev_priv->gtt.mappable_end = pci_resource_len(dev->pdev, 2);
> +	gtt_size = gen6_get_total_gtt_size(snb_gmch_ctl);

Calculating gtt size like this doesn't work too well on pre-gen6.
-Daniel

Ben Widawsky Jan. 18, 2013, 12:10 a.m. UTC | #2

On Fri, 18 Jan 2013 00:04:24 +0100
Daniel Vetter <[e-mail]> wrote:

> On Thu, Jan 17, 2013 at 12:45:18PM -0800, Ben Widawsky wrote:
> > Similar to gtt_mappable_entries we don't usually want the entries,
> > it's easy enough to calculate it when you need.
> > 
> > v2: Move relevant fields above pre-gen6 init
> > 
> > Reviewed-by: Rodrigo Vivi <[e-mail]>
> > Signed-off-by: Ben Widawsky <[e-mail]>
> 
> [snip]
> 
> > @@ -687,11 +690,18 @@ int i915_gem_gtt_init(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	phys_addr_t gtt_bus_addr;
> > +	u32 gtt_size;
> >  	u16 snb_gmch_ctl;
> >  	int ret;
> >  
> > +	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);
> >  	dev_priv->gtt.mappable_base = pci_resource_start(dev->pdev, 2);
> >  	dev_priv->gtt.mappable_end = pci_resource_len(dev->pdev, 2);
> > +	gtt_size = gen6_get_total_gtt_size(snb_gmch_ctl);
> 
> Calculating gtt size like this doesn't work too well on pre-gen6.
> -Daniel


Cc Chris in case he has an opinion.

Daniel, you're right as usual. I botched this again. We actually care
about those values in so few places, I thought it might be okay, but it
looks like it's not.

Just to update others on our private IRC conversation, my plan unless it
turns out too ugly is to create a vtable with a gmch probing function.
The vtable is required for other stuff coming soon (more below).

Something like:
int (*gmch_probe)(dev, *mappable_size, *gtt_size, *stolen_size);
and then add/update the relevant code in agp/intel-gtt.c.

This allows me to still kill intel_gtt, which for whatever reason I'm
pretty gung-ho on killing. I'll need the vtable for things like
clear_range, and write_entry among some others I know of and probably a
few I haven't yet discovered.

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 80aee98..63938f3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -377,6 +377,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 used;			/* Amount used <= total */
 
 	unsigned long mappable_end;	/* End offset that we can CPU map */
 	struct io_mapping *mappable;	/* Mapping to our CPU mappable region */
@@ -384,7 +385,9 @@  struct i915_gtt {
 
 	/** "Graphics Stolen Memory" holds the global PTEs */
 	void __iomem *gsm;
+
 };
+#define gtt_total_entries(gtt) ((gtt).total >> PAGE_SHIFT)
 
 #define I915_PPGTT_PD_ENTRIES 512
 #define I915_PPGTT_PT_ENTRIES 1024
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0f0db02..283f244 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)
@@ -368,7 +369,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) {
@@ -431,7 +432,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;
@@ -557,7 +558,9 @@  void i915_gem_setup_global_gtt(struct drm_device *dev,
 	}
 
 	dev_priv->gtt.start = start;
-	dev_priv->gtt.total = end - start;
+	dev_priv->gtt.used = end - start;
+	if (dev_priv->gtt.used < dev_priv->gtt.total)
+		DRM_DEBUG_DRIVER("Warning: using less than total memory\n");
 
 	/* Clear any non-preallocated blocks */
 	drm_mm_for_each_hole(entry, &dev_priv->mm.gtt_space,
@@ -594,7 +597,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)) {
@@ -687,11 +690,18 @@  int i915_gem_gtt_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	phys_addr_t gtt_bus_addr;
+	u32 gtt_size;
 	u16 snb_gmch_ctl;
 	int ret;
 
+	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);
 	dev_priv->gtt.mappable_base = pci_resource_start(dev->pdev, 2);
 	dev_priv->gtt.mappable_end = pci_resource_len(dev->pdev, 2);
+	gtt_size = gen6_get_total_gtt_size(snb_gmch_ctl);
+	dev_priv->gtt.total = (gtt_size / sizeof(gtt_pte_t)) << PAGE_SHIFT;
 
 	/* On modern platforms we need not worry ourself with the legacy
 	 * hostbridge query stuff. Skip it entirely
@@ -716,9 +726,6 @@  int i915_gem_gtt_init(struct drm_device *dev)
 	if (!dev_priv->mm.gtt)
 		return -ENOMEM;
 
-	if (!pci_set_dma_mask(dev->pdev, DMA_BIT_MASK(40)))
-		pci_set_consistent_dma_mask(dev->pdev, DMA_BIT_MASK(40));
-
 #ifdef CONFIG_INTEL_IOMMU
 	dev_priv->mm.gtt->needs_dmar = 1;
 #endif
@@ -726,10 +733,6 @@  int i915_gem_gtt_init(struct drm_device *dev)
 	/* 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
@@ -752,8 +755,7 @@  int i915_gem_gtt_init(struct drm_device *dev)
 		goto err_out;
 	}
 
-	dev_priv->gtt.gsm = ioremap_wc(gtt_bus_addr,
-				       dev_priv->mm.gtt->gtt_total_entries * sizeof(gtt_pte_t));
+	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");
 		teardown_scratch_page(dev);
@@ -762,7 +764,8 @@  int i915_gem_gtt_init(struct drm_device *dev)
 	}
 
 	/* 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);