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

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 <m.szyprowski@samsung.com>
Fixes: 05193dc38197 ("drm/bridge: Make the bridge chain a double-linked list")
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191227144124.210294-3-boris.brezillon@collabora.com

+24 -5
+24 -5
drivers/gpu/drm/exynos/exynos_drm_dsi.c
··· 1378 1378 static void exynos_dsi_enable(struct drm_encoder *encoder) 1379 1379 { 1380 1380 struct exynos_dsi *dsi = encoder_to_dsi(encoder); 1381 + struct drm_bridge *iter; 1381 1382 int ret; 1382 1383 1383 1384 if (dsi->state & DSIM_STATE_ENABLED) ··· 1392 1391 if (ret < 0) 1393 1392 goto err_put_sync; 1394 1393 } else { 1395 - drm_bridge_chain_pre_enable(dsi->out_bridge); 1394 + list_for_each_entry_reverse(iter, &dsi->bridge_chain, 1395 + chain_node) { 1396 + if (iter->funcs->pre_enable) 1397 + iter->funcs->pre_enable(iter); 1398 + } 1396 1399 } 1397 1400 1398 1401 exynos_dsi_set_display_mode(dsi); ··· 1407 1402 if (ret < 0) 1408 1403 goto err_display_disable; 1409 1404 } else { 1410 - drm_bridge_chain_enable(dsi->out_bridge); 1405 + list_for_each_entry(iter, &dsi->bridge_chain, chain_node) { 1406 + if (iter->funcs->enable) 1407 + iter->funcs->enable(iter); 1408 + } 1411 1409 } 1412 1410 1413 1411 dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE; ··· 1428 1420 static void exynos_dsi_disable(struct drm_encoder *encoder) 1429 1421 { 1430 1422 struct exynos_dsi *dsi = encoder_to_dsi(encoder); 1423 + struct drm_bridge *iter; 1431 1424 1432 1425 if (!(dsi->state & DSIM_STATE_ENABLED)) 1433 1426 return; ··· 1436 1427 dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE; 1437 1428 1438 1429 drm_panel_disable(dsi->panel); 1439 - drm_bridge_chain_disable(dsi->out_bridge); 1430 + 1431 + list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) { 1432 + if (iter->funcs->disable) 1433 + iter->funcs->disable(iter); 1434 + } 1435 + 1440 1436 exynos_dsi_set_display_enable(dsi, false); 1441 1437 drm_panel_unprepare(dsi->panel); 1442 - drm_bridge_chain_post_disable(dsi->out_bridge); 1438 + 1439 + list_for_each_entry(iter, &dsi->bridge_chain, chain_node) { 1440 + if (iter->funcs->post_disable) 1441 + iter->funcs->post_disable(iter); 1442 + } 1443 + 1443 1444 dsi->state &= ~DSIM_STATE_ENABLED; 1444 1445 pm_runtime_put_sync(dsi->dev); 1445 1446 } ··· 1542 1523 if (out_bridge) { 1543 1524 drm_bridge_attach(encoder, out_bridge, NULL); 1544 1525 dsi->out_bridge = out_bridge; 1545 - list_splice(&encoder->bridge_chain, &dsi->bridge_chain); 1526 + list_splice_init(&encoder->bridge_chain, &dsi->bridge_chain); 1546 1527 } else { 1547 1528 int ret = exynos_dsi_create_connector(encoder); 1548 1529