Re: [PATCH i-g-t 6/6] tests/kms_pipe_crc_basic: Add MST suspend-resume test

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 01 Apr 2026, Kunal Joshi <kunal1.joshi@intel.com> wrote:
> Add a new subtest mst-suspend-read-crc that verifies MST outputs
> preserve display content (via CRC comparison) across S3 suspend/resume.
>
> The test enumerates all connected DP MST topologies, and for each root
> connector exercises all non-empty subsets of the discovered MST outputs.
> For each subset it:
>  - Assigns pipes and selects non-joiner modes
>  - Fits modes within bandwidth constraints
>  - Creates green FBs, commits, and collects pre-suspend CRCs
>  - Suspends to memory and resumes
>  - Collects post-suspend CRCs and asserts they match

tests/kms_pipe_crc_basic.c is supposed to be the basic CRC test. Adding
MST to it is fine, I guess, but adding all the Intel specific joiner
stuff here seems questionable.

IMO basically anything that needs to know anything about the concept of
"pipes" (as opposed to CRTCs and CRTC indexes) should be inside
tests/intel or minimized within checks for Intel devices.


BR,
Jani.

>
> v2:
> - Add error handling for igt_pipe_crc_new (Thasleem)
> - Improve log format with index for clarity (Thasleem)
> - Order declarations in decreasing line length (Thasleem)
> - Add print message when no MST found (Thasleem)
> - Add bounds check for idx in subset loop (Thasleem)
> - Use igt_crtc_crc_new() instead of igt_pipe_crc_new() (Ville)
>
> v3:
> - framebuffers missing in commit (Bilal)
>
> v4:
> - Drop cleanup framework
>
> v5:
> - Use igt_debug instead of igt_info for CRC logging (Thasleem)
> - Remove  comments (Suraj)
> - Remove double vblank waits before/after suspend (Suraj)
> - Rename variable (Thasleem)
> - Two-pass approach (Suraj)
>
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> Reviewed-by: Mohammed Thasleem <mohammed.thasleem@intel.com>
> ---
>  tests/kms_pipe_crc_basic.c | 318 +++++++++++++++++++++++++++++++++++++
>  tests/meson.build          |   3 +
>  2 files changed, 321 insertions(+)
>
> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
> index 42e141545..d8369807e 100644
> --- a/tests/kms_pipe_crc_basic.c
> +++ b/tests/kms_pipe_crc_basic.c
> @@ -32,7 +32,10 @@
>  
>  #include "igt.h"
>  #include "igt_sysfs.h"
> +#include "intel/kms_mst_helper.h"
> +#include "intel/kms_joiner_helper.h"
>  #include <errno.h>
> +#include <limits.h>
>  #include <stdbool.h>
>  #include <stdio.h>
>  #include <string.h>
> @@ -77,6 +80,11 @@
>   *              CRTC does not cause issues.
>   */
>  
> +/**
> + * SUBTEST: mst-suspend-read-crc
> + * Description: MST suspend test for pipe CRC reads.
> + */
> +
>  static bool extended;
>  static int active_crtcs[IGT_MAX_PIPES];
>  static uint32_t last_crtc_index;
> @@ -96,6 +104,310 @@ static struct {
>  	{ .r = 0.0, .g = 1.0, .b = 1.0 },
>  };
>  
> +#define MAX_MST_OUTPUT	3
> +
> +struct mst_root_info {
> +	int root_id;
> +	igt_output_t *mst_outputs[IGT_MAX_PIPES];
> +	int num_mst;
> +};
> +
> +static void collect_single_crc(igt_crtc_t *crtc, igt_crc_t *crc)
> +{
> +	igt_pipe_crc_t *pipe_crc;
> +
> +	pipe_crc = igt_crtc_crc_new(crtc, IGT_PIPE_CRC_SOURCE_AUTO);
> +	igt_assert(pipe_crc);
> +	igt_pipe_crc_collect_crc(pipe_crc, crc);
> +	igt_pipe_crc_free(pipe_crc);
> +}
> +
> +static void collect_crc_for_active_outputs(igt_output_t **outputs,
> +					   int num_outputs,
> +					   igt_crc_t *crcs,
> +					   bool post_suspend)
> +{
> +	int i;
> +
> +	for (i = 0; i < num_outputs; i++) {
> +		igt_output_t *output = outputs[i];
> +		igt_crtc_t *crtc = igt_output_get_driving_crtc(output);
> +
> +		if (post_suspend)
> +			igt_require_f(crtc,
> +				      "POST-SUSPEND: no driving CRTC for %s (topology changed?)\n",
> +				      output->name);
> +		else
> +			igt_assert_f(crtc,
> +				     "PRE-SUSPEND: no driving CRTC for %s (bad pipe assignment?)\n",
> +				     output->name);
> +
> +		collect_single_crc(crtc, &crcs[i]);
> +	}
> +}
> +
> +static void wait_for_vblanks(igt_output_t **outputs, int num_outputs,
> +			     bool post_suspend)
> +{
> +	int i;
> +
> +	for (i = 0; i < num_outputs; i++) {
> +		igt_crtc_t *crtc = igt_output_get_driving_crtc(outputs[i]);
> +
> +		if (post_suspend)
> +			igt_require_f(crtc,
> +				      "POST-SUSPEND: no driving CRTC for %s during vblank wait (topology changed?)\n",
> +				      outputs[i]->name);
> +		else
> +			igt_assert_f(crtc,
> +				     "PRE-SUSPEND: no driving CRTC for %s during vblank wait (bad pipe assignment?)\n",
> +				     outputs[i]->name);
> +
> +		igt_wait_for_vblank(crtc);
> +	}
> +}
> +
> +static void log_crc_for_outputs(igt_output_t **outputs, int num_outputs,
> +				igt_crc_t *crcs, const char *stage)
> +{
> +	int i;
> +
> +	for (i = 0; i < num_outputs; i++) {
> +		igt_output_t *o = outputs[i];
> +		drmModeModeInfo *m = igt_output_get_mode(o);
> +		char *s = igt_crc_to_string(&crcs[i]);
> +		igt_crtc_t *crtc = igt_output_get_driving_crtc(o);
> +
> +		igt_debug("[%d] %s: output %s -> crtc %s, CRC %s, mode %dx%d@%d\n",
> +			  i, stage, o->name,
> +			  igt_crtc_name(crtc),
> +			  s,
> +			  m ? m->hdisplay : -1,
> +			  m ? m->vdisplay : -1,
> +			  m ? m->vrefresh : 0);
> +		free(s);
> +	}
> +}
> +
> +static void select_non_joiner_modes(int drm_fd, igt_output_t **outputs,
> +				    int num_outputs)
> +{
> +	int max_dotclock = igt_get_max_dotclock(drm_fd);
> +	int i;
> +
> +	if (max_dotclock <= 0)
> +		max_dotclock = INT_MAX;
> +
> +	for (i = 0; i < num_outputs; i++) {
> +		igt_output_t *output = outputs[i];
> +		drmModeModeInfo non_joiner_mode;
> +
> +		igt_require_f(max_non_joiner_mode_found(drm_fd,
> +							output->config.connector,
> +							max_dotclock,
> +							&non_joiner_mode),
> +			      "No non-joiner mode found for %s\n",
> +			      output->name);
> +		igt_output_override_mode(output, &non_joiner_mode);
> +	}
> +}
> +
> +static void create_fbs_for_outputs(int drm_fd, igt_output_t **outputs,
> +				   int num_outputs, struct igt_fb *fbs)
> +{
> +	int i;
> +
> +	for (i = 0; i < num_outputs; i++) {
> +		igt_output_t *output = outputs[i];
> +		drmModeModeInfo *mode = igt_output_get_mode(output);
> +		igt_plane_t *primary;
> +
> +		igt_require_f(mode, "No mode available for output %s\n", output->name);
> +
> +		primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +
> +		igt_create_color_fb(drm_fd,
> +				    mode->hdisplay, mode->vdisplay,
> +				    DRM_FORMAT_XRGB8888,
> +				    DRM_FORMAT_MOD_LINEAR,
> +				    0.0, 1.0, 0.0,
> +				    &fbs[i]);
> +		igt_plane_set_fb(primary, &fbs[i]);
> +	}
> +}
> +
> +static void run_mst_subset_and_verify(igt_display_t *display,
> +				      igt_output_t **active_outputs,
> +				      int num_active,
> +				      int n_crtcs,
> +				      uint32_t master_pipes_mask,
> +				      uint32_t valid_pipes_mask)
> +{
> +	igt_crc_t pre_crcs[MAX_MST_OUTPUT];
> +	igt_crc_t post_crcs[MAX_MST_OUTPUT];
> +	struct igt_fb fbs[MAX_MST_OUTPUT];
> +	uint32_t used_pipes_mask = 0;
> +	enum igt_suspend_test stest = SUSPEND_TEST_NONE;
> +	int drm_fd = display->drm_fd;
> +	int i;
> +
> +	igt_require_f(num_active <= n_crtcs,
> +		      "Not enough crtcs for MST subset\n");
> +
> +	igt_require(igt_assign_pipes_for_outputs(drm_fd,
> +						 active_outputs,
> +						 num_active,
> +						 n_crtcs,
> +						 &used_pipes_mask,
> +						 master_pipes_mask,
> +						 valid_pipes_mask));
> +
> +	select_non_joiner_modes(drm_fd, active_outputs, num_active);
> +
> +	igt_require_f(igt_fit_modes_in_bw(display),
> +		      "Unable to fit modes in bw\n");
> +
> +	create_fbs_for_outputs(drm_fd, active_outputs, num_active, fbs);
> +
> +	igt_display_commit2(display, COMMIT_ATOMIC);
> +
> +	/* rtcwake cmd is not supported on MTK devices */
> +	if (is_mtk_device(drm_fd))
> +		stest = SUSPEND_TEST_DEVICES;
> +
> +	igt_info("MST subset: %d outputs, n_crtcs=%d\n",
> +		 num_active, n_crtcs);
> +
> +	wait_for_vblanks(active_outputs, num_active, false);
> +
> +	collect_crc_for_active_outputs(active_outputs,
> +				       num_active,
> +				       pre_crcs, false);
> +
> +	log_crc_for_outputs(active_outputs, num_active, pre_crcs, "PRE-SUSPEND");
> +
> +	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, stest);
> +
> +	wait_for_vblanks(active_outputs, num_active, true);
> +
> +	collect_crc_for_active_outputs(active_outputs,
> +				       num_active,
> +				       post_crcs, true);
> +
> +	log_crc_for_outputs(active_outputs, num_active, post_crcs, "POST-SUSPEND");
> +
> +	for (i = 0; i < num_active; i++)
> +		igt_assert_crc_equal(&pre_crcs[i], &post_crcs[i]);
> +
> +	/* Detach FBs from planes and remove them to leave a clean state */
> +	for (i = 0; i < num_active; i++) {
> +		igt_plane_t *primary =
> +			igt_output_get_plane_type(active_outputs[i],
> +						  DRM_PLANE_TYPE_PRIMARY);
> +		igt_plane_set_fb(primary, NULL);
> +		igt_remove_fb(drm_fd, &fbs[i]);
> +	}
> +}
> +
> +static int discover_mst_roots(igt_display_t *display,
> +			      struct mst_root_info *roots)
> +{
> +	igt_output_t *output;
> +	int drm_fd = display->drm_fd;
> +	int num_roots = 0;
> +	int ret, i;
> +
> +	for_each_connected_output(display, output) {
> +		bool root_seen = false;
> +		int root_id;
> +
> +		root_id = igt_get_dp_mst_connector_id(output);
> +		if (root_id < 0)
> +			continue;
> +
> +		for (i = 0; i < num_roots; i++)
> +			if (roots[i].root_id == root_id)
> +				root_seen = true;
> +
> +		if (root_seen)
> +			continue;
> +
> +		if (num_roots >= IGT_MAX_PIPES)
> +			break;
> +
> +		roots[num_roots].root_id = root_id;
> +		roots[num_roots].num_mst = 0;
> +		ret = igt_find_all_mst_output_in_topology(drm_fd,
> +							  display, output,
> +							  roots[num_roots].mst_outputs,
> +							  &roots[num_roots].num_mst);
> +		igt_require(ret == 0);
> +
> +		if (roots[num_roots].num_mst == 0)
> +			continue;
> +
> +		if (roots[num_roots].num_mst > MAX_MST_OUTPUT)
> +			roots[num_roots].num_mst = MAX_MST_OUTPUT;
> +
> +		num_roots++;
> +	}
> +
> +	return num_roots;
> +}
> +
> +static void mst_suspend_read_crc(igt_display_t *display)
> +{
> +	struct mst_root_info roots[IGT_MAX_PIPES];
> +	igt_output_t *active_outputs[MAX_MST_OUTPUT];
> +	uint32_t valid_pipes_mask = 0;
> +	uint32_t master_pipes_mask;
> +	int n_crtcs = igt_display_n_crtcs(display);
> +	int num_roots, num_active, i;
> +	igt_crtc_t *crtc;
> +
> +	for_each_crtc(display, crtc)
> +		valid_pipes_mask |= BIT(crtc->pipe);
> +
> +	igt_set_all_master_pipes_for_platform(display, &master_pipes_mask);
> +
> +	num_roots = discover_mst_roots(display, roots);
> +	igt_require_f(num_roots > 0, "No MST roots found\n");
> +
> +	for (i = 0; i < num_roots; i++) {
> +		igt_dynamic_f("mst-root-%d", roots[i].root_id) {
> +			int mask;
> +
> +			for (mask = 1; mask < (1 << roots[i].num_mst); mask++) {
> +				int bit, idx = 0;
> +
> +				for (bit = 0; bit < roots[i].num_mst; bit++) {
> +					if (!(mask & (1 << bit)))
> +						continue;
> +
> +					if (idx >= MAX_MST_OUTPUT)
> +						break;
> +
> +					active_outputs[idx++] =
> +						roots[i].mst_outputs[bit];
> +				}
> +
> +				num_active = idx;
> +				if (!num_active)
> +					continue;
> +
> +				igt_display_reset(display);
> +
> +				run_mst_subset_and_verify(display,
> +							  active_outputs,
> +							  num_active,
> +							  n_crtcs,
> +							  master_pipes_mask,
> +							  valid_pipes_mask);
> +			}
> +		}
> +	}
> +}
> +
>  static bool simulation_constraint(igt_crtc_t *crtc)
>  {
>  	if (igt_run_in_simulation() && !extended &&
> @@ -525,6 +837,12 @@ int igt_main_args("e", NULL, help_str, opt_handler, NULL)
>  		}
>  	}
>  
> +	igt_describe("MST suspend test for pipe CRC reads.");
> +	igt_subtest_with_dynamic("mst-suspend-read-crc") {
> +		igt_require(igt_display_has_mst_output(&data.display));
> +		mst_suspend_read_crc(&data.display);
> +	}
> +
>  	igt_fixture() {
>  		igt_display_fini(&data.display);
>  		drm_close_driver(data.drm_fd);
> diff --git a/tests/meson.build b/tests/meson.build
> index cecb4a8ae..a1e42755e 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -402,6 +402,9 @@ extra_sources = {
>             join_paths ('intel', 'kms_joiner_helper.c') ],
>  	'kms_dsc': [ join_paths ('intel', 'kms_dsc_helper.c') ],
>  	'kms_joiner': [ join_paths ('intel', 'kms_joiner_helper.c') ],
> +	'kms_pipe_crc_basic': [
> +           join_paths ('intel', 'kms_mst_helper.c'),
> +           join_paths ('intel', 'kms_joiner_helper.c') ],
>  	'kms_psr2_sf':  [ join_paths ('intel', 'kms_dsc_helper.c') ],
>  }

-- 
Jani Nikula, Intel