Add missing pm_runtime_get_sync() into ltdc_crtc_atomic_enable() to match pm_runtime_put_sync() in ltdc_crtc_atomic_disable(), otherwise the LTDC might suspend via runtime PM, disable clock, and then fail to resume later on.
The test which triggers it is roughly -- run qt5 application which uses eglfs platform and etnaviv, stop the application, sleep for 15 minutes, run the application again. This leads to a timeout waiting for vsync, because the LTDC has suspended, but did not resume.
Fixes: 35ab6cfbf211 ("drm/stm: support runtime power management") Signed-off-by: Marek Vasut marex@denx.de Cc: Yannick Fertré yannick.fertre@st.com Cc: Philippe Cornu philippe.cornu@st.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: Maxime Coquelin mcoquelin.stm32@gmail.com Cc: Alexandre Torgue alexandre.torgue@st.com To: dri-devel@lists.freedesktop.org Cc: linux-stm32@st-md-mailman.stormreply.com Cc: linux-arm-kernel@lists.infradead.org --- ------------[ cut here ]------------ WARNING: CPU: 0 PID: 297 at drivers/gpu/drm/drm_atomic_helper.c:1494 drm_atomic_helper_wait_for_vblanks+0x1dc/0x200 [CRTC:35:crtc-0] vblank wait timed out Modules linked in: CPU: 0 PID: 297 Comm: QSGRenderThread Not tainted 5.6.0-rc3-next-20200228-00010-g318bf0fc08ef #2 Hardware name: STM32 (Device Tree Support) [<c010f18c>] (unwind_backtrace) from [<c010afb8>] (show_stack+0x10/0x14) [<c010afb8>] (show_stack) from [<c07b1d3c>] (dump_stack+0xb4/0xd0) [<c07b1d3c>] (dump_stack) from [<c011d8b8>] (__warn+0xd4/0xf0) [<c011d8b8>] (__warn) from [<c011dc4c>] (warn_slowpath_fmt+0x78/0xa8) [<c011dc4c>] (warn_slowpath_fmt) from [<c04a266c>] (drm_atomic_helper_wait_for_vblanks+0x1dc/0x200) [<c04a266c>] (drm_atomic_helper_wait_for_vblanks) from [<c04a510c>] (drm_atomic_helper_commit_tail+0 x50/0x60) [<c04a510c>] (drm_atomic_helper_commit_tail) from [<c04a52a8>] (commit_tail+0x12c/0x13c) [<c04a52a8>] (commit_tail) from [<c04a53b4>] (drm_atomic_helper_commit+0xf4/0x100) [<c04a53b4>] (drm_atomic_helper_commit) from [<c04a2d38>] (drm_atomic_helper_set_config+0x58/0x6c) [<c04a2d38>] (drm_atomic_helper_set_config) from [<c04b1994>] (drm_mode_setcrtc+0x450/0x550) [<c04b1994>] (drm_mode_setcrtc) from [<c04ad570>] (drm_ioctl_kernel+0x90/0xe8) [<c04ad570>] (drm_ioctl_kernel) from [<c04ad8ac>] (drm_ioctl+0x2e4/0x32c) [<c04ad8ac>] (drm_ioctl) from [<c0246784>] (vfs_ioctl+0x20/0x38) [<c0246784>] (vfs_ioctl) from [<c02470f0>] (ksys_ioctl+0xbc/0x7b0) [<c02470f0>] (ksys_ioctl) from [<c0101000>] (ret_fast_syscall+0x0/0x54) Exception stack(0xee8f3fa8 to 0xee8f3ff0) 3fa0: 00000005 adcbeb18 00000005 c06864a2 adcbeb18 00000001 3fc0: 00000005 adcbeb18 c06864a2 00000036 00000029 00000023 00000023 00000007 3fe0: b113b098 adcbeafc b1125413 b6155cf8 ---[ end trace 2ad5ba954ceb767a ]--- --- drivers/gpu/drm/stm/ltdc.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 99bf93e8b36f..301de0498078 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -425,9 +425,12 @@ static void ltdc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { struct ltdc_device *ldev = crtc_to_ltdc(crtc); + struct drm_device *ddev = crtc->dev;
DRM_DEBUG_DRIVER("\n");
+ pm_runtime_get_sync(ddev->dev); + /* Sets the background color value */ reg_write(ldev->regs, LTDC_BCCR, BCCR_BCBLACK);
Hello Marek, Thank for your patch. Pm_runtime_put_sync is also done into function ltdc_crtc_mode_fixup. To avoid several call of Pm_runtime_put_sync, it could be better to check pm_runtime activity:
+ int ret;
DRM_DEBUG_DRIVER("\n");
+ if (!pm_runtime_active(ddev->dev)) { + ret = pm_runtime_get_sync(ddev->dev); + if (ret) { + DRM_ERROR("Failed to enable crtc, cannot get sync\n"); + return; + } + } +
Best regards
Yannick Fertré
-----Original Message----- From: Marek Vasut marex@denx.de Sent: samedi 29 février 2020 23:17 To: dri-devel@lists.freedesktop.org Cc: Marek Vasut marex@denx.de; Yannick FERTRE yannick.fertre@st.com; Philippe CORNU philippe.cornu@st.com; Benjamin Gaignard benjamin.gaignard@linaro.org; Vincent ABRIOU vincent.abriou@st.com; Maxime Coquelin mcoquelin.stm32@gmail.com; Alexandre TORGUE alexandre.torgue@st.com; linux-stm32@st-md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org Subject: [PATCH] drm/stm: repair runtime power management
Add missing pm_runtime_get_sync() into ltdc_crtc_atomic_enable() to match pm_runtime_put_sync() in ltdc_crtc_atomic_disable(), otherwise the LTDC might suspend via runtime PM, disable clock, and then fail to resume later on.
The test which triggers it is roughly -- run qt5 application which uses eglfs platform and etnaviv, stop the application, sleep for 15 minutes, run the application again. This leads to a timeout waiting for vsync, because the LTDC has suspended, but did not resume.
Fixes: 35ab6cfbf211 ("drm/stm: support runtime power management") Signed-off-by: Marek Vasut marex@denx.de Cc: Yannick Fertré yannick.fertre@st.com Cc: Philippe Cornu philippe.cornu@st.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: Maxime Coquelin mcoquelin.stm32@gmail.com Cc: Alexandre Torgue alexandre.torgue@st.com To: dri-devel@lists.freedesktop.org Cc: linux-stm32@st-md-mailman.stormreply.com Cc: linux-arm-kernel@lists.infradead.org --- ------------[ cut here ]------------ WARNING: CPU: 0 PID: 297 at drivers/gpu/drm/drm_atomic_helper.c:1494 drm_atomic_helper_wait_for_vblanks+0x1dc/0x200 [CRTC:35:crtc-0] vblank wait timed out Modules linked in: CPU: 0 PID: 297 Comm: QSGRenderThread Not tainted 5.6.0-rc3-next-20200228-00010-g318bf0fc08ef #2 Hardware name: STM32 (Device Tree Support) [<c010f18c>] (unwind_backtrace) from [<c010afb8>] (show_stack+0x10/0x14) [<c010afb8>] (show_stack) from [<c07b1d3c>] (dump_stack+0xb4/0xd0) [<c07b1d3c>] (dump_stack) from [<c011d8b8>] (__warn+0xd4/0xf0) [<c011d8b8>] (__warn) from [<c011dc4c>] (warn_slowpath_fmt+0x78/0xa8) [<c011dc4c>] (warn_slowpath_fmt) from [<c04a266c>] (drm_atomic_helper_wait_for_vblanks+0x1dc/0x200) [<c04a266c>] (drm_atomic_helper_wait_for_vblanks) from [<c04a510c>] (drm_atomic_helper_commit_tail+0 x50/0x60) [<c04a510c>] (drm_atomic_helper_commit_tail) from [<c04a52a8>] (commit_tail+0x12c/0x13c) [<c04a52a8>] (commit_tail) from [<c04a53b4>] (drm_atomic_helper_commit+0xf4/0x100) [<c04a53b4>] (drm_atomic_helper_commit) from [<c04a2d38>] (drm_atomic_helper_set_config+0x58/0x6c) [<c04a2d38>] (drm_atomic_helper_set_config) from [<c04b1994>] (drm_mode_setcrtc+0x450/0x550) [<c04b1994>] (drm_mode_setcrtc) from [<c04ad570>] (drm_ioctl_kernel+0x90/0xe8) [<c04ad570>] (drm_ioctl_kernel) from [<c04ad8ac>] (drm_ioctl+0x2e4/0x32c) [<c04ad8ac>] (drm_ioctl) from [<c0246784>] (vfs_ioctl+0x20/0x38) [<c0246784>] (vfs_ioctl) from [<c02470f0>] (ksys_ioctl+0xbc/0x7b0) [<c02470f0>] (ksys_ioctl) from [<c0101000>] (ret_fast_syscall+0x0/0x54) Exception stack(0xee8f3fa8 to 0xee8f3ff0) 3fa0: 00000005 adcbeb18 00000005 c06864a2 adcbeb18 00000001 3fc0: 00000005 adcbeb18 c06864a2 00000036 00000029 00000023 00000023 00000007 3fe0: b113b098 adcbeafc b1125413 b6155cf8 ---[ end trace 2ad5ba954ceb767a ]--- --- drivers/gpu/drm/stm/ltdc.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 99bf93e8b36f..301de0498078 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -425,9 +425,12 @@ static void ltdc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { struct ltdc_device *ldev = crtc_to_ltdc(crtc); + struct drm_device *ddev = crtc->dev;
DRM_DEBUG_DRIVER("\n");
+ pm_runtime_get_sync(ddev->dev); + /* Sets the background color value */ reg_write(ldev->regs, LTDC_BCCR, BCCR_BCBLACK);
-- 2.25.0
On 3/9/20 11:35 AM, Yannick FERTRE wrote:
Hello Marek,
Hi,
(please stop top-posting)
Thank for your patch. Pm_runtime_put_sync is also done into function ltdc_crtc_mode_fixup. To avoid several call of Pm_runtime_put_sync, it could be better to check pm_runtime activity:
int ret;
DRM_DEBUG_DRIVER("\n");
if (!pm_runtime_active(ddev->dev)) {
ret = pm_runtime_get_sync(ddev->dev);if (ret) {DRM_ERROR("Failed to enable crtc, cannot get sync\n");return;}}
Where should this go ? And wouldn't that only hide nastier PM imbalance issues ?
Best regards
Yannick Fertré
-----Original Message----- From: Marek Vasut marex@denx.de Sent: samedi 29 février 2020 23:17 To: dri-devel@lists.freedesktop.org Cc: Marek Vasut marex@denx.de; Yannick FERTRE yannick.fertre@st.com; Philippe CORNU philippe.cornu@st.com; Benjamin Gaignard benjamin.gaignard@linaro.org; Vincent ABRIOU vincent.abriou@st.com; Maxime Coquelin mcoquelin.stm32@gmail.com; Alexandre TORGUE alexandre.torgue@st.com; linux-stm32@st-md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org Subject: [PATCH] drm/stm: repair runtime power management
Add missing pm_runtime_get_sync() into ltdc_crtc_atomic_enable() to match pm_runtime_put_sync() in ltdc_crtc_atomic_disable(), otherwise the LTDC might suspend via runtime PM, disable clock, and then fail to resume later on.
The test which triggers it is roughly -- run qt5 application which uses eglfs platform and etnaviv, stop the application, sleep for 15 minutes, run the application again. This leads to a timeout waiting for vsync, because the LTDC has suspended, but did not resume.
Fixes: 35ab6cfbf211 ("drm/stm: support runtime power management") Signed-off-by: Marek Vasut marex@denx.de Cc: Yannick Fertré yannick.fertre@st.com Cc: Philippe Cornu philippe.cornu@st.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: Maxime Coquelin mcoquelin.stm32@gmail.com Cc: Alexandre Torgue alexandre.torgue@st.com To: dri-devel@lists.freedesktop.org Cc: linux-stm32@st-md-mailman.stormreply.com Cc: linux-arm-kernel@lists.infradead.org
------------[ cut here ]------------ WARNING: CPU: 0 PID: 297 at drivers/gpu/drm/drm_atomic_helper.c:1494 drm_atomic_helper_wait_for_vblanks+0x1dc/0x200 [CRTC:35:crtc-0] vblank wait timed out Modules linked in: CPU: 0 PID: 297 Comm: QSGRenderThread Not tainted 5.6.0-rc3-next-20200228-00010-g318bf0fc08ef #2 Hardware name: STM32 (Device Tree Support) [<c010f18c>] (unwind_backtrace) from [<c010afb8>] (show_stack+0x10/0x14) [<c010afb8>] (show_stack) from [<c07b1d3c>] (dump_stack+0xb4/0xd0) [<c07b1d3c>] (dump_stack) from [<c011d8b8>] (__warn+0xd4/0xf0) [<c011d8b8>] (__warn) from [<c011dc4c>] (warn_slowpath_fmt+0x78/0xa8) [<c011dc4c>] (warn_slowpath_fmt) from [<c04a266c>] (drm_atomic_helper_wait_for_vblanks+0x1dc/0x200) [<c04a266c>] (drm_atomic_helper_wait_for_vblanks) from [<c04a510c>] (drm_atomic_helper_commit_tail+0 x50/0x60) [<c04a510c>] (drm_atomic_helper_commit_tail) from [<c04a52a8>] (commit_tail+0x12c/0x13c) [<c04a52a8>] (commit_tail) from [<c04a53b4>] (drm_atomic_helper_commit+0xf4/0x100) [<c04a53b4>] (drm_atomic_helper_commit) from [<c04a2d38>] (drm_atomic_helper_set_config+0x58/0x6c) [<c04a2d38>] (drm_atomic_helper_set_config) from [<c04b1994>] (drm_mode_setcrtc+0x450/0x550) [<c04b1994>] (drm_mode_setcrtc) from [<c04ad570>] (drm_ioctl_kernel+0x90/0xe8) [<c04ad570>] (drm_ioctl_kernel) from [<c04ad8ac>] (drm_ioctl+0x2e4/0x32c) [<c04ad8ac>] (drm_ioctl) from [<c0246784>] (vfs_ioctl+0x20/0x38) [<c0246784>] (vfs_ioctl) from [<c02470f0>] (ksys_ioctl+0xbc/0x7b0) [<c02470f0>] (ksys_ioctl) from [<c0101000>] (ret_fast_syscall+0x0/0x54) Exception stack(0xee8f3fa8 to 0xee8f3ff0) 3fa0: 00000005 adcbeb18 00000005 c06864a2 adcbeb18 00000001 3fc0: 00000005 adcbeb18 c06864a2 00000036 00000029 00000023 00000023 00000007 3fe0: b113b098 adcbeafc b1125413 b6155cf8 ---[ end trace 2ad5ba954ceb767a ]---
drivers/gpu/drm/stm/ltdc.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 99bf93e8b36f..301de0498078 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -425,9 +425,12 @@ static void ltdc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { struct ltdc_device *ldev = crtc_to_ltdc(crtc);
struct drm_device *ddev = crtc->dev;
DRM_DEBUG_DRIVER("\n");
pm_runtime_get_sync(ddev->dev);
/* Sets the background color value */ reg_write(ldev->regs, LTDC_BCCR, BCCR_BCBLACK);
-- 2.25.0
On 3/9/20 12:57 PM, Marek Vasut wrote:
On 3/9/20 11:35 AM, Yannick FERTRE wrote:
Hello Marek,
Hi,
(please stop top-posting)
Thank for your patch. Pm_runtime_put_sync is also done into function ltdc_crtc_mode_fixup. To avoid several call of Pm_runtime_put_sync, it could be better to check pm_runtime activity:
int ret;
DRM_DEBUG_DRIVER("\n");
if (!pm_runtime_active(ddev->dev)) {
ret = pm_runtime_get_sync(ddev->dev);if (ret) {DRM_ERROR("Failed to enable crtc, cannot get sync\n");return;}}
Where should this go ? And wouldn't that only hide nastier PM imbalance issues ?
Hi Marek, I tested the patch & it generate an error when I try wake up / sleep the board STM32MP1 DK2 with weston application. It need an additional patch drm-stm-ltdc-remove-call-of-pm-runtime-functions.
Thanks for the patch.
Tested-by: Yannick Fertre yannick.fertre@st.com
Best regards
Yannick Fertré
-----Original Message----- From: Marek Vasut marex@denx.de Sent: samedi 29 février 2020 23:17 To: dri-devel@lists.freedesktop.org Cc: Marek Vasut marex@denx.de; Yannick FERTRE yannick.fertre@st.com; Philippe CORNU philippe.cornu@st.com; Benjamin Gaignard benjamin.gaignard@linaro.org; Vincent ABRIOU vincent.abriou@st.com; Maxime Coquelin mcoquelin.stm32@gmail.com; Alexandre TORGUE alexandre.torgue@st.com; linux-stm32@st-md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org Subject: [PATCH] drm/stm: repair runtime power management
Add missing pm_runtime_get_sync() into ltdc_crtc_atomic_enable() to match pm_runtime_put_sync() in ltdc_crtc_atomic_disable(), otherwise the LTDC might suspend via runtime PM, disable clock, and then fail to resume later on.
The test which triggers it is roughly -- run qt5 application which uses eglfs platform and etnaviv, stop the application, sleep for 15 minutes, run the application again. This leads to a timeout waiting for vsync, because the LTDC has suspended, but did not resume.
Fixes: 35ab6cfbf211 ("drm/stm: support runtime power management") Signed-off-by: Marek Vasut marex@denx.de Cc: Yannick Fertré yannick.fertre@st.com Cc: Philippe Cornu philippe.cornu@st.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: Maxime Coquelin mcoquelin.stm32@gmail.com Cc: Alexandre Torgue alexandre.torgue@st.com To: dri-devel@lists.freedesktop.org Cc: linux-stm32@st-md-mailman.stormreply.com Cc: linux-arm-kernel@lists.infradead.org
------------[ cut here ]------------ WARNING: CPU: 0 PID: 297 at drivers/gpu/drm/drm_atomic_helper.c:1494 drm_atomic_helper_wait_for_vblanks+0x1dc/0x200 [CRTC:35:crtc-0] vblank wait timed out Modules linked in: CPU: 0 PID: 297 Comm: QSGRenderThread Not tainted 5.6.0-rc3-next-20200228-00010-g318bf0fc08ef #2 Hardware name: STM32 (Device Tree Support) [<c010f18c>] (unwind_backtrace) from [<c010afb8>] (show_stack+0x10/0x14) [<c010afb8>] (show_stack) from [<c07b1d3c>] (dump_stack+0xb4/0xd0) [<c07b1d3c>] (dump_stack) from [<c011d8b8>] (__warn+0xd4/0xf0) [<c011d8b8>] (__warn) from [<c011dc4c>] (warn_slowpath_fmt+0x78/0xa8) [<c011dc4c>] (warn_slowpath_fmt) from [<c04a266c>] (drm_atomic_helper_wait_for_vblanks+0x1dc/0x200) [<c04a266c>] (drm_atomic_helper_wait_for_vblanks) from [<c04a510c>] (drm_atomic_helper_commit_tail+0 x50/0x60) [<c04a510c>] (drm_atomic_helper_commit_tail) from [<c04a52a8>] (commit_tail+0x12c/0x13c) [<c04a52a8>] (commit_tail) from [<c04a53b4>] (drm_atomic_helper_commit+0xf4/0x100) [<c04a53b4>] (drm_atomic_helper_commit) from [<c04a2d38>] (drm_atomic_helper_set_config+0x58/0x6c) [<c04a2d38>] (drm_atomic_helper_set_config) from [<c04b1994>] (drm_mode_setcrtc+0x450/0x550) [<c04b1994>] (drm_mode_setcrtc) from [<c04ad570>] (drm_ioctl_kernel+0x90/0xe8) [<c04ad570>] (drm_ioctl_kernel) from [<c04ad8ac>] (drm_ioctl+0x2e4/0x32c) [<c04ad8ac>] (drm_ioctl) from [<c0246784>] (vfs_ioctl+0x20/0x38) [<c0246784>] (vfs_ioctl) from [<c02470f0>] (ksys_ioctl+0xbc/0x7b0) [<c02470f0>] (ksys_ioctl) from [<c0101000>] (ret_fast_syscall+0x0/0x54) Exception stack(0xee8f3fa8 to 0xee8f3ff0) 3fa0: 00000005 adcbeb18 00000005 c06864a2 adcbeb18 00000001 3fc0: 00000005 adcbeb18 c06864a2 00000036 00000029 00000023 00000023 00000007 3fe0: b113b098 adcbeafc b1125413 b6155cf8 ---[ end trace 2ad5ba954ceb767a ]---
drivers/gpu/drm/stm/ltdc.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 99bf93e8b36f..301de0498078 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -425,9 +425,12 @@ static void ltdc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { struct ltdc_device *ldev = crtc_to_ltdc(crtc);
struct drm_device *ddev = crtc->dev;
DRM_DEBUG_DRIVER("\n");
pm_runtime_get_sync(ddev->dev);
/* Sets the background color value */ reg_write(ldev->regs, LTDC_BCCR, BCCR_BCBLACK);
-- 2.25.0
On 7/1/20 2:14 PM, Yannick FERTRE wrote:
On 3/9/20 12:57 PM, Marek Vasut wrote:
On 3/9/20 11:35 AM, Yannick FERTRE wrote:
Hello Marek,
Hi,
(please stop top-posting)
Thank for your patch. Pm_runtime_put_sync is also done into function ltdc_crtc_mode_fixup. To avoid several call of Pm_runtime_put_sync, it could be better to check pm_runtime activity:
int ret;
DRM_DEBUG_DRIVER("\n");
if (!pm_runtime_active(ddev->dev)) {
ret = pm_runtime_get_sync(ddev->dev);if (ret) {DRM_ERROR("Failed to enable crtc, cannot get sync\n");return;}}
Where should this go ? And wouldn't that only hide nastier PM imbalance issues ?
Hi Marek, I tested the patch & it generate an error when I try wake up / sleep the board STM32MP1 DK2 with weston application. It need an additional patch drm-stm-ltdc-remove-call-of-pm-runtime-functions.
Thanks for the patch.
Tested-by: Yannick Fertre yannick.fertre@st.com
Hi Marek, before merging the 2 patches, I would like to be sure that Yannick's patch does not "break" your use case (Qt I think)? May I ask you please to give it a try? Note: If you think there is no need to do extra checks, simply tell me of course Philippe :-)
Best regards
Yannick Fertré
-----Original Message----- From: Marek Vasut marex@denx.de Sent: samedi 29 février 2020 23:17 To: dri-devel@lists.freedesktop.org Cc: Marek Vasut marex@denx.de; Yannick FERTRE yannick.fertre@st.com; Philippe CORNU philippe.cornu@st.com; Benjamin Gaignard benjamin.gaignard@linaro.org; Vincent ABRIOU vincent.abriou@st.com; Maxime Coquelin mcoquelin.stm32@gmail.com; Alexandre TORGUE alexandre.torgue@st.com; linux-stm32@st-md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org Subject: [PATCH] drm/stm: repair runtime power management
Add missing pm_runtime_get_sync() into ltdc_crtc_atomic_enable() to match pm_runtime_put_sync() in ltdc_crtc_atomic_disable(), otherwise the LTDC might suspend via runtime PM, disable clock, and then fail to resume later on.
The test which triggers it is roughly -- run qt5 application which uses eglfs platform and etnaviv, stop the application, sleep for 15 minutes, run the application again. This leads to a timeout waiting for vsync, because the LTDC has suspended, but did not resume.
Fixes: 35ab6cfbf211 ("drm/stm: support runtime power management") Signed-off-by: Marek Vasut marex@denx.de Cc: Yannick Fertré yannick.fertre@st.com Cc: Philippe Cornu philippe.cornu@st.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: Maxime Coquelin mcoquelin.stm32@gmail.com Cc: Alexandre Torgue alexandre.torgue@st.com To: dri-devel@lists.freedesktop.org Cc: linux-stm32@st-md-mailman.stormreply.com Cc: linux-arm-kernel@lists.infradead.org
------------[ cut here ]------------ WARNING: CPU: 0 PID: 297 at drivers/gpu/drm/drm_atomic_helper.c:1494 drm_atomic_helper_wait_for_vblanks+0x1dc/0x200 [CRTC:35:crtc-0] vblank wait timed out Modules linked in: CPU: 0 PID: 297 Comm: QSGRenderThread Not tainted 5.6.0-rc3-next-20200228-00010-g318bf0fc08ef #2 Hardware name: STM32 (Device Tree Support) [<c010f18c>] (unwind_backtrace) from [<c010afb8>] (show_stack+0x10/0x14) [<c010afb8>] (show_stack) from [<c07b1d3c>] (dump_stack+0xb4/0xd0) [<c07b1d3c>] (dump_stack) from [<c011d8b8>] (__warn+0xd4/0xf0) [<c011d8b8>] (__warn) from [<c011dc4c>] (warn_slowpath_fmt+0x78/0xa8) [<c011dc4c>] (warn_slowpath_fmt) from [<c04a266c>] (drm_atomic_helper_wait_for_vblanks+0x1dc/0x200) [<c04a266c>] (drm_atomic_helper_wait_for_vblanks) from [<c04a510c>] (drm_atomic_helper_commit_tail+0 x50/0x60) [<c04a510c>] (drm_atomic_helper_commit_tail) from [<c04a52a8>] (commit_tail+0x12c/0x13c) [<c04a52a8>] (commit_tail) from [<c04a53b4>] (drm_atomic_helper_commit+0xf4/0x100) [<c04a53b4>] (drm_atomic_helper_commit) from [<c04a2d38>] (drm_atomic_helper_set_config+0x58/0x6c) [<c04a2d38>] (drm_atomic_helper_set_config) from [<c04b1994>] (drm_mode_setcrtc+0x450/0x550) [<c04b1994>] (drm_mode_setcrtc) from [<c04ad570>] (drm_ioctl_kernel+0x90/0xe8) [<c04ad570>] (drm_ioctl_kernel) from [<c04ad8ac>] (drm_ioctl+0x2e4/0x32c) [<c04ad8ac>] (drm_ioctl) from [<c0246784>] (vfs_ioctl+0x20/0x38) [<c0246784>] (vfs_ioctl) from [<c02470f0>] (ksys_ioctl+0xbc/0x7b0) [<c02470f0>] (ksys_ioctl) from [<c0101000>] (ret_fast_syscall+0x0/0x54) Exception stack(0xee8f3fa8 to 0xee8f3ff0) 3fa0: 00000005 adcbeb18 00000005 c06864a2 adcbeb18 00000001 3fc0: 00000005 adcbeb18 c06864a2 00000036 00000029 00000023 00000023 00000007 3fe0: b113b098 adcbeafc b1125413 b6155cf8 ---[ end trace 2ad5ba954ceb767a ]---
drivers/gpu/drm/stm/ltdc.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 99bf93e8b36f..301de0498078 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -425,9 +425,12 @@ static void ltdc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { struct ltdc_device *ldev = crtc_to_ltdc(crtc);
struct drm_device *ddev = crtc->dev;
DRM_DEBUG_DRIVER("\n");
pm_runtime_get_sync(ddev->dev);
/* Sets the background color value */ reg_write(ldev->regs, LTDC_BCCR, BCCR_BCBLACK);
-- 2.25.0
On 7/2/20 12:07 PM, Philippe CORNU wrote:
Hi,
[...]
Thank for your patch. Pm_runtime_put_sync is also done into function ltdc_crtc_mode_fixup. To avoid several call of Pm_runtime_put_sync, it could be better to check pm_runtime activity:
int ret;
DRM_DEBUG_DRIVER("\n");
if (!pm_runtime_active(ddev->dev)) {
ret = pm_runtime_get_sync(ddev->dev);if (ret) {DRM_ERROR("Failed to enable crtc, cannot get sync\n");return;}}
Where should this go ? And wouldn't that only hide nastier PM imbalance issues ?
Hi Marek, I tested the patch & it generate an error when I try wake up / sleep the board STM32MP1 DK2 with weston application. It need an additional patch drm-stm-ltdc-remove-call-of-pm-runtime-functions.
Thanks for the patch.
Tested-by: Yannick Fertre yannick.fertre@st.com
Hi Marek, before merging the 2 patches, I would like to be sure that Yannick's patch does not "break" your use case (Qt I think)? May I ask you please to give it a try? Note: If you think there is no need to do extra checks, simply tell me of course
It's fine, thanks !
On 2/29/20 11:16 PM, Marek Vasut wrote:
Add missing pm_runtime_get_sync() into ltdc_crtc_atomic_enable() to match pm_runtime_put_sync() in ltdc_crtc_atomic_disable(), otherwise the LTDC might suspend via runtime PM, disable clock, and then fail to resume later on.
The test which triggers it is roughly -- run qt5 application which uses eglfs platform and etnaviv, stop the application, sleep for 15 minutes, run the application again. This leads to a timeout waiting for vsync, because the LTDC has suspended, but did not resume.
Fixes: 35ab6cfbf211 ("drm/stm: support runtime power management") Signed-off-by: Marek Vasut marex@denx.de Cc: Yannick Fertré yannick.fertre@st.com Cc: Philippe Cornu philippe.cornu@st.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: Maxime Coquelin mcoquelin.stm32@gmail.com Cc: Alexandre Torgue alexandre.torgue@st.com To: dri-devel@lists.freedesktop.org Cc: linux-stm32@st-md-mailman.stormreply.com Cc: linux-arm-kernel@lists.infradead.org
------------[ cut here ]------------ WARNING: CPU: 0 PID: 297 at drivers/gpu/drm/drm_atomic_helper.c:1494 drm_atomic_helper_wait_for_vblanks+0x1dc/0x200 [CRTC:35:crtc-0] vblank wait timed out Modules linked in: CPU: 0 PID: 297 Comm: QSGRenderThread Not tainted 5.6.0-rc3-next-20200228-00010-g318bf0fc08ef #2 Hardware name: STM32 (Device Tree Support) [<c010f18c>] (unwind_backtrace) from [<c010afb8>] (show_stack+0x10/0x14) [<c010afb8>] (show_stack) from [<c07b1d3c>] (dump_stack+0xb4/0xd0) [<c07b1d3c>] (dump_stack) from [<c011d8b8>] (__warn+0xd4/0xf0) [<c011d8b8>] (__warn) from [<c011dc4c>] (warn_slowpath_fmt+0x78/0xa8) [<c011dc4c>] (warn_slowpath_fmt) from [<c04a266c>] (drm_atomic_helper_wait_for_vblanks+0x1dc/0x200) [<c04a266c>] (drm_atomic_helper_wait_for_vblanks) from [<c04a510c>] (drm_atomic_helper_commit_tail+0 x50/0x60) [<c04a510c>] (drm_atomic_helper_commit_tail) from [<c04a52a8>] (commit_tail+0x12c/0x13c) [<c04a52a8>] (commit_tail) from [<c04a53b4>] (drm_atomic_helper_commit+0xf4/0x100) [<c04a53b4>] (drm_atomic_helper_commit) from [<c04a2d38>] (drm_atomic_helper_set_config+0x58/0x6c) [<c04a2d38>] (drm_atomic_helper_set_config) from [<c04b1994>] (drm_mode_setcrtc+0x450/0x550) [<c04b1994>] (drm_mode_setcrtc) from [<c04ad570>] (drm_ioctl_kernel+0x90/0xe8) [<c04ad570>] (drm_ioctl_kernel) from [<c04ad8ac>] (drm_ioctl+0x2e4/0x32c) [<c04ad8ac>] (drm_ioctl) from [<c0246784>] (vfs_ioctl+0x20/0x38) [<c0246784>] (vfs_ioctl) from [<c02470f0>] (ksys_ioctl+0xbc/0x7b0) [<c02470f0>] (ksys_ioctl) from [<c0101000>] (ret_fast_syscall+0x0/0x54) Exception stack(0xee8f3fa8 to 0xee8f3ff0) 3fa0: 00000005 adcbeb18 00000005 c06864a2 adcbeb18 00000001 3fc0: 00000005 adcbeb18 c06864a2 00000036 00000029 00000023 00000023 00000007 3fe0: b113b098 adcbeafc b1125413 b6155cf8
---[ end trace 2ad5ba954ceb767a ]---
drivers/gpu/drm/stm/ltdc.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 99bf93e8b36f..301de0498078 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -425,9 +425,12 @@ static void ltdc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { struct ltdc_device *ldev = crtc_to_ltdc(crtc);
struct drm_device *ddev = crtc->dev;
DRM_DEBUG_DRIVER("\n");
pm_runtime_get_sync(ddev->dev);
/* Sets the background color value */ reg_write(ldev->regs, LTDC_BCCR, BCCR_BCBLACK);
Hi Marek, Many thanks for your patch, Acked-by: Philippe Cornu philippe.cornu@st.com Tested-by: Yannick Fertre yannick.fertre@st.com
Hi Benjamin, Could you please apply "drm/stm: ltdc: remove call of pm-runtime functions" **before** this one. Thank you.
Philippe :-)
Le jeu. 2 juil. 2020 à 14:31, Philippe CORNU philippe.cornu@st.com a écrit :
On 2/29/20 11:16 PM, Marek Vasut wrote:
Add missing pm_runtime_get_sync() into ltdc_crtc_atomic_enable() to match pm_runtime_put_sync() in ltdc_crtc_atomic_disable(), otherwise the LTDC might suspend via runtime PM, disable clock, and then fail to resume later on.
The test which triggers it is roughly -- run qt5 application which uses eglfs platform and etnaviv, stop the application, sleep for 15 minutes, run the application again. This leads to a timeout waiting for vsync, because the LTDC has suspended, but did not resume.
Fixes: 35ab6cfbf211 ("drm/stm: support runtime power management") Signed-off-by: Marek Vasut marex@denx.de Cc: Yannick Fertré yannick.fertre@st.com Cc: Philippe Cornu philippe.cornu@st.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: Maxime Coquelin mcoquelin.stm32@gmail.com Cc: Alexandre Torgue alexandre.torgue@st.com To: dri-devel@lists.freedesktop.org Cc: linux-stm32@st-md-mailman.stormreply.com Cc: linux-arm-kernel@lists.infradead.org
------------[ cut here ]------------ WARNING: CPU: 0 PID: 297 at drivers/gpu/drm/drm_atomic_helper.c:1494 drm_atomic_helper_wait_for_vblanks+0x1dc/0x200 [CRTC:35:crtc-0] vblank wait timed out Modules linked in: CPU: 0 PID: 297 Comm: QSGRenderThread Not tainted 5.6.0-rc3-next-20200228-00010-g318bf0fc08ef #2 Hardware name: STM32 (Device Tree Support) [<c010f18c>] (unwind_backtrace) from [<c010afb8>] (show_stack+0x10/0x14) [<c010afb8>] (show_stack) from [<c07b1d3c>] (dump_stack+0xb4/0xd0) [<c07b1d3c>] (dump_stack) from [<c011d8b8>] (__warn+0xd4/0xf0) [<c011d8b8>] (__warn) from [<c011dc4c>] (warn_slowpath_fmt+0x78/0xa8) [<c011dc4c>] (warn_slowpath_fmt) from [<c04a266c>] (drm_atomic_helper_wait_for_vblanks+0x1dc/0x200) [<c04a266c>] (drm_atomic_helper_wait_for_vblanks) from [<c04a510c>] (drm_atomic_helper_commit_tail+0 x50/0x60) [<c04a510c>] (drm_atomic_helper_commit_tail) from [<c04a52a8>] (commit_tail+0x12c/0x13c) [<c04a52a8>] (commit_tail) from [<c04a53b4>] (drm_atomic_helper_commit+0xf4/0x100) [<c04a53b4>] (drm_atomic_helper_commit) from [<c04a2d38>] (drm_atomic_helper_set_config+0x58/0x6c) [<c04a2d38>] (drm_atomic_helper_set_config) from [<c04b1994>] (drm_mode_setcrtc+0x450/0x550) [<c04b1994>] (drm_mode_setcrtc) from [<c04ad570>] (drm_ioctl_kernel+0x90/0xe8) [<c04ad570>] (drm_ioctl_kernel) from [<c04ad8ac>] (drm_ioctl+0x2e4/0x32c) [<c04ad8ac>] (drm_ioctl) from [<c0246784>] (vfs_ioctl+0x20/0x38) [<c0246784>] (vfs_ioctl) from [<c02470f0>] (ksys_ioctl+0xbc/0x7b0) [<c02470f0>] (ksys_ioctl) from [<c0101000>] (ret_fast_syscall+0x0/0x54) Exception stack(0xee8f3fa8 to 0xee8f3ff0) 3fa0: 00000005 adcbeb18 00000005 c06864a2 adcbeb18 00000001 3fc0: 00000005 adcbeb18 c06864a2 00000036 00000029 00000023 00000023 00000007 3fe0: b113b098 adcbeafc b1125413 b6155cf8
---[ end trace 2ad5ba954ceb767a ]---
drivers/gpu/drm/stm/ltdc.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 99bf93e8b36f..301de0498078 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -425,9 +425,12 @@ static void ltdc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { struct ltdc_device *ldev = crtc_to_ltdc(crtc);
struct drm_device *ddev = crtc->dev; DRM_DEBUG_DRIVER("\n");pm_runtime_get_sync(ddev->dev);/* Sets the background color value */ reg_write(ldev->regs, LTDC_BCCR, BCCR_BCBLACK);Hi Marek, Many thanks for your patch, Acked-by: Philippe Cornu philippe.cornu@st.com Tested-by: Yannick Fertre yannick.fertre@st.com
Hi Benjamin, Could you please apply "drm/stm: ltdc: remove call of pm-runtime functions" **before** this one. Thank you.
Applied on drm-misc-next.
Thanks, Benjamin
Philippe :-)
dri-devel@lists.freedesktop.org
-
Benjamin Gaignard -
Marek Vasut -
Philippe CORNU -
Yannick FERTRE