Re: [PATCH v8 6/8] drm/i915: create context image vma in kernel context

[Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>]

On 29/05/18 21:26, Michel Thierry wrote:
> Hi,
>
> On 5/29/2018 12:16 PM, Lionel Landwerlin wrote:
> > We want to be able to modify other context images from the kernel
> > context in a following commit. To be able to do this we need to map
> > the context image into the kernel context's ppgtt.
> >
> > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
> > ---
> >   drivers/gpu/drm/i915/i915_gem_context.h |  1 +
> >   drivers/gpu/drm/i915/intel_lrc.c        | 19 ++++++++++++++-----
> >   2 files changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.h 
> > b/drivers/gpu/drm/i915/i915_gem_context.h
> > index f40d85448a28..9c313c2edb09 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.h
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.h
> > @@ -155,6 +155,7 @@ struct i915_gem_context {
> >       struct intel_context {
> >           struct i915_gem_context *gem_context;
> >           struct i915_vma *state;
> > +        struct i915_vma *kernel_state; /**/
> >           struct intel_ring *ring;
> >           u32 *lrc_reg_state;
> >           u64 lrc_desc;
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> > b/drivers/gpu/drm/i915/intel_lrc.c
> > index 7314e548fb4e..8a49323f6672 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -2739,7 +2739,7 @@ static int 
> > execlists_context_deferred_alloc(struct i915_gem_context *ctx,
> >                           struct intel_context *ce)
> >   {
> >       struct drm_i915_gem_object *ctx_obj;
> > -    struct i915_vma *vma;
> > +    struct i915_vma *ggtt_vma, *ppgtt_vma;
> >       uint32_t context_size;
> >       struct intel_ring *ring;
> >       struct i915_timeline *timeline;
> > @@ -2762,9 +2762,17 @@ static int 
> > execlists_context_deferred_alloc(struct i915_gem_context *ctx,
> >           goto error_deref_obj;
> >       }
> >   -    vma = i915_vma_instance(ctx_obj, &ctx->i915->ggtt.base, NULL);
> > -    if (IS_ERR(vma)) {
> > -        ret = PTR_ERR(vma);
> > +    ggtt_vma = i915_vma_instance(ctx_obj, &ctx->i915->ggtt.base, NULL);
> > +    if (IS_ERR(ggtt_vma)) {
> > +        ret = PTR_ERR(ggtt_vma);
> > +        goto error_deref_obj;
> > +    }
> > +
> > +    ppgtt_vma = i915_vma_instance(ctx_obj,
> > + &ctx->i915->kernel_context->ppgtt->base,
> This is dangerous if someone decides to use 'aliasing ppgtt' 
> (enable_ppgtt=1 is still a valid option).
>
> You'll need something like this,
> struct i915_hw_ppgtt *ppgtt = ctx->i915->kernel_context->ppgtt ?:
>                     ctx->i915->mm.aliasing_ppgtt;

Thanks a lot of the tip, I'm very much new to this.
Will fix and see if they are tests that can exercise this in igt.

-
Lionel

> > +                      NULL);
> > +    if (IS_ERR(ppgtt_vma)) {
> > +        ret = PTR_ERR(ppgtt_vma);
> >           goto error_deref_obj;
> >       }
> >   @@ -2788,7 +2796,8 @@ static int 
> > execlists_context_deferred_alloc(struct i915_gem_context *ctx,
> >       }
> >         ce->ring = ring;
> > -    ce->state = vma;
> > +    ce->state = ggtt_vma;
> > +    ce->kernel_state = ppgtt_vma;
> >         return 0;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx