Re: [PATCH 1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 10/06/16 18:29, Dave Gordon wrote:
> Convert all static functions in i915_guc_submission.c that currently
> take a 'dev' pointer to take 'dev_priv' instead (there are three,
> guc_client_alloc(), guc_client_free(), and gem_allocate_guc_obj().
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 39 +++++++++++++++---------------
>   1 file changed, 19 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 2db1182..1bd0fac 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -591,7 +591,7 @@ int i915_guc_submit(struct drm_i915_gem_request *rq)
>
>   /**
>    * gem_allocate_guc_obj() - Allocate gem object for GuC usage
> - * @dev:	drm device
> + * @dev_priv:	driver private data structure
>    * @size:	size of object
>    *
>    * This is a wrapper to create a gem obj. In order to use it inside GuC, the
> @@ -600,13 +600,12 @@ int i915_guc_submit(struct drm_i915_gem_request *rq)
>    *
>    * Return:	A drm_i915_gem_object if successful, otherwise NULL.
>    */
> -static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev,
> -							u32 size)
> +static struct drm_i915_gem_object *
> +gem_allocate_guc_obj(struct drm_i915_private *dev_priv, u32 size)
>   {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct drm_i915_gem_object *obj;
>
> -	obj = i915_gem_object_create(dev, size);
> +	obj = i915_gem_object_create(dev_priv->dev, size);
>   	if (IS_ERR(obj))
>   		return NULL;
>
> @@ -642,10 +641,10 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
>   	drm_gem_object_unreference(&obj->base);
>   }
>
> -static void guc_client_free(struct drm_device *dev,
> -			    struct i915_guc_client *client)
> +static void
> +guc_client_free(struct drm_i915_private *dev_priv,
> +		struct i915_guc_client *client)
>   {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_guc *guc = &dev_priv->guc;
>
>   	if (!client)
> @@ -688,7 +687,7 @@ static void guc_client_free(struct drm_device *dev,
>
>   /**
>    * guc_client_alloc() - Allocate an i915_guc_client
> - * @dev:	drm device
> + * @dev_priv:	driver private data structure
>    * @priority:	four levels priority _CRITICAL, _HIGH, _NORMAL and _LOW
>    * 		The kernel client to replace ExecList submission is created with
>    * 		NORMAL priority. Priority of a client for scheduler can be HIGH,
> @@ -698,12 +697,12 @@ static void guc_client_free(struct drm_device *dev,
>    *
>    * Return:	An i915_guc_client object if success, else NULL.
>    */
> -static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
> -						uint32_t priority,
> -						struct i915_gem_context *ctx)
> +static struct i915_guc_client *
> +guc_client_alloc(struct drm_i915_private *dev_priv,
> +		 uint32_t priority,
> +		 struct i915_gem_context *ctx)
>   {
>   	struct i915_guc_client *client;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_guc *guc = &dev_priv->guc;
>   	struct drm_i915_gem_object *obj;
>
> @@ -724,7 +723,7 @@ static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
>   	}
>
>   	/* The first page is doorbell/proc_desc. Two followed pages are wq. */
> -	obj = gem_allocate_guc_obj(dev, GUC_DB_SIZE + GUC_WQ_SIZE);
> +	obj = gem_allocate_guc_obj(dev_priv, GUC_DB_SIZE + GUC_WQ_SIZE);
>   	if (!obj)
>   		goto err;
>
> @@ -768,7 +767,7 @@ static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
>   err:
>   	DRM_ERROR("FAILED to create priority %u GuC client!\n", priority);
>
> -	guc_client_free(dev, client);
> +	guc_client_free(dev_priv, client);
>   	return NULL;
>   }
>
> @@ -793,7 +792,7 @@ static void guc_create_log(struct intel_guc *guc)
>
>   	obj = guc->log_obj;
>   	if (!obj) {
> -		obj = gem_allocate_guc_obj(dev_priv->dev, size);
> +		obj = gem_allocate_guc_obj(dev_priv, size);
>   		if (!obj) {
>   			/* logging will be off */
>   			i915.guc_log_level = -1;
> @@ -853,7 +852,7 @@ static void guc_create_ads(struct intel_guc *guc)
>
>   	obj = guc->ads_obj;
>   	if (!obj) {
> -		obj = gem_allocate_guc_obj(dev_priv->dev, PAGE_ALIGN(size));
> +		obj = gem_allocate_guc_obj(dev_priv, PAGE_ALIGN(size));
>   		if (!obj)
>   			return;
>
> @@ -925,7 +924,7 @@ int i915_guc_submission_init(struct drm_device *dev)
>   	if (guc->ctx_pool_obj)
>   		return 0; /* already allocated */
>
> -	guc->ctx_pool_obj = gem_allocate_guc_obj(dev_priv->dev, gemsize);
> +	guc->ctx_pool_obj = gem_allocate_guc_obj(dev_priv, gemsize);
>   	if (!guc->ctx_pool_obj)
>   		return -ENOMEM;
>
> @@ -943,7 +942,7 @@ int i915_guc_submission_enable(struct drm_device *dev)
>   	struct i915_guc_client *client;
>
>   	/* client for execbuf submission */
> -	client = guc_client_alloc(dev,
> +	client = guc_client_alloc(dev_priv,
>   				  GUC_CTX_PRIORITY_KMD_NORMAL,
>   				  dev_priv->kernel_context);
>   	if (!client) {
> @@ -963,7 +962,7 @@ void i915_guc_submission_disable(struct drm_device *dev)
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_guc *guc = &dev_priv->guc;
>
> -	guc_client_free(dev, guc->execbuf_client);
> +	guc_client_free(dev_priv, guc->execbuf_client);
>   	guc->execbuf_client = NULL;
>   }
>
>

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

And you can keep the r-b if you rename gem_allocate_guc_obj as Chris has 
suggested.

Regards,

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