On Wed, 29 Jan 2014, Imre Deak <imre.deak@xxxxxxxxx> wrote:
> schedule_timeout_uninterruptible() takes jiffies not ms.
>
> Also we should check whether jiffies has overflowed since the timestamp
> for event A was taken. This is highly unlikely on 64 bit, but on 32 bit
> machines jiffies initially is -300*HZ. If the panel power is initially
> off the first wait from edp_panel_vdd_on()->wait_panel_power_cycle()
> will result in a call timestamp_jiffies of 0, so on 32 bit machines we
> would wait ~300 sec + to_wait_ms. Fix this by checking if the initial
> timestamp is not in the future.
>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3673ba1..6a80393 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2643,12 +2643,13 @@ timespec_to_jiffies_timeout(const struct timespec *value)
> * doesn't happen exactly after event A, you record the timestamp (jiffies) of
> * when event A happened, then just before event B you call this function and
> * pass the timestamp as the first argument, and X as the second argument.
> + * Note that the recorded timestamp (timestamp_jiffies) can't be in the future
> + * otherwise the function won't wait at all.
> */
> static inline void
> wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
> {
> - unsigned long target_jiffies, tmp_jiffies;
> - unsigned int remaining_ms;
> + unsigned long target_jiffies, tmp_jiffies, remaining_jiffies;
>
> /*
> * Don't re-read the value of "jiffies" every time since it may change
> @@ -2658,12 +2659,12 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
> target_jiffies = timestamp_jiffies +
> msecs_to_jiffies_timeout(to_wait_ms);
>
> - if (time_after(target_jiffies, tmp_jiffies)) {
> - remaining_ms = jiffies_to_msecs((long)target_jiffies -
> - (long)tmp_jiffies);
> - while (remaining_ms)
> - remaining_ms =
> - schedule_timeout_uninterruptible(remaining_ms);
> + if (time_after(target_jiffies, tmp_jiffies) &&
> + time_before_eq(timestamp_jiffies, tmp_jiffies)) {
> + remaining_jiffies = target_jiffies - tmp_jiffies;
> + while (remaining_jiffies)
> + remaining_jiffies =
> + schedule_timeout_uninterruptible(remaining_jiffies);
> }
> }
For the record, I spotted the jiffies vs. ms mistake in review [1],
Paulo posted v5 [2], but apparently Daniel applied v4 anyway:
commit dce56b3c626fb1d533258a624d42a1a3fc17da17
Author: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
Date: Thu Dec 19 14:29:40 2013 -0200
drm/i915: save some time when waiting the eDP timings
Wrap around was also discussed.
BR,
Jani.
[1] http://mid.gmane.org/87fvpnkgyg.fsf@xxxxxxxxx
[2] http://mid.gmane.org/1388778311-2020-1-git-send-email-przanoni@xxxxxxxxx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
