On Wed, Aug 06, 2014 at 08:19:53PM +0200, Daniel Vetter wrote:
> Currently we abuse the aliasing ppgtt to set up the ppgtt support in
> general. Which is a bit backwards since with full ppgtt we don't ever
> need the aliasing ppgtt.
>
> So untangling this and separate the ppgtt init from the aliasing
> ppgtt. While at it drag it out of the context enabling (which just
> does a switch to the default context).
>
> Note that we still have the differentiation between synchronous and
> asynchronous ppgtt setup, but that will soon vanish. So also correctly
> wire up the return value handling to be prepared for when ->switch_mm
> drops the synchronous parameter and could start to fail.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Aside: This will conflict quite a bit with Alistars gpu reset rework to
ditch the synchronous parameter from ppgtt->switch_mm. If Alistars patch
is ready first I'll rebase and resend, otherwise I'll fix it up while
applying. It shouldn't be too hairy really.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 8 ++++
> drivers/gpu/drm/i915/i915_gem_context.c | 7 ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 84 +++++++++++++--------------------
> drivers/gpu/drm/i915/i915_gem_gtt.h | 1 +
> 4 files changed, 42 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index a79deb189146..caf92bac2152 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4742,6 +4742,14 @@ i915_gem_init_hw(struct drm_device *dev)
> if (ret && ret != -EIO) {
> DRM_ERROR("Context enable failed %d\n", ret);
> i915_gem_cleanup_ringbuffer(dev);
> +
> + return ret;
> + }
> +
> + ret = i915_ppgtt_init_hw(dev);
> + if (ret && ret != -EIO) {
> + DRM_ERROR("PPGTT enable failed %d\n", ret);
> + i915_gem_cleanup_ringbuffer(dev);
> }
>
> return ret;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 3b8367aa8404..4af5f05b24e8 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -424,13 +424,6 @@ int i915_gem_context_enable(struct drm_i915_private *dev_priv)
> struct intel_engine_cs *ring;
> int ret, i;
>
> - /* 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;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 4fa7807ed4d5..bf70ab44b968 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -78,7 +78,6 @@ static void ppgtt_bind_vma(struct i915_vma *vma,
> enum i915_cache_level cache_level,
> u32 flags);
> static void ppgtt_unbind_vma(struct i915_vma *vma);
> -static int gen8_ppgtt_enable(struct i915_hw_ppgtt *ppgtt);
>
> static inline gen8_gtt_pte_t gen8_pte_encode(dma_addr_t addr,
> enum i915_cache_level level,
> @@ -615,7 +614,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
> kunmap_atomic(pd_vaddr);
> }
>
> - ppgtt->enable = gen8_ppgtt_enable;
> ppgtt->switch_mm = gen8_mm_switch;
> ppgtt->base.clear_range = gen8_ppgtt_clear_range;
> ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
> @@ -836,39 +834,20 @@ static int gen6_mm_switch(struct i915_hw_ppgtt *ppgtt,
> return 0;
> }
>
> -static int gen8_ppgtt_enable(struct i915_hw_ppgtt *ppgtt)
> +static void gen8_ppgtt_enable(struct drm_device *dev)
> {
> - struct drm_device *dev = ppgtt->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_engine_cs *ring;
> - int j, ret;
> + int j;
>
> for_each_ring(ring, dev_priv, j) {
> I915_WRITE(RING_MODE_GEN7(ring),
> _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
> -
> - /* We promise to do a switch later with FULL PPGTT. If this is
> - * aliasing, this is the one and only switch we'll do */
> - if (USES_FULL_PPGTT(dev))
> - continue;
> -
> - ret = ppgtt->switch_mm(ppgtt, ring, true);
> - if (ret)
> - goto err_out;
> }
> -
> - return 0;
> -
> -err_out:
> - for_each_ring(ring, dev_priv, j)
> - I915_WRITE(RING_MODE_GEN7(ring),
> - _MASKED_BIT_DISABLE(GFX_PPGTT_ENABLE));
> - return ret;
> }
>
> -static int gen7_ppgtt_enable(struct i915_hw_ppgtt *ppgtt)
> +static void gen7_ppgtt_enable(struct drm_device *dev)
> {
> - struct drm_device *dev = ppgtt->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_engine_cs *ring;
> uint32_t ecochk, ecobits;
> @@ -887,31 +866,16 @@ static int gen7_ppgtt_enable(struct i915_hw_ppgtt *ppgtt)
> I915_WRITE(GAM_ECOCHK, ecochk);
>
> for_each_ring(ring, dev_priv, i) {
> - int ret;
> /* GFX_MODE is per-ring on gen7+ */
> I915_WRITE(RING_MODE_GEN7(ring),
> _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
> -
> - /* We promise to do a switch later with FULL PPGTT. If this is
> - * aliasing, this is the one and only switch we'll do */
> - if (USES_FULL_PPGTT(dev))
> - continue;
> -
> - ret = ppgtt->switch_mm(ppgtt, ring, true);
> - if (ret)
> - return ret;
> }
> -
> - return 0;
> }
>
> -static int gen6_ppgtt_enable(struct i915_hw_ppgtt *ppgtt)
> +static void gen6_ppgtt_enable(struct drm_device *dev)
> {
> - struct drm_device *dev = ppgtt->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_engine_cs *ring;
> uint32_t ecochk, gab_ctl, ecobits;
> - int i;
>
> ecobits = I915_READ(GAC_ECO_BITS);
> I915_WRITE(GAC_ECO_BITS, ecobits | ECOBITS_SNB_BIT |
> @@ -924,14 +888,6 @@ static int gen6_ppgtt_enable(struct i915_hw_ppgtt *ppgtt)
> I915_WRITE(GAM_ECOCHK, ecochk | ECOCHK_SNB_BIT | ECOCHK_PPGTT_CACHE64B);
>
> I915_WRITE(GFX_MODE, _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
> -
> - for_each_ring(ring, dev_priv, i) {
> - int ret = ppgtt->switch_mm(ppgtt, ring, true);
> - if (ret)
> - return ret;
> - }
> -
> - return 0;
> }
>
> /* PPGTT support for Sandybdrige/Gen6 and later */
> @@ -1151,13 +1107,10 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>
> ppgtt->base.pte_encode = dev_priv->gtt.base.pte_encode;
> if (IS_GEN6(dev)) {
> - ppgtt->enable = gen6_ppgtt_enable;
> ppgtt->switch_mm = gen6_mm_switch;
> } else if (IS_HASWELL(dev)) {
> - ppgtt->enable = gen7_ppgtt_enable;
> ppgtt->switch_mm = hsw_mm_switch;
> } else if (IS_GEN7(dev)) {
> - ppgtt->enable = gen7_ppgtt_enable;
> ppgtt->switch_mm = gen7_mm_switch;
> } else
> BUG();
> @@ -1222,6 +1175,35 @@ int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
> return ret;
> }
>
> +int i915_ppgtt_init_hw(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_engine_cs *ring;
> + struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
> + int i, ret = 0;
> +
> + if (!USES_PPGTT(dev))
> + return 0;
> +
> + if (IS_GEN6(dev))
> + gen6_ppgtt_enable(dev);
> + else if (IS_GEN7(dev))
> + gen7_ppgtt_enable(dev);
> + else if (INTEL_INFO(dev)->gen >= 8)
> + gen8_ppgtt_enable(dev);
> + else
> + WARN_ON(1);
> +
> + if (ppgtt) {
> + for_each_ring(ring, dev_priv, i) {
> + ret = ppgtt->switch_mm(ppgtt, ring, true);
> + if (ret != 0)
> + return ret;
> + }
> + }
> +
> + return ret;
> +}
> struct i915_hw_ppgtt *
> i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv)
> {
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index bea3541d5525..45aa15fa4af2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -277,6 +277,7 @@ int i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
> bool intel_enable_ppgtt(struct drm_device *dev, bool full);
>
> int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt);
> +int i915_ppgtt_init_hw(struct drm_device *dev);
> void i915_ppgtt_release(struct kref *kref);
> struct i915_hw_ppgtt *i915_ppgtt_create(struct drm_device *dev,
> struct drm_i915_file_private *fpriv);
> --
> 1.9.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
