Quoting Lionel Landwerlin (2018-05-29 22:35:08) > 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; /**/ It's debatable if we want to keep the pointer around. We have one for the context state vma and ring vma, because we need to keep a pointer for the object and so choose to keep the vma instead as we use that more often than the drm_i915_gem_object and can derive it from vma->obj. For this piece of code, which should be run that often I don't see a lot of advantage over using the rbtree search, and since the number of objects in that tree will not be large (2 at most), quick. /* Don't forget to declare the dependency tree! */ prev = i915_gem_active_raw(&ce->ring->timeline->last_request, &rq->i915->drm.struct_mutex); if (prev) { err = i915_sw_fence_await_sw_fence_gfp(&rq->submit, &prev->submit, I915_FENCE_GFP); if (err < 0) return err; } vma = i915_vma_instance(ce->state->obj, my_vm, NULL); if (IS_ERR(vma)) ... err = i915_vma_pin(vma, 0, PIN_USER); if (err) ... err = i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE); i915_vma_unpin(vma); if (err) ... /* not today, but tomorrow */ Now you can cs = intel_ring_begin(rq, 4); ... *cs++ = gen8_canonical_addr(vma->node.state + offset); ... intel_ring_advance(rq, cs); On error, you will have to submit the request since it has interacted with the tracking. Don't bother pushing this into an engine vfunc, if you don't intend to have multiple implementations. It's only a SDM, nothing that appears the need to be specialised. Although you might argue the context layout will require it later? As for the SDM, Joonas might complain about the proliferation, but I'm not sure if we have a good repeating pattern yet. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx