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 <[email protected]>
>
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 <[email protected]>
>>> Sent: samedi 29 février 2020 23:17
>>> To: [email protected]
>>> Cc: Marek Vasut <[email protected]>; Yannick FERTRE <[email protected]>;
>>> Philippe CORNU <[email protected]>; Benjamin Gaignard
>>> <[email protected]>; Vincent ABRIOU <[email protected]>;
>>> Maxime Coquelin <[email protected]>; Alexandre TORGUE
>>> <[email protected]>; [email protected];
>>> [email protected]
>>> 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 <[email protected]>
>>> Cc: Yannick Fertré <[email protected]>
>>> Cc: Philippe Cornu <[email protected]>
>>> Cc: Benjamin Gaignard <[email protected]>
>>> Cc: Vincent Abriou <[email protected]>
>>> Cc: Maxime Coquelin <[email protected]>
>>> Cc: Alexandre Torgue <[email protected]>
>>> To: [email protected]
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> ---
>>> ------------[ 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
