[PATCH 1/2] ARM: tegra: venice2: add GK20A GPU DT node

[PATCH 1/2] ARM: tegra: venice2: add GK20A GPU DT node

[Alexandre Courbot]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Add the device-tree node for the GK20A GPU and leave it disabled by
default. It is the responsability of the bootloader to enable it if the
VPR registers have been programmed such as the GPU can operate.

Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/tegra124-venice2.dts | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
index 13008858e967..cadaab74e649 100644
--- a/arch/arm/boot/dts/tegra124-venice2.dts
+++ b/arch/arm/boot/dts/tegra124-venice2.dts
@@ -42,6 +42,12 @@
 		};
 	};
 
+	gpu@0,57000000 {
+		status = "disabled";
+
+		vdd-supply = <&vdd_gpu>;
+	};
+
 	pinmux: pinmux@0,70000868 {
 		pinctrl-names = "boot";
 		pinctrl-0 = <&pinmux_boot>;
@@ -734,7 +740,7 @@
 					regulator-always-on;
 				};
 
-				sd6 {
+				vdd_gpu: sd6 {
 					regulator-name = "+VDD_GPU_AP";
 					regulator-min-microvolt = <650000>;
 					regulator-max-microvolt = <1200000>;
-- 
2.1.2

[PATCH 2/2] ARM: tegra: jetson-tk1: add GK20A GPU DT node

[Alexandre Courbot]

Add the device-tree node for the GK20A GPU and leave it disabled by
default. It is the responsability of the bootloader to enable it if the
VPR registers have been programmed such as the GPU can operate.

Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/tegra124-jetson-tk1.dts | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
index 029c9a021541..6f84c7e231a0 100644
--- a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
+++ b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
@@ -50,6 +50,12 @@
 		};
 	};
 
+	gpu@0,57000000 {
+		status = "disabled";
+
+		vdd-supply = <&vdd_gpu>;
+	};
+
 	pinmux: pinmux@0,70000868 {
 		pinctrl-names = "boot";
 		pinctrl-0 = <&state_boot>;
@@ -1560,7 +1566,7 @@
 					regulator-always-on;
 				};
 
-				sd6 {
+				vdd_gpu: sd6 {
 					regulator-name = "+VDD_GPU_AP";
 					regulator-min-microvolt = <650000>;
 					regulator-max-microvolt = <1200000>;
-- 
2.1.2

Re: [PATCH 1/2] ARM: tegra: venice2: add GK20A GPU DT node

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 776 bytes --]

On Thu, Oct 09, 2014 at 03:11:51PM +0900, Alexandre Courbot wrote:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> Add the device-tree node for the GK20A GPU and leave it disabled by
> default. It is the responsability of the bootloader to enable it if the
> VPR registers have been programmed such as the GPU can operate.
> 
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  arch/arm/boot/dts/tegra124-venice2.dts | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

Are you going to send the corresponding patches for U-Boot to update
the status property if it's initialized VPR?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/2] ARM: tegra: venice2: add GK20A GPU DT node

[Alexandre Courbot]

On 10/09/2014 04:43 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Thu, Oct 09, 2014 at 03:11:51PM +0900, Alexandre Courbot wrote:
>> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>
>> Add the device-tree node for the GK20A GPU and leave it disabled by
>> default. It is the responsability of the bootloader to enable it if the
>> VPR registers have been programmed such as the GPU can operate.
>>
>> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>>   arch/arm/boot/dts/tegra124-venice2.dts | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> Are you going to send the corresponding patches for U-Boot to update
> the status property if it's initialized VPR?

Yes. But maybe I should wait until upstream Nouveau actually works 
properly for that? Right now it will probe successfully, but will crash 
as soon as the GPU is used because there still are memory coherency 
problems.

Re: [PATCH 1/2] ARM: tegra: venice2: add GK20A GPU DT node

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]

On Thu, Oct 09, 2014 at 04:48:56PM +0900, Alexandre Courbot wrote:
> On 10/09/2014 04:43 PM, Thierry Reding wrote:
> >* PGP Signed by an unknown key
> >
> >On Thu, Oct 09, 2014 at 03:11:51PM +0900, Alexandre Courbot wrote:
> >>From: Thierry Reding <treding@nvidia.com>
> >>
> >>Add the device-tree node for the GK20A GPU and leave it disabled by
> >>default. It is the responsability of the bootloader to enable it if the
> >>VPR registers have been programmed such as the GPU can operate.
> >>
> >>Signed-off-by: Thierry Reding <treding@nvidia.com>
> >>Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> >>---
> >>  arch/arm/boot/dts/tegra124-venice2.dts | 8 +++++++-
> >>  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> >Are you going to send the corresponding patches for U-Boot to update
> >the status property if it's initialized VPR?
> 
> Yes. But maybe I should wait until upstream Nouveau actually works properly
> for that? Right now it will probe successfully, but will crash as soon as
> the GPU is used because there still are memory coherency problems.

I think the same really goes for these patches. Applying these will make
the node available to U-Boot, so technically some future U-Boot version
could modify a kernel DTB and boot a version where nouveau didn't yet
work out-of-the-box.

There's also still the issue about firmware loading and so on, so maybe
holding off on applying these patches until gk20a is fully enabled is a
good idea. Normally I guess this would be done by not adding the kernel
driver until it's expected to be at least usable to some degree, but I
guess it is a bit late for that now.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/2] ARM: tegra: venice2: add GK20A GPU DT node

[Alexandre Courbot]

On Thu, Oct 9, 2014 at 5:00 PM, Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Oct 09, 2014 at 04:48:56PM +0900, Alexandre Courbot wrote:
>> On 10/09/2014 04:43 PM, Thierry Reding wrote:
>> >* PGP Signed by an unknown key
>> >
>> >On Thu, Oct 09, 2014 at 03:11:51PM +0900, Alexandre Courbot wrote:
>> >>From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> >>
>> >>Add the device-tree node for the GK20A GPU and leave it disabled by
>> >>default. It is the responsability of the bootloader to enable it if the
>> >>VPR registers have been programmed such as the GPU can operate.
>> >>
>> >>Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> >>Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> >>---
>> >>  arch/arm/boot/dts/tegra124-venice2.dts | 8 +++++++-
>> >>  1 file changed, 7 insertions(+), 1 deletion(-)
>> >
>> >Are you going to send the corresponding patches for U-Boot to update
>> >the status property if it's initialized VPR?
>>
>> Yes. But maybe I should wait until upstream Nouveau actually works properly
>> for that? Right now it will probe successfully, but will crash as soon as
>> the GPU is used because there still are memory coherency problems.
>
> I think the same really goes for these patches. Applying these will make
> the node available to U-Boot, so technically some future U-Boot version
> could modify a kernel DTB and boot a version where nouveau didn't yet
> work out-of-the-box.
>
> There's also still the issue about firmware loading and so on, so maybe
> holding off on applying these patches until gk20a is fully enabled is a
> good idea. Normally I guess this would be done by not adding the kernel
> driver until it's expected to be at least usable to some degree, but I
> guess it is a bit late for that now.

I'm fine with that too. Dropping these patches fow now.

Re: [PATCH 1/2] ARM: tegra: venice2: add GK20A GPU DT node

[Stephen Warren]

On 10/08/2014 11:11 PM, Alexandre Courbot wrote:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> Add the device-tree node for the GK20A GPU and leave it disabled by
> default. It is the responsability of the bootloader to enable it if the
> VPR registers have been programmed such as the GPU can operate.

> diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts

> +	gpu@0,57000000 {
> +		status = "disabled";

status="disabled" usually goes into tegra124.dtsi. A board would only
override status to "okay" if necessary, and include any board specific
properties ...

> +		vdd-supply = <&vdd_gpu>;

... such as that.

Re: [PATCH 1/2] ARM: tegra: venice2: add GK20A GPU DT node

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

On Thu, Oct 09, 2014 at 10:10:29PM -0700, Stephen Warren wrote:
> On 10/08/2014 11:11 PM, Alexandre Courbot wrote:
> > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > 
> > Add the device-tree node for the GK20A GPU and leave it disabled by
> > default. It is the responsability of the bootloader to enable it if the
> > VPR registers have been programmed such as the GPU can operate.
> 
> > diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
> 
> > +	gpu@0,57000000 {
> > +		status = "disabled";
> 
> status="disabled" usually goes into tegra124.dtsi. A board would only
> override status to "okay" if necessary, and include any board specific
> properties ...

True. I guess this is somewhat redundant here since the .dtsi file
already sets it to "disabled". Given how it is rather unusual for a
board's DTS file to not enable a node I think there would be some
advantage in keeping this explicitly to avoid confusion. Alternatively
perhaps a comment in the DTS file about why this isn't enabled by
default would be a good replacement.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]