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 > > <[email protected]> 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 <[email protected]> > > > > > > > > > > > > > > > > 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 <[email protected]> > > > > > > > > > > > > > > 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/[email protected]/ > > > > > > > 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
