| Message ID | 1426768264-16996-19-git-send-email-John.C.Harrison@Intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19/03/2015 12:30, John.C.Harrison@Intel.com wrote: > From: John Harrison <John.C.Harrison@Intel.com> > > The i915_gem_init_hw() function calls a bunch of smaller initialisation > functions. Multiple of which have generic sections and per ring sections. This > means multiple passes are done over the rings. Each pass writes data to the ring > which floats around in that ring's OLR until some random point in the future > when an add_request() is done by some random other piece of code. > > This patch breaks i915_ppgtt_init_hw() in two with the per ring initialisation > now being done in i915_ppgtt_init_ring(). The ring looping is now done at the > top level in i915_gem_init_hw(). > > For: VIZ-5115 > Signed-off-by: John Harrison <John.C.Harrison@Intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 25 +++++++++++++++++++------ > drivers/gpu/drm/i915/i915_gem_gtt.c | 28 +++++++++++++++------------- > drivers/gpu/drm/i915/i915_gem_gtt.h | 1 + > 3 files changed, 35 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 8677293..683fc80 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4883,19 +4883,32 @@ i915_gem_init_hw(struct drm_device *dev) > */ > init_unused_rings(dev); > > + ret = i915_ppgtt_init_hw(dev); > + if (ret) { > + DRM_ERROR("PPGTT enable HW failed %d\n", ret); > + goto out; > + } > + > + /* Need to do basic initialisation of all rings first: */ > for_each_ring(ring, dev_priv, i) { > ret = ring->init_hw(ring); > if (ret) > goto out; > } > > - for (i = 0; i < NUM_L3_SLICES(dev); i++) > - i915_gem_l3_remap(&dev_priv->ring[RCS], i); > + /* Now it is safe to go back round and do everything else: */ > + for_each_ring(ring, dev_priv, i) { > + if (ring->id == RCS) { > + for (i = 0; i < NUM_L3_SLICES(dev); i++) > + i915_gem_l3_remap(ring, i); > + } > > - ret = i915_ppgtt_init_hw(dev); > - if (ret && ret != -EIO) { > - DRM_ERROR("PPGTT enable failed %d\n", ret); > - i915_gem_cleanup_ringbuffer(dev); > + ret = i915_ppgtt_init_ring(ring); > + if (ret && ret != -EIO) { > + DRM_ERROR("PPGTT enable ring #%d failed %d\n", i, ret); > + i915_gem_cleanup_ringbuffer(dev); > + goto out; > + } > } > > ret = i915_gem_context_enable(dev_priv); > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index d8ff1a8..83076d7 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -1280,11 +1280,6 @@ int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) > > 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; > - > /* In the case of execlists, PPGTT is enabled by the context descriptor > * and the PDPs are contained within the context itself. We don't > * need to do anything here. */ > @@ -1303,16 +1298,23 @@ int i915_ppgtt_init_hw(struct drm_device *dev) > else > MISSING_CASE(INTEL_INFO(dev)->gen); > > - if (ppgtt) { > - for_each_ring(ring, dev_priv, i) { > - ret = ppgtt->switch_mm(ppgtt, ring); > - if (ret != 0) > - return ret; > - } > - } > + return 0; > +} > > - return ret; > +int i915_ppgtt_init_ring(struct intel_engine_cs *ring) > +{ > + struct drm_i915_private *dev_priv = ring->dev->dev_private; > + struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt; > + > + if (i915.enable_execlists) > + return 0; > + > + if (!ppgtt) > + return 0; > + > + return ppgtt->switch_mm(ppgtt, ring); > } > + > 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 c9e93f5..2941fbb 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -311,6 +311,7 @@ void i915_global_gtt_cleanup(struct drm_device *dev); > > int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt); > int i915_ppgtt_init_hw(struct drm_device *dev); > +int i915_ppgtt_init_ring(struct intel_engine_cs *ring); > void i915_ppgtt_release(struct kref *kref); > struct i915_hw_ppgtt *i915_ppgtt_create(struct drm_device *dev, > struct drm_i915_file_private *fpriv); > Reviewed-by: Tomas Elf <tomas.elf@intel.com> Thanks, Tomas
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 8677293..683fc80 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4883,19 +4883,32 @@ i915_gem_init_hw(struct drm_device *dev) */ init_unused_rings(dev); + ret = i915_ppgtt_init_hw(dev); + if (ret) { + DRM_ERROR("PPGTT enable HW failed %d\n", ret); + goto out; + } + + /* Need to do basic initialisation of all rings first: */ for_each_ring(ring, dev_priv, i) { ret = ring->init_hw(ring); if (ret) goto out; } - for (i = 0; i < NUM_L3_SLICES(dev); i++) - i915_gem_l3_remap(&dev_priv->ring[RCS], i); + /* Now it is safe to go back round and do everything else: */ + for_each_ring(ring, dev_priv, i) { + if (ring->id == RCS) { + for (i = 0; i < NUM_L3_SLICES(dev); i++) + i915_gem_l3_remap(ring, i); + } - ret = i915_ppgtt_init_hw(dev); - if (ret && ret != -EIO) { - DRM_ERROR("PPGTT enable failed %d\n", ret); - i915_gem_cleanup_ringbuffer(dev); + ret = i915_ppgtt_init_ring(ring); + if (ret && ret != -EIO) { + DRM_ERROR("PPGTT enable ring #%d failed %d\n", i, ret); + i915_gem_cleanup_ringbuffer(dev); + goto out; + } } ret = i915_gem_context_enable(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index d8ff1a8..83076d7 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1280,11 +1280,6 @@ int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) 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; - /* In the case of execlists, PPGTT is enabled by the context descriptor * and the PDPs are contained within the context itself. We don't * need to do anything here. */ @@ -1303,16 +1298,23 @@ int i915_ppgtt_init_hw(struct drm_device *dev) else MISSING_CASE(INTEL_INFO(dev)->gen); - if (ppgtt) { - for_each_ring(ring, dev_priv, i) { - ret = ppgtt->switch_mm(ppgtt, ring); - if (ret != 0) - return ret; - } - } + return 0; +} - return ret; +int i915_ppgtt_init_ring(struct intel_engine_cs *ring) +{ + struct drm_i915_private *dev_priv = ring->dev->dev_private; + struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt; + + if (i915.enable_execlists) + return 0; + + if (!ppgtt) + return 0; + + return ppgtt->switch_mm(ppgtt, ring); } + 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 c9e93f5..2941fbb 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -311,6 +311,7 @@ void i915_global_gtt_cleanup(struct drm_device *dev); int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt); int i915_ppgtt_init_hw(struct drm_device *dev); +int i915_ppgtt_init_ring(struct intel_engine_cs *ring); void i915_ppgtt_release(struct kref *kref); struct i915_hw_ppgtt *i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv);