On Mon, May 23, 2016 at 10:17:01AM +0100, Tvrtko Ursulin wrote:
>
> On 22/05/16 14:02, Chris Wilson wrote:
> >As these are wrappers around kref_get/kref_put() it is preferable to
> >follow the naming convention and use the same verb get/put in our
> >wrapper names for manipulating a reference to the context.
>
> Not so sure about this one. We got objects, framebuffers and requests using
> (un)reference naming so don't see that making contexts different is a good
> idea.
And drm_blob too. For core modeset stuff I think converting to _get/put
might be worth it, too, but needs 3-stage approach: Add new _get/put
funcs. 2) cocci-generated patches for each driver/file to convert over. 3)
drop _reference/unreference variants once no longer used.
-Daniel
>
> Regards,
>
> Tvrtko
>
>
> >Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> >---
> > drivers/gpu/drm/i915/i915_drv.h | 6 ++++--
> > drivers/gpu/drm/i915/i915_gem.c | 7 +++----
> > drivers/gpu/drm/i915/i915_gem_context.c | 22 ++++++++++------------
> > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 +++---
> > drivers/gpu/drm/i915/intel_lrc.c | 4 ++--
> > 5 files changed, 22 insertions(+), 23 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >index e4e3614335ec..0a3442fcd93e 100644
> >--- a/drivers/gpu/drm/i915/i915_drv.h
> >+++ b/drivers/gpu/drm/i915/i915_drv.h
> >@@ -3435,12 +3435,14 @@ i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
> > return ctx;
> > }
> >
> >-static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
> >+static inline struct i915_gem_context *
> >+i915_gem_context_get(struct i915_gem_context *ctx)
> > {
> > kref_get(&ctx->ref);
> >+ return ctx;
> > }
> >
> >-static inline void i915_gem_context_unreference(struct i915_gem_context *ctx)
> >+static inline void i915_gem_context_put(struct i915_gem_context *ctx)
> > {
> > lockdep_assert_held(&ctx->i915->dev->struct_mutex);
> > kref_put(&ctx->ref, i915_gem_context_free);
> >diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >index 728b66911840..d7ae030e5a0b 100644
> >--- a/drivers/gpu/drm/i915/i915_gem.c
> >+++ b/drivers/gpu/drm/i915/i915_gem.c
> >@@ -1418,7 +1418,7 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
> > request->engine);
> > }
> >
> >- i915_gem_context_unreference(request->ctx);
> >+ i915_gem_context_put(request->ctx);
> > i915_gem_request_unreference(request);
> > }
> >
> >@@ -2775,8 +2775,7 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
> > req->i915 = dev_priv;
> > req->engine = engine;
> > req->reset_counter = reset_counter;
> >- req->ctx = ctx;
> >- i915_gem_context_reference(req->ctx);
> >+ req->ctx = i915_gem_context_get(ctx);
> >
> > /*
> > * Reserve space in the ring buffer for all the commands required to
> >@@ -2798,7 +2797,7 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
> > return 0;
> >
> > err_ctx:
> >- i915_gem_context_unreference(ctx);
> >+ i915_gem_context_put(ctx);
> > err:
> > kmem_cache_free(dev_priv->requests, req);
> > return ret;
> >diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> >index aa329175f73a..a4c6377eea32 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_context.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_context.c
> >@@ -290,7 +290,7 @@ __create_hw_context(struct drm_device *dev,
> > return ctx;
> >
> > err_out:
> >- i915_gem_context_unreference(ctx);
> >+ i915_gem_context_put(ctx);
> > return ERR_PTR(ret);
> > }
> >
> >@@ -351,7 +351,7 @@ err_unpin:
> > i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
> > err_destroy:
> > idr_remove(&file_priv->context_idr, ctx->user_handle);
> >- i915_gem_context_unreference(ctx);
> >+ i915_gem_context_put(ctx);
> > return ERR_PTR(ret);
> > }
> >
> >@@ -363,7 +363,7 @@ static void i915_gem_context_unpin(struct i915_gem_context *ctx,
> > } else {
> > if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state)
> > i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
> >- i915_gem_context_unreference(ctx);
> >+ i915_gem_context_put(ctx);
> > }
> > }
> >
> >@@ -463,7 +463,7 @@ void i915_gem_context_fini(struct drm_device *dev)
> > if (dctx->legacy_hw_ctx.rcs_state)
> > i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
> >
> >- i915_gem_context_unreference(dctx);
> >+ i915_gem_context_put(dctx);
> > dev_priv->kernel_context = NULL;
> >
> > ida_destroy(&dev_priv->context_hw_ida);
> >@@ -473,7 +473,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
> > {
> > struct i915_gem_context *ctx = p;
> >
> >- i915_gem_context_unreference(ctx);
> >+ i915_gem_context_put(ctx);
> > return 0;
> > }
> >
> >@@ -785,10 +785,9 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
> >
> > /* obj is kept alive until the next request by its active ref */
> > i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
> >- i915_gem_context_unreference(from);
> >+ i915_gem_context_put(from);
> > }
> >- i915_gem_context_reference(to);
> >- engine->last_context = to;
> >+ engine->last_context = i915_gem_context_get(to);
> >
> > /* GEN8 does *not* require an explicit reload if the PDPs have been
> > * setup, and we do not wish to move them.
> >@@ -873,10 +872,9 @@ int i915_switch_context(struct drm_i915_gem_request *req)
> > }
> >
> > if (to != engine->last_context) {
> >- i915_gem_context_reference(to);
> > if (engine->last_context)
> >- i915_gem_context_unreference(engine->last_context);
> >- engine->last_context = to;
> >+ i915_gem_context_put(engine->last_context);
> >+ engine->last_context = i915_gem_context_get(to);
> > }
> >
> > return 0;
> >@@ -944,7 +942,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
> > }
> >
> > idr_remove(&ctx->file_priv->context_idr, ctx->user_handle);
> >- i915_gem_context_unreference(ctx);
> >+ i915_gem_context_put(ctx);
> > mutex_unlock(&dev->struct_mutex);
> >
> > DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
> >diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >index 84d990331abd..14628ce91273 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >@@ -1492,7 +1492,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> > goto pre_mutex_err;
> > }
> >
> >- i915_gem_context_reference(ctx);
> >+ i915_gem_context_get(ctx);
> >
> > if (ctx->ppgtt)
> > vm = &ctx->ppgtt->base;
> >@@ -1503,7 +1503,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> >
> > eb = eb_create(args);
> > if (eb == NULL) {
> >- i915_gem_context_unreference(ctx);
> >+ i915_gem_context_put(ctx);
> > mutex_unlock(&dev->struct_mutex);
> > ret = -ENOMEM;
> > goto pre_mutex_err;
> >@@ -1647,7 +1647,7 @@ err_batch_unpin:
> >
> > err:
> > /* the request owns the ref now */
> >- i915_gem_context_unreference(ctx);
> >+ i915_gem_context_put(ctx);
> > eb_destroy(eb);
> >
> > mutex_unlock(&dev->struct_mutex);
> >diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >index 426dc90aceed..9a55478e23ac 100644
> >--- a/drivers/gpu/drm/i915/intel_lrc.c
> >+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >@@ -966,7 +966,6 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
> > if (ret)
> > goto unpin_map;
> >
> >- i915_gem_context_reference(ctx);
> > ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
> > intel_lr_context_descriptor_update(ctx, engine);
> > lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start;
> >@@ -977,6 +976,7 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
> > if (i915.enable_guc_submission)
> > I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
> >
> >+ i915_gem_context_get(ctx);
> > return 0;
> >
> > unpin_map:
> >@@ -1009,7 +1009,7 @@ void intel_lr_context_unpin(struct i915_gem_context *ctx,
> > ctx->engine[engine->id].lrc_desc = 0;
> > ctx->engine[engine->id].lrc_reg_state = NULL;
> >
> >- i915_gem_context_unreference(ctx);
> >+ i915_gem_context_put(ctx);
> > }
> >
> > static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
