Re: [PATCH v5 06/19] drm/i915/gen8: implement alloc/free for 4lvl

Michel Thierry
2015-07-29T14:35:01Z

On 7/16/2015 10:33 AM, Michel Thierry wrote:
> PML4 has no special attributes, and there will always be a PML4.
> So simply initialize it at creation, and destroy it at the end.
>
> The code for 4lvl is able to call into the existing 3lvl page table code
> to handle all of the lower levels.
>
> v2: Return something at the end of gen8_alloc_va_range_4lvl to keep the
> compiler happy. And define ret only in one place.
> Updated gen8_ppgtt_unmap_pages and gen8_ppgtt_free to handle 4lvl.
> v3: Use i915_dma_unmap_single instead of pci API. Fix a
> couple of incorrect checks when unmapping pdp and pd pages (Akash).
> v4: Call __pdp_fini also for 32b PPGTT. Clean up alloc_pdp param list.
> v5: Prevent (harmless) out of range access in gen8_for_each_pml4e.
> v6: Simplify alloc_vma_range_4lvl and gen8_ppgtt_init_common error
> paths. (Akash)
> v7: Rebase, s/gen8_ppgtt_free_*/gen8_ppgtt_cleanup_*/.
> v8: Change location of pml4_init/fini. It will make next patches
> cleaner.
> v9: Rebase after Mika's ppgtt cleanup / scratch merge patch series, while
> trying to reuse as much as possible for pdp alloc. pml4_init/fini
> replaced by setup/cleanup_px macros.
> v10: Rebase after Mika's merged ppgtt cleanup patch series.
> v11: Rebase after final merged version of Mika's ppgtt/scratch
> patches.
> v12: Fix pdpe start value in trace (Akash)
> v13: Define all 4lvl functions in this patch directly, instead of
> previous patches, add i915_page_directory_pointer_entry_alloc here,
> use test_bit to detect when pdp is already allocated (Akash).
>
> Cc: Akash Goel <akash.goel@intel.com>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com> (v2+)
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 163 ++++++++++++++++++++++++++++++++----
> drivers/gpu/drm/i915/i915_gem_gtt.h | 13 ++-
> drivers/gpu/drm/i915/i915_trace.h | 8 ++
> 3 files changed, 167 insertions(+), 17 deletions(-)
>
> @@ -1065,6 +1120,79 @@ err_out:
> return ret;
> }
>
> +static int gen8_alloc_va_range_4lvl(struct i915_address_space *vm,
> + struct i915_pml4 *pml4,
> + uint64_t start,
> + uint64_t length)
> +{
> + DECLARE_BITMAP(new_pdps, GEN8_PML4ES_PER_PML4);
> + struct i915_page_directory_pointer *pdp;
> + const uint64_t orig_start = start;
> + const uint64_t orig_length = length;
> + uint64_t temp, pml4e;
> + int ret = 0;
> +
> + /* Do the pml4 allocations first, so we don't need to track the newly
> + * allocated tables below the pdp */
> + bitmap_zero(new_pdps, GEN8_PML4ES_PER_PML4);
> +
> + /* The pagedirectory and pagetable allocations are done in the shared 3
> + * and 4 level code. Just allocate the pdps.
> + */
> + gen8_for_each_pml4e(pdp, pml4, start, length, temp, pml4e) {
> + if (!test_bit(pml4e, pml4->used_pml4es)) {
> + pdp = alloc_pdp(vm->dev);
> + if (IS_ERR(pdp))
> + goto err_out;
> +
> + pml4->pdps[pml4e] = pdp;
> + __set_bit(pml4e, new_pdps);
> + trace_i915_page_directory_pointer_entry_alloc(vm,
> + pml4e,
> + start,
> + GEN8_PML4E_SHIFT);
> + }
> + }
> +

PDP allocation should be moved to a new
gen8_ppgtt_alloc_page_dirpointers function (as we do for pds and pts);
it should also have the pd and pdp setup functions which are mistakenly
added until a later patch ("drm/i915/gen8: Add 4 level switching
infrastructure and lrc support").