Re: [PATCH v2 1/2] dt-bindings: display: bridge: Add GPIO display mux binding
Hello Pin-yen, On Fri, Feb 10, 2023 at 03:38:00PM +0800, Pin-yen Lin wrote: > On Tue, Feb 7, 2023 at 11:21 PM Laurent Pinchart wrote: > > On Tue, Feb 07, 2023 at 06:30:36PM +0800, Pin-yen Lin wrote: > > > On Tue, Feb 7, 2023 at 6:25 PM Laurent Pinchart wrote: > > > > On Tue, Feb 07, 2023 at 06:07:44PM +0800, Pin-yen Lin wrote: > > > > > On Wed, Jan 18, 2023 at 4:17 AM Rob Herring wrote: > > > > > > On Mon, Jan 16, 2023 at 07:08:19PM +0800, Pin-yen Lin wrote: > > > > > > > From: Nicolas Boichat > > > > > > > > > > > > > > Add bindings for Generic GPIO mux driver. > > > > > > > > > > > > > > Signed-off-by: Nicolas Boichat > > > > > > > Signed-off-by: Pin-yen Lin > > > > > > > --- > > > > > > > > > > > > > > Changes in v2: > > > > > > > - Referenced existing dt-binding schemas from graph.yaml > > > > > > > - Added ddc-i2c-bus into the bindings > > > > > > > > > > > > > > .../bindings/display/bridge/gpio-mux.yaml | 95 > > > > > > > +++ > > > > > > > 1 file changed, 95 insertions(+) > > > > > > > create mode 100644 > > > > > > > Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml > > > > > > > > > > > > > > diff --git > > > > > > > a/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml > > > > > > > b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml > > > > > > > new file mode 100644 > > > > > > > index ..da29ba078f05 > > > > > > > --- /dev/null > > > > > > > +++ > > > > > > > b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml > > > > > > > @@ -0,0 +1,95 @@ > > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > > > +%YAML 1.2 > > > > > > > +--- > > > > > > > +$id: http://devicetree.org/schemas/display/bridge/gpio-mux.yaml# > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > > + > > > > > > > +title: Generic display mux (1 input, 2 outputs) > > > > > > > + > > > > > > > +maintainers: > > > > > > > + - Nicolas Boichat > > > > > > > + > > > > > > > +description: | > > > > > > > + This bindings describes a simple display (e.g. HDMI) mux, that > > > > > > > has 1 > > > > > > > + input, and 2 outputs. The mux status is controlled by > > > > > > > hardware, and > > > > > > > + its status is read back using a GPIO. > > > > > > > + > > > > > > > +properties: > > > > > > > + compatible: > > > > > > > +const: gpio-display-mux > > > > > > > + > > > > > > > + detect-gpios: > > > > > > > +maxItems: 1 > > > > > > > +description: GPIO that indicates the active output > > > > > > > > > > > > What are we detecting? That implies an input, but this is selecting > > > > > > the > > > > > > output path, right? Or what does 'mux status is controlled by > > > > > > hardware' > > > > > > mean exactly? Something else? That does not sound very generic. > > > > > > > > > > The GPIO (or any kind of MUX) is an input that indicates where the > > > > > output should go. The actual "output selection" procedure is done in > > > > > the driver. That is, the driver monitors this GPIO and selects the > > > > > output path accordingly. In our use case, the GPIO is reported by the > > > > > embedded controller on the device. > > > > > > > > > > [1] listed other similar bridges that can leverage this driver, so we > > > > > called this driver "generic". > > > > > > > > > > [1]: > > > > > https://lore.kernel.org/all/CAJMQK-jGw8kJFNjoHjeZUL+3NCiOS2hgGERnAnMwNsL_cm_J=q...@mail.gmail.com/ > > > > > > > > > > > In any case, we have a common mux binding so any kind of mux control > > > > > > could be used here, not just GPIO. Then you can make this just a > > > > > > generic > > > > > > display mux. > > > > > > > > > > Thanks for sharing this, I'll update the binding in the next version. > > > > > > > > > > > > + > > > > > > > + ddc-i2c-bus: > > > > > > > +description: phandle link to the I2C controller used for DDC > > > > > > > EDID probing > > > > > > > +$ref: /schemas/types.yaml#/definitions/phandle > > > > > > > > > > > > This belongs in the connector node(s). > > > > > > > > > > The HDMI bridge before the MUX doesn't (and doesn't have to) know that > > > > > its next bridge is a MUX. We put it here so that the HDMI bridge can > > > > > parse the phandle and get the bus node. > > > > > > > > How does that work, does the HDMI encoder driver parse the ddc-i2c-bus > > > > property of the next DT node in the OF graph ? > > > > > > Yes. In our use case, mtk_hdmi.c[2] checks the remote node of its > > > output port to get the bus phandle. sun4i_hdmi_enc.c[3] seems to use a > > > similar approach as well. > > > > Peeking into nodes of other devices is a bad practice. I don't know how > > the code you mention below got merged, but I'm pretty sure I would have > > flagged it if I had reviewed the patches :-) > > > > The ddc-i2c-bus property should instead be specified in the node where > > it logically belongs (in this case, the connector node), and handled by > > the
Re: [PATCH v2 1/2] dt-bindings: display: bridge: Add GPIO display mux binding
Hi Laurent, On Tue, Feb 7, 2023 at 11:21 PM Laurent Pinchart wrote: > > Hello Pin-yen, > > On Tue, Feb 07, 2023 at 06:30:36PM +0800, Pin-yen Lin wrote: > > On Tue, Feb 7, 2023 at 6:25 PM Laurent Pinchart wrote: > > > On Tue, Feb 07, 2023 at 06:07:44PM +0800, Pin-yen Lin wrote: > > > > On Wed, Jan 18, 2023 at 4:17 AM Rob Herring wrote: > > > > > On Mon, Jan 16, 2023 at 07:08:19PM +0800, Pin-yen Lin wrote: > > > > > > From: Nicolas Boichat > > > > > > > > > > > > Add bindings for Generic GPIO mux driver. > > > > > > > > > > > > Signed-off-by: Nicolas Boichat > > > > > > Signed-off-by: Pin-yen Lin > > > > > > --- > > > > > > > > > > > > Changes in v2: > > > > > > - Referenced existing dt-binding schemas from graph.yaml > > > > > > - Added ddc-i2c-bus into the bindings > > > > > > > > > > > > .../bindings/display/bridge/gpio-mux.yaml | 95 > > > > > > +++ > > > > > > 1 file changed, 95 insertions(+) > > > > > > create mode 100644 > > > > > > Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml > > > > > > > > > > > > diff --git > > > > > > a/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml > > > > > > b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml > > > > > > new file mode 100644 > > > > > > index ..da29ba078f05 > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml > > > > > > @@ -0,0 +1,95 @@ > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > > +%YAML 1.2 > > > > > > +--- > > > > > > +$id: http://devicetree.org/schemas/display/bridge/gpio-mux.yaml# > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > + > > > > > > +title: Generic display mux (1 input, 2 outputs) > > > > > > + > > > > > > +maintainers: > > > > > > + - Nicolas Boichat > > > > > > + > > > > > > +description: | > > > > > > + This bindings describes a simple display (e.g. HDMI) mux, that > > > > > > has 1 > > > > > > + input, and 2 outputs. The mux status is controlled by hardware, > > > > > > and > > > > > > + its status is read back using a GPIO. > > > > > > + > > > > > > +properties: > > > > > > + compatible: > > > > > > +const: gpio-display-mux > > > > > > + > > > > > > + detect-gpios: > > > > > > +maxItems: 1 > > > > > > +description: GPIO that indicates the active output > > > > > > > > > > What are we detecting? That implies an input, but this is selecting > > > > > the > > > > > output path, right? Or what does 'mux status is controlled by > > > > > hardware' > > > > > mean exactly? Something else? That does not sound very generic. > > > > > > > > The GPIO (or any kind of MUX) is an input that indicates where the > > > > output should go. The actual "output selection" procedure is done in > > > > the driver. That is, the driver monitors this GPIO and selects the > > > > output path accordingly. In our use case, the GPIO is reported by the > > > > embedded controller on the device. > > > > > > > > [1] listed other similar bridges that can leverage this driver, so we > > > > called this driver "generic". > > > > > > > > [1]: > > > > https://lore.kernel.org/all/CAJMQK-jGw8kJFNjoHjeZUL+3NCiOS2hgGERnAnMwNsL_cm_J=q...@mail.gmail.com/ > > > > > > > > > In any case, we have a common mux binding so any kind of mux control > > > > > could be used here, not just GPIO. Then you can make this just a > > > > > generic > > > > > display mux. > > > > > > > > Thanks for sharing this, I'll update the binding in the next version. > > > > > > > > > > + > > > > > > + ddc-i2c-bus: > > > > > > +description: phandle link to the I2C controller used for DDC > > > > > > EDID probing > > > > > > +$ref: /schemas/types.yaml#/definitions/phandle > > > > > > > > > > This belongs in the connector node(s). > > > > > > > > The HDMI bridge before the MUX doesn't (and doesn't have to) know that > > > > its next bridge is a MUX. We put it here so that the HDMI bridge can > > > > parse the phandle and get the bus node. > > > > > > How does that work, does the HDMI encoder driver parse the ddc-i2c-bus > > > property of the next DT node in the OF graph ? > > > > Yes. In our use case, mtk_hdmi.c[2] checks the remote node of its > > output port to get the bus phandle. sun4i_hdmi_enc.c[3] seems to use a > > similar approach as well. > > Peeking into nodes of other devices is a bad practice. I don't know how > the code you mention below got merged, but I'm pretty sure I would have > flagged it if I had reviewed the patches :-) > > The ddc-i2c-bus property should instead be specified in the node where > it logically belongs (in this case, the connector node), and handled by > the connector driver. You can then use drm_bridge operations to tie > things together, like done in the drm_bridge_connector helper. I'd > recommend using the drm_bridge_connector helper if you can, either > as-is, or by extending it. So, even if the connector does not have its own i2c
Re: [PATCH v2 1/2] dt-bindings: display: bridge: Add GPIO display mux binding
Hello Pin-yen, On Tue, Feb 07, 2023 at 06:30:36PM +0800, Pin-yen Lin wrote: > On Tue, Feb 7, 2023 at 6:25 PM Laurent Pinchart wrote: > > On Tue, Feb 07, 2023 at 06:07:44PM +0800, Pin-yen Lin wrote: > > > On Wed, Jan 18, 2023 at 4:17 AM Rob Herring wrote: > > > > On Mon, Jan 16, 2023 at 07:08:19PM +0800, Pin-yen Lin wrote: > > > > > From: Nicolas Boichat > > > > > > > > > > Add bindings for Generic GPIO mux driver. > > > > > > > > > > Signed-off-by: Nicolas Boichat > > > > > Signed-off-by: Pin-yen Lin > > > > > --- > > > > > > > > > > Changes in v2: > > > > > - Referenced existing dt-binding schemas from graph.yaml > > > > > - Added ddc-i2c-bus into the bindings > > > > > > > > > > .../bindings/display/bridge/gpio-mux.yaml | 95 > > > > > +++ > > > > > 1 file changed, 95 insertions(+) > > > > > create mode 100644 > > > > > Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml > > > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml > > > > > b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml > > > > > new file mode 100644 > > > > > index ..da29ba078f05 > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml > > > > > @@ -0,0 +1,95 @@ > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > +%YAML 1.2 > > > > > +--- > > > > > +$id: http://devicetree.org/schemas/display/bridge/gpio-mux.yaml# > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > + > > > > > +title: Generic display mux (1 input, 2 outputs) > > > > > + > > > > > +maintainers: > > > > > + - Nicolas Boichat > > > > > + > > > > > +description: | > > > > > + This bindings describes a simple display (e.g. HDMI) mux, that has > > > > > 1 > > > > > + input, and 2 outputs. The mux status is controlled by hardware, and > > > > > + its status is read back using a GPIO. > > > > > + > > > > > +properties: > > > > > + compatible: > > > > > +const: gpio-display-mux > > > > > + > > > > > + detect-gpios: > > > > > +maxItems: 1 > > > > > +description: GPIO that indicates the active output > > > > > > > > What are we detecting? That implies an input, but this is selecting the > > > > output path, right? Or what does 'mux status is controlled by hardware' > > > > mean exactly? Something else? That does not sound very generic. > > > > > > The GPIO (or any kind of MUX) is an input that indicates where the > > > output should go. The actual "output selection" procedure is done in > > > the driver. That is, the driver monitors this GPIO and selects the > > > output path accordingly. In our use case, the GPIO is reported by the > > > embedded controller on the device. > > > > > > [1] listed other similar bridges that can leverage this driver, so we > > > called this driver "generic". > > > > > > [1]: > > > https://lore.kernel.org/all/CAJMQK-jGw8kJFNjoHjeZUL+3NCiOS2hgGERnAnMwNsL_cm_J=q...@mail.gmail.com/ > > > > > > > In any case, we have a common mux binding so any kind of mux control > > > > could be used here, not just GPIO. Then you can make this just a generic > > > > display mux. > > > > > > Thanks for sharing this, I'll update the binding in the next version. > > > > > > > > + > > > > > + ddc-i2c-bus: > > > > > +description: phandle link to the I2C controller used for DDC > > > > > EDID probing > > > > > +$ref: /schemas/types.yaml#/definitions/phandle > > > > > > > > This belongs in the connector node(s). > > > > > > The HDMI bridge before the MUX doesn't (and doesn't have to) know that > > > its next bridge is a MUX. We put it here so that the HDMI bridge can > > > parse the phandle and get the bus node. > > > > How does that work, does the HDMI encoder driver parse the ddc-i2c-bus > > property of the next DT node in the OF graph ? > > Yes. In our use case, mtk_hdmi.c[2] checks the remote node of its > output port to get the bus phandle. sun4i_hdmi_enc.c[3] seems to use a > similar approach as well. Peeking into nodes of other devices is a bad practice. I don't know how the code you mention below got merged, but I'm pretty sure I would have flagged it if I had reviewed the patches :-) The ddc-i2c-bus property should instead be specified in the node where it logically belongs (in this case, the connector node), and handled by the connector driver. You can then use drm_bridge operations to tie things together, like done in the drm_bridge_connector helper. I'd recommend using the drm_bridge_connector helper if you can, either as-is, or by extending it. > [2]: > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/mediatek/mtk_hdmi.c#L1500 > [3]: > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c#L240 > > > > > > + > > > > > + ports: > > > > > +$ref: /schemas/graph.yaml#/properties/ports > > > > > + > > > > > +properties: > > > > > + port@0: > > > > > +$ref:
Re: [PATCH v2 1/2] dt-bindings: display: bridge: Add GPIO display mux binding
Hi Laurent,
On Tue, Feb 7, 2023 at 6:25 PM Laurent Pinchart
wrote:
>
> On Tue, Feb 07, 2023 at 06:07:44PM +0800, Pin-yen Lin wrote:
> > On Wed, Jan 18, 2023 at 4:17 AM Rob Herring wrote:
> > > On Mon, Jan 16, 2023 at 07:08:19PM +0800, Pin-yen Lin wrote:
> > > > From: Nicolas Boichat
> > > >
> > > > Add bindings for Generic GPIO mux driver.
> > > >
> > > > Signed-off-by: Nicolas Boichat
> > > > Signed-off-by: Pin-yen Lin
> > > > ---
> > > >
> > > > Changes in v2:
> > > > - Referenced existing dt-binding schemas from graph.yaml
> > > > - Added ddc-i2c-bus into the bindings
> > > >
> > > > .../bindings/display/bridge/gpio-mux.yaml | 95 +++
> > > > 1 file changed, 95 insertions(+)
> > > > create mode 100644
> > > > Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> > > > b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> > > > new file mode 100644
> > > > index ..da29ba078f05
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> > > > @@ -0,0 +1,95 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/display/bridge/gpio-mux.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Generic display mux (1 input, 2 outputs)
> > > > +
> > > > +maintainers:
> > > > + - Nicolas Boichat
> > > > +
> > > > +description: |
> > > > + This bindings describes a simple display (e.g. HDMI) mux, that has 1
> > > > + input, and 2 outputs. The mux status is controlled by hardware, and
> > > > + its status is read back using a GPIO.
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > +const: gpio-display-mux
> > > > +
> > > > + detect-gpios:
> > > > +maxItems: 1
> > > > +description: GPIO that indicates the active output
> > >
> > > What are we detecting? That implies an input, but this is selecting the
> > > output path, right? Or what does 'mux status is controlled by hardware'
> > > mean exactly? Something else? That does not sound very generic.
> >
> > The GPIO (or any kind of MUX) is an input that indicates where the
> > output should go. The actual "output selection" procedure is done in
> > the driver. That is, the driver monitors this GPIO and selects the
> > output path accordingly. In our use case, the GPIO is reported by the
> > embedded controller on the device.
> >
> > [1] listed other similar bridges that can leverage this driver, so we
> > called this driver "generic".
> >
> > [1]:
> > https://lore.kernel.org/all/CAJMQK-jGw8kJFNjoHjeZUL+3NCiOS2hgGERnAnMwNsL_cm_J=q...@mail.gmail.com/
> >
> > > In any case, we have a common mux binding so any kind of mux control
> > > could be used here, not just GPIO. Then you can make this just a generic
> > > display mux.
> >
> > Thanks for sharing this, I'll update the binding in the next version.
> >
> > > > +
> > > > + ddc-i2c-bus:
> > > > +description: phandle link to the I2C controller used for DDC EDID
> > > > probing
> > > > +$ref: /schemas/types.yaml#/definitions/phandle
> > >
> > > This belongs in the connector node(s).
> >
> > The HDMI bridge before the MUX doesn't (and doesn't have to) know that
> > its next bridge is a MUX. We put it here so that the HDMI bridge can
> > parse the phandle and get the bus node.
>
> How does that work, does the HDMI encoder driver parse the ddc-i2c-bus
> property of the next DT node in the OF graph ?
Yes. In our use case, mtk_hdmi.c[2] checks the remote node of its
output port to get the bus phandle. sun4i_hdmi_enc.c[3] seems to use a
similar approach as well.
[2]:
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/mediatek/mtk_hdmi.c#L1500
[3]:
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c#L240
Regards,
Pin-yen
>
> > > > +
> > > > + ports:
> > > > +$ref: /schemas/graph.yaml#/properties/ports
> > > > +
> > > > +properties:
> > > > + port@0:
> > > > +$ref: /schemas/graph.yaml#/properties/port
> > > > +description: |
> > > > + Video port for input.
> > > > +
> > > > + port@1:
> > > > +$ref: /schemas/graph.yaml#/properties/port
> > > > +description: |
> > > > + 2 video ports for output.
> > > > + The reg value in the endpoints matches the GPIO status: when
> > > > + GPIO is asserted, endpoint with reg value <1> is selected.
> > > > +
> > > > +required:
> > > > + - port@0
> > > > + - port@1
> > > > +
> > > > +required:
> > > > + - compatible
> > > > + - detect-gpios
> > > > + - ports
> > > > +
> > > > +unevaluatedProperties: false
> > > > +
> > > > +examples:
> > > > + - |
> > > > +#include
> > > > +hdmi_mux: hdmi_mux {
> > > > + compatible = "gpio-display-mux";
> > > > +
Re: [PATCH v2 1/2] dt-bindings: display: bridge: Add GPIO display mux binding
On Tue, Feb 07, 2023 at 06:07:44PM +0800, Pin-yen Lin wrote:
> On Wed, Jan 18, 2023 at 4:17 AM Rob Herring wrote:
> > On Mon, Jan 16, 2023 at 07:08:19PM +0800, Pin-yen Lin wrote:
> > > From: Nicolas Boichat
> > >
> > > Add bindings for Generic GPIO mux driver.
> > >
> > > Signed-off-by: Nicolas Boichat
> > > Signed-off-by: Pin-yen Lin
> > > ---
> > >
> > > Changes in v2:
> > > - Referenced existing dt-binding schemas from graph.yaml
> > > - Added ddc-i2c-bus into the bindings
> > >
> > > .../bindings/display/bridge/gpio-mux.yaml | 95 +++
> > > 1 file changed, 95 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> > > b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> > > new file mode 100644
> > > index ..da29ba078f05
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> > > @@ -0,0 +1,95 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/display/bridge/gpio-mux.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Generic display mux (1 input, 2 outputs)
> > > +
> > > +maintainers:
> > > + - Nicolas Boichat
> > > +
> > > +description: |
> > > + This bindings describes a simple display (e.g. HDMI) mux, that has 1
> > > + input, and 2 outputs. The mux status is controlled by hardware, and
> > > + its status is read back using a GPIO.
> > > +
> > > +properties:
> > > + compatible:
> > > +const: gpio-display-mux
> > > +
> > > + detect-gpios:
> > > +maxItems: 1
> > > +description: GPIO that indicates the active output
> >
> > What are we detecting? That implies an input, but this is selecting the
> > output path, right? Or what does 'mux status is controlled by hardware'
> > mean exactly? Something else? That does not sound very generic.
>
> The GPIO (or any kind of MUX) is an input that indicates where the
> output should go. The actual "output selection" procedure is done in
> the driver. That is, the driver monitors this GPIO and selects the
> output path accordingly. In our use case, the GPIO is reported by the
> embedded controller on the device.
>
> [1] listed other similar bridges that can leverage this driver, so we
> called this driver "generic".
>
> [1]:
> https://lore.kernel.org/all/CAJMQK-jGw8kJFNjoHjeZUL+3NCiOS2hgGERnAnMwNsL_cm_J=q...@mail.gmail.com/
>
> > In any case, we have a common mux binding so any kind of mux control
> > could be used here, not just GPIO. Then you can make this just a generic
> > display mux.
>
> Thanks for sharing this, I'll update the binding in the next version.
>
> > > +
> > > + ddc-i2c-bus:
> > > +description: phandle link to the I2C controller used for DDC EDID
> > > probing
> > > +$ref: /schemas/types.yaml#/definitions/phandle
> >
> > This belongs in the connector node(s).
>
> The HDMI bridge before the MUX doesn't (and doesn't have to) know that
> its next bridge is a MUX. We put it here so that the HDMI bridge can
> parse the phandle and get the bus node.
How does that work, does the HDMI encoder driver parse the ddc-i2c-bus
property of the next DT node in the OF graph ?
> > > +
> > > + ports:
> > > +$ref: /schemas/graph.yaml#/properties/ports
> > > +
> > > +properties:
> > > + port@0:
> > > +$ref: /schemas/graph.yaml#/properties/port
> > > +description: |
> > > + Video port for input.
> > > +
> > > + port@1:
> > > +$ref: /schemas/graph.yaml#/properties/port
> > > +description: |
> > > + 2 video ports for output.
> > > + The reg value in the endpoints matches the GPIO status: when
> > > + GPIO is asserted, endpoint with reg value <1> is selected.
> > > +
> > > +required:
> > > + - port@0
> > > + - port@1
> > > +
> > > +required:
> > > + - compatible
> > > + - detect-gpios
> > > + - ports
> > > +
> > > +unevaluatedProperties: false
> > > +
> > > +examples:
> > > + - |
> > > +#include
> > > +hdmi_mux: hdmi_mux {
> > > + compatible = "gpio-display-mux";
> > > + detect-gpios = < 36 GPIO_ACTIVE_HIGH>;
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <_mux_pins>;
> > > + ddc-i2c-bus = <>;
> > > +
> > > + ports {
> > > +#address-cells = <1>;
> > > +#size-cells = <0>;
> > > +
> > > +port@0 { /* input */
> > > + reg = <0>;
> > > +
> > > + hdmi_mux_in: endpoint {
> > > +remote-endpoint = <_out>;
> > > + };
> > > +};
> > > +
> > > +port@1 { /* output */
> > > + reg = <1>;
> > > +
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + hdmi_mux_out_anx: endpoint@0 {
> > > +reg =
Re: [PATCH v2 1/2] dt-bindings: display: bridge: Add GPIO display mux binding
Hi Rob,
Thanks for the review.
On Wed, Jan 18, 2023 at 4:17 AM Rob Herring wrote:
>
> On Mon, Jan 16, 2023 at 07:08:19PM +0800, Pin-yen Lin wrote:
> > From: Nicolas Boichat
> >
> > Add bindings for Generic GPIO mux driver.
> >
> > Signed-off-by: Nicolas Boichat
> > Signed-off-by: Pin-yen Lin
> > ---
> >
> > Changes in v2:
> > - Referenced existing dt-binding schemas from graph.yaml
> > - Added ddc-i2c-bus into the bindings
> >
> > .../bindings/display/bridge/gpio-mux.yaml | 95 +++
> > 1 file changed, 95 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> > b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> > new file mode 100644
> > index ..da29ba078f05
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> > @@ -0,0 +1,95 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/bridge/gpio-mux.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Generic display mux (1 input, 2 outputs)
> > +
> > +maintainers:
> > + - Nicolas Boichat
> > +
> > +description: |
> > + This bindings describes a simple display (e.g. HDMI) mux, that has 1
> > + input, and 2 outputs. The mux status is controlled by hardware, and
> > + its status is read back using a GPIO.
> > +
> > +properties:
> > + compatible:
> > +const: gpio-display-mux
> > +
> > + detect-gpios:
> > +maxItems: 1
> > +description: GPIO that indicates the active output
>
> What are we detecting? That implies an input, but this is selecting the
> output path, right? Or what does 'mux status is controlled by hardware'
> mean exactly? Something else? That does not sound very generic.
The GPIO (or any kind of MUX) is an input that indicates where the
output should go. The actual "output selection" procedure is done in
the driver. That is, the driver monitors this GPIO and selects the
output path accordingly. In our use case, the GPIO is reported by the
embedded controller on the device.
[1] listed other similar bridges that can leverage this driver, so we
called this driver "generic".
[1]:
https://lore.kernel.org/all/CAJMQK-jGw8kJFNjoHjeZUL+3NCiOS2hgGERnAnMwNsL_cm_J=q...@mail.gmail.com/
>
> In any case, we have a common mux binding so any kind of mux control
> could be used here, not just GPIO. Then you can make this just a generic
> display mux.
Thanks for sharing this, I'll update the binding in the next version.
>
> > +
> > + ddc-i2c-bus:
> > +description: phandle link to the I2C controller used for DDC EDID
> > probing
> > +$ref: /schemas/types.yaml#/definitions/phandle
>
> This belongs in the connector node(s).
The HDMI bridge before the MUX doesn't (and doesn't have to) know that
its next bridge is a MUX. We put it here so that the HDMI bridge can
parse the phandle and get the bus node.
Regards,
Pin-yen
>
> > +
> > + ports:
> > +$ref: /schemas/graph.yaml#/properties/ports
> > +
> > +properties:
> > + port@0:
> > +$ref: /schemas/graph.yaml#/properties/port
> > +description: |
> > + Video port for input.
> > +
> > + port@1:
> > +$ref: /schemas/graph.yaml#/properties/port
> > +description: |
> > + 2 video ports for output.
> > + The reg value in the endpoints matches the GPIO status: when
> > + GPIO is asserted, endpoint with reg value <1> is selected.
> > +
> > +required:
> > + - port@0
> > + - port@1
> > +
> > +required:
> > + - compatible
> > + - detect-gpios
> > + - ports
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > +#include
> > +hdmi_mux: hdmi_mux {
> > + compatible = "gpio-display-mux";
> > + detect-gpios = < 36 GPIO_ACTIVE_HIGH>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <_mux_pins>;
> > + ddc-i2c-bus = <>;
> > +
> > + ports {
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +
> > +port@0 { /* input */
> > + reg = <0>;
> > +
> > + hdmi_mux_in: endpoint {
> > +remote-endpoint = <_out>;
> > + };
> > +};
> > +
> > +port@1 { /* output */
> > + reg = <1>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + hdmi_mux_out_anx: endpoint@0 {
> > +reg = <0>;
> > +remote-endpoint = <_bridge_in>;
> > + };
> > +
> > + hdmi_mux_out_hdmi: endpoint@1 {
> > +reg = <1>;
> > +remote-endpoint = <_connector_in>;
> > + };
> > +};
> > + };
> > +};
> > --
> > 2.39.0.314.g84b9a713c41-goog
> >
Re: [PATCH v2 1/2] dt-bindings: display: bridge: Add GPIO display mux binding
On Mon, Jan 16, 2023 at 07:08:19PM +0800, Pin-yen Lin wrote:
> From: Nicolas Boichat
>
> Add bindings for Generic GPIO mux driver.
>
> Signed-off-by: Nicolas Boichat
> Signed-off-by: Pin-yen Lin
> ---
>
> Changes in v2:
> - Referenced existing dt-binding schemas from graph.yaml
> - Added ddc-i2c-bus into the bindings
>
> .../bindings/display/bridge/gpio-mux.yaml | 95 +++
> 1 file changed, 95 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> new file mode 100644
> index ..da29ba078f05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
> @@ -0,0 +1,95 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/gpio-mux.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic display mux (1 input, 2 outputs)
> +
> +maintainers:
> + - Nicolas Boichat
> +
> +description: |
> + This bindings describes a simple display (e.g. HDMI) mux, that has 1
> + input, and 2 outputs. The mux status is controlled by hardware, and
> + its status is read back using a GPIO.
> +
> +properties:
> + compatible:
> +const: gpio-display-mux
> +
> + detect-gpios:
> +maxItems: 1
> +description: GPIO that indicates the active output
What are we detecting? That implies an input, but this is selecting the
output path, right? Or what does 'mux status is controlled by hardware'
mean exactly? Something else? That does not sound very generic.
In any case, we have a common mux binding so any kind of mux control
could be used here, not just GPIO. Then you can make this just a generic
display mux.
> +
> + ddc-i2c-bus:
> +description: phandle link to the I2C controller used for DDC EDID probing
> +$ref: /schemas/types.yaml#/definitions/phandle
This belongs in the connector node(s).
> +
> + ports:
> +$ref: /schemas/graph.yaml#/properties/ports
> +
> +properties:
> + port@0:
> +$ref: /schemas/graph.yaml#/properties/port
> +description: |
> + Video port for input.
> +
> + port@1:
> +$ref: /schemas/graph.yaml#/properties/port
> +description: |
> + 2 video ports for output.
> + The reg value in the endpoints matches the GPIO status: when
> + GPIO is asserted, endpoint with reg value <1> is selected.
> +
> +required:
> + - port@0
> + - port@1
> +
> +required:
> + - compatible
> + - detect-gpios
> + - ports
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> +#include
> +hdmi_mux: hdmi_mux {
> + compatible = "gpio-display-mux";
> + detect-gpios = < 36 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_mux_pins>;
> + ddc-i2c-bus = <>;
> +
> + ports {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +port@0 { /* input */
> + reg = <0>;
> +
> + hdmi_mux_in: endpoint {
> +remote-endpoint = <_out>;
> + };
> +};
> +
> +port@1 { /* output */
> + reg = <1>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + hdmi_mux_out_anx: endpoint@0 {
> +reg = <0>;
> +remote-endpoint = <_bridge_in>;
> + };
> +
> + hdmi_mux_out_hdmi: endpoint@1 {
> +reg = <1>;
> +remote-endpoint = <_connector_in>;
> + };
> +};
> + };
> +};
> --
> 2.39.0.314.g84b9a713c41-goog
>
[PATCH v2 1/2] dt-bindings: display: bridge: Add GPIO display mux binding
From: Nicolas Boichat
Add bindings for Generic GPIO mux driver.
Signed-off-by: Nicolas Boichat
Signed-off-by: Pin-yen Lin
---
Changes in v2:
- Referenced existing dt-binding schemas from graph.yaml
- Added ddc-i2c-bus into the bindings
.../bindings/display/bridge/gpio-mux.yaml | 95 +++
1 file changed, 95 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
diff --git a/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
new file mode 100644
index ..da29ba078f05
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/gpio-mux.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/gpio-mux.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic display mux (1 input, 2 outputs)
+
+maintainers:
+ - Nicolas Boichat
+
+description: |
+ This bindings describes a simple display (e.g. HDMI) mux, that has 1
+ input, and 2 outputs. The mux status is controlled by hardware, and
+ its status is read back using a GPIO.
+
+properties:
+ compatible:
+const: gpio-display-mux
+
+ detect-gpios:
+maxItems: 1
+description: GPIO that indicates the active output
+
+ ddc-i2c-bus:
+description: phandle link to the I2C controller used for DDC EDID probing
+$ref: /schemas/types.yaml#/definitions/phandle
+
+ ports:
+$ref: /schemas/graph.yaml#/properties/ports
+
+properties:
+ port@0:
+$ref: /schemas/graph.yaml#/properties/port
+description: |
+ Video port for input.
+
+ port@1:
+$ref: /schemas/graph.yaml#/properties/port
+description: |
+ 2 video ports for output.
+ The reg value in the endpoints matches the GPIO status: when
+ GPIO is asserted, endpoint with reg value <1> is selected.
+
+required:
+ - port@0
+ - port@1
+
+required:
+ - compatible
+ - detect-gpios
+ - ports
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+hdmi_mux: hdmi_mux {
+ compatible = "gpio-display-mux";
+ detect-gpios = < 36 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <_mux_pins>;
+ ddc-i2c-bus = <>;
+
+ ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@0 { /* input */
+ reg = <0>;
+
+ hdmi_mux_in: endpoint {
+remote-endpoint = <_out>;
+ };
+};
+
+port@1 { /* output */
+ reg = <1>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ hdmi_mux_out_anx: endpoint@0 {
+reg = <0>;
+remote-endpoint = <_bridge_in>;
+ };
+
+ hdmi_mux_out_hdmi: endpoint@1 {
+reg = <1>;
+remote-endpoint = <_connector_in>;
+ };
+};
+ };
+};
--
2.39.0.314.g84b9a713c41-goog
