[PATCH i-g-t v3 4/5] tests/kms_flip: Track and report unexpected events with pass rate metric

[Karthik B S]

Hi,

On 6/16/2025 12:08 PM, Naladala Ramanaidu wrote:
> At higher refresh rates, There is a possibility of flips being

Nit: 'There' -> 'there'

Other than this, the overall patch LGTM.

I would prefer having this patch combined with patch 2.

Thanks,
Karthik.B.S

> submitted during the evasion window, which can result in one extra
> frame and cause the test to fail. To address this, the test has
> been modified to calculate a pass rate based on the number of
> expected versus erroneous frames. A threshold of 85% is enforced
> to determine test success.
>
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala at intel.com>
> ---
>   tests/kms_flip.c | 26 ++++++++++++++------------
>   1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index 5873591fa..5ac0e2c4d 100755
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -309,6 +309,7 @@ struct event_state {
>   	unsigned int current_seq;		/* kernel reported seq. num */
>   
>   	int count;				/* # of events of this type */
> +	int err_frames;				/* # of unexpected events */
>   
>   	/* Step between the current and next 'target' sequence number. */
>   	int seq_step;
> @@ -677,7 +678,7 @@ static void vblank_handler(int fd, unsigned int frame, unsigned int sec,
>   	fixup_premature_vblank_ts(o, &o->vblank_state);
>   }
>   
> -static bool check_state(const struct test_output *o, const struct event_state *es)
> +static bool check_state(const struct test_output *o, struct event_state *es)
>   {
>   	struct timeval diff;
>   
> @@ -706,6 +707,7 @@ static bool check_state(const struct test_output *o, const struct event_state *e
>   	    es->current_seq - (es->last_seq + o->seq_step) > 1UL << 23) {
>   		igt_debug("unexpected %s seq %u, should be >= %u\n",
>   			  es->name, es->current_seq, es->last_seq + o->seq_step);
> +		es->err_frames++;
>   		return true;
>   	}
>   
> @@ -735,7 +737,7 @@ static bool check_state(const struct test_output *o, const struct event_state *e
>   			igt_debug("unexpected %s seq %u, expected %u\n",
>   				  es->name, es->current_seq,
>   				  es->last_seq + o->seq_step);
> -
> +			es->err_frames++;
>   			return true;
>   		}
>   	}
> @@ -1233,16 +1235,16 @@ static bool check_final_state(const struct test_output *o,
>   	 * those use some funny fake timings behind userspace's back. */
>   	if (o->flags & TEST_CHECK_TS) {
>   		int count = es->count * o->seq_step;
> -		unsigned int min = actual_frame_time(o) * (count - 1);
> -		unsigned int max = actual_frame_time(o) * (count + 1);
> -
> -		igt_debug("expected %d, counted %d, encoder type %d\n",
> -			  (int)(elapsed / actual_frame_time(o)), count,
> -			  o->kencoder[0]->encoder_type);
> -		if (elapsed < min || elapsed > max) {
> -			igt_debug("dropped frames, expected %d, counted %d, encoder type %d\n",
> -				  (int)(elapsed / actual_frame_time(o)), count,
> -				  o->kencoder[0]->encoder_type);
> +		int error_count = es->err_frames * o->seq_step;
> +		int expected = elapsed / actual_frame_time(o);
> +		float pass_rate = ((float)(count - error_count) / count) * 100;
> +
> +		igt_info("Event %s: expected %d, counted %d, passrate = %.2f%%, encoder type %d\n",
> +			  es->name, expected, count, pass_rate, o->kencoder[0]->encoder_type);
> +
> +		if (pass_rate < 85) {
> +			igt_debug("dropped frames, expected %d, counted %d, passrate = %.2f%%, encoder type %d\n",
> +				  expected, count, pass_rate, o->kencoder[0]->encoder_type);
>   
>   			return false;
>   		}