[PATCH 3/3] drm/exynos: move Exynos platform drivers registration to init
Inki Dae
inki.dae at samsung.com
Sun Nov 23 23:57:22 PST 2014
On 2014년 11월 21일 08:42, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan at 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 at collabora.co.uk>
> Signed-off-by: Gustavo Padovan <gustavo.padovan at 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();
>
More information about the dri-devel
mailing list