diff mbox series

[08/38] dt-bindings: display: tegra: Document interconnect paths

Message ID 20200612141903.2391044-9-thierry.reding@gmail.com
State Changes Requested
Headers show
Series dt-bindings: json-schema conversions and cleanups | expand

Checks

Context Check Description
robh/checkpatch success
robh/checkpatch success

Commit Message

Thierry Reding June 12, 2020, 2:18 p.m. UTC
From: Thierry Reding <treding@nvidia.com>

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 .../display/tegra/nvidia,tegra20-host1x.yaml  | 52 ++++++++++++++++---
 1 file changed, 46 insertions(+), 6 deletions(-)

Comments

Dmitry Osipenko June 12, 2020, 3:52 p.m. UTC | #1
Hello Thierry,

12.06.2020 17:18, Thierry Reding пишет:
> From: Thierry Reding <treding@nvidia.com>
Commit description is missing, checkpatch should warn about it.

> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  .../display/tegra/nvidia,tegra20-host1x.yaml  | 52 ++++++++++++++++---
>  1 file changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
> index 3347e1b3c8f0..684fe25641f1 100644
> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
> @@ -97,8 +97,17 @@ properties:
>    iommus:
>      $ref: "/schemas/iommu/iommu.yaml#/properties/iommus"
>  
> -  memory-controllers:
> -    $ref: /schemas/types.yaml#/definitions/phandle-array

Why memory-controllers property is removed?

> +  interconnects:
> +    description: Description of the interconnect paths for the host1x
> +      controller; see ../interconnect/interconnect.txt for details.
> +    items:
> +      - description: memory read client for host1x
> +
> +  interconnect-names:
> +    description: A list of names identifying each entry listed in the
> +      "interconnects" property.
> +    items:
> +      - const: dma-mem # read

Please notice that Host1x has two memory clients: one for DMA engine and
second I don't know what's for, maybe for indirect memory accesses. Why
you skipped the second path?

>  required:
>    - compatible
> @@ -489,6 +498,26 @@ allOf:
>              iommus:
>                $ref: "/schemas/types.yaml#/definitions/phandle-array"
>  
> +            #interconnects:
> +            #  items:
> +            #    - description: memory read client for window A
> +            #    - description: memory read client for window B
> +            #    - description: memory read client for window C
> +            #    - description: memory read client for cursor
> +            #    # disp only
> +            #    - description: memory read client for window T
> +            #    - description: memory read client for window D
> +
> +            #interconnect-names:
> +            #  items:
> +            #    - const: wina
> +            #    - const: winb
> +            #    - const: winc
> +            #    - const: cursor
> +            #    # disp only
> +            #    - const: wint
> +            #    - const: wind

Is this really intended to be commented out? Looks like this is an
unfinished patch.

In the patch [1] I used memory client names for the interconnect paths.
I like yours variant of the naming, it is more intuitive.

[1]
https://patchwork.ozlabs.org/project/linux-tegra/patch/20200609131404.17523-23-digetx@gmail.com/

I'll rebase my series on top of yours patches once you'll get them into
linux-next. Looking forward to v2!
Thierry Reding June 16, 2020, 2:47 p.m. UTC | #2
On Fri, Jun 12, 2020 at 06:52:44PM +0300, Dmitry Osipenko wrote:
> Hello Thierry,
> 
> 12.06.2020 17:18, Thierry Reding пишет:
> > From: Thierry Reding <treding@nvidia.com>
> Commit description is missing, checkpatch should warn about it.
> 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  .../display/tegra/nvidia,tegra20-host1x.yaml  | 52 ++++++++++++++++---
> >  1 file changed, 46 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
> > index 3347e1b3c8f0..684fe25641f1 100644
> > --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
> > +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
> > @@ -97,8 +97,17 @@ properties:
> >    iommus:
> >      $ref: "/schemas/iommu/iommu.yaml#/properties/iommus"
> >  
> > -  memory-controllers:
> > -    $ref: /schemas/types.yaml#/definitions/phandle-array
> 
> Why memory-controllers property is removed?

Because it was accidentally added in patch 7. This is leftover from an
earlier set of patches.

> > +  interconnects:
> > +    description: Description of the interconnect paths for the host1x
> > +      controller; see ../interconnect/interconnect.txt for details.
> > +    items:
> > +      - description: memory read client for host1x
> > +
> > +  interconnect-names:
> > +    description: A list of names identifying each entry listed in the
> > +      "interconnects" property.
> > +    items:
> > +      - const: dma-mem # read
> 
> Please notice that Host1x has two memory clients: one for DMA engine and
> second I don't know what's for, maybe for indirect memory accesses. Why
> you skipped the second path?

I'm primarily targetting Tegra186 and Tegra194 with these patches
because we need the interconnects properties on those platforms in order
to correctly set the DMA masks at a global scope. For Tegra186 and
Tegra194 I can only see a single memory client for host1x.

Looking at the register documentation, the host1xw and host1xr clients
do exist on Tegra210 and earlier and they are used for indirect writes
and reads, respectively.

I don't think we use indirect reads/writes, so we can probably do
without the corresponding memory clients. Alternatively we could
conditionally add them on Tegra210 and earlier.

Thierry

> >  required:
> >    - compatible
> > @@ -489,6 +498,26 @@ allOf:
> >              iommus:
> >                $ref: "/schemas/types.yaml#/definitions/phandle-array"
> >  
> > +            #interconnects:
> > +            #  items:
> > +            #    - description: memory read client for window A
> > +            #    - description: memory read client for window B
> > +            #    - description: memory read client for window C
> > +            #    - description: memory read client for cursor
> > +            #    # disp only
> > +            #    - description: memory read client for window T
> > +            #    - description: memory read client for window D
> > +
> > +            #interconnect-names:
> > +            #  items:
> > +            #    - const: wina
> > +            #    - const: winb
> > +            #    - const: winc
> > +            #    - const: cursor
> > +            #    # disp only
> > +            #    - const: wint
> > +            #    - const: wind
> 
> Is this really intended to be commented out? Looks like this is an
> unfinished patch.

I started adding this after finishing Tegra186 and Tegra194, but then I
ran into various other issues and wanted to do this step by step, so I
plan to go back to Tegra210 and earlier later on and stub them out.

However, since I had already done the research I wanted to leave these
in here for later reference, so I didn't have to go back and look this
up again.

> In the patch [1] I used memory client names for the interconnect paths.
> I like yours variant of the naming, it is more intuitive.

I only noticed after posting this series that you had sent out the
series for interconnects and that it conflicted in the bindings.

But yeah, I do also like the contextual names better because they are
more intuitive.

> [1]
> https://patchwork.ozlabs.org/project/linux-tegra/patch/20200609131404.17523-23-digetx@gmail.com/
> 
> I'll rebase my series on top of yours patches once you'll get them into
> linux-next. Looking forward to v2!

There's quite a bit to review here, but I also have a couple of patches
that depend on these, so yeah, it'd be great if this could go in sooner
rather than later.

Thierry
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
index 3347e1b3c8f0..684fe25641f1 100644
--- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
+++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
@@ -97,8 +97,17 @@  properties:
   iommus:
     $ref: "/schemas/iommu/iommu.yaml#/properties/iommus"
 
-  memory-controllers:
-    $ref: /schemas/types.yaml#/definitions/phandle-array
+  interconnects:
+    description: Description of the interconnect paths for the host1x
+      controller; see ../interconnect/interconnect.txt for details.
+    items:
+      - description: memory read client for host1x
+
+  interconnect-names:
+    description: A list of names identifying each entry listed in the
+      "interconnects" property.
+    items:
+      - const: dma-mem # read
 
 required:
   - compatible
@@ -489,6 +498,26 @@  allOf:
             iommus:
               $ref: "/schemas/types.yaml#/definitions/phandle-array"
 
+            #interconnects:
+            #  items:
+            #    - description: memory read client for window A
+            #    - description: memory read client for window B
+            #    - description: memory read client for window C
+            #    - description: memory read client for cursor
+            #    # disp only
+            #    - description: memory read client for window T
+            #    - description: memory read client for window D
+
+            #interconnect-names:
+            #  items:
+            #    - const: wina
+            #    - const: winb
+            #    - const: winc
+            #    - const: cursor
+            #    # disp only
+            #    - const: wint
+            #    - const: wind
+
             nvidia,head:
               description: The number of the display controller head. This is
                 used to setup the various types of output to receive video
@@ -634,8 +663,15 @@  allOf:
                     master interface of this display controller.
                   $ref: "/schemas/types.yaml#/definitions/phandle-array"
 
-                memory-controllers:
-                  $ref: /schemas/types.yaml#/definitions/phandle-array
+                interconnects:
+                  description: Description of the interconnect paths for the
+                    display controller; see ../interconnect/interconnect.txt
+                    for details.
+
+                interconnect-names:
+                  items:
+                    - const: dma-mem # read-0
+                    - const: read-1
 
                 nvidia,outputs:
                   description: A list of phandles of outputs that this display
@@ -1027,8 +1063,12 @@  allOf:
             iommus:
               $ref: "/schemas/types.yaml#/definitions/phandle-array"
 
-            memory-controllers:
-              $ref: /schemas/types.yaml#/definitions/phandle-array
+            interconnects: true
+
+            interconnect-names:
+              items:
+                - const: dma-mem # read
+                - const: write
 
           unevaluatedProperties: false