[PATCH] drm/mediatek: Convert legacy DRM logging to drm_* helpers in mtk_dsi.c

[PATCH] drm/mediatek: Convert legacy DRM logging to drm_* helpers in mtk_dsi.c

[Abhishek Rajput]

Replace DRM_INFO(), DRM_WARN() and DRM_ERROR() calls in
drivers/gpu/drm/mediatek/mtk_dsi.c with the corresponding
drm_info(), drm_warn() and drm_err() helpers.

The drm_*() logging helpers take a struct drm_device * argument,
allowing the DRM core to prefix log messages with the correct device
name and instance. This is required to correctly distinguish log
messages on systems with multiple GPUs.

This change aligns the radeon driver with the DRM TODO item:
"Convert logging to drm_* functions with drm_device parameter".

Signed-off-by: Abhishek Rajput <abhiraj21put@gmail.com>

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 0e2bcd5f67b7..a67ad575f5f0 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -510,6 +510,7 @@ static void mtk_dsi_config_vdo_timing_per_line_lp(struct mtk_dsi *dsi)
 	u32 delta;
 	struct mtk_phy_timing *timing = &dsi->phy_timing;
 	struct videomode *vm = &dsi->vm;
+	struct drm_device *drm = dsi->bridge.dev;
 
 	if (dsi->format == MIPI_DSI_FMT_RGB565)
 		dsi_tmp_buf_bpp = 2;
@@ -543,7 +544,7 @@ static void mtk_dsi_config_vdo_timing_per_line_lp(struct mtk_dsi *dsi)
 					     horizontal_backporch_byte /
 					     horizontal_front_back_byte;
 	} else {
-		DRM_WARN("HFP + HBP less than d-phy, FPS will under 60Hz\n");
+		drm_warn(drm, "HFP + HBP less than d-phy, FPS will under 60Hz\n");
 	}
 
 	if ((dsi->mode_flags & MIPI_DSI_HS_PKT_END_ALIGNED) &&
@@ -623,12 +624,13 @@ static s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
 {
 	s32 ret = 0;
 	unsigned long jiffies = msecs_to_jiffies(timeout);
+	struct drm_device *drm = dsi->bridge.dev;
 
 	ret = wait_event_interruptible_timeout(dsi->irq_wait_queue,
 					       dsi->irq_data & irq_flag,
 					       jiffies);
 	if (ret == 0) {
-		DRM_WARN("Wait DSI IRQ(0x%08x) Timeout\n", irq_flag);
+		drm_warn(drm, "Wait DSI IRQ(0x%08x) Timeout\n", irq_flag);
 
 		mtk_dsi_enable(dsi);
 		mtk_dsi_reset_engine(dsi);
@@ -663,9 +665,10 @@ static s32 mtk_dsi_switch_to_cmd_mode(struct mtk_dsi *dsi, u8 irq_flag, u32 t)
 {
 	mtk_dsi_irq_data_clear(dsi, irq_flag);
 	mtk_dsi_set_cmd_mode(dsi);
+	struct drm_device *drm = dsi->bridge.dev;
 
 	if (!mtk_dsi_wait_for_irq_done(dsi, irq_flag, t)) {
-		DRM_ERROR("failed to switch cmd mode\n");
+		drm_err(drm, "failed to switch cmd mode\n");
 		return -ETIME;
 	} else {
 		return 0;
@@ -849,11 +852,12 @@ static void mtk_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
 					     struct drm_atomic_state *state)
 {
 	struct mtk_dsi *dsi = bridge_to_dsi(bridge);
+	struct drm_device *drm = bridge->dev;
 	int ret;
 
 	ret = mtk_dsi_poweron(dsi);
 	if (ret < 0)
-		DRM_ERROR("failed to power on dsi\n");
+		drm_err(drm, "failed to power on dsi\n");
 }
 
 static void mtk_dsi_bridge_atomic_post_disable(struct drm_bridge *bridge,
@@ -916,7 +920,7 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi)
 	ret = drm_simple_encoder_init(drm, &dsi->encoder,
 				      DRM_MODE_ENCODER_DSI);
 	if (ret) {
-		DRM_ERROR("Failed to encoder init to drm\n");
+		drm_err(drm, "Failed to encoder init to drm\n");
 		return ret;
 	}
 
@@ -932,7 +936,7 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi)
 
 	dsi->connector = drm_bridge_connector_init(drm, &dsi->encoder);
 	if (IS_ERR(dsi->connector)) {
-		DRM_ERROR("Unable to create bridge connector\n");
+		drm_err(drm, "Unable to create bridge connector\n");
 		ret = PTR_ERR(dsi->connector);
 		goto err_cleanup_encoder;
 	}
@@ -985,6 +989,7 @@ static int mtk_dsi_host_attach(struct mipi_dsi_host *host,
 {
 	struct mtk_dsi *dsi = host_to_dsi(host);
 	struct device *dev = host->dev;
+	struct drm_device *drm = dsi->bridge.dev;
 	int ret;
 
 	dsi->lanes = device->lanes;
@@ -1012,7 +1017,7 @@ static int mtk_dsi_host_attach(struct mipi_dsi_host *host,
 
 	ret = component_add(host->dev, &mtk_dsi_component_ops);
 	if (ret) {
-		DRM_ERROR("failed to add dsi_host component: %d\n", ret);
+		drm_err(drm, "failed to add dsi_host component: %d\n", ret);
 		drm_bridge_remove(&dsi->bridge);
 		return ret;
 	}
@@ -1034,11 +1039,12 @@ static void mtk_dsi_wait_for_idle(struct mtk_dsi *dsi)
 {
 	int ret;
 	u32 val;
+	struct drm_device *drm = dsi->bridge.dev;
 
 	ret = readl_poll_timeout(dsi->regs + DSI_INTSTA, val, !(val & DSI_BUSY),
 				 4, 2000000);
 	if (ret) {
-		DRM_WARN("polling dsi wait not busy timeout!\n");
+		drm_warn(drm, "polling dsi wait not busy timeout!\n");
 
 		mtk_dsi_enable(dsi);
 		mtk_dsi_reset_engine(dsi);
@@ -1123,6 +1129,7 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host,
 				     const struct mipi_dsi_msg *msg)
 {
 	struct mtk_dsi *dsi = host_to_dsi(host);
+	struct drm_device *drm = dsi->bridge.dev;
 	ssize_t recv_cnt;
 	u8 read_data[16];
 	void *src_addr;
@@ -1153,7 +1160,7 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host,
 	}
 
 	if (!msg->rx_buf) {
-		DRM_ERROR("dsi receive buffer size may be NULL\n");
+		drm_err(drm, "dsi receive buffer size may be NULL\n");
 		ret = -EINVAL;
 		goto restore_dsi_mode;
 	}
@@ -1177,7 +1184,7 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host,
 	if (recv_cnt)
 		memcpy(msg->rx_buf, src_addr, recv_cnt);
 
-	DRM_INFO("dsi get %zd byte data from the panel address(0x%x)\n",
+	drm_info(drm, "dsi get %zd byte data from the panel address(0x%x)\n",
 		 recv_cnt, *((u8 *)(msg->tx_buf)));
 
 restore_dsi_mode:
-- 
2.43.0

Claude review: drm/mediatek: Convert legacy DRM logging to drm_* helpers in mtk_dsi.c

[Claude Code Review Bot]

Overall Series Review

Subject: drm/mediatek: Convert legacy DRM logging to drm_* helpers in mtk_dsi.c
Author: Abhishek Rajput <abhiraj21put@gmail.com>
Patches: 1
Reviewed: 2026-04-23T10:21:29.034621

---

This patch converts legacy `DRM_INFO()`/`DRM_WARN()`/`DRM_ERROR()` calls in `mtk_dsi.c` to the preferred `drm_info()`/`drm_warn()`/`drm_err()` helpers that take a `struct drm_device *`. The goal is correct and aligns with a longstanding DRM TODO item. However, the patch has two significant bugs and a commit message error.

**Critical issues:**
1. **NULL pointer dereference in `mtk_dsi_host_attach()`**: The patch reads `dsi->bridge.dev` at the top of the function, but at this point the bridge has not been attached to any DRM device yet. `bridge.dev` is only set later during `drm_bridge_attach()` (called indirectly via `component_add()` → `mtk_dsi_bind()` → `mtk_dsi_encoder_init()`). This will be NULL when the error path is hit, causing a crash.

2. **Declaration after statement in `mtk_dsi_switch_to_cmd_mode()`**: The variable declaration `struct drm_device *drm = dsi->bridge.dev;` is placed after two function call statements (`mtk_dsi_irq_data_clear`, `mtk_dsi_set_cmd_mode`). The kernel builds with `-Wdeclaration-after-statement` (and `-Werror` in many configs), so this will cause a build failure. All declarations must come before any statements in a block.

3. **Wrong subsystem in commit message**: The commit message says "This change aligns the radeon driver..." but the patch modifies the MediaTek DSI driver. This is a copy-paste error from a different patch.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/mediatek: Convert legacy DRM logging to drm_* helpers in mtk_dsi.c

[Claude Code Review Bot]

Patch Review

**Commit message error:**

> This change aligns the radeon driver with the DRM TODO item

This should say "mediatek driver", not "radeon driver". Copy-paste error.

**Bug: Declaration after statement (build break)**

At `mtk_dsi_switch_to_cmd_mode()`, the new variable is declared after executable statements:

```c
static s32 mtk_dsi_switch_to_cmd_mode(struct mtk_dsi *dsi, u8 irq_flag, u32 t)
{
	mtk_dsi_irq_data_clear(dsi, irq_flag);
	mtk_dsi_set_cmd_mode(dsi);
	struct drm_device *drm = dsi->bridge.dev;  // <-- after statements
```

The declaration must be moved to the top of the function, before the two function calls. This will fail to compile with `-Wdeclaration-after-statement -Werror`.

**Bug: NULL `bridge.dev` in `mtk_dsi_host_attach()` (potential NULL deref)**

```c
static int mtk_dsi_host_attach(struct mipi_dsi_host *host,
			       struct mipi_dsi_device *device)
{
	struct mtk_dsi *dsi = host_to_dsi(host);
	struct device *dev = host->dev;
	struct drm_device *drm = dsi->bridge.dev;  // <-- NULL here
	...
	drm_bridge_add(&dsi->bridge);              // does NOT set bridge.dev

	ret = component_add(host->dev, &mtk_dsi_component_ops);
	if (ret) {
		drm_err(drm, ...);  // passes NULL drm_device
```

`bridge.dev` is set by `drm_bridge_attach()` (called inside `mtk_dsi_encoder_init()` which runs during `mtk_dsi_bind()`, triggered by the component framework *after* `component_add()`). At the point where `drm` is captured, it is NULL. The `drm_err(drm, ...)` call with a NULL first argument will crash. This call site should either keep using `DRM_ERROR()` or use `dev_err(host->dev, ...)` instead.

**Similarly suspect: `mtk_dsi_host_transfer()` and `mtk_dsi_wait_for_idle()`**

These functions also access `dsi->bridge.dev`. While `bridge.dev` is likely valid during normal runtime (the bridge should be attached by then), it's worth verifying that these code paths cannot be reached before the bridge is fully attached, or during teardown after `drm_bridge_detach()` has cleared `bridge.dev` to NULL.

**Minor: trailing `\n` in drm_* calls**

The `drm_*()` helpers already append a newline. The existing `DRM_*()` macros also append a newline, so the `\n` in the format strings was always redundant (producing double newlines). While this patch is just a mechanical conversion and isn't introducing the issue, it would be nice to clean up the trailing `\n` as part of this conversion.

**Correct conversions:**

The conversions in `mtk_dsi_config_vdo_timing_per_line_lp()`, `mtk_dsi_wait_for_irq_done()`, `mtk_dsi_bridge_atomic_pre_enable()`, and `mtk_dsi_encoder_init()` all look correct — `dsi->bridge.dev` or `bridge->dev` is valid in those contexts.

---
Generated by Claude Code Patch Reviewer