[Intel-gfx] [PATCH 5/9] drm/i915/bdw: Reorganize PT allocations

[Chris Wilson]

On Wed, Feb 12, 2014 at 02:28:48PM -0800, Ben Widawsky wrote:
> -		for (i = first_pte; i < last_pte; i++)
> +		for (i = which_pte; i < last_pte; i++) {
>  			pt_vaddr[i] = scratch_pte;
> +			num_entries--;
> +			BUG_ON(num_entries < 0);
> +		}
>  
>  		kunmap_atomic(pt_vaddr);
>  
> -		num_entries -= last_pte - first_pte;

I'm going to moan about this being replaced by a BUG_ON inside the inner
loop.

> -		first_pte = 0;
> -		act_pt++;
> +		which_pte = 0;

> +		if (which_pde + 1 == GEN8_PDES_PER_PAGE)
> +			which_pdpe++;
> +		which_pde = (which_pde + 1) & GEN8_PDE_MASK;

I think this would be clearer written as
  if (++which_pde == GEN8_PDES_PER_PAGE) {
     which_pdpe++;
     which_pde = 0;
   }
as then the relationship between pdpe and pde is much more apparent to
me. Do we feel that which_pte, which_pde, which_pdpe are really any
better than pte, pde, pdpe? Or is it important to question ourselves
every step of the way?

And I may as well moan about having to preallocate everything. ;-)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre