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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <muyjifaaftf7zmgl755xng3dx5uzgttauqanfdqktdfdd6n3qc@jsev7gs4dwsi>
Date: Fri, 5 Sep 2025 03:18:05 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Mike Looijmans <mike.looijmans@...ic.nl>
Cc: Maxime Ripard <mripard@...nel.org>, dri-devel@...ts.freedesktop.org,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        Conor Dooley <conor+dt@...nel.org>, David Airlie <airlied@...il.com>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Jonas Karlman <jonas@...boo.se>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Rob Herring <robh@...nel.org>, Robert Foss <rfoss@...nel.org>,
        Simona Vetter <simona@...ll.ch>,
        Thomas Zimmermann <tzimmermann@...e.de>, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 1/2] dt-bindings: drm/bridge: ti-tmds181: Add TI
 TMDS181 and SN65DP159 bindings

On Thu, Sep 04, 2025 at 07:54:30AM +0200, Mike Looijmans wrote:
> 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.

If I understand correctly, it's a part of the DP++ standard, so it might
be a good idea to keep it enabled.

> 
> 
> > 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.

-- 
With best wishes
Dmitry