diff mbox

[v2,3/5] dt-bindings: Add documentation for GM20B GPU

Message ID 1458010724-10945-4-git-send-email-acourbot@nvidia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alexandre Courbot March 15, 2016, 2:58 a.m. UTC
GM20B's definition is mostly similar to GK20A's, but requires an
additional clock.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 .../devicetree/bindings/gpu/nvidia,gk20a.txt       | 27 ++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

Comments

Rob Herring (Arm) March 18, 2016, 8:47 p.m. UTC | #1
On Tue, Mar 15, 2016 at 11:58:42AM +0900, Alexandre Courbot wrote:
> GM20B's definition is mostly similar to GK20A's, but requires an
> additional clock.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  .../devicetree/bindings/gpu/nvidia,gk20a.txt       | 27 ++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

One nit below.

> 
> diff --git a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
> index 1e3748337319..d9ad6b87fbbc 100644
> --- a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
> +++ b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
> @@ -1,9 +1,10 @@
> -NVIDIA GK20A Graphics Processing Unit
> +NVIDIA Tegra Graphics Processing Units
>  
>  Required properties:
>  - compatible: "nvidia,<gpu>"
>    Currently recognized values:
>    - nvidia,gk20a
> +  - nvidia,gm20b
>  - reg: Physical base address and length of the controller's registers.
>    Must contain two entries:
>    - first entry for bar0
> @@ -19,6 +20,9 @@ Required properties:
>  - clock-names: Must include the following entries:
>    - gpu
>    - pwr
> +If the compatible string is "nvidia,gm20b", then the following clock
> +is also required:
> +  - ref
>  - resets: Must contain an entry for each entry in reset-names.
>    See ../reset/reset.txt for details.
>  - reset-names: Must include the following entries:
> @@ -27,7 +31,7 @@ Required properties:
>  Optional properties:
>  - iommus: A reference to the IOMMU. See ../iommu/iommu.txt for details.
>  
> -Example:
> +Example for GK20A:
>  
>  	gpu@0,57000000 {
>  		compatible = "nvidia,gk20a";
> @@ -45,3 +49,22 @@ Example:
>  		iommus = <&mc TEGRA_SWGROUP_GPU>;
>  		status = "disabled";
>  	};
> +
> +Example for GM20B:
> +
> +	gpu@0,57000000 {

Drop the comma and leading zero.

> +		compatible = "nvidia,gm20b";
> +		reg = <0x0 0x57000000 0x0 0x01000000>,
> +		      <0x0 0x58000000 0x0 0x01000000>;
> +		interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "stall", "nonstall";
> +		clocks = <&tegra_car TEGRA210_CLK_GPU>,
> +			 <&tegra_car TEGRA210_CLK_PLL_P_OUT5>,
> +			 <&tegra_car TEGRA210_CLK_PLL_G_REF>;
> +		clock-names = "gpu", "pwr", "ref";
> +		resets = <&tegra_car 184>;
> +		reset-names = "gpu";
> +		iommus = <&mc TEGRA_SWGROUP_GPU>;
> +		status = "disabled";
> +	};
> -- 
> 2.7.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexandre Courbot March 20, 2016, 6:55 a.m. UTC | #2
On Sat, Mar 19, 2016 at 5:47 AM, Rob Herring <robh@kernel.org> wrote:
> On Tue, Mar 15, 2016 at 11:58:42AM +0900, Alexandre Courbot wrote:
>> GM20B's definition is mostly similar to GK20A's, but requires an
>> additional clock.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>>  .../devicetree/bindings/gpu/nvidia,gk20a.txt       | 27 ++++++++++++++++++++--
>>  1 file changed, 25 insertions(+), 2 deletions(-)
>
> Acked-by: Rob Herring <robh@kernel.org>
>
> One nit below.
>
>>
>> diff --git a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
>> index 1e3748337319..d9ad6b87fbbc 100644
>> --- a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
>> +++ b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
>> @@ -1,9 +1,10 @@
>> -NVIDIA GK20A Graphics Processing Unit
>> +NVIDIA Tegra Graphics Processing Units
>>
>>  Required properties:
>>  - compatible: "nvidia,<gpu>"
>>    Currently recognized values:
>>    - nvidia,gk20a
>> +  - nvidia,gm20b
>>  - reg: Physical base address and length of the controller's registers.
>>    Must contain two entries:
>>    - first entry for bar0
>> @@ -19,6 +20,9 @@ Required properties:
>>  - clock-names: Must include the following entries:
>>    - gpu
>>    - pwr
>> +If the compatible string is "nvidia,gm20b", then the following clock
>> +is also required:
>> +  - ref
>>  - resets: Must contain an entry for each entry in reset-names.
>>    See ../reset/reset.txt for details.
>>  - reset-names: Must include the following entries:
>> @@ -27,7 +31,7 @@ Required properties:
>>  Optional properties:
>>  - iommus: A reference to the IOMMU. See ../iommu/iommu.txt for details.
>>
>> -Example:
>> +Example for GK20A:
>>
>>       gpu@0,57000000 {
>>               compatible = "nvidia,gk20a";
>> @@ -45,3 +49,22 @@ Example:
>>               iommus = <&mc TEGRA_SWGROUP_GPU>;
>>               status = "disabled";
>>       };
>> +
>> +Example for GM20B:
>> +
>> +     gpu@0,57000000 {
>
> Drop the comma and leading zero.

Even though this is how it appears in the actual DT?
Rob Herring (Arm) March 22, 2016, 1:41 a.m. UTC | #3
On Sun, Mar 20, 2016 at 1:55 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Sat, Mar 19, 2016 at 5:47 AM, Rob Herring <robh@kernel.org> wrote:
>> On Tue, Mar 15, 2016 at 11:58:42AM +0900, Alexandre Courbot wrote:
>>> GM20B's definition is mostly similar to GK20A's, but requires an
>>> additional clock.

[...]

>>>       gpu@0,57000000 {
>>>               compatible = "nvidia,gk20a";
>>> @@ -45,3 +49,22 @@ Example:
>>>               iommus = <&mc TEGRA_SWGROUP_GPU>;
>>>               status = "disabled";
>>>       };
>>> +
>>> +Example for GM20B:
>>> +
>>> +     gpu@0,57000000 {
>>
>> Drop the comma and leading zero.
>
> Even though this is how it appears in the actual DT?

Yes, those will need to get fixed, too.

Rob
Alexandre Courbot March 22, 2016, 4 a.m. UTC | #4
On 03/22/2016 10:41 AM, Rob Herring wrote:
> On Sun, Mar 20, 2016 at 1:55 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
>> On Sat, Mar 19, 2016 at 5:47 AM, Rob Herring <robh@kernel.org> wrote:
>>> On Tue, Mar 15, 2016 at 11:58:42AM +0900, Alexandre Courbot wrote:
>>>> GM20B's definition is mostly similar to GK20A's, but requires an
>>>> additional clock.
>
> [...]
>
>>>>        gpu@0,57000000 {
>>>>                compatible = "nvidia,gk20a";
>>>> @@ -45,3 +49,22 @@ Example:
>>>>                iommus = <&mc TEGRA_SWGROUP_GPU>;
>>>>                status = "disabled";
>>>>        };
>>>> +
>>>> +Example for GM20B:
>>>> +
>>>> +     gpu@0,57000000 {
>>>
>>> Drop the comma and leading zero.
>>
>> Even though this is how it appears in the actual DT?
>
> Yes, those will need to get fixed, too.

Sorry, I just want to confirm that I understand why this needs to be 
fixed. The parent node has #address-cells = <2>, and the practice of 
specifying two cells in the node name is consistent with what I see in 
http://www.devicetree.org/Device_Tree_Usage.

However in the device tree usage example one can interpret the two cells 
as being two different components of the address, whereas in our case we 
are using two cells because the address is 64-bit - hence we should 
specify it in the name as a single entity. Is this correct?

Thanks,
Alex.
Rob Herring (Arm) March 22, 2016, 12:44 p.m. UTC | #5
On Mon, Mar 21, 2016 at 11:00 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> On 03/22/2016 10:41 AM, Rob Herring wrote:
>>
>> On Sun, Mar 20, 2016 at 1:55 AM, Alexandre Courbot <gnurou@gmail.com>
>> wrote:
>>>
>>> On Sat, Mar 19, 2016 at 5:47 AM, Rob Herring <robh@kernel.org> wrote:
>>>>
>>>> On Tue, Mar 15, 2016 at 11:58:42AM +0900, Alexandre Courbot wrote:
>>>>>
>>>>> GM20B's definition is mostly similar to GK20A's, but requires an
>>>>> additional clock.
>>
>>
>> [...]
>>
>>>>>        gpu@0,57000000 {
>>>>>                compatible = "nvidia,gk20a";
>>>>> @@ -45,3 +49,22 @@ Example:
>>>>>                iommus = <&mc TEGRA_SWGROUP_GPU>;
>>>>>                status = "disabled";
>>>>>        };
>>>>> +
>>>>> +Example for GM20B:
>>>>> +
>>>>> +     gpu@0,57000000 {
>>>>
>>>>
>>>> Drop the comma and leading zero.
>>>
>>>
>>> Even though this is how it appears in the actual DT?
>>
>>
>> Yes, those will need to get fixed, too.
>
>
> Sorry, I just want to confirm that I understand why this needs to be fixed.
> The parent node has #address-cells = <2>, and the practice of specifying two
> cells in the node name is consistent with what I see in
> http://www.devicetree.org/Device_Tree_Usage.
>
> However in the device tree usage example one can interpret the two cells as
> being two different components of the address, whereas in our case we are
> using two cells because the address is 64-bit - hence we should specify it
> in the name as a single entity. Is this correct?

Exactly, commas are for separating distinct fields like chip select
and offset in the wiki example. A 64-bit address is a single field.

The other reason to fix it is dtc is going to start warning for this.

Rob
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
index 1e3748337319..d9ad6b87fbbc 100644
--- a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
+++ b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
@@ -1,9 +1,10 @@ 
-NVIDIA GK20A Graphics Processing Unit
+NVIDIA Tegra Graphics Processing Units
 
 Required properties:
 - compatible: "nvidia,<gpu>"
   Currently recognized values:
   - nvidia,gk20a
+  - nvidia,gm20b
 - reg: Physical base address and length of the controller's registers.
   Must contain two entries:
   - first entry for bar0
@@ -19,6 +20,9 @@  Required properties:
 - clock-names: Must include the following entries:
   - gpu
   - pwr
+If the compatible string is "nvidia,gm20b", then the following clock
+is also required:
+  - ref
 - resets: Must contain an entry for each entry in reset-names.
   See ../reset/reset.txt for details.
 - reset-names: Must include the following entries:
@@ -27,7 +31,7 @@  Required properties:
 Optional properties:
 - iommus: A reference to the IOMMU. See ../iommu/iommu.txt for details.
 
-Example:
+Example for GK20A:
 
 	gpu@0,57000000 {
 		compatible = "nvidia,gk20a";
@@ -45,3 +49,22 @@  Example:
 		iommus = <&mc TEGRA_SWGROUP_GPU>;
 		status = "disabled";
 	};
+
+Example for GM20B:
+
+	gpu@0,57000000 {
+		compatible = "nvidia,gm20b";
+		reg = <0x0 0x57000000 0x0 0x01000000>,
+		      <0x0 0x58000000 0x0 0x01000000>;
+		interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "stall", "nonstall";
+		clocks = <&tegra_car TEGRA210_CLK_GPU>,
+			 <&tegra_car TEGRA210_CLK_PLL_P_OUT5>,
+			 <&tegra_car TEGRA210_CLK_PLL_G_REF>;
+		clock-names = "gpu", "pwr", "ref";
+		resets = <&tegra_car 184>;
+		reset-names = "gpu";
+		iommus = <&mc TEGRA_SWGROUP_GPU>;
+		status = "disabled";
+	};