Hi Icenowy, Thomas,
Thanks for the review.
I agree that using drmm_kms_helper_poll_init() is the better fix here. It keeps the polling cleanup tied to the DRM device lifetime and avoids adding manual unwind and remove-path cleanup.
I sent a v2 that switches lsdc_pci_probe() to drmm_kms_helper_poll_init().
Thanks,
Myeonghun
Hi
Am 11.05.26 um 19:22 schrieb Icenowy Zheng:
> 在 2026-05-12二的 02:01 +0900,Myeonghun Pak写道:
>> 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 remove
>> path has
>> the same lifetime gap when tearing down a successfully registered
>> device.
>>
>> Route those probe failures through a poll cleanup label. Also
>> finalize
>> polling from remove before unregistering the DRM device.
> Interesting, but it looks like a `drmm_kms_helper_poll_init` function
> exists (while rarely used).
>
> Maybe it's better to switch to this? Or maybe there's some reason not
> to use this?
Agreed. DRM drivers are advised to use managed cleanup for their data
structures. So rather switch to drmm_kms_helper_poll_init(). That also
resolves the problem that loongson never calls poll_fini on regular
removals.
Best regards
Thomas
>
> Thanks,
> Icenowy
>
>> 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>
>> ---
>> drivers/gpu/drm/loongson/lsdc_drv.c | 11 ++++++++---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/loongson/lsdc_drv.c
>> b/drivers/gpu/drm/loongson/lsdc_drv.c
>> index abf5bf68ee..3db1f8690a 100644
>> --- a/drivers/gpu/drm/loongson/lsdc_drv.c
>> +++ b/drivers/gpu/drm/loongson/lsdc_drv.c
>> @@ -297,7 +297,7 @@ static int lsdc_pci_probe(struct pci_dev *pdev,
>> const struct pci_device_id *ent)
>> if (loongson_vblank) {
>> ret = drm_vblank_init(ddev, descp->num_of_crtc);
>> if (ret)
>> - return ret;
>> + goto err_poll_fini;
>>
>> ret = devm_request_irq(&pdev->dev, pdev->irq,
>> descp->funcs->irq_handler,
>> @@ -305,7 +305,7 @@ static int lsdc_pci_probe(struct pci_dev *pdev,
>> const struct pci_device_id *ent)
>> dev_name(&pdev->dev), ddev);
>> if (ret) {
>> drm_err(ddev, "Failed to register interrupt:
>> %d\n", ret);
>> - return ret;
>> + goto err_poll_fini;
>> }
>>
>> drm_info(ddev, "registered irq: %u\n", pdev->irq);
>> @@ -313,17 +313,22 @@ static int lsdc_pci_probe(struct pci_dev *pdev,
>> const struct pci_device_id *ent)
>>
>> ret = drm_dev_register(ddev, 0);
>> if (ret)
>> - return ret;
>> + goto err_poll_fini;
>>
>> drm_client_setup(ddev, NULL);
>>
>> return 0;
>> +
>> +err_poll_fini:
>> + drm_kms_helper_poll_fini(ddev);
>> + return ret;
>> }
>>
>> static void lsdc_pci_remove(struct pci_dev *pdev)
>> {
>> struct drm_device *ddev = pci_get_drvdata(pdev);
>>
>> + drm_kms_helper_poll_fini(ddev);
>> drm_dev_unregister(ddev);
>> drm_atomic_helper_shutdown(ddev);
>> }
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)