Re: [PATCH i-g-t v2] tests/kms_flip: Adjust tolerance when counting frames

[Jani Nikula <jani.nikula@intel.com>]

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);
>>>   	}
>>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx