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.fer...@st.com> > >> Best regards >> >> Yannick Fertré >> >> >> -----Original Message----- >> From: Marek Vasut <ma...@denx.de> >> Sent: samedi 29 février 2020 23:17 >> To: dri-devel@lists.freedesktop.org >> Cc: Marek Vasut <ma...@denx.de>; Yannick FERTRE <yannick.fer...@st.com>; >> Philippe CORNU <philippe.co...@st.com>; Benjamin Gaignard >> <benjamin.gaign...@linaro.org>; Vincent ABRIOU <vincent.abr...@st.com>; >> Maxime Coquelin <mcoquelin.st...@gmail.com>; Alexandre TORGUE >> <alexandre.tor...@st.com>; linux-st...@st-md-mailman.stormreply.com; >> linux-arm-ker...@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 <ma...@denx.de> >> Cc: Yannick Fertré <yannick.fer...@st.com> >> Cc: Philippe Cornu <philippe.co...@st.com> >> Cc: Benjamin Gaignard <benjamin.gaign...@linaro.org> >> Cc: Vincent Abriou <vincent.abr...@st.com> >> Cc: Maxime Coquelin <mcoquelin.st...@gmail.com> >> Cc: Alexandre Torgue <alexandre.tor...@st.com> >> To: dri-devel@lists.freedesktop.org >> Cc: linux-st...@st-md-mailman.stormreply.com >> Cc: linux-arm-ker...@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 >> > > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel