* [PATCH v3] drm/loongson: Use managed KMS polling
@ 2026-05-13 6:57 Myeonghun Pak
2026-05-16 2:30 ` Claude review: " Claude Code Review Bot
2026-05-16 2:30 ` Claude Code Review Bot
0 siblings, 2 replies; 7+ messages in thread
From: Myeonghun Pak @ 2026-05-13 6:57 UTC (permalink / raw)
To: dri-devel
Cc: Myeonghun Pak, Icenowy Zheng, Thomas Zimmermann, Sui Jingfeng,
Jianmin Lv, Qianhai Wu, Huacai Chen, Mingcong Bai, Xi Ruoyao,
Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
linux-kernel, stable, Ijae Kim, Huacai Chen
lsdc_pci_probe() initializes KMS polling before setting up vblank support,
requesting the IRQ and registering the DRM device. If any of those later
steps fails, probe returns without finalizing polling. The driver also
never finalizes polling on regular removal.
Use drmm_kms_helper_poll_init() so polling is tied to the DRM device
lifetime and automatically finalized on probe failure and device removal.
This issue was identified during our ongoing static-analysis research while
reviewing kernel code.
Fixes: f39db26c5428 ("drm: Add kms driver for loongson display controller")
Cc: stable@vger.kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Jianmin Lv <lvjianmin@loongson.cn>
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
Changes in v3:
- Capitalize the subject as suggested by Huacai Chen.
- Add Reviewed-by and Acked-by tags.
Changes in v2:
- Switch to drmm_kms_helper_poll_init() as suggested by Icenowy Zheng
and Thomas Zimmermann instead of adding manual cleanup paths.
drivers/gpu/drm/loongson/lsdc_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/loongson/lsdc_drv.c b/drivers/gpu/drm/loongson/lsdc_drv.c
index abf5bf68ee..4b97750897 100644
--- a/drivers/gpu/drm/loongson/lsdc_drv.c
+++ b/drivers/gpu/drm/loongson/lsdc_drv.c
@@ -292,7 +292,7 @@ static int lsdc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
vga_client_register(pdev, lsdc_vga_set_decode);
- drm_kms_helper_poll_init(ddev);
+ drmm_kms_helper_poll_init(ddev);
if (loongson_vblank) {
ret = drm_vblank_init(ddev, descp->num_of_crtc);
--
2.47.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Claude review: drm/loongson: Use managed KMS polling
2026-05-13 6:57 [PATCH v3] drm/loongson: Use managed KMS polling Myeonghun Pak
@ 2026-05-16 2:30 ` Claude Code Review Bot
2026-05-16 2:30 ` Claude Code Review Bot
1 sibling, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 2:30 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/loongson: Use managed KMS polling
Author: Myeonghun Pak <mhun512@gmail.com>
Patches: 1
Reviewed: 2026-05-16T12:30:12.889221
---
This is a single-patch series that replaces `drm_kms_helper_poll_init()` with its managed counterpart `drmm_kms_helper_poll_init()` in the loongson LSDC driver. The change is correct, well-motivated, and has already been reviewed/acked by the relevant maintainers.
**Assessment: Good to merge.**
The commit message clearly identifies the problem: `drm_kms_helper_poll_init()` is called early in probe, but if later steps (`drm_vblank_init`, `devm_request_irq`, `drm_dev_register`) fail, the driver returns without calling `drm_kms_helper_poll_fini()`. Additionally, `lsdc_pci_remove()` (line 324-330 in the current tree) never calls `drm_kms_helper_poll_fini()` either — it only does `drm_dev_unregister()` and `drm_atomic_helper_shutdown()`. Using `drmm_kms_helper_poll_init()` ties the polling lifecycle to the DRM device managed cleanup, fixing both the error-path leak and the missing finalization on removal.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 7+ messages in thread
* Claude review: drm/loongson: Use managed KMS polling
2026-05-13 6:57 [PATCH v3] drm/loongson: Use managed KMS polling Myeonghun Pak
2026-05-16 2:30 ` Claude review: " Claude Code Review Bot
@ 2026-05-16 2:30 ` Claude Code Review Bot
1 sibling, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 2:30 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correctness: No issues.**
The change from:
```c
- drm_kms_helper_poll_init(ddev);
+ drmm_kms_helper_poll_init(ddev);
```
is the right fix. `drmm_kms_helper_poll_init()` is declared in `drm/drm_probe_helper.h` and registers a managed cleanup action via `drmm_add_action_or_reset()`, so `drm_kms_helper_poll_fini()` is called automatically when the DRM device is released.
Looking at the probe function, the error paths at lines 300-301 and 308-310 return directly without any cleanup of the poll init — this patch fixes those leaks. The removal path in `lsdc_pci_remove()` also lacks a `drm_kms_helper_poll_fini()` call, which is likewise fixed by this change.
**Tags:** The patch has appropriate `Fixes:`, `Cc: stable`, `Reviewed-by` (Thomas Zimmermann, Huacai Chen), and `Acked-by` (Jianmin Lv) tags.
**Nit (non-blocking):** The return type of `drmm_kms_helper_poll_init()` is `void`, so there's no error to check — the current usage is correct.
**Verdict: Looks good.** No changes needed.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] drm/loongson: use managed KMS polling
@ 2026-05-12 6:53 Myeonghun Pak
2026-05-16 4:07 ` Claude review: " Claude Code Review Bot
2026-05-16 4:07 ` Claude Code Review Bot
0 siblings, 2 replies; 7+ messages in thread
From: Myeonghun Pak @ 2026-05-12 6:53 UTC (permalink / raw)
To: Tobias Klauser, Greg Kroah-Hartman, Jiri Slaby, dri-devel
Cc: Myeonghun Pak, Icenowy Zheng, Thomas Zimmermann, Sui Jingfeng,
Jianmin Lv, Qianhai Wu, Huacai Chen, Mingcong Bai, Xi Ruoyao,
Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
linux-kernel, stable, Ijae Kim
lsdc_pci_probe() initializes KMS polling before setting up vblank support,
requesting the IRQ and registering the DRM device. If any of those later
steps fails, probe returns without finalizing polling. The driver also
never finalizes polling on regular removal.
Use drmm_kms_helper_poll_init() so polling is tied to the DRM device
lifetime and automatically finalized on probe failure and device removal.
This issue was identified during our ongoing static-analysis research while
reviewing kernel code.
Fixes: f39db26c5428 ("drm: Add kms driver for loongson display controller")
Cc: stable@vger.kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
Changes in v2:
- Switch to drmm_kms_helper_poll_init() as suggested by Icenowy Zheng
and Thomas Zimmermann instead of adding manual cleanup paths.
drivers/gpu/drm/loongson/lsdc_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/loongson/lsdc_drv.c b/drivers/gpu/drm/loongson/lsdc_drv.c
index abf5bf68ee..4b97750897 100644
--- a/drivers/gpu/drm/loongson/lsdc_drv.c
+++ b/drivers/gpu/drm/loongson/lsdc_drv.c
@@ -292,7 +292,7 @@ static int lsdc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
vga_client_register(pdev, lsdc_vga_set_decode);
- drm_kms_helper_poll_init(ddev);
+ drmm_kms_helper_poll_init(ddev);
if (loongson_vblank) {
ret = drm_vblank_init(ddev, descp->num_of_crtc);
--
2.47.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Claude review: drm/loongson: use managed KMS polling
2026-05-12 6:53 [PATCH v2] drm/loongson: use " Myeonghun Pak
@ 2026-05-16 4:07 ` Claude Code Review Bot
2026-05-16 4:07 ` Claude Code Review Bot
1 sibling, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 4:07 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/loongson: use managed KMS polling
Author: Myeonghun Pak <mhun512@gmail.com>
Patches: 2
Reviewed: 2026-05-16T14:07:27.468757
---
This is a single-patch series that replaces `drm_kms_helper_poll_init()` with its managed counterpart `drmm_kms_helper_poll_init()` in the loongson display controller driver. The fix is correct and well-motivated.
**The problem is real:** `lsdc_pci_probe()` calls `drm_kms_helper_poll_init()` at line 296, but if subsequent steps fail (`drm_vblank_init`, `devm_request_irq`, or `drm_dev_register`), the function returns without calling `drm_kms_helper_poll_fini()`. Furthermore, `lsdc_pci_remove()` also never calls `drm_kms_helper_poll_fini()`, so polling is never finalized even on normal device removal. The managed variant ties the polling lifecycle to the DRM device and handles cleanup automatically.
**Verdict: Patch looks good.** One minor observation below.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 7+ messages in thread
* Claude review: drm/loongson: use managed KMS polling
2026-05-12 6:53 [PATCH v2] drm/loongson: use " Myeonghun Pak
2026-05-16 4:07 ` Claude review: " Claude Code Review Bot
@ 2026-05-16 4:07 ` Claude Code Review Bot
1 sibling, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 4:07 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Code change:**
```c
- drm_kms_helper_poll_init(ddev);
+ drmm_kms_helper_poll_init(ddev);
```
This is a clean one-line substitution. `drmm_kms_helper_poll_init()` is the established managed wrapper (already used by `ast`, `hibmc`, and `udl` drivers in the tree), and it registers a cleanup action via `drmm_add_action_or_reset()` so that `drm_kms_helper_poll_fini()` is called automatically when the DRM device is released.
**Correctness:** The switch is safe here. `drmm_kms_helper_poll_init()` has the same signature (takes only `struct drm_device *`). The managed cleanup will run after `drm_dev_unregister()` tears down the device, which is the correct ordering — polling should be finalized after the device is unregistered, and the `drmm` action ordering guarantees this since the poll_init action is registered before `drm_dev_register()`.
**Minor observation:** The commit message and Fixes tag are appropriate. The `Cc: stable` tag is reasonable since this is a resource leak on error paths that has existed since the driver was introduced. The Co-developed-by / Signed-off-by chain is correctly formatted.
**No issues found.**
Reviewed-by worthy as-is.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] drm/loongson: use managed KMS polling
@ 2026-05-12 6:36 Myeonghun Pak
2026-05-16 4:17 ` Claude review: " Claude Code Review Bot
2026-05-16 4:17 ` Claude Code Review Bot
0 siblings, 2 replies; 7+ messages in thread
From: Myeonghun Pak @ 2026-05-12 6:36 UTC (permalink / raw)
To: dri-devel
Cc: Myeonghun Pak, Icenowy Zheng, Thomas Zimmermann, Sui Jingfeng,
Jianmin Lv, Qianhai Wu, Huacai Chen, Mingcong Bai, Xi Ruoyao,
Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
linux-kernel, stable, Ijae Kim
lsdc_pci_probe() initializes KMS polling before setting up vblank support,
requesting the IRQ and registering the DRM device. If any of those later
steps fails, probe returns without finalizing polling. The driver also
never finalizes polling on regular removal.
Use drmm_kms_helper_poll_init() so polling is tied to the DRM device
lifetime and automatically finalized on probe failure and device removal.
This issue was identified during our ongoing static-analysis research while
reviewing kernel code.
Fixes: f39db26c5428 ("drm: Add kms driver for loongson display controller")
Cc: stable@vger.kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
Changes in v2:
- Switch to drmm_kms_helper_poll_init() as suggested by Icenowy Zheng
and Thomas Zimmermann instead of adding manual cleanup paths.
drivers/gpu/drm/loongson/lsdc_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/loongson/lsdc_drv.c b/drivers/gpu/drm/loongson/lsdc_drv.c
index abf5bf68ee..4b97750897 100644
--- a/drivers/gpu/drm/loongson/lsdc_drv.c
+++ b/drivers/gpu/drm/loongson/lsdc_drv.c
@@ -292,7 +292,7 @@ static int lsdc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
vga_client_register(pdev, lsdc_vga_set_decode);
- drm_kms_helper_poll_init(ddev);
+ drmm_kms_helper_poll_init(ddev);
if (loongson_vblank) {
ret = drm_vblank_init(ddev, descp->num_of_crtc);
--
2.47.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Claude review: drm/loongson: use managed KMS polling
2026-05-12 6:36 [PATCH v2] " Myeonghun Pak
@ 2026-05-16 4:17 ` Claude Code Review Bot
2026-05-16 4:17 ` Claude Code Review Bot
1 sibling, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 4:17 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Analysis of the problem:**
Looking at the current `lsdc_pci_probe()` (line 296), `drm_kms_helper_poll_init(ddev)` is called, but there are three subsequent failure paths (lines 300, 309, 316) that all `return ret` without finalizing polling. The `lsdc_pci_remove()` function (line 324-330) also lacks a `drm_kms_helper_poll_fini()` call:
```c
static void lsdc_pci_remove(struct pci_dev *pdev)
{
struct drm_device *ddev = pci_get_drvdata(pdev);
drm_dev_unregister(ddev);
drm_atomic_helper_shutdown(ddev);
}
```
**The fix:**
```diff
- drm_kms_helper_poll_init(ddev);
+ drmm_kms_helper_poll_init(ddev);
```
This is the right approach. The managed function internally calls `drm_kms_helper_poll_init()` and then registers a cleanup action via `drmm_add_action_or_reset()`, so no manual `drm_kms_helper_poll_fini()` calls are needed in error paths or remove/shutdown.
**No issues found.** The function signature is `void drmm_kms_helper_poll_init(struct drm_device *dev)` — same as the non-managed version — so the one-line substitution is a complete fix.
The Fixes tag and Cc: stable are appropriate since this is a real resource leak present since the driver was introduced.
**Reviewed-by recommendation:** This patch is ready to merge.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 7+ messages in thread* Claude review: drm/loongson: use managed KMS polling
2026-05-12 6:36 [PATCH v2] " Myeonghun Pak
2026-05-16 4:17 ` Claude review: " Claude Code Review Bot
@ 2026-05-16 4:17 ` Claude Code Review Bot
1 sibling, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 4:17 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/loongson: use managed KMS polling
Author: Myeonghun Pak <mhun512@gmail.com>
Patches: 5
Reviewed: 2026-05-16T14:17:59.205016
---
This is a single-patch series that converts the loongson DRM driver from `drm_kms_helper_poll_init()` to the managed variant `drmm_kms_helper_poll_init()`. The change is correct, well-motivated, and minimal.
The commit message clearly describes the problem: `lsdc_pci_probe()` calls `drm_kms_helper_poll_init()` early, but if subsequent steps (`drm_vblank_init`, `devm_request_irq`, `drm_dev_register`) fail, the probe function returns without calling `drm_kms_helper_poll_fini()`. Additionally, `lsdc_pci_remove()` (line 324-330 of `lsdc_drv.c`) never calls `drm_kms_helper_poll_fini()` either, so even on clean removal, polling resources leak.
The fix is appropriate: `drmm_kms_helper_poll_init()` uses `drmm_add_action_or_reset()` to tie the poll cleanup to the DRM device lifetime, so finalization happens automatically on both error-path teardown and normal device removal.
**Verdict: Looks good.** This is a clean, minimal fix following the same pattern already adopted by `ast`, `hibmc`, and `udl` drivers in-tree.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-16 4:17 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-13 6:57 [PATCH v3] drm/loongson: Use managed KMS polling Myeonghun Pak
2026-05-16 2:30 ` Claude review: " Claude Code Review Bot
2026-05-16 2:30 ` Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-05-12 6:53 [PATCH v2] drm/loongson: use " Myeonghun Pak
2026-05-16 4:07 ` Claude review: " Claude Code Review Bot
2026-05-16 4:07 ` Claude Code Review Bot
2026-05-12 6:36 [PATCH v2] " Myeonghun Pak
2026-05-16 4:17 ` Claude review: " Claude Code Review Bot
2026-05-16 4:17 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox