drm/i915/gtt: Fix rounding for 36b
diff mbox series

Message ID 20190719130737.5835-1-chris@chris-wilson.co.uk
State New
Headers show
Series
  • drm/i915/gtt: Fix rounding for 36b
Related show

Commit Message

Chris Wilson July 19, 2019, 1:07 p.m. UTC
The top-level page directory for 36b is a single entry, not multiple
like 32b. Fix up the rounding on the calculation of the size of the top
level so that we populate the 4th level correctly for 36b.

Reported-by: Jose Souza <jose.souza@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Fixes: 1eda701eace2 ("drm/i915/gtt: Recursive cleanup for gen8")
Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Jose Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

Comments

Souza, Jose July 19, 2019, 5:39 p.m. UTC | #1
On Fri, 2019-07-19 at 14:07 +0100, Chris Wilson wrote:
> The top-level page directory for 36b is a single entry, not multiple
> like 32b. Fix up the rounding on the calculation of the size of the
> top
> level so that we populate the 4th level correctly for 36b.
> 

It fixed the bug, thanks

Tested-by: José Roberto de Souza <jose.souza@intel.com>

> Reported-by: Jose Souza <jose.souza@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Fixes: 1eda701eace2 ("drm/i915/gtt: Recursive cleanup for gen8")
> Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Cc: Jose Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 220aba5a94d2..7dd08ff6c0eb 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -905,6 +905,12 @@ static inline unsigned int gen8_pt_count(u64
> start, u64 end)
>  		return end - start;
>  }
>  
> +static inline unsigned int gen8_pd_top_count(const struct
> i915_address_space *vm)
> +{
> +	unsigned int shift = __gen8_pte_shift(vm->top);
> +	return (vm->total + (1ull << shift) - 1) >> shift;
> +}
> +
>  static void __gen8_ppgtt_cleanup(struct i915_address_space *vm,
>  				 struct i915_page_directory *pd,
>  				 int count, int lvl)
> @@ -930,9 +936,7 @@ static void gen8_ppgtt_cleanup(struct
> i915_address_space *vm)
>  	if (intel_vgpu_active(vm->i915))
>  		gen8_ppgtt_notify_vgt(ppgtt, false);
>  
> -	__gen8_ppgtt_cleanup(vm, ppgtt->pd,
> -			     vm->total >> __gen8_pte_shift(vm->top),
> -			     vm->top);
> +	__gen8_ppgtt_cleanup(vm, ppgtt->pd, gen8_pd_top_count(vm), vm-
> >top);
>  	free_scratch(vm);
>  }
>  
> @@ -1391,7 +1395,7 @@ static int
> gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
>  	unsigned int idx;
>  
>  	GEM_BUG_ON(vm->top != 2);
> -	GEM_BUG_ON((vm->total >> __gen8_pte_shift(2)) !=
> GEN8_3LVL_PDPES);
> +	GEM_BUG_ON(gen8_pd_top_count(vm) != GEN8_3LVL_PDPES);
>  
>  	for (idx = 0; idx < GEN8_3LVL_PDPES; idx++) {
>  		struct i915_page_directory *pde;
> @@ -1428,7 +1432,7 @@ static void ppgtt_init(struct i915_ppgtt
> *ppgtt, struct intel_gt *gt)
>  static struct i915_page_directory *
>  gen8_alloc_top_pd(struct i915_address_space *vm)
>  {
> -	const unsigned int count = vm->total >> __gen8_pte_shift(vm-
> >top);
> +	const unsigned int count = gen8_pd_top_count(vm);
>  	struct i915_page_directory *pd;
>  
>  	GEM_BUG_ON(count > ARRAY_SIZE(pd->entry));
> @@ -1514,8 +1518,7 @@ static struct i915_ppgtt
> *gen8_ppgtt_create(struct drm_i915_private *i915)
>  
>  err_free_pd:
>  	__gen8_ppgtt_cleanup(&ppgtt->vm, ppgtt->pd,
> -			     ppgtt->vm.total >> __gen8_pte_shift(ppgtt-
> >vm.top),
> -			     ppgtt->vm.top);
> +			     gen8_pd_top_count(&ppgtt->vm), ppgtt-
> >vm.top);
>  err_free_scratch:
>  	free_scratch(&ppgtt->vm);
>  err_free:
Abdiel Janulgue July 22, 2019, 11:26 a.m. UTC | #2
On 19/07/2019 16.07, Chris Wilson wrote:
> The top-level page directory for 36b is a single entry, not multiple
> like 32b. Fix up the rounding on the calculation of the size of the top
> level so that we populate the 4th level correctly for 36b.
> 

Reviewed-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>

> Reported-by: Jose Souza <jose.souza@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Fixes: 1eda701eace2 ("drm/i915/gtt: Recursive cleanup for gen8")
> Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Cc: Jose Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 220aba5a94d2..7dd08ff6c0eb 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -905,6 +905,12 @@ static inline unsigned int gen8_pt_count(u64 start, u64 end)
>  		return end - start;
>  }
>  
> +static inline unsigned int gen8_pd_top_count(const struct i915_address_space *vm)
> +{
> +	unsigned int shift = __gen8_pte_shift(vm->top);
> +	return (vm->total + (1ull << shift) - 1) >> shift;
> +}
> +
>  static void __gen8_ppgtt_cleanup(struct i915_address_space *vm,
>  				 struct i915_page_directory *pd,
>  				 int count, int lvl)
> @@ -930,9 +936,7 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
>  	if (intel_vgpu_active(vm->i915))
>  		gen8_ppgtt_notify_vgt(ppgtt, false);
>  
> -	__gen8_ppgtt_cleanup(vm, ppgtt->pd,
> -			     vm->total >> __gen8_pte_shift(vm->top),
> -			     vm->top);
> +	__gen8_ppgtt_cleanup(vm, ppgtt->pd, gen8_pd_top_count(vm), vm->top);
>  	free_scratch(vm);
>  }
>  
> @@ -1391,7 +1395,7 @@ static int gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
>  	unsigned int idx;
>  
>  	GEM_BUG_ON(vm->top != 2);
> -	GEM_BUG_ON((vm->total >> __gen8_pte_shift(2)) != GEN8_3LVL_PDPES);
> +	GEM_BUG_ON(gen8_pd_top_count(vm) != GEN8_3LVL_PDPES);
>  
>  	for (idx = 0; idx < GEN8_3LVL_PDPES; idx++) {
>  		struct i915_page_directory *pde;
> @@ -1428,7 +1432,7 @@ static void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt)
>  static struct i915_page_directory *
>  gen8_alloc_top_pd(struct i915_address_space *vm)
>  {
> -	const unsigned int count = vm->total >> __gen8_pte_shift(vm->top);
> +	const unsigned int count = gen8_pd_top_count(vm);
>  	struct i915_page_directory *pd;
>  
>  	GEM_BUG_ON(count > ARRAY_SIZE(pd->entry));
> @@ -1514,8 +1518,7 @@ static struct i915_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
>  
>  err_free_pd:
>  	__gen8_ppgtt_cleanup(&ppgtt->vm, ppgtt->pd,
> -			     ppgtt->vm.total >> __gen8_pte_shift(ppgtt->vm.top),
> -			     ppgtt->vm.top);
> +			     gen8_pd_top_count(&ppgtt->vm), ppgtt->vm.top);
>  err_free_scratch:
>  	free_scratch(&ppgtt->vm);
>  err_free:
>

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 220aba5a94d2..7dd08ff6c0eb 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -905,6 +905,12 @@  static inline unsigned int gen8_pt_count(u64 start, u64 end)
 		return end - start;
 }
 
+static inline unsigned int gen8_pd_top_count(const struct i915_address_space *vm)
+{
+	unsigned int shift = __gen8_pte_shift(vm->top);
+	return (vm->total + (1ull << shift) - 1) >> shift;
+}
+
 static void __gen8_ppgtt_cleanup(struct i915_address_space *vm,
 				 struct i915_page_directory *pd,
 				 int count, int lvl)
@@ -930,9 +936,7 @@  static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
 	if (intel_vgpu_active(vm->i915))
 		gen8_ppgtt_notify_vgt(ppgtt, false);
 
-	__gen8_ppgtt_cleanup(vm, ppgtt->pd,
-			     vm->total >> __gen8_pte_shift(vm->top),
-			     vm->top);
+	__gen8_ppgtt_cleanup(vm, ppgtt->pd, gen8_pd_top_count(vm), vm->top);
 	free_scratch(vm);
 }
 
@@ -1391,7 +1395,7 @@  static int gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
 	unsigned int idx;
 
 	GEM_BUG_ON(vm->top != 2);
-	GEM_BUG_ON((vm->total >> __gen8_pte_shift(2)) != GEN8_3LVL_PDPES);
+	GEM_BUG_ON(gen8_pd_top_count(vm) != GEN8_3LVL_PDPES);
 
 	for (idx = 0; idx < GEN8_3LVL_PDPES; idx++) {
 		struct i915_page_directory *pde;
@@ -1428,7 +1432,7 @@  static void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt)
 static struct i915_page_directory *
 gen8_alloc_top_pd(struct i915_address_space *vm)
 {
-	const unsigned int count = vm->total >> __gen8_pte_shift(vm->top);
+	const unsigned int count = gen8_pd_top_count(vm);
 	struct i915_page_directory *pd;
 
 	GEM_BUG_ON(count > ARRAY_SIZE(pd->entry));
@@ -1514,8 +1518,7 @@  static struct i915_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 
 err_free_pd:
 	__gen8_ppgtt_cleanup(&ppgtt->vm, ppgtt->pd,
-			     ppgtt->vm.total >> __gen8_pte_shift(ppgtt->vm.top),
-			     ppgtt->vm.top);
+			     gen8_pd_top_count(&ppgtt->vm), ppgtt->vm.top);
 err_free_scratch:
 	free_scratch(&ppgtt->vm);
 err_free: