[2/3] drm/bridge: analogix_dp: Postpone enabling runtime power management

Message ID 20180305085741.18896-3-m.szyprowski@samsung.com
State New
Headers show
Series
  • Analogix DP fixes for Chromebook2 Peach-Pit (Exynos5420)
Related show

Commit Message

Marek Szyprowski March 5, 2018, 8:57 a.m.
Enabling runtime power management early in analogix_dp_bind() causes following
kernel NULL pointer dereference:

Unable to handle kernel NULL pointer dereference at virtual address 000007d8
pgd = 28ffa2e4
[000007d8] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 6 PID: 69 Comm: kworker/6:1 Not tainted 4.16.0-rc1-00062-ge25751974ba8 #3622
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
Workqueue: events deferred_probe_work_func
PC is at analogix_dp_resume+0x8/0xc0
LR is at pm_generic_runtime_resume+0x2c/0x38
pc : [<c0531b98>]    lr : [<c0543fec>] psr: a0000113
sp : ee13fbd8  ip : 0000001a  fp : 00000001
r10: ee0eb080  r9 : c0552bd8  r8 : c0fb1d98
r7 : eebb1010  r6 : eeae9808  r5 : 00000000  r4 : d4850415
r3 : ee0ed010  r2 : b2d05e00  r1 : 00000000  r0 : 00000000
Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM Segment none
Control: 10c5387d  Table: 2000406a  DAC: 00000051
Process kworker/6:1 (pid: 69, stack limit = 0x913205b4)
Stack: (0xee13fbd8 to 0xee140000)
...
[<c0531b98>] (analogix_dp_resume) from [<c0543fec>] (pm_generic_runtime_resume+0x2c/0x38)
[<c0543fec>] (pm_generic_runtime_resume) from [<c054ffb4>] (__genpd_runtime_resume+0x2c/0x8c)
[<c054ffb4>] (__genpd_runtime_resume) from [<c0552d24>] (genpd_runtime_resume+0x14c/0x258)
[<c0552d24>] (genpd_runtime_resume) from [<c0547798>] (__rpm_callback+0x134/0x214)
[<c0547798>] (__rpm_callback) from [<c0547898>] (rpm_callback+0x20/0x80)
[<c0547898>] (rpm_callback) from [<c0546ff4>] (rpm_resume+0x3a0/0x734)
[<c0546ff4>] (rpm_resume) from [<c05475ec>] (__pm_runtime_resume+0x64/0x9c)
[<c05475ec>] (__pm_runtime_resume) from [<c053b95c>] (__device_attach+0x8c/0x134)
[<c053b95c>] (__device_attach) from [<c053ad08>] (bus_probe_device+0x88/0x90)
[<c053ad08>] (bus_probe_device) from [<c05390d0>] (device_add+0x3a8/0x580)
[<c05390d0>] (device_add) from [<c06764c4>] (i2c_register_adapter+0xd4/0x3ec)
[<c06764c4>] (i2c_register_adapter) from [<c05321c8>] (analogix_dp_bind+0x2a0/0x410)
[<c05321c8>] (analogix_dp_bind) from [<c0528e90>] (exynos_dp_bind+0x9c/0x12c)
[<c0528e90>] (exynos_dp_bind) from [<c0535bc4>] (component_bind_all+0xfc/0x258)
[<c0535bc4>] (component_bind_all) from [<c0522ee8>] (exynos_drm_bind+0x15c/0x28c)
[<c0522ee8>] (exynos_drm_bind) from [<c0536378>] (try_to_bring_up_master+0x1b8/0x29c)
[<c0536378>] (try_to_bring_up_master) from [<c05364fc>] (component_add+0xa0/0x170)
[<c05364fc>] (component_add) from [<c0528fe4>] (exynos_dp_probe+0x64/0xb8)
[<c0528fe4>] (exynos_dp_probe) from [<c053debc>] (platform_drv_probe+0x50/0xb0)
[<c053debc>] (platform_drv_probe) from [<c053bd18>] (driver_probe_device+0x2b8/0x4a0)
[<c053bd18>] (driver_probe_device) from [<c0539e4c>] (bus_for_each_drv+0x44/0x8c)
[<c0539e4c>] (bus_for_each_drv) from [<c053b970>] (__device_attach+0xa0/0x134)
[<c053b970>] (__device_attach) from [<c053ad08>] (bus_probe_device+0x88/0x90)
[<c053ad08>] (bus_probe_device) from [<c053b258>] (deferred_probe_work_func+0x3c/0x168)
[<c053b258>] (deferred_probe_work_func) from [<c014352c>] (process_one_work+0x1d0/0x7bc)
[<c014352c>] (process_one_work) from [<c0143b84>] (worker_thread+0x34/0x4dc)
[<c0143b84>] (worker_thread) from [<c014a30c>] (kthread+0x128/0x164)
[<c014a30c>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20)
Exception stack(0xee13ffb0 to 0xee13fff8)
ffa0:                                     00000000 00000000 00000000 00000000
ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
ffe0: 00000000 00000000 00000000 00000000 00000013 00000000
Code: e2800e37 eafee601 e92d4070 e1a05000 (e59067d8)
---[ end trace bf6046013df7cab2 ]---

This oops happens, because analogix_dp_bind() calls drm_dp_aux_register()
which registers i2c adapter. I2C core tries to runtime get i2c host
device during registration. This ends in analogix_dp_resume(), but dp
context is NULL there. dp context is set in exynos_dp_bind() after
executing analogix_dp_bind(). Fix this issue by postponing enabling runtime
power management after drm_dp_aux_register().

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Heiko Stuebner March 5, 2018, 12:44 p.m. | #1
Am Montag, 5. März 2018, 09:57:40 CET schrieb Marek Szyprowski:
> Enabling runtime power management early in analogix_dp_bind() causes

> following kernel NULL pointer dereference:

> 

> Unable to handle kernel NULL pointer dereference at virtual address 000007d8

> pgd = 28ffa2e4

> [000007d8] *pgd=00000000

> Internal error: Oops: 5 [#1] PREEMPT SMP ARM

> Modules linked in:

> CPU: 6 PID: 69 Comm: kworker/6:1 Not tainted 4.16.0-rc1-00062-ge25751974ba8

> #3622 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)

> Workqueue: events deferred_probe_work_func

> PC is at analogix_dp_resume+0x8/0xc0

> LR is at pm_generic_runtime_resume+0x2c/0x38

> pc : [<c0531b98>]    lr : [<c0543fec>] psr: a0000113

> sp : ee13fbd8  ip : 0000001a  fp : 00000001

> r10: ee0eb080  r9 : c0552bd8  r8 : c0fb1d98

> r7 : eebb1010  r6 : eeae9808  r5 : 00000000  r4 : d4850415

> r3 : ee0ed010  r2 : b2d05e00  r1 : 00000000  r0 : 00000000

> Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM Segment none

> Control: 10c5387d  Table: 2000406a  DAC: 00000051

> Process kworker/6:1 (pid: 69, stack limit = 0x913205b4)

> Stack: (0xee13fbd8 to 0xee140000)

> ...

> [<c0531b98>] (analogix_dp_resume) from [<c0543fec>]

> (pm_generic_runtime_resume+0x2c/0x38) [<c0543fec>]

> (pm_generic_runtime_resume) from [<c054ffb4>]

> (__genpd_runtime_resume+0x2c/0x8c) [<c054ffb4>] (__genpd_runtime_resume)

> from [<c0552d24>] (genpd_runtime_resume+0x14c/0x258) [<c0552d24>]

> (genpd_runtime_resume) from [<c0547798>] (__rpm_callback+0x134/0x214)

> [<c0547798>] (__rpm_callback) from [<c0547898>] (rpm_callback+0x20/0x80)

> [<c0547898>] (rpm_callback) from [<c0546ff4>] (rpm_resume+0x3a0/0x734)

> [<c0546ff4>] (rpm_resume) from [<c05475ec>] (__pm_runtime_resume+0x64/0x9c)

> [<c05475ec>] (__pm_runtime_resume) from [<c053b95c>]

> (__device_attach+0x8c/0x134) [<c053b95c>] (__device_attach) from

> [<c053ad08>] (bus_probe_device+0x88/0x90) [<c053ad08>] (bus_probe_device)

> from [<c05390d0>] (device_add+0x3a8/0x580) [<c05390d0>] (device_add) from

> [<c06764c4>] (i2c_register_adapter+0xd4/0x3ec) [<c06764c4>]

> (i2c_register_adapter) from [<c05321c8>] (analogix_dp_bind+0x2a0/0x410)

> [<c05321c8>] (analogix_dp_bind) from [<c0528e90>]

> (exynos_dp_bind+0x9c/0x12c) [<c0528e90>] (exynos_dp_bind) from [<c0535bc4>]

> (component_bind_all+0xfc/0x258) [<c0535bc4>] (component_bind_all) from

> [<c0522ee8>] (exynos_drm_bind+0x15c/0x28c) [<c0522ee8>] (exynos_drm_bind)

> from [<c0536378>] (try_to_bring_up_master+0x1b8/0x29c) [<c0536378>]

> (try_to_bring_up_master) from [<c05364fc>] (component_add+0xa0/0x170)

> [<c05364fc>] (component_add) from [<c0528fe4>] (exynos_dp_probe+0x64/0xb8)

> [<c0528fe4>] (exynos_dp_probe) from [<c053debc>]

> (platform_drv_probe+0x50/0xb0) [<c053debc>] (platform_drv_probe) from

> [<c053bd18>] (driver_probe_device+0x2b8/0x4a0) [<c053bd18>]

> (driver_probe_device) from [<c0539e4c>] (bus_for_each_drv+0x44/0x8c)

> [<c0539e4c>] (bus_for_each_drv) from [<c053b970>]

> (__device_attach+0xa0/0x134) [<c053b970>] (__device_attach) from

> [<c053ad08>] (bus_probe_device+0x88/0x90) [<c053ad08>] (bus_probe_device)

> from [<c053b258>] (deferred_probe_work_func+0x3c/0x168) [<c053b258>]

> (deferred_probe_work_func) from [<c014352c>] (process_one_work+0x1d0/0x7bc)

> [<c014352c>] (process_one_work) from [<c0143b84>]

> (worker_thread+0x34/0x4dc) [<c0143b84>] (worker_thread) from [<c014a30c>]

> (kthread+0x128/0x164) [<c014a30c>] (kthread) from [<c01010b4>]

> (ret_from_fork+0x14/0x20) Exception stack(0xee13ffb0 to 0xee13fff8)

> ffa0:                                     00000000 00000000 00000000

> 00000000 ffc0: 00000000 00000000 00000000 00000000 00000000 00000000

> 00000000 00000000 ffe0: 00000000 00000000 00000000 00000000 00000013

> 00000000

> Code: e2800e37 eafee601 e92d4070 e1a05000 (e59067d8)

> ---[ end trace bf6046013df7cab2 ]---

> 

> This oops happens, because analogix_dp_bind() calls drm_dp_aux_register()

> which registers i2c adapter. I2C core tries to runtime get i2c host

> device during registration. This ends in analogix_dp_resume(), but dp

> context is NULL there. dp context is set in exynos_dp_bind() after

> executing analogix_dp_bind(). Fix this issue by postponing enabling runtime

> power management after drm_dp_aux_register().

> 

> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>


on top of current drm-misc-next on rk3288 and rk3399
Tested-by: Heiko Stuebner <heiko@sntech.de>


and the actual change also looks sane of course, so
Reviewed-by: Heiko Stuebner <heiko@sntech.de>

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 8475749baae5..b391d149db91 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1376,8 +1376,6 @@  analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
 		return ERR_PTR(-ENODEV);
 	}
 
-	pm_runtime_enable(dev);
-
 	ret = devm_request_threaded_irq(&pdev->dev, dp->irq,
 					analogix_dp_hardirq,
 					analogix_dp_irq_thread,
@@ -1397,7 +1395,9 @@  analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
 
 	ret = drm_dp_aux_register(&dp->aux);
 	if (ret)
-		goto err_disable_pm_runtime;
+		return ERR_PTR(ret);
+
+	pm_runtime_enable(dev);
 
 	ret = analogix_dp_create_bridge(drm_dev, dp);
 	if (ret) {