| Message ID | 20210111130019.3515669-19-mperttunen@nvidia.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v5,01/21] gpu: host1x: Use different lock classes for each client | expand |
On Mon, Jan 11, 2021 at 03:00:16PM +0200, Mikko Perttunen wrote: > To avoid duplication, allocate the per-engine shared channel in the > core code instead. Once MLOCKs are implemented on Host1x side, we > can also update this to avoid allocating a shared channel when > MLOCKs are enabled. > > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com> > --- > drivers/gpu/drm/tegra/drm.c | 11 +++++++++++ > drivers/gpu/drm/tegra/drm.h | 4 ++++ > 2 files changed, 15 insertions(+) It'd be helpful if the commit message explained what these per-engine shared channels are used for. > > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c > index cd81b52a9e06..afd3f143c5e0 100644 > --- a/drivers/gpu/drm/tegra/drm.c > +++ b/drivers/gpu/drm/tegra/drm.c > @@ -880,6 +880,14 @@ static const struct drm_driver tegra_drm_driver = { > int tegra_drm_register_client(struct tegra_drm *tegra, > struct tegra_drm_client *client) > { > + /* > + * When MLOCKs are implemented, change to allocate a shared channel > + * only when MLOCKs are disabled. > + */ > + client->shared_channel = host1x_channel_request(&client->base); > + if (!client->shared_channel) > + return -EBUSY; > + > mutex_lock(&tegra->clients_lock); > list_add_tail(&client->list, &tegra->clients); > client->drm = tegra; > @@ -896,6 +904,9 @@ int tegra_drm_unregister_client(struct tegra_drm *tegra, > client->drm = NULL; > mutex_unlock(&tegra->clients_lock); > > + if (client->shared_channel) > + host1x_channel_put(client->shared_channel); > + > return 0; > } > > diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h > index f38de08e0c95..0f38f159aa8e 100644 > --- a/drivers/gpu/drm/tegra/drm.h > +++ b/drivers/gpu/drm/tegra/drm.h > @@ -87,8 +87,12 @@ struct tegra_drm_client { > struct list_head list; > struct tegra_drm *drm; > > + /* Set by driver */ > unsigned int version; > const struct tegra_drm_client_ops *ops; > + > + /* Set by TegraDRM core */ > + struct host1x_channel *shared_channel; Perhaps reorder this so that the core-initialized fields are closer to the top and the client-initialized fields are closer to the bottom? That seems like a more natural order. Thierry
On 3/23/21 2:35 PM, Thierry Reding wrote: > On Mon, Jan 11, 2021 at 03:00:16PM +0200, Mikko Perttunen wrote: >> To avoid duplication, allocate the per-engine shared channel in the >> core code instead. Once MLOCKs are implemented on Host1x side, we >> can also update this to avoid allocating a shared channel when >> MLOCKs are enabled. >> >> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com> >> --- >> drivers/gpu/drm/tegra/drm.c | 11 +++++++++++ >> drivers/gpu/drm/tegra/drm.h | 4 ++++ >> 2 files changed, 15 insertions(+) > > It'd be helpful if the commit message explained what these per-engine > shared channels are used for. The per-engine shared channel is just the normal HW channel we are currently using across each userspace logical channel (per engine). In the future the plan is to use one HW channel for each logical channel opened by the userspace on Tegra186+, so this will be extended then. I will rephrase to make it clearer. > >> >> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c >> index cd81b52a9e06..afd3f143c5e0 100644 >> --- a/drivers/gpu/drm/tegra/drm.c >> +++ b/drivers/gpu/drm/tegra/drm.c >> @@ -880,6 +880,14 @@ static const struct drm_driver tegra_drm_driver = { >> int tegra_drm_register_client(struct tegra_drm *tegra, >> struct tegra_drm_client *client) >> { >> + /* >> + * When MLOCKs are implemented, change to allocate a shared channel >> + * only when MLOCKs are disabled. >> + */ >> + client->shared_channel = host1x_channel_request(&client->base); >> + if (!client->shared_channel) >> + return -EBUSY; >> + >> mutex_lock(&tegra->clients_lock); >> list_add_tail(&client->list, &tegra->clients); >> client->drm = tegra; >> @@ -896,6 +904,9 @@ int tegra_drm_unregister_client(struct tegra_drm *tegra, >> client->drm = NULL; >> mutex_unlock(&tegra->clients_lock); >> >> + if (client->shared_channel) >> + host1x_channel_put(client->shared_channel); >> + >> return 0; >> } >> >> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h >> index f38de08e0c95..0f38f159aa8e 100644 >> --- a/drivers/gpu/drm/tegra/drm.h >> +++ b/drivers/gpu/drm/tegra/drm.h >> @@ -87,8 +87,12 @@ struct tegra_drm_client { >> struct list_head list; >> struct tegra_drm *drm; >> >> + /* Set by driver */ >> unsigned int version; >> const struct tegra_drm_client_ops *ops; >> + >> + /* Set by TegraDRM core */ >> + struct host1x_channel *shared_channel; > > Perhaps reorder this so that the core-initialized fields are closer to > the top and the client-initialized fields are closer to the bottom? That > seems like a more natural order. Will do. > > Thierry > Mikko
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index cd81b52a9e06..afd3f143c5e0 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -880,6 +880,14 @@ static const struct drm_driver tegra_drm_driver = { int tegra_drm_register_client(struct tegra_drm *tegra, struct tegra_drm_client *client) { + /* + * When MLOCKs are implemented, change to allocate a shared channel + * only when MLOCKs are disabled. + */ + client->shared_channel = host1x_channel_request(&client->base); + if (!client->shared_channel) + return -EBUSY; + mutex_lock(&tegra->clients_lock); list_add_tail(&client->list, &tegra->clients); client->drm = tegra; @@ -896,6 +904,9 @@ int tegra_drm_unregister_client(struct tegra_drm *tegra, client->drm = NULL; mutex_unlock(&tegra->clients_lock); + if (client->shared_channel) + host1x_channel_put(client->shared_channel); + return 0; } diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h index f38de08e0c95..0f38f159aa8e 100644 --- a/drivers/gpu/drm/tegra/drm.h +++ b/drivers/gpu/drm/tegra/drm.h @@ -87,8 +87,12 @@ struct tegra_drm_client { struct list_head list; struct tegra_drm *drm; + /* Set by driver */ unsigned int version; const struct tegra_drm_client_ops *ops; + + /* Set by TegraDRM core */ + struct host1x_channel *shared_channel; }; static inline struct tegra_drm_client *
To avoid duplication, allocate the per-engine shared channel in the core code instead. Once MLOCKs are implemented on Host1x side, we can also update this to avoid allocating a shared channel when MLOCKs are enabled. Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com> --- drivers/gpu/drm/tegra/drm.c | 11 +++++++++++ drivers/gpu/drm/tegra/drm.h | 4 ++++ 2 files changed, 15 insertions(+)