From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 60D47CD4851 for ; Tue, 12 May 2026 06:36:15 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C94D610E02C; Tue, 12 May 2026 06:36:14 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="k+d+6sIE"; dkim-atps=neutral Received: from mail-ej1-f43.google.com (mail-ej1-f43.google.com [209.85.218.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id EC42B10E02C for ; Tue, 12 May 2026 06:36:12 +0000 (UTC) Received: by mail-ej1-f43.google.com with SMTP id a640c23a62f3a-b8f9568e074so794432666b.0 for ; Mon, 11 May 2026 23:36:12 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1778567771; cv=none; d=google.com; s=arc-20240605; b=IQV2sVLhOSh0tBcy9WnVw0NQ7M1n7gQfPsUpm3bjTeQWsFoBcl5QPIpvl5q7kgqWKf 7FvgG/eLl/XR6d8pqTCGTO8erDA+1wvjivS6cyb5Ch/rwXTENeAUzKt4gmmC4p/QqtsG ueVoqPmwNNSDz4MV/wxtiYhphLxCAGbtM3BZw7Wpqgjj+xBGxbd7THauNDW/MfUYY9Qz H2rq8u+cFq4IgcPIHcUDYegDTnXUfqP90QL5twH3WTzOr5/tjHjKYmSZ+HZpJUp0ox+Q q0cU9fWfmBHCnngPlffOkTP/SndukznYiDwCoLC+373nrBlId10tj6MSj7G7TNHEQMcr Lwxw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:dkim-signature; bh=Sx5o5aPm7ymYyPfKNR1Xg0WbG0dD44QgA1MLgaIOszY=; fh=tYfim0CTlD8Etci9OwO8a6QKlIL8p/qXNYYMK+8ajGI=; b=bTZVWma2QLyWS/3mdK2nmkmmFyHcCETQqFJrj1lnFNeHKq+Fyo/cQ9L8qW/4NhRKMQ ugF8TrMUwo1s94QXep6f5e9gNgNWmhI+vMwi/9zkTVAcMh0UGBqXZflLyjBnPT0xVQib bNvwd97yJClpSPL0jpQumbA1xlJ9ovhZwshFC2izYFZsNXpeALjEqtfwFXEtBmLUOM/P 1bRJc8LVsq3GamPfd+szTLMToCKgYL82VnOOakPt1jfPwDQAHqH37II4NHbDS/vqiGrK AhUIOARwj5I20IPxMZ4OktllUYvAb96O6l1HMrzLt8iPtNbB2GAIT1FIfMs8t/kjkEvq V9EA==; darn=lists.freedesktop.org ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1778567771; x=1779172571; darn=lists.freedesktop.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Sx5o5aPm7ymYyPfKNR1Xg0WbG0dD44QgA1MLgaIOszY=; b=k+d+6sIEE1emr+x2tP7RV24LHvhEgJDFbemjcdma4bbzd/Z7b+NKqYG1wrbTefb9Ml PIv51s8IyBdGGjx4BhgHYvQoapojI44DtKRwFifm+pqTba9gCg2TG2BFgN8pm1PZBoKL a1RIDgRD4b4/VW09vs208YXTtfvHcJYYZMlXOKPIp0WFIA6aa+BN56AjtyAkwRZ1TzBH OIVkdzFE+pNyam8Y185cTpdrXeAiUn2V/bMnVP7uQleeYV3hca9DMJS93xqr0Ce9qaCi chtGk4LR9uIj2sgygsjO936k/iL8NDdHLq4oJ/bEP9kNw+g/y/666BUqhlR9OPuyRDpd u0hg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778567771; x=1779172571; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=Sx5o5aPm7ymYyPfKNR1Xg0WbG0dD44QgA1MLgaIOszY=; b=K4e2e5Os9nO1We1UVdBhandgR8VN4IOxTzBE08iIwdZBuhOC8So8IHHASxnYiRJ7LY wLZXov5UFcQVKrlU27Tlxvos92r8Ecl8+FeWtWMqKYR5dSD4ry/7ALnU2p5BmT0uq2+A wj8rQmWZoQs8Ae/JUs0CcWOweaMtlriw5MCJV/67YyYusqwwYOoJivwUd60hk8yH11eY /vTZ2L0SP0qbprYl2uXu3uXU2unznFT8Hh527qjUIJMLQA9GIV+lYLFQggH65Dzg6Eu6 Oz3zGrPG4+qJHtMCzcIYzUV6tiWIHWlJe48Nyl6jJPjjLriF9rbMGyEDOHeafdDexhAJ OR1Q== X-Forwarded-Encrypted: i=1; AFNElJ8WWfRC90TD7LvGMhq9ZyOlpolrF68tqg0MvowjG6856QGmn8tJKJad9i7qEp8mkGYgioCExbcPiXM=@lists.freedesktop.org X-Gm-Message-State: AOJu0YxtINvixLosKddDQsVEL+Yr4kZKmPDYOYPwBT42iV5IIkzvD7Iy qphsDsrL0KiTfpjg5Oadb5atj182jEjmfT2wvmFyH89AAP/r0jqifoF+92MYyLLn04sjlpbEDU+ gyeQABGZgavy7lBeBD2bZtZwzGhMeRw== X-Gm-Gg: Acq92OGAHFuIzBTbQSDf/WavfAGqHp/Jt/DSkuXN4kW16KAFiTwdQ7E71rxxB6dD2C1 qqQAA2y4/ZMk2GUOmmiYOV9aYqt56okfzXCC+wYjVGAgSSMMPFEtQ2B1xSJQ5TSdCdAVtXwMhB+ t1FgAWIEWGBqDNWjfDZ0mNd/hAXZEYzMUjFNsgC9RByx9Xj+Ce1mt/il5qGEnR87YpLXcJTlsrA VpaMgX3Q3/aObnfN3s8qiKodEUAqsSFtOMUchO+fDKQ/U/kjbJyZQG1puN/sYI/muR9Mz+rdhQu PsInqM47Tk3eYzBwgmsWymmsBaiJbtrZ+B188+WjFvb2sPfNsA== X-Received: by 2002:a17:906:f59a:b0:ba5:1970:2bb6 with SMTP id a640c23a62f3a-bd28f6967aemr81796366b.34.1778567770933; Mon, 11 May 2026 23:36:10 -0700 (PDT) MIME-Version: 1.0 References: <20260511170152.16957-1-mhun512@gmail.com> <3ee00ebc4ff5bcf2a8754466f8d9d04b24f81858.camel@iscas.ac.cn> In-Reply-To: From: Myeonghun Pak Date: Tue, 12 May 2026 15:35:59 +0900 X-Gm-Features: AVHnY4JGcr-utocqGbdFL5KVpAjH4SDyn-A0dH0db8ax19oFwBDBVe8qe_RB83U Message-ID: Subject: Re: [PATCH] drm/loongson: clean up KMS polling on probe failure To: Thomas Zimmermann Cc: Icenowy Zheng , dri-devel@lists.freedesktop.org, Sui Jingfeng , Jianmin Lv , Qianhai Wu , Huacai Chen , Mingcong Bai , Xi Ruoyao , Maarten Lankhorst , Maxime Ripard , David Airlie , Simona Vetter , linux-kernel@vger.kernel.org, stable@vger.kernel.org, Ijae Kim Content-Type: multipart/alternative; boundary="00000000000032102406519914d0" X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" --00000000000032102406519914d0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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=EB=85=84 5=EC=9B=94 12=EC=9D=BC (=ED=99=94) =EC=98=A4=ED=9B=84 3:26, T= homas Zimmermann =EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84=B1= : > Hi > > Am 11.05.26 um 19:22 schrieb Icenowy Zheng: > > =E5=9C=A8 2026-05-12=E4=BA=8C=E7=9A=84 02:01 +0900=EF=BC=8CMyeonghun Pa= k=E5=86=99=E9=81=93=EF=BC=9A > >> 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 > >> Signed-off-by: Ijae Kim > >> Signed-off-by: Myeonghun Pak > >> --- > >> 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 =3D drm_vblank_init(ddev, descp->num_of_crtc); > >> if (ret) > >> - return ret; > >> + goto err_poll_fini; > >> > >> ret =3D 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 =3D 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 =3D 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=C3=BCrnberg, Germany, www.suse.com > GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG > N=C3=BCrnberg) > > > --00000000000032102406519914d0 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Icenowy, Thomas,

Thanks for the review.

I= agree that using drmm_kms_helper_poll_init() is the better fix here. It ke= eps the polling cleanup tied to the DRM device lifetime and avoids adding m= anual unwind and remove-path cleanup.

I sent a v2 that switches lsdc= _pci_probe() to drmm_kms_helper_poll_init().

Thanks,
Myeonghun


2026=EB=85=84 5=EC=9B=94 12=EC=9D=BC (=ED=99=94)= =EC=98=A4=ED=9B=84 3:26, Thomas Zimmermann <tzimmermann@suse.de>=EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84= =B1:
Hi

Am 11.05.26 um 19:22 schrieb Icenowy Zheng:
> =E5=9C=A8 2026-05-12=E4=BA=8C=E7=9A=84 02:01 +0900=EF=BC=8CMyeonghun P= ak=E5=86=99=E9=81=93=EF=BC=9A
>> lsdc_pci_probe() initializes KMS polling before setting up vblank<= br> >> support,
>> requesting the IRQ and registering the DRM device. If any of those=
>> later
>> steps fails, probe returns without finalizing polling. The remove<= br> >> path has
>> the same lifetime gap when tearing down a successfully registered<= br> >> 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<= br> > exists (while rarely used).
>
> Maybe it's better to switch to this? Or maybe there's some rea= son 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 resea= rch
>> while
>> reviewing kernel code.
>>
>> Fixes: f39db26c5428 ("drm: Add kms driver for loongson displa= y
>> controller")
>> Cc: st= able@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>
>> ---
>>=C2=A0 =C2=A0drivers/gpu/drm/loongson/lsdc_drv.c | 11 ++++++++--- >>=C2=A0 =C2=A01 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)
>>=C2=A0 =C2=A0=C2=A0 =C2=A0if (loongson_vblank) {
>>=C2=A0 =C2=A0=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ret =3D drm_v= blank_init(ddev, descp->num_of_crtc);
>>=C2=A0 =C2=A0=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (ret)
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 return ret;
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 goto err_poll_fini;
>>=C2=A0 =C2=A0
>>=C2=A0 =C2=A0=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ret =3D devm_= request_irq(&pdev->dev, pdev->irq,
>>=C2=A0 =C2=A0=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 descp->funcs->irq_handler,
>> @@ -305,7 +305,7 @@ static int lsdc_pci_probe(struct pci_dev *pdev= ,
>> const struct pci_device_id *ent)
>>=C2=A0 =C2=A0=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 dev_name(&pdev->dev), ddev);
>>=C2=A0 =C2=A0=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (ret) { >>=C2=A0 =C2=A0=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0drm_err(ddev, "Failed to register interrupt:
>> %d\n", ret);
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 return ret;
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 goto err_poll_fini;
>>=C2=A0 =C2=A0=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>>=C2=A0 =C2=A0
>>=C2=A0 =C2=A0=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0drm_info(ddev= , "registered irq: %u\n", pdev->irq);
>> @@ -313,17 +313,22 @@ static int lsdc_pci_probe(struct pci_dev *pd= ev,
>> const struct pci_device_id *ent)
>>=C2=A0 =C2=A0
>>=C2=A0 =C2=A0=C2=A0 =C2=A0ret =3D drm_dev_register(ddev, 0);
>>=C2=A0 =C2=A0=C2=A0 =C2=A0if (ret)
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return ret;
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 goto err_poll_fini;
>>=C2=A0 =C2=A0
>>=C2=A0 =C2=A0=C2=A0 =C2=A0drm_client_setup(ddev, NULL);
>>=C2=A0 =C2=A0
>>=C2=A0 =C2=A0=C2=A0 =C2=A0return 0;
>> +
>> +err_poll_fini:
>> +=C2=A0 =C2=A0 drm_kms_helper_poll_fini(ddev);
>> +=C2=A0 =C2=A0 return ret;
>>=C2=A0 =C2=A0}
>>=C2=A0 =C2=A0
>>=C2=A0 =C2=A0static void lsdc_pci_remove(struct pci_dev *pdev)
>>=C2=A0 =C2=A0{
>>=C2=A0 =C2=A0=C2=A0 =C2=A0struct drm_device *ddev =3D pci_get_drvda= ta(pdev);
>>=C2=A0 =C2=A0
>> +=C2=A0 =C2=A0 drm_kms_helper_poll_fini(ddev);
>>=C2=A0 =C2=A0=C2=A0 =C2=A0drm_dev_unregister(ddev);
>>=C2=A0 =C2=A0=C2=A0 =C2=A0drm_atomic_helper_shutdown(ddev);
>>=C2=A0 =C2=A0}

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 N=C3=BCrnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG N=C3=BCr= nberg)


--00000000000032102406519914d0--