diff mbox series

[v5,18/21] drm/tegra: Allocate per-engine channel in core code

Message ID 20210111130019.3515669-19-mperttunen@nvidia.com
State New
Headers show
Series [v5,01/21] gpu: host1x: Use different lock classes for each client | expand

Commit Message

Mikko Perttunen Jan. 11, 2021, 1 p.m. UTC
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(+)

Comments

Thierry Reding March 23, 2021, 12:35 p.m. UTC | #1
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
Mikko Perttunen March 23, 2021, 1:15 p.m. UTC | #2
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 mbox series

Patch

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 *