Re: [PATCH i-g-t 1/1] tests/kms_async_flips: add async flips suspend resume tests

["Sharma, Swati2" <swati2.sharma@intel.com>]

Hi Santosh,

Please find my review comments below:

Fix subject,
s/tests/test

Also, cover-letter can be avoided for single patch.

On 06-Sep-24 12:58 PM, Santhosh Reddy Guddati wrote:
> ---

Commit message missing. Please add.

>   tests/kms_async_flips.c | 53 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 53 insertions(+)
> 
> diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
> index 5b70c4033..fd851397e 100644
> --- a/tests/kms_async_flips.c
> +++ b/tests/kms_async_flips.c
> @@ -66,6 +66,9 @@
>    *
>    * SUBTEST: invalid-async-flip
>    * Description: Negative case to verify if changes in fb are rejected from kernel as expected
> + *
> + * SUBTEST: async-flip-suspend-resume
> + * Description: Verify the async flip functionality with suspend and resume cycle
>    */
>   
>   #define CURSOR_POS 128
> @@ -672,6 +675,51 @@ static void run_test_with_modifiers(data_t *data, void (*test)(data_t *))
>   	}
>   }
>   
> +static void test_async_flip_suspend_resume(data_t *data)

Instead of adding new func(), is it possible to modify test_async_flip() 
to accommodate suspend resume api calls ?

> +{
> +	int ret, frame;
> +	long long int fps;
> +	struct timeval start, end, diff;
> +	int suspend_time = 1;
> +	int resume_time = 1;
> +
> +	igt_display_commit2(&data->display, data->display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> +
> +	gettimeofday(&start, NULL);
> +	frame = 1;
> +	do {
> +		int flags = DRM_MODE_PAGE_FLIP_ASYNC | DRM_MODE_PAGE_FLIP_EVENT;
> +
> +		ret = drmModePageFlip(data->drm_fd, data->crtc_id,
> +				      data->bufs[frame % NUM_FBS].fb_id,
> +				      flags, data);
> +		igt_assert_eq(ret, 0);
> +
> +		wait_flip_event(data);
> +
> +		gettimeofday(&end, NULL);
> +		timersub(&end, &start, &diff);
> +
> +		if (diff.tv_sec == suspend_time) {
> +			igt_info("system suspending\n");
> +			igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
> +			suspend_time = 0;
> +		}
> +
> +		if (diff.tv_sec == resume_time) {
> +			igt_info("system resuming\n");
> +			igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
> +			resume_time = 0;
> +		}
> +
> +		frame++;
> +	} while (diff.tv_sec < RUN_TIME);
> +
> +	fps = frame * 1000 / RUN_TIME;
> +	igt_assert_f((fps / 1000) > (data->refresh_rate * MIN_FLIPS_PER_FRAME),
> +		     "FPS should be significantly higher than the refresh rate\n");
> +}
> +
>   static data_t data;
>   
>   igt_main
> @@ -750,6 +798,11 @@ igt_main
>   		run_test(&data, test_crc);
>   	}
>   
> +	igt_describe("Verify the async flip functionality after suspend and resume cycle");
> +	igt_subtest_with_dynamic("async-flip-suspend-resume") {
> +		run_test(&data, test_async_flip_suspend_resume);
> +	}
> +
>   	igt_fixture {
>   		for (i = 0; i < NUM_FBS; i++)
>   			igt_remove_fb(data.drm_fd, &data.bufs[i]);