[Intel-gfx] [PATCH 24/66] drm/i915: Move aliasing_ppgtt

Daniel Vetter daniel at ffwll.ch
Sun Jun 30 15:27:44 CEST 2013


On Thu, Jun 27, 2013 at 04:30:25PM -0700, Ben Widawsky wrote:
> for file in `ls drivers/gpu/drm/i915/*.c` ; do
> 	sed -i "s/mm.aliasing/gtt.aliasing/" $file;
> done

Commit message should explain _why_ we do something. Again I'm asking
since I'm unclear about how things fit all together and what the different
responsibilities are. I think I understand your design here to make both
real ppgtt work and keep aliasing ppgtt going, but I'd like you to explain
this in your words ;-)

One thing which looks a bit peculiar at the end is that struct
i915_hw_ppgtt is actually used as the real ppgtt object (since it
subclases i915_address space). My original plan was that we'll add a new
struct i915_ppgtt {
	struct i915_address_space base;
	struct i915_hw_ppgtt hw_ppgtt;
}

To fit into your design the alising ppgtt pointer in dev_priv->gtt would
then only point at a hw_ppgtt struct, not the full deal with address space
and everything else around attached.

Cheers, Daniel

> 
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c        |  4 ++--
>  drivers/gpu/drm/i915/i915_dma.c            |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h            |  6 +++---
>  drivers/gpu/drm/i915/i915_gem.c            | 12 ++++++------
>  drivers/gpu/drm/i915/i915_gem_context.c    |  4 ++--
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 ++--
>  drivers/gpu/drm/i915/i915_gem_gtt.c        |  6 +++---
>  7 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index c10a690..f3c76ab 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1816,8 +1816,8 @@ static int i915_ppgtt_info(struct seq_file *m, void *data)
>  		seq_printf(m, "PP_DIR_BASE_READ: 0x%08x\n", I915_READ(RING_PP_DIR_BASE_READ(ring)));
>  		seq_printf(m, "PP_DIR_DCLV: 0x%08x\n", I915_READ(RING_PP_DIR_DCLV(ring)));
>  	}
> -	if (dev_priv->mm.aliasing_ppgtt) {
> -		struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
> +	if (dev_priv->gtt.aliasing_ppgtt) {
> +		struct i915_hw_ppgtt *ppgtt = dev_priv->gtt.aliasing_ppgtt;
>  
>  		seq_printf(m, "aliasing PPGTT:\n");
>  		seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd_offset);
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 3535ced..ef00847 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -977,7 +977,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  		value = HAS_LLC(dev);
>  		break;
>  	case I915_PARAM_HAS_ALIASING_PPGTT:
> -		if (intel_enable_ppgtt(dev) && dev_priv->mm.aliasing_ppgtt)
> +		if (intel_enable_ppgtt(dev) && dev_priv->gtt.aliasing_ppgtt)
>  			value = 1;
>  		break;
>  	case I915_PARAM_HAS_WAIT_TIMEOUT:
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7016074..0fa7a21 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -482,6 +482,9 @@ struct i915_gtt {
>  	struct io_mapping *mappable;	/* Mapping to our CPU mappable region */
>  	phys_addr_t mappable_base;	/* PA of our GMADR */
>  
> +	/** PPGTT used for aliasing the PPGTT with the GTT */
> +	struct i915_hw_ppgtt *aliasing_ppgtt;
> +
>  	/** "Graphics Stolen Memory" holds the global PTEs */
>  	void __iomem *gsm;
>  
> @@ -843,9 +846,6 @@ struct i915_gem_mm {
>  	 */
>  	struct list_head unbound_list;
>  
> -	/** PPGTT used for aliasing the PPGTT with the GTT */
> -	struct i915_hw_ppgtt *aliasing_ppgtt;
> -
>  	struct shrinker inactive_shrinker;
>  	bool shrinker_no_lock_stealing;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e31ed47..eb78c5b 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2620,7 +2620,7 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj)
>  	if (obj->has_global_gtt_mapping)
>  		i915_gem_gtt_unbind_object(obj);
>  	if (obj->has_aliasing_ppgtt_mapping) {
> -		i915_ppgtt_unbind_object(dev_priv->mm.aliasing_ppgtt, obj);
> +		i915_ppgtt_unbind_object(dev_priv->gtt.aliasing_ppgtt, obj);
>  		obj->has_aliasing_ppgtt_mapping = 0;
>  	}
>  	i915_gem_gtt_finish_object(obj);
> @@ -3359,7 +3359,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
>  		if (obj->has_global_gtt_mapping)
>  			i915_gem_gtt_bind_object(obj, cache_level);
>  		if (obj->has_aliasing_ppgtt_mapping)
> -			i915_ppgtt_bind_object(dev_priv->mm.aliasing_ppgtt,
> +			i915_ppgtt_bind_object(dev_priv->gtt.aliasing_ppgtt,
>  					       obj, cache_level);
>  
>  		obj->gtt_space->color = cache_level;
> @@ -3668,7 +3668,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
>  		if (ret)
>  			return ret;
>  
> -		if (!dev_priv->mm.aliasing_ppgtt)
> +		if (!dev_priv->gtt.aliasing_ppgtt)
>  			i915_gem_gtt_bind_object(obj, obj->cache_level);
>  	}
>  
> @@ -4191,10 +4191,10 @@ i915_gem_init_hw(struct drm_device *dev)
>  	 * the do_switch), but before enabling PPGTT. So don't move this.
>  	 */
>  	ret = i915_gem_context_enable(dev_priv);
> -	if (ret || !dev_priv->mm.aliasing_ppgtt)
> +	if (ret || !dev_priv->gtt.aliasing_ppgtt)
>  		goto disable_ctx_out;
>  
> -	ret = dev_priv->mm.aliasing_ppgtt->enable(dev);
> +	ret = dev_priv->gtt.aliasing_ppgtt->enable(dev);
>  	if (ret)
>  		goto disable_ctx_out;
>  
> @@ -4236,7 +4236,7 @@ int i915_gem_init(struct drm_device *dev)
>  		dev_priv->hw_contexts_disabled = true;
>  
>  ggtt_only:
> -	if (!dev_priv->mm.aliasing_ppgtt) {
> +	if (!dev_priv->gtt.aliasing_ppgtt) {
>  		if (HAS_HW_CONTEXTS(dev))
>  			DRM_DEBUG_DRIVER("Context setup failed %d\n", ret);
>  		i915_gem_setup_global_gtt(dev, 0, dev_priv->gtt.mappable_end,
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index d92f121..aa4fc4a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -226,7 +226,7 @@ static int create_default_context(struct drm_i915_private *dev_priv)
>  	}
>  
>  	dev_priv->ring[RCS].default_context = ctx;
> -	dev_priv->mm.aliasing_ppgtt = &ctx->ppgtt;
> +	dev_priv->gtt.aliasing_ppgtt = &ctx->ppgtt;
>  
>  	DRM_DEBUG_DRIVER("Default HW context loaded\n");
>  	return 0;
> @@ -300,7 +300,7 @@ void i915_gem_context_fini(struct drm_device *dev)
>  	i915_gem_context_unreference(dctx);
>  	dev_priv->ring[RCS].default_context = NULL;
>  	dev_priv->ring[RCS].last_context = NULL;
> -	dev_priv->mm.aliasing_ppgtt = NULL;
> +	dev_priv->gtt.aliasing_ppgtt = NULL;
>  }
>  
>  int i915_gem_context_enable(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 7fcd6c0..93870bb 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -429,8 +429,8 @@ i915_gem_execbuffer_reserve_object(struct drm_i915_gem_object *obj,
>  	}
>  
>  	/* Ensure ppgtt mapping exists if needed */
> -	if (dev_priv->mm.aliasing_ppgtt && !obj->has_aliasing_ppgtt_mapping) {
> -		i915_ppgtt_bind_object(dev_priv->mm.aliasing_ppgtt,
> +	if (dev_priv->gtt.aliasing_ppgtt && !obj->has_aliasing_ppgtt_mapping) {
> +		i915_ppgtt_bind_object(dev_priv->gtt.aliasing_ppgtt,
>  				       obj, obj->cache_level);
>  
>  		obj->has_aliasing_ppgtt_mapping = 1;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 6de75c7..18820cb 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -127,7 +127,7 @@ static int gen6_ppgtt_enable(struct drm_device *dev)
>  	drm_i915_private_t *dev_priv = dev->dev_private;
>  	uint32_t pd_offset;
>  	struct intel_ring_buffer *ring;
> -	struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
> +	struct i915_hw_ppgtt *ppgtt = dev_priv->gtt.aliasing_ppgtt;
>  	int i;
>  
>  	BUG_ON(ppgtt->pd_offset & 0x3f);
> @@ -445,8 +445,8 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  				       i915_gtt_vm->start / PAGE_SIZE,
>  				       i915_gtt_vm->total / PAGE_SIZE);
>  
> -	if (dev_priv->mm.aliasing_ppgtt)
> -		gen6_write_pdes(dev_priv->mm.aliasing_ppgtt);
> +	if (dev_priv->gtt.aliasing_ppgtt)
> +		gen6_write_pdes(dev_priv->gtt.aliasing_ppgtt);
>  
>  	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
>  		i915_gem_clflush_object(obj);
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch



More information about the Intel-gfx mailing list