[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b8dfa3de-9f84-27be-fce3-e63d68f99ccc@samsung.com>
Date: Tue, 24 Aug 2021 15:14:12 +0200
From: Andrzej Hajda <a.hajda@...sung.com>
To: Maxime Ripard <maxime@...no.tech>, Jonas Karlman <jonas@...boo.se>,
Sam Ravnborg <sam@...nborg.org>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Thierry Reding <thierry.reding@...il.com>,
Daniel Vetter <daniel.vetter@...el.com>,
David Airlie <airlied@...ux.ie>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Thomas Zimmermann <tzimmermann@...e.de>,
Neil Armstrong <narmstrong@...libre.com>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Robert Foss <robert.foss@...aro.org>
Cc: linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v3 6/8] drm/bridge: ps8640: Register and attach our DSI
device at probe
W dniu 23.08.2021 o 10:47, Maxime Ripard pisze:
> In order to avoid any probe ordering issue, the best practice is to move
> the secondary MIPI-DSI device registration and attachment to the
> MIPI-DSI host at probe time. Let's do this.
>
> Signed-off-by: Maxime Ripard <maxime@...no.tech>
> ---
> drivers/gpu/drm/bridge/parade-ps8640.c | 93 ++++++++++++++------------
> 1 file changed, 51 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> index 794c9516b05d..37f7d166a3c6 100644
> --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> @@ -213,52 +213,10 @@ static int ps8640_bridge_attach(struct drm_bridge *bridge,
> enum drm_bridge_attach_flags flags)
> {
> struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> - struct device *dev = &ps_bridge->page[0]->dev;
> - struct device_node *in_ep, *dsi_node;
> - struct mipi_dsi_device *dsi;
> - struct mipi_dsi_host *host;
> - int ret;
> - const struct mipi_dsi_device_info info = { .type = "ps8640",
> - .channel = 0,
> - .node = NULL,
> - };
>
> if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR))
> return -EINVAL;
>
> - /* port@0 is ps8640 dsi input port */
> - in_ep = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1);
> - if (!in_ep)
> - return -ENODEV;
> -
> - dsi_node = of_graph_get_remote_port_parent(in_ep);
> - of_node_put(in_ep);
> - if (!dsi_node)
> - return -ENODEV;
> -
> - host = of_find_mipi_dsi_host_by_node(dsi_node);
> - of_node_put(dsi_node);
> - if (!host)
> - return -ENODEV;
> -
> - dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
> - if (IS_ERR(dsi)) {
> - dev_err(dev, "failed to create dsi device\n");
> - ret = PTR_ERR(dsi);
> - return ret;
> - }
> -
> - ps_bridge->dsi = dsi;
> -
> - dsi->host = host;
> - dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> - MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> - dsi->format = MIPI_DSI_FMT_RGB888;
> - dsi->lanes = DP_NUM_LANES;
> - ret = devm_mipi_dsi_attach(dev, dsi);
> - if (ret)
> - return ret;
> -
> /* Attach the panel-bridge to the dsi bridge */
> return drm_bridge_attach(bridge->encoder, ps_bridge->panel_bridge,
> &ps_bridge->bridge, flags);
> @@ -305,6 +263,53 @@ static const struct drm_bridge_funcs ps8640_bridge_funcs = {
> .pre_enable = ps8640_pre_enable,
> };
>
> +static int ps8640_bridge_host_attach(struct device *dev, struct ps8640 *ps_bridge)
> +{
> + struct device_node *in_ep, *dsi_node;
> + struct mipi_dsi_device *dsi;
> + struct mipi_dsi_host *host;
> + int ret;
> + const struct mipi_dsi_device_info info = { .type = "ps8640",
> + .channel = 0,
> + .node = NULL,
> + };
> +
> + /* port@0 is ps8640 dsi input port */
> + in_ep = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1);
> + if (!in_ep)
> + return -ENODEV;
> +
> + dsi_node = of_graph_get_remote_port_parent(in_ep);
> + of_node_put(in_ep);
> + if (!dsi_node)
> + return -ENODEV;
> +
> + host = of_find_mipi_dsi_host_by_node(dsi_node);
> + of_node_put(dsi_node);
> + if (!host)
> + return -EPROBE_DEFER;
> +
> + dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
> + if (IS_ERR(dsi)) {
> + dev_err(dev, "failed to create dsi device\n");
> + return PTR_ERR(dsi);
> + }
> +
> + ps_bridge->dsi = dsi;
> +
> + dsi->host = host;
> + dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> + MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> + dsi->format = MIPI_DSI_FMT_RGB888;
> + dsi->lanes = DP_NUM_LANES;
> +
> + ret = devm_mipi_dsi_attach(dev, dsi);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> static int ps8640_probe(struct i2c_client *client)
> {
> struct device *dev = &client->dev;
> @@ -371,6 +376,10 @@ static int ps8640_probe(struct i2c_client *client)
>
> drm_bridge_add(&ps_bridge->bridge);
>
> + ret = ps8640_bridge_host_attach(dev, ps_bridge);
> + if (ret)
> + return ret;
I do not see drm_bridge_remove on error path, the same for sn65dsi83.
Regards
Andrzej
> +
> return 0;
> }
>
Powered by blists - more mailing lists