Re: [Intel-gfx] [PATCH 4/4] drm/i915/gtt: Tidy up ppgtt insertion for gen8
Quoting Abdiel Janulgue (2019-07-16 16:30:09) > > > On 12/07/2019 14.27, Chris Wilson wrote: > > Apply the new radix shift helpers to extract the multi-level indices > > cleanly when inserting pte into the gtt tree. > > > Reviewed-by: Abdiel Janulgue Thanks for the reviews, -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915/gtt: Tidy up ppgtt insertion for gen8
On 12/07/2019 14.27, Chris Wilson wrote:
> Apply the new radix shift helpers to extract the multi-level indices
> cleanly when inserting pte into the gtt tree.
>
Reviewed-by: Abdiel Janulgue
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 115 +++-
> drivers/gpu/drm/i915/i915_gem_gtt.h | 90 ++
> 2 files changed, 48 insertions(+), 157 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 72e0f9799a46..de78dc8c425c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1131,47 +1131,28 @@ static inline struct sgt_dma {
> return (struct sgt_dma) { sg, addr, addr + sg->length };
> }
>
> -struct gen8_insert_pte {
> - u16 pml4e;
> - u16 pdpe;
> - u16 pde;
> - u16 pte;
> -};
> -
> -static __always_inline struct gen8_insert_pte gen8_insert_pte(u64 start)
> -{
> - return (struct gen8_insert_pte) {
> - gen8_pml4e_index(start),
> - gen8_pdpe_index(start),
> - gen8_pde_index(start),
> - gen8_pte_index(start),
> - };
> -}
> -
> -static __always_inline bool
> +static __always_inline u64
> gen8_ppgtt_insert_pte_entries(struct i915_ppgtt *ppgtt,
> struct i915_page_directory *pdp,
> struct sgt_dma *iter,
> - struct gen8_insert_pte *idx,
> + u64 idx,
> enum i915_cache_level cache_level,
> u32 flags)
> {
> struct i915_page_directory *pd;
> const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags);
> gen8_pte_t *vaddr;
> - bool ret;
>
> - GEM_BUG_ON(idx->pdpe >= i915_pdpes_per_pdp(&ppgtt->vm));
> - pd = i915_pd_entry(pdp, idx->pdpe);
> - vaddr = kmap_atomic_px(i915_pt_entry(pd, idx->pde));
> + pd = i915_pd_entry(pdp, gen8_pd_index(idx, 2));
> + vaddr = kmap_atomic_px(i915_pt_entry(pd, gen8_pd_index(idx, 1)));
> do {
> - vaddr[idx->pte] = pte_encode | iter->dma;
> + vaddr[gen8_pd_index(idx, 0)] = pte_encode | iter->dma;
>
> iter->dma += I915_GTT_PAGE_SIZE;
> if (iter->dma >= iter->max) {
> iter->sg = __sg_next(iter->sg);
> if (!iter->sg) {
> - ret = false;
> + idx = 0;
> break;
> }
>
> @@ -1179,30 +1160,22 @@ gen8_ppgtt_insert_pte_entries(struct i915_ppgtt
> *ppgtt,
> iter->max = iter->dma + iter->sg->length;
> }
>
> - if (++idx->pte == GEN8_PTES) {
> - idx->pte = 0;
> -
> - if (++idx->pde == I915_PDES) {
> - idx->pde = 0;
> -
> + if (gen8_pd_index(++idx, 0) == 0) {
> + if (gen8_pd_index(idx, 1) == 0) {
> /* Limited by sg length for 3lvl */
> - if (++idx->pdpe == GEN8_PML4ES_PER_PML4) {
> - idx->pdpe = 0;
> - ret = true;
> + if (gen8_pd_index(idx, 2) == 0)
> break;
> - }
>
> - GEM_BUG_ON(idx->pdpe >=
> i915_pdpes_per_pdp(&ppgtt->vm));
> - pd = pdp->entry[idx->pdpe];
> + pd = pdp->entry[gen8_pd_index(idx, 2)];
> }
>
> kunmap_atomic(vaddr);
> - vaddr = kmap_atomic_px(i915_pt_entry(pd, idx->pde));
> + vaddr = kmap_atomic_px(i915_pt_entry(pd,
> gen8_pd_index(idx, 1)));
> }
> } while (1);
> kunmap_atomic(vaddr);
>
> - return ret;
> + return idx;
> }
>
> static void gen8_ppgtt_insert_3lvl(struct i915_address_space *vm,
> @@ -1212,9 +1185,9 @@ static void gen8_ppgtt_insert_3lvl(struct
> i915_address_space *vm,
> {
> struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
> struct sgt_dma iter = sgt_dma(vma);
> - struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
>
> - gen8_ppgtt_insert_pte_entries(ppgtt, ppgtt->pd, &iter, &idx,
> + gen8_ppgtt_insert_pte_entries(ppgtt, ppgtt->pd, &iter,
> + vma->node.start >> GEN8_PTE_SHIFT,
> cache_level, flags);
>
> vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
> @@ -1231,39 +1204,38 @@ static void gen8_ppgtt_insert_huge_entries(struct
> i915_vma *vma,
> dma_addr_t rem = iter->sg->length;
>
> do {
> - struct gen8_insert_pte idx = gen8_insert_pte(start);
> struct i915_page_directory
[Intel-gfx] [PATCH 4/4] drm/i915/gtt: Tidy up ppgtt insertion for gen8
Apply the new radix shift helpers to extract the multi-level indices
cleanly when inserting pte into the gtt tree.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 115 +++-
drivers/gpu/drm/i915/i915_gem_gtt.h | 90 ++
2 files changed, 48 insertions(+), 157 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 72e0f9799a46..de78dc8c425c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1131,47 +1131,28 @@ static inline struct sgt_dma {
return (struct sgt_dma) { sg, addr, addr + sg->length };
}
-struct gen8_insert_pte {
- u16 pml4e;
- u16 pdpe;
- u16 pde;
- u16 pte;
-};
-
-static __always_inline struct gen8_insert_pte gen8_insert_pte(u64 start)
-{
- return (struct gen8_insert_pte) {
-gen8_pml4e_index(start),
-gen8_pdpe_index(start),
-gen8_pde_index(start),
-gen8_pte_index(start),
- };
-}
-
-static __always_inline bool
+static __always_inline u64
gen8_ppgtt_insert_pte_entries(struct i915_ppgtt *ppgtt,
struct i915_page_directory *pdp,
struct sgt_dma *iter,
- struct gen8_insert_pte *idx,
+ u64 idx,
enum i915_cache_level cache_level,
u32 flags)
{
struct i915_page_directory *pd;
const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags);
gen8_pte_t *vaddr;
- bool ret;
- GEM_BUG_ON(idx->pdpe >= i915_pdpes_per_pdp(&ppgtt->vm));
- pd = i915_pd_entry(pdp, idx->pdpe);
- vaddr = kmap_atomic_px(i915_pt_entry(pd, idx->pde));
+ pd = i915_pd_entry(pdp, gen8_pd_index(idx, 2));
+ vaddr = kmap_atomic_px(i915_pt_entry(pd, gen8_pd_index(idx, 1)));
do {
- vaddr[idx->pte] = pte_encode | iter->dma;
+ vaddr[gen8_pd_index(idx, 0)] = pte_encode | iter->dma;
iter->dma += I915_GTT_PAGE_SIZE;
if (iter->dma >= iter->max) {
iter->sg = __sg_next(iter->sg);
if (!iter->sg) {
- ret = false;
+ idx = 0;
break;
}
@@ -1179,30 +1160,22 @@ gen8_ppgtt_insert_pte_entries(struct i915_ppgtt *ppgtt,
iter->max = iter->dma + iter->sg->length;
}
- if (++idx->pte == GEN8_PTES) {
- idx->pte = 0;
-
- if (++idx->pde == I915_PDES) {
- idx->pde = 0;
-
+ if (gen8_pd_index(++idx, 0) == 0) {
+ if (gen8_pd_index(idx, 1) == 0) {
/* Limited by sg length for 3lvl */
- if (++idx->pdpe == GEN8_PML4ES_PER_PML4) {
- idx->pdpe = 0;
- ret = true;
+ if (gen8_pd_index(idx, 2) == 0)
break;
- }
- GEM_BUG_ON(idx->pdpe >=
i915_pdpes_per_pdp(&ppgtt->vm));
- pd = pdp->entry[idx->pdpe];
+ pd = pdp->entry[gen8_pd_index(idx, 2)];
}
kunmap_atomic(vaddr);
- vaddr = kmap_atomic_px(i915_pt_entry(pd, idx->pde));
+ vaddr = kmap_atomic_px(i915_pt_entry(pd,
gen8_pd_index(idx, 1)));
}
} while (1);
kunmap_atomic(vaddr);
- return ret;
+ return idx;
}
static void gen8_ppgtt_insert_3lvl(struct i915_address_space *vm,
@@ -1212,9 +1185,9 @@ static void gen8_ppgtt_insert_3lvl(struct
i915_address_space *vm,
{
struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
struct sgt_dma iter = sgt_dma(vma);
- struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
- gen8_ppgtt_insert_pte_entries(ppgtt, ppgtt->pd, &iter, &idx,
+ gen8_ppgtt_insert_pte_entries(ppgtt, ppgtt->pd, &iter,
+ vma->node.start >> GEN8_PTE_SHIFT,
cache_level, flags);
vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
@@ -1231,39 +1204,38 @@ static void gen8_ppgtt_insert_huge_entries(struct
i915_vma *vma,
dma_addr_t rem = iter->sg->length;
do {
- struct gen8_insert_pte idx = gen8_insert_pte(start);
struct i915_page_directory *pdp =
- i915_pdp_entry(pml4, idx.pml4e);
- struct i915_page_directory *pd = i915_pd_entry(pdp, idx.pdpe);
- unsigned int page_s
