| Message ID | 20210111130019.3515669-3-mperttunen@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Host1x/TegraDRM UAPI | expand |
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
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
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) { 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) goto free_base; - sp->client = client; - sp->name = name; + sp->name = full_name; if (flags & HOST1X_SYNCPT_CLIENT_MANAGED) sp->client_managed = true; @@ -87,6 +85,7 @@ static struct host1x_syncpt *host1x_syncpt_alloc(struct host1x *host, mutex_unlock(&host->syncpt_mutex); return NULL; } +EXPORT_SYMBOL(host1x_syncpt_alloc); /** * host1x_syncpt_id() - retrieve syncpoint ID @@ -401,7 +400,7 @@ int host1x_syncpt_init(struct host1x *host) host1x_hw_syncpt_enable_protection(host); /* Allocate sync point to use for clearing waits for expired fences */ - host->nop_sp = host1x_syncpt_alloc(host, NULL, 0); + host->nop_sp = host1x_syncpt_alloc(host, 0, "reserved-nop"); if (!host->nop_sp) return -ENOMEM; @@ -423,7 +422,7 @@ struct host1x_syncpt *host1x_syncpt_request(struct host1x_client *client, { struct host1x *host = dev_get_drvdata(client->host->parent); - return host1x_syncpt_alloc(host, client, flags); + return host1x_syncpt_alloc(host, flags, dev_name(client->dev)); } EXPORT_SYMBOL(host1x_syncpt_request); @@ -447,7 +446,6 @@ void host1x_syncpt_free(struct host1x_syncpt *sp) host1x_syncpt_base_free(sp->base); kfree(sp->name); sp->base = NULL; - sp->client = NULL; sp->name = NULL; sp->client_managed = false; diff --git a/drivers/gpu/host1x/syncpt.h b/drivers/gpu/host1x/syncpt.h index 8e1d04dacaa0..3aa6b25b1b9c 100644 --- a/drivers/gpu/host1x/syncpt.h +++ b/drivers/gpu/host1x/syncpt.h @@ -33,7 +33,6 @@ struct host1x_syncpt { const char *name; bool client_managed; struct host1x *host; - struct host1x_client *client; struct host1x_syncpt_base *base; /* interrupt data */ diff --git a/include/linux/host1x.h b/include/linux/host1x.h index 9eb77c87a83b..7137ce0e35d4 100644 --- a/include/linux/host1x.h +++ b/include/linux/host1x.h @@ -154,6 +154,9 @@ int host1x_syncpt_wait(struct host1x_syncpt *sp, u32 thresh, long timeout, struct host1x_syncpt *host1x_syncpt_request(struct host1x_client *client, unsigned long flags); void host1x_syncpt_free(struct host1x_syncpt *sp); +struct host1x_syncpt *host1x_syncpt_alloc(struct host1x *host, + unsigned long flags, + const char *name); struct host1x_syncpt_base *host1x_syncpt_get_base(struct host1x_syncpt *sp); u32 host1x_syncpt_base_id(struct host1x_syncpt_base *base);
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(-)