diff mbox series

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

Message ID 20211112084302.2447931-1-yangcong5@huaqin.corp-partner.google.com (mailing list archive)
State New, archived
Headers show
Series drm/bridge: parade-ps8640: Fix additional suspend/resume at bootup | expand

Commit Message

cong yang Nov. 12, 2021, 8:43 a.m. UTC
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 <yangcong5@huaqin.corp-partner.google.com>
---
 drivers/gpu/drm/bridge/parade-ps8640.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Doug Anderson Nov. 12, 2021, 4:32 p.m. UTC | #1
Hi,

On Fri, Nov 12, 2021 at 12:43 AM yangcong
<yangcong5@huaqin.corp-partner.google.com> 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 <dianders@chromium.org>
Stephen Boyd Nov. 15, 2021, 11:44 p.m. UTC | #2
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 <yangcong5@huaqin.corp-partner.google.com>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Doug Anderson Nov. 17, 2021, 4:50 p.m. UTC | #3
Hi,

On Fri, Nov 12, 2021 at 8:32 AM Doug Anderson <dianders@google.com> wrote:
>
> Hi,
>
> On Fri, Nov 12, 2021 at 12:43 AM yangcong
> <yangcong5@huaqin.corp-partner.google.com> 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 <dianders@chromium.org>

Pushed to drm-misc-next:

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

Patch

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);