From: Boris Brezillon <boris.brezillon@collabora.com>
To: Andrzej Hajda <a.hajda@samsung.com>,
Neil Armstrong <narmstrong@baylibre.com>,
Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
Jonas Karlman <jonas@kwiboo.se>,
Jernej Skrabec <jernej.skrabec@siol.net>,
Inki Dae <inki.dae@samsung.com>,
Joonyoung Shim <jy0922.shim@samsung.com>,
Seung-Woo Kim <sw0312.kim@samsung.com>,
Kyungmin Park <kyungmin.park@samsung.com>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Eric Anholt <eric@anholt.net>
Cc: Boris Brezillon <boris.brezillon@collabora.com>,
dri-devel@lists.freedesktop.org
Subject: [PATCH 2/3] drm/vc4: dsi: Fix bridge chain handling
Date: Fri, 27 Dec 2019 15:41:23 +0100 [thread overview]
Message-ID: <20191227144124.210294-2-boris.brezillon@collabora.com> (raw)
In-Reply-To: <20191227144124.210294-1-boris.brezillon@collabora.com>
Commit 05193dc38197 ("drm/bridge: Make the bridge chain a double-linked
list") patched the bridge chain logic to use a double-linked list instead
of a single-linked list. This change induced changes to the VC4 driver
which was manually resetting the encoder->bridge element to NULL to
control the enable/disable sequence of the bridge chain. During this
conversion, 2 bugs were introduced:
1/ list_splice() was used to move chain elements to our own internal
chain, but list_splice() does not reset the source list to an empty
state, leading to unexpected bridge hook calls when
drm_bridge_chain_xxx() helpers were called by the core. Replacing
those list_splice() calls by list_splice_init() ones fixes this
problem.
2/ drm_bridge_chain_xxx() helpers operate on the
bridge->encoder->bridge_chain list, which is now empty. When the
helper uses list_for_each_entry_reverse() we end up with no operation
done which is not what we want. But that's even worse when the helper
uses list_for_each_entry_from(), because in that case we end up in
an infinite loop searching for the list head element which is no
longer encoder->bridge_chain but vc4_dsi->bridge_chain. To address
that problem we stop using the bridge chain helpers and call the
hooks directly.
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Eric Anholt <eric@anholt.net>
Fixes: 05193dc38197 ("drm/bridge: Make the bridge chain a double-linked list")
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
drivers/gpu/drm/vc4/vc4_dsi.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index 6c5b80ad6154..fd8a2eb60505 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -753,10 +753,19 @@ static void vc4_dsi_encoder_disable(struct drm_encoder *encoder)
struct vc4_dsi_encoder *vc4_encoder = to_vc4_dsi_encoder(encoder);
struct vc4_dsi *dsi = vc4_encoder->dsi;
struct device *dev = &dsi->pdev->dev;
+ struct drm_bridge *iter;
+
+ list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) {
+ if (iter->funcs->disable)
+ iter->funcs->disable(iter);
+ }
- drm_bridge_chain_disable(dsi->bridge);
vc4_dsi_ulps(dsi, true);
- drm_bridge_chain_post_disable(dsi->bridge);
+
+ list_for_each_entry_from(iter, &dsi->bridge_chain, chain_node) {
+ if (iter->funcs->post_disable)
+ iter->funcs->post_disable(iter);
+ }
clk_disable_unprepare(dsi->pll_phy_clock);
clk_disable_unprepare(dsi->escape_clock);
@@ -824,6 +833,7 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
struct vc4_dsi *dsi = vc4_encoder->dsi;
struct device *dev = &dsi->pdev->dev;
bool debug_dump_regs = false;
+ struct drm_bridge *iter;
unsigned long hs_clock;
u32 ui_ns;
/* Minimum LP state duration in escape clock cycles. */
@@ -1056,7 +1066,10 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
vc4_dsi_ulps(dsi, false);
- drm_bridge_chain_pre_enable(dsi->bridge);
+ list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) {
+ if (iter->funcs->pre_enable)
+ iter->funcs->pre_enable(iter);
+ }
if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
DSI_PORT_WRITE(DISP0_CTRL,
@@ -1073,7 +1086,10 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
DSI_DISP0_ENABLE);
}
- drm_bridge_chain_enable(dsi->bridge);
+ list_for_each_entry(iter, &dsi->bridge_chain, chain_node) {
+ if (iter->funcs->enable)
+ iter->funcs->enable(iter);
+ }
if (debug_dump_regs) {
struct drm_printer p = drm_info_printer(&dsi->pdev->dev);
@@ -1613,7 +1629,7 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data)
* from our driver, since we need to sequence them within the
* encoder's enable/disable paths.
*/
- list_splice(&dsi->encoder->bridge_chain, &dsi->bridge_chain);
+ list_splice_init(&dsi->encoder->bridge_chain, &dsi->bridge_chain);
if (dsi->port == 0)
vc4_debugfs_add_regset32(drm, "dsi0_regs", &dsi->regset);
@@ -1639,7 +1655,7 @@ static void vc4_dsi_unbind(struct device *dev, struct device *master,
* Restore the bridge_chain so the bridge detach procedure can happen
* normally.
*/
- list_splice(&dsi->bridge_chain, &dsi->encoder->bridge_chain);
+ list_splice_init(&dsi->bridge_chain, &dsi->encoder->bridge_chain);
vc4_dsi_encoder_destroy(dsi->encoder);
if (dsi->port == 1)
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2019-12-27 14:41 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-27 14:41 [PATCH 1/3] drm/bridge: Fix drm_bridge_chain_pre_enable() Boris Brezillon
2019-12-27 14:41 ` Boris Brezillon [this message]
2020-01-07 15:17 ` [PATCH 2/3] drm/vc4: dsi: Fix bridge chain handling Andrzej Hajda
2019-12-27 14:41 ` [PATCH 3/3] drm/exynos: " Boris Brezillon
2020-01-06 7:41 ` Boris Brezillon
2020-01-07 9:11 ` Marek Szyprowski
2020-01-07 13:34 ` Boris Brezillon
2020-01-07 14:30 ` Andrzej Hajda
2020-01-06 10:29 ` [PATCH 1/3] drm/bridge: Fix drm_bridge_chain_pre_enable() Laurent Pinchart
2020-01-07 15:27 ` Andrzej Hajda
2020-01-07 15:33 ` Boris Brezillon
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=20191227144124.210294-2-boris.brezillon@collabora.com \
--to=boris.brezillon@collabora.com \
--cc=Laurent.pinchart@ideasonboard.com \
--cc=a.hajda@samsung.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=eric@anholt.net \
--cc=inki.dae@samsung.com \
--cc=jernej.skrabec@siol.net \
--cc=jonas@kwiboo.se \
--cc=jy0922.shim@samsung.com \
--cc=kyungmin.park@samsung.com \
--cc=m.szyprowski@samsung.com \
--cc=narmstrong@baylibre.com \
--cc=sw0312.kim@samsung.com \
/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.