On 16.09.15 14:39, Savolainen, Petri (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: EXT Ivan Khoronzhuk [mailto:[email protected]]
Sent: Wednesday, September 16, 2015 1:47 PM
To: [email protected]; Savolainen, Petri (Nokia - FI/Espoo)
Subject: Re: [lng-odp] [PATCH v2 3/5] linux-generic: odp_time: reutrn 0
if t2 = t1 instead of MAX for diff
On 16.09.15 13:23, Ivan Khoronzhuk wrote:
Petri,
What about this fix? It's similar to to CPU API.
On 11.09.15 13:04, Ivan Khoronzhuk wrote:
It's better to describe by example:
cur = 15;
start = 15;
diff = 2;
while (odp_time_cycles_diff(start, cur) < diff) {
cur = odp_time_cycles();
}
This example has to work. It's possible only when t2 - t1 = 0 if t2
= t1.
The validation test on it will be added later.
Signed-off-by: Ivan Khoronzhuk <[email protected]>
---
platform/linux-generic/odp_time.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/platform/linux-generic/odp_time.c b/platform/linux-
generic/odp_time.c
index a08833d..a007d69 100644
--- a/platform/linux-generic/odp_time.c
+++ b/platform/linux-generic/odp_time.c
@@ -14,7 +14,7 @@
uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2)
{
- if (odp_likely(t2 > t1))
+ if (odp_likely(t2 >= t1))
return t2 - t1;
return t2 + (UINT64_MAX - t1);
But I have additional proposition. Maybe I'm wrong, but
one cycle can be lost here (equal as in CPU api, I'm ready to fix it
also)
For instance:
start = MAX - 2;
cur = 1
res = MAX - MAX + 2 + 1 = 3;
It's correct. But in real it will be:
(MAX - 2)
-> 1 cycle
(MAX - 1)
-> 2 cycle
MAX
-> 3 cycle
0
-> 4 cycle
1
The function returns 3 cycles difference,
but due to 0, physically, timer counts 4 cycles.
Not sure, but I should send +1 patch that corrects it to:
return t2 + (UINT64_MAX - t1) + 1;
due to counter in continuous mode is reset to 0, then continues
counting.
Can we apply this on cycle counter? (then I need correct CPU API
implementation also)
Is it reseted to zero or wraps to 1 for all arches?
For instance, from here
http://download.intel.com/design/intelxscale/27347302.pdf,
(Intel XScale(r) Core) CCNT behaves like:
"When CCNT reaches its maximum value 0xFFFF,FFFF, the next clock
cycle will cause it to roll over to zero"
But I'm not sure about other arches hidden under linux-generic.
I tend to believe that it's applicable for all cases.
--
Regards,
Ivan Khoronzhuk
uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2)
{
if (odp_unlikely(t2 == t1))
return 0;
if (odp_likely(t2 > t1))
return t2 - t1;
Sorry, but why not
if (odp_likely(t2 >= t1))
return t2 - t1;
as in patch.
return t2 + (UINT64_MAX - t1) + 1;
}
t1 = MAX
t2 = MAX - 1, diff = MAX - 1 + MAX - MAX + 1 = MAX
True, as we count +1 cycle in real.
Ok.
t2 = MAX, diff = 0
True.
t2 = 0, diff = 0 + MAX - MAX + 1 = 1
Also true. As for count from MAX to 0 we need 1 cycle.
Ok.
t1 = MAX - 1
t2 = MAX - 2, diff = MAX - 2 + MAX - MAX + 1 + 1 = MAX
True and OK.
t2 = MAX - 1, diff = 0
True and OK.
t2 = MAX, diff = MAX - MAX + 1 = 1
True and OK.
t2 = 0, diff = 0 + MAX - MAX + 1 + 1 = 2
True, as we need 2 cycles to reach 0.
Ok.
t2 = 1, diff = 1 + MAX - MAX + 1 + 1 = 3
True, as we need 3 cycles to reach 1.
Ok.
It's very likely that when t1 == t2, the correct result is 0.
Yep, I'm not proposing to change this patch. I propose to send additional patch
later.
It will be rather series, as I want to correct cpu API also.
Let's catch that first. Wrap around case can have then the extra +1.
Right, that is I worry about. But seems on current arches it's applicable.
Also we can limit it to arches we are sure, as -1 cycle it's obvious error now
and
it adds additional error in accuracy +-2 cycles instead of 1. In case of bad
cycle counter
resolution, say 64, it can have valuable impact, especially if it's multiplied
on some value.
-Petri
--
Regards,
Ivan Khoronzhuk
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp