public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] tests/kms_hdr: Move capability checks inside dynamic subtests
@ 2026-04-27 14:49 Pranay Samala
  2026-04-28  0:18 ` ✗ i915.CI.BAT: failure for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Pranay Samala @ 2026-04-27 14:49 UTC (permalink / raw)
  To: igt-dev; +Cc: karthik.b.s, sameer.lattannavar, pranay.samala

Earlier, output capability checks were done in the outer loops
using continue/break. This caused a problem where if an earlier
output (e.g., eDP) passed but a later output (e.g., DP) failed
a check (such as reading bpc), the entire subtest would be skipped,
discarding the earlier valid result and reporting a false skip/failure.

Fix this by moving all output capability checks inside dynamic
block. Each pipe-output-format combination is now an independent
dynamic subtest that passes or skips on its own merit, without
affecting other combinations.

Closes: https://gitlab.freedesktop.org/drm/xe/kernel/issues/4879
Signed-off-by: Pranay Samala <pranay.samala@intel.com>
---
 tests/kms_hdr.c | 211 ++++++++++++++++++++++--------------------------
 1 file changed, 96 insertions(+), 115 deletions(-)

diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c
index b215b0e6c..20838469b 100644
--- a/tests/kms_hdr.c
+++ b/tests/kms_hdr.c
@@ -325,48 +325,41 @@ static void test_bpc_switch(data_t *data, uint32_t flags)
 	for_each_connected_output(display, output) {
 		igt_crtc_t *crtc;
 
-		if (!has_max_bpc(output)) {
-			igt_info("%s: Doesn't support IGT_CONNECTOR_MAX_BPC.\n",
-				 igt_output_name(output));
-			continue;
-		}
-
-		if (igt_get_output_max_bpc(output) < 10) {
-			igt_info("%s: Doesn't support 10 bpc.\n", igt_output_name(output));
-			continue;
-		}
-
 		for_each_crtc(display, crtc) {
-			igt_output_set_crtc(output,
-					    crtc);
-			if (!intel_pipe_output_combo_valid(display)) {
-				igt_output_set_crtc(output, NULL);
-				continue;
-			}
-
 			for (int i = 0; i < ARRAY_SIZE(hdr_test_formats); i++) {
-				prepare_test(data, output,
-					     crtc);
-
-				if (is_intel_device(data->fd) &&
-				    !igt_max_bpc_constraint(display, crtc, output, 10)) {
-					igt_info("%s: No suitable mode found to use 10 bpc.\n",
-						  igt_output_name(output));
-
-					test_fini(data);
-					break;
-				}
-
-				data->mode = igt_output_get_mode(output);
-				data->w = data->mode->hdisplay;
-				data->h = data->mode->vdisplay;
-
 				igt_dynamic_f("pipe-%s-%s-%s",
 					      igt_crtc_name(crtc), output->name,
-					      igt_format_str(hdr_test_formats[i]))
-					      test_bpc_switch_on_output(data,
-								       crtc,
-								       output, hdr_test_formats[i], flags);
+					      igt_format_str(hdr_test_formats[i])) {
+					igt_require_f(has_max_bpc(output),
+						      "%s: Doesn't support IGT_CONNECTOR_MAX_BPC.\n",
+						      igt_output_name(output));
+
+					igt_require_f(igt_get_output_max_bpc(output) >= 10,
+						      "%s: Doesn't support 10 bpc.\n",
+						      igt_output_name(output));
+
+					prepare_test(data, output, crtc);
+
+					igt_require_f(intel_pipe_output_combo_valid(display),
+						      "pipe-%s-%s: combo invalid.\n",
+						      igt_crtc_name(crtc), output->name);
+
+					if (is_intel_device(data->fd))
+						igt_require_f(igt_max_bpc_constraint(display,
+										     crtc,
+										     output,
+										     10),
+							      "%s: No suitable mode found to "
+							      "use 10 bpc.\n",
+							      igt_output_name(output));
+
+					data->mode = igt_output_get_mode(output);
+					data->w = data->mode->hdisplay;
+					data->h = data->mode->vdisplay;
+
+					test_bpc_switch_on_output(data, crtc, output,
+								  hdr_test_formats[i], flags);
+				}
 			}
 
 			/* One pipe is enough */
@@ -725,88 +718,76 @@ static void test_hdr(data_t *data, uint32_t flags)
 	for_each_connected_output(display, output) {
 		igt_crtc_t *crtc;
 
-		/* To test HDR, 10 bpc is required, so we need to
-		 * set MAX_BPC property to 10bpc prior to setting
-		 * HDR metadata property. Therefore, checking.
-		 */
-		if (!has_max_bpc(output) || !has_hdr(output)) {
-			igt_info("%s: Doesn't support IGT_CONNECTOR_MAX_BPC or IGT_CONNECTOR_HDR_OUTPUT_METADATA.\n",
-				 igt_output_name(output));
-			continue;
-		}
-
-		/* For negative test, panel should be non-hdr. */
-		if ((flags & TEST_INVALID_HDR) && is_panel_hdr(data, output)) {
-			igt_info("%s: Can't run negative test on HDR panel.\n",
-				 igt_output_name(output));
-			continue;
-		}
-
-		if ((flags & ~TEST_INVALID_HDR) && !is_panel_hdr(data, output)) {
-			igt_info("%s: Can't run HDR tests on non-HDR panel.\n",
-				 igt_output_name(output));
-			continue;
-		}
-
-		if (igt_get_output_max_bpc(output) < 10) {
-			igt_info("%s: Doesn't support 10 bpc.\n", igt_output_name(output));
-			continue;
-		}
-
-		if ((flags & TEST_BRIGHTNESS) && !output_is_internal_panel(output)) {
-			igt_info("%s: Can't run brightness test on non-internal panel.\n",
-				 igt_output_name(output));
-			continue;
-		}
-
 		for_each_crtc(display, crtc) {
-			igt_output_set_crtc(output,
-					    crtc);
-			if (!intel_pipe_output_combo_valid(display)) {
-				igt_output_set_crtc(output, NULL);
-				continue;
-			}
-
 			for (int i = 0; i < ARRAY_SIZE(hdr_test_formats); i++) {
-				prepare_test(data, output,
-					     crtc);
-
-				/* Signal HDR requirement via metadata */
-				fill_hdr_output_metadata_st2084(&hdr);
-				set_hdr_output_metadata(data, &hdr);
-				if (igt_display_try_commit2(display, display->is_atomic ?
-							    COMMIT_ATOMIC : COMMIT_LEGACY)) {
-					igt_info("%s: Couldn't set HDR metadata\n",
-						  igt_output_name(output));
-					test_fini(data);
-					break;
-				}
-
-				if (is_intel_device(data->fd) &&
-				    !igt_max_bpc_constraint(display, crtc, output, 10)) {
-					igt_info("%s: No suitable mode found to use 10 bpc.\n",
-						  igt_output_name(output));
-
-					test_fini(data);
-					break;
-				}
-
-				if (igt_is_dsc_enabled(data->fd, output->name))
-					flags |= TEST_NEEDS_DSC;
-				else
-					flags &= ~TEST_NEEDS_DSC;
-
-				set_hdr_output_metadata(data, NULL);
-				igt_display_commit2(display, display->is_atomic ?
-						    COMMIT_ATOMIC : COMMIT_LEGACY);
-
-				data->mode = igt_output_get_mode(output);
-				data->w = data->mode->hdisplay;
-				data->h = data->mode->vdisplay;
-
 				igt_dynamic_f("pipe-%s-%s-%s",
 					       igt_crtc_name(crtc), output->name,
 					       igt_format_str(hdr_test_formats[i])) {
+					igt_require_f(has_max_bpc(output) && has_hdr(output),
+						      "%s: Doesn't support IGT_CONNECTOR_MAX_BPC "
+						      "or IGT_CONNECTOR_HDR_OUTPUT_METADATA.\n",
+						      igt_output_name(output));
+
+					if (flags & TEST_INVALID_HDR)
+						igt_require_f(!is_panel_hdr(data, output),
+							      "%s: Can't run negative test on "
+							      "HDR panel.\n",
+							      igt_output_name(output));
+
+					if (!(flags & TEST_INVALID_HDR))
+						igt_require_f(is_panel_hdr(data, output),
+							      "%s: Can't run HDR tests on "
+							      "non-HDR panel.\n",
+							      igt_output_name(output));
+
+					igt_require_f(igt_get_output_max_bpc(output) >= 10,
+						      "%s: Doesn't support 10 bpc.\n",
+						      igt_output_name(output));
+
+					if (flags & TEST_BRIGHTNESS)
+						igt_require_f(output_is_internal_panel(output),
+							      "%s: Can't run brightness test on "
+							      "non-internal panel.\n",
+							      igt_output_name(output));
+
+					prepare_test(data, output, crtc);
+
+					igt_require_f(intel_pipe_output_combo_valid(display),
+						      "pipe-%s-%s: combo invalid.\n",
+						      igt_crtc_name(crtc), output->name);
+
+					/* Signal HDR requirement via metadata */
+					fill_hdr_output_metadata_st2084(&hdr);
+					set_hdr_output_metadata(data, &hdr);
+					igt_require_f(!igt_display_try_commit2(display,
+									       display->is_atomic ?
+									       COMMIT_ATOMIC :
+									       COMMIT_LEGACY),
+						      "%s: Couldn't set HDR metadata\n",
+						      igt_output_name(output));
+
+					if (is_intel_device(data->fd))
+						igt_require_f(igt_max_bpc_constraint(display,
+										     crtc,
+										     output,
+										     10),
+							      "%s: No suitable mode found to "
+							      "use 10 bpc.\n",
+							      igt_output_name(output));
+
+					if (igt_is_dsc_enabled(data->fd, output->name))
+						flags |= TEST_NEEDS_DSC;
+					else
+						flags &= ~TEST_NEEDS_DSC;
+
+					set_hdr_output_metadata(data, NULL);
+					igt_display_commit2(display, display->is_atomic ?
+							    COMMIT_ATOMIC : COMMIT_LEGACY);
+
+					data->mode = igt_output_get_mode(output);
+					data->w = data->mode->hdisplay;
+					data->h = data->mode->vdisplay;
+
 					if (flags & (TEST_NONE | TEST_DPMS | TEST_SUSPEND |
 						     TEST_INVALID_HDR | TEST_BRIGHTNESS))
 						test_static_toggle(data,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-04-29  4:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-27 14:49 [PATCH i-g-t] tests/kms_hdr: Move capability checks inside dynamic subtests Pranay Samala
2026-04-28  0:18 ` ✗ i915.CI.BAT: failure for " Patchwork
2026-04-28  1:31 ` ✓ Xe.CI.BAT: success " Patchwork
2026-04-28  9:10 ` ✗ Xe.CI.FULL: failure " Patchwork
2026-04-29  4:17 ` [PATCH i-g-t] " Karthik B S

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox