Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drm/vc4: 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 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>
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
Acked-by: Eric Anholt <eric@anholt.net>
Link: https://patchwork.freedesktop.org/patch/msgid/20191227144124.210294-2-boris.brezillon@collabora.com

+22 -6
+22 -6
drivers/gpu/drm/vc4/vc4_dsi.c
··· 753 753 struct vc4_dsi_encoder *vc4_encoder = to_vc4_dsi_encoder(encoder); 754 754 struct vc4_dsi *dsi = vc4_encoder->dsi; 755 755 struct device *dev = &dsi->pdev->dev; 756 + struct drm_bridge *iter; 756 757 757 - drm_bridge_chain_disable(dsi->bridge); 758 + list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) { 759 + if (iter->funcs->disable) 760 + iter->funcs->disable(iter); 761 + } 762 + 758 763 vc4_dsi_ulps(dsi, true); 759 - drm_bridge_chain_post_disable(dsi->bridge); 764 + 765 + list_for_each_entry_from(iter, &dsi->bridge_chain, chain_node) { 766 + if (iter->funcs->post_disable) 767 + iter->funcs->post_disable(iter); 768 + } 760 769 761 770 clk_disable_unprepare(dsi->pll_phy_clock); 762 771 clk_disable_unprepare(dsi->escape_clock); ··· 833 824 struct vc4_dsi *dsi = vc4_encoder->dsi; 834 825 struct device *dev = &dsi->pdev->dev; 835 826 bool debug_dump_regs = false; 827 + struct drm_bridge *iter; 836 828 unsigned long hs_clock; 837 829 u32 ui_ns; 838 830 /* Minimum LP state duration in escape clock cycles. */ ··· 1066 1056 1067 1057 vc4_dsi_ulps(dsi, false); 1068 1058 1069 - drm_bridge_chain_pre_enable(dsi->bridge); 1059 + list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) { 1060 + if (iter->funcs->pre_enable) 1061 + iter->funcs->pre_enable(iter); 1062 + } 1070 1063 1071 1064 if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) { 1072 1065 DSI_PORT_WRITE(DISP0_CTRL, ··· 1086 1073 DSI_DISP0_ENABLE); 1087 1074 } 1088 1075 1089 - drm_bridge_chain_enable(dsi->bridge); 1076 + list_for_each_entry(iter, &dsi->bridge_chain, chain_node) { 1077 + if (iter->funcs->enable) 1078 + iter->funcs->enable(iter); 1079 + } 1090 1080 1091 1081 if (debug_dump_regs) { 1092 1082 struct drm_printer p = drm_info_printer(&dsi->pdev->dev); ··· 1629 1613 * from our driver, since we need to sequence them within the 1630 1614 * encoder's enable/disable paths. 1631 1615 */ 1632 - list_splice(&dsi->encoder->bridge_chain, &dsi->bridge_chain); 1616 + list_splice_init(&dsi->encoder->bridge_chain, &dsi->bridge_chain); 1633 1617 1634 1618 if (dsi->port == 0) 1635 1619 vc4_debugfs_add_regset32(drm, "dsi0_regs", &dsi->regset); ··· 1655 1639 * Restore the bridge_chain so the bridge detach procedure can happen 1656 1640 * normally. 1657 1641 */ 1658 - list_splice(&dsi->bridge_chain, &dsi->encoder->bridge_chain); 1642 + list_splice_init(&dsi->bridge_chain, &dsi->encoder->bridge_chain); 1659 1643 vc4_dsi_encoder_destroy(dsi->encoder); 1660 1644 1661 1645 if (dsi->port == 1)