Re: [PATCH 4/6] drm/i915: Move even more gtt code to i915_gem_gtt

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Ben Widawsky <benjamin.widawsky@intel.com> writes:

> This really should have been part of the kill agp series.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     |  7 ++---
>  drivers/gpu/drm/i915/i915_gem.c     | 51 ++------------------------------
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 59 ++++++++++++++++++++++++++++++++++---
>  3 files changed, 61 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2ab476d..75003c3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1585,10 +1585,9 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
>  				enum i915_cache_level cache_level);
>  void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj);
>  void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
> -void i915_gem_init_global_gtt(struct drm_device *dev,
> -			      unsigned long start,
> -			      unsigned long mappable_end,
> -			      unsigned long end);
> +void i915_gem_init_global_gtt(struct drm_device *dev);
> +void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
> +			       unsigned long mappable_end, unsigned long end);
>  int i915_gem_gtt_init(struct drm_device *dev);
>  void i915_gem_gtt_fini(struct drm_device *dev);
>  static inline void i915_gem_chipset_flush(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 4c6f043..42b948b 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -163,8 +163,8 @@ i915_gem_init_ioctl(struct drm_device *dev, void *data,
>  		return -ENODEV;
>  
>  	mutex_lock(&dev->struct_mutex);
> -	i915_gem_init_global_gtt(dev, args->gtt_start,
> -				 args->gtt_end, args->gtt_end);
> +	i915_gem_setup_global_gtt(dev, args->gtt_start, args->gtt_end,
> +				  args->gtt_end);
>  	mutex_unlock(&dev->struct_mutex);
>  
>  	return 0;
> @@ -3959,58 +3959,13 @@ cleanup_render_ring:
>  	return ret;
>  }
>  
> -static bool
> -intel_enable_ppgtt(struct drm_device *dev)
> -{
> -	if (i915_enable_ppgtt >= 0)
> -		return i915_enable_ppgtt;
> -
> -#ifdef CONFIG_INTEL_IOMMU
> -	/* Disable ppgtt on SNB if VT-d is on. */
> -	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
> -		return false;
> -#endif
> -
> -	return true;
> -}
> -
>  int i915_gem_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	unsigned long gtt_size, mappable_size;
>  	int ret;
>  
> -	gtt_size = dev_priv->mm.gtt->gtt_total_entries << PAGE_SHIFT;
> -	mappable_size = dev_priv->mm.gtt->gtt_mappable_entries << PAGE_SHIFT;
> -
>  	mutex_lock(&dev->struct_mutex);
> -	if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
> -		/* PPGTT pdes are stolen from global gtt ptes, so shrink the
> -		 * aperture accordingly when using aliasing ppgtt. */
> -		gtt_size -= I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
> -
> -		i915_gem_init_global_gtt(dev, 0, mappable_size, gtt_size);
> -
> -		ret = i915_gem_init_aliasing_ppgtt(dev);
> -		if (ret) {
> -			mutex_unlock(&dev->struct_mutex);
> -			return ret;
> -		}
> -	} else {
> -		/* Let GEM Manage all of the aperture.
> -		 *
> -		 * However, leave one page at the end still bound to the scratch
> -		 * page.  There are a number of places where the hardware
> -		 * apparently prefetches past the end of the object, and we've
> -		 * seen multiple hangs with the GPU head pointer stuck in a
> -		 * batchbuffer bound at the last page of the aperture.  One page
> -		 * should be enough to keep any prefetching inside of the
> -		 * aperture.
> -		 */
> -		i915_gem_init_global_gtt(dev, 0, mappable_size,
> -					 gtt_size);
> -	}
> -
> +	i915_gem_init_global_gtt(dev);
>  	ret = i915_gem_init_hw(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  	if (ret) {
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index b699a04..fc3c08a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -525,10 +525,10 @@ static void i915_gtt_color_adjust(struct drm_mm_node *node,
>  	}
>  }
>  
> -void i915_gem_init_global_gtt(struct drm_device *dev,
> -			      unsigned long start,
> -			      unsigned long mappable_end,
> -			      unsigned long end)
> +void i915_gem_setup_global_gtt(struct drm_device *dev,
> +			       unsigned long start,
> +			       unsigned long mappable_end,
> +			       unsigned long end)
>  {
>  	drm_i915_private_t *dev_priv = dev->dev_private;
>  	struct drm_mm_node *entry;
> @@ -573,6 +573,57 @@ void i915_gem_init_global_gtt(struct drm_device *dev,
>  	i915_ggtt_clear_range(dev, end / PAGE_SIZE - 1, 1);
>  }
>  
> +static bool
> +intel_enable_ppgtt(struct drm_device *dev)
> +{
> +	if (i915_enable_ppgtt >= 0)
> +		return i915_enable_ppgtt;
> +
> +#ifdef CONFIG_INTEL_IOMMU
> +	/* Disable ppgtt on SNB if VT-d is on. */
> +	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
> +		return false;
> +#endif
> +
> +	return true;
> +}
> +
> +void i915_gem_init_global_gtt(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	unsigned long gtt_size, mappable_size;
> +	int ret;
> +
> +	gtt_size = dev_priv->mm.gtt->gtt_total_entries << PAGE_SHIFT;
> +	mappable_size = dev_priv->mm.gtt->gtt_mappable_entries << PAGE_SHIFT;
> +
> +	if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) {
> +		/* PPGTT pdes are stolen from global gtt ptes, so shrink the
> +		 * aperture accordingly when using aliasing ppgtt. */
> +		gtt_size -= I915_PPGTT_PD_ENTRIES*PAGE_SIZE;
> +
> +		i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
> +
> +		ret = i915_gem_init_aliasing_ppgtt(dev);
> +		if (ret) {
> +			mutex_unlock(&dev->struct_mutex);
> +			return;
> +		}
> +	} else {
> +		/* Let GEM Manage all of the aperture.
> +		 *
> +		 * However, leave one page at the end still bound to the scratch
> +		 * page.  There are a number of places where the hardware
> +		 * apparently prefetches past the end of the object, and we've
> +		 * seen multiple hangs with the GPU head pointer stuck in a
> +		 * batchbuffer bound at the last page of the aperture.  One page
> +		 * should be enough to keep any prefetching inside of the
> +		 * aperture.
> +		 */
> +		i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
> +	}
> +}
> +
>  static int setup_scratch_page(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -- 
> 1.8.0.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx