| Message ID | 20221110145102.1.I51639dc112bbbe27259df6bdad56dbabd655d91a@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panel-edp: Use ktime_get_boottime for delays | expand |
Hi, On Thu, Nov 10, 2022 at 1:51 PM Drew Davenport <ddavenport@chromium.org> wrote: > > ktime_get is based on CLOCK_MONOTONIC which stops on suspend. On > suspend, the time that the panel was powerd off is recorded with > ktime_get, and on resume this time is compared to the current ktime_get > time to determine if the driver should wait for the panel to power down > completely before re-enabling it. > > Because we're using ktime_get, this delay doesn't account for the time > that the device is suspended, during which the power down delay may have > already elapsed. > > Change to use ktime_get_boottime throughout, which uses CLOCK_BOOTTIME > which does not stop when suspended. This ensures that the resume path > will not be delayed if the power off delay has already been met while > the device is suspended. > > Signed-off-by: Drew Davenport <ddavenport@chromium.org> > > --- > > drivers/gpu/drm/panel/panel-edp.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) Nice! Reviewed-by: Douglas Anderson <dianders@chromium.org> My plan will be to land this to drm-misc-next early next week (Tuesday maybe?) unless someone has any objections. BTW: any chance you'd be willing to post against two similar drivers: panel-simple.c and panel-samsung-atna33xc20.c? They have nearly the same code (and, yes, these drivers are purposely copies since there was overall consensus that having one giant panel driver to handle all possible panels was getting far too confusing) -Doug
Hi, On Fri, Nov 11, 2022 at 12:44 PM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Thu, Nov 10, 2022 at 1:51 PM Drew Davenport <ddavenport@chromium.org> wrote: > > > > ktime_get is based on CLOCK_MONOTONIC which stops on suspend. On > > suspend, the time that the panel was powerd off is recorded with > > ktime_get, and on resume this time is compared to the current ktime_get > > time to determine if the driver should wait for the panel to power down > > completely before re-enabling it. > > > > Because we're using ktime_get, this delay doesn't account for the time > > that the device is suspended, during which the power down delay may have > > already elapsed. > > > > Change to use ktime_get_boottime throughout, which uses CLOCK_BOOTTIME > > which does not stop when suspended. This ensures that the resume path > > will not be delayed if the power off delay has already been met while > > the device is suspended. > > > > Signed-off-by: Drew Davenport <ddavenport@chromium.org> > > > > --- > > > > drivers/gpu/drm/panel/panel-edp.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > Nice! > > Reviewed-by: Douglas Anderson <dianders@chromium.org> > > My plan will be to land this to drm-misc-next early next week (Tuesday > maybe?) unless someone has any objections. > > BTW: any chance you'd be willing to post against two similar drivers: > panel-simple.c and panel-samsung-atna33xc20.c? They have nearly the > same code (and, yes, these drivers are purposely copies since there > was overall consensus that having one giant panel driver to handle all > possible panels was getting far too confusing) Breadcrumbs: after discussion, this got vacuumed up into a larger series and is now at: https://lore.kernel.org/r/20221117133655.2.Iebd9f79aba0a62015fd2383fe6986c2d6fe12cfd@changeid
diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c index 5cb8dc2ebe184..a0a7ab35e08c9 100644 --- a/drivers/gpu/drm/panel/panel-edp.c +++ b/drivers/gpu/drm/panel/panel-edp.c @@ -351,7 +351,7 @@ static void panel_edp_wait(ktime_t start_ktime, unsigned int min_ms) return; min_ktime = ktime_add(start_ktime, ms_to_ktime(min_ms)); - now_ktime = ktime_get(); + now_ktime = ktime_get_boottime(); if (ktime_before(now_ktime, min_ktime)) msleep(ktime_to_ms(ktime_sub(min_ktime, now_ktime)) + 1); @@ -378,7 +378,7 @@ static int panel_edp_suspend(struct device *dev) gpiod_set_value_cansleep(p->enable_gpio, 0); regulator_disable(p->supply); - p->unprepared_time = ktime_get(); + p->unprepared_time = ktime_get_boottime(); return 0; } @@ -464,14 +464,14 @@ static int panel_edp_prepare_once(struct panel_edp *p) } } - p->prepared_time = ktime_get(); + p->prepared_time = ktime_get_boottime(); return 0; error: gpiod_set_value_cansleep(p->enable_gpio, 0); regulator_disable(p->supply); - p->unprepared_time = ktime_get(); + p->unprepared_time = ktime_get_boottime(); return err; }
ktime_get is based on CLOCK_MONOTONIC which stops on suspend. On suspend, the time that the panel was powerd off is recorded with ktime_get, and on resume this time is compared to the current ktime_get time to determine if the driver should wait for the panel to power down completely before re-enabling it. Because we're using ktime_get, this delay doesn't account for the time that the device is suspended, during which the power down delay may have already elapsed. Change to use ktime_get_boottime throughout, which uses CLOCK_BOOTTIME which does not stop when suspended. This ensures that the resume path will not be delayed if the power off delay has already been met while the device is suspended. Signed-off-by: Drew Davenport <ddavenport@chromium.org> --- drivers/gpu/drm/panel/panel-edp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)