[PATCH v2 0/2] drm/tidss: Two minor fixes

[PATCH v2 0/2] drm/tidss: Two minor fixes

[Tomi Valkeinen]

Two minor fixes. One to drop an extra drm_mode_config_reset() call,
another to fix a "Missing drm_bridge_add() before attach" warning print.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
Changes in v2:
- Fix incorrect title for the second patch
- Add Maxime's acks
- Link to v1: https://lore.kernel.org/r/20260311-tidss-minor-fixes-v1-0-ee5e6e14a566@ideasonboard.com

---
Tomi Valkeinen (2):
      drm/tidss: Drop extra drm_mode_config_reset() call
      drm/tidss: Fix missing drm_bridge_add() call

 drivers/gpu/drm/tidss/tidss_encoder.c | 2 ++
 drivers/gpu/drm/tidss/tidss_kms.c     | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)
---
base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
change-id: 20260311-tidss-minor-fixes-c0f853a5326f

Best regards,
-- 
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

[PATCH v2 1/2] drm/tidss: Drop extra drm_mode_config_reset() call

[Tomi Valkeinen]

We are calling drm_mode_config_reset() twice at probe time. There's no
reason for this and the second call can be removed, reducing work at
probe time slightly.

Fixes: 32a1795f57ee ("drm/tidss: New driver for TI Keystone platform Display SubSystem")
Acked-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/tidss/tidss_kms.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c
index 8bb93194e5ac..b4779c09a1bf 100644
--- a/drivers/gpu/drm/tidss/tidss_kms.c
+++ b/drivers/gpu/drm/tidss/tidss_kms.c
@@ -287,8 +287,6 @@ int tidss_modeset_init(struct tidss_device *tidss)
 	if (ret)
 		return ret;
 
-	drm_mode_config_reset(ddev);
-
 	dev_dbg(tidss->dev, "%s done\n", __func__);
 
 	return 0;

-- 
2.43.0

Claude review: drm/tidss: Drop extra drm_mode_config_reset() call

[Claude Code Review Bot]

Patch Review

**Status: Looks good.**

The patch removes `drm_mode_config_reset(ddev)` from `tidss_kms.c` (`tidss_modeset_init()`). Checking the kernel tree confirms there are currently two calls:
- `tidss_kms.c:290` — inside `tidss_modeset_init()` (the one being removed)
- `tidss_drv.c:174` — in the probe path, called *after* `tidss_modeset_init()` returns

Since `tidss_drv.c:174` runs later (after `drm_kms_helper_poll_init()` and before `drm_dev_register()`), the earlier call in `tidss_kms.c` is indeed redundant — the state will just get reset again. Removing it is correct and the Fixes tag to the original driver commit is appropriate.

No issues.

---
Generated by Claude Code Patch Reviewer

[PATCH v2 2/2] drm/tidss: Fix missing drm_bridge_add() call

[Tomi Valkeinen]

tidss encoder-bridge is not added with drm_bridge_add() call, which
leads to:

[drm] Missing drm_bridge_add() before attach

Add the missing call, using devm_drm_bridge_add() variant to get the
drm_bridge_remove() handled automatically.

The commit marked with the Fixes tag (from v6.6) is the commit that
added the encoder bridge without drm_bridge_add(). However, this fix is
not directly applicable there as devm_drm_bridge_alloc() was not used to
alloc the bridge, so using devm version for drm_bridge_add() wouldn't be
safe. Instead, drm_bridge_add() and drm_bridge_remove() would be needed
there, but that would require new plumbing code as we don't have a
separate cleanup function in the tidss_encoder.c, not in the tidss_kms.c
from which the encoder is created.

Also, there has been no reported bugs caused by the missing
drm_bridge_add(). The drm_bridge_add() initializes the bridge's
hpd_mutex, but HPD is not used for the encoder bridge. drm_bridge_add()
also adds the bridge to the global bridge_list, which is only used in
of_drm_find_bridge(), and again that is not used for the encoder bridge.

Thus, while the original commit is not right, there should be no bugs
caused by it, and for the time being I'm not sending a patch for the
stable kernels for the original commit.

This fix applies on top of commit 66cdf05f8548 ("drm/tidss: encoder:
convert to devm_drm_bridge_alloc()"), which changes the tidss_encoder.c
to use the devm variant (added in v6.17). The warning print was added in
v6.19, so applying this fix to v6.17+ gets rid of the warning for all
kernel versions.

Cc: stable@vger.kernel.org # v6.17+
Fixes: c932ced6b585 ("drm/tidss: Update encoder/bridge chain connect model")
Acked-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/tidss/tidss_encoder.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c b/drivers/gpu/drm/tidss/tidss_encoder.c
index 81a04f767770..db467bbcdb77 100644
--- a/drivers/gpu/drm/tidss/tidss_encoder.c
+++ b/drivers/gpu/drm/tidss/tidss_encoder.c
@@ -106,6 +106,8 @@ int tidss_encoder_create(struct tidss_device *tidss,
 	enc = &t_enc->encoder;
 	enc->possible_crtcs = possible_crtcs;
 
+	devm_drm_bridge_add(tidss->dev, &t_enc->bridge);
+
 	/* Attaching first bridge to the encoder */
 	ret = drm_bridge_attach(enc, &t_enc->bridge, NULL,
 				DRM_BRIDGE_ATTACH_NO_CONNECTOR);

-- 
2.43.0

Claude review: drm/tidss: Fix missing drm_bridge_add() call

[Claude Code Review Bot]

Patch Review

**Status: Minor issue — missing return value check.**

The fix correctly adds `devm_drm_bridge_add()` before the `drm_bridge_attach()` call, which resolves the "Missing drm_bridge_add() before attach" warning. The placement at line 108 (after setting `possible_crtcs`, before attaching) is correct.

The commit message is excellent — it carefully documents why the Fixes tag references one commit but a stable backport to pre-v6.17 would need different code, and why the bug hasn't caused real issues despite being present since v6.6.

**Issue: `devm_drm_bridge_add()` returns `int` and can fail** (it calls `devm_add_action_or_reset()` internally), but the return value is not checked:

```c
+	devm_drm_bridge_add(tidss->dev, &t_enc->bridge);
```

Other callers in the tree check this return value (e.g., `msm/hdmi/hdmi_bridge.c`). This should be:

```c
	ret = devm_drm_bridge_add(tidss->dev, &t_enc->bridge);
	if (ret)
		return ret;
```

While the failure mode (OOM from `devm_add_action_or_reset`) is unlikely, it's good practice to propagate the error, especially since the function already has `ret` declared and returns errors from other calls on the same path.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/tidss: Two minor fixes

[Claude Code Review Bot]

Overall Series Review

Subject: drm/tidss: Two minor fixes
Author: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Patches: 3
Reviewed: 2026-03-12T07:10:39.595409

---

This is a clean, well-explained 2-patch series fixing two minor issues in the TI TIDSS DRM driver. Patch 1 removes a redundant `drm_mode_config_reset()` call, and patch 2 adds a missing `drm_bridge_add()` registration. The commit messages are thorough — particularly patch 2, which carefully explains the Fixes tag scope and stable backport considerations. The series is low risk and the changes are correct in intent, though patch 2 has a minor issue.

---
Generated by Claude Code Patch Reviewer