2021-11-12 08:43:24

by cong yang

[permalink] [raw]
Subject: [PATCH] drm/bridge: parade-ps8640: Fix additional suspend/resume at bootup

Through log and waveform, we can see that there will be additional
suspend/resume when booting. This timing does not meet the ps8640 spec.
It seems that the delay of 500ms does not satisfied drm_panel_get_modes.
I increased it to 900ms and it seems that this problem can be solved.
To be safe, I'd just round up to a full 1000.

Signed-off-by: yangcong <[email protected]>
---
drivers/gpu/drm/bridge/parade-ps8640.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
index 0c7aab42b04f..0749fa628bfb 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -635,13 +635,13 @@ static int ps8640_probe(struct i2c_client *client)
pm_runtime_enable(dev);
/*
* Powering on ps8640 takes ~300ms. To avoid wasting time on power
- * cycling ps8640 too often, set autosuspend_delay to 500ms to ensure
+ * cycling ps8640 too often, set autosuspend_delay to 1000ms to ensure
* the bridge wouldn't suspend in between each _aux_transfer_msg() call
* during EDID read (~20ms in my experiment) and in between the last
* _aux_transfer_msg() call during EDID read and the _pre_enable() call
* (~100ms in my experiment).
*/
- pm_runtime_set_autosuspend_delay(dev, 500);
+ pm_runtime_set_autosuspend_delay(dev, 1000);
pm_runtime_use_autosuspend(dev);
pm_suspend_ignore_children(dev, true);
ret = devm_add_action_or_reset(dev, ps8640_runtime_disable, dev);
--
2.25.1



2021-11-12 16:32:19

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH] drm/bridge: parade-ps8640: Fix additional suspend/resume at bootup

Hi,

On Fri, Nov 12, 2021 at 12:43 AM yangcong
<[email protected]> wrote:
>
> Through log and waveform, we can see that there will be additional
> suspend/resume when booting. This timing does not meet the ps8640 spec.
> It seems that the delay of 500ms does not satisfied drm_panel_get_modes.
> I increased it to 900ms and it seems that this problem can be solved.
> To be safe, I'd just round up to a full 1000.

Do be clear: I'm still not convinced that the old 500 ms actually
causes any real problems. I think someone is just measuring with a
scope and upset that they see the device power on and then power off
again. In any case, if we can avoid an extra power cycle at boot then
that seems sane to me. Since this is a tiny change, I'll plan to merge
it some time next week unless someone yells.

Reviewed-by: Douglas Anderson <[email protected]>

2021-11-16 03:14:10

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH] drm/bridge: parade-ps8640: Fix additional suspend/resume at bootup

Quoting yangcong (2021-11-12 00:43:02)
> Through log and waveform, we can see that there will be additional
> suspend/resume when booting. This timing does not meet the ps8640 spec.
> It seems that the delay of 500ms does not satisfied drm_panel_get_modes.
> I increased it to 900ms and it seems that this problem can be solved.
> To be safe, I'd just round up to a full 1000.
>
> Signed-off-by: yangcong <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2021-11-17 16:50:54

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH] drm/bridge: parade-ps8640: Fix additional suspend/resume at bootup

Hi,

On Fri, Nov 12, 2021 at 8:32 AM Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Fri, Nov 12, 2021 at 12:43 AM yangcong
> <[email protected]> wrote:
> >
> > Through log and waveform, we can see that there will be additional
> > suspend/resume when booting. This timing does not meet the ps8640 spec.
> > It seems that the delay of 500ms does not satisfied drm_panel_get_modes.
> > I increased it to 900ms and it seems that this problem can be solved.
> > To be safe, I'd just round up to a full 1000.
>
> Do be clear: I'm still not convinced that the old 500 ms actually
> causes any real problems. I think someone is just measuring with a
> scope and upset that they see the device power on and then power off
> again. In any case, if we can avoid an extra power cycle at boot then
> that seems sane to me. Since this is a tiny change, I'll plan to merge
> it some time next week unless someone yells.
>
> Reviewed-by: Douglas Anderson <[email protected]>

Pushed to drm-misc-next:

aa70a0996b0e drm/bridge: parade-ps8640: Fix additional suspend/resume at bootup