Re: [PATCH 04/12] drm/i915: Rename and inline i915_gem_context_get()

[Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>]

On 22/05/16 14:02, Chris Wilson wrote:
> i915_gem_context_get() is a very simple wrapper around idr_find(), so
> simple that it would be smaller to do the lookup inline. Also we use the
> verb 'lookup' to return a pointer from a handle, freeing 'get' to imply
> obtaining a reference to the context.
>
> 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            | 17 +++++++++++++++--
>   drivers/gpu/drm/i915/i915_gem_context.c    | 22 ++++------------------
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |  2 +-
>   3 files changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 48a222ddd8b3..e4e3614335ec 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3417,11 +3417,24 @@ void i915_gem_context_reset(struct drm_device *dev);
>   int i915_gem_context_open(struct drm_device *dev, struct drm_file *file);
>   void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
>   int i915_switch_context(struct drm_i915_gem_request *req);
> -struct i915_gem_context *
> -i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
>   void i915_gem_context_free(struct kref *ctx_ref);
>   struct drm_i915_gem_object *
>   i915_gem_alloc_context_obj(struct drm_device *dev, size_t size);
> +
> +static inline struct i915_gem_context *
> +i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
> +{
> +	struct i915_gem_context *ctx;
> +
> +	lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex);
> +
> +	ctx = idr_find(&file_priv->context_idr, id);
> +	if (!ctx)
> +		return ERR_PTR(-ENOENT);
> +
> +	return ctx;
> +}
> +
>   static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
>   {
>   	kref_get(&ctx->ref);
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index af747f14522e..aa329175f73a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -506,20 +506,6 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
>   	idr_destroy(&file_priv->context_idr);
>   }
>
> -struct i915_gem_context *
> -i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
> -{
> -	struct i915_gem_context *ctx;
> -
> -	lockdep_assert_held(&file_priv->dev_priv->dev->struct_mutex);
> -
> -	ctx = idr_find(&file_priv->context_idr, id);
> -	if (!ctx)
> -		return ERR_PTR(-ENOENT);
> -
> -	return ctx;
> -}
> -
>   static inline int
>   mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
>   {
> @@ -951,7 +937,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		return ret;
>
> -	ctx = i915_gem_context_get(file_priv, args->ctx_id);
> +	ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
>   	if (IS_ERR(ctx)) {
>   		mutex_unlock(&dev->struct_mutex);
>   		return PTR_ERR(ctx);
> @@ -977,7 +963,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		return ret;
>
> -	ctx = i915_gem_context_get(file_priv, args->ctx_id);
> +	ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
>   	if (IS_ERR(ctx)) {
>   		mutex_unlock(&dev->struct_mutex);
>   		return PTR_ERR(ctx);
> @@ -1020,7 +1006,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		return ret;
>
> -	ctx = i915_gem_context_get(file_priv, args->ctx_id);
> +	ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
>   	if (IS_ERR(ctx)) {
>   		mutex_unlock(&dev->struct_mutex);
>   		return PTR_ERR(ctx);
> @@ -1072,7 +1058,7 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
>   	if (ret)
>   		return ret;
>
> -	ctx = i915_gem_context_get(file->driver_priv, args->ctx_id);
> +	ctx = i915_gem_context_lookup(file->driver_priv, args->ctx_id);
>   	if (IS_ERR(ctx)) {
>   		mutex_unlock(&dev->struct_mutex);
>   		return PTR_ERR(ctx);
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index e61b92d7b0bc..84d990331abd 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1075,7 +1075,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
>   	if (engine->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE)
>   		return ERR_PTR(-EINVAL);
>
> -	ctx = i915_gem_context_get(file->driver_priv, ctx_id);
> +	ctx = i915_gem_context_lookup(file->driver_priv, ctx_id);
>   	if (IS_ERR(ctx))
>   		return ctx;

Makes sense.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Regards,

Tvrtko

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