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

Michel Thierry
2015-07-07T13:35:42Z

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);