From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Doug Anderson <dianders@chromium.org> Cc: Stephen Boyd <swboyd@chromium.org>, Andrzej Hajda <a.hajda@samsung.com>, David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>, Neil Armstrong <narmstrong@baylibre.com>, Rob Herring <robh+dt@kernel.org>, Sandeep Panda <spanda@codeaurora.org>, Jonas Karlman <jonas@kwiboo.se>, Bjorn Andersson <bjorn.andersson@linaro.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, Jeffrey Hugo <jeffrey.l.hugo@gmail.com>, Jernej Skrabec <jernej.skrabec@siol.net>, linux-arm-msm <linux-arm-msm@vger.kernel.org>, Rob Clark <robdclark@chromium.org>, dri-devel <dri-devel@lists.freedesktop.org>, LKML <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 2/3] dt-bindings: drm/bridge: ti-sn65dsi86: Add hpd-gpios to the bindings Date: Thu, 16 Apr 2020 03:54:09 +0300 [thread overview] Message-ID: <20200416005409.GR4758@pendragon.ideasonboard.com> (raw) In-Reply-To: <CAD=FV=U1U7y_U4+zySzA9e_uYE0ECdM1Bd-ew0OxG3ciqjRVSA@mail.gmail.com> Hi Doug, On Wed, Apr 15, 2020 at 04:49:00PM -0700, Doug Anderson wrote: > On Wed, Apr 15, 2020 at 1:33 PM Laurent Pinchart 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@chromium.org> > > > > --- > > > > > > > > .../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. If you were using it for DP, you would need a DT node for the DP connector (with bindings to be added to Documentation/devicetree/bindings/display/connector/, similar to the ones we already have for other connectors). That DT node should reference the HPD pin GPIO. The bridge driver for the connector (drivers/gpu/drm/bridge/display-connector.c) would then handle HPD. The good news is that it already does :-) > 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. Bridge drivers should only implement support for features available from the corresponding hardware. If an HPD signal is connected to a dedicated pin of the bridge, and the bridge can generate an interrupt and expose the HPD status through I2C, then it should implement HPD-related operations. If the HPD pin from the connector is hooked up to a GPIO of the SoC, it should be handled by the connector bridge driver. -- Regards, Laurent Pinchart
WARNING: multiple messages have this Message-ID (diff)
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Doug Anderson <dianders@chromium.org> Cc: Rob Clark <robdclark@chromium.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, Jernej Skrabec <jernej.skrabec@siol.net>, Neil Armstrong <narmstrong@baylibre.com>, David Airlie <airlied@linux.ie>, linux-arm-msm <linux-arm-msm@vger.kernel.org>, Jonas Karlman <jonas@kwiboo.se>, Jeffrey Hugo <jeffrey.l.hugo@gmail.com>, Sandeep Panda <spanda@codeaurora.org>, dri-devel <dri-devel@lists.freedesktop.org>, Stephen Boyd <swboyd@chromium.org>, Andrzej Hajda <a.hajda@samsung.com>, Rob Herring <robh+dt@kernel.org>, Bjorn Andersson <bjorn.andersson@linaro.org>, LKML <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 2/3] dt-bindings: drm/bridge: ti-sn65dsi86: Add hpd-gpios to the bindings Date: Thu, 16 Apr 2020 03:54:09 +0300 [thread overview] Message-ID: <20200416005409.GR4758@pendragon.ideasonboard.com> (raw) In-Reply-To: <CAD=FV=U1U7y_U4+zySzA9e_uYE0ECdM1Bd-ew0OxG3ciqjRVSA@mail.gmail.com> Hi Doug, On Wed, Apr 15, 2020 at 04:49:00PM -0700, Doug Anderson wrote: > On Wed, Apr 15, 2020 at 1:33 PM Laurent Pinchart 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@chromium.org> > > > > --- > > > > > > > > .../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. If you were using it for DP, you would need a DT node for the DP connector (with bindings to be added to Documentation/devicetree/bindings/display/connector/, similar to the ones we already have for other connectors). That DT node should reference the HPD pin GPIO. The bridge driver for the connector (drivers/gpu/drm/bridge/display-connector.c) would then handle HPD. The good news is that it already does :-) > 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. Bridge drivers should only implement support for features available from the corresponding hardware. If an HPD signal is connected to a dedicated pin of the bridge, and the bridge can generate an interrupt and expose the HPD status through I2C, then it should implement HPD-related operations. If the HPD pin from the connector is hooked up to a GPIO of the SoC, it should be handled by the connector bridge driver. -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-04-16 0:54 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-04-15 15:48 [PATCH 1/3] dt-bindings: drm/bridge: ti-sn65dsi86: Convert to yaml Douglas Anderson 2020-04-15 15:48 ` Douglas Anderson 2020-04-15 15:48 ` [PATCH 2/3] dt-bindings: drm/bridge: ti-sn65dsi86: Add hpd-gpios to the bindings Douglas Anderson 2020-04-15 15:48 ` Douglas Anderson 2020-04-15 19:53 ` Stephen Boyd 2020-04-15 19:53 ` Stephen Boyd 2020-04-15 20:32 ` Laurent Pinchart 2020-04-15 20:32 ` Laurent Pinchart 2020-04-15 23:49 ` Doug Anderson 2020-04-15 23:49 ` Doug Anderson 2020-04-16 0:54 ` Laurent Pinchart [this message] 2020-04-16 0:54 ` Laurent Pinchart 2020-04-16 21:53 ` Doug Anderson 2020-04-16 21:53 ` Doug Anderson 2020-04-17 18:08 ` Laurent Pinchart 2020-04-17 18:08 ` Laurent Pinchart 2020-04-21 5:10 ` Doug Anderson 2020-04-21 5:10 ` Doug Anderson 2020-04-15 15:48 ` [PATCH 3/3] drm/bridge: ti-sn65dsi86: Allow one of the bridge GPIOs to be HPD Douglas Anderson 2020-04-15 15:48 ` Douglas Anderson 2020-04-15 19:50 ` [PATCH 1/3] dt-bindings: drm/bridge: ti-sn65dsi86: Convert to yaml Stephen Boyd 2020-04-15 19:50 ` Stephen Boyd 2020-04-15 20:31 ` Laurent Pinchart 2020-04-15 20:31 ` Laurent Pinchart 2020-04-21 5:07 ` Doug Anderson 2020-04-21 5:07 ` Doug Anderson
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200416005409.GR4758@pendragon.ideasonboard.com \ --to=laurent.pinchart@ideasonboard.com \ --cc=a.hajda@samsung.com \ --cc=airlied@linux.ie \ --cc=bjorn.andersson@linaro.org \ --cc=daniel@ffwll.ch \ --cc=devicetree@vger.kernel.org \ --cc=dianders@chromium.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=jeffrey.l.hugo@gmail.com \ --cc=jernej.skrabec@siol.net \ --cc=jonas@kwiboo.se \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=narmstrong@baylibre.com \ --cc=robdclark@chromium.org \ --cc=robh+dt@kernel.org \ --cc=spanda@codeaurora.org \ --cc=swboyd@chromium.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.