> -----Original Message-----
> From: Intel-gfx [mailto:[email protected]] On Behalf
> Of Daniel Vetter
> Sent: Wednesday, August 06, 2014 7:20 PM
> To: Intel Graphics Development
> Cc: Daniel Vetter
> Subject: [Intel-gfx] [PATCH 1/2] drm/i915: Rework ppgtt init to no require
an
> aliasing ppgtt
> 
> 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 <[email protected]>
> ---
>  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

Reviewed-by: Michel Thierry <[email protected]>

> 
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Attachment: smime.p7s
Description: S/MIME cryptographic signature

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to