diff mbox

[05/10] drm/i915: move more pte encoding to pte encode

Message ID 1350956055-3224-6-git-send-email-ben@bwidawsk.net (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Widawsky Oct. 23, 2012, 1:34 a.m. UTC 
In order to handle differences in pte encoding between architectures it
is desirable to have one helper function, pte_encode, do it all for us.
As such, this commit moves the code around so we're in good shape to do
that.

Luckily the ppgtt pte and the ggtt pte look very similar.

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

Comments

Jesse Barnes Oct. 25, 2012, 8:51 p.m. UTC | #1 
On Mon, 22 Oct 2012 18:34:10 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:

> In order to handle differences in pte encoding between architectures it
> is desirable to have one helper function, pte_encode, do it all for us.
> As such, this commit moves the code around so we're in good shape to do
> that.
> 
> Luckily the ppgtt pte and the ggtt pte look very similar.
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 56 ++++++++++++++++++-------------------
>  1 file changed, 27 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index da9c1fa..89c273f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -33,11 +33,32 @@ typedef uint32_t gtt_pte_t;
>  
>  static inline gtt_pte_t pte_encode(struct drm_device *dev,
>  				   dma_addr_t addr,
> -				   gtt_pte_t cache_bits)
> +				   enum i915_cache_level level)
>  {
>  	gtt_pte_t pte = GEN6_PTE_VALID;
>  	pte |= GEN6_PTE_ADDR_ENCODE(addr);
> -	pte |= cache_bits;
> +
> +	switch (level) {
> +	case I915_CACHE_LLC_MLC:
> +		/* Haswell doesn't set L3 this way */
> +		if (IS_HASWELL(dev))
> +			pte |= GEN6_PTE_CACHE_LLC;
> +		else
> +			pte |= GEN6_PTE_CACHE_LLC_MLC;
> +		break;
> +	case I915_CACHE_LLC:
> +		pte |= GEN6_PTE_CACHE_LLC;
> +		break;
> +	case I915_CACHE_NONE:
> +		if (IS_HASWELL(dev))
> +			pte |= HSW_PTE_UNCACHED;
> +		else
> +			pte |= GEN6_PTE_UNCACHED;
> +		break;
> +	default:
> +		BUG();
> +	}
> +
>  
>  	return pte;
>  }
> @@ -54,7 +75,7 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
>  	unsigned last_pte, i;
>  
>  	scratch_pte = pte_encode(ppgtt->dev, ppgtt->scratch_page_dma_addr,
> -				 GEN6_PTE_CACHE_LLC);
> +				 I915_CACHE_LLC);
>  
>  	while (num_entries) {
>  		last_pte = first_pte + num_entries;
> @@ -183,7 +204,7 @@ void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev)
>  static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
>  					 const struct sg_table *pages,
>  					 unsigned first_entry,
> -					 gtt_pte_t pte_flags)
> +					 enum i915_cache_level cache_level)
>  {
>  	gtt_pte_t *pt_vaddr;
>  	unsigned act_pd = first_entry / I915_PPGTT_PT_ENTRIES;
> @@ -204,7 +225,7 @@ static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
>  		for (j = first_pte; j < I915_PPGTT_PT_ENTRIES; j++) {
>  			page_addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
>  			pt_vaddr[j] = pte_encode(ppgtt->dev, page_addr,
> -						 pte_flags);
> +						 cache_level);
>  
>  			/* grab the next page */
>  			if (++m == segment_len) {
> @@ -228,33 +249,10 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
>  			    struct drm_i915_gem_object *obj,
>  			    enum i915_cache_level cache_level)
>  {
> -	gtt_pte_t pte_flags = GEN6_PTE_VALID;
> -
> -	switch (cache_level) {
> -	case I915_CACHE_LLC_MLC:
> -		/* Haswell doesn't set L3 this way */
> -		if (IS_HASWELL(ppgtt->dev))
> -			pte_flags |= GEN6_PTE_CACHE_LLC;
> -		else
> -			pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
> -		break;
> -	case I915_CACHE_LLC:
> -		pte_flags |= GEN6_PTE_CACHE_LLC;
> -		break;
> -	case I915_CACHE_NONE:
> -		if (IS_HASWELL(ppgtt->dev))
> -			pte_flags |= HSW_PTE_UNCACHED;
> -		else
> -			pte_flags |= GEN6_PTE_UNCACHED;
> -		break;
> -	default:
> -		BUG();
> -	}
> -
>  	i915_ppgtt_insert_sg_entries(ppgtt,
>  				     obj->pages,
>  				     obj->gtt_space->start >> PAGE_SHIFT,
> -				     pte_flags);
> +				     cache_level);
>  }
>  
>  void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Daniel Vetter Oct. 31, 2012, 10:52 a.m. UTC | #2 
On Thu, Oct 25, 2012 at 01:51:27PM -0700, Jesse Barnes wrote:
> On Mon, 22 Oct 2012 18:34:10 -0700
> Ben Widawsky <ben@bwidawsk.net> wrote:
> 
> > In order to handle differences in pte encoding between architectures it
> > is desirable to have one helper function, pte_encode, do it all for us.
> > As such, this commit moves the code around so we're in good shape to do
> > that.
> > 
> > Luckily the ppgtt pte and the ggtt pte look very similar.
> > 
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

Queued for -next up to this patch, thanks.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index da9c1fa..89c273f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -33,11 +33,32 @@  typedef uint32_t gtt_pte_t;
 
 static inline gtt_pte_t pte_encode(struct drm_device *dev,
 				   dma_addr_t addr,
-				   gtt_pte_t cache_bits)
+				   enum i915_cache_level level)
 {
 	gtt_pte_t pte = GEN6_PTE_VALID;
 	pte |= GEN6_PTE_ADDR_ENCODE(addr);
-	pte |= cache_bits;
+
+	switch (level) {
+	case I915_CACHE_LLC_MLC:
+		/* Haswell doesn't set L3 this way */
+		if (IS_HASWELL(dev))
+			pte |= GEN6_PTE_CACHE_LLC;
+		else
+			pte |= GEN6_PTE_CACHE_LLC_MLC;
+		break;
+	case I915_CACHE_LLC:
+		pte |= GEN6_PTE_CACHE_LLC;
+		break;
+	case I915_CACHE_NONE:
+		if (IS_HASWELL(dev))
+			pte |= HSW_PTE_UNCACHED;
+		else
+			pte |= GEN6_PTE_UNCACHED;
+		break;
+	default:
+		BUG();
+	}
+
 
 	return pte;
 }
@@ -54,7 +75,7 @@  static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
 	unsigned last_pte, i;
 
 	scratch_pte = pte_encode(ppgtt->dev, ppgtt->scratch_page_dma_addr,
-				 GEN6_PTE_CACHE_LLC);
+				 I915_CACHE_LLC);
 
 	while (num_entries) {
 		last_pte = first_pte + num_entries;
@@ -183,7 +204,7 @@  void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev)
 static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
 					 const struct sg_table *pages,
 					 unsigned first_entry,
-					 gtt_pte_t pte_flags)
+					 enum i915_cache_level cache_level)
 {
 	gtt_pte_t *pt_vaddr;
 	unsigned act_pd = first_entry / I915_PPGTT_PT_ENTRIES;
@@ -204,7 +225,7 @@  static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
 		for (j = first_pte; j < I915_PPGTT_PT_ENTRIES; j++) {
 			page_addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
 			pt_vaddr[j] = pte_encode(ppgtt->dev, page_addr,
-						 pte_flags);
+						 cache_level);
 
 			/* grab the next page */
 			if (++m == segment_len) {
@@ -228,33 +249,10 @@  void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
 			    struct drm_i915_gem_object *obj,
 			    enum i915_cache_level cache_level)
 {
-	gtt_pte_t pte_flags = GEN6_PTE_VALID;
-
-	switch (cache_level) {
-	case I915_CACHE_LLC_MLC:
-		/* Haswell doesn't set L3 this way */
-		if (IS_HASWELL(ppgtt->dev))
-			pte_flags |= GEN6_PTE_CACHE_LLC;
-		else
-			pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
-		break;
-	case I915_CACHE_LLC:
-		pte_flags |= GEN6_PTE_CACHE_LLC;
-		break;
-	case I915_CACHE_NONE:
-		if (IS_HASWELL(ppgtt->dev))
-			pte_flags |= HSW_PTE_UNCACHED;
-		else
-			pte_flags |= GEN6_PTE_UNCACHED;
-		break;
-	default:
-		BUG();
-	}
-
 	i915_ppgtt_insert_sg_entries(ppgtt,
 				     obj->pages,
 				     obj->gtt_space->start >> PAGE_SHIFT,
-				     pte_flags);
+				     cache_level);
 }
 
 void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,