Re: [PATCH i-g-t 4/4] tests/intel/kms_pm_dc: Add new test for dc3co framedrop validation

["Thasleem, Mohammed" <mohammed.thasleem@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 5259 bytes --]


On 04-03-2026 10:08 am, Jeevan B wrote:
> Add a new subtest to validate that no frame drops occur during
> DC3CO entry, ensuring that no frame drops are detected and DC3CO
> is successfully triggered during the test.
>
> Signed-off-by: Jeevan B<jeevan.b@intel.com>
> ---
>   tests/intel/kms_pm_dc.c | 83 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 83 insertions(+)
>
> diff --git a/tests/intel/kms_pm_dc.c b/tests/intel/kms_pm_dc.c
> index 10450474a..b0e35dca1 100644
> --- a/tests/intel/kms_pm_dc.c
> +++ b/tests/intel/kms_pm_dc.c
> @@ -51,6 +51,10 @@
>    * Description: Make sure that system enters DC3CO when PSR2 is active and system
>    *              is in SLEEP state
>    *
> + * SUBTEST: dc3co-framedrop-check
> + * Description: Verify that DC3CO entry does not cause frame drops and successfully
> + * 		enters the power state
> + *
>    * SUBTEST: dc5-dpms
>    * Description: Validate display engine entry to DC5 state while all connectors's
>    *              DPMS property set to OFF
> @@ -315,6 +319,50 @@ static void check_dc3co_with_videoplayback_like_load(data_t *data)
>   	check_dc_counter(data, IGT_INTEL_CHECK_DC3CO, dc3co_prev_cnt);
>   }
>   
> +static void check_framedrop(data_t *data)
> +{
> +	igt_plane_t *primary;
> +	uint32_t dc3co_prev_cnt, dc3co_cnt;
> +	int delay, frame_count, max_count = 100, ret;
-->set frame_count with 0.
> +	bool dc3co_flag = false;
> +	drmVBlank wait;
> +
> +	primary = igt_output_get_plane_type(data->output,
> +					    DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, NULL);
> +	dc3co_prev_cnt = igt_read_dc_counter(data->debugfs_fd,
> +					     IGT_INTEL_CHECK_DC3CO);
> +
> +	/* Calculate delay to generate idle frame in usec*/
> +	delay = 1.5 * ((1000 * 1000) / data->mode->vrefresh);
-->better to have : (int) (1.5 * ((<rest code>));  so proepr conversion 
to integer usec
> +
> +	for (int i = 0; i < max_count; i++) {
> +		if (i % 2 == 0)
> +			igt_plane_set_fb(primary, &data->fb_rgb);
> +		else
> +			igt_plane_set_fb(primary, &data->fb_rgr);
> +
> +		igt_display_commit(&data->display);
> +		frame_count++;
> +
> +		memset(&wait, 0, sizeof(wait));
> +		wait.request.type = DRM_VBLANK_RELATIVE;
> +		wait.request.sequence = 1;
> +
> +		ret = drmWaitVBlank(data->drm_fd, &wait);
> +		igt_assert_eq(ret, 0);
> +		dc3co_cnt = igt_read_dc_counter(data->debugfs_fd,
> +						IGT_INTEL_CHECK_DC3CO);
> +		if (dc3co_cnt > dc3co_prev_cnt)
> +			dc3co_flag = true;
> +
> +		usleep(delay);
> +	}
> +
> +	igt_assert_f(dc3co_flag, "DC3CO entry failed.\n");
> +	igt_assert_f(frame_count == max_count, "Framedrop seen during vpb scenario.\n");

--> Here we are just counting a loop itiration, as per my understanding 
the real frame drop should be vblank seq numers.
         I think we have to do like:
         before start loop read  prev_vblank = wait.reply.sequence and 
inside loop current_vblank = wait.reply.sequence;
         then vblank_diff = current_vblank - prev_vblank, so the gap 
greter than 1 indicates frame drop.
         on everyloop if  the gap is greter than previous, we can 
increment frame_drop + = vblank_diff-1,
         and updated the prev_vblank with current_vblank in side the 
loop till the loop repeats.
         which will give vblank interval w.r.t actual expected, Please 
check it...

-->  if it is intention then better to updated message  some thing like 
-->"Frame processing" instead of "Frame drop" ??

-->also print number of frame count and max count along with the message.

> +}
> +
>   static void setup_dc3co(data_t *data, enum psr_mode mode)
>   {
>   	data->op_psr_mode = mode;
> @@ -333,6 +381,16 @@ static void test_dc3co_vpb_simulation(data_t *data, enum psr_mode mode)
>   	cleanup_dc3co_fbs(data);
>   }
>   
> +static void test_framedrop_dc3co(data_t *data, enum psr_mode mode)
> +{
> +	igt_require_dc_counter(data->debugfs_fd, IGT_INTEL_CHECK_DC3CO);
> +	setup_output(data);
> +	setup_dc3co(data, mode);
> +	setup_videoplayback(data);
> +	check_framedrop(data);
> +	cleanup_dc3co_fbs(data);
> +}
> +
>   static void test_dc5_retention_flops(data_t *data, int dc_flag)
>   {
>   	uint32_t dc_counter_before_psr;
> @@ -660,6 +718,31 @@ int igt_main()
>   		}
>   	}
>   
> +	igt_describe("Verify that DC3CO entry does not cause frame drops "
> +		     "and successfully enters the power state");
> +	igt_subtest_with_dynamic("dc3co-framedrop-check") {
> +		int modes[] = {PSR_MODE_2, PR_MODE};
> +		const char *append_subtest_name[2] = {
> +			"psr2-",
> +			"pr-",
-->here also same comments, provided at patch 2
> +		};
> +
> +		for (int i = 0; i < ARRAY_SIZE(modes); i++) {
> +			igt_dynamic_f("%s-dc3co-framedrop", append_subtest_name[i]) {
> +				igt_require(psr_sink_support(data.drm_fd, data.debugfs_fd,
> +							     modes[i], NULL));
> +
> +				if (modes[i] == PSR_MODE_2)
> +					igt_require(IS_TIGERLAKE(data.devid) ||
> +						    intel_display_ver(data.devid) >= 35);
> +				else if (modes[i] == PR_MODE)
> +					igt_require(intel_display_ver(data.devid) >= 35);
> +
> +				test_framedrop_dc3co(&data, modes[i]);
> +			}
> +		}
> +	}
> +
>   	igt_describe("This test validates display engine entry to DC5 state "
>   		     "while PSR is active");
>   	igt_subtest("dc5-psr") {

[-- Attachment #2: Type: text/html, Size: 6637 bytes --]