linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Heiko Stübner" <heiko@sntech.de>
To: Yakir Yang <ykk@rock-chips.com>, Thierry Reding <treding@nvidia.com>
Cc: Inki Dae <inki.dae@samsung.com>,
	Mark Yao <mark.yao@rock-chips.com>,
	Jingoo Han <jingoohan1@gmail.com>,
	Krzysztof Kozlowski <k.kozlowski@samsung.com>,
	Rob Herring <robh+dt@kernel.org>,
	Andrzej Hajda <a.hajda@samsung.com>,
	Joonyoung Shim <jy0922.shim@samsung.com>,
	Seung-Woo Kim <sw0312.kim@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Russell King <linux@arm.linux.org.uk>,
	emil.l.velikov@gmail.com,
	Gustavo Padovan <gustavo.padovan@collabora.co.uk>,
	Kishon Vijay Abraham I <kishon@ti.com>,
	ajaynumb@gmail.com, javier@osg.samsung.com,
	Andy Yan <andy.yan@rock-chips.com>,
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
	linux-rockchip@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v10 0/17] Add Analogix Core Display Port Driver
Date: Tue, 08 Dec 2015 16:33:39 +0100	[thread overview]
Message-ID: <1982691.ZDuzTFTUUe@diego> (raw)
In-Reply-To: <1449470239-30667-1-git-send-email-ykk@rock-chips.com>

Hi Yakir,

Am Montag, 7. Dezember 2015, 14:37:19 schrieb Yakir Yang:
>    The Samsung Exynos eDP controller and Rockchip RK3288 eDP controller
> share the same IP, so a lot of parts can be re-used. I split the common
> code into bridge directory, then rk3288 and exynos only need to keep
> some platform code. Cause I can't find the exact IP name of exynos dp
> controller, so I decide to name dp core driver with "analogix" which I
> find in rk3288 eDP TRM

[...]

> Changes in v10:
> - Correct the ROCKCHIP_ANALOGIX_DP indentation in Kconfig to tabs here
> (Heiko) - Fix the wrong macro value of
> GRF_EDP_REF_CLK_SEL_INTER_HIWORD_MASK BIT(4) -> BIT(20)
> - Remove the surplus "plat_data" check. (Heiko)
> -       switch (dp->plat_data && dp->plat_data->dev_type) {
> +       switch (dp->plat_data->dev_type) {
> - Revert parts of Gustavo Padovan's changes in commit:
> 	drm/exynos: do not start enabling DP at bind() phase
>   Add dp phy poweron function in bind time.

The hotplug issue is still present, but I think I found the cause. When
the first detect call happens, the display simply is still off. I just did
some very basic tracing [0] and it seems the display simply is not enabled
when it is supposed to get detected.

And it seems injecting a drm_panel_prepare early for _testing_ [1] really
did make the hotplug work on both my jerry and minnie.

So I guess we should somehow make sure the panel is actually powered when
detection is running. Although I'm not sure yet, how that should look like.


Intuition suggests, making drm_panel calls nestable (similar to
clk_prepare/unprepare, etc) and simply wrapping the detection code
in a prepare-unprepare calls, but I'm not sure if Thierry might have other
ideas ;-)


Also my "log" below suggests some sort of mismatch between
prepare/unprepare calls, as there are a lot more of the prepare-side.


And the locking issue also seems to be still there [2].


Heiko


[0]
[    2.797383] analogix_dp_reset
[    2.800709] analogix_dp_init_hpd
[    2.803960] analogix_dp_init_video
[    2.807653] rockchip-drm display-subsystem: bound ff970000.dp (ops rockchip_dp_component_ops)
[    2.817176] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[    2.823799] [drm] No driver support for vblank timestamp query.
[    2.829947] analogix_dp_detect
[    2.833015] analogix_dp_get_plug_in_status: hpd status 0
...
[    2.893425] analogix_dp_get_plug_in_status: hpd status 0
[    2.893456] rockchip-dp ff970000.dp: failed to get hpd plug status, try to force hpd
[    2.893458] analogix_dp_force_hpd
[    2.893464] analogix_dp_get_plug_in_status: hpd status 112
[    2.893470] panel_simple_prepare
[    2.952183] rockchip-dp ff970000.dp: EDID data does not include any extensions.
[    2.961727] panel_simple_get_modes
[    3.432154] analogix_dp_detect
[    3.432158] analogix_dp_get_plug_in_status: hpd status 120
[    3.432160] panel_simple_prepare
[    3.433731] rockchip-dp ff970000.dp: EDID data does not include any extensions.
[    3.443268] panel_simple_get_modes
[    3.444668] panel_simple_prepare
[    3.444755] analogix_dp_reset
[    3.445078] analogix_dp_init_hpd
[    3.445096] panel_simple_disable
[    3.455349] analogix_dp_init_video
[    3.558323] rockchip-dp ff970000.dp: Timeout of video streamclk ok
[    3.558326] rockchip-dp ff970000.dp: unable to config video
[    3.558328] panel_simple_enable
[    3.573915] analogix_dp_detect
[    3.573919] analogix_dp_get_plug_in_status: hpd status 72
[    3.573921] panel_simple_prepare


[1]
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 3990951..0c2dca5 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -399,6 +399,8 @@ static int rockchip_dp_probe(struct platform_device *pdev)
 
        dp->plat_data.panel = panel;
 
+drm_panel_prepare(dp->plat_data.panel);
+
        /*
         * We just use the drvdata until driver run into component
         * add function, and then we would set drvdata to null, so


[2]
[   11.971277] panel_simple_get_modes
[  OK  ] Started LSB: X display manager for KDE.
[  OK  ] Started LSB: Speech Dispatcher.
[   12.007120] panel_simple_disable
[   12.012323] 
[   12.013820] ======================================================
[   12.019993] [ INFO: possible circular locking dependency detected ]
[   12.026250] 4.4.0-rc3+ #2755 Not tainted
[   12.030165] -------------------------------------------------------
[  12.036417] Xorg/793 is trying to acquire lock:
[   12.040855]  ((&dp->hotplug_work)){+.+...}[   12.040870] 
[   12.040870] but task is already holding lock:
[   12.040871]  (crtc_ww_class_mutex){+.+.+.}, at: [<c0383600>] drm_modeset_lock+0x84/0x104
[   12.040881] 
[   12.040881] which lock already depends on the new lock.
[   12.040881] 
[   12.040882] 
[   12.040882] the existing dependency chain (in reverse order) is:
[   12.040883] 
[   12.040883] -> #2 (crtc_ww_class_mutex){+.+.+.}:
[   12.040887]        [<c0756374>] mutex_lock_nested+0x78/0x41c
[   12.040893]        [<c038365c>] drm_modeset_lock+0xe0/0x104
[   12.040896]        [<c0377888>] drm_mode_getconnector+0x168/0x38c
[   12.040902]        [<c036ae58>] drm_ioctl+0x274/0x408
[   12.040907]        [<c017cbc8>] do_vfs_ioctl+0x670/0x750
[   12.040911]        [<c017cd04>] SyS_ioctl+0x5c/0x84
[   12.040914]        [<c000ff80>] ret_fast_syscall+0x0/0x1c
[   12.040920] 
[   12.040920] -> #1 (&dev->mode_config.mutex){+.+.+.}:
[   12.040924]        [<c0756374>] mutex_lock_nested+0x78/0x41c
[   12.040928]        [<c035b3b4>] drm_helper_hpd_irq_event+0x40/0x150
[   12.040934]        [<c038e0b0>] analogix_dp_hotplug+0x24/0x28
[   12.040938]        [<c00450c4>] process_one_work+0x328/0x668
[   12.040942]        [<c0046334>] worker_thread+0x2cc/0x41c
[   12.040945]        [<c004bbe8>] kthread+0xf4/0x10c
[   12.040950]        [<c0010010>] ret_from_fork+0x14/0x24
[   12.040954] 
[   12.040954] -> #0 ((&dp->hotplug_work)){+.+...}:
[   12.040958]        [<c007f4ac>] lock_acquire+0x178/0x218
[   12.040963]        [<c00437d0>] flush_work+0x4c/0x22c
[   12.040966]        [<c038e45c>] analogix_dp_bridge_disable+0x74/0xf0
[   12.040970]        [<c0385ef4>] drm_bridge_disable+0x34/0x38
[   12.040973]        [<c0359044>] drm_crtc_helper_set_mode+0x200/0x424
[   12.040977]        [<c0359a38>] drm_crtc_helper_set_config+0x6c0/0x988
[   12.040981]        [<c0373ba8>] drm_mode_set_config_internal+0x60/0xdc
[   12.040984]        [<c0378938>] drm_mode_setcrtc+0x3cc/0x474
[   12.040988]        [<c036ae58>] drm_ioctl+0x274/0x408
[   12.040991]        [<c017cbc8>] do_vfs_ioctl+0x670/0x750
[   12.040994]        [<c017cd04>] SyS_ioctl+0x5c/0x84
[   12.040997]        [<c000ff80>] ret_fast_syscall+0x0/0x1c
[   12.041001] 
[   12.041001] other info that might help us debug this:
[   12.041001] 
[   12.041002] Chain exists of:
[   12.041002]   (&dp->hotplug_work) --> &dev->mode_config.mutex --> crtc_ww_class_mutex
[   12.041007] 
[   12.041008]  Possible unsafe locking scenario:
[   12.041008] 
[   12.041009]        CPU0                    CPU1
[   12.041010]        ----                    ----
[   12.041011]   lock(crtc_ww_class_mutex);
[   12.041013]                                lock(&dev->mode_config.mutex);
[   12.041016]                                lock(crtc_ww_class_mutex);
[   12.041018]   lock((&dp->hotplug_work));
[   12.041020] 
[   12.041020]  *** DEADLOCK ***
[   12.041020] 
[   12.041023] 3 locks held by Xorg/793:
[   12.041023]  #0:  (&dev->mode_config.mutex){+.+.+.}, at: [<c0383d48>] drm_modeset_lock_all+0x50/0xd8
[   12.041030]  #1:  (crtc_ww_class_acquire){+.+.+.}, at: [<c0383d58>] drm_modeset_lock_all+0x60/0xd8
[   12.041037]  #2:  (crtc_ww_class_mutex){+.+.+.}, at: [<c0383600>] drm_modeset_lock+0x84/0x104
[   12.041044] 
[   12.041044] stack backtrace:
[   12.041048] CPU: 3 PID: 793 Comm: Xorg Not tainted 4.4.0-rc3+ #2755
[   12.041049] Hardware name: Rockchip (Device Tree)
[   12.041059] [<c0019914>] (unwind_backtrace) from [<c0014bcc>] (show_stack+0x20/0x24)
[   12.041066] [<c0014bcc>] (show_stack) from [<c02c4484>] (dump_stack+0x84/0xb8)
[   12.041072] [<c02c4484>] (dump_stack) from [<c007a2b8>] (print_circular_bug+0x278/0x2cc)
[   12.041078] [<c007a2b8>] (print_circular_bug) from [<c007e850>] (__lock_acquire+0x14a0/0x1aec)
[   12.041082] [<c007e850>] (__lock_acquire) from [<c007f4ac>] (lock_acquire+0x178/0x218)
[   12.041086] [<c007f4ac>] (lock_acquire) from [<c00437d0>] (flush_work+0x4c/0x22c)
[   12.041091] [<c00437d0>] (flush_work) from [<c038e45c>] (analogix_dp_bridge_disable+0x74/0xf0)
[   12.041097] [<c038e45c>] (analogix_dp_bridge_disable) from [<c0385ef4>] (drm_bridge_disable+0x34/0x38)
[   12.041102] [<c0385ef4>] (drm_bridge_disable) from [<c0359044>] (drm_crtc_helper_set_mode+0x200/0x424)
[   12.041108] [<c0359044>] (drm_crtc_helper_set_mode) from [<c0359a38>] (drm_crtc_helper_set_config+0x6c0/0x988)
[   12.041114] [<c0359a38>] (drm_crtc_helper_set_config) from [<c0373ba8>] (drm_mode_set_config_internal+0x60/0xdc)
[   12.041119] [<c0373ba8>] (drm_mode_set_config_internal) from [<c0378938>] (drm_mode_setcrtc+0x3cc/0x474)
[   12.041124] [<c0378938>] (drm_mode_setcrtc) from [<c036ae58>] (drm_ioctl+0x274/0x408)
[   12.041129] [<c036ae58>] (drm_ioctl) from [<c017cbc8>] (do_vfs_ioctl+0x670/0x750)
[   12.041134] [<c017cbc8>] (do_vfs_ioctl) from [<c017cd04>] (SyS_ioctl+0x5c/0x84)
[   12.041138] [<c017cd04>] (SyS_ioctl) from [<c000ff80>] (ret_fast_syscall+0x0/0x1c)
[   12.041210] panel_simple_unprepare
[   12.041359] panel_simple_prepare
[   12.101638] analogix_dp_reset
[   12.101879] analogix_dp_init_hpd
[   12.101897] panel_simple_disable
[   12.108543] analogix_dp_irq_handler: irq-type 0
[   12.110452] rockchip-dp ff970000.dp: Link Training Clock Recovery success
[   12.111900] rockchip-dp ff970000.dp: Link Training success!
[   12.113384] analogix_dp_init_video
[   12.215825] rockchip-dp ff970000.dp: Timeout of video streamclk ok
[   12.215829] rockchip-dp ff970000.dp: unable to config video
[   12.215832] panel_simple_enable
,at: [<c0043784>] flush_work+0x0/0x22c


  parent reply	other threads:[~2015-12-08 15:34 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-07  6:37 [PATCH v10 0/17] Add Analogix Core Display Port Driver Yakir Yang
2015-12-07  6:38 ` [PATCH v10 01/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory Yakir Yang
2015-12-07  7:37   ` Yakir Yang
2015-12-07  6:38 ` [PATCH v10 02/17] drm: bridge: analogix/dp: fix some obvious code style Yakir Yang
2015-12-07  6:38 ` [PATCH v10 03/17] drm: bridge: analogix/dp: remove duplicate configuration of link rate and link count Yakir Yang
2015-12-07  6:38 ` [PATCH v10 04/17] drm: bridge: analogix/dp: dynamic parse sync_pol & interlace & dynamic_range Yakir Yang
2015-12-07  6:39 ` [PATCH v10 05/17] dt-bindings: add document for analogix display port driver Yakir Yang
2015-12-08 15:01   ` Rob Herring
2015-12-09  1:02     ` Yakir Yang
2015-12-07  6:39 ` [PATCH v10 06/17] ARM: dts: exynos/dp: remove some properties that deprecated by analogix_dp driver Yakir Yang
2015-12-07  6:39 ` [PATCH v10 07/17] drm: rockchip: dp: add rockchip platform dp driver Yakir Yang
2015-12-07  6:39 ` [PATCH v10 08/17] dt-bindings: add document for rockchip variant of analogix_dp Yakir Yang
2015-12-08 15:03   ` Rob Herring
2015-12-09  1:05     ` Yakir Yang
2015-12-07  6:39 ` [PATCH v10 09/17] phy: Add driver for rockchip Display Port PHY Yakir Yang
2015-12-07  6:39 ` [PATCH v10 10/17] dt-bindings: add document for rockchip dp phy Yakir Yang
2015-12-08 15:06   ` Rob Herring
2015-12-09  1:16     ` Yakir Yang
2015-12-07  6:40 ` [PATCH v10 11/17] drm: rockchip: vop: add bpc and color mode setting Yakir Yang
2015-12-07  6:40 ` [PATCH v10 12/17] drm: bridge: analogix/dp: add some rk3288 special registers setting Yakir Yang
2015-12-07  6:40 ` [PATCH v10 13/17] drm: bridge: analogix/dp: add max link rate and lane count limit for RK3288 Yakir Yang
2015-12-07  6:40 ` [PATCH v10 14/17] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Yakir Yang
2015-12-08 15:09   ` Rob Herring
2015-12-09  1:21     ` Yakir Yang
2015-12-07  6:40 ` [PATCH v10 15/17] drm: bridge: analogix/dp: move hpd detect to connector detect function Yakir Yang
2015-12-07  6:40 ` [PATCH v10 16/17] drm: bridge: analogix/dp: add edid modes parse in get_modes method Yakir Yang
2015-12-07  6:41 ` [PATCH v10 17/17] drm: bridge: analogix/dp: expand the look time for waiting AUX CH reply Yakir Yang
2015-12-07  7:52 ` [PATCH v11 01/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory Yakir Yang
2015-12-08 15:33 ` Heiko Stübner [this message]
     [not found]   ` <5667A4B6.4090708@rock-chips.com>
2015-12-09 14:51     ` [PATCH v10 0/17] Add Analogix Core Display Port Driver Heiko Stübner
2015-12-09  2:06 ` [PATCH v10.1 05/17] dt-bindings: add document for analogix display port driver Yakir Yang
2015-12-09  2:07 ` [PATCH v10.1 08/17] dt-bindings: add document for rockchip variant of analogix_dp Yakir Yang
2015-12-09  2:07 ` [PATCH v10.1 10/17] dt-bindings: add document for rockchip dp phy Yakir Yang
2015-12-09  2:10 ` [PATCH v10.1 14/17] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Yakir Yang
2015-12-09  4:25   ` Rob Herring
2015-12-09  5:55     ` Yakir Yang
2015-12-09  6:17 ` [PATCH v10.2 " Yakir Yang
2015-12-10  8:50 ` [PATCH v10.1 15/19] drm: bridge: analogix/dp: move hpd detect to connector detect function Yakir Yang
2015-12-10  8:52 ` [PATCH v10 18/19] drm: bridge: analogix/dp: add panel prepare/unprepare in suspend/resume time Yakir Yang
2015-12-10  8:55 ` [PATCH v10 19/19] drm: bridge: analogix/dp: Fix the possible dead lock in bridge disable time Yakir Yang
2015-12-15  0:06 ` [PATCH v10 0/17] Add Analogix Core Display Port Driver Heiko Stübner
2015-12-16  0:50   ` Yakir Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1982691.ZDuzTFTUUe@diego \
    --to=heiko@sntech.de \
    --cc=a.hajda@samsung.com \
    --cc=ajaynumb@gmail.com \
    --cc=andy.yan@rock-chips.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=emil.l.velikov@gmail.com \
    --cc=gustavo.padovan@collabora.co.uk \
    --cc=inki.dae@samsung.com \
    --cc=javier@osg.samsung.com \
    --cc=jingoohan1@gmail.com \
    --cc=jy0922.shim@samsung.com \
    --cc=k.kozlowski@samsung.com \
    --cc=kishon@ti.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=mark.yao@rock-chips.com \
    --cc=robh+dt@kernel.org \
    --cc=sw0312.kim@samsung.com \
    --cc=treding@nvidia.com \
    --cc=ykk@rock-chips.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).