Re: [PATCH 3/6] drm/i915: reset eDP timestamps on resume

Daniel Vetter
2014-01-17T21:34:51Z

On Fri, Jan 17, 2014 at 10:21 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Fri, Jan 17, 2014 at 07:11:14PM -0200, Paulo Zanoni wrote:
>> 2014/1/17 Chris Wilson <chris@chris-wilson.co.uk>:
>> > On Fri, Jan 17, 2014 at 06:17:42PM -0200, Paulo Zanoni wrote:
>> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> >>
>> >> The eDP code records a few timestamps containing the last time we took
>> >> some actions, because we need to wait before doing some other actions.
>> >> The problem is that if we store a timestamp when suspending and then
>> >> look at it when resuming, we'll ignore the unknown amount of time we
>> >> actually were suspended.
>> >>
>> >> This happens with the panel power cycle delay: it's 500ms on my
>> >> machine, and it's delaying the resume sequence by 200ms due to a
>> >> timestamp we recorded before suspending. This patch should solve this
>> >> problem by resetting the timestamps.
>> >
>> > But you don't explain why this is safe. The code nerfs the timeouts so
>> > that they are ignored, yet the delays are independent. Should this be
>> > based on realtime rather than jiffies?
>>
>> I'm not sure I understand your question. What's the problem you see exactly?
>
> Given the fast suspend & resume, we will not have waited the required
> panel off time before poking it again etc. What makes that safe?

Even worse the kernel might abort the suspend due to some issue and
we'll immediately resume. Also, and immediate thaw operation after
freezing is how hibernate works. Iirc the hw always enforces the full
power off delay after a power reset for exactly this reason (at least
on current platforms afaik). But with the minimal delays in patch 6
that won't help any more, either.

I fear we need a bit more smarts here using a realtime clock source to
figure out whether actually sufficient time elapsed :(
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch