From bdf4fd7ea0765966c920f62a360532e3929177ca Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Fri, 6 Dec 2013 14:11:18 -0800 Subject: [PATCH] drm/i915: Do aliasing PPGTT init with contexts We have a default context which suits the aliasing PPGTT well. Tie them together so it looks like any other context/PPGTT pair. This makes the code cleaner as it won't have to special case aliasing as often. The patch has one slightly tricky part in the default context creation function. In the future (and on aliased setup) we create a new VM for a context (potentially). However, if we have aliasing PPGTT, which occurs at this point in time for all platforms GEN6+, we can simply manage the refcounting to allow things to behave as normal. Now is a good time to recall that the aliasing_ppgtt doesn't have a real VM, it uses the GGTT drm_mm. Signed-off-by: Ben Widawsky Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_dma.c | 4 +- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/i915_gem.c | 13 +-- drivers/gpu/drm/i915/i915_gem_context.c | 100 ++++++++++++++++++++---- drivers/gpu/drm/i915/i915_gem_gtt.c | 32 -------- 5 files changed, 87 insertions(+), 63 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index a5d010c86368..0817dd1bd2ff 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1364,7 +1364,7 @@ cleanup_gem: i915_gem_cleanup_ringbuffer(dev); i915_gem_context_fini(dev); mutex_unlock(&dev->struct_mutex); - i915_gem_cleanup_aliasing_ppgtt(dev); + WARN_ON(dev_priv->mm.aliasing_ppgtt); drm_mm_takedown(&dev_priv->gtt.base.mm); cleanup_power: intel_display_power_put(dev, POWER_DOMAIN_VGA); @@ -1765,8 +1765,8 @@ int i915_driver_unload(struct drm_device *dev) i915_gem_free_all_phys_object(dev); i915_gem_cleanup_ringbuffer(dev); i915_gem_context_fini(dev); + WARN_ON(dev_priv->mm.aliasing_ppgtt); mutex_unlock(&dev->struct_mutex); - i915_gem_cleanup_aliasing_ppgtt(dev); i915_gem_cleanup_stolen(dev); if (!I915_NEED_GFX_HWS(dev)) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a47a43e8e8e9..6dcfa1892c6f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2260,7 +2260,6 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file *file); /* i915_gem_gtt.c */ -void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev); void i915_check_and_clear_faults(struct drm_device *dev); void i915_gem_suspend_gtt_mappings(struct drm_device *dev); void i915_gem_restore_gtt_mappings(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index cc1ac797fba5..427596b8d66a 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4404,7 +4404,6 @@ int i915_gem_init_hw(struct drm_device *dev) { drm_i915_private_t *dev_priv = dev->dev_private; - struct i915_hw_ppgtt *ppgtt; int ret, i; if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt()) @@ -4446,16 +4445,6 @@ i915_gem_init_hw(struct drm_device *dev) goto err_out; } - if (dev_priv->mm.aliasing_ppgtt) { - ppgtt = dev_priv->mm.aliasing_ppgtt; - ret = ppgtt->enable(ppgtt); - if (ret) { - i915_gem_cleanup_aliasing_ppgtt(dev); - DRM_INFO("PPGTT enable failed. This is not fatal, but unexpected\n"); - ret = 0; - } - } - return 0; err_out: @@ -4486,8 +4475,8 @@ int i915_gem_init(struct drm_device *dev) ret = i915_gem_init_hw(dev); mutex_unlock(&dev->struct_mutex); if (ret) { + WARN_ON(dev_priv->mm.aliasing_ppgtt); i915_gem_context_fini(dev); - i915_gem_cleanup_aliasing_ppgtt(dev); drm_mm_takedown(&dev_priv->gtt.base.mm); return ret; } diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 0b32bcff5d66..215a36d38b6d 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -157,6 +157,25 @@ void i915_gem_context_free(struct kref *ctx_ref) kfree(ctx); } +static struct i915_hw_ppgtt * +create_vm_for_ctx(struct drm_device *dev, struct i915_hw_context *ctx) +{ + struct i915_hw_ppgtt *ppgtt; + int ret; + + ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL); + if (!ppgtt) + return ERR_PTR(-ENOMEM); + + ret = i915_gem_init_ppgtt(dev, ppgtt); + if (ret) { + kfree(ppgtt); + return ERR_PTR(ret); + } + + return ppgtt; +} + static struct i915_hw_context * create_hw_context(struct drm_device *dev, struct drm_i915_file_private *file_priv) @@ -223,31 +242,70 @@ static inline bool is_default_context(struct i915_hw_context *ctx) * well as an idle case. */ static struct i915_hw_context * -create_default_context(struct drm_device *dev) +create_default_context(struct drm_device *dev, + struct drm_i915_file_private *file_priv, + bool create_vm) { + struct drm_i915_private *dev_priv = dev->dev_private; struct i915_hw_context *ctx; - int ret; + int ret = 0; BUG_ON(!mutex_is_locked(&dev->struct_mutex)); - ctx = create_hw_context(dev, NULL); + /* Not yet supported */ + BUG_ON(file_priv); + + ctx = create_hw_context(dev, file_priv); if (IS_ERR(ctx)) return ctx; - /* We may need to do things with the shrinker which require us to - * immediately switch back to the default context. This can cause a - * problem as pinning the default context also requires GTT space which - * may not be available. To avoid this we always pin the - * default context. - */ - ret = i915_gem_obj_ggtt_pin(ctx->obj, get_context_alignment(dev), - false, false); - if (ret) { - DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret); - goto err_destroy; + if (create_vm) { + struct i915_hw_ppgtt *ppgtt = create_vm_for_ctx(dev, ctx); + + if (IS_ERR_OR_NULL(ppgtt)) { + DRM_ERROR("PPGTT setup failed (%ld)\n", PTR_ERR(ppgtt)); + ret = PTR_ERR(ppgtt); + goto err_destroy; + } else + ctx->vm = &ppgtt->base; + + /* This case is reserved for the global default context and + * should only happen once. */ + if (!file_priv) { + if (WARN_ON(dev_priv->mm.aliasing_ppgtt)) { + ret = -EEXIST; + goto err_destroy; + } + + dev_priv->mm.aliasing_ppgtt = ppgtt; + + /* We may need to do things with the shrinker which + * require us to immediately switch back to the default + * context. This can cause a problem as pinning the + * default context also requires GTT space which may not + * be available. To avoid this we always pin the default + * context. + */ + ret = i915_gem_obj_ggtt_pin(ctx->obj, + get_context_alignment(dev), + false, false); + if (ret) { + DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret); + goto err_destroy; + } + } + } else if (USES_ALIASING_PPGTT(dev)) { + /* For platforms which only have aliasing PPGTT, we fake the + * address space and refcounting. */ + kref_get(&dev_priv->mm.aliasing_ppgtt->ref); } - DRM_DEBUG_DRIVER("Default HW context loaded\n"); + /* TODO: Until full ppgtt... */ + if (USES_ALIASING_PPGTT(dev)) + ctx->vm = &dev_priv->mm.aliasing_ppgtt->base; + else + ctx->vm = &dev_priv->gtt.base; + return ctx; err_destroy: @@ -319,8 +377,9 @@ int i915_gem_context_init(struct drm_device *dev) return -E2BIG; } + dev_priv->ring[RCS].default_context = + create_default_context(dev, NULL, USES_ALIASING_PPGTT(dev)); - dev_priv->ring[RCS].default_context = create_default_context(dev); if (IS_ERR_OR_NULL(dev_priv->ring[RCS].default_context)) { DRM_DEBUG_DRIVER("Disabling HW Contexts; create failed %ld\n", PTR_ERR(dev_priv->ring[RCS].default_context)); @@ -384,6 +443,7 @@ void i915_gem_context_fini(struct drm_device *dev) i915_gem_object_ggtt_unpin(dctx->obj); i915_gem_context_unreference(dctx); + dev_priv->mm.aliasing_ppgtt = NULL; } int i915_gem_context_enable(struct drm_i915_private *dev_priv) @@ -394,11 +454,19 @@ int i915_gem_context_enable(struct drm_i915_private *dev_priv) if (!HAS_HW_CONTEXTS(dev_priv->dev)) return 0; + /* This is the only place the aliasing PPGTT gets enabled, which means + * it has to happen before we bail on reset */ + if (dev_priv->mm.aliasing_ppgtt) { + struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt; + ppgtt->enable(ppgtt); + } + /* FIXME: We should make this work, even in reset */ if (i915_reset_in_progress(&dev_priv->gpu_error)) return 0; BUG_ON(!dev_priv->ring[RCS].default_context); + for_each_ring(ring, dev_priv, i) { ret = do_switch(ring, ring->default_context); if (ret) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 5e2efcac76d0..a6211e079010 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -909,19 +909,6 @@ int i915_gem_init_ppgtt(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) return ret; } -void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt; - - if (!ppgtt) - return; - - kref_put(&dev_priv->mm.aliasing_ppgtt->ref, ppgtt_release); - - dev_priv->mm.aliasing_ppgtt = NULL; -} - static void __always_unused ppgtt_bind_vma(struct i915_vma *vma, enum i915_cache_level cache_level, @@ -1422,25 +1409,6 @@ void i915_gem_init_global_gtt(struct drm_device *dev) mappable_size = dev_priv->gtt.mappable_end; i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size); - if (USES_ALIASING_PPGTT(dev)) { - struct i915_hw_ppgtt *ppgtt; - int ret; - - ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL); - if (!ppgtt) { - DRM_ERROR("Aliased PPGTT setup failed -ENOMEM\n"); - return; - } - - ret = i915_gem_init_ppgtt(dev, ppgtt); - if (!ret) { - dev_priv->mm.aliasing_ppgtt = ppgtt; - return; - } - - kfree(ppgtt); - DRM_ERROR("Aliased PPGTT setup failed %d\n", ret); - } } static int setup_scratch_page(struct drm_device *dev) -- 2.20.1