From: Boris Brezillon Date: Fri, 27 Dec 2019 14:41:24 +0000 (+0100) Subject: drm/exynos: dsi: Fix bridge chain handling X-Git-Tag: v5.6-rc1~114^2~13^2~23 X-Git-Url: https://asedeno.scripts.mit.edu/gitweb/?a=commitdiff_plain;ds=sidebyside;h=f66ff55a9bd06855f9c6b8f29d8760f77bfe43c2;p=linux.git drm/exynos: dsi: Fix bridge chain handling 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 Exynos 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 the list_splice() call by list_splice_init() 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 exynos_dsi->bridge_chain. To address that problem we stop using the bridge chain helpers and call the hooks directly. Reported-by: Marek Szyprowski Fixes: 05193dc38197 ("drm/bridge: Make the bridge chain a double-linked list") Signed-off-by: Boris Brezillon Tested-by: Marek Szyprowski Reviewed-by: Andrzej Hajda Link: https://patchwork.freedesktop.org/patch/msgid/20191227144124.210294-3-boris.brezillon@collabora.com --- diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index 3955f84dc893..33628d85edad 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -1378,6 +1378,7 @@ static void exynos_dsi_unregister_te_irq(struct exynos_dsi *dsi) static void exynos_dsi_enable(struct drm_encoder *encoder) { struct exynos_dsi *dsi = encoder_to_dsi(encoder); + struct drm_bridge *iter; int ret; if (dsi->state & DSIM_STATE_ENABLED) @@ -1391,7 +1392,11 @@ static void exynos_dsi_enable(struct drm_encoder *encoder) if (ret < 0) goto err_put_sync; } else { - drm_bridge_chain_pre_enable(dsi->out_bridge); + list_for_each_entry_reverse(iter, &dsi->bridge_chain, + chain_node) { + if (iter->funcs->pre_enable) + iter->funcs->pre_enable(iter); + } } exynos_dsi_set_display_mode(dsi); @@ -1402,7 +1407,10 @@ static void exynos_dsi_enable(struct drm_encoder *encoder) if (ret < 0) goto err_display_disable; } else { - drm_bridge_chain_enable(dsi->out_bridge); + list_for_each_entry(iter, &dsi->bridge_chain, chain_node) { + if (iter->funcs->enable) + iter->funcs->enable(iter); + } } dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE; @@ -1420,6 +1428,7 @@ static void exynos_dsi_enable(struct drm_encoder *encoder) static void exynos_dsi_disable(struct drm_encoder *encoder) { struct exynos_dsi *dsi = encoder_to_dsi(encoder); + struct drm_bridge *iter; if (!(dsi->state & DSIM_STATE_ENABLED)) return; @@ -1427,10 +1436,20 @@ static void exynos_dsi_disable(struct drm_encoder *encoder) dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE; drm_panel_disable(dsi->panel); - drm_bridge_chain_disable(dsi->out_bridge); + + list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) { + if (iter->funcs->disable) + iter->funcs->disable(iter); + } + exynos_dsi_set_display_enable(dsi, false); drm_panel_unprepare(dsi->panel); - drm_bridge_chain_post_disable(dsi->out_bridge); + + list_for_each_entry(iter, &dsi->bridge_chain, chain_node) { + if (iter->funcs->post_disable) + iter->funcs->post_disable(iter); + } + dsi->state &= ~DSIM_STATE_ENABLED; pm_runtime_put_sync(dsi->dev); } @@ -1523,7 +1542,7 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, if (out_bridge) { drm_bridge_attach(encoder, out_bridge, NULL); dsi->out_bridge = out_bridge; - list_splice(&encoder->bridge_chain, &dsi->bridge_chain); + list_splice_init(&encoder->bridge_chain, &dsi->bridge_chain); } else { int ret = exynos_dsi_create_connector(encoder);