| Message ID | 573326F7.7030908@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 11 May 2016, Gabriel Feceoru <gabriel.feceoru@intel.com> wrote: > On 10.05.2016 18:39, Jani Nikula wrote: >> On Tue, 10 May 2016, Gabriel Feceoru <gabriel.feceoru@intel.com> wrote: >>> If count == 100 and expected == 99 this condition fails (99*101/100 = 99.99). >>> >>> (v2): Increased the tolerance range, as suggested by Jani. >>> >>> Cc: Jani Nikula <jani.nikula@intel.com> >>> Cc: Daniel Vetter <daniel.vetter@intel.com> >>> Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com> >>> --- >>> tests/kms_flip.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tests/kms_flip.c b/tests/kms_flip.c >>> index eda2fcc..ceb0e0b 100644 >>> --- a/tests/kms_flip.c >>> +++ b/tests/kms_flip.c >>> @@ -1187,7 +1187,7 @@ static void check_final_state(struct test_output *o, struct event_state *es, >>> >>> count *= o->seq_step; >>> expected = elapsed / frame_time(o); >>> - igt_assert_f(count >= expected * 99/100 && count <= expected * 101/100, >>> + igt_assert_f(count >= expected * 98/100 && count <= expected * 102/100, >> >> I was thinking of >> >> #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d)) >> >> igt_assert_f(count >= expected * 99 / 100 && >> count <= DIV_ROUND_UP(expected * 101, 100)); >> >> but maybe someone who knows how accurate this should really be could >> chime in. >> >> BR, >> Jani. > > I've just realized that frame_time(0) is a float, so converting to int > in that division will lose precision. > > Proposing this now: > > diff --git a/tests/kms_flip.c b/tests/kms_flip.c > index eda2fcc..c2d3929 100644 > --- a/tests/kms_flip.c > +++ b/tests/kms_flip.c > @@ -1182,13 +1182,13 @@ static void check_final_state(struct test_output > *o, struct event_state *es, > /* Verify we drop no frames, but only if it's not a TV encoder, > since > * those use some funny fake timings behind userspace's back. */ > if (o->flags & TEST_CHECK_TS && !analog_tv_connector(o)) { > - int expected; > + double expected; > int count = es->count; > > count *= o->seq_step; > - expected = elapsed / frame_time(o); > - igt_assert_f(count >= expected * 99/100 && count <= > expected * 101/100, > - "dropped frames, expected %d, counted %d, > encoder type %d\n", > + expected = (double)elapsed / frame_time(o); > + igt_assert_f(fabs((double)count - expected)/expected < 0.01, > + "dropped frames, expected %f, counted %d, Heh, I've been working on the kernel so long that I was trying to avoid floats in my suggestion above. This seems fine to me, except the condition should be <= not < I guess. Please send a proper patch. BR, Jani. > encoder type %d\n", > expected, count, > o->kencoder[0]->encoder_type); > } > } > > Regards, > Gabriel. > >> >> >>> "dropped frames, expected %d, counted %d, encoder type %d\n", >>> expected, count, o->kencoder[0]->encoder_type); >>> } >>
diff --git a/tests/kms_flip.c b/tests/kms_flip.c index eda2fcc..c2d3929 100644 --- a/tests/kms_flip.c +++ b/tests/kms_flip.c @@ -1182,13 +1182,13 @@ static void check_final_state(struct test_output *o, struct event_state *es, /* Verify we drop no frames, but only if it's not a TV encoder, since * those use some funny fake timings behind userspace's back. */ if (o->flags & TEST_CHECK_TS && !analog_tv_connector(o)) { - int expected; + double expected; int count = es->count; count *= o->seq_step; - expected = elapsed / frame_time(o); - igt_assert_f(count >= expected * 99/100 && count <= expected * 101/100, - "dropped frames, expected %d, counted %d, encoder type %d\n", + expected = (double)elapsed / frame_time(o); + igt_assert_f(fabs((double)count - expected)/expected < 0.01,