[PATCH v4 0/2] drm: bridge: Add TI tmds181 and sn65dp159 driver

[PATCH v4 0/2] drm: bridge: Add TI tmds181 and sn65dp159 driver

[Mike Looijmans]

In the past I've seen (and contributed to) hacks that model the chips as
phy or even (really!) clock drivers. Since the chip usually sits between
a signal that is (almost) HDMI and a HDMI connector, I decided to stop
lying and write it as a DRM bridge driver.

Our experience with these chips is that they work best under manual control
enabling them only once the signal is active. At low resolutions (under 4k),
the optimal setting is usually to only use redriver mode. Setting the
termination to 150-300 Ohms improves EMC performance at lower resolutions,
hence the driver enables 75-150 Ohms for HDMI2 modes and defaults to
150-300 Ohm termination for other modes.


Changes in v4:
Use fallback compatible
dev_err_probe, this_module, of_match_ptr
Use fallback compatible in driver
Add vcc-supply and vdd-supply in driver

Changes in v3:
Fix duplicate links
Add vcc-supply and vdd-supply
Fix missing type for ti,slew-rate
Lower-case hex values and use defines for EYESCAN registers
Remove equalizer code (unlikely to be used)
Remove attributes (no longer useful, undocumented)
Fix build for 6.17 kernel
Use devm_drm_bridge_alloc
Sort includes and add linux/bitfield.h
Check chip type and complain on mismatch

Changes in v2:
Document driver specific bindings like slew-rate and threshold
Use atomic_enable/disable
Use #defines for bit fields in registers
Allow HDMI 2 compliance
Filter modes on clock range
Use cross-over pixel frequency instead of manual overides
Devicetree bindings according to standards

Mike Looijmans (2):
  dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159
    bindings
  drm: bridge: Add TI tmds181 and sn65dp159 driver

 .../bindings/display/bridge/ti,tmds181.yaml   | 152 +++++++
 drivers/gpu/drm/bridge/Kconfig                |  11 +
 drivers/gpu/drm/bridge/Makefile               |   1 +
 drivers/gpu/drm/bridge/ti-tmds181.c           | 401 ++++++++++++++++++
 4 files changed, 565 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
 create mode 100644 drivers/gpu/drm/bridge/ti-tmds181.c

-- 
2.43.0

base-commit: 53e760d8949895390e256e723e7ee46618310361
branch: drm-ti-tmds181

Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Please consider the environment before printing this e-mail

[PATCH v4 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159 bindings

[Mike Looijmans]

Add DT binding document for TI TMDS181 and SN65DP159 HDMI retimers.

The two chips have similar register maps, but different applications
(source vs. sink).

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>

---

Changes in v4:
Use fallback compatible

Changes in v3:
Fix duplicate links
Add vcc-supply and vdd-supply
Fix missing type for ti,slew-rate

Changes in v2:
Document driver specific bindings like slew-rate and threshold

 .../bindings/display/bridge/ti,tmds181.yaml   | 152 ++++++++++++++++++
 1 file changed, 152 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
new file mode 100644
index 000000000000..c6387c482431
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
@@ -0,0 +1,152 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/ti,tmds181.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TMDS181 and SN65DP159 HDMI retimer/redriver chips
+
+maintainers:
+  - Mike Looijmans <mike.looijmans@topic.nl>
+
+description: |
+  Texas Instruments TMDS181 and SN65DP159 retimer and redriver chips.
+  https://www.ti.com/product/TMDS181
+  https://www.ti.com/product/SN65DP159
+
+properties:
+  compatible:
+    oneOf:
+      - const: ti,tmds181
+      - items:
+          - const: ti,sn65dp159
+          - const: ti,tmds181
+
+  reg:
+    enum:
+      - 0x5b
+      - 0x5c
+      - 0x5d
+      - 0x5e
+
+  oe-gpios:
+    maxItems: 1
+    description: GPIO specifier for OE pin (active high).
+
+  vdd-supply:
+    description: Core power supply, 1.1V
+
+  vcc-supply:
+    description: IO power supply, 3.3V
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description: Video port for HDMI (ish) input
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+      port@1:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description: Video port for HDMI output (panel or bridge)
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+    required:
+      - port@0
+      - port@1
+
+  ti,source-mode:
+    type: boolean
+    description:
+      Force chip to operate in "source" mode. Allows to use
+      a TMDS181 chip (which defaults to sink) as cable driver.
+
+  ti,sink-mode:
+    type: boolean
+    description:
+      Force chip to operate in "sink" mode. Allows to use
+      a DP159 chip (defaults to source) for incoming signals.
+
+  ti,retimer-threshold-hz:
+    minimum: 25000000
+    maximum: 600000000
+    default: 200000000
+    description:
+      Cross-over point. Up until this pixel clock frequency
+      the chip remains in the low-power redriver mode. Above
+      the threshold the chip should operate in retimer mode.
+
+  ti,dvi-mode:
+    type: boolean
+    description: Makes the DP159 chip operate in DVI mode.
+
+  ti,slew-rate:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 3
+    default: 3
+    description: Set slew rate, 0 is slowest, 3 is fastest.
+
+  ti,disable-equalizer:
+    type: boolean
+    description: Disable the equalizer (to save power).
+
+  ti,adaptive-equalizer:
+    type: boolean
+    description: Set the equalizer to adaptive mode.
+
+required:
+  - compatible
+  - reg
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bridge@5b {
+            compatible = "ti,sn65dp159", "ti,tmds181";
+            reg = <0x5b>;
+
+            oe-gpios = <&gpio2 1 GPIO_ACTIVE_HIGH>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+
+                    endpoint {
+                        remote-endpoint = <&encoder_out>;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+
+                    endpoint {
+                        remote-endpoint = <&hdmi_connector_in>;
+                    };
+                };
+            };
+        };
+    };
-- 
2.43.0


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Please consider the environment before printing this e-mail

Re: [PATCH v4 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159 bindings

[Krzysztof Kozlowski]

On Mon, Sep 01, 2025 at 04:29:01PM +0200, Mike Looijmans wrote:
>  .../bindings/display/bridge/ti,tmds181.yaml   | 152 ++++++++++++++++++
>  1 file changed, 152 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
> new file mode 100644
> index 000000000000..c6387c482431
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
> @@ -0,0 +1,152 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/ti,tmds181.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TMDS181 and SN65DP159 HDMI retimer/redriver chips
> +
> +maintainers:
> +  - Mike Looijmans <mike.looijmans@topic.nl>
> +
> +description: |
> +  Texas Instruments TMDS181 and SN65DP159 retimer and redriver chips.
> +  https://www.ti.com/product/TMDS181
> +  https://www.ti.com/product/SN65DP159
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: ti,tmds181
> +      - items:
> +          - const: ti,sn65dp159
> +          - const: ti,tmds181
> +
> +  reg:
> +    enum:
> +      - 0x5b
> +      - 0x5c
> +      - 0x5d
> +      - 0x5e
> +
> +  oe-gpios:
> +    maxItems: 1
> +    description: GPIO specifier for OE pin (active high).

So that's reset-gpios or powerdown-gpios (see gpio-consumer-common). At
least datasheet calls them that in one place.

> +
> +  vdd-supply:
> +    description: Core power supply, 1.1V
> +
> +  vcc-supply:
> +    description: IO power supply, 3.3V
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description: Video port for HDMI (ish) input
> +
> +        properties:
> +          endpoint:
> +            $ref: /schemas/media/video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description: Video port for HDMI output (panel or bridge)
> +
> +        properties:
> +          endpoint:
> +            $ref: /schemas/media/video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +    required:
> +      - port@0
> +      - port@1
> +
> +  ti,source-mode:
> +    type: boolean
> +    description:
> +      Force chip to operate in "source" mode. Allows to use
> +      a TMDS181 chip (which defaults to sink) as cable driver.
> +
> +  ti,sink-mode:

Aren't these two mutually exclusive? Can same device operate in source
and in sink mode simultaneously?


> +    type: boolean
> +    description:
> +      Force chip to operate in "sink" mode. Allows to use
> +      a DP159 chip (defaults to source) for incoming signals.
> +
> +  ti,retimer-threshold-hz:
> +    minimum: 25000000
> +    maximum: 600000000
> +    default: 200000000
> +    description:
> +      Cross-over point. Up until this pixel clock frequency
> +      the chip remains in the low-power redriver mode. Above
> +      the threshold the chip should operate in retimer mode.
> +
> +  ti,dvi-mode:
> +    type: boolean
> +    description: Makes the DP159 chip operate in DVI mode.

This suggest it is not applicable to TMDS, so you need to restrict it to
disallow it there (see example-schema).

Actually several other properties say they are applicable only to DP159.

> +
> +  ti,slew-rate:

Common property is "slew-rate" - see pincfg-node.yaml

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 3
> +    default: 3
> +    description: Set slew rate, 0 is slowest, 3 is fastest.
> +
> +  ti,disable-equalizer:
> +    type: boolean
> +    description: Disable the equalizer (to save power).
> +
> +  ti,adaptive-equalizer:
> +    type: boolean
> +    description: Set the equalizer to adaptive mode.

Can equalizer be disabled and adaptive the same time?

> +
> +required:
> +  - compatible
> +  - reg
> +  - ports
> +
> +additionalProperties: false

Best regards,
Krzysztof

Re: [PATCH v4 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159 bindings

[Mike Looijmans]

On 9/2/25 08:53, Krzysztof Kozlowski wrote:
> On Mon, Sep 01, 2025 at 04:29:01PM +0200, Mike Looijmans wrote:
>>   .../bindings/display/bridge/ti,tmds181.yaml   | 152 ++++++++++++++++++
>>   1 file changed, 152 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
>> new file mode 100644
>> index 000000000000..c6387c482431
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tmds181.yaml
>> @@ -0,0 +1,152 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/bridge/ti,tmds181.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: TMDS181 and SN65DP159 HDMI retimer/redriver chips
>> +
>> +maintainers:
>> +  - Mike Looijmans <mike.looijmans@topic.nl>
>> +
>> +description: |
>> +  Texas Instruments TMDS181 and SN65DP159 retimer and redriver chips.
>> +  https://www.ti.com/product/TMDS181
>> +  https://www.ti.com/product/SN65DP159
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - const: ti,tmds181
>> +      - items:
>> +          - const: ti,sn65dp159
>> +          - const: ti,tmds181
>> +
>> +  reg:
>> +    enum:
>> +      - 0x5b
>> +      - 0x5c
>> +      - 0x5d
>> +      - 0x5e
>> +
>> +  oe-gpios:
>> +    maxItems: 1
>> +    description: GPIO specifier for OE pin (active high).
> So that's reset-gpios or powerdown-gpios (see gpio-consumer-common). At
> least datasheet calls them that in one place.

reset-gpios fits reasonably. Though the pin is labeled "OE".

(Our experience: The pin must be driven low until the power supplies are 
up. Failure to do so may put the chip in an error state, pulling the I2C 
lines low, that only a power cycle may resolve. So it's close enough to 
"reset")

>> +
>> +  vdd-supply:
>> +    description: Core power supply, 1.1V
>> +
>> +  vcc-supply:
>> +    description: IO power supply, 3.3V
>> +
>> +  ports:
>> +    $ref: /schemas/graph.yaml#/properties/ports
>> +
>> +    properties:
>> +      port@0:
>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>> +        unevaluatedProperties: false
>> +        description: Video port for HDMI (ish) input
>> +
>> +        properties:
>> +          endpoint:
>> +            $ref: /schemas/media/video-interfaces.yaml#
>> +            unevaluatedProperties: false
>> +
>> +      port@1:
>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>> +        unevaluatedProperties: false
>> +        description: Video port for HDMI output (panel or bridge)
>> +
>> +        properties:
>> +          endpoint:
>> +            $ref: /schemas/media/video-interfaces.yaml#
>> +            unevaluatedProperties: false
>> +
>> +    required:
>> +      - port@0
>> +      - port@1
>> +
>> +  ti,source-mode:
>> +    type: boolean
>> +    description:
>> +      Force chip to operate in "source" mode. Allows to use
>> +      a TMDS181 chip (which defaults to sink) as cable driver.
>> +
>> +  ti,sink-mode:
> Aren't these two mutually exclusive? Can same device operate in source
> and in sink mode simultaneously?

They're exclusive, yes. Will add that.


>> +    type: boolean
>> +    description:
>> +      Force chip to operate in "sink" mode. Allows to use
>> +      a DP159 chip (defaults to source) for incoming signals.
>> +
>> +  ti,retimer-threshold-hz:
>> +    minimum: 25000000
>> +    maximum: 600000000
>> +    default: 200000000
>> +    description:
>> +      Cross-over point. Up until this pixel clock frequency
>> +      the chip remains in the low-power redriver mode. Above
>> +      the threshold the chip should operate in retimer mode.
>> +
>> +  ti,dvi-mode:
>> +    type: boolean
>> +    description: Makes the DP159 chip operate in DVI mode.
> This suggest it is not applicable to TMDS, so you need to restrict it to
> disallow it there (see example-schema).
>
> Actually several other properties say they are applicable only to DP159.

Will do.


>
>> +
>> +  ti,slew-rate:
> Common property is "slew-rate" - see pincfg-node.yaml

Will change to slew-rate.


>
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 0
>> +    maximum: 3
>> +    default: 3
>> +    description: Set slew rate, 0 is slowest, 3 is fastest.
>> +
>> +  ti,disable-equalizer:
>> +    type: boolean
>> +    description: Disable the equalizer (to save power).
>> +
>> +  ti,adaptive-equalizer:
>> +    type: boolean
>> +    description: Set the equalizer to adaptive mode.
> Can equalizer be disabled and adaptive the same time?

No, since "adaptive" is the default, I'll just remove that option.


>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - ports
>> +
>> +additionalProperties: false
> Best regards,
> Krzysztof
>

-- 
Mike Looijmans

Re: [PATCH v4 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159 bindings

[Krzysztof Kozlowski]

On 02/09/2025 10:46, Mike Looijmans wrote:
>>> +          endpoint:
>>> +            $ref: /schemas/media/video-interfaces.yaml#
>>> +            unevaluatedProperties: false
>>> +
>>> +      port@1:
>>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>>> +        unevaluatedProperties: false
>>> +        description: Video port for HDMI output (panel or bridge)
>>> +
>>> +        properties:
>>> +          endpoint:
>>> +            $ref: /schemas/media/video-interfaces.yaml#
>>> +            unevaluatedProperties: false
>>> +
>>> +    required:
>>> +      - port@0
>>> +      - port@1
>>> +
>>> +  ti,source-mode:
>>> +    type: boolean
>>> +    description:
>>> +      Force chip to operate in "source" mode. Allows to use
>>> +      a TMDS181 chip (which defaults to sink) as cable driver.
>>> +
>>> +  ti,sink-mode:
>> Aren't these two mutually exclusive? Can same device operate in source
>> and in sink mode simultaneously?
> 
> They're exclusive, yes. Will add that.

Then either define constraints per variant in if:then: or maybe better
use string enum. Not sure what applies where, so tricky to say which
choice is better.

Best regards,
Krzysztof

Re: [PATCH v4 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159 bindings

[Mike Looijmans]

On 9/2/25 15:46, Krzysztof Kozlowski wrote:
> On 02/09/2025 10:46, Mike Looijmans wrote:
>>>> +          endpoint:
>>>> +            $ref: /schemas/media/video-interfaces.yaml#
>>>> +            unevaluatedProperties: false
>>>> +
>>>> +      port@1:
>>>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>>>> +        unevaluatedProperties: false
>>>> +        description: Video port for HDMI output (panel or bridge)
>>>> +
>>>> +        properties:
>>>> +          endpoint:
>>>> +            $ref: /schemas/media/video-interfaces.yaml#
>>>> +            unevaluatedProperties: false
>>>> +
>>>> +    required:
>>>> +      - port@0
>>>> +      - port@1
>>>> +
>>>> +  ti,source-mode:
>>>> +    type: boolean
>>>> +    description:
>>>> +      Force chip to operate in "source" mode. Allows to use
>>>> +      a TMDS181 chip (which defaults to sink) as cable driver.
>>>> +
>>>> +  ti,sink-mode:
>>> Aren't these two mutually exclusive? Can same device operate in source
>>> and in sink mode simultaneously?
>> They're exclusive, yes. Will add that.
> Then either define constraints per variant in if:then: or maybe better
> use string enum. Not sure what applies where, so tricky to say which
> choice is better.
>
> Best regards,
> Krzysztof

Since there's already going to be an "if" block, it simplifies parsing 
to keep the booleans. My first attempt was this, but that doesn't work 
as I'd expect. Adding "slew-rate" to the example dts results in an 
error, so apparently the "if" block doesn't do what I think it would and 
I haven't figured out yet what the correct syntax must be:

if:
   properties:
     compatible:
       contains:
         const: ti,sn65dp159
then:
   properties:
     ti,sink-mode:
       type: boolean
       description:
         Force chip to operate in "sink" mode. Allows to use
         a DP159 chip (defaults to source) for incoming signals.
     ti,dvi-mode:
       type: boolean
       description: Makes the DP159 chip operate in DVI mode.
     slew-rate:
       $ref: /schemas/types.yaml#/definitions/uint32
       minimum: 0
       maximum: 3
       default: 3
       description: Set slew rate, 0 is slowest, 3 is fastest.
else:
   properties:
     ti,source-mode:
       type: boolean
       description:
         Force chip to operate in "source" mode. Allows to use
         a TMDS181 chip (which defaults to sink) as cable driver.



-- 
Mike Looijmans
System Expert

TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Re: [PATCH v4 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159 bindings

[Mike Looijmans]

On 9/2/25 16:41, Mike Looijmans wrote:
> On 9/2/25 15:46, Krzysztof Kozlowski wrote:
>> On 02/09/2025 10:46, Mike Looijmans wrote:
>>>>> +          endpoint:
>>>>> +            $ref: /schemas/media/video-interfaces.yaml#
>>>>> +            unevaluatedProperties: false
>>>>> +
>>>>> +      port@1:
>>>>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>>>>> +        unevaluatedProperties: false
>>>>> +        description: Video port for HDMI output (panel or bridge)
>>>>> +
>>>>> +        properties:
>>>>> +          endpoint:
>>>>> +            $ref: /schemas/media/video-interfaces.yaml#
>>>>> +            unevaluatedProperties: false
>>>>> +
>>>>> +    required:
>>>>> +      - port@0
>>>>> +      - port@1
>>>>> +
>>>>> +  ti,source-mode:
>>>>> +    type: boolean
>>>>> +    description:
>>>>> +      Force chip to operate in "source" mode. Allows to use
>>>>> +      a TMDS181 chip (which defaults to sink) as cable driver.
>>>>> +
>>>>> +  ti,sink-mode:
>>>> Aren't these two mutually exclusive? Can same device operate in source
>>>> and in sink mode simultaneously?
>>> They're exclusive, yes. Will add that.
>> Then either define constraints per variant in if:then: or maybe better
>> use string enum. Not sure what applies where, so tricky to say which
>> choice is better.
>>
>> Best regards,
>> Krzysztof
>
> Since there's already going to be an "if" block, it simplifies parsing 
> to keep the booleans. My first attempt was this, but that doesn't work 
> as I'd expect. Adding "slew-rate" to the example dts results in an 
> error, so apparently the "if" block doesn't do what I think it would 
> and I haven't figured out yet what the correct syntax must be:
>
> if:
>   properties:
>     compatible:
>       contains:
>         const: ti,sn65dp159
> then:
>   properties:
>     ti,sink-mode:
>       type: boolean
>       description:
>         Force chip to operate in "sink" mode. Allows to use
>         a DP159 chip (defaults to source) for incoming signals.
>     ti,dvi-mode:
>       type: boolean
>       description: Makes the DP159 chip operate in DVI mode.
>     slew-rate:
>       $ref: /schemas/types.yaml#/definitions/uint32
>       minimum: 0
>       maximum: 3
>       default: 3
>       description: Set slew rate, 0 is slowest, 3 is fastest.
> else:
>   properties:
>     ti,source-mode:
>       type: boolean
>       description:
>         Force chip to operate in "source" mode. Allows to use
>         a TMDS181 chip (which defaults to sink) as cable driver.
>
Hmm, apparently one cannot "add" properties in the "if:" block? The 
opposite approach, disallowing properties, works, e.g.:

if:
   properties:
     compatible:
       contains:
         const: ti,sn65dp159
then:
   properties:
     ti,source-mode: false
else:
   properties:
     ti,sink-mode: false
     ti,dvi-mode: false
     slew-rate: false


This produces the wanted behavior, though it looks counter-intuitive to me.


-- 
Mike Looijmans

Re: [PATCH v4 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159 bindings

[Krzysztof Kozlowski]

On 02/09/2025 16:52, Mike Looijmans wrote:
>> if:
>>   properties:
>>     compatible:
>>       contains:
>>         const: ti,sn65dp159
>> then:
>>   properties:
>>     ti,sink-mode:
>>       type: boolean
>>       description:
>>         Force chip to operate in "sink" mode. Allows to use
>>         a DP159 chip (defaults to source) for incoming signals.
>>     ti,dvi-mode:
>>       type: boolean
>>       description: Makes the DP159 chip operate in DVI mode.
>>     slew-rate:
>>       $ref: /schemas/types.yaml#/definitions/uint32
>>       minimum: 0
>>       maximum: 3
>>       default: 3
>>       description: Set slew rate, 0 is slowest, 3 is fastest.
>> else:
>>   properties:
>>     ti,source-mode:
>>       type: boolean
>>       description:
>>         Force chip to operate in "source" mode. Allows to use
>>         a TMDS181 chip (which defaults to sink) as cable driver.
>>
> Hmm, apparently one cannot "add" properties in the "if:" block? The 

One can, but should not. Define properties in top level and disallow them.

See also example-schema.


> opposite approach, disallowing properties, works, e.g.:
> 
> if:
>    properties:
>      compatible:
>        contains:
>          const: ti,sn65dp159
> then:
>    properties:
>      ti,source-mode: false
> else:
>    properties:
>      ti,sink-mode: false
>      ti,dvi-mode: false
>      slew-rate: false


So any device can work in any mode? Then this should be just enum with
different defaults.

Best regards,
Krzysztof

Re: [PATCH v4 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159 bindings

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]

On Mon, Sep 01, 2025 at 04:29:01PM +0200, Mike Looijmans wrote:
> +  ti,retimer-threshold-hz:
> +    minimum: 25000000
> +    maximum: 600000000
> +    default: 200000000
> +    description:
> +      Cross-over point. Up until this pixel clock frequency
> +      the chip remains in the low-power redriver mode. Above
> +      the threshold the chip should operate in retimer mode.

Why should anyone want to tune this at the firmware level?

> +  ti,dvi-mode:
> +    type: boolean
> +    description: Makes the DP159 chip operate in DVI mode.

Ditto. Both describe policy, not hardware.

> +  ti,slew-rate:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 3
> +    default: 3
> +    description: Set slew rate, 0 is slowest, 3 is fastest.
> +
> +  ti,disable-equalizer:
> +    type: boolean
> +    description: Disable the equalizer (to save power).

Why shouldn't we disable all the time then? Again, it looks like a
policy, and not something that should be set in stone in the firmware.

> +  ti,adaptive-equalizer:
> +    type: boolean
> +    description: Set the equalizer to adaptive mode.

Ditto.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v4 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159 bindings

[Mike Looijmans]

On 02-09-2025 19:29, Maxime Ripard wrote:
> On Mon, Sep 01, 2025 at 04:29:01PM +0200, Mike Looijmans wrote:
>> +  ti,retimer-threshold-hz:
>> +    minimum: 25000000
>> +    maximum: 600000000
>> +    default: 200000000
>> +    description:
>> +      Cross-over point. Up until this pixel clock frequency
>> +      the chip remains in the low-power redriver mode. Above
>> +      the threshold the chip should operate in retimer mode.
> Why should anyone want to tune this at the firmware level?

It's a board property. You'd set this based on the hardware you've soldered 
on. If your clock and serdes are good quality, there's no need for the chip to 
be in retimer mode (it will consume more power and actually make the signal 
worse). At higher speeds, that situation may change, hence the need for a way 
to describe that. The chip has a similar function built in, but with only 2 
choices of cross-over point.

To tune these parameters (retimer, equalizer), you'll probably have to take 
your equipment to a test facility (like we did). It's not something that 
end-users would want to tune themselves.

Most of these settings can also be done using pin strapping. I guess it'd be 
helpful if I added that to the description.


>> +  ti,dvi-mode:
>> +    type: boolean
>> +    description: Makes the DP159 chip operate in DVI mode.
> Ditto. Both describe policy, not hardware.

I would set this flag if I've soldered on a DVI connector instead of a HDMI 
one. I'd consider that hardware.


>> +  ti,slew-rate:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 0
>> +    maximum: 3
>> +    default: 3
>> +    description: Set slew rate, 0 is slowest, 3 is fastest.
>> +
>> +  ti,disable-equalizer:
>> +    type: boolean
>> +    description: Disable the equalizer (to save power).
> Why shouldn't we disable all the time then? Again, it looks like a
> policy, and not something that should be set in stone in the firmware.

Again, board property. The equalizer is there to make up for things like PCB 
losses (mismatch maybe?) or serdes running at (or beyond) its maximum. Again, 
depending on your board you may need this or not. It replaces a pinstrapping 
option.


>> +  ti,adaptive-equalizer:
>> +    type: boolean
>> +    description: Set the equalizer to adaptive mode.

It's the default setting of the chip so this flag will be removed in the next 
version.


> Maxime

Mike.

Re: [PATCH v4 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159 bindings

[Mike Looijmans]

On 03-09-2025 08:17, Mike Looijmans wrote:
> On 02-09-2025 19:29, Maxime Ripard wrote:
>> On Mon, Sep 01, 2025 at 04:29:01PM +0200, Mike Looijmans wrote:
>>> +  ti,retimer-threshold-hz:
>>> +    minimum: 25000000
>>> +    maximum: 600000000
>>> +    default: 200000000
>>> +    description:
>>> +      Cross-over point. Up until this pixel clock frequency
>>> +      the chip remains in the low-power redriver mode. Above
>>> +      the threshold the chip should operate in retimer mode.
>> Why should anyone want to tune this at the firmware level?
>
> It's a board property. You'd set this based on the hardware you've soldered 
> on. If your clock and serdes are good quality, there's no need for the chip 
> to be in retimer mode (it will consume more power and actually make the 
> signal worse). At higher speeds, that situation may change, hence the need 
> for a way to describe that. The chip has a similar function built in, but 
> with only 2 choices of cross-over point.
>
> To tune these parameters (retimer, equalizer), you'll probably have to take 
> your equipment to a test facility (like we did). It's not something that 
> end-users would want to tune themselves.
>
> Most of these settings can also be done using pin strapping. I guess it'd be 
> helpful if I added that to the description.

Looking back at the datasheet - once you enable the I2C interface of the pin, 
many of the pinstrapping options are no longer available (such as slew-rate, 
dvi/hdmi mode, equalizer and termination). The devicetree properties allow 
these settings to be applied again.

> ...
>
M.

Re: [PATCH v4 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159 bindings

[Dmitry Baryshkov]

On Wed, Sep 03, 2025 at 08:17:33AM +0200, Mike Looijmans wrote:
> On 02-09-2025 19:29, Maxime Ripard wrote:
> > On Mon, Sep 01, 2025 at 04:29:01PM +0200, Mike Looijmans wrote:
> > > +  ti,retimer-threshold-hz:
> > > +    minimum: 25000000
> > > +    maximum: 600000000
> > > +    default: 200000000
> > > +    description:
> > > +      Cross-over point. Up until this pixel clock frequency
> > > +      the chip remains in the low-power redriver mode. Above
> > > +      the threshold the chip should operate in retimer mode.
> > Why should anyone want to tune this at the firmware level?
> 
> It's a board property. You'd set this based on the hardware you've soldered
> on. If your clock and serdes are good quality, there's no need for the chip
> to be in retimer mode (it will consume more power and actually make the
> signal worse). At higher speeds, that situation may change, hence the need
> for a way to describe that. The chip has a similar function built in, but
> with only 2 choices of cross-over point.
> 
> To tune these parameters (retimer, equalizer), you'll probably have to take
> your equipment to a test facility (like we did). It's not something that
> end-users would want to tune themselves.
> 
> Most of these settings can also be done using pin strapping. I guess it'd be
> helpful if I added that to the description.
> 
> 
> > > +  ti,dvi-mode:
> > > +    type: boolean
> > > +    description: Makes the DP159 chip operate in DVI mode.
> > Ditto. Both describe policy, not hardware.
> 
> I would set this flag if I've soldered on a DVI connector instead of a HDMI
> one. I'd consider that hardware.

Do you need to set this if you have DVI monitor attached to the HDMI
connector via the passive cable?

As for the connector type, you can check it in the .atomic_enable by
checking drm_connector::connector_type.

-- 
With best wishes
Dmitry

Re: [PATCH v4 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI TMDS181 and SN65DP159 bindings

[Mike Looijmans]

On 03-09-2025 17:25, Dmitry Baryshkov wrote:
> On Wed, Sep 03, 2025 at 08:17:33AM +0200, Mike Looijmans wrote:
>> On 02-09-2025 19:29, Maxime Ripard wrote:
>>> On Mon, Sep 01, 2025 at 04:29:01PM +0200, Mike Looijmans wrote:
>>>> +  ti,retimer-threshold-hz:
>>>> +    minimum: 25000000
>>>> +    maximum: 600000000
>>>> +    default: 200000000
>>>> +    description:
>>>> +      Cross-over point. Up until this pixel clock frequency
>>>> +      the chip remains in the low-power redriver mode. Above
>>>> +      the threshold the chip should operate in retimer mode.
>>> Why should anyone want to tune this at the firmware level?
>> It's a board property. You'd set this based on the hardware you've soldered
>> on. If your clock and serdes are good quality, there's no need for the chip
>> to be in retimer mode (it will consume more power and actually make the
>> signal worse). At higher speeds, that situation may change, hence the need
>> for a way to describe that. The chip has a similar function built in, but
>> with only 2 choices of cross-over point.
>>
>> To tune these parameters (retimer, equalizer), you'll probably have to take
>> your equipment to a test facility (like we did). It's not something that
>> end-users would want to tune themselves.
>>
>> Most of these settings can also be done using pin strapping. I guess it'd be
>> helpful if I added that to the description.
>>
>>
>>>> +  ti,dvi-mode:
>>>> +    type: boolean
>>>> +    description: Makes the DP159 chip operate in DVI mode.
>>> Ditto. Both describe policy, not hardware.
>> I would set this flag if I've soldered on a DVI connector instead of a HDMI
>> one. I'd consider that hardware.
> Do you need to set this if you have DVI monitor attached to the HDMI
> connector via the passive cable?

In our testing, the setting didn't appear to have any effect on the video 
signals. Activating the I2C interface blocks the pinstrapping setting of this 
bit, that's the only reason it's there.

I've been digging through the datasheet. It appears that the DVI mode disables 
an I2C slave on the DDC bus, when in HDMI_SEL mode the datasheet says:  
"Device configured for HDMI (Adaptor ID block is readable through I2C or 
I2C-over-AUX."

Apparently a better name for the pin would be "disable-adaptor-id-block".

Another approach would be to just always disable it in the driver. It appears 
to make the retimer "visible" in the DDC path and even allows to change some 
settings that way (most noticeably, turn it on/off), which will interfere with 
the driver's functions. If we're directly controlling the device over I2C, one 
shouldn't be using this secondary interface.


> As for the connector type, you can check it in the .atomic_enable by
> checking drm_connector::connector_type.

Assuming this is the last bridge, yes. But see above, I suggest removing this 
from the bindings and just always set it in the driver.

M.