Re: [Intel-gfx] [PATCH 07/11] drm/i915: i915_vma_move_to_active prep patch
On 14/12/15 11:36, Chris Wilson wrote: This patch is broken out of the next just to remove the code motion from that patch and make it more readable. What we do here is move the i915_vma_move_to_active() to i915_gem_execbuffer.c and put the three stages (read, write, fenced) together so that future modifications to active handling are all located in the same spot. The importance of this is so that we can more simply control the order in which the requests are place in the retirement list (i.e. control the order at which we retire and so control the lifetimes to avoid having to hold onto references). Signed-off-by: Chris Wilson--- drivers/gpu/drm/i915/i915_drv.h | 3 +- drivers/gpu/drm/i915/i915_gem.c | 15 --- drivers/gpu/drm/i915/i915_gem_context.c | 7 ++-- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 63 ++-- drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +- 5 files changed, 49 insertions(+), 41 deletions(-) Reviewed-by: Tvrtko Ursulin Regards, Tvrtko diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b32a00f60e98..eb775eb1c693 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2775,7 +2775,8 @@ int __must_check i915_mutex_lock_interruptible(struct drm_device *dev); int i915_gem_object_sync(struct drm_i915_gem_object *obj, struct drm_i915_gem_request *to); void i915_vma_move_to_active(struct i915_vma *vma, -struct drm_i915_gem_request *req); +struct drm_i915_gem_request *req, +unsigned flags); int i915_gem_dumb_create(struct drm_file *file_priv, struct drm_device *dev, struct drm_mode_create_dumb *args); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 144e92df8137..8a824c5d5348 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2016,21 +2016,6 @@ void *i915_gem_object_pin_vmap(struct drm_i915_gem_object *obj) return obj->vmapping; } -void i915_vma_move_to_active(struct i915_vma *vma, -struct drm_i915_gem_request *req) -{ - struct drm_i915_gem_object *obj = vma->obj; - struct intel_engine_cs *engine = req->engine; - - /* Add a reference if we're newly entering the active list. */ - if (obj->active == 0) - drm_gem_object_reference(>base); - obj->active |= intel_engine_flag(engine); - - i915_gem_request_mark_active(req, >last_read[engine->id]); - list_move_tail(>vm_link, >vm->active_list); -} - static void i915_gem_object_retire__fence(struct drm_i915_gem_request_active *active, struct drm_i915_gem_request *req) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index dcb4603a7f03..c4a8a64cd1b2 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -766,8 +766,8 @@ static int do_switch(struct drm_i915_gem_request *req) * MI_SET_CONTEXT instead of when the next seqno has completed. */ if (from != NULL) { - from->legacy_hw_ctx.rcs_state->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION; - i915_vma_move_to_active(i915_gem_obj_to_ggtt(from->legacy_hw_ctx.rcs_state), req); + struct drm_i915_gem_object *obj = from->legacy_hw_ctx.rcs_state; + /* As long as MI_SET_CONTEXT is serializing, ie. it flushes the * whole damn pipeline, we don't need to explicitly mark the * object dirty. The only exception is that the context must be @@ -775,7 +775,8 @@ static int do_switch(struct drm_i915_gem_request *req) * able to defer doing this until we know the object would be * swapped, but there is no way to do that yet. */ - from->legacy_hw_ctx.rcs_state->dirty = 1; + obj->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION; + i915_vma_move_to_active(i915_gem_obj_to_ggtt(obj), req, 0); /* obj is kept alive until the next request by its active ref */ i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state); diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 6788f71ad989..6de8681bb64c 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1064,6 +1064,44 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file, return ctx; } +void i915_vma_move_to_active(struct i915_vma *vma, +struct drm_i915_gem_request *req, +unsigned flags) +{ +
[Intel-gfx] [PATCH 07/11] drm/i915: i915_vma_move_to_active prep patch
This patch is broken out of the next just to remove the code motion from that patch and make it more readable. What we do here is move the i915_vma_move_to_active() to i915_gem_execbuffer.c and put the three stages (read, write, fenced) together so that future modifications to active handling are all located in the same spot. The importance of this is so that we can more simply control the order in which the requests are place in the retirement list (i.e. control the order at which we retire and so control the lifetimes to avoid having to hold onto references). Signed-off-by: Chris Wilson--- drivers/gpu/drm/i915/i915_drv.h | 3 +- drivers/gpu/drm/i915/i915_gem.c | 15 --- drivers/gpu/drm/i915/i915_gem_context.c | 7 ++-- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 63 ++-- drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +- 5 files changed, 49 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b32a00f60e98..eb775eb1c693 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2775,7 +2775,8 @@ int __must_check i915_mutex_lock_interruptible(struct drm_device *dev); int i915_gem_object_sync(struct drm_i915_gem_object *obj, struct drm_i915_gem_request *to); void i915_vma_move_to_active(struct i915_vma *vma, -struct drm_i915_gem_request *req); +struct drm_i915_gem_request *req, +unsigned flags); int i915_gem_dumb_create(struct drm_file *file_priv, struct drm_device *dev, struct drm_mode_create_dumb *args); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 144e92df8137..8a824c5d5348 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2016,21 +2016,6 @@ void *i915_gem_object_pin_vmap(struct drm_i915_gem_object *obj) return obj->vmapping; } -void i915_vma_move_to_active(struct i915_vma *vma, -struct drm_i915_gem_request *req) -{ - struct drm_i915_gem_object *obj = vma->obj; - struct intel_engine_cs *engine = req->engine; - - /* Add a reference if we're newly entering the active list. */ - if (obj->active == 0) - drm_gem_object_reference(>base); - obj->active |= intel_engine_flag(engine); - - i915_gem_request_mark_active(req, >last_read[engine->id]); - list_move_tail(>vm_link, >vm->active_list); -} - static void i915_gem_object_retire__fence(struct drm_i915_gem_request_active *active, struct drm_i915_gem_request *req) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index dcb4603a7f03..c4a8a64cd1b2 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -766,8 +766,8 @@ static int do_switch(struct drm_i915_gem_request *req) * MI_SET_CONTEXT instead of when the next seqno has completed. */ if (from != NULL) { - from->legacy_hw_ctx.rcs_state->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION; - i915_vma_move_to_active(i915_gem_obj_to_ggtt(from->legacy_hw_ctx.rcs_state), req); + struct drm_i915_gem_object *obj = from->legacy_hw_ctx.rcs_state; + /* As long as MI_SET_CONTEXT is serializing, ie. it flushes the * whole damn pipeline, we don't need to explicitly mark the * object dirty. The only exception is that the context must be @@ -775,7 +775,8 @@ static int do_switch(struct drm_i915_gem_request *req) * able to defer doing this until we know the object would be * swapped, but there is no way to do that yet. */ - from->legacy_hw_ctx.rcs_state->dirty = 1; + obj->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION; + i915_vma_move_to_active(i915_gem_obj_to_ggtt(obj), req, 0); /* obj is kept alive until the next request by its active ref */ i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state); diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 6788f71ad989..6de8681bb64c 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1064,6 +1064,44 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file, return ctx; } +void i915_vma_move_to_active(struct i915_vma *vma, +struct drm_i915_gem_request *req, +unsigned flags) +{ + struct drm_i915_gem_object *obj = vma->obj; + const unsigned engine = req->engine->id; + +