Re: [PATCH 07/66] drm/i915: Use drm_mm for PPGTT PDEs

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Thu, 27 Jun 2013 16:30:08 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:

> When PPGTT support was originally enabled, it was only designed to
> support 1 PPGTT. It therefore made sense to simply hide the GGTT space
> required to enable this from the drm_mm allocator.
> 
> Since we intend to support full PPGTT, which means more than 1, and they
> can be created and destroyed ad hoc it will be required to use the
> proper allocation techniques we already have.
> 
> The first step here is to make the existing single PPGTT use the allocator.
> 
> v2: Align PDEs to 64b in GTT
> Allocate the node dynamically so we can use drm_mm_put_block
> Now tested on IGT
> Allocate node at the top to avoid fragmentation (Chris)
> 
> v3: Use Chris' top down allocator
> 
> v4: Embed drm_mm_node into ppgtt struct (Jesse)
> Remove hunks which didn't belong (Jesse)
> 
> v5: Don't subtract guard page since we now killed the guard page prior
> to this patch. (Ben)
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     |  1 +
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 45 ++++++++++++++++++++++++-------------
>  2 files changed, 31 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c677d6c..659b4aa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -484,6 +484,7 @@ struct i915_gtt {
>  #define gtt_total_entries(gtt) ((gtt).total >> PAGE_SHIFT)
>  
>  struct i915_hw_ppgtt {
> +	struct drm_mm_node node;
>  	struct drm_device *dev;
>  	unsigned num_pd_entries;
>  	struct page **pt_pages;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index fb30d65..5284dc5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -247,6 +247,8 @@ static void gen6_ppgtt_cleanup(struct i915_hw_ppgtt *ppgtt)
>  {
>  	int i;
>  
> +	drm_mm_remove_node(&ppgtt->node);
> +
>  	if (ppgtt->pt_dma_addr) {
>  		for (i = 0; i < ppgtt->num_pd_entries; i++)
>  			pci_unmap_page(ppgtt->dev->pdev,
> @@ -263,16 +265,27 @@ static void gen6_ppgtt_cleanup(struct i915_hw_ppgtt *ppgtt)
>  
>  static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>  {
> +#define GEN6_PD_ALIGN (PAGE_SIZE * 16)
> +#define GEN6_PD_SIZE (GEN6_PPGTT_PD_ENTRIES * PAGE_SIZE)
>  	struct drm_device *dev = ppgtt->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	unsigned first_pd_entry_in_global_pt;
>  	int i;
>  	int ret = -ENOMEM;
>  
> -	/* ppgtt PDEs reside in the global gtt pagetable, which has 512*1024
> -	 * entries. For aliasing ppgtt support we just steal them at the end for
> -	 * now. */
> -	first_pd_entry_in_global_pt = gtt_total_entries(dev_priv->gtt);
> +	/* PPGTT PDEs reside in the GGTT stolen space, and consists of 512
> +	 * entries. The allocator works in address space sizes, so it's
> +	 * multiplied by page size. We allocate at the top of the GTT to avoid
> +	 * fragmentation.
> +	 */
> +	BUG_ON(!drm_mm_initialized(&dev_priv->mm.gtt_space));
> +	ret = drm_mm_insert_node_in_range_generic(&dev_priv->mm.gtt_space,
> +						  &ppgtt->node, GEN6_PD_SIZE,
> +						  GEN6_PD_ALIGN, 0,
> +						  dev_priv->gtt.mappable_end,
> +						  dev_priv->gtt.total,
> +						  DRM_MM_TOPDOWN);
> +	if (ret)
> +		return ret;
>  
>  	if (IS_HASWELL(dev)) {
>  		ppgtt->pte_encode = hsw_pte_encode;
> @@ -288,8 +301,10 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>  	ppgtt->cleanup = gen6_ppgtt_cleanup;
>  	ppgtt->pt_pages = kzalloc(sizeof(struct page *)*ppgtt->num_pd_entries,
>  				  GFP_KERNEL);
> -	if (!ppgtt->pt_pages)
> +	if (!ppgtt->pt_pages) {
> +		drm_mm_remove_node(&ppgtt->node);
>  		return -ENOMEM;
> +	}
>  
>  	for (i = 0; i < ppgtt->num_pd_entries; i++) {
>  		ppgtt->pt_pages[i] = alloc_page(GFP_KERNEL);
> @@ -319,7 +334,11 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>  	ppgtt->clear_range(ppgtt, 0,
>  			   ppgtt->num_pd_entries*I915_PPGTT_PT_ENTRIES);
>  
> -	ppgtt->pd_offset = first_pd_entry_in_global_pt * sizeof(gen6_gtt_pte_t);
> +	DRM_DEBUG_DRIVER("Allocated pde space (%ldM) at GTT entry: %lx\n",
> +			 ppgtt->node.size >> 20,
> +			 ppgtt->node.start / PAGE_SIZE);
> +	ppgtt->pd_offset =
> +		ppgtt->node.start / PAGE_SIZE * sizeof(gen6_gtt_pte_t);
>  
>  	return 0;
>  
> @@ -336,6 +355,7 @@ err_pt_alloc:
>  			__free_page(ppgtt->pt_pages[i]);
>  	}
>  	kfree(ppgtt->pt_pages);
> +	drm_mm_remove_node(&ppgtt->node);
>  
>  	return ret;
>  }
> @@ -442,6 +462,9 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  	dev_priv->gtt.gtt_clear_range(dev, dev_priv->gtt.start / PAGE_SIZE,
>  				      dev_priv->gtt.total / PAGE_SIZE);
>  
> +	if (dev_priv->mm.aliasing_ppgtt)
> +		gen6_write_pdes(dev_priv->mm.aliasing_ppgtt);
> +
>  	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
>  		i915_gem_clflush_object(obj);
>  		i915_gem_gtt_bind_object(obj, obj->cache_level);
> @@ -711,21 +734,13 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
>  	if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
>  		int ret;
>  
> -		if (INTEL_INFO(dev)->gen <= 7) {
> -			/* PPGTT pdes are stolen from global gtt ptes, so shrink the
> -			 * aperture accordingly when using aliasing ppgtt. */
> -			gtt_size -= GEN6_PPGTT_PD_ENTRIES * PAGE_SIZE;
> -		}
> -
>  		i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size, 0);
> -
>  		ret = i915_gem_init_aliasing_ppgtt(dev);
>  		if (!ret)
>  			return;
>  
>  		DRM_ERROR("Aliased PPGTT setup failed %d\n", ret);
>  		drm_mm_takedown(&dev_priv->mm.gtt_space);
> -		gtt_size += GEN6_PPGTT_PD_ENTRIES * PAGE_SIZE;
>  	}
>  	i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size, PAGE_SIZE);
>  }

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center