[2/4] gpu: host1x: Enable gather filter

Message ID 20170818161553.27597-3-mperttunen@nvidia.com
State Superseded
Headers show

Commit Message

Mikko Perttunen Aug. 18, 2017, 4:15 p.m.
The gather filter is a feature present on Tegra124 and newer where the
hardware prevents GATHERed command buffers from executing commands
normally reserved for the CDMA pushbuffer which is maintained by the
kernel driver.

This commit enables the gather filter on all supporting hardware.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/gpu/host1x/hw/channel_hw.c          | 22 ++++++++++++++++++++++
 drivers/gpu/host1x/hw/hw_host1x04_channel.h | 12 ++++++++++++
 drivers/gpu/host1x/hw/hw_host1x05_channel.h | 12 ++++++++++++
 3 files changed, 46 insertions(+)

Comments

Dmitry Osipenko Aug. 19, 2017, 10:42 a.m. | #1
On 18.08.2017 19:15, Mikko Perttunen wrote:
> The gather filter is a feature present on Tegra124 and newer where the
> hardware prevents GATHERed command buffers from executing commands
> normally reserved for the CDMA pushbuffer which is maintained by the
> kernel driver.
> 
> This commit enables the gather filter on all supporting hardware.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---

TRM says that "Invalid Gbuffer cmd" interrupt would be raised when filtering
happens. Is that interrupt disabled by default or it would cause 'unhandled
interrupt'?
Mikko Perttunen Aug. 19, 2017, 10:46 a.m. | #2
On 08/19/2017 01:42 PM, Dmitry Osipenko wrote:
> On 18.08.2017 19:15, Mikko Perttunen wrote:
>> The gather filter is a feature present on Tegra124 and newer where the
>> hardware prevents GATHERed command buffers from executing commands
>> normally reserved for the CDMA pushbuffer which is maintained by the
>> kernel driver.
>>
>> This commit enables the gather filter on all supporting hardware.
>>
>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>> ---
> 
> TRM says that "Invalid Gbuffer cmd" interrupt would be raised when filtering
> happens. Is that interrupt disabled by default or it would cause 'unhandled
> interrupt'?
> 

It's disabled by default. Jobs that are stopped by the filter are then 
handled by the usual timeout mechanism.

Mikko
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Osipenko Aug. 19, 2017, 12:05 p.m. | #3
On 19.08.2017 13:46, Mikko Perttunen wrote:
> On 08/19/2017 01:42 PM, Dmitry Osipenko wrote:
>> On 18.08.2017 19:15, Mikko Perttunen wrote:
>>> The gather filter is a feature present on Tegra124 and newer where the
>>> hardware prevents GATHERed command buffers from executing commands
>>> normally reserved for the CDMA pushbuffer which is maintained by the
>>> kernel driver.
>>>
>>> This commit enables the gather filter on all supporting hardware.
>>>
>>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>>> ---
>>
>> TRM says that "Invalid Gbuffer cmd" interrupt would be raised when filtering
>> happens. Is that interrupt disabled by default or it would cause 'unhandled
>> interrupt'?
>>
> 
> It's disabled by default. Jobs that are stopped by the filter are then handled
> by the usual timeout mechanism.
> 

Alright, then it looks good to me.

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Dmitry Osipenko Aug. 20, 2017, 4:24 p.m. | #4
On 18.08.2017 19:15, Mikko Perttunen wrote:
> The gather filter is a feature present on Tegra124 and newer where the
> hardware prevents GATHERed command buffers from executing commands
> normally reserved for the CDMA pushbuffer which is maintained by the
> kernel driver.
> 
> This commit enables the gather filter on all supporting hardware.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  drivers/gpu/host1x/hw/channel_hw.c          | 22 ++++++++++++++++++++++
>  drivers/gpu/host1x/hw/hw_host1x04_channel.h | 12 ++++++++++++
>  drivers/gpu/host1x/hw/hw_host1x05_channel.h | 12 ++++++++++++
>  3 files changed, 46 insertions(+)
> 
> diff --git a/drivers/gpu/host1x/hw/channel_hw.c b/drivers/gpu/host1x/hw/channel_hw.c
> index 0161da331702..5c0dc6bb51d1 100644
> --- a/drivers/gpu/host1x/hw/channel_hw.c
> +++ b/drivers/gpu/host1x/hw/channel_hw.c
> @@ -181,10 +181,32 @@ static int channel_submit(struct host1x_job *job)
>  	return err;
>  }
>  
> +static void enable_gather_filter(struct host1x *host,
> +				 struct host1x_channel *ch)
> +{
> +#if HOST1X_HW >= 6
> +	u32 val;
> +
> +	if (!host->hv_regs)
> +		return;

Is it really possible that gather filter could be not present on HW without
hypervisor? Maybe there is other way to enable it in that case?

Is possible at all that hypervisor could be missed?

> +
> +	val = host1x_hypervisor_readl(
> +		host, HOST1X_HV_CH_KERNEL_FILTER_GBUFFER(ch->id / 32));
> +	val |= BIT(ch->id % 32);
> +	host1x_hypervisor_writel(
> +		host, val, HOST1X_HV_CH_KERNEL_FILTER_GBUFFER(ch->id / 32));
> +#elif HOST1X_HW >= 4
> +	host1x_ch_writel(ch,
> +			 HOST1X_CHANNEL_CHANNELCTRL_KERNEL_FILTER_GBUFFER(1),
> +			 HOST1X_CHANNEL_CHANNELCTRL);
> +#endif
> +}
> +
>  static int host1x_channel_init(struct host1x_channel *ch, struct host1x *dev,
>  			       unsigned int index)
>  {
>  	ch->regs = dev->regs + index * HOST1X_CHANNEL_SIZE;
> +	enable_gather_filter(dev, ch);
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/host1x/hw/hw_host1x04_channel.h b/drivers/gpu/host1x/hw/hw_host1x04_channel.h
> index 95e6f96142b9..2e8b635aa660 100644
> --- a/drivers/gpu/host1x/hw/hw_host1x04_channel.h
> +++ b/drivers/gpu/host1x/hw/hw_host1x04_channel.h
> @@ -117,5 +117,17 @@ static inline u32 host1x_channel_dmactrl_dmainitget(void)
>  }
>  #define HOST1X_CHANNEL_DMACTRL_DMAINITGET \
>  	host1x_channel_dmactrl_dmainitget()
> +static inline u32 host1x_channel_channelctrl_r(void)
> +{
> +	return 0x98;
> +}
> +#define HOST1X_CHANNEL_CHANNELCTRL \
> +	host1x_channel_channelctrl_r()
> +static inline u32 host1x_channel_channelctrl_kernel_filter_gbuffer_f(u32 v)
> +{
> +	return (v & 0x1) << 2;
> +}
> +#define HOST1X_CHANNEL_CHANNELCTRL_KERNEL_FILTER_GBUFFER(v) \
> +	host1x_channel_channelctrl_kernel_filter_gbuffer_f(v)
>  
>  #endif
> diff --git a/drivers/gpu/host1x/hw/hw_host1x05_channel.h b/drivers/gpu/host1x/hw/hw_host1x05_channel.h
> index fce6e2c1ff4c..abbbc2641ce6 100644
> --- a/drivers/gpu/host1x/hw/hw_host1x05_channel.h
> +++ b/drivers/gpu/host1x/hw/hw_host1x05_channel.h
> @@ -117,5 +117,17 @@ static inline u32 host1x_channel_dmactrl_dmainitget(void)
>  }
>  #define HOST1X_CHANNEL_DMACTRL_DMAINITGET \
>  	host1x_channel_dmactrl_dmainitget()
> +static inline u32 host1x_channel_channelctrl_r(void)
> +{
> +	return 0x98;
> +}
> +#define HOST1X_CHANNEL_CHANNELCTRL \
> +	host1x_channel_channelctrl_r()
> +static inline u32 host1x_channel_channelctrl_kernel_filter_gbuffer_f(u32 v)
> +{
> +	return (v & 0x1) << 2;
> +}
> +#define HOST1X_CHANNEL_CHANNELCTRL_KERNEL_FILTER_GBUFFER(v) \
> +	host1x_channel_channelctrl_kernel_filter_gbuffer_f(v)
>  
>  #endif
>
Dmitry Osipenko Aug. 20, 2017, 4:44 p.m. | #5
On 20.08.2017 19:24, Dmitry Osipenko wrote:
> On 18.08.2017 19:15, Mikko Perttunen wrote:
>> The gather filter is a feature present on Tegra124 and newer where the
>> hardware prevents GATHERed command buffers from executing commands
>> normally reserved for the CDMA pushbuffer which is maintained by the
>> kernel driver.
>>
>> This commit enables the gather filter on all supporting hardware.
>>
>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>> ---
>>  drivers/gpu/host1x/hw/channel_hw.c          | 22 ++++++++++++++++++++++
>>  drivers/gpu/host1x/hw/hw_host1x04_channel.h | 12 ++++++++++++
>>  drivers/gpu/host1x/hw/hw_host1x05_channel.h | 12 ++++++++++++
>>  3 files changed, 46 insertions(+)
>>
>> diff --git a/drivers/gpu/host1x/hw/channel_hw.c b/drivers/gpu/host1x/hw/channel_hw.c
>> index 0161da331702..5c0dc6bb51d1 100644
>> --- a/drivers/gpu/host1x/hw/channel_hw.c
>> +++ b/drivers/gpu/host1x/hw/channel_hw.c
>> @@ -181,10 +181,32 @@ static int channel_submit(struct host1x_job *job)
>>  	return err;
>>  }
>>  
>> +static void enable_gather_filter(struct host1x *host,
>> +				 struct host1x_channel *ch)
>> +{
>> +#if HOST1X_HW >= 6
>> +	u32 val;
>> +
>> +	if (!host->hv_regs)
>> +		return;
> 
> Is it really possible that gather filter could be not present on HW without
> hypervisor? Maybe there is other way to enable it in that case?
> 
> Is possible at all that hypervisor could be missed?

BTW, this is also incoherent with the 'syncpoint protection' patch which doesn't
check for hypervisor presence.
Dmitry Osipenko Aug. 20, 2017, 4:59 p.m. | #6
On 20.08.2017 19:44, Dmitry Osipenko wrote:
> On 20.08.2017 19:24, Dmitry Osipenko wrote:
>> On 18.08.2017 19:15, Mikko Perttunen wrote:
>>> The gather filter is a feature present on Tegra124 and newer where the
>>> hardware prevents GATHERed command buffers from executing commands
>>> normally reserved for the CDMA pushbuffer which is maintained by the
>>> kernel driver.
>>>
>>> This commit enables the gather filter on all supporting hardware.
>>>
>>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>>> ---
>>>  drivers/gpu/host1x/hw/channel_hw.c          | 22 ++++++++++++++++++++++
>>>  drivers/gpu/host1x/hw/hw_host1x04_channel.h | 12 ++++++++++++
>>>  drivers/gpu/host1x/hw/hw_host1x05_channel.h | 12 ++++++++++++
>>>  3 files changed, 46 insertions(+)
>>>
>>> diff --git a/drivers/gpu/host1x/hw/channel_hw.c b/drivers/gpu/host1x/hw/channel_hw.c
>>> index 0161da331702..5c0dc6bb51d1 100644
>>> --- a/drivers/gpu/host1x/hw/channel_hw.c
>>> +++ b/drivers/gpu/host1x/hw/channel_hw.c
>>> @@ -181,10 +181,32 @@ static int channel_submit(struct host1x_job *job)
>>>  	return err;
>>>  }
>>>  
>>> +static void enable_gather_filter(struct host1x *host,
>>> +				 struct host1x_channel *ch)
>>> +{
>>> +#if HOST1X_HW >= 6
>>> +	u32 val;
>>> +
>>> +	if (!host->hv_regs)
>>> +		return;
>>
>> Is it really possible that gather filter could be not present on HW without
>> hypervisor? Maybe there is other way to enable it in that case?
>>
>> Is possible at all that hypervisor could be missed?
> 
> BTW, this is also incoherent with the 'syncpoint protection' patch which doesn't
> check for hypervisor presence.
> 

However, I noticed that check and it's wrongly placed ;) See comment to the
'syncpoint protection' patch.
Mikko Perttunen Aug. 21, 2017, 5:27 p.m. | #7
On 08/20/2017 07:59 PM, Dmitry Osipenko wrote:
> On 20.08.2017 19:44, Dmitry Osipenko wrote:
>> On 20.08.2017 19:24, Dmitry Osipenko wrote:
>>> On 18.08.2017 19:15, Mikko Perttunen wrote:
>>>> The gather filter is a feature present on Tegra124 and newer where the
>>>> hardware prevents GATHERed command buffers from executing commands
>>>> normally reserved for the CDMA pushbuffer which is maintained by the
>>>> kernel driver.
>>>>
>>>> This commit enables the gather filter on all supporting hardware.
>>>>
>>>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>>>> ---
>>>>   drivers/gpu/host1x/hw/channel_hw.c          | 22 ++++++++++++++++++++++
>>>>   drivers/gpu/host1x/hw/hw_host1x04_channel.h | 12 ++++++++++++
>>>>   drivers/gpu/host1x/hw/hw_host1x05_channel.h | 12 ++++++++++++
>>>>   3 files changed, 46 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/host1x/hw/channel_hw.c b/drivers/gpu/host1x/hw/channel_hw.c
>>>> index 0161da331702..5c0dc6bb51d1 100644
>>>> --- a/drivers/gpu/host1x/hw/channel_hw.c
>>>> +++ b/drivers/gpu/host1x/hw/channel_hw.c
>>>> @@ -181,10 +181,32 @@ static int channel_submit(struct host1x_job *job)
>>>>   	return err;
>>>>   }
>>>>   
>>>> +static void enable_gather_filter(struct host1x *host,
>>>> +				 struct host1x_channel *ch)
>>>> +{
>>>> +#if HOST1X_HW >= 6
>>>> +	u32 val;
>>>> +
>>>> +	if (!host->hv_regs)
>>>> +		return;
>>>
>>> Is it really possible that gather filter could be not present on HW without
>>> hypervisor? Maybe there is other way to enable it in that case?

The hardware may have the hypervisor but Linux may be running as a 
virtual machine without access to the hypervisor, in which case we 
cannot access the registers, and it's the responsibility of the other OS 
acting as hypervisor to enable the gather filter.

>>>
>>> Is possible at all that hypervisor could be missed?
>>
>> BTW, this is also incoherent with the 'syncpoint protection' patch which doesn't
>> check for hypervisor presence.
>>
> 
> However, I noticed that check and it's wrongly placed ;) See comment to the
> 'syncpoint protection' patch.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mikko Perttunen Aug. 21, 2017, 5:28 p.m. | #8
On 08/21/2017 08:27 PM, Mikko Perttunen wrote:
> 
> 
> On 08/20/2017 07:59 PM, Dmitry Osipenko wrote:
>> On 20.08.2017 19:44, Dmitry Osipenko wrote:
>>> On 20.08.2017 19:24, Dmitry Osipenko wrote:
>>>> On 18.08.2017 19:15, Mikko Perttunen wrote:
>>>>> The gather filter is a feature present on Tegra124 and newer where the
>>>>> hardware prevents GATHERed command buffers from executing commands
>>>>> normally reserved for the CDMA pushbuffer which is maintained by the
>>>>> kernel driver.
>>>>>
>>>>> This commit enables the gather filter on all supporting hardware.
>>>>>
>>>>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>>>>> ---
>>>>>   drivers/gpu/host1x/hw/channel_hw.c          | 22 
>>>>> ++++++++++++++++++++++
>>>>>   drivers/gpu/host1x/hw/hw_host1x04_channel.h | 12 ++++++++++++
>>>>>   drivers/gpu/host1x/hw/hw_host1x05_channel.h | 12 ++++++++++++
>>>>>   3 files changed, 46 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/host1x/hw/channel_hw.c 
>>>>> b/drivers/gpu/host1x/hw/channel_hw.c
>>>>> index 0161da331702..5c0dc6bb51d1 100644
>>>>> --- a/drivers/gpu/host1x/hw/channel_hw.c
>>>>> +++ b/drivers/gpu/host1x/hw/channel_hw.c
>>>>> @@ -181,10 +181,32 @@ static int channel_submit(struct host1x_job 
>>>>> *job)
>>>>>       return err;
>>>>>   }
>>>>> +static void enable_gather_filter(struct host1x *host,
>>>>> +                 struct host1x_channel *ch)
>>>>> +{
>>>>> +#if HOST1X_HW >= 6
>>>>> +    u32 val;
>>>>> +
>>>>> +    if (!host->hv_regs)
>>>>> +        return;
>>>>
>>>> Is it really possible that gather filter could be not present on HW 
>>>> without
>>>> hypervisor? Maybe there is other way to enable it in that case?
> 
> The hardware may have the hypervisor but Linux may be running as a 
> virtual machine without access to the hypervisor, in which case we 
> cannot access the registers, and it's the responsibility of the other OS 
> acting as hypervisor to enable the gather filter.
> 
>>>>
>>>> Is possible at all that hypervisor could be missed?
>>>
>>> BTW, this is also incoherent with the 'syncpoint protection' patch 
>>> which doesn't
>>> check for hypervisor presence.
>>>
>>
>> However, I noticed that check and it's wrongly placed ;) See comment 
>> to the
>> 'syncpoint protection' patch.

Also - thanks, I added the missing check to that patch :)

>>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/gpu/host1x/hw/channel_hw.c b/drivers/gpu/host1x/hw/channel_hw.c
index 0161da331702..5c0dc6bb51d1 100644
--- a/drivers/gpu/host1x/hw/channel_hw.c
+++ b/drivers/gpu/host1x/hw/channel_hw.c
@@ -181,10 +181,32 @@  static int channel_submit(struct host1x_job *job)
 	return err;
 }
 
+static void enable_gather_filter(struct host1x *host,
+				 struct host1x_channel *ch)
+{
+#if HOST1X_HW >= 6
+	u32 val;
+
+	if (!host->hv_regs)
+		return;
+
+	val = host1x_hypervisor_readl(
+		host, HOST1X_HV_CH_KERNEL_FILTER_GBUFFER(ch->id / 32));
+	val |= BIT(ch->id % 32);
+	host1x_hypervisor_writel(
+		host, val, HOST1X_HV_CH_KERNEL_FILTER_GBUFFER(ch->id / 32));
+#elif HOST1X_HW >= 4
+	host1x_ch_writel(ch,
+			 HOST1X_CHANNEL_CHANNELCTRL_KERNEL_FILTER_GBUFFER(1),
+			 HOST1X_CHANNEL_CHANNELCTRL);
+#endif
+}
+
 static int host1x_channel_init(struct host1x_channel *ch, struct host1x *dev,
 			       unsigned int index)
 {
 	ch->regs = dev->regs + index * HOST1X_CHANNEL_SIZE;
+	enable_gather_filter(dev, ch);
 	return 0;
 }
 
diff --git a/drivers/gpu/host1x/hw/hw_host1x04_channel.h b/drivers/gpu/host1x/hw/hw_host1x04_channel.h
index 95e6f96142b9..2e8b635aa660 100644
--- a/drivers/gpu/host1x/hw/hw_host1x04_channel.h
+++ b/drivers/gpu/host1x/hw/hw_host1x04_channel.h
@@ -117,5 +117,17 @@  static inline u32 host1x_channel_dmactrl_dmainitget(void)
 }
 #define HOST1X_CHANNEL_DMACTRL_DMAINITGET \
 	host1x_channel_dmactrl_dmainitget()
+static inline u32 host1x_channel_channelctrl_r(void)
+{
+	return 0x98;
+}
+#define HOST1X_CHANNEL_CHANNELCTRL \
+	host1x_channel_channelctrl_r()
+static inline u32 host1x_channel_channelctrl_kernel_filter_gbuffer_f(u32 v)
+{
+	return (v & 0x1) << 2;
+}
+#define HOST1X_CHANNEL_CHANNELCTRL_KERNEL_FILTER_GBUFFER(v) \
+	host1x_channel_channelctrl_kernel_filter_gbuffer_f(v)
 
 #endif
diff --git a/drivers/gpu/host1x/hw/hw_host1x05_channel.h b/drivers/gpu/host1x/hw/hw_host1x05_channel.h
index fce6e2c1ff4c..abbbc2641ce6 100644
--- a/drivers/gpu/host1x/hw/hw_host1x05_channel.h
+++ b/drivers/gpu/host1x/hw/hw_host1x05_channel.h
@@ -117,5 +117,17 @@  static inline u32 host1x_channel_dmactrl_dmainitget(void)
 }
 #define HOST1X_CHANNEL_DMACTRL_DMAINITGET \
 	host1x_channel_dmactrl_dmainitget()
+static inline u32 host1x_channel_channelctrl_r(void)
+{
+	return 0x98;
+}
+#define HOST1X_CHANNEL_CHANNELCTRL \
+	host1x_channel_channelctrl_r()
+static inline u32 host1x_channel_channelctrl_kernel_filter_gbuffer_f(u32 v)
+{
+	return (v & 0x1) << 2;
+}
+#define HOST1X_CHANNEL_CHANNELCTRL_KERNEL_FILTER_GBUFFER(v) \
+	host1x_channel_channelctrl_kernel_filter_gbuffer_f(v)
 
 #endif