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

Thierry Reding
2021-03-23T10:11:48Z

[-- 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 --]