| Message ID | 20190611172731.19174-8-mika.kuoppala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/9] drm/i915/gtt: No need to zero the table for page dirs | expand |
Quoting Mika Kuoppala (2019-06-11 18:27:30) > Swapping a pd entry is same across the page directories, if > we succeed we need to increment the count and write the phys page > entry. Make a common function for it. > > Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 41 +++++++++++++++++++---------- > 1 file changed, 27 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index f1d7874834e2..9b0d0a077e31 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -752,6 +752,27 @@ static void __set_pd_entry(struct i915_page_directory * const pd, > gen8_pde_encode(px_dma(to), I915_CACHE_LLC)); \ > }) > > +static void *__swap_pd_entry(struct i915_page_directory * const pd, > + const unsigned short pde, > + void * const old_val, > + void * const new_val, > + const u64 encoded_entry) Mark this as inline and pass in the encode function, the compiler should do the rest. > +{ > + void * const old = cmpxchg(&pd->entry[pde], old_val, new_val); > + > + if (likely(old == old_val)) { > + atomic_inc(&pd->used); Hmm, looking at this again, this would be safer if atomic_inc was before the cmpxchg, with an atomic_dec on the fail path. > + if (likely(pd_has_phys_page(pd))) > + __set_pd_entry(pd, pde, encoded_entry); > + } > + > + return old; > +} -Chris
Quoting Chris Wilson (2019-06-11 20:50:09) > Quoting Mika Kuoppala (2019-06-11 18:27:30) > > Swapping a pd entry is same across the page directories, if > > we succeed we need to increment the count and write the phys page > > entry. Make a common function for it. > > > > Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 41 +++++++++++++++++++---------- > > 1 file changed, 27 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index f1d7874834e2..9b0d0a077e31 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -752,6 +752,27 @@ static void __set_pd_entry(struct i915_page_directory * const pd, > > gen8_pde_encode(px_dma(to), I915_CACHE_LLC)); \ > > }) > > > > +static void *__swap_pd_entry(struct i915_page_directory * const pd, > > + const unsigned short pde, > > + void * const old_val, > > + void * const new_val, > > + const u64 encoded_entry) > > Mark this as inline and pass in the encode function, the compiler should > do the rest. > > > +{ > > + void * const old = cmpxchg(&pd->entry[pde], old_val, new_val); > > + > > + if (likely(old == old_val)) { > > + atomic_inc(&pd->used); > > Hmm, looking at this again, this would be safer if atomic_inc was before > the cmpxchg, with an atomic_dec on the fail path. Actually already taken care of, the parent pins the child page directory before entering. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index f1d7874834e2..9b0d0a077e31 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -752,6 +752,27 @@ static void __set_pd_entry(struct i915_page_directory * const pd, gen8_pde_encode(px_dma(to), I915_CACHE_LLC)); \ }) +static void *__swap_pd_entry(struct i915_page_directory * const pd, + const unsigned short pde, + void * const old_val, + void * const new_val, + const u64 encoded_entry) +{ + void * const old = cmpxchg(&pd->entry[pde], old_val, new_val); + + if (likely(old == old_val)) { + atomic_inc(&pd->used); + if (likely(pd_has_phys_page(pd))) + __set_pd_entry(pd, pde, encoded_entry); + } + + return old; +} + +#define swap_pd_entry(pd, pde, old, to) \ + __swap_pd_entry((pd), (pde), (old), (to), \ + gen8_pde_encode(px_dma(to), I915_CACHE_LLC)) + /* * PDE TLBs are a pain to invalidate on GEN8+. When we modify * the page table structures, we mark them dirty so that @@ -1326,11 +1347,8 @@ static int gen8_ppgtt_alloc_pd(struct i915_address_space *vm, if (count < GEN8_PTES || intel_vgpu_active(vm->i915)) gen8_initialize_pt(vm, pt); - old = cmpxchg(&pd->entry[pde], vm->scratch_pt, pt); - if (old == vm->scratch_pt) { - set_pd_entry(pd, pde, pt); - atomic_inc(&pd->used); - } else { + old = swap_pd_entry(pd, pde, vm->scratch_pt, pt); + if (unlikely(old != vm->scratch_pt)) { free_pt(vm, pt); pt = old; } @@ -1371,11 +1389,8 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm, init_pd(vm, pd, vm->scratch_pt); - old = cmpxchg(&pdp->entry[pdpe], vm->scratch_pd, pd); - if (old == vm->scratch_pd) { - set_pd_entry(pdp, pdpe, pd); - atomic_inc(&pdp->used); - } else { + old = swap_pd_entry(pdp, pdpe, vm->scratch_pd, pd); + if (unlikely(old != vm->scratch_pd)) { free_pd(vm, pd); pd = old; } @@ -1440,10 +1455,8 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm, init_pd(vm, pdp, vm->scratch_pd); - old = cmpxchg(&pml4->entry[pml4e], vm->scratch_pdp, pdp); - if (old == vm->scratch_pdp) { - set_pd_entry(pml4, pml4e, pdp); - } else { + old = swap_pd_entry(pml4, pml4e, vm->scratch_pdp, pdp); + if (unlikely(old != vm->scratch_pdp)) { free_pd(vm, pdp); pdp = old; }
Swapping a pd entry is same across the page directories, if we succeed we need to increment the count and write the phys page entry. Make a common function for it. Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- drivers/gpu/drm/i915/i915_gem_gtt.c | 41 +++++++++++++++++++---------- 1 file changed, 27 insertions(+), 14 deletions(-)