mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/mediatek: mtk_crtc: Fix connector route handling
@ 2026-10-11  0:42 zoan37
  2026-10-11  0:42 ` [PATCH 1/3] drm/mediatek: mtk_crtc: Look up connector routes in the CRTC's own mmsys zoan37
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: zoan37 @ 2026-10-11  0:42 UTC (permalink / raw)
  To: Chun-Kuang Hu, Philipp Zabel
  Cc: David Airlie, Simona Vetter, Matthias Brugger,
	AngeloGioacchino Del Regno, Jason-JH Lin, CK Hu, Nancy Lin,
	Nathan Lu, dri-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel

Three fixes for the connector routes of mtk_crtc (the output component
picked at enable time from the encoder in use), found while bringing up
the external display path of an MT8189 Chromebook, where one CRTC
drives either an HDMI bridge on DSI0 or USB-C DisplayPort on DVO1:

1. The route components are looked up in the private data of the mmsys
   at index drm_crtc_index(), not of the CRTC's own mmsys. It only works
   by coincidence today (the only route table is on MT8188's CRTC 0);
   on MT8189 it oopsed on the first HDMI hotplug.
2. Destroying a CRTC whose route slot was never filled dereferences
   NULL.
3. The route is only picked when connectors_changed is set, so a CRTC
   whose connector was attached or swapped while it was off is enabled
   with no output (NULL dereference) or with the old one.

All three were compile-tested with W=1 on next-20261008 and run on the
MT8189 board; the per-patch notes say what was and wasn't exercised.
The MT8189 display support itself isn't upstream yet; these patches
don't depend on it.

The changes were written with the help of an AI coding assistant
(hence the Assisted-by tags).

zoan37 (3):
  drm/mediatek: mtk_crtc: Look up connector routes in the CRTC's own
    mmsys
  drm/mediatek: mtk_crtc: Skip the empty connector route slot in destroy
  drm/mediatek: mtk_crtc: Pick the connector route on every CRTC enable

 drivers/gpu/drm/mediatek/mtk_crtc.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)


base-commit: aac26bee2287c88af5be5a5ff96d783b19a28790
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] drm/mediatek: mtk_crtc: Look up connector routes in the CRTC's own mmsys
  2026-10-11  0:42 [PATCH 0/3] drm/mediatek: mtk_crtc: Fix connector route handling zoan37
@ 2026-10-11  0:42 ` zoan37
  2026-10-11  0:42 ` [PATCH 2/3] drm/mediatek: mtk_crtc: Skip the empty connector route slot in destroy zoan37
  2026-10-11  0:42 ` [PATCH 3/3] drm/mediatek: mtk_crtc: Pick the connector route on every CRTC enable zoan37
  2 siblings, 0 replies; 4+ messages in thread
From: zoan37 @ 2026-10-11  0:42 UTC (permalink / raw)
  To: Chun-Kuang Hu, Philipp Zabel
  Cc: David Airlie, Simona Vetter, Matthias Brugger,
	AngeloGioacchino Del Regno, Jason-JH Lin, CK Hu, Nancy Lin,
	Nathan Lu, dri-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel

mtk_crtc_update_output() finds the private data of the display
controller (mmsys) that a CRTC belongs to with

  priv->all_drm_private[drm_crtc_index(crtc)]

but all_drm_private[] has one entry per mmsys (mmsys_dev_num entries),
not one per CRTC. mtk_crtc_create() gets the right entry from its
priv_data_index argument, and the two only agree when every mmsys
drives exactly one CRTC and the CRTCs are created in mmsys order.

When a CRTC with connector routes sits on an mmsys that drives more
than one CRTC, the lookup reads past the end of all_drm_private[] (or
picks another mmsys' data), and the route component taken from that
data's ddp_comp[] array is bogus. On an MT8189 board with connector
routes on the external display path (CRTC 1 on the only mmsys), the
first HDMI hotplug oopsed in the compositor's atomic commit.

With the route tables currently in the tree (only MT8188 VDOSYS0, whose
main path is always CRTC 0 and all_drm_private[0]) the two indices
happen to match, so nothing in-tree hits this yet. It triggers as soon
as routes are used on a path that is not the first CRTC of the first
mmsys.

Remember the mmsys private data mtk_crtc_create() used for the CRTC and
use it in mtk_crtc_update_output().

Fixes: 01389b324c97 ("drm/mediatek: Add connector dynamic selection capability")
Assisted-by: LLM
Signed-off-by: zoan37 <agentzoan@gmail.com>
---

Notes:
    Testing: on an MT8189 Chromebook (Lenovo IdeaPad Slim 3 Chromebook,
    "quigon") running next-20261008 with the not yet merged MT8189 display
    support and a local change that puts the connector routes
    ({1, DVO1}, {1, DSI0}) on the external path. Without this patch the
    first HDMI hotplug oopsed in the compositor's commit; with a
    functionally identical patch (the field had another name), HDMI
    (DSI0 -> IT61620 bridge, 3840x2160@30) and USB-C DisplayPort (DVO1)
    both come up on CRTC 1. Not tested on MT8188, the only in-tree user of
    connector routes, where the looked-up entry does not change. This exact
    patch was compile-tested (W=1) on next-20261008.

 drivers/gpu/drm/mediatek/mtk_crtc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
index 2920f3198ef6..b54f8565d2fa 100644
--- a/drivers/gpu/drm/mediatek/mtk_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
@@ -59,6 +59,7 @@ struct mtk_crtc {
 #endif
 
 	struct device			*mmsys_dev;
+	struct mtk_drm_private		*mmsys_priv;
 	struct device			*dma_dev;
 	struct mtk_mutex		*mutex;
 	unsigned int			ddp_comp_nr;
@@ -696,7 +697,7 @@ static void mtk_crtc_update_output(struct drm_crtc *crtc,
 	if (!mtk_crtc->num_conn_routes)
 		return;
 
-	priv = ((struct mtk_drm_private *)crtc->dev->dev_private)->all_drm_private[crtc_index];
+	priv = mtk_crtc->mmsys_priv;
 	dev = priv->dev;
 
 	dev_dbg(dev, "connector change:%d, encoder mask:0x%x for crtc:%d\n",
@@ -1055,6 +1056,7 @@ int mtk_crtc_create(struct drm_device *drm_dev, const unsigned int *path,
 
 	mtk_crtc->ddp_comp_nr = path_len;
 	mtk_crtc->mmsys_dev = priv->mmsys_dev;
+	mtk_crtc->mmsys_priv = priv;
 
 	mtk_crtc->mutex = mtk_mutex_get(priv->mutex_dev);
 	if (IS_ERR(mtk_crtc->mutex)) {
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/3] drm/mediatek: mtk_crtc: Skip the empty connector route slot in destroy
  2026-10-11  0:42 [PATCH 0/3] drm/mediatek: mtk_crtc: Fix connector route handling zoan37
  2026-10-11  0:42 ` [PATCH 1/3] drm/mediatek: mtk_crtc: Look up connector routes in the CRTC's own mmsys zoan37
@ 2026-10-11  0:42 ` zoan37
  2026-10-11  0:42 ` [PATCH 3/3] drm/mediatek: mtk_crtc: Pick the connector route on every CRTC enable zoan37
  2 siblings, 0 replies; 4+ messages in thread
From: zoan37 @ 2026-10-11  0:42 UTC (permalink / raw)
  To: Chun-Kuang Hu, Philipp Zabel
  Cc: David Airlie, Simona Vetter, Matthias Brugger,
	AngeloGioacchino Del Regno, Jason-JH Lin, CK Hu, Nancy Lin,
	Nathan Lu, dri-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel

A CRTC created with connector routes gets one extra ddp_comp[] slot for
its output component, and ddp_comp_nr counts it, but the slot stays
NULL until mtk_crtc_update_output() fills it in the first atomic
enable.

mtk_crtc_destroy() walks all ddp_comp_nr slots and passes each one to
mtk_ddp_comp_unregister_vblank_cb(), which dereferences it. Tearing
down a CRTC that was never enabled therefore dereferences NULL: unbinding
or removing mediatek-drm while that display path was never used, or
an error in mtk_drm_kms_init() after the CRTC was created.

Skip the empty slot. Nothing was registered for it anyway: the routed
components (DSI, DPI, DP_INTF, ...) have no vblank callback.

Fixes: 01389b324c97 ("drm/mediatek: Add connector dynamic selection capability")
Assisted-by: LLM
Signed-off-by: zoan37 <agentzoan@gmail.com>
---

Notes:
    Testing: found by reading the code; not reproduced. The same check runs
    on the MT8189 Chromebook described in patch 1 (boot, HDMI/DP hotplug,
    suspend/resume), but that does not exercise the destroy path (the
    driver is built in and was not unbound). Compile-tested (W=1) on
    next-20261008.

 drivers/gpu/drm/mediatek/mtk_crtc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
index b54f8565d2fa..704931b369b6 100644
--- a/drivers/gpu/drm/mediatek/mtk_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
@@ -145,6 +145,10 @@ static void mtk_crtc_destroy(struct drm_crtc *crtc)
 		struct mtk_ddp_comp *comp;
 
 		comp = mtk_crtc->ddp_comp[i];
+		/* The connector route slot is empty until the first enable */
+		if (!comp)
+			continue;
+
 		mtk_ddp_comp_unregister_vblank_cb(comp);
 	}
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 3/3] drm/mediatek: mtk_crtc: Pick the connector route on every CRTC enable
  2026-10-11  0:42 [PATCH 0/3] drm/mediatek: mtk_crtc: Fix connector route handling zoan37
  2026-10-11  0:42 ` [PATCH 1/3] drm/mediatek: mtk_crtc: Look up connector routes in the CRTC's own mmsys zoan37
  2026-10-11  0:42 ` [PATCH 2/3] drm/mediatek: mtk_crtc: Skip the empty connector route slot in destroy zoan37
@ 2026-10-11  0:42 ` zoan37
  2 siblings, 0 replies; 4+ messages in thread
From: zoan37 @ 2026-10-11  0:42 UTC (permalink / raw)
  To: Chun-Kuang Hu, Philipp Zabel
  Cc: David Airlie, Simona Vetter, Matthias Brugger,
	AngeloGioacchino Del Regno, Jason-JH Lin, CK Hu, Nancy Lin,
	Nathan Lu, dri-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel

mtk_crtc_update_output() only picks the output component of a CRTC
with connector routes when connectors_changed is set in the commit that
enables the CRTC. But connectors_changed only says that the connectors
changed in that commit, not since the CRTC was last enabled. A connector
can be attached to (or moved to) the CRTC while it is inactive, and the
later commit that only sets ACTIVE then has active_changed but not
connectors_changed:

- If the CRTC had never been enabled, the route slot is still NULL and
  mtk_crtc_ddp_hw_init() dereferences it (mtk_ddp_comp_clk_enable()
  and so on).
- If the CRTC was driven through another route before, e.g. a CRTC
  shared by an HDMI bridge on DSI and a DisplayPort output that is
  switched off (DPMS) while the monitor moves from one to the other,
  the pipeline is set up for the old output.

update_output() only runs from atomic_enable and the lookup is a short
loop over the routes, so do it on every enable.

Fixes: 01389b324c97 ("drm/mediatek: Add connector dynamic selection capability")
Assisted-by: LLM
Signed-off-by: zoan37 <agentzoan@gmail.com>
---

Notes:
    Testing: found by reading the code. The same change runs on the MT8189
    Chromebook described in patch 1, where one routed CRTC drives either
    HDMI or USB-C DisplayPort: DPMS off/on and suspend/resume with a DP
    monitor attached come back. The sequence that fails without it
    (connector attached or swapped while the CRTC is inactive, then only
    ACTIVE set) was not reproduced on purpose. Compile-tested (W=1) on
    next-20261008.

 drivers/gpu/drm/mediatek/mtk_crtc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
index 704931b369b6..0bfd17f68282 100644
--- a/drivers/gpu/drm/mediatek/mtk_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
@@ -695,9 +695,6 @@ static void mtk_crtc_update_output(struct drm_crtc *crtc,
 	struct mtk_drm_private *priv;
 	unsigned int encoder_mask = crtc_state->encoder_mask;
 
-	if (!crtc_state->connectors_changed)
-		return;
-
 	if (!mtk_crtc->num_conn_routes)
 		return;
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-11  0:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-11  0:42 [PATCH 0/3] drm/mediatek: mtk_crtc: Fix connector route handling zoan37
2026-10-11  0:42 ` [PATCH 1/3] drm/mediatek: mtk_crtc: Look up connector routes in the CRTC's own mmsys zoan37
2026-10-11  0:42 ` [PATCH 2/3] drm/mediatek: mtk_crtc: Skip the empty connector route slot in destroy zoan37
2026-10-11  0:42 ` [PATCH 3/3] drm/mediatek: mtk_crtc: Pick the connector route on every CRTC enable zoan37

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®