diff mbox series

[v7,03/10] dt-bindings: drm/bridge: ti-sn65dsi86: Add aux-bus child

Message ID 20210517130450.v7.3.I98bf729846c37c4c143f6ab88b1e299280e2fe26@changeid (mailing list archive)
State Superseded
Headers show
Series drm: Fix EDID reading on ti-sn65dsi86 by introducing the DP AUX bus | expand

Commit Message

Doug Anderson May 17, 2021, 8:09 p.m. UTC
We want to be able to list an eDP panel as a child of a ti-sn65dsi86
node to represent the fact that the panel is connected to the bridge's
DP AUX bus. Though the panel and the bridge chip are connected in
several ways, the DP AUX bus is the primary control interface between
the two and thus makes the most sense to model in device tree
hierarchy.

Listing a panel in this way makes it possible for the panel driver to
easily get access to the DP AUX bus that it resides on, which can be
useful to help in auto-detecting the panel and for turning on various
bits.

NOTE: it's still possible to continue using the bridge chip and point
to a panel that _isn't_ listed as a child of the bridge chip (since
it's worked that way previously), but that should be deprecated since
there is no downside to listing the panel under the bridge chip.

The idea for this bus's design was hashed out over IRC [1].

[1] https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&date=2021-05-11

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Possibly we might want something fancier that could be included by
other eDP controller bindings. If we want to do this, I'd love to be
pointed at a good example to follow.

Changes in v7:
- ti-sn65dsi86: Add aux-bus child patch new for v7.

 .../bindings/display/bridge/ti,sn65dsi86.yaml | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Comments

Rob Herring May 19, 2021, 8:01 p.m. UTC | #1
On Mon, May 17, 2021 at 01:09:00PM -0700, Douglas Anderson wrote:
> We want to be able to list an eDP panel as a child of a ti-sn65dsi86
> node to represent the fact that the panel is connected to the bridge's
> DP AUX bus. Though the panel and the bridge chip are connected in
> several ways, the DP AUX bus is the primary control interface between
> the two and thus makes the most sense to model in device tree
> hierarchy.
> 
> Listing a panel in this way makes it possible for the panel driver to
> easily get access to the DP AUX bus that it resides on, which can be
> useful to help in auto-detecting the panel and for turning on various
> bits.
> 
> NOTE: it's still possible to continue using the bridge chip and point
> to a panel that _isn't_ listed as a child of the bridge chip (since
> it's worked that way previously), but that should be deprecated since
> there is no downside to listing the panel under the bridge chip.
> 
> The idea for this bus's design was hashed out over IRC [1].
> 
> [1] https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&date=2021-05-11
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> Possibly we might want something fancier that could be included by
> other eDP controller bindings. If we want to do this, I'd love to be
> pointed at a good example to follow.
> 
> Changes in v7:
> - ti-sn65dsi86: Add aux-bus child patch new for v7.
> 
>  .../bindings/display/bridge/ti,sn65dsi86.yaml | 22 ++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> index 26932d2e86ab..51f5a29e216c 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> @@ -70,6 +70,11 @@ properties:
>      const: 1
>      description: See ../../pwm/pwm.yaml for description of the cell formats.
>  
> +  aux-bus:

As this is a node:

type: object

> +    description:
> +      It is recommended that you place your panel under the aux-bus node
> +      here to represent the control hierarchy.
> +
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
>  
> @@ -201,11 +206,26 @@ examples:
>  
>            port@1 {
>              reg = <1>;
> -            endpoint {
> +            sn65dsi86_out: endpoint {
>                remote-endpoint = <&panel_in_edp>;
>              };
>            };
>          };
> +
> +        aux-bus {
> +          panel {

We should perhaps have a separate aux-bus schema. Something should 
define the child node is 'panel' and nothing else. Though perhaps 
connectors are valid too?

> +            compatible = "boe,nv133fhm-n62";
> +            power-supply = <&pp3300_dx_edp>;
> +            backlight = <&backlight>;
> +            hpd-gpios = <&sn65dsi86_bridge 2 GPIO_ACTIVE_HIGH>;
> +
> +            port {
> +              panel_in_edp: endpoint {
> +                remote-endpoint = <&sn65dsi86_out>;
> +              };
> +            };
> +          };
> +        };
>        };
>      };
>    - |
> -- 
> 2.31.1.751.gd2f1c929bd-goog
>
Doug Anderson May 19, 2021, 9:06 p.m. UTC | #2
Hi,

On Wed, May 19, 2021 at 1:02 PM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, May 17, 2021 at 01:09:00PM -0700, Douglas Anderson wrote:
> > We want to be able to list an eDP panel as a child of a ti-sn65dsi86
> > node to represent the fact that the panel is connected to the bridge's
> > DP AUX bus. Though the panel and the bridge chip are connected in
> > several ways, the DP AUX bus is the primary control interface between
> > the two and thus makes the most sense to model in device tree
> > hierarchy.
> >
> > Listing a panel in this way makes it possible for the panel driver to
> > easily get access to the DP AUX bus that it resides on, which can be
> > useful to help in auto-detecting the panel and for turning on various
> > bits.
> >
> > NOTE: it's still possible to continue using the bridge chip and point
> > to a panel that _isn't_ listed as a child of the bridge chip (since
> > it's worked that way previously), but that should be deprecated since
> > there is no downside to listing the panel under the bridge chip.
> >
> > The idea for this bus's design was hashed out over IRC [1].
> >
> > [1] https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&date=2021-05-11
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> > Possibly we might want something fancier that could be included by
> > other eDP controller bindings. If we want to do this, I'd love to be
> > pointed at a good example to follow.
> >
> > Changes in v7:
> > - ti-sn65dsi86: Add aux-bus child patch new for v7.
> >
> >  .../bindings/display/bridge/ti,sn65dsi86.yaml | 22 ++++++++++++++++++-
> >  1 file changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> > index 26932d2e86ab..51f5a29e216c 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> > @@ -70,6 +70,11 @@ properties:
> >      const: 1
> >      description: See ../../pwm/pwm.yaml for description of the cell formats.
> >
> > +  aux-bus:
>
> As this is a node:
>
> type: object
>
> > +    description:
> > +      It is recommended that you place your panel under the aux-bus node
> > +      here to represent the control hierarchy.
> > +
> >    ports:
> >      $ref: /schemas/graph.yaml#/properties/ports
> >
> > @@ -201,11 +206,26 @@ examples:
> >
> >            port@1 {
> >              reg = <1>;
> > -            endpoint {
> > +            sn65dsi86_out: endpoint {
> >                remote-endpoint = <&panel_in_edp>;
> >              };
> >            };
> >          };
> > +
> > +        aux-bus {
> > +          panel {
>
> We should perhaps have a separate aux-bus schema.

Yeah. Before spending lots of time digging into how to do this I
wanted to see if anyone was going to give me a big-old NAK on the
whole approach. ;-)

I guess I'd make a file called "dp-aux-bus.yaml" (maybe right under
bindings/display?) and then I'd include it like this:

aux-bus:
  $ref: "../dp-aux-bus.yaml#"


> Something should
> define the child node is 'panel' and nothing else.

At the moment the code also requires that the node name is 'aux-bus'.
Any objections to that?


> Though perhaps
> connectors are valid too?

They might be. We could always add it later?
Rob Herring May 20, 2021, 1:25 p.m. UTC | #3
On Wed, May 19, 2021 at 4:06 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Wed, May 19, 2021 at 1:02 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Mon, May 17, 2021 at 01:09:00PM -0700, Douglas Anderson wrote:
> > > We want to be able to list an eDP panel as a child of a ti-sn65dsi86
> > > node to represent the fact that the panel is connected to the bridge's
> > > DP AUX bus. Though the panel and the bridge chip are connected in
> > > several ways, the DP AUX bus is the primary control interface between
> > > the two and thus makes the most sense to model in device tree
> > > hierarchy.
> > >
> > > Listing a panel in this way makes it possible for the panel driver to
> > > easily get access to the DP AUX bus that it resides on, which can be
> > > useful to help in auto-detecting the panel and for turning on various
> > > bits.
> > >
> > > NOTE: it's still possible to continue using the bridge chip and point
> > > to a panel that _isn't_ listed as a child of the bridge chip (since
> > > it's worked that way previously), but that should be deprecated since
> > > there is no downside to listing the panel under the bridge chip.
> > >
> > > The idea for this bus's design was hashed out over IRC [1].
> > >
> > > [1] https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&date=2021-05-11
> > >
> > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > ---
> > > Possibly we might want something fancier that could be included by
> > > other eDP controller bindings. If we want to do this, I'd love to be
> > > pointed at a good example to follow.
> > >
> > > Changes in v7:
> > > - ti-sn65dsi86: Add aux-bus child patch new for v7.
> > >
> > >  .../bindings/display/bridge/ti,sn65dsi86.yaml | 22 ++++++++++++++++++-
> > >  1 file changed, 21 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> > > index 26932d2e86ab..51f5a29e216c 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> > > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> > > @@ -70,6 +70,11 @@ properties:
> > >      const: 1
> > >      description: See ../../pwm/pwm.yaml for description of the cell formats.
> > >
> > > +  aux-bus:
> >
> > As this is a node:
> >
> > type: object
> >
> > > +    description:
> > > +      It is recommended that you place your panel under the aux-bus node
> > > +      here to represent the control hierarchy.
> > > +
> > >    ports:
> > >      $ref: /schemas/graph.yaml#/properties/ports
> > >
> > > @@ -201,11 +206,26 @@ examples:
> > >
> > >            port@1 {
> > >              reg = <1>;
> > > -            endpoint {
> > > +            sn65dsi86_out: endpoint {
> > >                remote-endpoint = <&panel_in_edp>;
> > >              };
> > >            };
> > >          };
> > > +
> > > +        aux-bus {
> > > +          panel {
> >
> > We should perhaps have a separate aux-bus schema.
>
> Yeah. Before spending lots of time digging into how to do this I
> wanted to see if anyone was going to give me a big-old NAK on the
> whole approach. ;-)
>
> I guess I'd make a file called "dp-aux-bus.yaml" (maybe right under
> bindings/display?) and then I'd include it like this:
>
> aux-bus:
>   $ref: "../dp-aux-bus.yaml#"

Right.

> > Something should
> > define the child node is 'panel' and nothing else.
>
> At the moment the code also requires that the node name is 'aux-bus'.
> Any objections to that?

No. There's 2 ways to do that. The above does and adding $nodename in
dp-aux-bus.yaml will. The latter also means the schema will be applied
automatically to any node named 'aux-bus'. That means the schema will
be applied twice unless you have 'select: false'. The main advantage
of the latter case is it gets applied even without all the users
converted to schema.

> > Though perhaps
> > connectors are valid too?
>
> They might be. We could always add it later?

Sure.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
index 26932d2e86ab..51f5a29e216c 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
@@ -70,6 +70,11 @@  properties:
     const: 1
     description: See ../../pwm/pwm.yaml for description of the cell formats.
 
+  aux-bus:
+    description:
+      It is recommended that you place your panel under the aux-bus node
+      here to represent the control hierarchy.
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
@@ -201,11 +206,26 @@  examples:
 
           port@1 {
             reg = <1>;
-            endpoint {
+            sn65dsi86_out: endpoint {
               remote-endpoint = <&panel_in_edp>;
             };
           };
         };
+
+        aux-bus {
+          panel {
+            compatible = "boe,nv133fhm-n62";
+            power-supply = <&pp3300_dx_edp>;
+            backlight = <&backlight>;
+            hpd-gpios = <&sn65dsi86_bridge 2 GPIO_ACTIVE_HIGH>;
+
+            port {
+              panel_in_edp: endpoint {
+                remote-endpoint = <&sn65dsi86_out>;
+              };
+            };
+          };
+        };
       };
     };
   - |