[PATCH v3 1/2] dt-bindings: display: bridge: add TI TDP158

Tue Jul 30 08:44:15 UTC 2024

On Wed, Jul 24, 2024 at 07:28:41PM GMT, Marc Gonzalez wrote:
> On 24/07/2024 19:25, Dmitry Baryshkov wrote:
> > On Wed, Jul 24, 2024 at 07:18:39PM GMT, Marc Gonzalez wrote:
> >> On 15/07/2024 16:42, Maxime Ripard wrote:
> >>> On Mon, Jul 08, 2024 at 11:29:46PM GMT, Dmitry Baryshkov wrote:
> >>>> On Mon, Jul 08, 2024 at 04:59:23PM GMT, Maxime Ripard wrote:
> >>>>> On Mon, Jul 01, 2024 at 05:36:18PM GMT, Marc Gonzalez wrote:
> >>>>>> On 01/07/2024 15:50, Maxime Ripard wrote:
> >>>>>>
> >>>>>>> On Thu, Jun 27, 2024 at 01:13:03PM GMT, Marc Gonzalez wrote:
> >>>>>>>
> >>>>>>>> TDP158 is an AC-coupled DVI / HDMI to TMDS level shifting Redriver.
> >>>>>>>> It supports DVI 1.0, HDMI 1.4b and 2.0b.
> >>>>>>>> It supports 4 TMDS channels, HPD, and a DDC interface.
> >>>>>>>> It supports dual power supply rails (1.1V on VDD, 3.3V on VCC)
> >>>>>>>> for power reduction. Several methods of power management are
> >>>>>>>> implemented to reduce overall power consumption.
> >>>>>>>> It supports fixed receiver EQ gain using I2C or pin strap to
> >>>>>>>> compensate for different lengths input cable or board traces.
> >>>>>>>>
> >>>>>>>> Features
> >>>>>>>>
> >>>>>>>> - AC-coupled TMDS or DisplayPort dual-mode physical layer input
> >>>>>>>> to HDMI 2.0b TMDS physical layer output supporting up to 6Gbps
> >>>>>>>> data rate, compatible with HDMI 2.0b electrical parameters
> >>>>>>>> - DisplayPort dual-mode standard version 1.1
> >>>>>>>> - Programmable fixed receiver equalizer up to 15.5dB
> >>>>>>>> - Global or independent high speed lane control, pre-emphasis
> >>>>>>>> and transmit swing, and slew rate control
> >>>>>>>> - I2C or pin strap programmable
> >>>>>>>> - Configurable as a DisplayPort redriver through I2C
> >>>>>>>> - Full lane swap on main lanes
> >>>>>>>> - Low power consumption (200 mW at 6Gbps, 8 mW in shutdown)
> >>>>>>>>
> >>>>>>>> https://www.ti.com/lit/ds/symlink/tdp158.pdf
> >>>>>>>>
> >>>>>>>> Signed-off-by: Marc Gonzalez <mgonzalez at freebox.fr>
> >>>>>>>> ---
> >>>>>>>>  .../bindings/display/bridge/ti,tdp158.yaml         | 51 ++++++++++++++++++++++
> >>>>>>>>  1 file changed, 51 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tdp158.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tdp158.yaml
> >>>>>>>> new file mode 100644
> >>>>>>>> index 0000000000000..21c8585c3bb2d
> >>>>>>>> --- /dev/null
> >>>>>>>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tdp158.yaml
> >>>>>>>> @@ -0,0 +1,51 @@
> >>>>>>>> +# SPDX-License-Identifier: GPL-2.0-only
> >>>>>>>> +%YAML 1.2
> >>>>>>>> +---
> >>>>>>>> +$id: http://devicetree.org/schemas/display/bridge/ti,tdp158.yaml#
> >>>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>>>>> +
> >>>>>>>> +title: TI TDP158 HDMI to TMDS Redriver
> >>>>>>>> +
> >>>>>>>> +maintainers:
> >>>>>>>> +  - Arnaud Vrac <avrac at freebox.fr>
> >>>>>>>> +  - Pierre-Hugues Husson <phhusson at freebox.fr>
> >>>>>>>> +
> >>>>>>>> +properties:
> >>>>>>>> +  compatible:
> >>>>>>>> +    const: ti,tdp158
> >>>>>>>> +
> >>>>>>>> +  reg:
> >>>>>>>> +    description: I2C address of the device
> >>>>>>>> +
> >>>>>>>> +  enable-gpios:
> >>>>>>>> +    description: GPIO controlling bridge enable
> >>>>>>>> +
> >>>>>>>> +  vcc-supply:
> >>>>>>>> +    description: Power supply 3.3V
> >>>>>>>> +
> >>>>>>>> +  vdd-supply:
> >>>>>>>> +    description: Power supply 1.1V
> >>>>>>>> +
> >>>>>>>> +  ports:
> >>>>>>>> +    $ref: /schemas/graph.yaml#/properties/ports
> >>>>>>>> +
> >>>>>>>> +    properties:
> >>>>>>>> +      port at 0:
> >>>>>>>> +        $ref: /schemas/graph.yaml#/properties/port
> >>>>>>>> +        description: Bridge input
> >>>>>>>> +
> >>>>>>>> +      port at 1:
> >>>>>>>> +        $ref: /schemas/graph.yaml#/properties/port
> >>>>>>>> +        description: Bridge output
> >>>>>>>> +
> >>>>>>>> +    required:
> >>>>>>>> +      - port at 0
> >>>>>>>> +      - port at 1
> >>>>>>>
> >>>>>>> The device supports DVI, HDMI or DP input, with various requirements and
> >>>>>>> capabilities depending on the input. Your binding doesn't address that.
> >>>>>>>
> >>>>>>> Similarly, it can do lane-swapping, so we should probably have a
> >>>>>>> property to describe what mapping we want to use.
> >>>>>>>
> >>>>>>> The i2c register access (and the whole behaviour of the device) is
> >>>>>>> constrained on the I2C_EN pin status, and you can't read it from the
> >>>>>>> device, so it's also something we need to have in the DT.
> >>>>>>
> >>>>>> We are using the device in its default configuration.
> >>>>>> (Power on via OE, then it works as expected)
> >>>>>
> >>>>> I know, but that doesn't really matter for a binding.
> >>>>>
> >>>>>> Can we leave any additional properties to be defined by whomever needs
> >>>>>> them in the future?
> >>>>>
> >>>>> If you can guarantee that doing so would be backward compatible, sure.
> >>>>> But that means being able to answer those questions with a reasonable
> >>>>> plan.
> >>>>
> >>>> I think proposed bindings are generic enough to cover other possible
> >>>> usecases in future.
> >>>
> >>> I don't think it is. The current binding is for a I2C device that
> >>> shouldn't be accessed through I2C.
> >>>
> >>> It's working for now because the driver doesn't do any access, so it's
> >>> all great, but as soon as we add support for the other case, then we'll
> >>> have to add a property that states that while it's an i2c device, it
> >>> shouldn't be accessed.
> >>>
> >>> And adding such a property is a compatibility-breaking change.
> >>
> >> Why do you say:
> >> "current binding is for a I2C device that
> >> shouldn't be accessed through I2C" ?
> >>
> >> As a matter of fact, my debug code queries the device ID using
> >> regmap_read() to make sure I set the correct I2C address.
> > 
> > Please note: bingdings do not cover the driver at all. The driver might
> > do whatever it wants. The bindings describe the hardware.
> 
> OK.
> How does the proposed binding say
> "I2C device shouldn't be accessed through I2C" ?

It doesn't, but then again, neither your commit log or cover letter say
"it can be accessed by i2c" either, so we went on the wrong track.

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 273 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20240730/01427726/attachment.sig>