[PATCH v3 1/2] dt-bindings: drm/bridge: Add no-hpd property

Tue Apr 15 15:40:35 UTC 2025

On Tue, Apr 15, 2025 at 03:50:46PM +0300, Tomi Valkeinen wrote:
> Hi,
> 
> On 18/03/2025 21:51, Doug Anderson wrote:
> > Hi,
> > 
> > On Tue, Mar 18, 2025 at 8:50 AM Tomi Valkeinen
> > <tomi.valkeinen at ideasonboard.com> wrote:
> > > 
> > > Hi,
> > > 
> > > On 12/03/2025 14:52, Dmitry Baryshkov wrote:
> > > > On Wed, Mar 12, 2025 at 11:56:41AM +0530, Harikrishna Shenoy wrote:
> > > > > 
> > > > > 
> > > > > On 05/02/25 19:03, Dmitry Baryshkov wrote:
> > > > > > On Wed, Feb 05, 2025 at 12:52:52PM +0100, Krzysztof Kozlowski wrote:
> > > > > > > On 05/02/2025 12:50, Harikrishna Shenoy wrote:
> > > > > > > > From: Rahul T R <r-ravikumar at ti.com>
> > > > > > > > 
> > > > > > > > The mhdp bridge can work without its HPD pin hooked up to the connector,
> > > > > > > > but the current bridge driver throws an error when hpd line is not
> > > > > > > > connected to the connector. For such cases, we need an indication for
> > > > > > > > no-hpd, using which we can bypass the hpd detection and instead use the
> > > > > > > > auxiliary channels connected to the DP connector to confirm the
> > > > > > > > connection.
> > > > > > > > So add no-hpd property to the bindings, to disable hpd when not
> > > > > > > > connected or unusable due to DP0-HPD not connected to correct HPD
> > > > > > > > pin on SOC like in case of J721S2.
> > > > > > > > 
> > > > > > > > Signed-off-by: Rahul T R <r-ravikumar at ti.com>
> > > > > > > 
> > > > > > > Why are you sending over and over the same? You already got feedback.
> > > > > > > Then you send v2. You got the same feedback.
> > > > > > > 
> > > > > > > Now you send v3?
> > > > > > > 
> > > > > > > So the same feedback, but this time: NAK
> > 
> > I only spent a few minutes on it, but I couldn't find a v2. If there's
> > a link I'm happy to read it, but otherwise all my comments below are
> > without any context from prior verisons...
> 
> There was a link in the intro letter, although it seems to point to a reply
> to the v2 thread... Here's v2 intro letter:
> 
> https://lore.kernel.org/all/20230405142440.191939-1-j-choudhary@ti.com/
> 
> > > > > > Krzysztof's email forced me to take a look at the actual boards that you
> > > > > > are trying to enable. I couldn't stop by notice that the HPD signal
> > > > > > _is_ connected to a GPIO pin. Please stop hacking the bridge driver and
> > > > > > use the tools that are already provided to you: add the HPD pin to the
> > > > > > dp-controller device node. And then fix any possible issues coming from
> > > > > > the bridge driver not being able to handle HPD signals being delivered
> > > > > > by the DRM framework via the .hpd_notify() callback.
> > > > > > 
> > > > > > TL;DR: also a NAK from my side, add HPD gpio to dp-controller.
> > > > > > 
> > > > > We tried implementing a interrupt based HPD functionality as HPD signal is
> > > > > connected to GPIO0_18 pin, we were able to get interrupt based HPD working
> > > > > however to route this signal to SoC we are loosing audio capability due to
> > > > > MUX conflict. Due to board level limitations to
> > > > > route the signal to SoC, we will not be able to support interrupt
> > > > > based HPD and polling seems a possible way without loosing on audio
> > > > > capability.
> > > > 
> > > > Still NAK for the no-hpd property. HPD pin is a requirement for
> > > > DisplayPort to work, as it is used e.g. for the 'attention' IRQs being
> > > > sent by the DP sink. I'm not sure what kind of idea you HW engineers had
> > > > in mind.
> > > 
> > > It's true that for normal DP functionality the HPD is required, but
> > > afaik DP works "fine" without HPD too. This is not the first board that
> > > has DP connector, but doesn't have HPD, that I have seen or worked on.
> > > Polling can be used for the IRQs too.
> > 
> > I have less familiarity with DP than with eDP, but from what I know
> > I'd agree with Tomi here that it would probably work "fine" by some
> > definition of "fine". As Dmitry says, the "attention" IRQ wouldn't
> > work, but as I understand it that's not really part of the normal flow
> > of using DP. As evidence, some people have made "ti-sn65dsi86" (which
> > is supposed to be for eDP only) work with DP. While the ti-sn65dsi86
> > hardware _does_ support HPD, because of the forced (slow) debouncing
> > it turned out not to be terribly useful for eDP and we designed our
> > boards to route HPD to a GPIO. ...and because of that nobody ever
> > wrote the code to handle the "attention" IRQ. Apparently people are
> > still using this bridge w/ some success on DP monitors.
> > 
> > 
> > > For eDP HPD is optional, and some of the cases I've worked with involved
> > > a chip intended for eDP, but used with a full DP connector, and no HPD.
> > 
> > I definitely agree. The eDP spec explicitly states that HPD is
> > optional even though it's also documented to be an "attention" IRQ
> > there. We've hooked up large numbers of eDP panels and the lack of the
> > attention IRQ wasn't a problem.
> > 
> > 
> > > However, in this particular case the DP chip supports full DP, so it's
> > > just a board design error.
> > > 
> > > My question is, is J721s2 EVM something that's used widely? Or is it a
> > > rare board? If it's a rare one, maybe there's no point in solving this
> > > in upstream? But if it's widely used, I don't see why we wouldn't
> > > support it in upstream. The HW is broken, but we need to live with it.
> > > 
> > > Another question is, if eDP support is added to the cdns-mhdp driver,
> > > and used with a panel that doesn't have an HPD, how would that code look
> > > like? If that would be solved with a "no-hpd" property, identical to the
> > > one proposed in this series, then... There's even less reason to not
> > > support this.
> > > 
> > > Disclaimer: I didn't study the schematics, and I haven't thought or
> > > looked at how eDP is implemented in other drm drivers.
> > 
> > I spent lots of time working through this on ti-sn65dsi86. How it
> > works today (and how it's documented in the bindings) is that it's
> > possible to specify "no-hpd" on both the eDP panel node and on the
> > bridge chip. They mean different things.
> 
> As this text covers only eDP with Panel, I'll fill in some lines here about
> DP and HDMI connectors. I think we need to consider all the cases.
> 
> > The HPD-related properties that can be specified on the panel are
> > a) <nothing> - HPD hooked up to the bridge
> > b) no-hpd - HPD isn't hooked up at all
> > c) hpd-gpios - HPD is hooked up to a GPIO
> 
> For DP and HDMI connectors (dp-connector.yaml, hdmi-connector.yaml) we have
> only 'hpd-gpios'. There hasn't been need for no-hpd.
> 
> > The HPD-related properties that can be specified on ti-sn65dsi86 are:
> > a) <nothing> - HPD is hooked up to the bridge
> > b) no-hpd - HPD is not hooked up to the bridge
> 
> More generally speaking (also with HDMI), I think this is device specific.
> E.g. TFP410 doesn't have any kind of HPD support, so 'no-hpd' flag doesn't
> make sense. That said, probably most of the chips do have HPD support, and
> no-hpd is needed.

TFP410 has the EDGE/HTPLG pin, which should be used to monitor DVI /
HDMI hot plugging pin.

> 
> > NOTE: The "ti-sn65dsi86" controller needs to be programmed to ignore
> > its HPD line if HPD isn't hooked up. IIRC the hardware itself will not
> > transfer things over the AUX bus unless you either tell the controller
> > to ignore HPD or HPD is asserted.
> > 
> > 
> > Here are the combinations:
> > 
> > 1. Panel has no HPD-related properties, ti-sn65dsi86 has no
> > HPD-related properties
> > 
> > HPD is assumed to be hooked up to the dedicated HPD pin on the bridge.
> > Panel driver queries the bridge driver to know the status of HPD. In
> > Linux today ti-sn65dsi86 doesn't really implement this and the bridge
> > chip just has a big, fixed, non-optimized delay that tries to account
> > for the max delay any panel could need.
> 
> For the connector case, I don't think there's any assumption about HPD in
> this scenario. The connector does not handle the HPD, and it's up to the
> bridge to decide if it does something about it or not.

Hmm? display-connector definitely supports HPD for DVI, HDMI and DP
connectors.

> 
> > 2. Panel has "hpd-gpios", ti-sn65dsi86 has no HPD-related properties
> > 
> > In theory, I guess this would say that HPD goes _both_ to a GPIO and
> > to the HPD of the bridge. Maybe handy if the bridge doesn't provide a
> > "debounced" signal but still wants HPD hooked up to get the
> > "attention" IRQ?
> 
> Both the bridge and the panel handling HPD doesn't sound good to me...
> For the connector case, this case would mean that the connector driver
> handles the HPD, and the bridge doesn't. If the bridge has HPD support, I
> think it would make sense to disable it with 'no-hpd' property (i.e. this
> would then be case 5).

I'm not so sure. eDP / DP link has special meaning for HPD pin, so it
might be worth handling it on both sides. I can imaging the bridge
handling HPD pin to report attention IRQs, while GPIO is used for main
plug / unplug detection.

> 
> > 3. Panel has "no-hpd", ti-sn65dsi86 has no HPD-related properties
> > 
> > Doesn't really make sense. Says that panel should delay the max amount
> > but there's no good reason to do this if HPD is hooked up on
> > ti-sn65dsi86.
> 
> The connectors don't have no-hpd, so this doesn't apply there.

Connectors can simply skip the hpd-gpios property which should have the
same effect.

> 
> > 4. Panel has no HPD-related properties, ti-sn65dsi86 has "no-hpd"
> > 
> > Doesn't really make sense. Says that the panel should assume the
> > bridge has HPD hooked up but then the bridge doesn't.
> 
> For connectors, this would just mean no HPD at all connected (i.e. the case
> discussed in this series).

Same as above. Connectors don't require special handling for no HPD
case.

> 
> > 5. Panel has "hpd-gpios", ti-sn65dsi86 has "no-hpd"
> > 
> > This is the sc7180-trogdor config. Says the panel should use the GPIO
> > to read HPD for power sequencing purposes. Tells us that HPD is not
> > hooked up to the bridge chip so we should program the bridge chip to
> > ignore HPD.
> 
> For the connector case, this would be the same as 2, except the bridge
> requires disabling the HPD support via a property.

see above

> 
> > 6. Panel has "no-hpd", ti-sn65dsi86 has "no-hpd"
> > 
> > Says HPD is just not hooked up at all. panel-edp will delay for
> > "hpd-absent-delay-ms". Bridge chip should be programmed to tell the
> > hardware to ignore the HPD signal.
> 
> For connectors, this would be the same as 4.
> 
> > How we got there was fairly organic and quite a long time ago, but it
> > all sorta makes sense even if it is a bit convoluted.
> 
> I think it makes sense, and is quite similar for connectors.
> 
> Going back to this series, I think the no-hpd property makes sense to solve
> the TI issue.
> 
> However, my question about "is this needed in upstream" is still unanswered.
> If these boards are widely available, let's add this. If there are just a
> few boards here and there, with customers who anyway use TI BSP kernel, and
> the next revision of the board has the issue fixed, maybe it's not worth it?
> This change doesn't exactly make the driver cleaner or easier to maintain
> =).

I'd say, the driver needs some cleanup, if we are to land this patch.
I'd suggest to rework HPD enablement / disablement to use hpd_enable /
disable functions. Make no-hpd disable OP_HPD. Make actual detect / plug
handling tied to the hpd_notify callback, etc.

> 
>  Tomi
> 

-- 
With best wishes
Dmitry