diff mbox

[1/5] drm/i915/gtt: Make I915_PDPES_PER_PDP inline function

Message ID 1488295691-9404-1-git-send-email-mika.kuoppala@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mika Kuoppala Feb. 28, 2017, 3:28 p.m. UTC
The macro takes a vm pointer at some sites, and dev_priv on others
We were saved as the internal macro never deferences the pointer
given.

As the number of pdpes depend on vm configuration, make it
as a inline function that accepts vm pointer.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 12 +++++++-----
 drivers/gpu/drm/i915/i915_gem_gtt.h | 26 ++++++++++++++++----------
 2 files changed, 23 insertions(+), 15 deletions(-)

Comments

Chris Wilson Feb. 28, 2017, 3:36 p.m. UTC | #1
On Tue, Feb 28, 2017 at 05:28:07PM +0200, Mika Kuoppala wrote:
> +static inline unsigned int
> +i915_pdpes_per_pdp(const struct i915_address_space *vm)
> +{
> +	if (i915_vm_is_48bit(vm))
> +		return GEN8_PML4ES_PER_PML4;
> +
> +	return GEN8_LEGACY_PDPES;
> +}

Does this need to be in the header? Isn't it private to i915_gem_ppgtt.c?

>  /* Equivalent to the gen6 version, For each pde iterates over every pde
>   * between from start until start + length. On gen8+ it simply iterates
>   * over every page directory entry in a page directory.
> @@ -471,7 +483,7 @@ static inline u32 gen6_pde_index(u32 addr)
>  
>  #define gen8_for_each_pdpe(pd, pdp, start, length, iter)		\
>  	for (iter = gen8_pdpe_index(start);				\
> -	     length > 0 && iter < I915_PDPES_PER_PDP(dev) &&		\
> +	     length > 0 && iter < i915_pdpes_per_pdp(vm) &&		\

Oh. Because of a pair of impossible conditions here.

Add rewriting these iterators based on the knowleged that length is
assert to be > 0 and <= total - start to the task list. 

Reviewed-by: Chris Wilson <chris@chris-wsilon.co.uk>
-Chris
Mika Kuoppala March 3, 2017, 2:50 p.m. UTC | #2
Patchwork <patchwork@emeril.freedesktop.org> writes:

> == Series Details ==
>
> Series: series starting with [1/5] drm/i915/gtt: Make I915_PDPES_PER_PDP inline function
> URL   : https://patchwork.freedesktop.org/series/20405/
> State : warning
>
> == Summary ==
>
> Series 20405v1 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/20405/revisions/1/mbox/
>
> Test gem_exec_parallel:
>         Subgroup basic:
>                 pass       -> DMESG-WARN (fi-skl-6700k)

https://bugs.freedesktop.org/show_bug.cgi?id=100051

Pushed, thanks for review.
-Mika


>
> fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11 
> fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39 
> fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19 
> fi-bxt-t5700     total:108  pass:95   dwarn:0   dfail:0   fail:0   skip:12 
> fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31 
> fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
> fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
> fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50 
> fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
> fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
> fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18 
> fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
> fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17 
> fi-skl-6700k     total:278  pass:255  dwarn:5   dfail:0   fail:0   skip:18 
> fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
> fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28 
> fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29 
>
> 5d37006b578e38562382215e8782cfced9c992ce drm-tip: 2017y-02m-28d-16h-27m-13s UTC integration manifest
> 6d729bfd drm/i915/gtt: Setup vm callbacks late
> 0e34328 drm/i915: Avoid using word legacy with ppgtt
> dd57d6d drm/i915/gtt: Prefer i915_vm_is_48bit() over macro
> 19b9ac8 drm/i915: Don't mark pdps clear if pdps are not submitted
> b36ec89 drm/i915/gtt: Make I915_PDPES_PER_PDP inline function
>
> == Logs ==
>
> For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4008/
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e0c9542..5299600 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -528,7 +528,7 @@  static void gen8_initialize_pd(struct i915_address_space *vm,
 static int __pdp_init(struct i915_address_space *vm,
 		      struct i915_page_directory_pointer *pdp)
 {
-	const unsigned int pdpes = I915_PDPES_PER_PDP(vm->i915);
+	const unsigned int pdpes = i915_pdpes_per_pdp(vm);
 	unsigned int i;
 
 	pdp->page_directory = kmalloc_array(pdpes, sizeof(*pdp->page_directory),
@@ -852,7 +852,7 @@  gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt *ppgtt,
 	gen8_pte_t *vaddr;
 	bool ret;
 
-	GEM_BUG_ON(idx->pdpe >= I915_PDPES_PER_PDP(vm));
+	GEM_BUG_ON(idx->pdpe >= i915_pdpes_per_pdp(&ppgtt->base));
 	pd = pdp->page_directory[idx->pdpe];
 	vaddr = kmap_atomic_px(pd->page_table[idx->pde]);
 	do {
@@ -883,7 +883,7 @@  gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt *ppgtt,
 					break;
 				}
 
-				GEM_BUG_ON(idx->pdpe >= I915_PDPES_PER_PDP(vm));
+				GEM_BUG_ON(idx->pdpe >= i915_pdpes_per_pdp(&ppgtt->base));
 				pd = pdp->page_directory[idx->pdpe];
 			}
 
@@ -1036,9 +1036,10 @@  static void gen8_free_scratch(struct i915_address_space *vm)
 static void gen8_ppgtt_cleanup_3lvl(struct i915_address_space *vm,
 				    struct i915_page_directory_pointer *pdp)
 {
+	const unsigned int pdpes = i915_pdpes_per_pdp(vm);
 	int i;
 
-	for (i = 0; i < I915_PDPES_PER_PDP(vm->i915); i++) {
+	for (i = 0; i < pdpes; i++) {
 		if (pdp->page_directory[i] == vm->scratch_pd)
 			continue;
 
@@ -1127,7 +1128,7 @@  static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 			gen8_initialize_pd(vm, pd);
 			gen8_ppgtt_set_pdpe(vm, pdp, pd, pdpe);
 			pdp->used_pdpes++;
-			GEM_BUG_ON(pdp->used_pdpes > I915_PDPES_PER_PDP(vm));
+			GEM_BUG_ON(pdp->used_pdpes > i915_pdpes_per_pdp(vm));
 
 			mark_tlbs_dirty(i915_vm_to_ppgtt(vm));
 		}
@@ -1201,6 +1202,7 @@  static void gen8_dump_pdp(struct i915_hw_ppgtt *ppgtt,
 			  gen8_pte_t scratch_pte,
 			  struct seq_file *m)
 {
+	struct i915_address_space *vm = &ppgtt->base;
 	struct i915_page_directory *pd;
 	u32 pdpe;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index f7d4e19..562c632 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -125,9 +125,6 @@  typedef u64 gen8_ppgtt_pml4e_t;
 #define GEN8_LEGACY_PDPES		4
 #define GEN8_PTES			I915_PTES(sizeof(gen8_pte_t))
 
-#define I915_PDPES_PER_PDP(dev_priv)	(USES_FULL_48BIT_PPGTT(dev_priv) ?\
-					GEN8_PML4ES_PER_PML4 : GEN8_LEGACY_PDPES)
-
 #define PPAT_UNCACHED_INDEX		(_PAGE_PWT | _PAGE_PCD)
 #define PPAT_CACHED_PDE_INDEX		0 /* WB LLC */
 #define PPAT_CACHED_INDEX		_PAGE_PAT /* WB LLCeLLC */
@@ -332,6 +329,12 @@  struct i915_address_space {
 
 #define i915_is_ggtt(V) (!(V)->file)
 
+static inline bool
+i915_vm_is_48bit(const struct i915_address_space *vm)
+{
+	return (vm->total - 1) >> 32;
+}
+
 /* The Graphics Translation Table is the way in which GEN hardware translates a
  * Graphics Virtual Address into a Physical Address. In addition to the normal
  * collateral associated with any va->pa translations GEN hardware also has a
@@ -457,6 +460,15 @@  static inline u32 gen6_pde_index(u32 addr)
 	return i915_pde_index(addr, GEN6_PDE_SHIFT);
 }
 
+static inline unsigned int
+i915_pdpes_per_pdp(const struct i915_address_space *vm)
+{
+	if (i915_vm_is_48bit(vm))
+		return GEN8_PML4ES_PER_PML4;
+
+	return GEN8_LEGACY_PDPES;
+}
+
 /* Equivalent to the gen6 version, For each pde iterates over every pde
  * between from start until start + length. On gen8+ it simply iterates
  * over every page directory entry in a page directory.
@@ -471,7 +483,7 @@  static inline u32 gen6_pde_index(u32 addr)
 
 #define gen8_for_each_pdpe(pd, pdp, start, length, iter)		\
 	for (iter = gen8_pdpe_index(start);				\
-	     length > 0 && iter < I915_PDPES_PER_PDP(dev) &&		\
+	     length > 0 && iter < i915_pdpes_per_pdp(vm) &&		\
 		(pd = (pdp)->page_directory[iter], true);		\
 	     ({ u64 temp = ALIGN(start+1, 1 << GEN8_PDPE_SHIFT);	\
 		    temp = min(temp - start, length);			\
@@ -523,12 +535,6 @@  i915_vm_to_ggtt(struct i915_address_space *vm)
 	return container_of(vm, struct i915_ggtt, base);
 }
 
-static inline bool
-i915_vm_is_48bit(const struct i915_address_space *vm)
-{
-	return (vm->total - 1) >> 32;
-}
-
 int i915_gem_init_aliasing_ppgtt(struct drm_i915_private *i915);
 void i915_gem_fini_aliasing_ppgtt(struct drm_i915_private *i915);