| 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 |
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 >
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?
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 --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>; + }; + }; + }; + }; }; }; - |
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(-)