| Message ID | 1483091866-1088-1-git-send-email-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Fri, Dec 30, 2016 at 4:57 AM, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > Analogix_dp_bind() can be called from component framework, which doesn't > guarantee proper runtime PM state of the device during bind operation, > so ensure that device is runtime active before doing any register access. > This ensures that the power domain, to which DP module belongs, is turned > on. While at it, also fix the unbalanced call to phy_power_on() in > analogix_dp_bind() function. > > This patch solves the following kernel oops on Samsung Exynos5250 Snow > board: > > Unhandled fault: imprecise external abort (0x406) at 0x00000000 > pgd = c0004000 > [00000000] *pgd=00000000 > Internal error: : 406 [#1] PREEMPT SMP ARM > Modules linked in: > CPU: 0 PID: 75 Comm: kworker/0:2 Not tainted 4.9.0 #1046 > Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) > Workqueue: events deferred_probe_work_func > task: ee272300 task.stack: ee312000 > PC is at analogix_dp_enable_sw_function+0x18/0x2c > LR is at analogix_dp_init_dp+0x2c/0x50 > ... > [<c03fcb38>] (analogix_dp_enable_sw_function) from [<c03fa9c4>] (analogix_dp_init_dp+0x2c/0x50) > [<c03fa9c4>] (analogix_dp_init_dp) from [<c03fab6c>] (analogix_dp_bind+0x184/0x42c) > [<c03fab6c>] (analogix_dp_bind) from [<c03fdb84>] (component_bind_all+0xf0/0x218) > [<c03fdb84>] (component_bind_all) from [<c03ed64c>] (exynos_drm_load+0x134/0x200) > [<c03ed64c>] (exynos_drm_load) from [<c03d5058>] (drm_dev_register+0xa0/0xd0) > [<c03d5058>] (drm_dev_register) from [<c03d66b8>] (drm_platform_init+0x58/0xb0) > [<c03d66b8>] (drm_platform_init) from [<c03fe0c4>] (try_to_bring_up_master+0x14c/0x188) > [<c03fe0c4>] (try_to_bring_up_master) from [<c03fe188>] (component_add+0x88/0x138) > [<c03fe188>] (component_add) from [<c0403a38>] (platform_drv_probe+0x50/0xb0) > [<c0403a38>] (platform_drv_probe) from [<c0402470>] (driver_probe_device+0x1f0/0x2a8) > [<c0402470>] (driver_probe_device) from [<c0400a54>] (bus_for_each_drv+0x44/0x8c) > [<c0400a54>] (bus_for_each_drv) from [<c04021f8>] (__device_attach+0x9c/0x100) > [<c04021f8>] (__device_attach) from [<c04018e8>] (bus_probe_device+0x84/0x8c) > [<c04018e8>] (bus_probe_device) from [<c0401d1c>] (deferred_probe_work_func+0x60/0x8c) > [<c0401d1c>] (deferred_probe_work_func) from [<c012fc14>] (process_one_work+0x120/0x318) > [<c012fc14>] (process_one_work) from [<c012fe34>] (process_scheduled_works+0x28/0x38) > [<c012fe34>] (process_scheduled_works) from [<c0130048>] (worker_thread+0x204/0x4ac) > [<c0130048>] (worker_thread) from [<c01352c4>] (kthread+0xd8/0xf4) > [<c01352c4>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c) > Code: e59035f0 e5935018 f57ff04f e3c55001 (f57ff04e) > ---[ end trace 3d1d0d87796de344 ]--- > Reviewed-by: Sean Paul <seanpaul@chromium.org> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > index eb9bf8786c24..18eefdcbf1ba 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > @@ -1382,6 +1382,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, > > pm_runtime_enable(dev); > > + pm_runtime_get_sync(dev); > phy_power_on(dp->phy); > > analogix_dp_init_dp(dp); > @@ -1414,9 +1415,15 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, > goto err_disable_pm_runtime; > } > > + phy_power_off(dp->phy); > + pm_runtime_put(dev); > + > return 0; > > err_disable_pm_runtime: > + > + phy_power_off(dp->phy); > + pm_runtime_put(dev); > pm_runtime_disable(dev); > > return ret; > -- > 1.9.1 >
On 01/05/2017 01:01 PM, Sean Paul wrote: > On Fri, Dec 30, 2016 at 4:57 AM, Marek Szyprowski > <m.szyprowski@samsung.com> wrote: >> Analogix_dp_bind() can be called from component framework, which doesn't >> guarantee proper runtime PM state of the device during bind operation, >> so ensure that device is runtime active before doing any register access. >> This ensures that the power domain, to which DP module belongs, is turned >> on. While at it, also fix the unbalanced call to phy_power_on() in >> analogix_dp_bind() function. Pushed to drm-misc-fixes. Thanks, Archit >> >> This patch solves the following kernel oops on Samsung Exynos5250 Snow >> board: >> >> Unhandled fault: imprecise external abort (0x406) at 0x00000000 >> pgd = c0004000 >> [00000000] *pgd=00000000 >> Internal error: : 406 [#1] PREEMPT SMP ARM >> Modules linked in: >> CPU: 0 PID: 75 Comm: kworker/0:2 Not tainted 4.9.0 #1046 >> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >> Workqueue: events deferred_probe_work_func >> task: ee272300 task.stack: ee312000 >> PC is at analogix_dp_enable_sw_function+0x18/0x2c >> LR is at analogix_dp_init_dp+0x2c/0x50 >> ... >> [<c03fcb38>] (analogix_dp_enable_sw_function) from [<c03fa9c4>] (analogix_dp_init_dp+0x2c/0x50) >> [<c03fa9c4>] (analogix_dp_init_dp) from [<c03fab6c>] (analogix_dp_bind+0x184/0x42c) >> [<c03fab6c>] (analogix_dp_bind) from [<c03fdb84>] (component_bind_all+0xf0/0x218) >> [<c03fdb84>] (component_bind_all) from [<c03ed64c>] (exynos_drm_load+0x134/0x200) >> [<c03ed64c>] (exynos_drm_load) from [<c03d5058>] (drm_dev_register+0xa0/0xd0) >> [<c03d5058>] (drm_dev_register) from [<c03d66b8>] (drm_platform_init+0x58/0xb0) >> [<c03d66b8>] (drm_platform_init) from [<c03fe0c4>] (try_to_bring_up_master+0x14c/0x188) >> [<c03fe0c4>] (try_to_bring_up_master) from [<c03fe188>] (component_add+0x88/0x138) >> [<c03fe188>] (component_add) from [<c0403a38>] (platform_drv_probe+0x50/0xb0) >> [<c0403a38>] (platform_drv_probe) from [<c0402470>] (driver_probe_device+0x1f0/0x2a8) >> [<c0402470>] (driver_probe_device) from [<c0400a54>] (bus_for_each_drv+0x44/0x8c) >> [<c0400a54>] (bus_for_each_drv) from [<c04021f8>] (__device_attach+0x9c/0x100) >> [<c04021f8>] (__device_attach) from [<c04018e8>] (bus_probe_device+0x84/0x8c) >> [<c04018e8>] (bus_probe_device) from [<c0401d1c>] (deferred_probe_work_func+0x60/0x8c) >> [<c0401d1c>] (deferred_probe_work_func) from [<c012fc14>] (process_one_work+0x120/0x318) >> [<c012fc14>] (process_one_work) from [<c012fe34>] (process_scheduled_works+0x28/0x38) >> [<c012fe34>] (process_scheduled_works) from [<c0130048>] (worker_thread+0x204/0x4ac) >> [<c0130048>] (worker_thread) from [<c01352c4>] (kthread+0xd8/0xf4) >> [<c01352c4>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c) >> Code: e59035f0 e5935018 f57ff04f e3c55001 (f57ff04e) >> ---[ end trace 3d1d0d87796de344 ]--- >> > > Reviewed-by: Sean Paul <seanpaul@chromium.org> > >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c >> index eb9bf8786c24..18eefdcbf1ba 100644 >> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c >> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c >> @@ -1382,6 +1382,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, >> >> pm_runtime_enable(dev); >> >> + pm_runtime_get_sync(dev); >> phy_power_on(dp->phy); >> >> analogix_dp_init_dp(dp); >> @@ -1414,9 +1415,15 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, >> goto err_disable_pm_runtime; >> } >> >> + phy_power_off(dp->phy); >> + pm_runtime_put(dev); >> + >> return 0; >> >> err_disable_pm_runtime: >> + >> + phy_power_off(dp->phy); >> + pm_runtime_put(dev); >> pm_runtime_disable(dev); >> >> return ret; >> -- >> 1.9.1 >> > > >
Hello Archit, On 01/09/2017 06:47 AM, Archit Taneja wrote: > > > On 01/05/2017 01:01 PM, Sean Paul wrote: >> On Fri, Dec 30, 2016 at 4:57 AM, Marek Szyprowski >> <m.szyprowski@samsung.com> wrote: >>> Analogix_dp_bind() can be called from component framework, which doesn't >>> guarantee proper runtime PM state of the device during bind operation, >>> so ensure that device is runtime active before doing any register access. >>> This ensures that the power domain, to which DP module belongs, is turned >>> on. While at it, also fix the unbalanced call to phy_power_on() in >>> analogix_dp_bind() function. > > Pushed to drm-misc-fixes. > Do you know when this fix will land in mainline? The regression is still present in v4.10-rc5: https://storage.kernelci.org/mainline/v4.10-rc5/arm-exynos_defconfig/lab-collabora/boot-exynos5250-snow.html > Thanks, > Archit > Best regards,
Hi, On 01/23/2017 10:58 PM, Javier Martinez Canillas wrote: > Hello Archit, > > On 01/09/2017 06:47 AM, Archit Taneja wrote: >> >> >> On 01/05/2017 01:01 PM, Sean Paul wrote: >>> On Fri, Dec 30, 2016 at 4:57 AM, Marek Szyprowski >>> <m.szyprowski@samsung.com> wrote: >>>> Analogix_dp_bind() can be called from component framework, which doesn't >>>> guarantee proper runtime PM state of the device during bind operation, >>>> so ensure that device is runtime active before doing any register access. >>>> This ensures that the power domain, to which DP module belongs, is turned >>>> on. While at it, also fix the unbalanced call to phy_power_on() in >>>> analogix_dp_bind() function. >> >> Pushed to drm-misc-fixes. >> > > Do you know when this fix will land in mainline? The regression is still > present in v4.10-rc5: > > https://storage.kernelci.org/mainline/v4.10-rc5/arm-exynos_defconfig/lab-collabora/boot-exynos5250-snow.html It's there in Dave's 4.10-rc6 pull request: https://lkml.org/lkml/2017/1/22/305 Archit
Hello Archit, On 01/23/2017 11:40 PM, Archit Taneja wrote: > Hi, > > On 01/23/2017 10:58 PM, Javier Martinez Canillas wrote: >> Hello Archit, >> >> On 01/09/2017 06:47 AM, Archit Taneja wrote: >>> >>> >>> On 01/05/2017 01:01 PM, Sean Paul wrote: >>>> On Fri, Dec 30, 2016 at 4:57 AM, Marek Szyprowski >>>> <m.szyprowski@samsung.com> wrote: >>>>> Analogix_dp_bind() can be called from component framework, which doesn't >>>>> guarantee proper runtime PM state of the device during bind operation, >>>>> so ensure that device is runtime active before doing any register access. >>>>> This ensures that the power domain, to which DP module belongs, is turned >>>>> on. While at it, also fix the unbalanced call to phy_power_on() in >>>>> analogix_dp_bind() function. >>> >>> Pushed to drm-misc-fixes. >>> >> >> Do you know when this fix will land in mainline? The regression is still >> present in v4.10-rc5: >> >> https://storage.kernelci.org/mainline/v4.10-rc5/arm-exynos_defconfig/lab-collabora/boot-exynos5250-snow.html > > It's there in Dave's 4.10-rc6 pull request: > > https://lkml.org/lkml/2017/1/22/305 Thanks a lot for the information. > Archit > Best regards,
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index eb9bf8786c24..18eefdcbf1ba 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1382,6 +1382,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, pm_runtime_enable(dev); + pm_runtime_get_sync(dev); phy_power_on(dp->phy); analogix_dp_init_dp(dp); @@ -1414,9 +1415,15 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, goto err_disable_pm_runtime; } + phy_power_off(dp->phy); + pm_runtime_put(dev); + return 0; err_disable_pm_runtime: + + phy_power_off(dp->phy); + pm_runtime_put(dev); pm_runtime_disable(dev); return ret;
Analogix_dp_bind() can be called from component framework, which doesn't guarantee proper runtime PM state of the device during bind operation, so ensure that device is runtime active before doing any register access. This ensures that the power domain, to which DP module belongs, is turned on. While at it, also fix the unbalanced call to phy_power_on() in analogix_dp_bind() function. This patch solves the following kernel oops on Samsung Exynos5250 Snow board: Unhandled fault: imprecise external abort (0x406) at 0x00000000 pgd = c0004000 [00000000] *pgd=00000000 Internal error: : 406 [#1] PREEMPT SMP ARM Modules linked in: CPU: 0 PID: 75 Comm: kworker/0:2 Not tainted 4.9.0 #1046 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) Workqueue: events deferred_probe_work_func task: ee272300 task.stack: ee312000 PC is at analogix_dp_enable_sw_function+0x18/0x2c LR is at analogix_dp_init_dp+0x2c/0x50 ... [<c03fcb38>] (analogix_dp_enable_sw_function) from [<c03fa9c4>] (analogix_dp_init_dp+0x2c/0x50) [<c03fa9c4>] (analogix_dp_init_dp) from [<c03fab6c>] (analogix_dp_bind+0x184/0x42c) [<c03fab6c>] (analogix_dp_bind) from [<c03fdb84>] (component_bind_all+0xf0/0x218) [<c03fdb84>] (component_bind_all) from [<c03ed64c>] (exynos_drm_load+0x134/0x200) [<c03ed64c>] (exynos_drm_load) from [<c03d5058>] (drm_dev_register+0xa0/0xd0) [<c03d5058>] (drm_dev_register) from [<c03d66b8>] (drm_platform_init+0x58/0xb0) [<c03d66b8>] (drm_platform_init) from [<c03fe0c4>] (try_to_bring_up_master+0x14c/0x188) [<c03fe0c4>] (try_to_bring_up_master) from [<c03fe188>] (component_add+0x88/0x138) [<c03fe188>] (component_add) from [<c0403a38>] (platform_drv_probe+0x50/0xb0) [<c0403a38>] (platform_drv_probe) from [<c0402470>] (driver_probe_device+0x1f0/0x2a8) [<c0402470>] (driver_probe_device) from [<c0400a54>] (bus_for_each_drv+0x44/0x8c) [<c0400a54>] (bus_for_each_drv) from [<c04021f8>] (__device_attach+0x9c/0x100) [<c04021f8>] (__device_attach) from [<c04018e8>] (bus_probe_device+0x84/0x8c) [<c04018e8>] (bus_probe_device) from [<c0401d1c>] (deferred_probe_work_func+0x60/0x8c) [<c0401d1c>] (deferred_probe_work_func) from [<c012fc14>] (process_one_work+0x120/0x318) [<c012fc14>] (process_one_work) from [<c012fe34>] (process_scheduled_works+0x28/0x38) [<c012fe34>] (process_scheduled_works) from [<c0130048>] (worker_thread+0x204/0x4ac) [<c0130048>] (worker_thread) from [<c01352c4>] (kthread+0xd8/0xf4) [<c01352c4>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c) Code: e59035f0 e5935018 f57ff04f e3c55001 (f57ff04e) ---[ end trace 3d1d0d87796de344 ]--- Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 7 +++++++ 1 file changed, 7 insertions(+)