On Sat, Nov 02, 2013 at 09:07:22PM -0700, Ben Widawsky wrote: > The upcoming clear and insert routines will expect that PDEs all point > to valid Page Directories. Doing that lazily doesn't really buy us > anything. > > The page allocation is done regardless earlier in init so it shouldn't > hurt set the PDEs. > > v2: Squash in patches to implement fixed PDE write function: > > - If I had done this in the first place, the bug that's going to be > fixed in an upcoming patch would have been much easier to find. > > - Use WB for PDEs. > > The PAT bit is used for page size. 2ME PDEs aren't even supported in > BDW, so this was completely invalid. The solution is to make our > PDEs WB+LLC instead of the pervious WB+eLLC. As far as I can guess, > this change won't matter for performance. > > Thanks to Ville for the quick correction when discussing on IRC. > > Cc: Ville Syrjälä <[email protected]> > Reviewed-by: Imre Deak <[email protected]> > Signed-off-by: Ben Widawsky <[email protected]>
With the small bikeshed below: Reviewed-by: Damien Lespiau <[email protected]> -- Damien > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 4a11f51..bae71b4 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -80,6 +80,19 @@ static inline gen8_gtt_pte_t gen8_pte_encode(dma_addr_t > addr, > return pte; > } > > +static inline gen8_gtt_pte_t gen8_pde_encode(struct drm_device *dev, > + dma_addr_t addr, > + enum i915_cache_level level) Should be returning a gen8_ppgtt_pde_t (not a bug as they are the same size, of course). > +{ > + gen8_ppgtt_pde_t pde = _PAGE_PRESENT | _PAGE_RW; > + pde |= addr; > + if (level != I915_CACHE_NONE) > + pde |= PPAT_CACHED_PDE_INDEX; > + else > + pde |= PPAT_UNCACHED_INDEX; > + return pde; > +} > + > static gen6_gtt_pte_t snb_pte_encode(dma_addr_t addr, > enum i915_cache_level level, > bool valid) > @@ -285,6 +298,20 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, > uint64_t size) > } > } > > + /* For now, the PPGTT helper functions all require that the PDEs are > + * plugged in correctly. So we do that now/here. For aliasing PPGTT, we > + * will never need to touch the PDEs again */ > + for (i = 0; i < max_pdp; i++) { > + gen8_ppgtt_pde_t *pd_vaddr; > + pd_vaddr = kmap_atomic(&ppgtt->pd_pages[i]); > + for (j = 0; j < GEN8_PDES_PER_PAGE; j++) { > + dma_addr_t addr = ppgtt->gen8_pt_dma_addr[i][j]; > + pd_vaddr[j] = gen8_pde_encode(ppgtt->base.dev, addr, > + I915_CACHE_LLC); > + } > + kunmap_atomic(pd_vaddr); > + } > + > DRM_DEBUG_DRIVER("Allocated %d pages for page directories (%d > wasted)\n", > ppgtt->num_pd_pages, ppgtt->num_pd_pages - max_pdp); > DRM_DEBUG_DRIVER("Allocated %d pages for page tables (%lld wasted)\n", > -- > 1.8.4.2 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
