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

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

[Gabriel Feceoru]

Comparing 2 numbers with 1% accuracy depends on which one is the
reference. If count == 100 and expected == 99 this condition fails,
although it should pass.

Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com>
---
 tests/kms_flip.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index eda2fcc..938b32d 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -1187,7 +1187,8 @@ 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 * 99/100 && count <= expected * 101/100) ||
+			     (expected >= count * 99/100 && expected <= count * 101/100),
 			     "dropped frames, expected %d, counted %d, encoder type %d\n",
 			     expected, count, o->kencoder[0]->encoder_type);
 	}
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

On Tue, 10 May 2016, Gabriel Feceoru <gabriel.feceoru@intel.com> wrote:
> Comparing 2 numbers with 1% accuracy depends on which one is the
> reference. If count == 100 and expected == 99 this condition fails,
> although it should pass.

Well, the expectation should be the reference. If you expect 50 at 50%
tolerance, 25..75 is okay. 100 is clearly out of tolerance, but your
method would accept it too.

Would it help to round the lower limit down and upper limit up? I think
that would be more acceptable.

BR,
Jani.

> Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com>
> ---
>  tests/kms_flip.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index eda2fcc..938b32d 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -1187,7 +1187,8 @@ 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 * 99/100 && count <= expected * 101/100) ||
> +			     (expected >= count * 99/100 && expected <= count * 101/100),
>  			     "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

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

[Gabriel Feceoru]

On 10.05.2016 16:52, Jani Nikula wrote:
> On Tue, 10 May 2016, Gabriel Feceoru <gabriel.feceoru@intel.com> wrote:
>> Comparing 2 numbers with 1% accuracy depends on which one is the
>> reference. If count == 100 and expected == 99 this condition fails,
>> although it should pass.
>
> Well, the expectation should be the reference. If you expect 50 at 50%
> tolerance, 25..75 is okay. 100 is clearly out of tolerance, but your
> method would accept it too.
>
> Would it help to round the lower limit down and upper limit up? I think
> that would be more acceptable.

Yes, you are right. I'll adjust it to 98/100 and 102/100.

Thanks,
Gabriel.

>
> BR,
> Jani.
>
>> Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com>
>> ---
>>   tests/kms_flip.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
>> index eda2fcc..938b32d 100644
>> --- a/tests/kms_flip.c
>> +++ b/tests/kms_flip.c
>> @@ -1187,7 +1187,8 @@ 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 * 99/100 && count <= expected * 101/100) ||
>> +			     (expected >= count * 99/100 && expected <= count * 101/100),
>>   			     "dropped frames, expected %d, counted %d, encoder type %d\n",
>>   			     expected, count, o->kencoder[0]->encoder_type);
>>   	}
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Gabriel Feceoru]

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,
 			     "dropped frames, expected %d, counted %d, encoder type %d\n",
 			     expected, count, o->kencoder[0]->encoder_type);
 	}
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

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.


>  			     "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

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

[Gabriel Feceoru]

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, 
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);
>>   	}
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

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

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

[Gabriel Feceoru]

basic-flip-vs-wf_vblank subtest sometimes fails asserting counted frames to
be aproximately equal with the estimated number.

This is a false negative, one of the reasons being the precision lost when
truncating a fractional number.

Fixed this by using floating point arithmetic.

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 | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index eda2fcc..6ec97d0 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, encoder type %d\n",
 			     expected, count, o->kencoder[0]->encoder_type);
 	}
 }
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

On Fri, May 13, 2016 at 02:45:09PM +0300, Gabriel Feceoru wrote:
> basic-flip-vs-wf_vblank subtest sometimes fails asserting counted frames to
> be aproximately equal with the estimated number.
> 
> This is a false negative, one of the reasons being the precision lost when
> truncating a fractional number.
> 
> Fixed this by using floating point arithmetic.
> 
> 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 | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index eda2fcc..6ec97d0 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);

int expected = count * frame_time(o);
igt_assert_f(100 * expected >= elasped * 99 && 100 * count <= expected * 101,
	     "dropped frames, expected %d, counted %d, encoder type %d\n",
	     elapsed / frame_time(o), count, o->kencoder[0]->encoder_type);

if I understood your concern correctly, as the only loss of precison
there would be from calculating 'expected'.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

On Fri, May 13, 2016 at 01:00:33PM +0100, Chris Wilson wrote:
> On Fri, May 13, 2016 at 02:45:09PM +0300, Gabriel Feceoru wrote:
> > basic-flip-vs-wf_vblank subtest sometimes fails asserting counted frames to
> > be aproximately equal with the estimated number.
> > 
> > This is a false negative, one of the reasons being the precision lost when
> > truncating a fractional number.
> > 
> > Fixed this by using floating point arithmetic.
> > 
> > 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 | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> > index eda2fcc..6ec97d0 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);
> 
> int expected = count * frame_time(o);
> igt_assert_f(100 * expected >= elasped * 99 && 100 * count <= expected * 101,

igt_assert_f(100 * expected >= elasped * 99 && 100 * expected <= elasped * 101,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Gabriel Feceoru]

On 13.05.2016 15:00, Chris Wilson wrote:
> On Fri, May 13, 2016 at 02:45:09PM +0300, Gabriel Feceoru wrote:
>> basic-flip-vs-wf_vblank subtest sometimes fails asserting counted frames to
>> be aproximately equal with the estimated number.
>>
>> This is a false negative, one of the reasons being the precision lost when
>> truncating a fractional number.
>>
>> Fixed this by using floating point arithmetic.
>>
>> 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 | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
>> index eda2fcc..6ec97d0 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);
>
> int expected = count * frame_time(o);
> igt_assert_f(100 * expected >= elasped * 99 && 100 * count <= expected * 101,
> 	     "dropped frames, expected %d, counted %d, encoder type %d\n",
> 	     elapsed / frame_time(o), count, o->kencoder[0]->encoder_type);
>
> if I understood your concern correctly, as the only loss of precison
> there would be from calculating 'expected'.

That would also do it, but with care where to use the cast:

int expected =  (int)((double)count * frame_time(o));
vs
int expected = count * (int)frame_time(o);

Gabriel.

> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

On Tue, May 10, 2016 at 05:15:37PM +0300, Gabriel Feceoru wrote:
> 
> 
> On 10.05.2016 16:52, Jani Nikula wrote:
> >On Tue, 10 May 2016, Gabriel Feceoru <gabriel.feceoru@intel.com> wrote:
> >>Comparing 2 numbers with 1% accuracy depends on which one is the
> >>reference. If count == 100 and expected == 99 this condition fails,
> >>although it should pass.
> >
> >Well, the expectation should be the reference. If you expect 50 at 50%
> >tolerance, 25..75 is okay. 100 is clearly out of tolerance, but your
> >method would accept it too.
> >
> >Would it help to round the lower limit down and upper limit up? I think
> >that would be more acceptable.
> 
> Yes, you are right. I'll adjust it to 98/100 and 102/100.

Maybe also extract the computation for lower/upper limit into local
variables, to make the code less dense and easier to understand.
-Daniel

> 
> Thanks,
> Gabriel.
> 
> >
> >BR,
> >Jani.
> >
> >>Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com>
> >>---
> >>  tests/kms_flip.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> >>index eda2fcc..938b32d 100644
> >>--- a/tests/kms_flip.c
> >>+++ b/tests/kms_flip.c
> >>@@ -1187,7 +1187,8 @@ 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 * 99/100 && count <= expected * 101/100) ||
> >>+			     (expected >= count * 99/100 && expected <= count * 101/100),
> >>  			     "dropped frames, expected %d, counted %d, encoder type %d\n",
> >>  			     expected, count, o->kencoder[0]->encoder_type);
> >>  	}
> >

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

On Tue, May 17, 2016 at 09:30:59AM +0200, Daniel Vetter wrote:
> On Tue, May 10, 2016 at 05:15:37PM +0300, Gabriel Feceoru wrote:
> > 
> > 
> > On 10.05.2016 16:52, Jani Nikula wrote:
> > >On Tue, 10 May 2016, Gabriel Feceoru <gabriel.feceoru@intel.com> wrote:
> > >>Comparing 2 numbers with 1% accuracy depends on which one is the
> > >>reference. If count == 100 and expected == 99 this condition fails,
> > >>although it should pass.
> > >
> > >Well, the expectation should be the reference. If you expect 50 at 50%
> > >tolerance, 25..75 is okay. 100 is clearly out of tolerance, but your
> > >method would accept it too.
> > >
> > >Would it help to round the lower limit down and upper limit up? I think
> > >that would be more acceptable.
> > 
> > Yes, you are right. I'll adjust it to 98/100 and 102/100.
> 
> Maybe also extract the computation for lower/upper limit into local
> variables, to make the code less dense and easier to understand.

Just because I have this lying around (untested):

diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index eda2fcc..22b9c9d 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -1182,14 +1182,14 @@ 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;
-               int count = es->count;
+               int count = es->count * o->seq_step;
+               unsigned int min = frame_time(o) * (count - 1);
+               unsigned int max = frame_time(o) * (count + 1);
 
-               count *= o->seq_step;
-               expected = elapsed / frame_time(o);
-               igt_assert_f(count >= expected * 99/100 && count <= expected * 101/100,
+               igt_assert_f(elapsed > min && elapsed < max,
                             "dropped frames, expected %d, counted %d, encoder type %d\n",
-                            expected, count, o->kencoder[0]->encoder_type);
+                            elapsed / frame_time(o), count,
+                            o->kencoder[0]->encoder_type);
        }
 }

-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx