On Tue, Oct 21, 2025 at 03:14:35PM +0200, Kory Maincent wrote: > On Tue, 14 Oct 2025 18:21:22 +0200 > Kory Maincent <[email protected]> wrote: > > > Hello Maxime, > > > > On Tue, 14 Oct 2025 17:36:47 +0200 > > Maxime Ripard <[email protected]> wrote: > > > > > On Tue, Oct 14, 2025 at 04:32:28PM +0200, Kory Maincent wrote: > > > > From: "Kory Maincent (TI.com)" <[email protected]> > > > > > > > > The drm_kms_helper_poll_fini() and drm_atomic_helper_shutdown() helpers > > > > should only be called when the device has been successfully registered. > > > > Currently, these functions are called unconditionally in tilcdc_fini(), > > > > which causes warnings during probe deferral scenarios. > > > > > > > > [ 7.972317] WARNING: CPU: 0 PID: 23 at > > > > drivers/gpu/drm/drm_atomic_state_helper.c:175 > > > > drm_atomic_helper_crtc_duplicate_state+0x60/0x68 ... [ 8.005820] > > > > drm_atomic_helper_crtc_duplicate_state from > > > > drm_atomic_get_crtc_state+0x68/0x108 [ 8.005858] > > > > drm_atomic_get_crtc_state from drm_atomic_helper_disable_all+0x90/0x1c8 > > > > [ > > > > 8.005885] drm_atomic_helper_disable_all from > > > > drm_atomic_helper_shutdown+0x90/0x144 [ 8.005911] > > > > drm_atomic_helper_shutdown from tilcdc_fini+0x68/0xf8 [tilcdc] [ > > > > 8.005957] tilcdc_fini [tilcdc] from tilcdc_pdev_probe+0xb0/0x6d4 > > > > [tilcdc] > > > > > > > > Fix this by moving both drm_kms_helper_poll_fini() and > > > > drm_atomic_helper_shutdown() inside the priv->is_registered conditional > > > > block, ensuring they only execute after successful device registration. > > > > > > > > Fixes: 3c4babae3c4a ("drm: Call drm_atomic_helper_shutdown() at > > > > shutdown/remove time for misc drivers") Signed-off-by: Kory Maincent > > > > (TI.com) <[email protected]> --- > > > > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 8 ++++---- > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c > > > > b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 7caec4d38ddf..2031267a3490 > > > > 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c > > > > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c > > > > @@ -172,11 +172,11 @@ static void tilcdc_fini(struct drm_device *dev) > > > > if (priv->crtc) > > > > tilcdc_crtc_shutdown(priv->crtc); > > > > > > > > - if (priv->is_registered) > > > > + if (priv->is_registered) { > > > > drm_dev_unregister(dev); > > > > - > > > > - drm_kms_helper_poll_fini(dev); > > > > - drm_atomic_helper_shutdown(dev); > > > > + drm_kms_helper_poll_fini(dev); > > > > + drm_atomic_helper_shutdown(dev); > > > > + } > > > > tilcdc_irq_uninstall(dev); > > > > drm_mode_config_cleanup(dev); > > > > > > I don't think that's the right fix. tilcdc_fini is pretty complex > > > because it gets called from multiple locations with various level of > > > initialisation. > > > > > > This is done because tilcdc_init is using a bunch of deprecated > > > functions with better alternatives now, and those would make the job of > > > tilcdc_fini much easier. > > > > > > That's what we should be focusing on. > > > > I am also currently focusing on improving this driver (which has indeed some > > weird code leftover), but this work will land in drm misc next while this > > is a > > fix for the current implementation which fix an unwanted warning. > > Maxime is it okay to merge this to the right drm fix branch as I am currently > working on the tilcdc cleaning process that will land into drm misc next. > > Also I intend to remove the tilcdc panel subdriver and its binding as it > can be replaced by the simple panel driver. I know it is unusual to remove a > binding but the driver and the binding are crappy and legacy. What do you > think?
I don't see why what I was suggesting also couldn't be fixes, but at the end of the day, I don't care which way it goes. Maxime
signature.asc
Description: PGP signature
