Re: [PATCH i-g-t v8 5/7] tests/intel/kms_pm_dc: Add dc3co framedrop validation test

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

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


On 27-05-2026 02:19 pm, Jeevan B wrote:
> Add a subtest to verify that DC3CO entry does not introduce frame
> drops. The test alternates commits and waits for a kernel vblank
> after each commit, ensuring the vblank sequence continues to
> advance without stalls or dropped frames. Also verify that the
> DC3CO counter increments, confirming that DC3CO is entered
> successfully during the test.
>
> Signed-off-by: Jeevan B<jeevan.b@intel.com>
> ---
>   tests/intel/kms_pm_dc.c | 110 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 110 insertions(+)
>
> diff --git a/tests/intel/kms_pm_dc.c b/tests/intel/kms_pm_dc.c
> index 57dade47d..620ba9329 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 or PR 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
> @@ -342,6 +346,82 @@ static void test_dc3co_vpb_simulation(data_t *data)
>   	cleanup_dc3co_fbs(data);
>   }
>   
> +static uint32_t wait_for_next_vblank_seq(data_t *data)
> +{
> +	drmVBlank wait = {};
> +	igt_crtc_t *crtc = data->output->pending_crtc;
> +
> +	igt_assert_f(crtc, "No CRTC bound to output for vblank wait\n");
> +
> +	wait.request.type = kmstest_get_vbl_flag(crtc->crtc_index) |
> +						 DRM_VBLANK_RELATIVE |
> +						 DRM_VBLANK_NEXTONMISS;
> +	wait.request.sequence = 1;
> +	igt_assert_eq(drmWaitVBlank(data->drm_fd, &wait), 0);
> +
> +	return wait.reply.sequence;
> +}
> +
> +static void detect_dc3co_framedrop(data_t *data)
> +{
> +	igt_plane_t *primary;
> +	uint32_t dc3co_prev_cnt;
> +	uint32_t dc3co_cur_cnt;
> +	uint32_t prev_vblank_seq = 0;
> +	uint32_t vblank_seq;
> +	int delay;
> +	int dc3co_target_flips = 200;
> +	int verify_commits = 300;

-->Any specific reason having above magic numbers?

-->Define them globally instead hard coding..

> +	int committed = 0;
> +	bool dc3co_after_target = false;
> +	bool front = false;
> +
> +	igt_require_f(data->mode->vrefresh != 0, "Invalid vrefresh rate of 0\n");
> +
> +	primary = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, NULL);
> +	igt_display_commit(&data->display);
> +
> +	dc3co_prev_cnt = igt_read_dc_counter(data->debugfs_fd, IGT_INTEL_CHECK_DC3CO);
> +
> +	delay = (int)(1.5 * (1000000 / data->mode->vrefresh));
> +
> +	while (committed < verify_commits) {
> +		front = !front;
> +		igt_plane_set_fb(primary, front ? &data->fb_rgr : &data->fb_rgb);
> +		igt_display_commit(&data->display);
> +
> +		vblank_seq = wait_for_next_vblank_seq(data);
> +		if (prev_vblank_seq)

-->From above delay, (Say for 60hz =16.67ms) but current delay its 
around 25ms there migh be chance of getting 2nd commit could be expected 
2nd vblank
IMO, gap of 1 or 2 would be fine,  becuase of 1.5x sleep time crossing 
vblank time boundary.
-->read,  gab =  vblank seq -preve vblank seq;

-->and assert it like-->igt_assert_f(gab <=2 or gab>0 && gab<=2,
			     "Vblank sequence did not advance after commit %d: vblank gap=%u\n",
			     committed + 1, gab);

> +			igt_assert_f(igt_vblank_after(vblank_seq, prev_vblank_seq),
> +				     "Vblank sequence did not advance after commit %d\n",
> +				     committed + 1);
> +		prev_vblank_seq = vblank_seq;
> +		committed++;
> +		usleep(delay);
> +
> +		dc3co_cur_cnt = igt_read_dc_counter(data->debugfs_fd,
> +						    IGT_INTEL_CHECK_DC3CO);
> +		if (committed >= dc3co_target_flips && dc3co_cur_cnt > dc3co_prev_cnt)
IMO: better to avoaid un-conditional read of debugfs --> 
igt_read_dc_counter only if (committed >= dc3co_target_flips)
like:         if (committed >= dc3co_target_flips) {
dc3co_cur_cnt = igt_read_dc_counter(data->debugfs_fd, 
IGT_INTEL_CHECK_DC3CO);

		if (c3co_cur_cnt > dc3co_prev_cnt)

			dc3co_after_target = true;
	}

> +			dc3co_after_target = true;
> +	}
> +
> +	igt_assert_eq(committed, verify_commits);
> +	igt_assert_f(dc3co_after_target,
> +		     "DC3CO did not increment after %d flips while validating %d commits\n",
> +		     dc3co_target_flips, verify_commits);
> +}
> +
> +static void test_dc3co_framedrop(data_t *data)
> +{
> +	igt_require_dc_counter(data->debugfs_fd, IGT_INTEL_CHECK_DC3CO);
> +	setup_output(data);
> +	setup_dc3co(data);
> +	setup_videoplayback(data);
> +	detect_dc3co_framedrop(data);
> +	cleanup_dc3co_fbs(data);
> +}
> +
>   static void test_dc5_retention_flops(data_t *data, int dc_flag)
>   {
>   	uint32_t dc_counter_before_psr;
> @@ -693,6 +773,36 @@ int igt_main()
>   		}
>   	}
>   
> +	igt_describe("Validate that no frame drops occur during DC3CO entry "
> +			     "while alternating framebuffers with PSR2 or Panel Replay active");
> +	igt_subtest_with_dynamic("dc3co-framedrop-check") {
> +		static const struct dc3co_test_mode dc3co_modes[] = {
> +			{ PSR_MODE_2, "psr2" },
> +			{ PR_MODE,    "pr"   },
> +		};
> +
> +		for (int i = 0; i < ARRAY_SIZE(dc3co_modes); i++) {
> +			const char *name = dc3co_modes[i].name;
> +			data.op_psr_mode = dc3co_modes[i].mode;
> +
> +			igt_dynamic_f("%s", name) {
> +				igt_require(psr_sink_support(data.drm_fd,
> +						     data.debugfs_fd,
> +						     data.op_psr_mode, NULL));
> +
> +				if (data.op_psr_mode == PSR_MODE_2)
> +					igt_require_f(IS_TIGERLAKE(data.devid) ||
> +						      intel_display_ver(data.devid) >= 35,
> +						      "Platform does not support DC3CO with PSR2\n");
> +				else
> +					igt_require_f(intel_display_ver(data.devid) >= 35,
> +						      "Platform does not support DC3CO with Panel Replay\n");
> +
> +				test_dc3co_framedrop(&data);
> +			}
> +		}
> +	}
> +
>   	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: 7540 bytes --]