Message-ID: <CAD=FV=W45V-AZdbo4MBfZ-A9M4vf42Lda82s8iUoW5azVwM0hA@mail.gmail.com>
Date: Tue, 18 Mar 2025 12:51:19 -0700
From: Doug Anderson <dianders@...omium.org>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Cc: Dmitry Baryshkov <lumag@...nel.org>, Harikrishna Shenoy <a0512644@...com>, 
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, Krzysztof Kozlowski <krzk@...nel.org>, 
	Harikrishna Shenoy <h-shenoy@...com>, andrzej.hajda@...el.com, neil.armstrong@...aro.org, 
	rfoss@...nel.org, Laurent.pinchart@...asonboard.com, jonas@...boo.se, 
	jernej.skrabec@...il.com, simona@...ll.ch, maarten.lankhorst@...ux.intel.com, 
	mripard@...nel.org, tzimmermann@...e.de, robh@...nel.org, krzk+dt@...nel.org, 
	conor+dt@...nel.org, jani.nikula@...el.com, j-choudhary@...com, 
	sui.jingfeng@...ux.dev, viro@...iv.linux.org.uk, r-ravikumar@...com, 
	sjakhade@...ence.com, yamonkar@...ence.com, dri-devel@...ts.freedesktop.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] dt-bindings: drm/bridge: Add no-hpd property

Hi,

On Tue, Mar 18, 2025 at 8:50 AM Tomi Valkeinen
<tomi.valkeinen@...asonboard.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@...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@...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...


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

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

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

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.

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?

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.

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.

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.

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.


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.

-Doug

Powered by blists - more mailing lists