Re: [PATCH] drm/loongson: clean up KMS polling on probe failure

[Myeonghun Pak <mhun512@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4261 bytes --]

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


2026년 5월 12일 (화) 오후 3:26, Thomas Zimmermann <tzimmermann@suse.de>님이 작성:

> 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)
>
>
>

[-- Attachment #2: Type: text/html, Size: 5915 bytes --]