[RFC,13/38] drm/i915: Make gen6_write_pdes gen6_map_page_tables

Submitted by Michel Thierry on Oct. 7, 2014, 5:11 p.m.

Details

Message ID 1412701894-28905-14-git-send-email-michel.thierry@intel.com
State New, archived
Headers show

Not browsing as part of any series.

Commit Message

Michel Thierry Oct. 7, 2014, 5:11 p.m.
From: Ben Widawsky <benjamin.widawsky@intel.com>

Split out single mappings which will help with upcoming work. Also while
here, rename the function because it is a better description - but this
function is going away soon.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 39 ++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 16 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 00b5e5a..f5a1ac9 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -678,26 +678,33 @@  static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 	}
 }
 
-static void gen6_write_pdes(struct i915_hw_ppgtt *ppgtt)
+static void gen6_map_single(struct i915_hw_ppgtt *ppgtt,
+			    const unsigned pde_index,
+			    dma_addr_t daddr)
 {
 	struct drm_i915_private *dev_priv = ppgtt->base.dev->dev_private;
-	gen6_gtt_pte_t __iomem *pd_addr;
 	uint32_t pd_entry;
+	gen6_gtt_pte_t __iomem *pd_addr =
+		(gen6_gtt_pte_t __iomem*)dev_priv->gtt.gsm + ppgtt->pd_offset / sizeof(gen6_gtt_pte_t);
+
+	pd_entry = GEN6_PDE_ADDR_ENCODE(daddr);
+	pd_entry |= GEN6_PDE_VALID;
+
+	writel(pd_entry, pd_addr + pde_index);
+}
+
+/* Map all the page tables found in the ppgtt structure to incrementing page
+ * directories. */
+static void gen6_map_page_tables(struct i915_hw_ppgtt *ppgtt)
+{
+	struct drm_i915_private *dev_priv = ppgtt->base.dev->dev_private;
 	int i;
 
 	WARN_ON(ppgtt->pd_offset & 0x3f);
-	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;
+	for (i = 0; i < ppgtt->num_pd_entries; i++)
+		gen6_map_single(ppgtt, i, ppgtt->pt_dma_addr[i]);
 
-		writel(pd_entry, pd_addr + i);
-	}
-	readl(pd_addr);
+	readl(dev_priv->gtt.gsm);
 }
 
 static uint32_t get_pd_offset(struct i915_hw_ppgtt *ppgtt)
@@ -1087,7 +1094,7 @@  static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 			 ppgtt->node.size >> 20,
 			 ppgtt->node.start / PAGE_SIZE);
 
-	gen6_write_pdes(ppgtt);
+	gen6_map_page_tables(ppgtt);
 	DRM_DEBUG("Adding PPGTT at offset %x\n",
 		  ppgtt->pd_offset << 10);
 
@@ -1365,11 +1372,11 @@  void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 		/* TODO: Perhaps it shouldn't be gen6 specific */
 		if (i915_is_ggtt(vm)) {
 			if (dev_priv->mm.aliasing_ppgtt)
-				gen6_write_pdes(dev_priv->mm.aliasing_ppgtt);
+				gen6_map_page_tables(dev_priv->mm.aliasing_ppgtt);
 			continue;
 		}
 
-		gen6_write_pdes(container_of(vm, struct i915_hw_ppgtt, base));
+		gen6_map_page_tables(container_of(vm, struct i915_hw_ppgtt, base));
 	}
 
 	i915_ggtt_flush(dev_priv);

Comments

On Tue, Oct 07, 2014 at 06:11:09PM +0100, Michel Thierry wrote:
> From: Ben Widawsky <benjamin.widawsky@intel.com>
>
> Split out single mappings which will help with upcoming work. Also while
> here, rename the function because it is a better description - but this
> function is going away soon.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 39 ++++++++++++++++++++++---------------
>  1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 00b5e5a..f5a1ac9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -678,26 +678,33 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
>   }
>  }
>
> -static void gen6_write_pdes(struct i915_hw_ppgtt *ppgtt)
> +static void gen6_map_single(struct i915_hw_ppgtt *ppgtt,
> +    const unsigned pde_index,
> +    dma_addr_t daddr)

Maybe this will unravel later on, but I disagree on the naming scheme
here. Thus far we had

- bind/unbind for high-level/logical mapping on the gpu side.
- write/clear_entry for low-level pte munging on the gpu (this stuff
  here).
- dma_map/unmap for iommu mappings.

So using map/unmap here is fairly confusing.

Now if we actually want to bikeshed these functions we should switch
from the pci_map/unmap to the dma_map/unmap functions.
-Daniel

>  {
>   struct drm_i915_private *dev_priv = ppgtt->base.dev->dev_private;
> - gen6_gtt_pte_t __iomem *pd_addr;
>   uint32_t pd_entry;
> + gen6_gtt_pte_t __iomem *pd_addr =
> + (gen6_gtt_pte_t __iomem*)dev_priv->gtt.gsm + ppgtt->pd_offset / sizeof(gen6_gtt_pte_t);
> +
> + pd_entry = GEN6_PDE_ADDR_ENCODE(daddr);
> + pd_entry |= GEN6_PDE_VALID;
> +
> + writel(pd_entry, pd_addr + pde_index);
> +}
> +
> +/* Map all the page tables found in the ppgtt structure to incrementing page
> + * directories. */
> +static void gen6_map_page_tables(struct i915_hw_ppgtt *ppgtt)
> +{
> + struct drm_i915_private *dev_priv = ppgtt->base.dev->dev_private;
>   int i;
>
>   WARN_ON(ppgtt->pd_offset & 0x3f);
> - 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;
> + for (i = 0; i < ppgtt->num_pd_entries; i++)
> + gen6_map_single(ppgtt, i, ppgtt->pt_dma_addr[i]);
>
> - writel(pd_entry, pd_addr + i);
> - }
> - readl(pd_addr);
> + readl(dev_priv->gtt.gsm);
>  }
>
>  static uint32_t get_pd_offset(struct i915_hw_ppgtt *ppgtt)
> @@ -1087,7 +1094,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>   ppgtt->node.size >> 20,
>   ppgtt->node.start / PAGE_SIZE);
>
> - gen6_write_pdes(ppgtt);
> + gen6_map_page_tables(ppgtt);
>   DRM_DEBUG("Adding PPGTT at offset %x\n",
>    ppgtt->pd_offset << 10);
>
> @@ -1365,11 +1372,11 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>   /* TODO: Perhaps it shouldn't be gen6 specific */
>   if (i915_is_ggtt(vm)) {
>   if (dev_priv->mm.aliasing_ppgtt)
> - gen6_write_pdes(dev_priv->mm.aliasing_ppgtt);
> + gen6_map_page_tables(dev_priv->mm.aliasing_ppgtt);
>   continue;
>   }
>
> - gen6_write_pdes(container_of(vm, struct i915_hw_ppgtt, base));
> + gen6_map_page_tables(container_of(vm, struct i915_hw_ppgtt, base));
>   }
>
>   i915_ggtt_flush(dev_priv);
> --
> 2.0.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx