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

[Ben Widawsky <benjamin.widawsky@intel.com>]

On Tue, Mar 03, 2015 at 05:03:29PM +0200, Mika Kuoppala wrote:
> If the mappable size is less than what the full range
> of pdps can address, we end up setting pdps for only the
> mappable area.
> 
> The logical context however needs valid pdp entries.
> Prior to commit 06fda602dbca ("drm/i915: Create page table allocators")
> we just 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.
> 
> 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. This way we populate the lrc with valid
> pdps and gives us a base for dynamic page allocation to
> introduce code that truncates the page table structure.
> 
> The regression of oopsing in init was introduced by
> commit 06fda602dbca ("drm/i915: Create page table allocators")
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89350
> Tested-by: Valtteri Rantala <valtteri.rantala@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index bd95776..848a821 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -709,7 +709,7 @@ static int gen8_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt,
>   */
>  static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
>  {
> -	const int max_pdp = DIV_ROUND_UP(size, 1 << 30);
> +	const int max_pdp = GEN8_LEGACY_PDPES;
>  	const int min_pt_pages = GEN8_PDES_PER_PAGE * max_pdp;
>  	int i, j, ret;
>  

FWIW, I think I solved this later in my original series:
http://lists.freedesktop.org/archives/intel-gfx/2014-August/051162.html


-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx