From: Marek Vasut <marex@denx.de> To: Yannick FERTRE <yannick.fertre@st.com>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>, Philippe CORNU <philippe.cornu@st.com>, Maxime Coquelin <mcoquelin.stm32@gmail.com>, Vincent ABRIOU <vincent.abriou@st.com>, "linux-stm32@st-md-mailman.stormreply.com" <linux-stm32@st-md-mailman.stormreply.com>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, Alexandre TORGUE <alexandre.torgue@st.com> Subject: Re: [PATCH] drm/stm: repair runtime power management Date: Mon, 9 Mar 2020 12:57:55 +0100 [thread overview] Message-ID: <64ea7f77-0a0b-ae3a-2911-5fdc8633255e@denx.de> (raw) In-Reply-To: <a30ad5a774004221903292871797607a@SFHDAG6NODE1.st.com> 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 > -- Best regards, Marek Vasut _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Marek Vasut <marex@denx.de> To: Yannick FERTRE <yannick.fertre@st.com>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org> Cc: Philippe CORNU <philippe.cornu@st.com>, Maxime Coquelin <mcoquelin.stm32@gmail.com>, Vincent ABRIOU <vincent.abriou@st.com>, "linux-stm32@st-md-mailman.stormreply.com" <linux-stm32@st-md-mailman.stormreply.com>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, Alexandre TORGUE <alexandre.torgue@st.com> Subject: Re: [PATCH] drm/stm: repair runtime power management Date: Mon, 9 Mar 2020 12:57:55 +0100 [thread overview] Message-ID: <64ea7f77-0a0b-ae3a-2911-5fdc8633255e@denx.de> (raw) In-Reply-To: <a30ad5a774004221903292871797607a@SFHDAG6NODE1.st.com> 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 > -- Best regards, Marek Vasut _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-03-09 12:29 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-02-29 22:16 [PATCH] drm/stm: repair runtime power management Marek Vasut 2020-02-29 22:16 ` Marek Vasut 2020-03-09 10:35 ` Yannick FERTRE 2020-03-09 10:35 ` Yannick FERTRE 2020-03-09 11:57 ` Marek Vasut [this message] 2020-03-09 11:57 ` Marek Vasut 2020-07-01 12:14 ` Yannick FERTRE 2020-07-01 12:14 ` Yannick FERTRE 2020-07-02 10:07 ` Philippe CORNU 2020-07-02 10:07 ` Philippe CORNU 2020-07-02 10:53 ` Marek Vasut 2020-07-02 10:53 ` Marek Vasut 2020-07-02 12:31 ` Philippe CORNU 2020-07-02 12:31 ` Philippe CORNU 2020-07-08 9:49 ` Benjamin Gaignard 2020-07-08 9:49 ` Benjamin Gaignard
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=64ea7f77-0a0b-ae3a-2911-5fdc8633255e@denx.de \ --to=marex@denx.de \ --cc=alexandre.torgue@st.com \ --cc=benjamin.gaignard@linaro.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-stm32@st-md-mailman.stormreply.com \ --cc=mcoquelin.stm32@gmail.com \ --cc=philippe.cornu@st.com \ --cc=vincent.abriou@st.com \ --cc=yannick.fertre@st.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.