Re: [PATCH] drm/i915: Setup all page directories for gen8

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Mar 04, 2015 at 09:14:39PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 04, 2015 at 02:55:17PM +0200, Mika Kuoppala wrote:
> > If the requested size is less than what the full range
> > of pdps can address, we end up setting pdps for only the
> > requested area.
> > 
> > The logical context however needs all pdp entries to be valid.
> > Prior to commit 06fda602dbca ("drm/i915: Create page table allocators")
> > we have been writing pdp entries with dma address of zero instead
> > of valid pdps. This is supposedly bad even if those pdps are not
> > addressed.
> > 
> > As commit 06fda602dbca ("drm/i915: Create page table allocators")
> > introduced more dynamic structure for pdps, we ended up oopsing
> > when we populated the lrc context. Analyzing this oops revealed
> > the fact that we have not been writing valid pdps with bsw, as
> > it is doing the ppgtt init with 2GB limit in some cases.
> > 
> > We should do the right thing and setup the non addressable part
> > pdps/pde/pte to scratch page through the minimal structure by
> > having just pdp with pde entries pointing to same page with
> > pte entries pointing to scratch page.
> > 
> > But instead of going through that trouble, setup all the pdps
> > through individual pd pages and pt entries, even for non
> > addressable parts. And let the clear range point them to scratch
> > page. This way we populate the lrc with valid pdps and wait
> > for dynamic page allocation work to land, and do the heavy lifting
> > for truncating page table tree according to usage.
> > 
> > The regression of oopsing in init was introduced by
> > commit 06fda602dbca ("drm/i915: Create page table allocators")
> > 
> > v2: Clear the range for the unused part also (Ville)
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89350
> > Cc: Michel Thierry <michel.thierry@intel.com>
> > Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Tested-by: Valtteri Rantala <valtteri.rantala@intel.com>
> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> Seems sane enough assuming we're willing to pay the cost.
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> And then we could also throw another patch on top to actually increase the
> PPGTT size to 4GiB since there's no extra cost anymore :) Well, assuming
> everything else is 4GiB safe. But I guess it ought to be if BDW machines
> have already been running with 4GiB GGTT/PPGTT.

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx