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
