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

[Lionel Landwerlin <lionel.g.landwerlin@intel.com>]

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@intel.com>
>> ---
>>   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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx