On Thu, Jul 5, 2018 at 6:45 PM, Chris Wilson chris@chris-wilson.co.uk wrote:
Quoting Rodrigo Siqueira (2018-07-05 17:28:20)
Hi and thanks for all the feedback, I will work on the suggestions you sent, but I have some doubts:
On 07/05, Chris Wilson wrote:
Quoting Daniel Vetter (2018-07-05 09:20:13)
On Thu, Jul 05, 2018 at 12:48:43AM -0300, Rodrigo Siqueira wrote:
ktime_t current_timestamp;hrtimer_init(&out->vblank_hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);Can't we use absolute timer mode here? That avoids all the timestampt computations.
Where's your absolute timestamp being computed?
I did not understand the question. hrtimer_forward_now calculates the absolute timestamp from the relative value provided, this is what I am using. In any case, it would be easy to switch to absolute mode, but the code would not be smaller (or larger).
It was a question to Daniel, trying to illustrate that REL is just a convenience in the htrimer api over ABS.
Yeah I got confused and Chris corrected hat. _REL seems fine, just don't duplicate the timestamp computation. -Daniel
What's being done is recomputing what hrtimer already knows given a relative interval. output->expires should be equivalent to hrtimer->expires, and a lot of this code evaporates.
Indeed, output->expires can be removed; as for the rest of the code, that depends on the answer to question 2 below.
I have two questions:
- The timestamp that is returned to userspace is (A) the timestamp when the
interrupt was actually handled, allowing applications to detect when there is some irregularities in the interrupt handling timing, or (B) the timestamp when the current interrupt was *scheduled* to happen, allowing applications to detect overruns but not variations in the interrupt handling timing?
When the previous hrtimer actually occurred rolled back to the actual start of frame, i.e.
hrtimer_forward_now(hrtimer, vblank_interval); output->last_vblank_timestamp = ktime_sub(hrtimer->expires, vblank_interval);userspace is mainly oblivious to the delivery latency (although it can measure it as clock_gettim() - vblank->timestamp), but is concerned about knowing what the current and next vblank HW timing will be.
- If I use hrtimer with a period of 1s and return HRTIMER_RESTART, will I
be called back (A) 1s after the previous iteration was *scheduled to start* (i.e., I will actually be called back at regular intervals, so that after 1,000 iterations approximately 1,000s have elapsed) or (B) 1s after the previous iteration *ended* (i.e., I will be called back at intervals of 1s + the average processing time of the function, so that after 1,000 iterations significantly more than 1,000s have elapsed)?
No. When you get callback is controlled by the value you set in hrtimer->expires before you return RESTART. All RESTART does is immediately requeue the hrtimer, but more efficiently than calling hrtimer_start yourself. It is the call to hrtimer_forward that computes the next absolute hrtimer->expires based on the current time and your vblank interval.
The code I wrote assumes the answer to both questions is (B). If the answer to the second question is A, the code can indeed be made much simpler; if the answer to the first question is A, I have not been able to keep timing within the expected strict limits of the IGT test in a VM (maybe on physical hardware things would go better).
The code you wrote is actually A. hrtimer_forward() rolls the absolute timer forward such that its expiry is the next interval after now. It is while (hrtimer->expires < now) hrtimer->expires += interval; -Chris