| Message ID | 1366784140-2670-13-git-send-email-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 23, 2013 at 11:15:40PM -0700, Ben Widawsky wrote: > Going forward, every context will have its own address space. So rip off > the band aid now. If contexts fail, don't do ppgtt, and vice versa. > > Similarly to the last patch, this is somewhat wasteful of PPGTT address > space with contexts, since we're not actually utilizing the new PPGTT. What strategy do we have for isolating bugs between ctx and ppggt? I'd like to be able disable each of them independently and together. -Chris
On Wed, Apr 24, 2013 at 11:06:12AM +0100, Chris Wilson wrote: > On Tue, Apr 23, 2013 at 11:15:40PM -0700, Ben Widawsky wrote: > > Going forward, every context will have its own address space. So rip off > > the band aid now. If contexts fail, don't do ppgtt, and vice versa. > > > > Similarly to the last patch, this is somewhat wasteful of PPGTT address > > space with contexts, since we're not actually utilizing the new PPGTT. > > What strategy do we have for isolating bugs between ctx and ppggt? I'd > like to be able disable each of them independently and together. > -Chris None. Up through the end of this series we have the ability to turn off: PPGTT or Contexts & PPGTT. Given our history, I think that probably concerns you more than PPGTT. I don't know how to make you feel better.AFAIK, Mesa plans to become dependent on contexts anyway, so having a way of backing out of that, even for debug, seems to be of limited use to me. > > -- > Chris Wilson, Intel Open Source Technology Centre
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index cb9fcfb..53192e7 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4057,7 +4057,7 @@ i915_gem_init_hw(struct drm_device *dev) int i915_gem_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - int ret = -ENOMEM; + int ret = -ENODEV; mutex_lock(&dev->struct_mutex); @@ -4069,8 +4069,6 @@ int i915_gem_init(struct drm_device *dev) } if (intel_enable_ppgtt(dev) && HAS_ALIASING_PPGTT(dev)) { - struct i915_hw_ppgtt *ppgtt; - i915_gem_setup_global_gtt(dev, 0, dev_priv->gtt.mappable_end, dev_priv->gtt.total, false); i915_gem_context_init(dev); @@ -4079,15 +4077,9 @@ int i915_gem_init(struct drm_device *dev) goto ggtt_only; } - ppgtt = &dev_priv->ring[RCS].default_context->ppgtt; - - ret = i915_gem_ppgtt_init(dev, ppgtt); - if (ret) { - drm_mm_takedown(&dev_priv->mm.gtt_space); - goto ggtt_only; - } - - dev_priv->mm.aliasing_ppgtt = ppgtt; + dev_priv->mm.aliasing_ppgtt = + &dev_priv->ring[RCS].default_context->ppgtt; + ret = 0; } ggtt_only: diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index b2deddd..64f8009 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -129,6 +129,8 @@ static void do_destroy(struct i915_hw_context *ctx) if (ctx->file_priv) idr_remove(&ctx->file_priv->context_idr, ctx->id); + ctx->ppgtt.cleanup(&ctx->ppgtt); + drm_gem_object_unreference(&ctx->obj->base); kfree(ctx); } @@ -159,6 +161,10 @@ create_hw_context(struct drm_device *dev, goto err_out; } + ret = i915_gem_ppgtt_init(dev, &ctx->ppgtt); + if (ret) + goto err_out; + /* The ring associated with the context object is handled by the normal * object tracking code. We give an initial ring value simple to pass an * assertion in the context switch code.
Going forward, every context will have its own address space. So rip off the band aid now. If contexts fail, don't do ppgtt, and vice versa. Similarly to the last patch, this is somewhat wasteful of PPGTT address space with contexts, since we're not actually utilizing the new PPGTT. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_gem.c | 16 ++++------------ drivers/gpu/drm/i915/i915_gem_context.c | 6 ++++++ 2 files changed, 10 insertions(+), 12 deletions(-)