[v3,1/8] dt-bindings: display: Add bindings for LVDS bus-timings
diff mbox series

Message ID 1567017402-5895-2-git-send-email-fabrizio.castro@bp.renesas.com
State New
Delegated to: Kieran Bingham
Headers show
Series
  • Add dual-LVDS panel support to EK874
Related show

Commit Message

Fabrizio Castro Aug. 28, 2019, 6:36 p.m. UTC
Dual-LVDS connections need markers in the DT, this patch adds
some common documentation to be referenced by both panels and
bridges.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

---
v2->v3:
* new patch
---
 .../bindings/display/bus-timings/lvds.yaml         | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bus-timings/lvds.yaml

Comments

Geert Uytterhoeven Aug. 29, 2019, 7:57 a.m. UTC | #1
Hi Fabrizio,

On Wed, Aug 28, 2019 at 8:36 PM Fabrizio Castro
<fabrizio.castro@bp.renesas.com> wrote:
> Dual-LVDS connections need markers in the DT, this patch adds
> some common documentation to be referenced by both panels and
> bridges.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bus-timings/lvds.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common Properties for bus timings of LVDS interfaces
> +
> +maintainers:
> +  - Thierry Reding <thierry.reding@gmail.com>
> +  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> +
> +description: |
> +  This document defines device tree properties common to LVDS and dual-LVDS
> +  interfaces, where a dual-LVDS interface is a dual-link connection with even
> +  pixels traveling on one connection, and with odd pixels traveling on the other
> +  connection.
> +  This document doesn't constitue a device tree binding specification by itself
> +  but is meant to be referenced by device tree bindings.
> +  When referenced from panel or bridge device tree bindings, the properties
> +  defined in this document are defined as follows. The panel and bridge device
> +  tree bindings are responsible for defining whether each property is required
> +  or optional.
> +
> +properties:
> +  dual-lvds-even-pixels:
> +    type: boolean
> +    description:
> +      This property is specific to an input port of a sink device. When
> +      specified, it marks the port as recipient of even-pixels.
> +
> +  dual-lvds-odd-pixels:
> +    type: boolean
> +    description:
> +      This property is specific to an input port of a sink device. When
> +      specified, it marks the port as recipient of odd-pixels.

Do you need the "dual-" prefix? Isn't that implied by even/odd?
Or is it better to keep it, for readability?

I'm also thinking about a possible future extension to triple or quad LVDS.
As I'm not aware of English word equivalents of even/odd for triple/quad,
perhaps this should be specified using a numerical value instead?

If I go too far, please just say so ;-)

Gr{oetje,eeting}s,

                        Geert
Fabrizio Castro Aug. 29, 2019, 9:14 a.m. UTC | #2
Hi Geert,

Thank you for your feedback!

> From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Geert Uytterhoeven
> Sent: 29 August 2019 08:58
> Subject: Re: [PATCH v3 1/8] dt-bindings: display: Add bindings for LVDS bus-timings
> 
> Hi Fabrizio,
> 
> On Wed, Aug 28, 2019 at 8:36 PM Fabrizio Castro
> <fabrizio.castro@bp.renesas.com> wrote:
> > Dual-LVDS connections need markers in the DT, this patch adds
> > some common documentation to be referenced by both panels and
> > bridges.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> > @@ -0,0 +1,38 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/bus-timings/lvds.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Common Properties for bus timings of LVDS interfaces
> > +
> > +maintainers:
> > +  - Thierry Reding <thierry.reding@gmail.com>
> > +  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > +
> > +description: |
> > +  This document defines device tree properties common to LVDS and dual-LVDS
> > +  interfaces, where a dual-LVDS interface is a dual-link connection with even
> > +  pixels traveling on one connection, and with odd pixels traveling on the other
> > +  connection.
> > +  This document doesn't constitue a device tree binding specification by itself
> > +  but is meant to be referenced by device tree bindings.
> > +  When referenced from panel or bridge device tree bindings, the properties
> > +  defined in this document are defined as follows. The panel and bridge device
> > +  tree bindings are responsible for defining whether each property is required
> > +  or optional.
> > +
> > +properties:
> > +  dual-lvds-even-pixels:
> > +    type: boolean
> > +    description:
> > +      This property is specific to an input port of a sink device. When
> > +      specified, it marks the port as recipient of even-pixels.
> > +
> > +  dual-lvds-odd-pixels:
> > +    type: boolean
> > +    description:
> > +      This property is specific to an input port of a sink device. When
> > +      specified, it marks the port as recipient of odd-pixels.
> 
> Do you need the "dual-" prefix? Isn't that implied by even/odd?
> Or is it better to keep it, for readability?

I decided to go with "dual-lvds-even-pixels" and "dual-lvds-odd-pixels"
because the "dual-lvds" prefix uniquely identifies the type of bus, and I
decided to go with the "pixels" suffix because "dual-lvds-odd" just doesn't
sound right. I guess "dual-lvds-even-pixels" and "dual-lvds-odd-pixels"
are the most readable and future proof labels I could think of, but maybe
there is something better we can do? Laurent?

> 
> I'm also thinking about a possible future extension to triple or quad LVDS.
> As I'm not aware of English word equivalents of even/odd for triple/quad,
> perhaps this should be specified using a numerical value instead?

I would have to see a use case for other LVDS configurations for providing
a proper answer to this question, but perhaps we could accept that other
configurations for LVDS connections may come with labels that are tailored
to help uniquely identifying the ports while being readable? Perhaps numerical
values would work better in other cases? Laurent?

> 
> If I go too far, please just say so ;-)

Definitely worth discussing!

Thanks,
Fab

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
Rob Herring Aug. 29, 2019, 2:03 p.m. UTC | #3
On Wed, Aug 28, 2019 at 1:36 PM Fabrizio Castro
<fabrizio.castro@bp.renesas.com> wrote:
>
> Dual-LVDS connections need markers in the DT, this patch adds
> some common documentation to be referenced by both panels and
> bridges.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
>
> ---
> v2->v3:
> * new patch
> ---
>  .../bindings/display/bus-timings/lvds.yaml         | 38 ++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> new file mode 100644
> index 0000000..f35b55a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: GPL-2.0

(GPL-2.0-only OR BSD-2-Clause) is preferred for new bindings.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bus-timings/lvds.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common Properties for bus timings of LVDS interfaces
> +
> +maintainers:
> +  - Thierry Reding <thierry.reding@gmail.com>
> +  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> +
> +description: |
> +  This document defines device tree properties common to LVDS and dual-LVDS
> +  interfaces, where a dual-LVDS interface is a dual-link connection with even
> +  pixels traveling on one connection, and with odd pixels traveling on the other
> +  connection.
> +  This document doesn't constitue a device tree binding specification by itself

typo: constitute

> +  but is meant to be referenced by device tree bindings.
> +  When referenced from panel or bridge device tree bindings, the properties
> +  defined in this document are defined as follows. The panel and bridge device
> +  tree bindings are responsible for defining whether each property is required
> +  or optional.
> +
> +properties:
> +  dual-lvds-even-pixels:
> +    type: boolean
> +    description:
> +      This property is specific to an input port of a sink device. When

The schema should define what nodes these go in. The description seems
to indicate in 'port' nodes (or endpoint?), but your use in the panel
binding puts them in the parent.

> +      specified, it marks the port as recipient of even-pixels.
> +
> +  dual-lvds-odd-pixels:
> +    type: boolean
> +    description:
> +      This property is specific to an input port of a sink device. When
> +      specified, it marks the port as recipient of odd-pixels.

However, I don't think you even need these. A panel's port numbers are
fixed can imply even or odd. For example port@0 can be even and port@1
can be odd. The port numbering is typically panel specific, but we may
be able to define the numbering generically if we don't already have
panels with multiple ports.

Also, aren't there dual link DSI panels?

Rob
Fabrizio Castro Aug. 29, 2019, 2:38 p.m. UTC | #4
Hi Rob,

Thank you for your feedback!

> From: Rob Herring <robh+dt@kernel.org>
> Sent: 29 August 2019 15:03
> Subject: Re: [PATCH v3 1/8] dt-bindings: display: Add bindings for LVDS bus-timings
> 
> On Wed, Aug 28, 2019 at 1:36 PM Fabrizio Castro
> <fabrizio.castro@bp.renesas.com> wrote:
> >
> > Dual-LVDS connections need markers in the DT, this patch adds
> > some common documentation to be referenced by both panels and
> > bridges.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > ---
> > v2->v3:
> > * new patch
> > ---
> >  .../bindings/display/bus-timings/lvds.yaml         | 38 ++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml b/Documentation/devicetree/bindings/display/bus-
> timings/lvds.yaml
> > new file mode 100644
> > index 0000000..f35b55a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> > @@ -0,0 +1,38 @@
> > +# SPDX-License-Identifier: GPL-2.0
> 
> (GPL-2.0-only OR BSD-2-Clause) is preferred for new bindings.
> 
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/bus-timings/lvds.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Common Properties for bus timings of LVDS interfaces
> > +
> > +maintainers:
> > +  - Thierry Reding <thierry.reding@gmail.com>
> > +  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > +
> > +description: |
> > +  This document defines device tree properties common to LVDS and dual-LVDS
> > +  interfaces, where a dual-LVDS interface is a dual-link connection with even
> > +  pixels traveling on one connection, and with odd pixels traveling on the other
> > +  connection.
> > +  This document doesn't constitue a device tree binding specification by itself
> 
> typo: constitute

Well spotted!

> 
> > +  but is meant to be referenced by device tree bindings.
> > +  When referenced from panel or bridge device tree bindings, the properties
> > +  defined in this document are defined as follows. The panel and bridge device
> > +  tree bindings are responsible for defining whether each property is required
> > +  or optional.
> > +
> > +properties:
> > +  dual-lvds-even-pixels:
> > +    type: boolean
> > +    description:
> > +      This property is specific to an input port of a sink device. When
> 
> The schema should define what nodes these go in. The description seems
> to indicate in 'port' nodes (or endpoint?), but your use in the panel
> binding puts them in the parent.

Did you manage to read this?
https://patchwork.kernel.org/cover/11119607/

Could you please advice on how to do this properly?

> 
> > +      specified, it marks the port as recipient of even-pixels.
> > +
> > +  dual-lvds-odd-pixels:
> > +    type: boolean
> > +    description:
> > +      This property is specific to an input port of a sink device. When
> > +      specified, it marks the port as recipient of odd-pixels.
> 
> However, I don't think you even need these. A panel's port numbers are
> fixed can imply even or odd. For example port@0 can be even and port@1
> can be odd. The port numbering is typically panel specific, but we may
> be able to define the numbering generically if we don't already have
> panels with multiple ports.
> 
> Also, aren't there dual link DSI panels?

This is the result of a discussion on here:
https://patchwork.kernel.org/patch/11095547/

Have you come across it?

Thanks!
Fab

> 
> Rob
Laurent Pinchart Nov. 7, 2019, 6 p.m. UTC | #5
Hello Fabrizio,

On Thu, Aug 29, 2019 at 02:38:06PM +0000, Fabrizio Castro wrote:
> On 29 August 2019 15:03 Rob Herring wrote:
> > On Wed, Aug 28, 2019 at 1:36 PM Fabrizio Castro wrote:
> >>
> >> Dual-LVDS connections need markers in the DT, this patch adds
> >> some common documentation to be referenced by both panels and
> >> bridges.
> >>
> >> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >>
> >> ---
> >> v2->v3:
> >> * new patch
> >> ---
> >>  .../bindings/display/bus-timings/lvds.yaml         | 38 ++++++++++++++++++++++
> >>  1 file changed, 38 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> >> new file mode 100644
> >> index 0000000..f35b55a
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> >> @@ -0,0 +1,38 @@
> >> +# SPDX-License-Identifier: GPL-2.0
> > 
> > (GPL-2.0-only OR BSD-2-Clause) is preferred for new bindings.
> > 
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/display/bus-timings/lvds.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Common Properties for bus timings of LVDS interfaces
> >> +
> >> +maintainers:
> >> +  - Thierry Reding <thierry.reding@gmail.com>
> >> +  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >> +
> >> +description: |
> >> +  This document defines device tree properties common to LVDS and dual-LVDS
> >> +  interfaces, where a dual-LVDS interface is a dual-link connection with even
> >> +  pixels traveling on one connection, and with odd pixels traveling on the other
> >> +  connection.

As you define a dual-LVDS interface as "a dual-link connection", should
this be "even pixels traveling on one link, and with odd pixels
traveling on the other link" ?

> >> +  This document doesn't constitue a device tree binding specification by itself
> > 
> > typo: constitute
> 
> Well spotted!
> 
> >> +  but is meant to be referenced by device tree bindings.
> >> +  When referenced from panel or bridge device tree bindings, the properties
> >> +  defined in this document are defined as follows. The panel and bridge device
> >> +  tree bindings are responsible for defining whether each property is required
> >> +  or optional.
> >> +
> >> +properties:
> >> +  dual-lvds-even-pixels:
> >> +    type: boolean
> >> +    description:
> >> +      This property is specific to an input port of a sink device. When
> > 
> > The schema should define what nodes these go in. The description seems
> > to indicate in 'port' nodes (or endpoint?), but your use in the panel
> > binding puts them in the parent.
> 
> Did you manage to read this?
> https://patchwork.kernel.org/cover/11119607/
> 
> Could you please advice on how to do this properly?
> 
> >> +      specified, it marks the port as recipient of even-pixels.
> >> +
> >> +  dual-lvds-odd-pixels:
> >> +    type: boolean
> >> +    description:
> >> +      This property is specific to an input port of a sink device. When
> >> +      specified, it marks the port as recipient of odd-pixels.
> > 
> > However, I don't think you even need these. A panel's port numbers are
> > fixed can imply even or odd. For example port@0 can be even and port@1
> > can be odd. The port numbering is typically panel specific, but we may
> > be able to define the numbering generically if we don't already have
> > panels with multiple ports.
> > 
> > Also, aren't there dual link DSI panels?
> 
> This is the result of a discussion on here:
> https://patchwork.kernel.org/patch/11095547/
> 
> Have you come across it?

Let me repeat my comments from that thread for Rob in order to
centralize the discussion here.

> Taking one step back to look at the big picture, what we need is for the
> source to know what pixel (odd or even) to send on each LVDS output. For
> that it needs to know what pixel is expected by the sink the the inputs
> connected to the source's outputs. From each source output we can easily
> locate the DT nodes corresponding to the connected sink's input ports,
> but that doesn't give us enough information. From there, we could
> 
> - Infer the odd/even pixel expected by the source based on the port
>   numbers. This would require common DT bindings for all dual-link LVDS
>   sinks that specify the port ordering (for instance the bindings could
>   mandate that lowest numbered port correspond to even pixels).
> 
> - Read the odd/even pixel expected by the source from an explicit DT
>   property, as proposed above. This would also require common DT
>   bindings for all dual-link LVDS sinks that define these new
>   properties. This would I think be better than implicitly infering it
>   from DT port numbers.
> 
> - Retrieve the information from the sink drm_bridge at runtime. This
>   would require a way to query the bridge for the mapping from port
>   number to odd/even pixel. The DRM_LINK_DUAL_LVDS_ODD_EVEN flag could
>   be used for that, and would then be defined as "the lowest numbered
>   port corresponds to odd pixels and the highest numbered port
>   corresponds to even pixels".
> 
> The second and third options would both work I think. The third one is
> roughly what you've implemented, except that I think the flag
> description should be clarified.

Rob, what's your opinion ? We could certainly, in the context of a
panel, decide of a fixed mapping of port numbers to odd/even pixels, but
the issue is that sources need to know which pixels to send on which
link (assuming of course that this can be configured on the source
side). We thus need a way for the source to answer, at runtime, the
question "which of ports A and B of the sink correspond to even and odd
pixels ?". This can't be inferred by the source from the sink port
numbers, as the mapping between port number and odd/even pixels is
specific to each sink. We thus need to either store that property in the
DT node of the sink (option 2) or query it at runtime from the sink
(option 3).

I like option 2 as it's more explicit, but option 3 minimizes the
required properties in DT, which is always good. Patch 3/8 introduces a
helper that abstracts this from a sink point of view (which I think is a
very good idea), so once we decide which option to use, only 3/8 may
need to be adapted, the other patches should hopefully not require
rework.

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
new file mode 100644
index 0000000..f35b55a
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
@@ -0,0 +1,38 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bus-timings/lvds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common Properties for bus timings of LVDS interfaces
+
+maintainers:
+  - Thierry Reding <thierry.reding@gmail.com>
+  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
+
+description: |
+  This document defines device tree properties common to LVDS and dual-LVDS
+  interfaces, where a dual-LVDS interface is a dual-link connection with even
+  pixels traveling on one connection, and with odd pixels traveling on the other
+  connection.
+  This document doesn't constitue a device tree binding specification by itself
+  but is meant to be referenced by device tree bindings.
+  When referenced from panel or bridge device tree bindings, the properties
+  defined in this document are defined as follows. The panel and bridge device
+  tree bindings are responsible for defining whether each property is required
+  or optional.
+
+properties:
+  dual-lvds-even-pixels:
+    type: boolean
+    description:
+      This property is specific to an input port of a sink device. When
+      specified, it marks the port as recipient of even-pixels.
+
+  dual-lvds-odd-pixels:
+    type: boolean
+    description:
+      This property is specific to an input port of a sink device. When
+      specified, it marks the port as recipient of odd-pixels.
+
+...