[Intel-gfx] [PATCH v7 03/19] drm/i915/gen8: Abstract PDP usage
Daniel Vetter
daniel at ffwll.ch
Wed Aug 5 08:33:41 PDT 2015
On Fri, Jul 31, 2015 at 09:41:11AM +0530, Goel, Akash wrote:
> Reviewed the patch & it looks fine.
> Reviewed-by: "Akash Goel <akash.goel at intel.com>"
Just an aside "" is not required in your tag here and actually breaks it
as a proper mail address - "" if needed should only wrap the name, but
must not include the mail address itself.
-Daniel
>
>
> On 7/30/2015 3:32 PM, Michel Thierry wrote:
> >Up until now, ppgtt->pdp has always been the root of our page tables.
> >Legacy 32b addresses acted like it had 1 PDP with 4 PDPEs.
> >
> >In preparation for 4 level page tables, we need to stop using ppgtt->pdp
> >directly unless we know it's what we want. The future structure will use
> >ppgtt->pml4 for the top level, and the pdp is just one of the entries
> >being pointed to by a pml4e. The temporal pdp local variable will be
> >removed once the rest of the 4-level code lands.
> >
> >Also, start passing the vm pointer to the alloc functions, instead of
> >ppgtt.
> >
> >v2: Updated after dynamic page allocation changes.
> >v3: Rebase after s/page_tables/page_table/.
> >v4: Rebase after changes in "Dynamic page table allocations" patch.
> >v5: Rebase after Mika's ppgtt cleanup / scratch merge patch series.
> >v6: Rebase after final merged version of Mika's ppgtt/scratch patches.
> >v7: Keep pagetable map in-line (and avoid unnecessary for_each_pde
> >loops), remove redundant ppgtt pointer in _alloc_pagetabs (Akash)
> >v8: Fix text indentation in _alloc_pagetabs/page_directories (Chris)
> >v9: Defer gen8_alloc_va_range_4lvl definition until 4lvl is implemented,
> >clean-up gen8_ppgtt_cleanup [pun intended] (Akash).
> >v10: Clean-up commit message (Akash).
> >
> >Cc: Akash Goel <akash.goel at intel.com>
> >Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> >Signed-off-by: Michel Thierry <michel.thierry at intel.com> (v2+)
> >---
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 84 +++++++++++++++++++------------------
> > 1 file changed, 44 insertions(+), 40 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >index 28f3227..bd56979 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >@@ -607,6 +607,7 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
> > {
> > struct i915_hw_ppgtt *ppgtt =
> > container_of(vm, struct i915_hw_ppgtt, base);
> >+ struct i915_page_directory_pointer *pdp = &ppgtt->pdp; /* FIXME: 48b */
> > gen8_pte_t *pt_vaddr, scratch_pte;
> > unsigned pdpe = start >> GEN8_PDPE_SHIFT & GEN8_PDPE_MASK;
> > unsigned pde = start >> GEN8_PDE_SHIFT & GEN8_PDE_MASK;
> >@@ -621,10 +622,10 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
> > struct i915_page_directory *pd;
> > struct i915_page_table *pt;
> >
> >- if (WARN_ON(!ppgtt->pdp.page_directory[pdpe]))
> >+ if (WARN_ON(!pdp->page_directory[pdpe]))
> > break;
> >
> >- pd = ppgtt->pdp.page_directory[pdpe];
> >+ pd = pdp->page_directory[pdpe];
> >
> > if (WARN_ON(!pd->page_table[pde]))
> > break;
> >@@ -662,6 +663,7 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
> > {
> > struct i915_hw_ppgtt *ppgtt =
> > container_of(vm, struct i915_hw_ppgtt, base);
> >+ struct i915_page_directory_pointer *pdp = &ppgtt->pdp; /* FIXME: 48b */
> > gen8_pte_t *pt_vaddr;
> > unsigned pdpe = start >> GEN8_PDPE_SHIFT & GEN8_PDPE_MASK;
> > unsigned pde = start >> GEN8_PDE_SHIFT & GEN8_PDE_MASK;
> >@@ -675,7 +677,7 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
> > break;
> >
> > if (pt_vaddr == NULL) {
> >- struct i915_page_directory *pd = ppgtt->pdp.page_directory[pdpe];
> >+ struct i915_page_directory *pd = pdp->page_directory[pdpe];
> > struct i915_page_table *pt = pd->page_table[pde];
> > pt_vaddr = kmap_px(pt);
> > }
> >@@ -755,28 +757,29 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
> > {
> > struct i915_hw_ppgtt *ppgtt =
> > container_of(vm, struct i915_hw_ppgtt, base);
> >+ struct i915_page_directory_pointer *pdp = &ppgtt->pdp; /* FIXME: 48b */
> >+ struct drm_device *dev = ppgtt->base.dev;
> > int i;
> >
> >- for_each_set_bit(i, ppgtt->pdp.used_pdpes,
> >- I915_PDPES_PER_PDP(ppgtt->base.dev)) {
> >- if (WARN_ON(!ppgtt->pdp.page_directory[i]))
> >+ for_each_set_bit(i, pdp->used_pdpes, I915_PDPES_PER_PDP(dev)) {
> >+ if (WARN_ON(!pdp->page_directory[i]))
> > continue;
> >
> >- gen8_free_page_tables(ppgtt->base.dev,
> >- ppgtt->pdp.page_directory[i]);
> >- free_pd(ppgtt->base.dev, ppgtt->pdp.page_directory[i]);
> >+ gen8_free_page_tables(dev, pdp->page_directory[i]);
> >+ free_pd(dev, pdp->page_directory[i]);
> > }
> >
> >- free_pdp(ppgtt->base.dev, &ppgtt->pdp);
> >+ free_pdp(dev, pdp);
> >+
> > gen8_free_scratch(vm);
> > }
> >
> > /**
> > * gen8_ppgtt_alloc_pagetabs() - Allocate page tables for VA range.
> >- * @ppgtt: Master ppgtt structure.
> >- * @pd: Page directory for this address range.
> >+ * @vm: Master vm structure.
> >+ * @pd: Page directory for this address range.
> > * @start: Starting virtual address to begin allocations.
> >- * @length Size of the allocations.
> >+ * @length: Size of the allocations.
> > * @new_pts: Bitmap set by function with new allocations. Likely used by the
> > * caller to free on error.
> > *
> >@@ -789,13 +792,13 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
> > *
> > * Return: 0 if success; negative error code otherwise.
> > */
> >-static int gen8_ppgtt_alloc_pagetabs(struct i915_hw_ppgtt *ppgtt,
> >+static int gen8_ppgtt_alloc_pagetabs(struct i915_address_space *vm,
> > struct i915_page_directory *pd,
> > uint64_t start,
> > uint64_t length,
> > unsigned long *new_pts)
> > {
> >- struct drm_device *dev = ppgtt->base.dev;
> >+ struct drm_device *dev = vm->dev;
> > struct i915_page_table *pt;
> > uint64_t temp;
> > uint32_t pde;
> >@@ -804,7 +807,7 @@ static int gen8_ppgtt_alloc_pagetabs(struct i915_hw_ppgtt *ppgtt,
> > /* Don't reallocate page tables */
> > if (test_bit(pde, pd->used_pdes)) {
> > /* Scratch is never allocated this way */
> >- WARN_ON(pt == ppgtt->base.scratch_pt);
> >+ WARN_ON(pt == vm->scratch_pt);
> > continue;
> > }
> >
> >@@ -812,7 +815,7 @@ static int gen8_ppgtt_alloc_pagetabs(struct i915_hw_ppgtt *ppgtt,
> > if (IS_ERR(pt))
> > goto unwind_out;
> >
> >- gen8_initialize_pt(&ppgtt->base, pt);
> >+ gen8_initialize_pt(vm, pt);
> > pd->page_table[pde] = pt;
> > __set_bit(pde, new_pts);
> > }
> >@@ -828,11 +831,11 @@ unwind_out:
> >
> > /**
> > * gen8_ppgtt_alloc_page_directories() - Allocate page directories for VA range.
> >- * @ppgtt: Master ppgtt structure.
> >+ * @vm: Master vm structure.
> > * @pdp: Page directory pointer for this address range.
> > * @start: Starting virtual address to begin allocations.
> >- * @length Size of the allocations.
> >- * @new_pds Bitmap set by function with new allocations. Likely used by the
> >+ * @length: Size of the allocations.
> >+ * @new_pds: Bitmap set by function with new allocations. Likely used by the
> > * caller to free on error.
> > *
> > * Allocate the required number of page directories starting at the pde index of
> >@@ -849,13 +852,14 @@ unwind_out:
> > *
> > * Return: 0 if success; negative error code otherwise.
> > */
> >-static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
> >- struct i915_page_directory_pointer *pdp,
> >- uint64_t start,
> >- uint64_t length,
> >- unsigned long *new_pds)
> >+static int
> >+gen8_ppgtt_alloc_page_directories(struct i915_address_space *vm,
> >+ struct i915_page_directory_pointer *pdp,
> >+ uint64_t start,
> >+ uint64_t length,
> >+ unsigned long *new_pds)
> > {
> >- struct drm_device *dev = ppgtt->base.dev;
> >+ struct drm_device *dev = vm->dev;
> > struct i915_page_directory *pd;
> > uint64_t temp;
> > uint32_t pdpe;
> >@@ -871,7 +875,7 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
> > if (IS_ERR(pd))
> > goto unwind_out;
> >
> >- gen8_initialize_pd(&ppgtt->base, pd);
> >+ gen8_initialize_pd(vm, pd);
> > pdp->page_directory[pdpe] = pd;
> > __set_bit(pdpe, new_pds);
> > }
> >@@ -947,18 +951,19 @@ static void mark_tlbs_dirty(struct i915_hw_ppgtt *ppgtt)
> > }
> >
> > static int gen8_alloc_va_range(struct i915_address_space *vm,
> >- uint64_t start,
> >- uint64_t length)
> >+ uint64_t start, uint64_t length)
> > {
> > struct i915_hw_ppgtt *ppgtt =
> > container_of(vm, struct i915_hw_ppgtt, base);
> > unsigned long *new_page_dirs, **new_page_tables;
> >+ struct drm_device *dev = vm->dev;
> >+ struct i915_page_directory_pointer *pdp = &ppgtt->pdp; /* FIXME: 48b */
> > struct i915_page_directory *pd;
> > const uint64_t orig_start = start;
> > const uint64_t orig_length = length;
> > uint64_t temp;
> > uint32_t pdpe;
> >- uint32_t pdpes = I915_PDPES_PER_PDP(ppgtt->base.dev);
> >+ uint32_t pdpes = I915_PDPES_PER_PDP(dev);
> > int ret;
> >
> > /* Wrap is never okay since we can only represent 48b, and we don't
> >@@ -967,7 +972,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
> > if (WARN_ON(start + length < start))
> > return -ENODEV;
> >
> >- if (WARN_ON(start + length > ppgtt->base.total))
> >+ if (WARN_ON(start + length > vm->total))
> > return -ENODEV;
> >
> > ret = alloc_gen8_temp_bitmaps(&new_page_dirs, &new_page_tables, pdpes);
> >@@ -975,16 +980,16 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
> > return ret;
> >
> > /* Do the allocations first so we can easily bail out */
> >- ret = gen8_ppgtt_alloc_page_directories(ppgtt, &ppgtt->pdp, start, length,
> >- new_page_dirs);
> >+ ret = gen8_ppgtt_alloc_page_directories(vm, pdp, start, length,
> >+ new_page_dirs);
> > if (ret) {
> > free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
> > return ret;
> > }
> >
> > /* For every page directory referenced, allocate page tables */
> >- gen8_for_each_pdpe(pd, &ppgtt->pdp, start, length, temp, pdpe) {
> >- ret = gen8_ppgtt_alloc_pagetabs(ppgtt, pd, start, length,
> >+ gen8_for_each_pdpe(pd, pdp, start, length, temp, pdpe) {
> >+ ret = gen8_ppgtt_alloc_pagetabs(vm, pd, start, length,
> > new_page_tables[pdpe]);
> > if (ret)
> > goto err_out;
> >@@ -995,7 +1000,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
> >
> > /* Allocations have completed successfully, so set the bitmaps, and do
> > * the mappings. */
> >- gen8_for_each_pdpe(pd, &ppgtt->pdp, start, length, temp, pdpe) {
> >+ gen8_for_each_pdpe(pd, pdp, start, length, temp, pdpe) {
> > gen8_pde_t *const page_directory = kmap_px(pd);
> > struct i915_page_table *pt;
> > uint64_t pd_len = length;
> >@@ -1028,8 +1033,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
> > }
> >
> > kunmap_px(ppgtt, page_directory);
> >-
> >- __set_bit(pdpe, ppgtt->pdp.used_pdpes);
> >+ __set_bit(pdpe, pdp->used_pdpes);
> > }
> >
> > free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
> >@@ -1039,11 +1043,11 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
> > err_out:
> > while (pdpe--) {
> > for_each_set_bit(temp, new_page_tables[pdpe], I915_PDES)
> >- free_pt(vm->dev, ppgtt->pdp.page_directory[pdpe]->page_table[temp]);
> >+ free_pt(dev, pdp->page_directory[pdpe]->page_table[temp]);
> > }
> >
> > for_each_set_bit(pdpe, new_page_dirs, pdpes)
> >- free_pd(vm->dev, ppgtt->pdp.page_directory[pdpe]);
> >+ free_pd(dev, pdp->page_directory[pdpe]);
> >
> > free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
> > mark_tlbs_dirty(ppgtt);
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list