Re: [PATCH 2/3] dt-bindings: drm/bridge: ti-sn65dsi86: Add hpd-gpios to the bindings

[Doug Anderson]

Hi,

On Wed, Apr 15, 2020 at 1:33 PM Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Apr 15, 2020 at 12:53:02PM -0700, Stephen Boyd wrote:
> > Quoting Douglas Anderson (2020-04-15 08:48:40)
> > > Allow people to specify to use a GPIO for hot-plug-detect.  Add an
> > > example.
> > >
> > > NOTE: The current patch adding support for hpd-gpios to the Linux
> > > driver for hpd-gpios only adds enough support to the driver so that
> > > the bridge can use one of its own GPIOs.  The bindings, however, are
> > > written generically.
> > >
> > > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> > > ---
> > >
> > >  .../bindings/display/bridge/ti,sn65dsi86.yaml          | 10 +++++++++-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> > > index 8cacc6db33a9..554bfd003000 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> > > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> > > @@ -60,6 +60,10 @@ properties:
> > >      const: 1
> > >      description: See ../../pwm/pwm.yaml for description of the cell formats.
> > >
> > > +  hpd-gpios:
> > > +    maxItems: 1
> > > +    description: If present use the given GPIO for hot-plug-detect.
> >
> > Shouldn't this go in the panel node? And the panel driver should get the
> > gpio and poll it after powering up the panel? Presumably that's why we
> > have the no-hpd property in the simple panel binding vs. putting it here
> > in the bridge.
>
> Same question really, I think this belongs to the panel (or connector)
> node indeed.

Hrm.

To me "no-hpd" feels OK in the panel because the lack of a connection
is somewhat symmetric.  Thus it's OK to say either "HPD isn't hooked
up to the panel in this system" or "HPD isn't hooked up to the bridge
in this system" and both express the same thing (AKA that there is no
HPD connection between the bridge and the panel).  In the case of
"no-hpd" it's more convenient to express it on the panel side because
the panel driver is the one whose behavior has to change if HPD isn't
hooked up.  The panel datasheet is the one that says how long of a
delay we need if HPD isn't hooked up.

...but when you're talking about where the bridge driver should look
to find the HPD signal that it needs, that really feels like it should
be described as part of the bridge.  Specifically imagine we were
using our bridge for DP, not for eDP.  In that case simple-panel
wouldn't be involved because we could get any type of display plugged
in.  Thus it couldn't go in the panel node.  Here it feels clearer
that hpd-gpio needs to be a property of the bridge driver.

Looking at other usages of "hpd-gpio" in the kernel, it seems like the
usage I'm proposing is also common.  Grepping for "hpd-gpios" shows
numerous examples of "hpd-gpios" being defined at the display
controller level and (effectively) I believe the bridge is at the
equivalent level.


-Doug