> -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf > Of Daniel Vetter > Sent: Wednesday, August 06, 2014 7:20 PM > To: Intel Graphics Development > Cc: Daniel Vetter > Subject: [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 <daniel.vetter@xxxxxxxx> > --- > 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 <michel.thierry@xxxxxxxxx> > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
