Yes. Thanks! Seems there is an assumption if ctx->ppgtt == NULL then go back to use aliasing ppgtt. I see. I will drop that patch, without that patch we could also work -----Original Message----- From: Chris Wilson [mailto:chris@xxxxxxxxxxxxxxxxxx] Sent: Monday, May 16, 2016 8:13 AM To: Wang, Zhi A <zhi.a.wang@xxxxxxxxx> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Gordon, David S <david.s.gordon@xxxxxxxxx>; joonas.lahtinen@xxxxxxxxxxxxxxx; Tian, Kevin <kevin.tian@xxxxxxxxx>; Lv, Zhiyuan <zhiyuan.lv@xxxxxxxxx> Subject: Re: [PATCH 06/15] drm/i915: Allow the caller to create a intel_context without PPGTT On Mon, May 16, 2016 at 01:32:44AM +0800, Zhi Wang wrote: > GVT context will use its own shadow PPGTT, and it doesn't need a > i915_hw_ppgtt. > > This patch adds a "has_ppgtt" param to i915_gem_context(), which > allows the caller to create a context without PPGTT > > Signed-off-by: Zhi Wang <zhi.a.wang@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_gem_context.c | 34 > ++++++++++++++++++--------------- > 1 file changed, 19 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c > b/drivers/gpu/drm/i915/i915_gem_context.c > index 21498e5..b952e37 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -298,7 +298,8 @@ err_out: > */ > static struct intel_context * > i915_gem_create_context(struct drm_device *dev, > - struct drm_i915_file_private *file_priv) > + struct drm_i915_file_private *file_priv, > + bool has_ppgtt) > { > struct drm_i915_private *dev_priv = dev->dev_private; > const bool is_global_default_ctx = file_priv == NULL; @@ -327,19 > +328,22 @@ i915_gem_create_context(struct drm_device *dev, > } > } > > - if (USES_FULL_PPGTT(dev)) { > - struct i915_hw_ppgtt *ppgtt = i915_ppgtt_create(dev, file_priv); > + if (has_ppgtt) { > + if (USES_FULL_PPGTT(dev)) { > + struct i915_hw_ppgtt *ppgtt = > + i915_ppgtt_create(dev, file_priv); > > - if (IS_ERR_OR_NULL(ppgtt)) { > - DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n", > - PTR_ERR(ppgtt)); > - ret = PTR_ERR(ppgtt); > - goto err_unpin; > - } > + if (IS_ERR_OR_NULL(ppgtt)) { > + DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n", > + PTR_ERR(ppgtt)); > + ret = PTR_ERR(ppgtt); > + goto err_unpin; > + } > > - ctx->ppgtt = ppgtt; > - } else > - ctx->ppgtt = dev_priv->mm.aliasing_ppgtt; > + ctx->ppgtt = ppgtt; > + } else > + ctx->ppgtt = dev_priv->mm.aliasing_ppgtt; You have to first go through the driver and update the sematics for ctx->ppgtt == NULL. (Note in some instances you have to use the ggtt pointer and not the appgtt, just a minor case in execbuf!). Then tell us how you didn't spot the explosion when trying to use the aliasing_ppgtt after freeing it. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
