| Message ID | 20250105190659.99941-1-marex@denx.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/4] drm: bridge: dw_hdmi: Add flag to indicate output port is optional | expand |
Hi Marek, Thank you for the patch. On Sun, Jan 05, 2025 at 08:06:03PM +0100, Marek Vasut wrote: > Add a flag meant purely to work around broken i.MX8MP DTs which enable > HDMI but do not contain the HDMI connector node. This flag allows such > DTs to work by creating the connector in the HDMI bridge driver. Do not > use this flag, do not proliferate this flag, please fix your DTs and add > the connector node this way: > > ``` > / { > hdmi-connector { > compatible = "hdmi-connector"; > label = "FIXME-Board-Specific-Connector-Label"; // Modify this > type = "a"; > > port { > hdmi_connector_in: endpoint { > remote-endpoint = <&hdmi_tx_out>; > }; > }; > }; > }; > > &hdmi_tx { > ... > > ports { > port@1 { > hdmi_tx_out: endpoint { > remote-endpoint = <&hdmi_connector_in>; > }; > }; > }; > }; > ``` Are there any in-tree DT sources that use the old bindings ? > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Andrzej Hajda <andrzej.hajda@intel.com> > Cc: David Airlie <airlied@gmail.com> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: Jernej Skrabec <jernej.skrabec@gmail.com> > Cc: Jonas Karlman <jonas@kwiboo.se> > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> > Cc: Liu Ying <victor.liu@nxp.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Neil Armstrong <neil.armstrong@linaro.org> > Cc: Pengutronix Kernel Team <kernel@pengutronix.de> > Cc: Robert Foss <rfoss@kernel.org> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: Simona Vetter <simona@ffwll.ch> > Cc: Stefan Agner <stefan@agner.ch> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: dri-devel@lists.freedesktop.org > Cc: imx@lists.linux.dev > Cc: linux-arm-kernel@lists.infradead.org > --- > V3: New patch > V4: - Add HDMI connector node addition example into commit message > - Bail from dw_hdmi_bridge_attach() if DRM_BRIDGE_ATTACH_NO_CONNECTOR > is set and there is no hdmi->next_bridge , so the connector can > be created in scanout driver. > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 ++++++-- > include/drm/bridge/dw_hdmi.h | 2 ++ > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 996733ed2c004..e84693faf46dc 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -2893,9 +2893,13 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge, > { > struct dw_hdmi *hdmi = bridge->driver_private; > > - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) > + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) { > + if (!hdmi->next_bridge) > + return 0; > + > return drm_bridge_attach(bridge->encoder, hdmi->next_bridge, > bridge, flags); > + } > > return dw_hdmi_connector_create(hdmi); > } > @@ -3298,7 +3302,7 @@ static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi) > hdmi->plat_data->output_port, > -1); > if (!remote) > - return -ENODEV; > + return hdmi->plat_data->output_port_optional ? 0 : -ENODEV; Let's complain loudly with a WARN_ON if output_port_optional is set. > > hdmi->next_bridge = of_drm_find_bridge(remote); > of_node_put(remote); > diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h > index 6a46baa0737cd..3bb6e633424a8 100644 > --- a/include/drm/bridge/dw_hdmi.h > +++ b/include/drm/bridge/dw_hdmi.h > @@ -127,6 +127,8 @@ struct dw_hdmi_plat_data { > struct regmap *regm; > > unsigned int output_port; > + /* Used purely by MX8MP HDMI to work around broken DTs without HDMI connector node. */ > + bool output_port_optional; > > unsigned long input_bus_encoding; > bool use_drm_infoframe;
On 1/6/25 12:22 AM, Laurent Pinchart wrote: > Hi Marek, Hi, > Thank you for the patch. > > On Sun, Jan 05, 2025 at 08:06:03PM +0100, Marek Vasut wrote: >> Add a flag meant purely to work around broken i.MX8MP DTs which enable >> HDMI but do not contain the HDMI connector node. This flag allows such >> DTs to work by creating the connector in the HDMI bridge driver. Do not >> use this flag, do not proliferate this flag, please fix your DTs and add >> the connector node this way: >> >> ``` >> / { >> hdmi-connector { >> compatible = "hdmi-connector"; >> label = "FIXME-Board-Specific-Connector-Label"; // Modify this >> type = "a"; >> >> port { >> hdmi_connector_in: endpoint { >> remote-endpoint = <&hdmi_tx_out>; >> }; >> }; >> }; >> }; >> >> &hdmi_tx { >> ... >> >> ports { >> port@1 { >> hdmi_tx_out: endpoint { >> remote-endpoint = <&hdmi_connector_in>; >> }; >> }; >> }; >> }; >> ``` > > Are there any in-tree DT sources that use the old bindings ? See https://lore.kernel.org/dri-devel/AM7PR04MB704688150ACD5D209290246A98092@AM7PR04MB7046.eurprd04.prod.outlook.com/ The rest is fixed, thanks.
On Mon, Jan 06, 2025 at 03:48:52AM +0100, Marek Vasut wrote: > On 1/6/25 12:22 AM, Laurent Pinchart wrote: > > Hi Marek, > > Hi, > > > Thank you for the patch. > > > > On Sun, Jan 05, 2025 at 08:06:03PM +0100, Marek Vasut wrote: > >> Add a flag meant purely to work around broken i.MX8MP DTs which enable > >> HDMI but do not contain the HDMI connector node. This flag allows such > >> DTs to work by creating the connector in the HDMI bridge driver. Do not > >> use this flag, do not proliferate this flag, please fix your DTs and add > >> the connector node this way: > >> > >> ``` > >> / { > >> hdmi-connector { > >> compatible = "hdmi-connector"; > >> label = "FIXME-Board-Specific-Connector-Label"; // Modify this > >> type = "a"; > >> > >> port { > >> hdmi_connector_in: endpoint { > >> remote-endpoint = <&hdmi_tx_out>; > >> }; > >> }; > >> }; > >> }; > >> > >> &hdmi_tx { > >> ... > >> > >> ports { > >> port@1 { > >> hdmi_tx_out: endpoint { > >> remote-endpoint = <&hdmi_connector_in>; > >> }; > >> }; > >> }; > >> }; > >> ``` > > > > Are there any in-tree DT sources that use the old bindings ? > > See > https://lore.kernel.org/dri-devel/AM7PR04MB704688150ACD5D209290246A98092@AM7PR04MB7046.eurprd04.prod.outlook.com/ Maybe I'm missing something obvious, but where is the patch series that moves the DT sources mentioned in that mail thread to the new bindings ? > The rest is fixed, thanks.
On 1/6/25 8:05 AM, Laurent Pinchart wrote: > On Mon, Jan 06, 2025 at 03:48:52AM +0100, Marek Vasut wrote: >> On 1/6/25 12:22 AM, Laurent Pinchart wrote: >>> Hi Marek, >> >> Hi, >> >>> Thank you for the patch. >>> >>> On Sun, Jan 05, 2025 at 08:06:03PM +0100, Marek Vasut wrote: >>>> Add a flag meant purely to work around broken i.MX8MP DTs which enable >>>> HDMI but do not contain the HDMI connector node. This flag allows such >>>> DTs to work by creating the connector in the HDMI bridge driver. Do not >>>> use this flag, do not proliferate this flag, please fix your DTs and add >>>> the connector node this way: >>>> >>>> ``` >>>> / { >>>> hdmi-connector { >>>> compatible = "hdmi-connector"; >>>> label = "FIXME-Board-Specific-Connector-Label"; // Modify this >>>> type = "a"; >>>> >>>> port { >>>> hdmi_connector_in: endpoint { >>>> remote-endpoint = <&hdmi_tx_out>; >>>> }; >>>> }; >>>> }; >>>> }; >>>> >>>> &hdmi_tx { >>>> ... >>>> >>>> ports { >>>> port@1 { >>>> hdmi_tx_out: endpoint { >>>> remote-endpoint = <&hdmi_connector_in>; >>>> }; >>>> }; >>>> }; >>>> }; >>>> ``` >>> >>> Are there any in-tree DT sources that use the old bindings ? >> >> See >> https://lore.kernel.org/dri-devel/AM7PR04MB704688150ACD5D209290246A98092@AM7PR04MB7046.eurprd04.prod.outlook.com/ > > Maybe I'm missing something obvious, but where is the patch series that > moves the DT sources mentioned in that mail thread to the new bindings ? Since this optional flag is added, that DT update series can be done separately.
On Mon, Jan 06, 2025 at 04:36:26PM +0100, Marek Vasut wrote: > On 1/6/25 8:05 AM, Laurent Pinchart wrote: > > On Mon, Jan 06, 2025 at 03:48:52AM +0100, Marek Vasut wrote: > >> On 1/6/25 12:22 AM, Laurent Pinchart wrote: > >>> Hi Marek, > >> > >> Hi, > >> > >>> Thank you for the patch. > >>> > >>> On Sun, Jan 05, 2025 at 08:06:03PM +0100, Marek Vasut wrote: > >>>> Add a flag meant purely to work around broken i.MX8MP DTs which enable > >>>> HDMI but do not contain the HDMI connector node. This flag allows such > >>>> DTs to work by creating the connector in the HDMI bridge driver. Do not > >>>> use this flag, do not proliferate this flag, please fix your DTs and add > >>>> the connector node this way: > >>>> > >>>> ``` > >>>> / { > >>>> hdmi-connector { > >>>> compatible = "hdmi-connector"; > >>>> label = "FIXME-Board-Specific-Connector-Label"; // Modify this > >>>> type = "a"; > >>>> > >>>> port { > >>>> hdmi_connector_in: endpoint { > >>>> remote-endpoint = <&hdmi_tx_out>; > >>>> }; > >>>> }; > >>>> }; > >>>> }; > >>>> > >>>> &hdmi_tx { > >>>> ... > >>>> > >>>> ports { > >>>> port@1 { > >>>> hdmi_tx_out: endpoint { > >>>> remote-endpoint = <&hdmi_connector_in>; > >>>> }; > >>>> }; > >>>> }; > >>>> }; > >>>> ``` > >>> > >>> Are there any in-tree DT sources that use the old bindings ? > >> > >> See > >> https://lore.kernel.org/dri-devel/AM7PR04MB704688150ACD5D209290246A98092@AM7PR04MB7046.eurprd04.prod.outlook.com/ > > > > Maybe I'm missing something obvious, but where is the patch series that > > moves the DT sources mentioned in that mail thread to the new bindings ? > > Since this optional flag is added, that DT update series can be done > separately. It can, but I'd like to see it merged in the same time frame as this series, so it should be posted (and reviewed and tested).
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 996733ed2c004..e84693faf46dc 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2893,9 +2893,13 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge, { struct dw_hdmi *hdmi = bridge->driver_private; - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) { + if (!hdmi->next_bridge) + return 0; + return drm_bridge_attach(bridge->encoder, hdmi->next_bridge, bridge, flags); + } return dw_hdmi_connector_create(hdmi); } @@ -3298,7 +3302,7 @@ static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi) hdmi->plat_data->output_port, -1); if (!remote) - return -ENODEV; + return hdmi->plat_data->output_port_optional ? 0 : -ENODEV; hdmi->next_bridge = of_drm_find_bridge(remote); of_node_put(remote); diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h index 6a46baa0737cd..3bb6e633424a8 100644 --- a/include/drm/bridge/dw_hdmi.h +++ b/include/drm/bridge/dw_hdmi.h @@ -127,6 +127,8 @@ struct dw_hdmi_plat_data { struct regmap *regm; unsigned int output_port; + /* Used purely by MX8MP HDMI to work around broken DTs without HDMI connector node. */ + bool output_port_optional; unsigned long input_bus_encoding; bool use_drm_infoframe;
Add a flag meant purely to work around broken i.MX8MP DTs which enable HDMI but do not contain the HDMI connector node. This flag allows such DTs to work by creating the connector in the HDMI bridge driver. Do not use this flag, do not proliferate this flag, please fix your DTs and add the connector node this way: ``` / { hdmi-connector { compatible = "hdmi-connector"; label = "FIXME-Board-Specific-Connector-Label"; // Modify this type = "a"; port { hdmi_connector_in: endpoint { remote-endpoint = <&hdmi_tx_out>; }; }; }; }; &hdmi_tx { ... ports { port@1 { hdmi_tx_out: endpoint { remote-endpoint = <&hdmi_connector_in>; }; }; }; }; ``` Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: David Airlie <airlied@gmail.com> Cc: Fabio Estevam <festevam@gmail.com> Cc: Jernej Skrabec <jernej.skrabec@gmail.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Liu Ying <victor.liu@nxp.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Neil Armstrong <neil.armstrong@linaro.org> Cc: Pengutronix Kernel Team <kernel@pengutronix.de> Cc: Robert Foss <rfoss@kernel.org> Cc: Sascha Hauer <s.hauer@pengutronix.de> Cc: Shawn Guo <shawnguo@kernel.org> Cc: Simona Vetter <simona@ffwll.ch> Cc: Stefan Agner <stefan@agner.ch> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: dri-devel@lists.freedesktop.org Cc: imx@lists.linux.dev Cc: linux-arm-kernel@lists.infradead.org --- V3: New patch V4: - Add HDMI connector node addition example into commit message - Bail from dw_hdmi_bridge_attach() if DRM_BRIDGE_ATTACH_NO_CONNECTOR is set and there is no hdmi->next_bridge , so the connector can be created in scanout driver. --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 ++++++-- include/drm/bridge/dw_hdmi.h | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-)