Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
MIME-Version: 1.0
References: <CAA8EJpozZkEswnioKjRCqBg4fcjVHFwGivoFNTNHVwyocKprQw@mail.gmail.com>
 <20231127160658.2164612-1-mwalle@kernel.org> <CAA8EJpphwXoKnzDkY3cBqzsDZwdw+nze-Ev2toPBJm-2VJvY_g@mail.gmail.com>
 <dce76a6e1321a6374ad39125bead56b3@kernel.org> <CAA8EJpr9PDgSrTpP2-joQ09fxmJKZB1B+ESbDbMjkLNiqZ1m3A@mail.gmail.com>
 <14D9F495-425D-47FA-AD0D-F7299285936F@walle.cc> <CAA8EJpoYkH-0onMmNRk1iO5YeLN+5hpZMsfvtNo-7p6y2mjZwg@mail.gmail.com>
 <5eeade839ad3f71e8976965ce6cf3ed2@walle.cc>
In-Reply-To: <5eeade839ad3f71e8976965ce6cf3ed2@walle.cc>
From:   Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Date:   Wed, 29 Nov 2023 00:21:51 +0200
Message-ID: <CAA8EJppAhfm4FK4z_B=h9+o7LScjCyUD0XvdzQCe6tjrRx5KbA@mail.gmail.com>
Subject: Re: [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over
 the DSI link power state
To:     Michael Walle <michael@walle.cc>
Cc:     Michael Walle <mwalle@kernel.org>,
        Laurent.pinchart@ideasonboard.com, andrzej.hajda@intel.com,
        dave.stevenson@raspberrypi.com, dianders@chromium.org,
        dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org,
        jernej.skrabec@gmail.com, jonas@kwiboo.se,
        konrad.dybcio@linaro.org, linux-arm-msm@vger.kernel.org,
        linux-kernel@vger.kernel.org, marex@denx.de,
        marijn.suijten@somainline.org, mripard@kernel.org,
        neil.armstrong@linaro.org, quic_abhinavk@quicinc.com,
        quic_jesszhan@quicinc.com, rfoss@kernel.org, sean@poorly.run,
        tzimmermann@suse.de, tony@atomide.com,
        alexander.stein@ew.tq-group.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 29 Nov 2023 at 00:20, Michael Walle <michael@walle.cc> wrote:
>
> [sorry I fat fingered my former reply and converted all CCs to BCCs..]
>
> >> >> >> > DSI device lifetime has three different stages:
> >> >> >> > 1. before the DSI link being powered up and clocking,
> >> >> >> > 2. when the DSI link is in LP state (for the purpose of this question,
> >> >> >> > this is the time between the DSI link being powered up and the video
> >> >> >> > stream start)
> >> >> >> > 3. when the DSI link is in HS state (while streaming the video).
> >> >> >>
> >> >> >> It's not clear to me what (2) is. What is the state of the clock and
> >> >> >> data lanes?
> >> >> >
> >> >> > Clk an Data0 should be in the LP mode, ready for LP Data Transfer.
> >> >>
> >> >> Then this is somehow missing
> >> >> https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
> >> >>
> >> >>    A DSI host should keep the PHY powered down until the pre_enable
> >> >> operation
> >> >>    is called. All lanes are in an undefined idle state up to this point,
> >> >> and
> >> >>    it must not be assumed that it is LP-11. pre_enable should initialise
> >> >> the
> >> >>    PHY, set the data lanes to LP-11, and the clock lane to either LP-11
> >> >> or HS
> >> >>    depending on the mode_flag MIPI_DSI_CLOCK_NON_CONTINUOUS.
> >> >>
> >> >> So I don't think these three states are sufficient, see below, that
> >> >> there
> >> >> should be at least four.
> >> >
> >> >Which one is #4?
> >>
> >> enabled clock lane (HS mode), data lanes in LP-11
> >
> > What is the purpose of such a mode?
>
> To repeat my first mail:

Excuse me please, I either missed it, or forgot it.

>
> I'm facing similar issues with the tc358775 bridge. This bridge needs
> to release its reset while both clock and data lanes are in LP-11
> mode.
> But then it needs to be configured (via I2C) while the clock lane is
> in enabled (HS mode), but the data lanes are still in LP-11 mode.

This is quite an interesting requirement. For example, I'm not 100%
sure whether we can get that done on our (msm) hosts. I need to double
check that.
What frequency is expected on the CLK lane? Can it be an arbitrary
frequency or it should be the same freq as the one used later for the
video transfer?

>
> Therefore, for the correct init sequence is:
> (1) dsi host enables lanes, that is clock and data are in lp-11
> (2) dsi bridge driver releases reset of the bridge
> (3) dsi host enables clock lane, leaves data lanes in lp-11
> (4) dsi bridge driver configures the bridge
> (5) dsi host enables the video stream
> (6) dsi bridge enables the output port of the bridge
>
> -michael
>
> >> >> > I don't think we support ULPS currently.
> >> >> >
> >> >> >
> >> >> >>
> >> >> >> I'm facing similar issues with the tc358775 bridge. This bridge needs
> >> >> >> to release its reset while both clock and data lanes are in LP-11
> >> >> >> mode.
> >> >> >> But then it needs to be configured (via I2C) while the clock lane is
> >> >> >> in enabled (HS mode), but the data lanes are still in LP-11 mode.
> >> >> >>
> >> >> >> To me it looks like there is a fouth case then:
> >> >> >> 1. unpowered
> >> >> >> 2. DSI clock and data are in LP-11
> >> >> >> 3. DSI clock is in HS and data are in LP-11
> >> >> >> 4. DSI clock is in HS and data is in HS
> >> >> >>
> >> >> >> (And of course the bridge needs continuous clock mode).
> >> >> >>
> >> >> >> > Different DSI bridges have different requirements with respect to the
> >> >> >> > code being executed at stages 1 and 2. For example several DSI-to-eDP
> >> >> >> > bridges (ps8640, tc358767 require for the link to be quiet during
> >> >> >> > reset time.
> >> >> >> > The DSI-controlled bridges and DSI panels need to send some commands
> >> >> >> > in stage 2, before starting up video
> >> >> >> >
> >> >> >> > In the DRM subsystem stage 3 naturally maps to the
> >> >> >> > drm_bridge_funcs::enable, stage 1 also naturally maps to the
> >> >> >> > drm_bridge_funcs::pre_enable. Stage 2 doesn't have its own place in
> >> >> >> > the DRM call chain.
> >> >> >> > Earlier we attempted to solve that using the pre_enable_prev_first,
> >> >> >> > which remapped pre-enable callback execution order. However it has led
> >> >> >> > us to the two issues. First, at the DSI host driver we do not know
> >> >> >> > whether the panel / bridge were updated to use pre_enable_prev_first
> >> >> >> > or not. Second, if the bridge has to perform steps during both stages
> >> >> >> > 1 and 2, it can not do that.
> >> >> >> >
> >> >> >> > I'm trying to find a way to express the difference between stages 1
> >> >> >> > and 2 in the generic code, so that we do not to worry about particular
> >> >> >> > DSI host and DSI bridge / panel peculiarities when implementing the
> >> >> >> > DSI host and/or DSI panel driver.
> >> >> >>
> >> >> >> For now, I have a rather hacky ".dsi_lp11_notify" callback in
> >> >> >> drm_bridge_funcs which is supposed to be called by the DSI host while
> >> >> >> the
> >> >> >> clock and data lanes are in LP-11 mode. But that is rather an RFC and
> >> >> >> me
> >> >> >> needing something to get the driver for this bridge working. Because
> >> >> >> it's
> >> >> >> badly broken. FWIW, you can find my work-in-progress patches at
> >> >> >> https://github.com/mwalle/linux/tree/feature-tc358775-fixes
> >> >> >>
> >> >> >> -michael
> >> >> >>
> >> >> >
> >> >> >
> >> >> > --
> >> >> > With best wishes
> >> >> > Dmitry
> >> >
> >> >
> >> >
> >>


-- 
With best wishes
Dmitry