All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Stevenson <dave.stevenson@raspberrypi.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	dri-devel@lists.freedesktop.org
Cc: Marek Vasut <marex@denx.de>, Jonas Karlman <jonas@kwiboo.se>,
	Robert Foss <robert.foss@linaro.org>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Douglas Anderson <dianders@chromium.org>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	andrzej.hajda@gmail.com,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Andrzej Hajda <andrzej.hajda@intel.com>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Dave Stevenson <dave.stevenson@raspberrypi.com>,
	Jagan Teki <jagan@amarulasolutions.com>
Subject: [PATCH V2 0/3] DSI host and peripheral initialisation ordering
Date: Fri,  4 Mar 2022 15:17:55 +0000	[thread overview]
Message-ID: <cover.1646406653.git.dave.stevenson@raspberrypi.com> (raw)

Hi All

Changes from v1:
- New patch to refactor drm_bridge_chain_post_disable and drm_bridge_chain_pre_enable
  to reuse drm_atomic_bridge_chain_post_disable / drm_atomic_bridge_chain_pre_enable
  but with a NULL state.
- New patch that adds a pre_enable_upstream_first to drm_panel.
- changed from an OPS flag to a bool "pre_enable_upstream_first" in drm_bridge.
- Followed Andrzej's suggestion of using continue in the main loop to avoid
  needing 2 additional loops (one forward to find the last bridge wanting
  upstream first, and the second backwards again).
- Actioned Laurent's review comments on docs patch.

Original cover letter: 

Hopefully I've cc'ed all those that have bashed this problem around previously,
or are otherwise linked to DRM bridges.

There have been numerous discussions around how DSI support is currently broken
as it doesn't support initialising the PHY to LP-11 and potentially the clock
lane to HS prior to configuring the DSI peripheral. There is no op where the
interface is initialised but HS video isn't also being sent.
Currently you have:
- peripheral pre_enable (host not initialised yet)
- host pre_enable
- encoder enable
- host enable
- peripheral enable (video already running)

vc4 and exynos currently implement the DSI host as an encoder, and split the
bridge_chain. This fails if you want to switch to being a bridge and/or use
atomic calls as the state of all the elements split off are not added by
drm_atomic_add_encoder_bridges.

dw-mipi-dsi[1] and now msm[2] use the mode_set hook to initialise the PHY, so
the bridge/panel pre_enable can send commands. In their post_disable they then
call the downstream bridge/panel post_disable op manually so that shutdown
commands can be sent before shutting down the PHY. Nothing handles that fact,
so the framework then continues down the bridge chain and calls the post_disable
again, so we get unbalanced panel prepare/unprepare calls being reported [3].

There have been patches[4] proposing reversing the entire direction of
pre_enable and post_disable, but that risks driving voltage into devices that
have yet to be powered up.
There have been discussions about adding either a pre_pre_enable, or adding a
DSI host_op to initialise the host[5]. Both require significant reworking to all
existing drivers in moving initialisation phases.
We have patches that look like they may well be addressing race conditions in
starting up a DSI peripheral[6].

This patch takes a hybrid of the two: an optional reversing of the order for
specific links within the bridge chain within pre_enable and post_disable done
within the drm_bridge framework.
I'm more than happy to move where the flag exists in structures (currently as
DRM_BRIDGE_OP_UPSTREAM_FIRST in drm_bridge_ops, but it isn't an op), but does
this solve the problem posed? If not, then can you describe the actual scenario
it doesn't cover?
A DSI peripheral can set the flag to get the DSI host initialised first, and
therefore it has a stable LP-11 state before pre_enable. Likewise the peripheral
can still send shutdown commands prior to the DSI host being shut down in
post_disable. It also handles the case where there are multiple devices in the
chain that all want their upstream bridge enabled first, so should there be a
DSI mux between host and peripheral, then it can still get the host to the
correct state.

An example tree is at [7] which is drm-misc-next with these patches and then a
conversion of vc4_dsi to use the atomic bridge functions (will be upstreamed
once we're over this hurdle). It is working happily with the Toshiba TC358762 on
a Raspberry Pi 7" panel.
The same approach but on our vendor 5.15 tree[8] has also been tested
successfully on a TI SN65DSI83 and LVDS panel.

Whilst here, I've also documented the expected behaviour of DSI hosts and
peripherals to aid those who come along after.

Thanks
  Dave

[1] https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L940
[2] https://lists.freedesktop.org/archives/dri-devel/2022-January/337769.html
[3] https://lists.freedesktop.org/archives/dri-devel/2021-December/333908.html
[4] https://lists.freedesktop.org/archives/dri-devel/2021-October/328476.html
[5] https://lists.freedesktop.org/archives/dri-devel/2021-October/325853.html
[6] https://lists.freedesktop.org/archives/dri-devel/2022-February/341852.html
[7] https://github.com/6by9/linux/tree/drm-misc-next-vc4_dsi
[8] https://github.com/6by9/linux/tree/rpi-5.15.y-sn65dsi83

Dave Stevenson (4):
  drm/bridge: Remove duplication from drm_bridge and drm_atomic_bridge
    chains
  drm/bridge: Introduce pre_enable_upstream_first to alter bridge init
    order
  drm/panel: Add prepare_upstream_first flag to drm_panel
  drm/bridge: Document the expected behaviour of DSI host controllers

 Documentation/gpu/drm-kms-helpers.rst |   7 ++
 drivers/gpu/drm/bridge/panel.c        |   3 +
 drivers/gpu/drm/drm_bridge.c          | 181 ++++++++++++++++++++++++----------
 include/drm/drm_bridge.h              |   8 ++
 include/drm/drm_panel.h               |  10 ++
 5 files changed, 159 insertions(+), 50 deletions(-)

-- 
2.7.4


             reply	other threads:[~2022-03-04 15:18 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-04 15:17 Dave Stevenson [this message]
2022-03-04 15:17 ` [PATCH V2 1/4] drm/bridge: Remove duplication from drm_bridge and drm_atomic_bridge chains Dave Stevenson
2022-06-08 11:00   ` Dmitry Baryshkov
2022-07-18 18:16     ` Sam Ravnborg
2022-03-04 15:17 ` [PATCH V2 2/4] drm/bridge: Introduce pre_enable_upstream_first to alter bridge init order Dave Stevenson
2022-09-14  9:46   ` Jagan Teki
2022-03-04 15:17 ` [PATCH V2 3/4] drm/panel: Add prepare_upstream_first flag to drm_panel Dave Stevenson
2022-06-08 11:02   ` Dmitry Baryshkov
2022-10-06 14:25   ` Jagan Teki
2022-10-07 12:55     ` Dave Stevenson
2022-10-17  2:44       ` Jagan Teki
2022-10-18 17:10         ` Dave Stevenson
2022-03-04 15:17 ` [PATCH V2 4/4] drm/bridge: Document the expected behaviour of DSI host controllers Dave Stevenson
2022-03-18 12:25 ` [PATCH V2 0/3] DSI host and peripheral initialisation ordering Dave Stevenson
2022-04-05 11:43   ` Dave Stevenson
2022-05-11 14:58     ` Marek Szyprowski
2022-05-11 15:47       ` Marek Vasut
2022-05-11 17:29         ` Marek Szyprowski
2022-05-13 14:01         ` Jagan Teki
2022-05-11 15:47       ` Dave Stevenson
2022-05-18 14:05         ` Marek Szyprowski
2022-05-18 22:53           ` Andrzej Hajda
2022-05-19 12:56             ` Dave Stevenson
2022-06-08 10:49               ` Dave Stevenson
2022-06-08 10:57                 ` Jagan Teki
2022-06-07 19:46           ` Jagan Teki
2022-05-13 13:57       ` Jagan Teki
2022-06-10  7:52       ` Lucas Stach
2022-07-06  7:09         ` Frieder Schrempf
2022-07-06  7:13           ` Jagan Teki
2022-07-06 10:27           ` Dave Stevenson
2022-07-06 10:43             ` Frieder Schrempf
2022-07-19 14:36         ` Dave Stevenson
2022-07-18 20:52 ` Sam Ravnborg
2022-07-19 13:45   ` Dave Stevenson
2022-11-13 13:06     ` Dmitry Baryshkov
2022-11-15 14:14       ` Dave Stevenson
2022-11-15 14:21         ` Dmitry Baryshkov
2022-11-15 14:38           ` Dave Stevenson
2022-11-17 13:24             ` Dmitry Baryshkov
2022-11-17 14:34               ` Maxime Ripard
2022-11-17 14:35                 ` Dmitry Baryshkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1646406653.git.dave.stevenson@raspberrypi.com \
    --to=dave.stevenson@raspberrypi.com \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=airlied@linux.ie \
    --cc=andrzej.hajda@gmail.com \
    --cc=andrzej.hajda@intel.com \
    --cc=daniel@ffwll.ch \
    --cc=dianders@chromium.org \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jagan@amarulasolutions.com \
    --cc=jernej.skrabec@gmail.com \
    --cc=jonas@kwiboo.se \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=marex@denx.de \
    --cc=mripard@kernel.org \
    --cc=narmstrong@baylibre.com \
    --cc=robert.foss@linaro.org \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.