Re: [PATCH v5 02/21] gpu: host1x: Allow syncpoints without associated client

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2088 bytes --]

On Mon, Jan 11, 2021 at 03:00:00PM +0200, Mikko Perttunen wrote:
> Syncpoints don't need to be associated with any client,
> so remove the property, and expose host1x_syncpt_alloc.
> This will allow allocating syncpoints without prior knowledge
> of the engine that it will be used with.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> v3:
> * Clean up host1x_syncpt_alloc signature to allow specifying
>   a name for the syncpoint.
> * Export the function.
> ---
>  drivers/gpu/host1x/syncpt.c | 22 ++++++++++------------
>  drivers/gpu/host1x/syncpt.h |  1 -
>  include/linux/host1x.h      |  3 +++
>  3 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c
> index fce7892d5137..5982fdf64e1c 100644
> --- a/drivers/gpu/host1x/syncpt.c
> +++ b/drivers/gpu/host1x/syncpt.c
> @@ -42,13 +42,13 @@ static void host1x_syncpt_base_free(struct host1x_syncpt_base *base)
>  		base->requested = false;
>  }
>  
> -static struct host1x_syncpt *host1x_syncpt_alloc(struct host1x *host,
> -						 struct host1x_client *client,
> -						 unsigned long flags)
> +struct host1x_syncpt *host1x_syncpt_alloc(struct host1x *host,
> +					  unsigned long flags,
> +					  const char *name)

If we expose it publicly, it's a good idea to add kerneldoc.

>  {
>  	struct host1x_syncpt *sp = host->syncpt;
> +	char *full_name;
>  	unsigned int i;
> -	char *name;
>  
>  	mutex_lock(&host->syncpt_mutex);
>  
> @@ -64,13 +64,11 @@ static struct host1x_syncpt *host1x_syncpt_alloc(struct host1x *host,
>  			goto unlock;
>  	}
>  
> -	name = kasprintf(GFP_KERNEL, "%02u-%s", sp->id,
> -			 client ? dev_name(client->dev) : NULL);
> -	if (!name)
> +	full_name = kasprintf(GFP_KERNEL, "%u-%s", sp->id, name);
> +	if (!full_name)

I know this just keeps with the status quo, but I wonder if we should
change this to be just "%u" if name == NULL to avoid a weird-looking
name. Or perhaps we want to enforce name != NULL by failing if that's
not the case?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]