Re: [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over the DSI link power state

From: Michael Walle <michael@walle.cc>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Michael Walle <mwalle@kernel.org>,
	tony@atomide.com, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, Laurent.pinchart@ideasonboard.com,
	andrzej.hajda@intel.com, marijn.suijten@somainline.org,
	marex@denx.de, rfoss@kernel.org, dave.stevenson@raspberrypi.com,
	jernej.skrabec@gmail.com, alexander.stein@ew.tq-group.com,
	quic_jesszhan@quicinc.com, jonas@kwiboo.se,
	linux-arm-msm@vger.kernel.org, quic_abhinavk@quicinc.com,
	mripard@kernel.org, sean@poorly.run, neil.armstrong@linaro.org,
	dianders@chromium.org, konrad.dybcio@linaro.org,
	tzimmermann@suse.de, freedreno@lists.freedesktop.org
Subject: Re: [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over the DSI link power state
Date: Tue, 28 Nov 2023 23:20:06 +0100	[thread overview]
Message-ID: <5eeade839ad3f71e8976965ce6cf3ed2@walle.cc> (raw)
In-Reply-To: <CAA8EJpoYkH-0onMmNRk1iO5YeLN+5hpZMsfvtNo-7p6y2mjZwg@mail.gmail.com>

[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:

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.

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
>> >
>> >
>> >
>> 