| Message ID | 20171121074936.22520-1-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/21/2017 01:19 PM, Marek Szyprowski wrote: > get_modes() callback might be called asynchronously from the DRM core and > it is not synchronized with bridge_enable(), which sets proper runtime PM > state of the main DP device. Fix this by calling pm_runtime_get_sync() > before calling drm_get_edid(), which in turn calls drm_dp_i2c_xfer() and > analogix_dp_transfer() to ensure that main DP device is runtime active > when doing any access to its registers. Looks good to me. Would be nice to get an ack from rockchip too. Will queue it to drm-misc-fixes (so that it's merged for 4.15-rc2) if no one has any objections. Thanks, Archit > > This fixes the following kernel issue 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: 62 Comm: kworker/0:2 Not tainted 4.13.0-rc2-00364-g4a97a3da420b #3357 > Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) > Workqueue: events output_poll_execute > task: edc14800 task.stack: edcb2000 > PC is at analogix_dp_transfer+0x15c/0x2fc > LR is at analogix_dp_transfer+0x134/0x2fc > pc : [<c0468538>] lr : [<c0468510>] psr: 60000013 > sp : edcb3be8 ip : 0000002a fp : 00000001 > r10: 00000000 r9 : edcb3cd8 r8 : edcb3c40 > r7 : 00000000 r6 : edd3b380 r5 : edd3b010 r4 : 00000064 > r3 : 00000000 r2 : f0ad3000 r1 : edcb3c40 r0 : edd3b010 > Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > Control: 10c5387d Table: 4000406a DAC: 00000051 > Process kworker/0:2 (pid: 62, stack limit = 0xedcb2210) > Stack: (0xedcb3be8 to 0xedcb4000) > [<c0468538>] (analogix_dp_transfer) from [<c0424ba4>] (drm_dp_i2c_do_msg+0x8c/0x2b4) > [<c0424ba4>] (drm_dp_i2c_do_msg) from [<c0424e64>] (drm_dp_i2c_xfer+0x98/0x214) > [<c0424e64>] (drm_dp_i2c_xfer) from [<c057b2d8>] (__i2c_transfer+0x140/0x29c) > [<c057b2d8>] (__i2c_transfer) from [<c057b4a4>] (i2c_transfer+0x70/0xe4) > [<c057b4a4>] (i2c_transfer) from [<c0441de4>] (drm_do_probe_ddc_edid+0xb4/0x114) > [<c0441de4>] (drm_do_probe_ddc_edid) from [<c0441e5c>] (drm_probe_ddc+0x18/0x28) > [<c0441e5c>] (drm_probe_ddc) from [<c0445728>] (drm_get_edid+0x124/0x2d4) > [<c0445728>] (drm_get_edid) from [<c0465ea0>] (analogix_dp_get_modes+0x90/0x114) > [<c0465ea0>] (analogix_dp_get_modes) from [<c0425e8c>] (drm_helper_probe_single_connector_modes+0x198/0x68c) > [<c0425e8c>] (drm_helper_probe_single_connector_modes) from [<c04325d4>] (drm_setup_crtcs+0x1b4/0xd18) > [<c04325d4>] (drm_setup_crtcs) from [<c04344a8>] (drm_fb_helper_hotplug_event+0x94/0xd0) > [<c04344a8>] (drm_fb_helper_hotplug_event) from [<c0425a50>] (drm_kms_helper_hotplug_event+0x24/0x28) > [<c0425a50>] (drm_kms_helper_hotplug_event) from [<c04263ec>] (output_poll_execute+0x6c/0x174) > [<c04263ec>] (output_poll_execute) from [<c0136f18>] (process_one_work+0x188/0x3fc) > [<c0136f18>] (process_one_work) from [<c01371f4>] (worker_thread+0x30/0x4b8) > [<c01371f4>] (worker_thread) from [<c013daf8>] (kthread+0x128/0x164) > [<c013daf8>] (kthread) from [<c0108510>] (ret_from_fork+0x14/0x24) > Code: 0a000002 ea000009 e2544001 0a00004a (e59537c8) > ---[ end trace cddc7919c79f7878 ]--- > > Reported-by: Misha Komarovskiy <zombah@gmail.com> > CC: stable@vger.kernel.org # v4.10+ > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > This issue was there from the beginning, but recent changes to DRM > core revealed it. It makes sense to backport it to patch f0a8b49c03d2 > ("drm/bridge: analogix dp: Fix runtime PM state on driver bind"), > which fixed similar issue on driver bind, thus I've marked it for > stable v4.10+. > > Best regards > Marek Szyprowski > Samsung R&D Institute Poland > --- > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > index 5dd3f1cd074a..a8905049b9da 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > @@ -946,7 +946,9 @@ static int analogix_dp_get_modes(struct drm_connector *connector) > return 0; > } > > + pm_runtime_get_sync(dp->dev); > edid = drm_get_edid(connector, &dp->aux.ddc); > + pm_runtime_put(dp->dev); > if (edid) { > drm_mode_connector_update_edid_property(&dp->connector, > edid); >
Hello, On Mon, Nov 27, 2017 at 6:40 AM, Archit Taneja <architt@codeaurora.org> wrote: > > > On 11/21/2017 01:19 PM, Marek Szyprowski wrote: >> >> get_modes() callback might be called asynchronously from the DRM core and >> it is not synchronized with bridge_enable(), which sets proper runtime PM >> state of the main DP device. Fix this by calling pm_runtime_get_sync() >> before calling drm_get_edid(), which in turn calls drm_dp_i2c_xfer() and >> analogix_dp_transfer() to ensure that main DP device is runtime active >> when doing any access to its registers. > > > Looks good to me. Would be nice to get an ack from rockchip too. Will queue > it > to drm-misc-fixes (so that it's merged for 4.15-rc2) if no one has any > objections. > After this patch accepted to 4.15-rc2 any chances to have it also in 4.14 stable? I tested it manualy and it fixes boot freeze on snow with 4.14. If required, Tested-by: Misha Komarovskiy <zombah@gmail.com> > Thanks, > Archit > >> >> This fixes the following kernel issue 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: 62 Comm: kworker/0:2 Not tainted >> 4.13.0-rc2-00364-g4a97a3da420b #3357 >> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >> Workqueue: events output_poll_execute >> task: edc14800 task.stack: edcb2000 >> PC is at analogix_dp_transfer+0x15c/0x2fc >> LR is at analogix_dp_transfer+0x134/0x2fc >> pc : [<c0468538>] lr : [<c0468510>] psr: 60000013 >> sp : edcb3be8 ip : 0000002a fp : 00000001 >> r10: 00000000 r9 : edcb3cd8 r8 : edcb3c40 >> r7 : 00000000 r6 : edd3b380 r5 : edd3b010 r4 : 00000064 >> r3 : 00000000 r2 : f0ad3000 r1 : edcb3c40 r0 : edd3b010 >> Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none >> Control: 10c5387d Table: 4000406a DAC: 00000051 >> Process kworker/0:2 (pid: 62, stack limit = 0xedcb2210) >> Stack: (0xedcb3be8 to 0xedcb4000) >> [<c0468538>] (analogix_dp_transfer) from [<c0424ba4>] >> (drm_dp_i2c_do_msg+0x8c/0x2b4) >> [<c0424ba4>] (drm_dp_i2c_do_msg) from [<c0424e64>] >> (drm_dp_i2c_xfer+0x98/0x214) >> [<c0424e64>] (drm_dp_i2c_xfer) from [<c057b2d8>] >> (__i2c_transfer+0x140/0x29c) >> [<c057b2d8>] (__i2c_transfer) from [<c057b4a4>] (i2c_transfer+0x70/0xe4) >> [<c057b4a4>] (i2c_transfer) from [<c0441de4>] >> (drm_do_probe_ddc_edid+0xb4/0x114) >> [<c0441de4>] (drm_do_probe_ddc_edid) from [<c0441e5c>] >> (drm_probe_ddc+0x18/0x28) >> [<c0441e5c>] (drm_probe_ddc) from [<c0445728>] (drm_get_edid+0x124/0x2d4) >> [<c0445728>] (drm_get_edid) from [<c0465ea0>] >> (analogix_dp_get_modes+0x90/0x114) >> [<c0465ea0>] (analogix_dp_get_modes) from [<c0425e8c>] >> (drm_helper_probe_single_connector_modes+0x198/0x68c) >> [<c0425e8c>] (drm_helper_probe_single_connector_modes) from [<c04325d4>] >> (drm_setup_crtcs+0x1b4/0xd18) >> [<c04325d4>] (drm_setup_crtcs) from [<c04344a8>] >> (drm_fb_helper_hotplug_event+0x94/0xd0) >> [<c04344a8>] (drm_fb_helper_hotplug_event) from [<c0425a50>] >> (drm_kms_helper_hotplug_event+0x24/0x28) >> [<c0425a50>] (drm_kms_helper_hotplug_event) from [<c04263ec>] >> (output_poll_execute+0x6c/0x174) >> [<c04263ec>] (output_poll_execute) from [<c0136f18>] >> (process_one_work+0x188/0x3fc) >> [<c0136f18>] (process_one_work) from [<c01371f4>] >> (worker_thread+0x30/0x4b8) >> [<c01371f4>] (worker_thread) from [<c013daf8>] (kthread+0x128/0x164) >> [<c013daf8>] (kthread) from [<c0108510>] (ret_from_fork+0x14/0x24) >> Code: 0a000002 ea000009 e2544001 0a00004a (e59537c8) >> ---[ end trace cddc7919c79f7878 ]--- >> >> Reported-by: Misha Komarovskiy <zombah@gmail.com> >> CC: stable@vger.kernel.org # v4.10+ >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> This issue was there from the beginning, but recent changes to DRM >> core revealed it. It makes sense to backport it to patch f0a8b49c03d2 >> ("drm/bridge: analogix dp: Fix runtime PM state on driver bind"), >> which fixed similar issue on driver bind, thus I've marked it for >> stable v4.10+. >> >> Best regards >> Marek Szyprowski >> Samsung R&D Institute Poland >> --- >> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c >> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c >> index 5dd3f1cd074a..a8905049b9da 100644 >> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c >> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c >> @@ -946,7 +946,9 @@ static int analogix_dp_get_modes(struct drm_connector >> *connector) >> return 0; >> } >> + pm_runtime_get_sync(dp->dev); >> edid = drm_get_edid(connector, &dp->aux.ddc); >> + pm_runtime_put(dp->dev); >> if (edid) { >> >> drm_mode_connector_update_edid_property(&dp->connector, >> edid); >> > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project
Hi Misha, On 2017-12-05 15:06, Misha Komarovskiy wrote: > On Mon, Nov 27, 2017 at 6:40 AM, Archit Taneja <architt@codeaurora.org> wrote: >> On 11/21/2017 01:19 PM, Marek Szyprowski wrote: >>> get_modes() callback might be called asynchronously from the DRM core and >>> it is not synchronized with bridge_enable(), which sets proper runtime PM >>> state of the main DP device. Fix this by calling pm_runtime_get_sync() >>> before calling drm_get_edid(), which in turn calls drm_dp_i2c_xfer() and >>> analogix_dp_transfer() to ensure that main DP device is runtime active >>> when doing any access to its registers. >> Looks good to me. Would be nice to get an ack from rockchip too. Will queue >> it >> to drm-misc-fixes (so that it's merged for 4.15-rc2) if no one has any >> objections. > After this patch accepted to 4.15-rc2 any chances to have it also in > 4.14 stable? > > I tested it manualy and it fixes boot freeze on snow with 4.14. > If required, > Tested-by: Misha Komarovskiy <zombah@gmail.com> The patch is tagged cc: stable, so it should automatically get into v4.14.y series once it finally lands in v4.15-rcX series. > ... Best regards
Hello Marek, On Tue, Dec 5, 2017 at 5:12 PM, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > Hi Misha, > > On 2017-12-05 15:06, Misha Komarovskiy wrote: >> >> On Mon, Nov 27, 2017 at 6:40 AM, Archit Taneja <architt@codeaurora.org> >> wrote: >>> >>> On 11/21/2017 01:19 PM, Marek Szyprowski wrote: >>>> >>>> get_modes() callback might be called asynchronously from the DRM core >>>> and >>>> it is not synchronized with bridge_enable(), which sets proper runtime >>>> PM >>>> state of the main DP device. Fix this by calling pm_runtime_get_sync() >>>> before calling drm_get_edid(), which in turn calls drm_dp_i2c_xfer() and >>>> analogix_dp_transfer() to ensure that main DP device is runtime active >>>> when doing any access to its registers. >>> >>> Looks good to me. Would be nice to get an ack from rockchip too. Will >>> queue >>> it >>> to drm-misc-fixes (so that it's merged for 4.15-rc2) if no one has any >>> objections. >> >> After this patch accepted to 4.15-rc2 any chances to have it also in >> 4.14 stable? >> >> I tested it manualy and it fixes boot freeze on snow with 4.14. >> If required, >> Tested-by: Misha Komarovskiy <zombah@gmail.com> > > > The patch is tagged cc: stable, so it should automatically get into v4.14.y > series once it finally lands in v4.15-rcX series. > I see. Thank you for clarification. Sorry for the noise 8) >> ... > > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland >
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 5dd3f1cd074a..a8905049b9da 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -946,7 +946,9 @@ static int analogix_dp_get_modes(struct drm_connector *connector) return 0; } + pm_runtime_get_sync(dp->dev); edid = drm_get_edid(connector, &dp->aux.ddc); + pm_runtime_put(dp->dev); if (edid) { drm_mode_connector_update_edid_property(&dp->connector, edid);
get_modes() callback might be called asynchronously from the DRM core and it is not synchronized with bridge_enable(), which sets proper runtime PM state of the main DP device. Fix this by calling pm_runtime_get_sync() before calling drm_get_edid(), which in turn calls drm_dp_i2c_xfer() and analogix_dp_transfer() to ensure that main DP device is runtime active when doing any access to its registers. This fixes the following kernel issue 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: 62 Comm: kworker/0:2 Not tainted 4.13.0-rc2-00364-g4a97a3da420b #3357 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) Workqueue: events output_poll_execute task: edc14800 task.stack: edcb2000 PC is at analogix_dp_transfer+0x15c/0x2fc LR is at analogix_dp_transfer+0x134/0x2fc pc : [<c0468538>] lr : [<c0468510>] psr: 60000013 sp : edcb3be8 ip : 0000002a fp : 00000001 r10: 00000000 r9 : edcb3cd8 r8 : edcb3c40 r7 : 00000000 r6 : edd3b380 r5 : edd3b010 r4 : 00000064 r3 : 00000000 r2 : f0ad3000 r1 : edcb3c40 r0 : edd3b010 Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 4000406a DAC: 00000051 Process kworker/0:2 (pid: 62, stack limit = 0xedcb2210) Stack: (0xedcb3be8 to 0xedcb4000) [<c0468538>] (analogix_dp_transfer) from [<c0424ba4>] (drm_dp_i2c_do_msg+0x8c/0x2b4) [<c0424ba4>] (drm_dp_i2c_do_msg) from [<c0424e64>] (drm_dp_i2c_xfer+0x98/0x214) [<c0424e64>] (drm_dp_i2c_xfer) from [<c057b2d8>] (__i2c_transfer+0x140/0x29c) [<c057b2d8>] (__i2c_transfer) from [<c057b4a4>] (i2c_transfer+0x70/0xe4) [<c057b4a4>] (i2c_transfer) from [<c0441de4>] (drm_do_probe_ddc_edid+0xb4/0x114) [<c0441de4>] (drm_do_probe_ddc_edid) from [<c0441e5c>] (drm_probe_ddc+0x18/0x28) [<c0441e5c>] (drm_probe_ddc) from [<c0445728>] (drm_get_edid+0x124/0x2d4) [<c0445728>] (drm_get_edid) from [<c0465ea0>] (analogix_dp_get_modes+0x90/0x114) [<c0465ea0>] (analogix_dp_get_modes) from [<c0425e8c>] (drm_helper_probe_single_connector_modes+0x198/0x68c) [<c0425e8c>] (drm_helper_probe_single_connector_modes) from [<c04325d4>] (drm_setup_crtcs+0x1b4/0xd18) [<c04325d4>] (drm_setup_crtcs) from [<c04344a8>] (drm_fb_helper_hotplug_event+0x94/0xd0) [<c04344a8>] (drm_fb_helper_hotplug_event) from [<c0425a50>] (drm_kms_helper_hotplug_event+0x24/0x28) [<c0425a50>] (drm_kms_helper_hotplug_event) from [<c04263ec>] (output_poll_execute+0x6c/0x174) [<c04263ec>] (output_poll_execute) from [<c0136f18>] (process_one_work+0x188/0x3fc) [<c0136f18>] (process_one_work) from [<c01371f4>] (worker_thread+0x30/0x4b8) [<c01371f4>] (worker_thread) from [<c013daf8>] (kthread+0x128/0x164) [<c013daf8>] (kthread) from [<c0108510>] (ret_from_fork+0x14/0x24) Code: 0a000002 ea000009 e2544001 0a00004a (e59537c8) ---[ end trace cddc7919c79f7878 ]--- Reported-by: Misha Komarovskiy <zombah@gmail.com> CC: stable@vger.kernel.org # v4.10+ Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- This issue was there from the beginning, but recent changes to DRM core revealed it. It makes sense to backport it to patch f0a8b49c03d2 ("drm/bridge: analogix dp: Fix runtime PM state on driver bind"), which fixed similar issue on driver bind, thus I've marked it for stable v4.10+. Best regards Marek Szyprowski Samsung R&D Institute Poland --- drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 2 ++ 1 file changed, 2 insertions(+)