| Message ID | 1416526976-9467-4-git-send-email-gustavo@padovan.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2014? 11? 21? 08:42, Gustavo Padovan wrote: > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > Registering the Exynos DRM subdevices platform drivers in the probe > function is causing an infinite loop. Fix this by moving it to the > exynos_drm_init() function to register the drivers on module init. > > Registering drivers in the probe functions causes a deadlock in the parent > device lock. See Grant Likely explanation on the topic: > > "I think the problem is that exynos_drm_init() is registering a normal > (non-OF) platform device, so the parent will be /sys/devices/platform. > It immediately gets bound against exynos_drm_platform_driver which > calls the exynos drm_platform_probe() hook. The driver core obtains > device_lock() on the device *and on the device parent*. > > Inside the probe hook, additional platform_drivers get registered. > Each time one does, it tries to bind against every platform device in > the system, which includes the ones created by OF. When it attempts to > bind, it obtains device_lock() on the device *and on the device > parent*. > > Before the change to move of-generated platform devices into > /sys/devices/platform, the devices had different parents. Now both > devices have /sys/devices/platform as the parent, so yes they are > going to deadlock. > > The real problem is registering drivers from within a probe hook. That > is completely wrong for the above deadlock reason. __driver_attach() > will deadlock. Those registrations must be pulled out of .probe(). > > Registering devices in .probe() is okay because __device_attach() > doesn't try to obtain device_lock() on the parent." > > INFO: task swapper/0:1 blocked for more than 120 seconds. > Not tainted 3.18.0-rc3-next-20141105 #794 > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. > swapper/0 D c052534c 0 1 0 0x00000000 > [<c052534c>] (__schedule) from [<c0525b34>] (schedule_preempt_disabled+0x14/0x20) > [<c0525b34>] (schedule_preempt_disabled) from [<c0526d44>] (mutex_lock_nested+0x1c4/0x464 > > [<c0526d44>] (mutex_lock_nested) from [<c02be908>] (__driver_attach+0x48/0x98) > [<c02be908>] (__driver_attach) from [<c02bcc00>] (bus_for_each_dev+0x54/0x88) > [<c02bcc00>] (bus_for_each_dev) from [<c02bdce0>] (bus_add_driver+0xe4/0x200) > [<c02bdce0>] (bus_add_driver) from [<c02bef94>] (driver_register+0x78/0xf4) > [<c02bef94>] (driver_register) from [<c029e99c>] (exynos_drm_platform_probe+0x34/0x234) > [<c029e99c>] (exynos_drm_platform_probe) from [<c02bfcf0>] (platform_drv_probe+0x48/0xa4) > [<c02bfcf0>] (platform_drv_probe) from [<c02be680>] (driver_probe_device+0x13c/0x37c) > [<c02be680>] (driver_probe_device) from [<c02be954>] (__driver_attach+0x94/0x98) > [<c02be954>] (__driver_attach) from [<c02bcc00>] (bus_for_each_dev+0x54/0x88) > [<c02bcc00>] (bus_for_each_dev) from [<c02bdce0>] (bus_add_driver+0xe4/0x200) > [<c02bdce0>] (bus_add_driver) from [<c02bef94>] (driver_register+0x78/0xf4) > [<c02bef94>] (driver_register) from [<c029e938>] (exynos_drm_init+0x70/0xa0) > [<c029e938>] (exynos_drm_init) from [<c00089b0>] (do_one_initcall+0xac/0x1f0) > [<c00089b0>] (do_one_initcall) from [<c074bd90>] (kernel_init_freeable+0x10c/0x1d8) > [<c074bd90>] (kernel_init_freeable) from [<c051eabc>] (kernel_init+0x8/0xec) > [<c051eabc>] (kernel_init) from [<c000f268>] (ret_from_fork+0x14/0x2c) > 3 locks held by swapper/0/1: > #0: (&dev->mutex){......}, at: [<c02be908>] __driver_attach+0x48/0x98 > #1: (&dev->mutex){......}, at: [<c02be918>] __driver_attach+0x58/0x98 > #2: (&dev->mutex){......}, at: [<c02be908>] __driver_attach+0x48/0x98 > > Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > --- > drivers/gpu/drm/exynos/exynos_drm_drv.c | 124 +++++++++++++++----------------- > 1 file changed, 59 insertions(+), 65 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c > index 91891cf..cb3ed9b 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > @@ -548,6 +548,38 @@ static const struct component_master_ops exynos_drm_ops = { > .unbind = exynos_drm_unbind, > }; > > +static int exynos_drm_platform_probe(struct platform_device *pdev) > +{ > + struct component_match *match; > + > + pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32); > + exynos_drm_driver.num_ioctls = ARRAY_SIZE(exynos_ioctls); > + > + match = exynos_drm_match_add(&pdev->dev); > + if (IS_ERR(match)) { > + return PTR_ERR(match); > + } > + > + return component_master_add_with_match(&pdev->dev, &exynos_drm_ops, > + match); > +} > + > +static int exynos_drm_platform_remove(struct platform_device *pdev) > +{ > + component_master_del(&pdev->dev, &exynos_drm_ops); > + return 0; > +} > + > +static struct platform_driver exynos_drm_platform_driver = { > + .probe = exynos_drm_platform_probe, > + .remove = exynos_drm_platform_remove, > + .driver = { > + .owner = THIS_MODULE, > + .name = "exynos-drm", > + .pm = &exynos_drm_pm_ops, > + }, > +}; > + > static struct platform_driver *const exynos_drm_kms_drivers[] = { > #ifdef CONFIG_DRM_EXYNOS_FIMD > &fimd_driver, > @@ -582,13 +614,24 @@ static struct platform_driver *const exynos_drm_non_kms_drivers[] = { > #endif > }; > > -static int exynos_drm_platform_probe(struct platform_device *pdev) > +static int exynos_drm_init(void) > { > - struct component_match *match; > int ret, i, j; > > - pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32); > - exynos_drm_driver.num_ioctls = ARRAY_SIZE(exynos_ioctls); > + exynos_drm_pdev = platform_device_register_simple("exynos-drm", -1, > + NULL, 0); > + if (IS_ERR(exynos_drm_pdev)) > + return PTR_ERR(exynos_drm_pdev); > + > +#ifdef CONFIG_DRM_EXYNOS_VIDI > + ret = exynos_drm_probe_vidi(); > + if (ret < 0) > + goto err_unregister_pd; > +#endif If vidi driver is enabled then Exynos drm driver doesn't work. > + > + ret = platform_driver_register(&exynos_drm_platform_driver); > + if (ret) > + goto err_remove_vidi; Above platform_driver_register should be called after all kms and non-kms drivers are registered. And your patch should be re-based on top of exynos-drm-next. I just re-based it on top of exynos-drm-next and changed the platform_driver_register to be called at the end. Thanks, Inki Dae > > for (i = 0; i < ARRAY_SIZE(exynos_drm_kms_drivers); ++i) { > ret = platform_driver_register(exynos_drm_kms_drivers[i]); > @@ -596,26 +639,17 @@ static int exynos_drm_platform_probe(struct platform_device *pdev) > goto err_unregister_kms_drivers; > } > > - match = exynos_drm_match_add(&pdev->dev); > - if (IS_ERR(match)) { > - ret = PTR_ERR(match); > - goto err_unregister_kms_drivers; > - } > - > - ret = component_master_add_with_match(&pdev->dev, &exynos_drm_ops, > - match); > - if (ret < 0) > - goto err_unregister_kms_drivers; > - > for (j = 0; j < ARRAY_SIZE(exynos_drm_non_kms_drivers); ++j) { > ret = platform_driver_register(exynos_drm_non_kms_drivers[j]); > if (ret < 0) > - goto err_del_component_master; > + goto err_unregister_non_kms_drivers; > } > > +#ifdef CONFIG_DRM_EXYNOS_IPP > ret = exynos_platform_device_ipp_register(); > if (ret < 0) > goto err_unregister_non_kms_drivers; > +#endif > > return ret; > > @@ -626,17 +660,22 @@ err_unregister_non_kms_drivers: > while (--j >= 0) > platform_driver_unregister(exynos_drm_non_kms_drivers[j]); > > -err_del_component_master: > - component_master_del(&pdev->dev, &exynos_drm_ops); > - > err_unregister_kms_drivers: > while (--i >= 0) > platform_driver_unregister(exynos_drm_kms_drivers[i]); > > +err_remove_vidi: > +#ifdef CONFIG_DRM_EXYNOS_VIDI > + exynos_drm_remove_vidi(); > + > +err_unregister_pd: > +#endif > + platform_device_unregister(exynos_drm_pdev); > + > return ret; > } > > -static int exynos_drm_platform_remove(struct platform_device *pdev) > +static void exynos_drm_exit(void) > { > int i; > > @@ -647,54 +686,9 @@ static int exynos_drm_platform_remove(struct platform_device *pdev) > for (i = ARRAY_SIZE(exynos_drm_non_kms_drivers) - 1; i >= 0; --i) > platform_driver_unregister(exynos_drm_non_kms_drivers[i]); > > - component_master_del(&pdev->dev, &exynos_drm_ops); > - > for (i = ARRAY_SIZE(exynos_drm_kms_drivers) - 1; i >= 0; --i) > platform_driver_unregister(exynos_drm_kms_drivers[i]); > > - return 0; > -} > - > -static struct platform_driver exynos_drm_platform_driver = { > - .probe = exynos_drm_platform_probe, > - .remove = exynos_drm_platform_remove, > - .driver = { > - .owner = THIS_MODULE, > - .name = "exynos-drm", > - .pm = &exynos_drm_pm_ops, > - }, > -}; > - > -static int exynos_drm_init(void) > -{ > - int ret; > - > - exynos_drm_pdev = platform_device_register_simple("exynos-drm", -1, > - NULL, 0); > - if (IS_ERR(exynos_drm_pdev)) > - return PTR_ERR(exynos_drm_pdev); > - > - ret = exynos_drm_probe_vidi(); > - if (ret < 0) > - goto err_unregister_pd; > - > - ret = platform_driver_register(&exynos_drm_platform_driver); > - if (ret) > - goto err_remove_vidi; > - > - return 0; > - > -err_remove_vidi: > - exynos_drm_remove_vidi(); > - > -err_unregister_pd: > - platform_device_unregister(exynos_drm_pdev); > - > - return ret; > -} > - > -static void exynos_drm_exit(void) > -{ > platform_driver_unregister(&exynos_drm_platform_driver); > > exynos_drm_remove_vidi(); >
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index 91891cf..cb3ed9b 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -548,6 +548,38 @@ static const struct component_master_ops exynos_drm_ops = { .unbind = exynos_drm_unbind, }; +static int exynos_drm_platform_probe(struct platform_device *pdev) +{ + struct component_match *match; + + pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32); + exynos_drm_driver.num_ioctls = ARRAY_SIZE(exynos_ioctls); + + match = exynos_drm_match_add(&pdev->dev); + if (IS_ERR(match)) { + return PTR_ERR(match); + } + + return component_master_add_with_match(&pdev->dev, &exynos_drm_ops, + match); +} + +static int exynos_drm_platform_remove(struct platform_device *pdev) +{ + component_master_del(&pdev->dev, &exynos_drm_ops); + return 0; +} + +static struct platform_driver exynos_drm_platform_driver = { + .probe = exynos_drm_platform_probe, + .remove = exynos_drm_platform_remove, + .driver = { + .owner = THIS_MODULE, + .name = "exynos-drm", + .pm = &exynos_drm_pm_ops, + }, +}; + static struct platform_driver *const exynos_drm_kms_drivers[] = { #ifdef CONFIG_DRM_EXYNOS_FIMD &fimd_driver, @@ -582,13 +614,24 @@ static struct platform_driver *const exynos_drm_non_kms_drivers[] = { #endif }; -static int exynos_drm_platform_probe(struct platform_device *pdev) +static int exynos_drm_init(void) { - struct component_match *match; int ret, i, j; - pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32); - exynos_drm_driver.num_ioctls = ARRAY_SIZE(exynos_ioctls); + exynos_drm_pdev = platform_device_register_simple("exynos-drm", -1, + NULL, 0); + if (IS_ERR(exynos_drm_pdev)) + return PTR_ERR(exynos_drm_pdev); + +#ifdef CONFIG_DRM_EXYNOS_VIDI + ret = exynos_drm_probe_vidi(); + if (ret < 0) + goto err_unregister_pd; +#endif + + ret = platform_driver_register(&exynos_drm_platform_driver); + if (ret) + goto err_remove_vidi; for (i = 0; i < ARRAY_SIZE(exynos_drm_kms_drivers); ++i) { ret = platform_driver_register(exynos_drm_kms_drivers[i]); @@ -596,26 +639,17 @@ static int exynos_drm_platform_probe(struct platform_device *pdev) goto err_unregister_kms_drivers; } - match = exynos_drm_match_add(&pdev->dev); - if (IS_ERR(match)) { - ret = PTR_ERR(match); - goto err_unregister_kms_drivers; - } - - ret = component_master_add_with_match(&pdev->dev, &exynos_drm_ops, - match); - if (ret < 0) - goto err_unregister_kms_drivers; - for (j = 0; j < ARRAY_SIZE(exynos_drm_non_kms_drivers); ++j) { ret = platform_driver_register(exynos_drm_non_kms_drivers[j]); if (ret < 0) - goto err_del_component_master; + goto err_unregister_non_kms_drivers; } +#ifdef CONFIG_DRM_EXYNOS_IPP ret = exynos_platform_device_ipp_register(); if (ret < 0) goto err_unregister_non_kms_drivers; +#endif return ret; @@ -626,17 +660,22 @@ err_unregister_non_kms_drivers: while (--j >= 0) platform_driver_unregister(exynos_drm_non_kms_drivers[j]); -err_del_component_master: - component_master_del(&pdev->dev, &exynos_drm_ops); - err_unregister_kms_drivers: while (--i >= 0) platform_driver_unregister(exynos_drm_kms_drivers[i]); +err_remove_vidi: +#ifdef CONFIG_DRM_EXYNOS_VIDI + exynos_drm_remove_vidi(); + +err_unregister_pd: +#endif + platform_device_unregister(exynos_drm_pdev); + return ret; } -static int exynos_drm_platform_remove(struct platform_device *pdev) +static void exynos_drm_exit(void) { int i; @@ -647,54 +686,9 @@ static int exynos_drm_platform_remove(struct platform_device *pdev) for (i = ARRAY_SIZE(exynos_drm_non_kms_drivers) - 1; i >= 0; --i) platform_driver_unregister(exynos_drm_non_kms_drivers[i]); - component_master_del(&pdev->dev, &exynos_drm_ops); - for (i = ARRAY_SIZE(exynos_drm_kms_drivers) - 1; i >= 0; --i) platform_driver_unregister(exynos_drm_kms_drivers[i]); - return 0; -} - -static struct platform_driver exynos_drm_platform_driver = { - .probe = exynos_drm_platform_probe, - .remove = exynos_drm_platform_remove, - .driver = { - .owner = THIS_MODULE, - .name = "exynos-drm", - .pm = &exynos_drm_pm_ops, - }, -}; - -static int exynos_drm_init(void) -{ - int ret; - - exynos_drm_pdev = platform_device_register_simple("exynos-drm", -1, - NULL, 0); - if (IS_ERR(exynos_drm_pdev)) - return PTR_ERR(exynos_drm_pdev); - - ret = exynos_drm_probe_vidi(); - if (ret < 0) - goto err_unregister_pd; - - ret = platform_driver_register(&exynos_drm_platform_driver); - if (ret) - goto err_remove_vidi; - - return 0; - -err_remove_vidi: - exynos_drm_remove_vidi(); - -err_unregister_pd: - platform_device_unregister(exynos_drm_pdev); - - return ret; -} - -static void exynos_drm_exit(void) -{ platform_driver_unregister(&exynos_drm_platform_driver); exynos_drm_remove_vidi();