I understand what you say. Let me think about other approach. My requirement is to do nothing if a context doesn't have a ppgtt, but ctx->ppgtt == NULL means context is using aliasing ppgtt, so I need to find another way to achieve this. -----Original Message----- From: Tvrtko Ursulin [mailto:tvrtko.ursulin@xxxxxxxxxxxxxxx] Sent: Monday, May 16, 2016 6:31 AM To: Wang, Zhi A <zhi.a.wang@xxxxxxxxx>; 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 05/15] drm/i915: Set ctx->ppgtt for aliasing PPGTT in context creation On 15/05/16 18:32, Zhi Wang wrote: > Currently ctx->ppgtt would only be initialized when full PPGTT is used. > For aliasing PPGTT mode, ctx->ppgtt will be set when LRC context is > populated. > > This patch moves the assignment into i915_gem_create_context() for > better code structure. Hm, it doesn't move the assignment it adds it. Previously ctx->ppgtt was always NULL when !USES_FULL_PPGTT. Now it will point to aliasing ppgtt. Since there are various places in the code which do "if (ctx->ppgtt)" (more or less) I think you need to explain why those will still work OK. i915_gem_context_free for example? Regards, Tvrtko > Signed-off-by: Zhi Wang <zhi.a.wang@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_gem_context.c | 4 +++- > drivers/gpu/drm/i915/intel_lrc.c | 3 --- > 2 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c > b/drivers/gpu/drm/i915/i915_gem_context.c > index 2aedd18..21498e5 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -300,6 +300,7 @@ static struct intel_context * > i915_gem_create_context(struct drm_device *dev, > struct drm_i915_file_private *file_priv) > { > + struct drm_i915_private *dev_priv = dev->dev_private; > const bool is_global_default_ctx = file_priv == NULL; > struct intel_context *ctx; > int ret = 0; > @@ -337,7 +338,8 @@ i915_gem_create_context(struct drm_device *dev, > } > > ctx->ppgtt = ppgtt; > - } > + } else > + ctx->ppgtt = dev_priv->mm.aliasing_ppgtt; > > trace_i915_context_create(ctx); > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index db10c96..397fe65 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -2286,9 +2286,6 @@ populate_lr_context(struct intel_context *ctx, > u32 *reg_state; > int ret; > > - if (!ppgtt) > - ppgtt = dev_priv->mm.aliasing_ppgtt; > - > ret = i915_gem_object_set_to_cpu_domain(ctx_obj, true); > if (ret) { > DRM_DEBUG_DRIVER("Could not set to CPU domain\n"); > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
