Message-ID: <CACRpkdZ82tQw5wLXcJ4SwYr4_T+tmc93a-E_AED2dd7MzQ0h=Q@mail.gmail.com>
Date:   Wed, 9 Jun 2021 13:08:41 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Douglas Anderson <dianders@...omium.org>
Cc:     Andrzej Hajda <a.hajda@...sung.com>,
        Neil Armstrong <narmstrong@...libre.com>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Sam Ravnborg <sam@...nborg.org>,
        Rob Clark <robdclark@...omium.org>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Stanislav Lisovskiy <stanislav.lisovskiy@...el.com>,
        Steev Klimaszewski <steev@...i.org>,
        MSM <linux-arm-msm@...r.kernel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Thierry Reding <treding@...dia.com>,
        Lyude Paul <lyude@...hat.com>,
        "open list:DRM PANEL DRIVERS" <dri-devel@...ts.freedesktop.org>,
        Stephen Boyd <swboyd@...omium.org>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Robert Foss <robert.foss@...aro.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v9 07/11] drm/bridge: ti-sn65dsi86: Promote the AUX
 channel to its own sub-dev

On Mon, Jun 7, 2021 at 7:06 PM Douglas Anderson <dianders@...omium.org> wrote:

> On its own, this change looks a little strange and doesn't do too much
> useful. To understand why we're doing this we need to look forward to
> future patches where we're going to probe our panel using the new DP
> AUX bus. See the patch ("drm/bridge: ti-sn65dsi86: Add support for the
> DP AUX bus").
>
> Let's think about the set of steps we'll want to happen when we have
> the DP AUX bus:
>
> 1. We'll create the DP AUX bus.
> 2. We'll populate the devices on the DP AUX bus (AKA our panel).
> 3. For setting up the bridge-related functions of ti-sn65dsi86 we'll
>    need to get a reference to the panel.
>
> If we do #1 - #3 in a single probe call things _mostly_ will work, but
> it won't be massively robust. Let's explore.
>
> First let's think of the easy case of no -EPROBE_DEFER. In that case
> in step #2 when we populate the devices on the DP AUX bus it will
> actually try probing the panel right away. Since the panel probe
> doesn't defer then in step #3 we'll get a reference to the panel and
> we're golden.
>
> Second, let's think of the case when the panel returns
> -EPROBE_DEFER. In that case step #2 won't synchronously create the
> panel (it'll just add the device to the defer list to do it
> later). Step #3 will fail to get the panel and the bridge sub-device
> will return -EPROBE_DEFER. We'll depopulate the DP AUX bus. Later
> we'll try the whole sequence again. Presumably the panel will
> eventually stop returning -EPROBE_DEFER and we'll go back to the first
> case where things were golden. So this case is OK too even if it's a
> bit ugly that we have to keep creating / deleting the AUX bus over and
> over.
>
> So where is the problem? As I said, it's mostly about robustness. I
> don't believe that step #2 (creating the sub-devices) is really
> guaranteed to be synchronous. This is evidenced by the fact that it's
> allowed to "succeed" by just sticking the device on the deferred
> list. If anything about the process changes in Linux as a whole and
> step #2 just kicks off the probe of the DP AUX endpoints (our panel)
> in the background then we'd be in trouble because we might never get
> the panel in step #3.
>
> Adding an extra sub-device means we just don't need to worry about
> it. We'll create the sub-device for the DP AUX bus and it won't go
> away until the whole ti-sn65dsi86 driver goes away. If the bridge
> sub-device defers (maybe because it can't find the panel) that won't
> depopulate the DP AUX bus and so we don't need to worry about it.
>
> NOTE: there's a little bit of a trick here. Though the AUX channel can
> run without the MIPI-to-eDP bits of the code, the MIPI-to-eDP bits
> can't run without the AUX channel. We could come up a complicated
> signaling scheme (have the MIPI-to-eDP bits return EPROBE_DEFER for a
> while or wait on some sort of completion), but it seems simple enough
> to just not even bother creating the bridge device until the AUX
> channel probes. That's what we'll do.
>
> Signed-off-by: Douglas Anderson <dianders@...omium.org>
> Reviewed-by: Lyude Paul <lyude@...hat.com>

Reviewed-by: Linus Walleij <linus.walleij@...aro.org>

Yours,
Linus Walleij

Powered by blists - more mailing lists