linux-kernel - Re: [PATCH] drm/bridge: nwl-dsi: Move bridge add/remove to dsi callbacks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMty3ZAsn4K0WFRC_FNN2Mina0gSu4Nc1y1zVsoZ8GuSqcQFsA@mail.gmail.com>
Date:   Wed, 13 Oct 2021 17:54:36 +0530
From:   Jagan Teki <jagan@...rulasolutions.com>
To:     Guido Günther <agx@...xcpu.org>
Cc:     Andrzej Hajda <andrzej.hajda@...il.com>,
        Neil Armstrong <narmstrong@...libre.com>,
        Robert Foss <robert.foss@...aro.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/bridge: nwl-dsi: Move bridge add/remove to dsi callbacks

On Wed, Oct 13, 2021 at 5:44 PM Guido Günther <agx@...xcpu.org> wrote:
>
> Move the panel and bridge_{add,remove} from the bridge callbacks to the
> DSI callbacks to make sure we don't indicate readiness to participate in
> the display pipeline before the panel is attached.
>
> This was prompted by commit fb8d617f8fd6 ("drm/bridge: Centralize error
> message when bridge attach fails") which triggered
>
>   [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@...us@...00000/mipi-dsi@...0 0000 to encoder None-34: -517
>
> during boot.
>
> Signed-off-by: Guido Günther <agx@...xcpu.org>
> ---
> This was prompted by the discussion at
> https://lore.kernel.org/dri-devel/00493cc61d1443dab1c131c46c5890f95f6f9b25.1634068657.git.agx@sigxcpu.org/
>
>  drivers/gpu/drm/bridge/nwl-dsi.c | 64 ++++++++++++++++++--------------
>  1 file changed, 37 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c
> index a7389a0facfb..77aa6f13afef 100644
> --- a/drivers/gpu/drm/bridge/nwl-dsi.c
> +++ b/drivers/gpu/drm/bridge/nwl-dsi.c
> @@ -355,6 +355,9 @@ static int nwl_dsi_host_attach(struct mipi_dsi_host *dsi_host,
>  {
>         struct nwl_dsi *dsi = container_of(dsi_host, struct nwl_dsi, dsi_host);
>         struct device *dev = dsi->dev;
> +       struct drm_bridge *panel_bridge;
> +       struct drm_panel *panel;
> +       int ret;
>
>         DRM_DEV_INFO(dev, "lanes=%u, format=0x%x flags=0x%lx\n", device->lanes,
>                      device->format, device->mode_flags);
> @@ -362,10 +365,43 @@ static int nwl_dsi_host_attach(struct mipi_dsi_host *dsi_host,
>         if (device->lanes < 1 || device->lanes > 4)
>                 return -EINVAL;
>
> +       ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
> +                                         &panel_bridge);
> +       if (ret)
> +               return ret;
> +
> +       if (panel) {
> +               panel_bridge = drm_panel_bridge_add(panel);
> +               if (IS_ERR(panel_bridge))
> +                       return PTR_ERR(panel_bridge);
> +       }
> +       if (!panel_bridge)
> +               return -EPROBE_DEFER;
> +
> +       dsi->panel_bridge = panel_bridge;
>         dsi->lanes = device->lanes;
>         dsi->format = device->format;
>         dsi->dsi_mode_flags = device->mode_flags;
>
> +       /*
> +        * The DSI output has been properly configured, we can now safely
> +        * register the input to the bridge framework so that it can take place
> +        * in a display pipeline.
> +        */
> +       drm_bridge_add(&dsi->bridge);
> +
> +       return 0;
> +}
> +
> +static int nwl_dsi_host_detach(struct mipi_dsi_host *dsi_host,
> +                              struct mipi_dsi_device *dev)
> +{
> +       struct nwl_dsi *dsi = container_of(dsi_host, struct nwl_dsi, dsi_host);
> +
> +       drm_bridge_remove(&dsi->bridge);
> +       if (dsi->panel_bridge)
> +               drm_panel_bridge_remove(dsi->panel_bridge);
> +

If I'm correct this logic will failed to find the direct and I2C based
bridges. As these peripheral bridges are trying to find the bridge
device from bridge_attach unlike DSI panels are trying to find the
panel via host attach directly from probe.

Similar issue we have encounters with dw-mipi-dsi bridge.
c206c7faeb3263a7cc7b4de443a3877cd7a5e74b

Jagan.