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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 29/05/18 23:08, Chris Wilson wrote:
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;
}

Thanks, this is done by the caller.


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.

Yep, done in the caller too.

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
Hmm.. That's kind of useful to detect the ability to change powergating configs.

Thanks a lot,

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux