Re: [PATCH v3 03/17] drm/i915/gen8: Abstract PDP usage

From: Michel Thierry <michel.thierry@intel.com>
To: "Goel, Akash" <akash.goel@intel.com>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v3 03/17] drm/i915/gen8: Abstract PDP usage
Date: Tue, 7 Jul 2015 14:35:40 +0100	[thread overview]
Message-ID: <559BD5AC.9060606@intel.com> (raw)
In-Reply-To: <559BC95C.1010801@intel.com>

On 7/7/2015 1:43 PM, Goel, Akash wrote:
>
>
> On 7/1/2015 8:57 PM, Michel Thierry wrote:
>> @@ -795,13 +821,15 @@ 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 i915_hw_ppgtt *ppgtt =
>> +        container_of(vm, struct i915_hw_ppgtt, base);
>> +    struct drm_device *dev = vm->dev;
>
> The 'ppgtt' pointer can be completely dispensed with, by the use of 'vm'
> pointer.
>
Leftovers from old rebases, I'll clean these functions up.

>>       struct i915_page_table *pt;
>>       uint64_t temp;
>>       uint32_t pde;
>> @@ -1024,18 +1051,10 @@ static int gen8_alloc_va_range(struct
>> i915_address_space *vm,
>>
>>               /* Our pde is now pointing to the pagetable, pt */
>>               __set_bit(pde, pd->used_pdes);
>> -
>> -            /* Map the PDE to the page table */
>> -            page_directory[pde] = gen8_pde_encode(px_dma(pt),
>> -                                  I915_CACHE_LLC);
>> -
>> -            /* NB: We haven't yet mapped ptes to pages. At this
>> -             * point we're still relying on insert_entries() */
>>           }
>>
>> -        kunmap_px(ppgtt, page_directory);
>> -
>> -        __set_bit(pdpe, ppgtt->pdp.used_pdpes);
>> +        __set_bit(pdpe, pdp->used_pdpes);
>> +        gen8_map_pagetable_range(ppgtt, pd, start, length);
>
> No apparent benefit in use of "gen8_map_pagetable_range", considering
> that the Page Directory page is mapped at the start of outer loop
> (PDPEs) only and the inner loop (PDEs) maps the PDEs to the page tables,
> in an inline manner.
> The 'gen8_map_pagetable_range', will repeat the same inner loop of PDEs.
>

And I'll discard these changes.

-Michel
>>       }
>>
>>       free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx