diff mbox series

drm/bridge: analogix dp: Fix runtime PM state in get_modes() callback

Message ID 20171121074936.22520-1-m.szyprowski@samsung.com
State New
Headers show
Series drm/bridge: analogix dp: Fix runtime PM state in get_modes() callback | expand

Commit Message

Marek Szyprowski Nov. 21, 2017, 7:49 a.m. UTC 
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(+)

-- 
2.14.2

--
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

Archit Taneja Nov. 27, 2017, 3:40 a.m. UTC | #1 
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);

> 


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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
Misha Komarovskiy Dec. 5, 2017, 2:24 p.m. UTC | #2 
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

>




-- 
Best Regards,
Misha Komarovskiy
zombahatgmaildotcom
--
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
diff mbox series

Patch

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);