Re: [PATCH v3 12/17] drm/i915/gen8: Add ppgtt info and debug_dump

[Michel Thierry <michel.thierry@intel.com>]

On 7/7/2015 1:56 PM, Goel, Akash wrote:
> On 7/1/2015 8:57 PM, Michel Thierry wrote:
>> v2: Clean up patch after rebases.
>> v3: gen8_dump_ppgtt for 32b and 48b PPGTT.
>> v4: Use used_pml4es/pdpes (Akash).
>> v5: Rebase after Mika's ppgtt cleanup / scratch merge patch series.
>>
>> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
>> Signed-off-by: Michel Thierry <michel.thierry@intel.com> (v2+)
>> ---
>>   drivers/gpu/drm/i915/i915_debugfs.c | 18 ++++----
>>   drivers/gpu/drm/i915/i915_gem_gtt.c | 92
>> +++++++++++++++++++++++++++++++++++++
>>   2 files changed, 102 insertions(+), 8 deletions(-)
>> +static void gen8_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct
>> seq_file *m)
>> +{
>> +    struct i915_address_space *vm = &ppgtt->base;
>> +    uint64_t start = ppgtt->base.start;
>> +    uint64_t length = ppgtt->base.total;
>> +    gen8_pte_t scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
>> +                         I915_CACHE_LLC, true);
>> +
>> +    if (!USES_FULL_48BIT_PPGTT(vm->dev)) {
>> +        gen8_dump_pdp(&ppgtt->pdp, start, length, scratch_pte, m);
>> +    } else {
>> +        uint64_t templ4, pml4e;
>> +        struct i915_pml4 *pml4 = &ppgtt->pml4;
>> +        struct i915_page_directory_pointer *pdp;
>> +
>> +        gen8_for_each_pml4e(pdp, pml4, start, length, templ4, pml4e) {
>> +            uint64_t pdp_len = length;
>> +            uint64_t pdp_start = start;
>> +
>> +            if (!pdp)
>> +                continue;
>> +
> I think the "if (!test_bit(pml4e, pml4->used_pml4es))" check is
> foolproof & should suffice.
> No real need of the extra check of 'if (!pdp)'.
> Same for pdpe & pde loops in gen8_dump_pdp function
>
Right, I'll changed it to use test_bit across the board, also remove the 
unnecessary pdp_len/pdp_start variables:
-        if (!pd)
-            continue;
-
          if(!test_bit(pdpe, pdp->used_pdpes))
              continue;
-----------
-            if (!pt)
+            if(!test_bit(pde, pd->used_pdes))
                  continue;
-----------
          gen8_for_each_pml4e(pdp, pml4, start, length, templ4, pml4e) {
-            uint64_t pdp_len = length;
-            uint64_t pdp_start = start;
-
-            if (!pdp)
-                continue;
-
              if (!test_bit(pml4e, pml4->used_pml4es))
                  continue;

              seq_printf(m, "    PML4E #%llu\n", pml4e);
-            gen8_dump_pdp(pdp, pdp_start, pdp_len, scratch_pte, m);
+            gen8_dump_pdp(pdp, start, length, scratch_pte, m);
-----------

>> +            if (!test_bit(pml4e, pml4->used_pml4es))
>> +                continue;
>> +
>> +            seq_printf(m, "    PML4E #%llu\n", pml4e);
>> +            gen8_dump_pdp(pdp, pdp_start, pdp_len, scratch_pte, m);
>> +        }
>> +    }
>> +}
>> +
>>   /*
>>    * GEN8 legacy ppgtt programming is accomplished through a max 4 PDP
>> registers
>>    * with a net effect resembling a 2-level page table in normal x86
>> terms. Each
>> @@ -1359,6 +1450,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt
>> *ppgtt)
>>       ppgtt->base.clear_range = gen8_ppgtt_clear_range;
>>       ppgtt->base.unbind_vma = ppgtt_unbind_vma;
>>       ppgtt->base.bind_vma = ppgtt_bind_vma;
>> +    ppgtt->debug_dump = gen8_dump_ppgtt;
>>
>>       if (USES_FULL_48BIT_PPGTT(ppgtt->base.dev)) {
>>           ret = setup_px(ppgtt->base.dev, &ppgtt->pml4);
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx