* Re: [PATCH i-g-t 3/6] lib/igt_kms: Fix max_non_joiner_mode_found
2026-04-01 11:55 ` [PATCH i-g-t 3/6] lib/igt_kms: Fix max_non_joiner_mode_found Kunal Joshi
@ 2026-04-01 11:44 ` Jani Nikula
2026-04-01 11:57 ` Joshi, Kunal1
0 siblings, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2026-04-01 11:44 UTC (permalink / raw)
To: Kunal Joshi, igt-dev; +Cc: Kunal Joshi, Mohammed Thasleem, Suraj Kandpal
On Wed, 01 Apr 2026, Kunal Joshi <kunal1.joshi@intel.com> wrote:
> Previously the function, instead of giving the highest possible mode the
> connector could support, started the check by looking if the mode was
> equal to max_pipe_hdisplay. There may be corner cases where a connector never supports the highest mode our source can
> drive (e.g. a 4K mode on a 5K-capable platform).
>
> Fix this by iterating all modes and tracking the best one (highest
> hdisplay, then highest clock as a tiebreaker) that does not require a
> big joiner, as determined by igt_bigjoiner_possible(). This correctly
> handles connectors with any max resolution.
I'm not requesting changes right now, but I keep wondering about having
the joiner code in igt_kms.[ch] at all. It's an Intel implementation
detail. Why is so much of that in the generic file instead of hidden in
some Intel specific file or directory?
BR,
Jani.
>
> v2: Reword commit message (Suraj)
>
> Fixes: 3830ca6a5068 ("lib/igt_kms: Add support to check joiner mode limit")
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> Reviewed-by: Mohammed Thasleem <mohammed.thasleem@intel.com>
> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
> lib/igt_kms.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 04b1f3972..dbd419a9b 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -6979,21 +6979,26 @@ bool bigjoiner_mode_found(int drm_fd, drmModeConnector *connector,
> * false otherwise.
> */
> bool max_non_joiner_mode_found(int drm_fd, drmModeConnector *connector,
> - int max_dotclock, drmModeModeInfo *mode)
> + int max_dotclock, drmModeModeInfo *mode)
> {
> - int max_hdisplay = get_max_pipe_hdisplay(drm_fd);
> + bool found = false;
>
> for (int i = 0; i < connector->count_modes; i++) {
> drmModeModeInfo *current_mode = &connector->modes[i];
>
> - if (current_mode->hdisplay == max_hdisplay &&
> - current_mode->clock < max_dotclock) {
> + if (igt_bigjoiner_possible(drm_fd, current_mode, max_dotclock))
> + continue;
> +
> + if (!found ||
> + current_mode->hdisplay > mode->hdisplay ||
> + (current_mode->hdisplay == mode->hdisplay &&
> + current_mode->clock > mode->clock)) {
> *mode = *current_mode;
> - return true;
> + found = true;
> }
> }
>
> - return false;
> + return found;
> }
>
> /* TODO: Move these lib functions to the joiner-specific library file
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH i-g-t 4/6] lib/igt_kms: Export get_max_pipe_hdisplay as public helper
2026-04-01 11:55 ` [PATCH i-g-t 4/6] lib/igt_kms: Export get_max_pipe_hdisplay as public helper Kunal Joshi
@ 2026-04-01 11:47 ` Jani Nikula
2026-04-01 14:46 ` Joshi, Kunal1
0 siblings, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2026-04-01 11:47 UTC (permalink / raw)
To: Kunal Joshi, igt-dev; +Cc: Kunal Joshi
On Wed, 01 Apr 2026, Kunal Joshi <kunal1.joshi@intel.com> wrote:
> Rename the static get_max_pipe_hdisplay to igt_get_max_pipe_hdisplay
> and export it as a public API. This function returns the maximum
> hdisplay a single pipe can drive. Tests like basic-max-non-joiner
> need this value to distinguish outputs that can reach the single-pipe
> boundary from those that cannot
>
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> ---
> lib/igt_kms.c | 8 ++++----
> lib/igt_kms.h | 1 +
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index dbd419a9b..961538932 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -6907,12 +6907,12 @@ int igt_get_current_cdclk(int fd)
> }
>
> /**
> - * get_max_hdisplay:
> + * igt_get_max_pipe_hdisplay:
> * @drm_fd: drm file descriptor
> *
> * Returns: The maximum hdisplay supported per pipe.
> */
> -static int get_max_pipe_hdisplay(int drm_fd)
> +int igt_get_max_pipe_hdisplay(int drm_fd)
This is purely about Intel limitations, isn't it? The name is misleading
for anything else.
BR,
Jani.
> {
> int dev_id = intel_get_drm_devid(drm_fd);
>
> @@ -6933,7 +6933,7 @@ static int get_max_pipe_hdisplay(int drm_fd)
> */
> bool igt_bigjoiner_possible(int drm_fd, drmModeModeInfo *mode, int max_dotclock)
> {
> - return (mode->hdisplay > get_max_pipe_hdisplay(drm_fd) ||
> + return (mode->hdisplay > igt_get_max_pipe_hdisplay(drm_fd) ||
> mode->clock > max_dotclock);
> }
>
> @@ -7047,7 +7047,7 @@ bool igt_is_joiner_enabled_for_pipe(int drmfd, enum pipe pipe)
> */
> bool igt_ultrajoiner_possible(int drm_fd, drmModeModeInfo *mode, int max_dotclock)
> {
> - return (mode->hdisplay > 2 * get_max_pipe_hdisplay(drm_fd) ||
> + return (mode->hdisplay > 2 * igt_get_max_pipe_hdisplay(drm_fd) ||
> mode->clock > 2 * max_dotclock);
> }
>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index f994d91d3..2b5196c90 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -1236,6 +1236,7 @@ void igt_sort_connector_modes(drmModeConnector *connector,
> bool igt_max_bpc_constraint(igt_display_t *display, igt_crtc_t *crtc,
> igt_output_t *output, int bpc);
> int igt_get_max_dotclock(int fd);
> +int igt_get_max_pipe_hdisplay(int drm_fd);
> int igt_get_max_cdclk(int fd);
> int igt_get_current_cdclk(int fd);
> bool igt_bigjoiner_possible(int drm_fd, drmModeModeInfo *mode, int max_dotclock);
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH i-g-t 6/6] tests/kms_pipe_crc_basic: Add MST suspend-resume test
2026-04-01 11:55 ` [PATCH i-g-t 6/6] tests/kms_pipe_crc_basic: Add MST suspend-resume test Kunal Joshi
@ 2026-04-01 11:52 ` Jani Nikula
2026-04-01 11:59 ` Joshi, Kunal1
2026-04-01 12:06 ` Ville Syrjälä
0 siblings, 2 replies; 15+ messages in thread
From: Jani Nikula @ 2026-04-01 11:52 UTC (permalink / raw)
To: Kunal Joshi, igt-dev; +Cc: Kunal Joshi, Mohammed Thasleem
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH i-g-t 0/6] tests/kms_pipe_crc_basic: add mst suspend-resume test
@ 2026-04-01 11:55 Kunal Joshi
2026-04-01 11:55 ` [PATCH i-g-t 1/6] tests/intel/kms_mst_helper: Add kernel-doc for existing function Kunal Joshi
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Kunal Joshi @ 2026-04-01 11:55 UTC (permalink / raw)
To: igt-dev; +Cc: Kunal Joshi
This patch series introduces MST suspend-resume test
to kms_pipe_crc_basic. The MST test validates there
is no corruption/blank out across suspend/resume cycles
for all possible subsets.
Kunal Joshi (6):
tests/intel/kms_mst_helper: Add kernel-doc for existing function
tests/intel/kms_mst_helper: Add helper to check for MST outputs
lib/igt_kms: Fix max_non_joiner_mode_found
lib/igt_kms: Export get_max_pipe_hdisplay as public helper
tests/intel/kms_joiner: Require boundary mode for basic-max-non-joiner
tests/kms_pipe_crc_basic: Add MST suspend-resume test
lib/igt_kms.c | 25 +--
lib/igt_kms.h | 1 +
tests/intel/kms_joiner.c | 4 +
tests/intel/kms_mst_helper.c | 24 ++-
tests/intel/kms_mst_helper.h | 1 +
tests/kms_pipe_crc_basic.c | 318 +++++++++++++++++++++++++++++++++++
tests/meson.build | 3 +
7 files changed, 364 insertions(+), 12 deletions(-)
--
2.48.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH i-g-t 1/6] tests/intel/kms_mst_helper: Add kernel-doc for existing function
2026-04-01 11:55 [PATCH i-g-t 0/6] tests/kms_pipe_crc_basic: add mst suspend-resume test Kunal Joshi
@ 2026-04-01 11:55 ` Kunal Joshi
2026-04-01 11:55 ` [PATCH i-g-t 2/6] tests/intel/kms_mst_helper: Add helper to check for MST outputs Kunal Joshi
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Kunal Joshi @ 2026-04-01 11:55 UTC (permalink / raw)
To: igt-dev; +Cc: Kunal Joshi
Convert the doc comment on igt_find_all_mst_output_in_topology
to proper kernel-doc.
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
tests/intel/kms_mst_helper.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/intel/kms_mst_helper.c b/tests/intel/kms_mst_helper.c
index aef74cd31..38859cffc 100644
--- a/tests/intel/kms_mst_helper.c
+++ b/tests/intel/kms_mst_helper.c
@@ -5,11 +5,12 @@
#include "kms_mst_helper.h"
-/*
+/**
+ * igt_find_all_mst_output_in_topology:
* @drm_fd: DRM file descriptor
* @display: pointer to #igt_display_t structure
* @output: target output
- * @mst_outputs: filled with mst output of same toplogy as @output
+ * @mst_outputs: filled with mst output of same topology as @output
* @num_mst_outputs: filled with count of mst outputs found in topology
*
* Iterates over all connected outputs and adds each DP MST
--
2.48.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH i-g-t 2/6] tests/intel/kms_mst_helper: Add helper to check for MST outputs
2026-04-01 11:55 [PATCH i-g-t 0/6] tests/kms_pipe_crc_basic: add mst suspend-resume test Kunal Joshi
2026-04-01 11:55 ` [PATCH i-g-t 1/6] tests/intel/kms_mst_helper: Add kernel-doc for existing function Kunal Joshi
@ 2026-04-01 11:55 ` Kunal Joshi
2026-04-01 11:55 ` [PATCH i-g-t 3/6] lib/igt_kms: Fix max_non_joiner_mode_found Kunal Joshi
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Kunal Joshi @ 2026-04-01 11:55 UTC (permalink / raw)
To: igt-dev; +Cc: Kunal Joshi, Mohammed Thasleem
Add igt_display_has_mst_output() to check
if a display has at least one DP MST output
connected. This is useful for tests that need
to verify MST output availability before running.
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
Reviewed-by: Mohammed Thasleem <mohammed.thasleem@intel.com>
---
tests/intel/kms_mst_helper.c | 19 +++++++++++++++++++
tests/intel/kms_mst_helper.h | 1 +
2 files changed, 20 insertions(+)
diff --git a/tests/intel/kms_mst_helper.c b/tests/intel/kms_mst_helper.c
index 38859cffc..1918c8d10 100644
--- a/tests/intel/kms_mst_helper.c
+++ b/tests/intel/kms_mst_helper.c
@@ -47,3 +47,22 @@ int igt_find_all_mst_output_in_topology(int drm_fd, igt_display_t *display,
}
return 0;
}
+
+/**
+ * igt_display_has_mst_output:
+ * @display: pointer to #igt_display_t structure
+ *
+ * Check if a display has at least one DP MST output connected.
+ *
+ * Returns: true if MST output is found, false otherwise
+ */
+bool igt_display_has_mst_output(igt_display_t *display)
+{
+ igt_output_t *output;
+
+ for_each_connected_output(display, output) {
+ if (igt_check_output_is_dp_mst(output))
+ return true;
+ }
+ return false;
+}
diff --git a/tests/intel/kms_mst_helper.h b/tests/intel/kms_mst_helper.h
index 7391494ab..0e8ece0a0 100644
--- a/tests/intel/kms_mst_helper.h
+++ b/tests/intel/kms_mst_helper.h
@@ -12,4 +12,5 @@ int igt_find_all_mst_output_in_topology(int drm_fd, igt_display_t *display,
igt_output_t *output,
igt_output_t *mst_outputs[],
int *num_mst_outputs);
+bool igt_display_has_mst_output(igt_display_t *display);
#endif
--
2.48.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH i-g-t 3/6] lib/igt_kms: Fix max_non_joiner_mode_found
2026-04-01 11:55 [PATCH i-g-t 0/6] tests/kms_pipe_crc_basic: add mst suspend-resume test Kunal Joshi
2026-04-01 11:55 ` [PATCH i-g-t 1/6] tests/intel/kms_mst_helper: Add kernel-doc for existing function Kunal Joshi
2026-04-01 11:55 ` [PATCH i-g-t 2/6] tests/intel/kms_mst_helper: Add helper to check for MST outputs Kunal Joshi
@ 2026-04-01 11:55 ` Kunal Joshi
2026-04-01 11:44 ` Jani Nikula
2026-04-01 11:55 ` [PATCH i-g-t 4/6] lib/igt_kms: Export get_max_pipe_hdisplay as public helper Kunal Joshi
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Kunal Joshi @ 2026-04-01 11:55 UTC (permalink / raw)
To: igt-dev; +Cc: Kunal Joshi, Mohammed Thasleem, Suraj Kandpal
Previously the function, instead of giving the highest possible mode the
connector could support, started the check by looking if the mode was
equal to max_pipe_hdisplay. There may be corner cases where a connector never supports the highest mode our source can
drive (e.g. a 4K mode on a 5K-capable platform).
Fix this by iterating all modes and tracking the best one (highest
hdisplay, then highest clock as a tiebreaker) that does not require a
big joiner, as determined by igt_bigjoiner_possible(). This correctly
handles connectors with any max resolution.
v2: Reword commit message (Suraj)
Fixes: 3830ca6a5068 ("lib/igt_kms: Add support to check joiner mode limit")
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
Reviewed-by: Mohammed Thasleem <mohammed.thasleem@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
lib/igt_kms.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 04b1f3972..dbd419a9b 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -6979,21 +6979,26 @@ bool bigjoiner_mode_found(int drm_fd, drmModeConnector *connector,
* false otherwise.
*/
bool max_non_joiner_mode_found(int drm_fd, drmModeConnector *connector,
- int max_dotclock, drmModeModeInfo *mode)
+ int max_dotclock, drmModeModeInfo *mode)
{
- int max_hdisplay = get_max_pipe_hdisplay(drm_fd);
+ bool found = false;
for (int i = 0; i < connector->count_modes; i++) {
drmModeModeInfo *current_mode = &connector->modes[i];
- if (current_mode->hdisplay == max_hdisplay &&
- current_mode->clock < max_dotclock) {
+ if (igt_bigjoiner_possible(drm_fd, current_mode, max_dotclock))
+ continue;
+
+ if (!found ||
+ current_mode->hdisplay > mode->hdisplay ||
+ (current_mode->hdisplay == mode->hdisplay &&
+ current_mode->clock > mode->clock)) {
*mode = *current_mode;
- return true;
+ found = true;
}
}
- return false;
+ return found;
}
/* TODO: Move these lib functions to the joiner-specific library file
--
2.48.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH i-g-t 4/6] lib/igt_kms: Export get_max_pipe_hdisplay as public helper
2026-04-01 11:55 [PATCH i-g-t 0/6] tests/kms_pipe_crc_basic: add mst suspend-resume test Kunal Joshi
` (2 preceding siblings ...)
2026-04-01 11:55 ` [PATCH i-g-t 3/6] lib/igt_kms: Fix max_non_joiner_mode_found Kunal Joshi
@ 2026-04-01 11:55 ` Kunal Joshi
2026-04-01 11:47 ` Jani Nikula
2026-04-01 11:55 ` [PATCH i-g-t 5/6] tests/intel/kms_joiner: Require boundary mode for basic-max-non-joiner Kunal Joshi
2026-04-01 11:55 ` [PATCH i-g-t 6/6] tests/kms_pipe_crc_basic: Add MST suspend-resume test Kunal Joshi
5 siblings, 1 reply; 15+ messages in thread
From: Kunal Joshi @ 2026-04-01 11:55 UTC (permalink / raw)
To: igt-dev; +Cc: Kunal Joshi
Rename the static get_max_pipe_hdisplay to igt_get_max_pipe_hdisplay
and export it as a public API. This function returns the maximum
hdisplay a single pipe can drive. Tests like basic-max-non-joiner
need this value to distinguish outputs that can reach the single-pipe
boundary from those that cannot
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
lib/igt_kms.c | 8 ++++----
lib/igt_kms.h | 1 +
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index dbd419a9b..961538932 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -6907,12 +6907,12 @@ int igt_get_current_cdclk(int fd)
}
/**
- * get_max_hdisplay:
+ * igt_get_max_pipe_hdisplay:
* @drm_fd: drm file descriptor
*
* Returns: The maximum hdisplay supported per pipe.
*/
-static int get_max_pipe_hdisplay(int drm_fd)
+int igt_get_max_pipe_hdisplay(int drm_fd)
{
int dev_id = intel_get_drm_devid(drm_fd);
@@ -6933,7 +6933,7 @@ static int get_max_pipe_hdisplay(int drm_fd)
*/
bool igt_bigjoiner_possible(int drm_fd, drmModeModeInfo *mode, int max_dotclock)
{
- return (mode->hdisplay > get_max_pipe_hdisplay(drm_fd) ||
+ return (mode->hdisplay > igt_get_max_pipe_hdisplay(drm_fd) ||
mode->clock > max_dotclock);
}
@@ -7047,7 +7047,7 @@ bool igt_is_joiner_enabled_for_pipe(int drmfd, enum pipe pipe)
*/
bool igt_ultrajoiner_possible(int drm_fd, drmModeModeInfo *mode, int max_dotclock)
{
- return (mode->hdisplay > 2 * get_max_pipe_hdisplay(drm_fd) ||
+ return (mode->hdisplay > 2 * igt_get_max_pipe_hdisplay(drm_fd) ||
mode->clock > 2 * max_dotclock);
}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index f994d91d3..2b5196c90 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -1236,6 +1236,7 @@ void igt_sort_connector_modes(drmModeConnector *connector,
bool igt_max_bpc_constraint(igt_display_t *display, igt_crtc_t *crtc,
igt_output_t *output, int bpc);
int igt_get_max_dotclock(int fd);
+int igt_get_max_pipe_hdisplay(int drm_fd);
int igt_get_max_cdclk(int fd);
int igt_get_current_cdclk(int fd);
bool igt_bigjoiner_possible(int drm_fd, drmModeModeInfo *mode, int max_dotclock);
--
2.48.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH i-g-t 5/6] tests/intel/kms_joiner: Require boundary mode for basic-max-non-joiner
2026-04-01 11:55 [PATCH i-g-t 0/6] tests/kms_pipe_crc_basic: add mst suspend-resume test Kunal Joshi
` (3 preceding siblings ...)
2026-04-01 11:55 ` [PATCH i-g-t 4/6] lib/igt_kms: Export get_max_pipe_hdisplay as public helper Kunal Joshi
@ 2026-04-01 11:55 ` Kunal Joshi
2026-04-01 11:55 ` [PATCH i-g-t 6/6] tests/kms_pipe_crc_basic: Add MST suspend-resume test Kunal Joshi
5 siblings, 0 replies; 15+ messages in thread
From: Kunal Joshi @ 2026-04-01 11:55 UTC (permalink / raw)
To: igt-dev; +Cc: Kunal Joshi
Skip when the returned mode's hdisplay is below the
platform's single-pipe limit.
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
tests/intel/kms_joiner.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tests/intel/kms_joiner.c b/tests/intel/kms_joiner.c
index 86226a3ba..5f3914ca1 100644
--- a/tests/intel/kms_joiner.c
+++ b/tests/intel/kms_joiner.c
@@ -572,6 +572,7 @@ static void test_ultra_joiner(data_t *data, bool invalid_pipe, bool two_display,
static void test_basic_max_non_joiner(data_t *data)
{
igt_display_t *display = &data->display;
+ int max_hdisplay = igt_get_max_pipe_hdisplay(data->drm_fd);
int count;
enum pipe pipe;
igt_output_t **outputs, *output;
@@ -597,6 +598,9 @@ static void test_basic_max_non_joiner(data_t *data)
igt_require(max_non_joiner_mode_found(data->drm_fd,
output->config.connector,
max_dotclock, &mode));
+ igt_require_f(mode.hdisplay >= max_hdisplay,
+ "Max non-joiner mode %dx%d below single-pipe limit %d\n",
+ mode.hdisplay, mode.vdisplay, max_hdisplay);
igt_output_override_mode(output, &mode);
igt_info("Appplying mode = %dx%d@%d\n", mode.hdisplay,
mode.vdisplay, mode.vrefresh);
--
2.48.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH i-g-t 6/6] tests/kms_pipe_crc_basic: Add MST suspend-resume test
2026-04-01 11:55 [PATCH i-g-t 0/6] tests/kms_pipe_crc_basic: add mst suspend-resume test Kunal Joshi
` (4 preceding siblings ...)
2026-04-01 11:55 ` [PATCH i-g-t 5/6] tests/intel/kms_joiner: Require boundary mode for basic-max-non-joiner Kunal Joshi
@ 2026-04-01 11:55 ` Kunal Joshi
2026-04-01 11:52 ` Jani Nikula
5 siblings, 1 reply; 15+ messages in thread
From: Kunal Joshi @ 2026-04-01 11:55 UTC (permalink / raw)
To: igt-dev; +Cc: Kunal Joshi, Mohammed Thasleem
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
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') ],
}
--
2.48.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH i-g-t 3/6] lib/igt_kms: Fix max_non_joiner_mode_found
2026-04-01 11:44 ` Jani Nikula
@ 2026-04-01 11:57 ` Joshi, Kunal1
0 siblings, 0 replies; 15+ messages in thread
From: Joshi, Kunal1 @ 2026-04-01 11:57 UTC (permalink / raw)
To: Jani Nikula, igt-dev; +Cc: Mohammed Thasleem, Suraj Kandpal
Hello Jani,
On 01-04-2026 17:14, Jani Nikula wrote:
> On Wed, 01 Apr 2026, Kunal Joshi <kunal1.joshi@intel.com> wrote:
>> Previously the function, instead of giving the highest possible mode the
>> connector could support, started the check by looking if the mode was
>> equal to max_pipe_hdisplay. There may be corner cases where a connector never supports the highest mode our source can
>> drive (e.g. a 4K mode on a 5K-capable platform).
>>
>> Fix this by iterating all modes and tracking the best one (highest
>> hdisplay, then highest clock as a tiebreaker) that does not require a
>> big joiner, as determined by igt_bigjoiner_possible(). This correctly
>> handles connectors with any max resolution.
> I'm not requesting changes right now, but I keep wondering about having
> the joiner code in igt_kms.[ch] at all. It's an Intel implementation
> detail. Why is so much of that in the generic file instead of hidden in
> some Intel specific file or directory?
>
> BR,
> Jani.
Yeah right,
We actually have a helper for joiner related function (kms_joiner_helper),
Do you think that this approach makes sense to have feature helpers?
Thanks and Regards
Kunal Joshi
>
>
>> v2: Reword commit message (Suraj)
>>
>> Fixes: 3830ca6a5068 ("lib/igt_kms: Add support to check joiner mode limit")
>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>> Reviewed-by: Mohammed Thasleem <mohammed.thasleem@intel.com>
>> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
>> ---
>> lib/igt_kms.c | 17 +++++++++++------
>> 1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 04b1f3972..dbd419a9b 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -6979,21 +6979,26 @@ bool bigjoiner_mode_found(int drm_fd, drmModeConnector *connector,
>> * false otherwise.
>> */
>> bool max_non_joiner_mode_found(int drm_fd, drmModeConnector *connector,
>> - int max_dotclock, drmModeModeInfo *mode)
>> + int max_dotclock, drmModeModeInfo *mode)
>> {
>> - int max_hdisplay = get_max_pipe_hdisplay(drm_fd);
>> + bool found = false;
>>
>> for (int i = 0; i < connector->count_modes; i++) {
>> drmModeModeInfo *current_mode = &connector->modes[i];
>>
>> - if (current_mode->hdisplay == max_hdisplay &&
>> - current_mode->clock < max_dotclock) {
>> + if (igt_bigjoiner_possible(drm_fd, current_mode, max_dotclock))
>> + continue;
>> +
>> + if (!found ||
>> + current_mode->hdisplay > mode->hdisplay ||
>> + (current_mode->hdisplay == mode->hdisplay &&
>> + current_mode->clock > mode->clock)) {
>> *mode = *current_mode;
>> - return true;
>> + found = true;
>> }
>> }
>>
>> - return false;
>> + return found;
>> }
>>
>> /* TODO: Move these lib functions to the joiner-specific library file
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH i-g-t 6/6] tests/kms_pipe_crc_basic: Add MST suspend-resume test
2026-04-01 11:52 ` Jani Nikula
@ 2026-04-01 11:59 ` Joshi, Kunal1
2026-04-01 12:06 ` Ville Syrjälä
1 sibling, 0 replies; 15+ messages in thread
From: Joshi, Kunal1 @ 2026-04-01 11:59 UTC (permalink / raw)
To: Jani Nikula, igt-dev; +Cc: Mohammed Thasleem
Hello Jani,
On 01-04-2026 17:22, Jani Nikula wrote:
> 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.
Agreed, Will add checks.
Thanks and Regards
Kunal Joshi
>
>> 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') ],
>> }
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH i-g-t 6/6] tests/kms_pipe_crc_basic: Add MST suspend-resume test
2026-04-01 11:52 ` Jani Nikula
2026-04-01 11:59 ` Joshi, Kunal1
@ 2026-04-01 12:06 ` Ville Syrjälä
2026-04-01 12:17 ` Joshi, Kunal1
1 sibling, 1 reply; 15+ messages in thread
From: Ville Syrjälä @ 2026-04-01 12:06 UTC (permalink / raw)
To: Jani Nikula; +Cc: Kunal Joshi, igt-dev, Mohammed Thasleem
On Wed, Apr 01, 2026 at 02:52:12PM +0300, Jani Nikula wrote:
> 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,
I don't understand even the MST part. What does the "MST vs. not"
have to do verifying that pipe CRCs work?
Maybe the intention here is the opposite? As in verify that MST
suspend/resume works, and implement that by *using* pipe CRCs.
In that case this whole thing should be a completely different
test and not shoehorned into kms_pipe_crc_basic.
> 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
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH i-g-t 6/6] tests/kms_pipe_crc_basic: Add MST suspend-resume test
2026-04-01 12:06 ` Ville Syrjälä
@ 2026-04-01 12:17 ` Joshi, Kunal1
0 siblings, 0 replies; 15+ messages in thread
From: Joshi, Kunal1 @ 2026-04-01 12:17 UTC (permalink / raw)
To: Ville Syrjälä, Jani Nikula; +Cc: igt-dev, Mohammed Thasleem
Hello Ville,
On 01-04-2026 17:36, Ville Syrjälä wrote:
> On Wed, Apr 01, 2026 at 02:52:12PM +0300, Jani Nikula wrote:
>> 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,
> I don't understand even the MST part. What does the "MST vs. not"
> have to do verifying that pipe CRCs work?
>
> Maybe the intention here is the opposite? As in verify that MST
> suspend/resume works, and implement that by *using* pipe CRCs.
> In that case this whole thing should be a completely different
> test and not shoehorned into kms_pipe_crc_basic.
Yes, that is the intent: validate MST suspend/resume and use pipe CRC as
the correctness check. I agree this does not fit well within
`kms_pipe_crc_basic`, since CRC is only incidental here and MST
suspend/resume is the real feature under test. I’ll split this out into
a separate test.
Thanks and Regards
Kunal Joshi
>> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH i-g-t 4/6] lib/igt_kms: Export get_max_pipe_hdisplay as public helper
2026-04-01 11:47 ` Jani Nikula
@ 2026-04-01 14:46 ` Joshi, Kunal1
0 siblings, 0 replies; 15+ messages in thread
From: Joshi, Kunal1 @ 2026-04-01 14:46 UTC (permalink / raw)
To: Jani Nikula, igt-dev
Hello Jani,
On 01-04-2026 17:17, Jani Nikula wrote:
> On Wed, 01 Apr 2026, Kunal Joshi <kunal1.joshi@intel.com> wrote:
>> Rename the static get_max_pipe_hdisplay to igt_get_max_pipe_hdisplay
>> and export it as a public API. This function returns the maximum
>> hdisplay a single pipe can drive. Tests like basic-max-non-joiner
>> need this value to distinguish outputs that can reach the single-pipe
>> boundary from those that cannot
>>
>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>> ---
>> lib/igt_kms.c | 8 ++++----
>> lib/igt_kms.h | 1 +
>> 2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index dbd419a9b..961538932 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -6907,12 +6907,12 @@ int igt_get_current_cdclk(int fd)
>> }
>>
>> /**
>> - * get_max_hdisplay:
>> + * igt_get_max_pipe_hdisplay:
>> * @drm_fd: drm file descriptor
>> *
>> * Returns: The maximum hdisplay supported per pipe.
>> */
>> -static int get_max_pipe_hdisplay(int drm_fd)
>> +int igt_get_max_pipe_hdisplay(int drm_fd)
> This is purely about Intel limitations, isn't it? The name is misleading
> for anything else.
>
> BR,
> Jani.
Correct, Will add intel prefix.
Thanks and Regards
Kunal Joshi
>
>> {
>> int dev_id = intel_get_drm_devid(drm_fd);
>>
>> @@ -6933,7 +6933,7 @@ static int get_max_pipe_hdisplay(int drm_fd)
>> */
>> bool igt_bigjoiner_possible(int drm_fd, drmModeModeInfo *mode, int max_dotclock)
>> {
>> - return (mode->hdisplay > get_max_pipe_hdisplay(drm_fd) ||
>> + return (mode->hdisplay > igt_get_max_pipe_hdisplay(drm_fd) ||
>> mode->clock > max_dotclock);
>> }
>>
>> @@ -7047,7 +7047,7 @@ bool igt_is_joiner_enabled_for_pipe(int drmfd, enum pipe pipe)
>> */
>> bool igt_ultrajoiner_possible(int drm_fd, drmModeModeInfo *mode, int max_dotclock)
>> {
>> - return (mode->hdisplay > 2 * get_max_pipe_hdisplay(drm_fd) ||
>> + return (mode->hdisplay > 2 * igt_get_max_pipe_hdisplay(drm_fd) ||
>> mode->clock > 2 * max_dotclock);
>> }
>>
>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> index f994d91d3..2b5196c90 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -1236,6 +1236,7 @@ void igt_sort_connector_modes(drmModeConnector *connector,
>> bool igt_max_bpc_constraint(igt_display_t *display, igt_crtc_t *crtc,
>> igt_output_t *output, int bpc);
>> int igt_get_max_dotclock(int fd);
>> +int igt_get_max_pipe_hdisplay(int drm_fd);
>> int igt_get_max_cdclk(int fd);
>> int igt_get_current_cdclk(int fd);
>> bool igt_bigjoiner_possible(int drm_fd, drmModeModeInfo *mode, int max_dotclock);
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-04-01 14:46 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-01 11:55 [PATCH i-g-t 0/6] tests/kms_pipe_crc_basic: add mst suspend-resume test Kunal Joshi
2026-04-01 11:55 ` [PATCH i-g-t 1/6] tests/intel/kms_mst_helper: Add kernel-doc for existing function Kunal Joshi
2026-04-01 11:55 ` [PATCH i-g-t 2/6] tests/intel/kms_mst_helper: Add helper to check for MST outputs Kunal Joshi
2026-04-01 11:55 ` [PATCH i-g-t 3/6] lib/igt_kms: Fix max_non_joiner_mode_found Kunal Joshi
2026-04-01 11:44 ` Jani Nikula
2026-04-01 11:57 ` Joshi, Kunal1
2026-04-01 11:55 ` [PATCH i-g-t 4/6] lib/igt_kms: Export get_max_pipe_hdisplay as public helper Kunal Joshi
2026-04-01 11:47 ` Jani Nikula
2026-04-01 14:46 ` Joshi, Kunal1
2026-04-01 11:55 ` [PATCH i-g-t 5/6] tests/intel/kms_joiner: Require boundary mode for basic-max-non-joiner Kunal Joshi
2026-04-01 11:55 ` [PATCH i-g-t 6/6] tests/kms_pipe_crc_basic: Add MST suspend-resume test Kunal Joshi
2026-04-01 11:52 ` Jani Nikula
2026-04-01 11:59 ` Joshi, Kunal1
2026-04-01 12:06 ` Ville Syrjälä
2026-04-01 12:17 ` Joshi, Kunal1
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox