From: Jonas Karlman <jonas@kwiboo.se> To: Neil Armstrong <narmstrong@baylibre.com>, Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>, dri-devel@lists.freedesktop.org Cc: Jernej Skrabec <jernej.skrabec@siol.net>, Kieran Bingham <kieran.bingham@ideasonboard.com>, linux-renesas-soc@vger.kernel.org, Andrzej Hajda <a.hajda@samsung.com>, Sam Ravnborg <sam@ravnborg.org> Subject: Re: [PATCH 23/27] drm: bridge: dw-hdmi: Attach to next bridge if available Date: Tue, 26 May 2020 14:23:33 +0000 (UTC) [thread overview] Message-ID: <b803dc36-f3f1-f90c-ac53-302d24397c2b@kwiboo.se> (raw) In-Reply-To: <f75a9b4f-a283-53b1-ecb1-2bb6c9a278d6@baylibre.com> On 2020-05-26 14:50, Neil Armstrong wrote: > On 26/05/2020 03:15, Laurent Pinchart wrote: >> On all platforms except i.MX and Rockchip, the dw-hdmi DT bindings >> require a video output port connected to an HDMI sink (most likely an >> HDMI connector, in rare cases another bridges converting HDMI to another >> protocol). For those platforms, retrieve the next bridge and attach it >> from the dw-hdmi bridge attach handler. >> >> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> >> --- >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 52 ++++++++++++++++++++++- >> include/drm/bridge/dw_hdmi.h | 2 + >> 2 files changed, 53 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> index 6148a022569a..512e67bb1c32 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> @@ -143,6 +143,7 @@ struct dw_hdmi_phy_data { >> struct dw_hdmi { >> struct drm_connector connector; >> struct drm_bridge bridge; >> + struct drm_bridge *next_bridge; >> >> unsigned int version; >> >> @@ -2797,7 +2798,8 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge, >> struct dw_hdmi *hdmi = bridge->driver_private; >> >> if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) >> - return 0; >> + return drm_bridge_attach(bridge->encoder, hdmi->next_bridge, >> + bridge, flags); >> >> return dw_hdmi_connector_create(hdmi); >> } >> @@ -3179,6 +3181,50 @@ static void dw_hdmi_init_hw(struct dw_hdmi *hdmi) >> hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data); >> } >> >> +static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi) >> +{ >> + struct device_node *endpoint; >> + struct device_node *remote; >> + >> + if (!hdmi->plat_data->output_port) >> + return 0; >> + >> + endpoint = of_graph_get_endpoint_by_regs(hdmi->dev->of_node, >> + hdmi->plat_data->output_port, >> + -1); >> + if (!endpoint) { >> + /* >> + * Don't treat this as a fatal error as the Rockchip DW-HDMI >> + * binding doesn't make the output port mandatory. >> + */ >> + dev_dbg(hdmi->dev, "Missing endpoint in port@%u\n", >> + hdmi->plat_data->output_port); >> + return 0; After this series only rcar-du set output_port so this block should only run for rcar-du, for platforms without output_port the if-statement for !hdmi->plat_data->output_port already return success so you can probably return fatal error here. The comment is a little bit misleading because of the if-statement above or am I missing something? Regards, Jonas >> + } >> + >> + remote = of_graph_get_remote_port_parent(endpoint); >> + of_node_put(endpoint); >> + if (!remote) { >> + dev_err(hdmi->dev, "Endpoint in port@%u unconnected\n", >> + hdmi->plat_data->output_port); >> + return -ENODEV; >> + } >> + >> + if (!of_device_is_available(remote)) { >> + dev_err(hdmi->dev, "port@%u remote device is disabled\n", >> + hdmi->plat_data->output_port); >> + of_node_put(remote); >> + return -ENODEV; >> + } >> + >> + hdmi->next_bridge = of_drm_find_bridge(remote); >> + of_node_put(remote); >> + if (!hdmi->next_bridge) >> + return -EPROBE_DEFER; > > I'll be safer to print a warn for now until all platforms has been tested. > >> + >> + return 0; >> +} >> + >> static struct dw_hdmi * >> __dw_hdmi_probe(struct platform_device *pdev, >> const struct dw_hdmi_plat_data *plat_data) >> @@ -3216,6 +3262,10 @@ __dw_hdmi_probe(struct platform_device *pdev, >> mutex_init(&hdmi->cec_notifier_mutex); >> spin_lock_init(&hdmi->audio_lock); >> >> + ret = dw_hdmi_parse_dt(hdmi); >> + if (ret < 0) >> + return ERR_PTR(ret); >> + >> ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); >> if (ddc_node) { >> hdmi->ddc = of_get_i2c_adapter_by_node(ddc_node); >> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h >> index ea34ca146b82..8ebeb65d6371 100644 >> --- a/include/drm/bridge/dw_hdmi.h >> +++ b/include/drm/bridge/dw_hdmi.h >> @@ -126,6 +126,8 @@ struct dw_hdmi_phy_ops { >> struct dw_hdmi_plat_data { >> struct regmap *regm; >> >> + unsigned int output_port; >> + >> unsigned long input_bus_encoding; >> bool use_drm_infoframe; >> bool ycbcr_420_allowed; >> > > I must check on meson, since I'm not sure for now if the connector probes. > > Anyway, this looks fine. > > Reviewed-by: Neil Armstrong <narmstrong@baylibre.com> >
WARNING: multiple messages have this Message-ID (diff)
From: Jonas Karlman <jonas@kwiboo.se> To: Neil Armstrong <narmstrong@baylibre.com>, Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>, dri-devel@lists.freedesktop.org Cc: linux-renesas-soc@vger.kernel.org, Andrzej Hajda <a.hajda@samsung.com>, Jernej Skrabec <jernej.skrabec@siol.net>, Sam Ravnborg <sam@ravnborg.org>, Kieran Bingham <kieran.bingham@ideasonboard.com> Subject: Re: [PATCH 23/27] drm: bridge: dw-hdmi: Attach to next bridge if available Date: Tue, 26 May 2020 14:23:33 +0000 (UTC) [thread overview] Message-ID: <b803dc36-f3f1-f90c-ac53-302d24397c2b@kwiboo.se> (raw) In-Reply-To: <f75a9b4f-a283-53b1-ecb1-2bb6c9a278d6@baylibre.com> On 2020-05-26 14:50, Neil Armstrong wrote: > On 26/05/2020 03:15, Laurent Pinchart wrote: >> On all platforms except i.MX and Rockchip, the dw-hdmi DT bindings >> require a video output port connected to an HDMI sink (most likely an >> HDMI connector, in rare cases another bridges converting HDMI to another >> protocol). For those platforms, retrieve the next bridge and attach it >> from the dw-hdmi bridge attach handler. >> >> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> >> --- >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 52 ++++++++++++++++++++++- >> include/drm/bridge/dw_hdmi.h | 2 + >> 2 files changed, 53 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> index 6148a022569a..512e67bb1c32 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> @@ -143,6 +143,7 @@ struct dw_hdmi_phy_data { >> struct dw_hdmi { >> struct drm_connector connector; >> struct drm_bridge bridge; >> + struct drm_bridge *next_bridge; >> >> unsigned int version; >> >> @@ -2797,7 +2798,8 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge, >> struct dw_hdmi *hdmi = bridge->driver_private; >> >> if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) >> - return 0; >> + return drm_bridge_attach(bridge->encoder, hdmi->next_bridge, >> + bridge, flags); >> >> return dw_hdmi_connector_create(hdmi); >> } >> @@ -3179,6 +3181,50 @@ static void dw_hdmi_init_hw(struct dw_hdmi *hdmi) >> hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data); >> } >> >> +static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi) >> +{ >> + struct device_node *endpoint; >> + struct device_node *remote; >> + >> + if (!hdmi->plat_data->output_port) >> + return 0; >> + >> + endpoint = of_graph_get_endpoint_by_regs(hdmi->dev->of_node, >> + hdmi->plat_data->output_port, >> + -1); >> + if (!endpoint) { >> + /* >> + * Don't treat this as a fatal error as the Rockchip DW-HDMI >> + * binding doesn't make the output port mandatory. >> + */ >> + dev_dbg(hdmi->dev, "Missing endpoint in port@%u\n", >> + hdmi->plat_data->output_port); >> + return 0; After this series only rcar-du set output_port so this block should only run for rcar-du, for platforms without output_port the if-statement for !hdmi->plat_data->output_port already return success so you can probably return fatal error here. The comment is a little bit misleading because of the if-statement above or am I missing something? Regards, Jonas >> + } >> + >> + remote = of_graph_get_remote_port_parent(endpoint); >> + of_node_put(endpoint); >> + if (!remote) { >> + dev_err(hdmi->dev, "Endpoint in port@%u unconnected\n", >> + hdmi->plat_data->output_port); >> + return -ENODEV; >> + } >> + >> + if (!of_device_is_available(remote)) { >> + dev_err(hdmi->dev, "port@%u remote device is disabled\n", >> + hdmi->plat_data->output_port); >> + of_node_put(remote); >> + return -ENODEV; >> + } >> + >> + hdmi->next_bridge = of_drm_find_bridge(remote); >> + of_node_put(remote); >> + if (!hdmi->next_bridge) >> + return -EPROBE_DEFER; > > I'll be safer to print a warn for now until all platforms has been tested. > >> + >> + return 0; >> +} >> + >> static struct dw_hdmi * >> __dw_hdmi_probe(struct platform_device *pdev, >> const struct dw_hdmi_plat_data *plat_data) >> @@ -3216,6 +3262,10 @@ __dw_hdmi_probe(struct platform_device *pdev, >> mutex_init(&hdmi->cec_notifier_mutex); >> spin_lock_init(&hdmi->audio_lock); >> >> + ret = dw_hdmi_parse_dt(hdmi); >> + if (ret < 0) >> + return ERR_PTR(ret); >> + >> ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); >> if (ddc_node) { >> hdmi->ddc = of_get_i2c_adapter_by_node(ddc_node); >> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h >> index ea34ca146b82..8ebeb65d6371 100644 >> --- a/include/drm/bridge/dw_hdmi.h >> +++ b/include/drm/bridge/dw_hdmi.h >> @@ -126,6 +126,8 @@ struct dw_hdmi_phy_ops { >> struct dw_hdmi_plat_data { >> struct regmap *regm; >> >> + unsigned int output_port; >> + >> unsigned long input_bus_encoding; >> bool use_drm_infoframe; >> bool ycbcr_420_allowed; >> > > I must check on meson, since I'm not sure for now if the connector probes. > > Anyway, this looks fine. > > Reviewed-by: Neil Armstrong <narmstrong@baylibre.com> > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-05-26 14:28 UTC|newest] Thread overview: 148+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-26 1:14 [PATCH 00/27] Converter R-Car DU to the DRM bridge connector helper Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 1:14 ` [PATCH 01/27] drm: bridge: adv7511: Split EDID read to a separate function Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 1:14 ` [PATCH 02/27] drm: bridge: adv7511: Split connector creation " Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 1:14 ` [PATCH 03/27] drm: bridge: adv7511: Implement bridge connector operations Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-06-21 8:25 ` Sam Ravnborg 2020-06-21 8:25 ` Sam Ravnborg 2020-05-26 1:14 ` [PATCH 04/27] drm: bridge: adv7511: Make connector creation optional Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 1:14 ` [PATCH 05/27] drm: bridge: Return NULL on error from drm_bridge_get_edid() Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-06-21 8:26 ` Sam Ravnborg 2020-06-21 8:26 ` Sam Ravnborg 2020-05-26 1:14 ` [PATCH 06/27] drm: bridge: simple-bridge: Delegate operations to next bridge Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 1:14 ` [PATCH 07/27] drm: bridge: simple-bridge: Make connector creation optional Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 1:14 ` [PATCH 08/27] drm: rcar-du: lvds: Convert to DRM panel bridge helper Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 1:14 ` [PATCH 09/27] drm: edid: Constify connector argument to infoframe functions Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-06-21 8:27 ` Sam Ravnborg 2020-06-21 8:27 ` Sam Ravnborg 2020-05-26 1:14 ` [PATCH 10/27] drm: bridge: Pass drm_display_info to drm_bridge_funcs .mode_valid() Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 12:51 ` Neil Armstrong 2020-05-26 12:51 ` Neil Armstrong 2020-05-26 12:59 ` Boris Brezillon 2020-05-26 12:59 ` Boris Brezillon 2020-05-26 14:05 ` Guido Günther 2020-05-26 14:05 ` Guido Günther 2020-05-26 1:14 ` [PATCH 11/27] drm: bridge: dw-hdmi: Pass private data pointer to .mode_valid() Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 9:44 ` Neil Armstrong 2020-05-26 9:44 ` Neil Armstrong 2020-05-26 9:44 ` Neil Armstrong 2020-05-26 1:14 ` [PATCH 12/27] drm: bridge: dw-hdmi: Pass private data pointer to .configure_phy() Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 9:45 ` Neil Armstrong 2020-05-26 9:45 ` Neil Armstrong 2020-05-26 1:14 ` [PATCH 13/27] drm: bridge: dw-hdmi: Remove unused field from dw_hdmi_plat_data Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 9:45 ` Neil Armstrong 2020-05-26 9:45 ` Neil Armstrong 2020-05-26 1:14 ` [PATCH 14/27] drm: meson: dw-hdmi: Use dw_hdmi context to replace hack Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 12:32 ` Neil Armstrong 2020-05-26 12:32 ` Neil Armstrong 2020-05-26 12:32 ` Neil Armstrong 2020-05-26 1:14 ` [PATCH 15/27] drm: bridge: dw-hdmi: Pass drm_display_info to .mode_valid() Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 9:46 ` Neil Armstrong 2020-05-26 9:46 ` Neil Armstrong 2020-05-26 9:46 ` Neil Armstrong 2020-05-26 1:14 ` [PATCH 16/27] drm: bridge: dw-hdmi: Constify mode argument to dw_hdmi_phy_ops .init() Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 9:46 ` Neil Armstrong 2020-05-26 9:46 ` Neil Armstrong 2020-05-26 9:46 ` Neil Armstrong 2020-05-26 1:14 ` [PATCH 17/27] drm: bridge: dw-hdmi: Constify mode argument to internal functions Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 9:46 ` Neil Armstrong 2020-05-26 9:46 ` Neil Armstrong 2020-05-26 1:14 ` [PATCH 18/27] drm: bridge: dw-hdmi: Pass drm_display_info to dw_hdmi_support_scdc() Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 9:48 ` Neil Armstrong 2020-05-26 9:48 ` Neil Armstrong 2020-05-26 9:48 ` Neil Armstrong 2020-05-26 1:14 ` [PATCH 19/27] drm: bridge: dw-hdmi: Split connector creation to a separate function Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 9:49 ` Neil Armstrong 2020-05-26 9:49 ` Neil Armstrong 2020-05-26 1:14 ` [PATCH 20/27] drm: bridge: dw-hdmi: Store current connector in struct dw_hdmi Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 12:29 ` Neil Armstrong 2020-05-26 12:29 ` Neil Armstrong 2020-05-26 1:14 ` [PATCH 21/27] drm: bridge: dw-hdmi: Pass drm_connector to internal functions as needed Laurent Pinchart 2020-05-26 1:14 ` Laurent Pinchart 2020-05-26 12:29 ` Neil Armstrong 2020-05-26 12:29 ` Neil Armstrong 2020-05-26 1:15 ` [PATCH 22/27] drm: bridge: dw-hdmi: Make connector creation optional Laurent Pinchart 2020-05-26 1:15 ` Laurent Pinchart 2020-05-26 12:35 ` Neil Armstrong 2020-05-26 12:35 ` Neil Armstrong 2020-06-07 1:19 ` Laurent Pinchart 2020-06-07 1:19 ` Laurent Pinchart 2020-05-26 1:15 ` [PATCH 23/27] drm: bridge: dw-hdmi: Attach to next bridge if available Laurent Pinchart 2020-05-26 1:15 ` Laurent Pinchart 2020-05-26 12:50 ` Neil Armstrong 2020-05-26 12:50 ` Neil Armstrong 2020-05-26 14:23 ` Jonas Karlman [this message] 2020-05-26 14:23 ` Jonas Karlman 2020-06-07 1:24 ` Laurent Pinchart 2020-06-07 1:24 ` Laurent Pinchart 2020-06-07 1:22 ` Laurent Pinchart 2020-06-07 1:22 ` Laurent Pinchart 2020-05-26 1:15 ` [PATCH 24/27] drm: rcar-du: dw-hdmi: Set output port number Laurent Pinchart 2020-05-26 1:15 ` Laurent Pinchart 2020-05-26 1:15 ` [PATCH 25/27] drm: rcar-du: Fix error handling in rcar_du_encoder_init() Laurent Pinchart 2020-05-26 1:15 ` Laurent Pinchart 2020-05-26 1:15 ` [PATCH 26/27] drm: rcar-du: Use drm_bridge_connector_init() helper Laurent Pinchart 2020-05-26 1:15 ` Laurent Pinchart 2020-05-26 1:15 ` [PATCH 27/27] drm: Add default modes for connectors in unknown state Laurent Pinchart 2020-05-26 1:15 ` Laurent Pinchart 2020-06-21 8:40 ` Sam Ravnborg 2020-06-21 8:40 ` Sam Ravnborg 2020-06-24 1:12 ` Laurent Pinchart 2020-06-24 1:12 ` Laurent Pinchart 2020-06-24 7:23 ` Daniel Vetter 2020-06-24 7:23 ` Daniel Vetter 2020-06-24 15:24 ` Alex Deucher 2020-06-24 15:24 ` Alex Deucher 2020-06-24 19:31 ` Daniel Vetter 2020-06-24 19:31 ` Daniel Vetter 2020-06-24 19:40 ` Alex Deucher 2020-06-24 19:40 ` Alex Deucher 2020-06-25 7:56 ` Daniel Vetter 2020-06-25 7:56 ` Daniel Vetter 2020-06-25 7:57 ` Daniel Vetter 2020-06-25 7:57 ` Daniel Vetter 2020-06-25 10:31 ` Pekka Paalanen 2020-06-25 10:31 ` Pekka Paalanen 2020-06-25 10:44 ` Daniel Vetter 2020-06-25 10:44 ` Daniel Vetter 2020-06-26 8:59 ` Pekka Paalanen 2020-06-26 8:59 ` Pekka Paalanen 2020-06-26 9:25 ` Daniel Stone 2020-06-26 9:25 ` Daniel Stone 2020-06-26 13:35 ` Daniel Vetter 2020-06-26 13:35 ` Daniel Vetter 2020-06-24 22:47 ` Laurent Pinchart 2020-06-24 22:47 ` Laurent Pinchart 2020-06-23 18:55 ` [PATCH 00/27] Converter R-Car DU to the DRM bridge connector helper Sam Ravnborg 2020-06-23 18:55 ` Sam Ravnborg 2020-06-25 8:48 ` Liu Ying 2020-06-25 8:48 ` Liu Ying 2020-06-27 19:55 ` Sam Ravnborg 2020-06-27 19:55 ` Sam Ravnborg 2020-06-28 8:28 ` Laurent Pinchart 2020-06-28 8:28 ` Laurent Pinchart
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=b803dc36-f3f1-f90c-ac53-302d24397c2b@kwiboo.se \ --to=jonas@kwiboo.se \ --cc=a.hajda@samsung.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=jernej.skrabec@siol.net \ --cc=kieran.bingham@ideasonboard.com \ --cc=laurent.pinchart+renesas@ideasonboard.com \ --cc=linux-renesas-soc@vger.kernel.org \ --cc=narmstrong@baylibre.com \ --cc=sam@ravnborg.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.