From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3ED89D35177 for ; Wed, 1 Apr 2026 12:17:51 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D937110E21F; Wed, 1 Apr 2026 12:17:50 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="OmtLKXeL"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.10]) by gabe.freedesktop.org (Postfix) with ESMTPS id D0AEA10E21F for ; Wed, 1 Apr 2026 12:17:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1775045863; x=1806581863; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=/4OkmQ7ONKHkLrEDJwTEmixvpmYioRsdxNULh7ltouA=; b=OmtLKXeLpoM6a+mzEzqdo9p8vpmXwnPTCyFRBTzavlVsFiRsJ5HnSGMN qLlEXbBo5ACjiQA/+Pqv+VGyAyWl6CTIbDS7YS/KY2kuTErCmpMxtBjn6 51Y36EsWOo/FUS3fFrZ2Q33PsxahTlfnLgtkCiKnMgi9+sk0ZylzOrjT1 sNA8dAxPCwRB+Uk5an9+i77Kb9lP7FKezaZugeN7ISODRc01PSBVN+GLI jd/xab0ej2bAAvpSfmw6hyVGUy0lbJ9Y8ZetVEmeJmpdL93KgHzNxLOFa 5ggJpqZtld9d7I5Kwd9z3gjZ6WdZqvuwo6X0ySCMP6cn8dakLChnB/9cI Q==; X-CSE-ConnectionGUID: /F5NRo9MRDyoTIxJfFA+6Q== X-CSE-MsgGUID: ZDWpTstJTxeywRJyKbBoNg== X-IronPort-AV: E=McAfee;i="6800,10657,11745"; a="93472318" X-IronPort-AV: E=Sophos;i="6.23,153,1770624000"; d="scan'208";a="93472318" Received: from orviesa008.jf.intel.com ([10.64.159.148]) by orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Apr 2026 05:17:43 -0700 X-CSE-ConnectionGUID: S9TD9RgQRh2H0tQHhtTmVA== X-CSE-MsgGUID: YnFu3vHXSB+fOwHcnVMUqA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.23,153,1770624000"; d="scan'208";a="226643102" Received: from pravik2x-mobl.gar.corp.intel.com (HELO [10.247.196.34]) ([10.247.196.34]) by orviesa008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Apr 2026 05:17:41 -0700 Message-ID: Date: Wed, 1 Apr 2026 17:47:38 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH i-g-t 6/6] tests/kms_pipe_crc_basic: Add MST suspend-resume test To: =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Jani Nikula Cc: igt-dev@lists.freedesktop.org, Mohammed Thasleem References: <20260401115542.1074290-1-kunal1.joshi@intel.com> <20260401115542.1074290-7-kunal1.joshi@intel.com> <996f8fca71b8e9afad16b0a4ca3331636bfb34ac@intel.com> Content-Language: en-US From: "Joshi, Kunal1" In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: igt-dev@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development mailing list for IGT GPU Tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" 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 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 >>> Reviewed-by: Mohammed Thasleem >>> --- >>> 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 >>> +#include >>> #include >>> #include >>> #include >>> @@ -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