[PATCH 01/10] drm/i915: generalize pte vs. register BAR allocation

[PATCH 01/10] drm/i915: generalize pte vs. register BAR allocation

[Ben Widawsky]

All gen6+ parts so far have 1 BAR which holds both the register space
and the GTT PTEs. Up until now, that was a 4MB BAR with half allocated
to each.

I have a strong hunch (wink, nod, wink) that future gens will also keep
a similar 50-50 split though the sizes may change. To help this along
change the code to obey the rule of half the total size instead of a
hard-coded 2MB.

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

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 24a23b3..215d72c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -738,8 +738,10 @@ static int gen6_gmch_probe(struct drm_device *dev,
 
 	*gtt_total = (gtt_size / sizeof(gtt_pte_t)) << PAGE_SHIFT;
 
-	/* For GEN6+ the PTEs for the ggtt live at 2MB + BAR0 */
-	gtt_bus_addr = pci_resource_start(dev->pdev, 0) + (2<<20);
+	/* For Modern GENs the PTEs and register space are split in the BAR */
+	gtt_bus_addr = pci_resource_start(dev->pdev, 0) +
+		(pci_resource_len(dev->pdev, 0) / 2);
+
 	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");
-- 
1.8.2.1

[PATCH 02/10] drm/i915: Call out GEN6 PTE specificity

[Ben Widawsky]

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

We can assume that the PTE layout, and size changes for future
generations. To avoid confusion with the existing GEN6 PTE typedef, give
it a GEN6_ prefix.

v2: Fixup checkpatch warning and bikeshed commit message slightly.

v3: Rebase on top of Imre's for_each_sg_pages rework.

v4: Fixup conflicts in patch series reordering.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 215d72c..091b270 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -28,7 +28,7 @@
 #include "i915_trace.h"
 #include "intel_drv.h"
 
-typedef uint32_t gtt_pte_t;
+typedef uint32_t gen6_gtt_pte_t;
 
 /* PPGTT stuff */
 #define GEN6_GTT_ADDR_ENCODE(addr)	((addr) | (((addr) >> 28) & 0xff0))
@@ -44,11 +44,11 @@ typedef uint32_t gtt_pte_t;
 #define GEN6_PTE_CACHE_LLC_MLC		(3 << 1)
 #define GEN6_PTE_ADDR_ENCODE(addr)	GEN6_GTT_ADDR_ENCODE(addr)
 
-static inline gtt_pte_t gen6_pte_encode(struct drm_device *dev,
+static inline gen6_gtt_pte_t gen6_pte_encode(struct drm_device *dev,
 					dma_addr_t addr,
 					enum i915_cache_level level)
 {
-	gtt_pte_t pte = GEN6_PTE_VALID;
+	gen6_gtt_pte_t pte = GEN6_PTE_VALID;
 	pte |= GEN6_PTE_ADDR_ENCODE(addr);
 
 	switch (level) {
@@ -72,7 +72,6 @@ static inline gtt_pte_t gen6_pte_encode(struct drm_device *dev,
 		BUG();
 	}
 
-
 	return pte;
 }
 
@@ -81,8 +80,7 @@ static void gen6_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
 				   unsigned first_entry,
 				   unsigned num_entries)
 {
-	gtt_pte_t *pt_vaddr;
-	gtt_pte_t scratch_pte;
+	gen6_gtt_pte_t *pt_vaddr, scratch_pte;
 	unsigned act_pt = first_entry / I915_PPGTT_PT_ENTRIES;
 	unsigned first_pte = first_entry % I915_PPGTT_PT_ENTRIES;
 	unsigned last_pte, i;
@@ -114,7 +112,7 @@ static void gen6_ppgtt_insert_entries(struct i915_hw_ppgtt *ppgtt,
 				      unsigned first_entry,
 				      enum i915_cache_level cache_level)
 {
-	gtt_pte_t *pt_vaddr;
+	gen6_gtt_pte_t *pt_vaddr;
 	unsigned act_pt = first_entry / I915_PPGTT_PT_ENTRIES;
 	unsigned act_pte = first_entry % I915_PPGTT_PT_ENTRIES;
 	struct sg_page_iter sg_iter;
@@ -208,7 +206,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 	ppgtt->clear_range(ppgtt, 0,
 			   ppgtt->num_pd_entries*I915_PPGTT_PT_ENTRIES);
 
-	ppgtt->pd_offset = (first_pd_entry_in_global_pt)*sizeof(gtt_pte_t);
+	ppgtt->pd_offset = first_pd_entry_in_global_pt * sizeof(gen6_gtt_pte_t);
 
 	return 0;
 
@@ -284,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;
-	gtt_pte_t __iomem *pd_addr;
+	gen6_gtt_pte_t __iomem *pd_addr;
 	uint32_t pd_entry;
 	int i;
 
@@ -292,7 +290,8 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
 		return;
 
 
-	pd_addr = (gtt_pte_t __iomem*)dev_priv->gtt.gsm + ppgtt->pd_offset/sizeof(gtt_pte_t);
+	pd_addr = (gen6_gtt_pte_t __iomem *)dev_priv->gtt.gsm +
+		ppgtt->pd_offset / sizeof(gen6_gtt_pte_t);
 	for (i = 0; i < ppgtt->num_pd_entries; i++) {
 		dma_addr_t pt_addr;
 
@@ -416,8 +415,8 @@ static void gen6_ggtt_insert_entries(struct drm_device *dev,
 				     enum i915_cache_level level)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	gtt_pte_t __iomem *gtt_entries =
-		(gtt_pte_t __iomem *)dev_priv->gtt.gsm + first_entry;
+	gen6_gtt_pte_t __iomem *gtt_entries =
+		(gen6_gtt_pte_t __iomem *)dev_priv->gtt.gsm + first_entry;
 	int i = 0;
 	struct sg_page_iter sg_iter;
 	dma_addr_t addr;
@@ -451,8 +450,8 @@ static void gen6_ggtt_clear_range(struct drm_device *dev,
 				  unsigned int num_entries)
 {
 	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;
+	gen6_gtt_pte_t scratch_pte, __iomem *gtt_base =
+		(gen6_gtt_pte_t __iomem *) dev_priv->gtt.gsm + first_entry;
 	const int max_entries = gtt_total_entries(dev_priv->gtt) - first_entry;
 	int i;
 
@@ -736,7 +735,7 @@ static int gen6_gmch_probe(struct drm_device *dev,
 	else
 		*stolen = gen6_get_stolen_size(snb_gmch_ctl);
 
-	*gtt_total = (gtt_size / sizeof(gtt_pte_t)) << PAGE_SHIFT;
+	*gtt_total = (gtt_size / sizeof(gen6_gtt_pte_t)) << PAGE_SHIFT;
 
 	/* For Modern GENs the PTEs and register space are split in the BAR */
 	gtt_bus_addr = pci_resource_start(dev->pdev, 0) +
@@ -816,7 +815,7 @@ int i915_gem_gtt_init(struct drm_device *dev)
 	if (ret)
 		return ret;
 
-	gtt_size = (dev_priv->gtt.total >> PAGE_SHIFT) * sizeof(gtt_pte_t);
+	gtt_size = (dev_priv->gtt.total >> PAGE_SHIFT) * sizeof(gen6_gtt_pte_t);
 
 	/* GMADR is the PCI mmio aperture into the global GTT. */
 	DRM_INFO("Memory usable by graphics device = %zdM\n",
-- 
1.8.2.1

[PATCH 03/10] drm/i915: Map registers before GTT init

[Ben Widawsky]

This will allow us to read/write registers in GTT init.

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

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4a1a517..ad7f284 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1518,6 +1518,28 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 		goto free_priv;
 	}
 
+	mmio_bar = IS_GEN2(dev) ? 1 : 0;
+	/* Before gen4, the registers and the GTT are behind different BARs.
+	 * However, from gen4 onwards, the registers and the GTT are shared
+	 * in the same BAR, so we want to restrict this ioremap from
+	 * clobbering the GTT which we want ioremap_wc instead. Fortunately,
+	 * the register BAR remains the same size for all the earlier
+	 * generations up to Ironlake.
+	 */
+	if (info->gen < 5)
+		mmio_size = 512*1024;
+	else
+		mmio_size = 2*1024*1024;
+
+	dev_priv->regs = pci_iomap(dev->pdev, mmio_bar, mmio_size);
+	if (!dev_priv->regs) {
+		DRM_ERROR("failed to map registers\n");
+		ret = -EIO;
+		goto put_gmch;
+	}
+
+	intel_early_sanitize_regs(dev);
+
 	ret = i915_gem_gtt_init(dev);
 	if (ret)
 		goto put_bridge;
@@ -1542,28 +1564,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	if (IS_BROADWATER(dev) || IS_CRESTLINE(dev))
 		dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(32));
 
-	mmio_bar = IS_GEN2(dev) ? 1 : 0;
-	/* Before gen4, the registers and the GTT are behind different BARs.
-	 * However, from gen4 onwards, the registers and the GTT are shared
-	 * in the same BAR, so we want to restrict this ioremap from
-	 * clobbering the GTT which we want ioremap_wc instead. Fortunately,
-	 * the register BAR remains the same size for all the earlier
-	 * generations up to Ironlake.
-	 */
-	if (info->gen < 5)
-		mmio_size = 512*1024;
-	else
-		mmio_size = 2*1024*1024;
-
-	dev_priv->regs = pci_iomap(dev->pdev, mmio_bar, mmio_size);
-	if (!dev_priv->regs) {
-		DRM_ERROR("failed to map registers\n");
-		ret = -EIO;
-		goto put_gmch;
-	}
-
-	intel_early_sanitize_regs(dev);
-
 	aperture_size = dev_priv->gtt.mappable_end;
 
 	dev_priv->gtt.mappable =
@@ -1686,10 +1686,10 @@ out_mtrrfree:
 		dev_priv->mm.gtt_mtrr = -1;
 	}
 	io_mapping_free(dev_priv->gtt.mappable);
-out_rmmap:
-	pci_iounmap(dev->pdev, dev_priv->regs);
 put_gmch:
 	dev_priv->gtt.gtt_remove(dev);
+out_rmmap:
+	pci_iounmap(dev->pdev, dev_priv->regs);
 put_bridge:
 	pci_dev_put(dev_priv->bridge_dev);
 free_priv:
-- 
1.8.2.1

Re: [PATCH 03/10] drm/i915: Map registers before GTT init

[Daniel Vetter]

On Mon, Apr 08, 2013 at 06:43:49PM -0700, Ben Widawsky wrote:
> This will allow us to read/write registers in GTT init.
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 48 ++++++++++++++++++++---------------------
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 4a1a517..ad7f284 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1518,6 +1518,28 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  		goto free_priv;
>  	}
>  
> +	mmio_bar = IS_GEN2(dev) ? 1 : 0;
> +	/* Before gen4, the registers and the GTT are behind different BARs.
> +	 * However, from gen4 onwards, the registers and the GTT are shared
> +	 * in the same BAR, so we want to restrict this ioremap from
> +	 * clobbering the GTT which we want ioremap_wc instead. Fortunately,
> +	 * the register BAR remains the same size for all the earlier
> +	 * generations up to Ironlake.
> +	 */
> +	if (info->gen < 5)
> +		mmio_size = 512*1024;
> +	else
> +		mmio_size = 2*1024*1024;
> +
> +	dev_priv->regs = pci_iomap(dev->pdev, mmio_bar, mmio_size);
> +	if (!dev_priv->regs) {
> +		DRM_ERROR("failed to map registers\n");
> +		ret = -EIO;
> +		goto put_gmch;

goto put_bridge is what I've fixed up here.

Queued for -next, thanks for the patch.
-Daniel

> +	}
> +
> +	intel_early_sanitize_regs(dev);
> +
>  	ret = i915_gem_gtt_init(dev);
>  	if (ret)
>  		goto put_bridge;
> @@ -1542,28 +1564,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	if (IS_BROADWATER(dev) || IS_CRESTLINE(dev))
>  		dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(32));
>  
> -	mmio_bar = IS_GEN2(dev) ? 1 : 0;
> -	/* Before gen4, the registers and the GTT are behind different BARs.
> -	 * However, from gen4 onwards, the registers and the GTT are shared
> -	 * in the same BAR, so we want to restrict this ioremap from
> -	 * clobbering the GTT which we want ioremap_wc instead. Fortunately,
> -	 * the register BAR remains the same size for all the earlier
> -	 * generations up to Ironlake.
> -	 */
> -	if (info->gen < 5)
> -		mmio_size = 512*1024;
> -	else
> -		mmio_size = 2*1024*1024;
> -
> -	dev_priv->regs = pci_iomap(dev->pdev, mmio_bar, mmio_size);
> -	if (!dev_priv->regs) {
> -		DRM_ERROR("failed to map registers\n");
> -		ret = -EIO;
> -		goto put_gmch;
> -	}
> -
> -	intel_early_sanitize_regs(dev);
> -
>  	aperture_size = dev_priv->gtt.mappable_end;
>  
>  	dev_priv->gtt.mappable =
> @@ -1686,10 +1686,10 @@ out_mtrrfree:
>  		dev_priv->mm.gtt_mtrr = -1;
>  	}
>  	io_mapping_free(dev_priv->gtt.mappable);
> -out_rmmap:
> -	pci_iounmap(dev->pdev, dev_priv->regs);
>  put_gmch:
>  	dev_priv->gtt.gtt_remove(dev);
> +out_rmmap:
> +	pci_iounmap(dev->pdev, dev_priv->regs);
>  put_bridge:
>  	pci_dev_put(dev_priv->bridge_dev);
>  free_priv:
> -- 
> 1.8.2.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 04/10] drm/i915: random checkpatch fixes

[Ben Widawsky]

There used to be other fixes in this patch but they've slowly disappeared as
other parts have been fixed.

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

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 091b270..923c705 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -45,8 +45,8 @@ typedef uint32_t gen6_gtt_pte_t;
 #define GEN6_PTE_ADDR_ENCODE(addr)	GEN6_GTT_ADDR_ENCODE(addr)
 
 static inline gen6_gtt_pte_t gen6_pte_encode(struct drm_device *dev,
-					dma_addr_t addr,
-					enum i915_cache_level level)
+					     dma_addr_t addr,
+					     enum i915_cache_level level)
 {
 	gen6_gtt_pte_t pte = GEN6_PTE_VALID;
 	pte |= GEN6_PTE_ADDR_ENCODE(addr);
@@ -289,7 +289,6 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
 	if (!dev_priv->mm.aliasing_ppgtt)
 		return;
 
-
 	pd_addr = (gen6_gtt_pte_t __iomem *)dev_priv->gtt.gsm +
 		ppgtt->pd_offset / sizeof(gen6_gtt_pte_t);
 	for (i = 0; i < ppgtt->num_pd_entries; i++) {
-- 
1.8.2.1

[PATCH 05/10] drm/i915/ppgtt: Set scratch page "globally"

[Ben Widawsky]

The PPGTT scratch page is used for all gens, and doing it in the global
part of our PPGTT setup makes the code a bit nicer.

This was in a patch submitted earlier as part of the PPGTT cleanups.
Grumpy maintainer must have missed it, and I didn't yell when
appropriate. Apologies for everyone :-)

v2: Update commit message

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

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 923c705..0f214fb 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -201,8 +201,6 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 		ppgtt->pt_dma_addr[i] = pt_addr;
 	}
 
-	ppgtt->scratch_page_dma_addr = dev_priv->gtt.scratch_page_dma;
-
 	ppgtt->clear_range(ppgtt, 0,
 			   ppgtt->num_pd_entries*I915_PPGTT_PT_ENTRIES);
 
@@ -238,6 +236,7 @@ static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
 		return -ENOMEM;
 
 	ppgtt->dev = dev;
+	ppgtt->scratch_page_dma_addr = dev_priv->gtt.scratch_page_dma;
 
 	ret = gen6_ppgtt_init(ppgtt);
 	if (ret)
-- 
1.8.2.1

[PATCH 06/10] drm/i915: Conditionally carve out GGTT PDE

[Ben Widawsky]

It only works that way on GEN6 and GEN7. Let's not assume GENn will be
the same.

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

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0f214fb..b4ba488 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -623,9 +623,12 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
 
 	if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
 		int ret;
-		/* 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;
+
+		if (INTEL_INFO(dev)->gen <= 7) {
+			/* 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);
 
-- 
1.8.2.1

[PATCH 07/10] drm/i915: Rework PPGTT init code

[Ben Widawsky]

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

This rework will help if future platforms choose to be a bit different.
Should have no functional impact.

v2: Don't move around the vtable setup (Daniel)

v3: Squash in the disable-by-default patch.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b4ba488..b13ba9d 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -238,7 +238,11 @@ static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
 	ppgtt->dev = dev;
 	ppgtt->scratch_page_dma_addr = dev_priv->gtt.scratch_page_dma;
 
-	ret = gen6_ppgtt_init(ppgtt);
+	if (INTEL_INFO(dev)->gen < 8)
+		ret = gen6_ppgtt_init(ppgtt);
+	else
+		BUG();
+
 	if (ret)
 		kfree(ppgtt);
 	else
-- 
1.8.2.1

[PATCH 08/10] drm/i915: Abstract PPGTT enabling

[Ben Widawsky]

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

Since we've already set up a nice vtable to abstract other PPGTT
functions, also abstract the actual register programming to enable
things.

This function will probably need to change a bit as we implement real
processes.

v2: Resolve conflicts due to patch series reordering.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c     |   3 +-
 drivers/gpu/drm/i915/i915_drv.h     |   2 +-
 drivers/gpu/drm/i915/i915_gem.c     |   3 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c | 114 ++++++++++++++++++------------------
 4 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index bd92e7d..70d10de 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -927,7 +927,8 @@ int i915_reset(struct drm_device *dev)
 			ring->init(ring);
 
 		i915_gem_context_init(dev);
-		i915_gem_init_ppgtt(dev);
+		if (dev_priv->mm.aliasing_ppgtt)
+			dev_priv->mm.aliasing_ppgtt->enable(dev);
 
 		/*
 		 * It would make sense to re-init all the other hw state, at
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 69ddfd1..c121321 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -449,6 +449,7 @@ struct i915_hw_ppgtt {
 			       struct sg_table *st,
 			       unsigned int pg_start,
 			       enum i915_cache_level cache_level);
+	void (*enable)(struct drm_device *dev);
 	void (*cleanup)(struct i915_hw_ppgtt *ppgtt);
 };
 
@@ -1642,7 +1643,6 @@ int __must_check i915_gem_init(struct drm_device *dev);
 int __must_check i915_gem_init_hw(struct drm_device *dev);
 void i915_gem_l3_remap(struct drm_device *dev);
 void i915_gem_init_swizzling(struct drm_device *dev);
-void i915_gem_init_ppgtt(struct drm_device *dev);
 void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
 int __must_check i915_gpu_idle(struct drm_device *dev);
 int __must_check i915_gem_idle(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 63c05dd..8a73a68 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4029,7 +4029,8 @@ i915_gem_init_hw(struct drm_device *dev)
 	 * contexts before PPGTT.
 	 */
 	i915_gem_context_init(dev);
-	i915_gem_init_ppgtt(dev);
+	if (dev_priv->mm.aliasing_ppgtt)
+		dev_priv->mm.aliasing_ppgtt->enable(dev);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b13ba9d..c56de21 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -75,6 +75,61 @@ static inline gen6_gtt_pte_t gen6_pte_encode(struct drm_device *dev,
 	return pte;
 }
 
+static void gen6_ppgtt_enable(struct drm_device *dev)
+{
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	uint32_t pd_offset;
+	struct intel_ring_buffer *ring;
+	struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
+	gen6_gtt_pte_t __iomem *pd_addr;
+	uint32_t pd_entry;
+	int i;
+
+	pd_addr = (gen6_gtt_pte_t __iomem*)dev_priv->gtt.gsm +
+		ppgtt->pd_offset / sizeof(gen6_gtt_pte_t);
+	for (i = 0; i < ppgtt->num_pd_entries; i++) {
+		dma_addr_t pt_addr;
+
+		pt_addr = ppgtt->pt_dma_addr[i];
+		pd_entry = GEN6_PDE_ADDR_ENCODE(pt_addr);
+		pd_entry |= GEN6_PDE_VALID;
+
+		writel(pd_entry, pd_addr + i);
+	}
+	readl(pd_addr);
+
+	pd_offset = ppgtt->pd_offset;
+	pd_offset /= 64; /* in cachelines, */
+	pd_offset <<= 16;
+
+	if (INTEL_INFO(dev)->gen == 6) {
+		uint32_t ecochk, gab_ctl, ecobits;
+
+		ecobits = I915_READ(GAC_ECO_BITS);
+		I915_WRITE(GAC_ECO_BITS, ecobits | ECOBITS_PPGTT_CACHE64B);
+
+		gab_ctl = I915_READ(GAB_CTL);
+		I915_WRITE(GAB_CTL, gab_ctl | GAB_CTL_CONT_AFTER_PAGEFAULT);
+
+		ecochk = I915_READ(GAM_ECOCHK);
+		I915_WRITE(GAM_ECOCHK, ecochk | ECOCHK_SNB_BIT |
+				       ECOCHK_PPGTT_CACHE64B);
+		I915_WRITE(GFX_MODE, _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
+	} else if (INTEL_INFO(dev)->gen >= 7) {
+		I915_WRITE(GAM_ECOCHK, ECOCHK_PPGTT_CACHE64B);
+		/* GFX_MODE is per-ring on gen7+ */
+	}
+
+	for_each_ring(ring, dev_priv, i) {
+		if (INTEL_INFO(dev)->gen >= 7)
+			I915_WRITE(RING_MODE_GEN7(ring),
+				   _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
+
+		I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G);
+		I915_WRITE(RING_PP_DIR_BASE(ring), pd_offset);
+	}
+}
+
 /* PPGTT support for Sandybdrige/Gen6 and later */
 static void gen6_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
 				   unsigned first_entry,
@@ -168,6 +223,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 		gtt_total_entries(dev_priv->gtt) - I915_PPGTT_PD_ENTRIES;
 
 	ppgtt->num_pd_entries = I915_PPGTT_PD_ENTRIES;
+	ppgtt->enable = gen6_ppgtt_enable;
 	ppgtt->clear_range = gen6_ppgtt_clear_range;
 	ppgtt->insert_entries = gen6_ppgtt_insert_entries;
 	ppgtt->cleanup = gen6_ppgtt_cleanup;
@@ -279,64 +335,6 @@ void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
 			   obj->base.size >> PAGE_SHIFT);
 }
 
-void i915_gem_init_ppgtt(struct drm_device *dev)
-{
-	drm_i915_private_t *dev_priv = dev->dev_private;
-	uint32_t pd_offset;
-	struct intel_ring_buffer *ring;
-	struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
-	gen6_gtt_pte_t __iomem *pd_addr;
-	uint32_t pd_entry;
-	int i;
-
-	if (!dev_priv->mm.aliasing_ppgtt)
-		return;
-
-	pd_addr = (gen6_gtt_pte_t __iomem *)dev_priv->gtt.gsm +
-		ppgtt->pd_offset / sizeof(gen6_gtt_pte_t);
-	for (i = 0; i < ppgtt->num_pd_entries; i++) {
-		dma_addr_t pt_addr;
-
-		pt_addr = ppgtt->pt_dma_addr[i];
-		pd_entry = GEN6_PDE_ADDR_ENCODE(pt_addr);
-		pd_entry |= GEN6_PDE_VALID;
-
-		writel(pd_entry, pd_addr + i);
-	}
-	readl(pd_addr);
-
-	pd_offset = ppgtt->pd_offset;
-	pd_offset /= 64; /* in cachelines, */
-	pd_offset <<= 16;
-
-	if (INTEL_INFO(dev)->gen == 6) {
-		uint32_t ecochk, gab_ctl, ecobits;
-
-		ecobits = I915_READ(GAC_ECO_BITS);
-		I915_WRITE(GAC_ECO_BITS, ecobits | ECOBITS_PPGTT_CACHE64B);
-
-		gab_ctl = I915_READ(GAB_CTL);
-		I915_WRITE(GAB_CTL, gab_ctl | GAB_CTL_CONT_AFTER_PAGEFAULT);
-
-		ecochk = I915_READ(GAM_ECOCHK);
-		I915_WRITE(GAM_ECOCHK, ecochk | ECOCHK_SNB_BIT |
-				       ECOCHK_PPGTT_CACHE64B);
-		I915_WRITE(GFX_MODE, _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
-	} else if (INTEL_INFO(dev)->gen >= 7) {
-		I915_WRITE(GAM_ECOCHK, ECOCHK_PPGTT_CACHE64B);
-		/* GFX_MODE is per-ring on gen7+ */
-	}
-
-	for_each_ring(ring, dev_priv, i) {
-		if (INTEL_INFO(dev)->gen >= 7)
-			I915_WRITE(RING_MODE_GEN7(ring),
-				   _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
-
-		I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G);
-		I915_WRITE(RING_PP_DIR_BASE(ring), pd_offset);
-	}
-}
-
 extern int intel_iommu_gfx_mapped;
 /* Certain Gen5 chipsets require require idling the GPU before
  * unmapping anything from the GTT when VT-d is enabled.
-- 
1.8.2.1

[PATCH 09/10] drm/i915: NULL aliasing_ppgtt on cleanup

[Ben Widawsky]

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

This will allow us to carry on if we've cleaned up the PPGTT. The usage
for this is coming up.

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

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index c56de21..d32912e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -316,6 +316,7 @@ void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev)
 		return;
 
 	ppgtt->cleanup(ppgtt);
+	dev_priv->mm.aliasing_ppgtt = NULL;
 }
 
 void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
-- 
1.8.2.1

[PATCH 10/10] drm/i915: Allow PPGTT enable to fail

[Ben Widawsky]

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

I'm really not happy that we have to support this, but this will be the
simplest way to handle cases where PPGTT init can fail, which I promise
will be coming in the future.

v2: Resolve conflicts due to patch series reordering.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c     | 7 +++++--
 drivers/gpu/drm/i915/i915_drv.h     | 2 +-
 drivers/gpu/drm/i915/i915_gem.c     | 9 +++++++--
 drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ++-
 4 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 70d10de..bddb9a5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -927,8 +927,11 @@ int i915_reset(struct drm_device *dev)
 			ring->init(ring);
 
 		i915_gem_context_init(dev);
-		if (dev_priv->mm.aliasing_ppgtt)
-			dev_priv->mm.aliasing_ppgtt->enable(dev);
+		if (dev_priv->mm.aliasing_ppgtt) {
+			ret = dev_priv->mm.aliasing_ppgtt->enable(dev);
+			if (ret)
+				i915_gem_cleanup_aliasing_ppgtt(dev);
+		}
 
 		/*
 		 * It would make sense to re-init all the other hw state, at
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c121321..f59a388 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -449,7 +449,7 @@ struct i915_hw_ppgtt {
 			       struct sg_table *st,
 			       unsigned int pg_start,
 			       enum i915_cache_level cache_level);
-	void (*enable)(struct drm_device *dev);
+	int (*enable)(struct drm_device *dev);
 	void (*cleanup)(struct i915_hw_ppgtt *ppgtt);
 };
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8a73a68..da6d6de 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4029,8 +4029,13 @@ i915_gem_init_hw(struct drm_device *dev)
 	 * contexts before PPGTT.
 	 */
 	i915_gem_context_init(dev);
-	if (dev_priv->mm.aliasing_ppgtt)
-		dev_priv->mm.aliasing_ppgtt->enable(dev);
+	if (dev_priv->mm.aliasing_ppgtt) {
+		ret = dev_priv->mm.aliasing_ppgtt->enable(dev);
+		if (ret) {
+			i915_gem_cleanup_aliasing_ppgtt(dev);
+			DRM_INFO("PPGTT enable failed. This is not fatal, but unexpected\n");
+		}
+	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d32912e..11143b4 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -75,7 +75,7 @@ static inline gen6_gtt_pte_t gen6_pte_encode(struct drm_device *dev,
 	return pte;
 }
 
-static void gen6_ppgtt_enable(struct drm_device *dev)
+static int gen6_ppgtt_enable(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	uint32_t pd_offset;
@@ -128,6 +128,7 @@ static void gen6_ppgtt_enable(struct drm_device *dev)
 		I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G);
 		I915_WRITE(RING_PP_DIR_BASE(ring), pd_offset);
 	}
+	return 0;
 }
 
 /* PPGTT support for Sandybdrige/Gen6 and later */
-- 
1.8.2.1

Re: [PATCH 10/10] drm/i915: Allow PPGTT enable to fail

[Daniel Vetter]

On Mon, Apr 08, 2013 at 06:43:56PM -0700, Ben Widawsky wrote:
> From: Ben Widawsky <benjamin.widawsky@intel.com>
> 
> I'm really not happy that we have to support this, but this will be the
> simplest way to handle cases where PPGTT init can fail, which I promise
> will be coming in the future.
> 
> v2: Resolve conflicts due to patch series reordering.
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> (v1)
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Queued all 10 patches for -next, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.c     | 7 +++++--
>  drivers/gpu/drm/i915/i915_drv.h     | 2 +-
>  drivers/gpu/drm/i915/i915_gem.c     | 9 +++++++--
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ++-
>  4 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 70d10de..bddb9a5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -927,8 +927,11 @@ int i915_reset(struct drm_device *dev)
>  			ring->init(ring);
>  
>  		i915_gem_context_init(dev);
> -		if (dev_priv->mm.aliasing_ppgtt)
> -			dev_priv->mm.aliasing_ppgtt->enable(dev);
> +		if (dev_priv->mm.aliasing_ppgtt) {
> +			ret = dev_priv->mm.aliasing_ppgtt->enable(dev);
> +			if (ret)
> +				i915_gem_cleanup_aliasing_ppgtt(dev);
> +		}
>  
>  		/*
>  		 * It would make sense to re-init all the other hw state, at
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c121321..f59a388 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -449,7 +449,7 @@ struct i915_hw_ppgtt {
>  			       struct sg_table *st,
>  			       unsigned int pg_start,
>  			       enum i915_cache_level cache_level);
> -	void (*enable)(struct drm_device *dev);
> +	int (*enable)(struct drm_device *dev);
>  	void (*cleanup)(struct i915_hw_ppgtt *ppgtt);
>  };
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8a73a68..da6d6de 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4029,8 +4029,13 @@ i915_gem_init_hw(struct drm_device *dev)
>  	 * contexts before PPGTT.
>  	 */
>  	i915_gem_context_init(dev);
> -	if (dev_priv->mm.aliasing_ppgtt)
> -		dev_priv->mm.aliasing_ppgtt->enable(dev);
> +	if (dev_priv->mm.aliasing_ppgtt) {
> +		ret = dev_priv->mm.aliasing_ppgtt->enable(dev);
> +		if (ret) {
> +			i915_gem_cleanup_aliasing_ppgtt(dev);
> +			DRM_INFO("PPGTT enable failed. This is not fatal, but unexpected\n");
> +		}
> +	}
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index d32912e..11143b4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -75,7 +75,7 @@ static inline gen6_gtt_pte_t gen6_pte_encode(struct drm_device *dev,
>  	return pte;
>  }
>  
> -static void gen6_ppgtt_enable(struct drm_device *dev)
> +static int gen6_ppgtt_enable(struct drm_device *dev)
>  {
>  	drm_i915_private_t *dev_priv = dev->dev_private;
>  	uint32_t pd_offset;
> @@ -128,6 +128,7 @@ static void gen6_ppgtt_enable(struct drm_device *dev)
>  		I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G);
>  		I915_WRITE(RING_PP_DIR_BASE(ring), pd_offset);
>  	}
> +	return 0;
>  }
>  
>  /* PPGTT support for Sandybdrige/Gen6 and later */
> -- 
> 1.8.2.1
> 

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