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

Mikko Perttunen
2021-03-23T10:33:08Z

On 3/23/21 12:10 PM, Thierry Reding wrote:
> 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.

Will fix.

>
>> {
>> 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?

I'll see about making the name mandatory.

>
> Thierry
>

Mikko