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

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