[PATCH i-g-t] tests/intel/kms_cdclk: Refactor mode setting and CD clock verification

[Swati Sharma]

Add conditions to filter valid outputs. Encapsulate the
mode-setting functionality in the set_mode function to reduce
redundancy. Remove redundant framebuffer and size setting
operations for the plane. Simplify the CD clock frequency
checks to ensure it increases after the mode transition. Clean
up unnecessary plane operations and redundant framebuffer
handling. Enhance the readability and modularity of the mode
transition logic to handle multiple outputs more effectively.

Signed-off-by: Swati Sharma <swati2.sharma at intel.com>
---
 tests/intel/kms_cdclk.c | 112 +++++++++++++++++++---------------------
 1 file changed, 54 insertions(+), 58 deletions(-)

diff --git a/tests/intel/kms_cdclk.c b/tests/intel/kms_cdclk.c
index 382b3e9d1..93276f45e 100644
--- a/tests/intel/kms_cdclk.c
+++ b/tests/intel/kms_cdclk.c
@@ -54,6 +54,7 @@ IGT_TEST_DESCRIPTION("Test cdclk features : crawling and squashing");
 #define VDISPLAY_4K	2160
 #define VREFRESH	60
 #define MAX_CDCLK_4K	307200
+#define MIN_DISPLAYS	2
 
 /* Test flags */
 enum {
@@ -269,90 +270,86 @@ static void test_mode_transition(data_t *data, enum pipe pipe, igt_output_t *out
 	igt_remove_fb(display->drm_fd, &fb);
 }
 
+
+static void set_mode(data_t *data, drmModeModeInfo *mode, int count,
+		     struct igt_fb fb, igt_output_t **valid_outputs)
+{
+	igt_display_t *display = &data->display;
+	igt_pipe_t *pipe;
+	igt_plane_t *plane;
+
+	for (int k = 0; k < count; k++) {
+		pipe = &display->pipes[k];
+		plane = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
+
+		igt_output_override_mode(valid_outputs[k], &mode[k]);
+
+		igt_plane_set_fb(plane, &fb);
+		igt_fb_set_size(&fb, plane, mode[k].hdisplay, mode[k].vdisplay);
+		igt_plane_set_size(plane, mode[k].hdisplay, mode[k].vdisplay);
+	}
+}
+
 static void test_mode_transition_on_all_outputs(data_t *data)
 {
 	igt_display_t *display = &data->display;
 	int debugfs_fd = data->debugfs_fd;
-	drmModeModeInfo *mode, *mode_hi, *mode_lo;
+	drmModeModeInfo *mode, mode_hi[IGT_MAX_PIPES] = {0}, mode_lo[IGT_MAX_PIPES] = {0};
+	igt_output_t *valid_outputs[IGT_MAX_PIPES] = {NULL};
 	igt_output_t *output;
-	int valid_outputs = 0;
+	int count = 0;
 	int cdclk_ref, cdclk_new;
 	uint16_t width = 0, height = 0;
 	struct igt_fb fb;
-	igt_pipe_t *pipe;
-	igt_plane_t *plane;
-	int i = 0, j = 0;
 
 	do_cleanup_display(display);
 	igt_display_reset(display);
 
-	for_each_connected_output(&data->display, output)
-		valid_outputs++;
-
-	i = 0;
 	for_each_connected_output(display, output) {
-		mode = igt_output_get_mode(output);
-		igt_assert(mode);
+		const drmModeModeInfo *highres_mode;
+		const drmModeModeInfo *lowres_mode;
 
-		width = max(width, mode->hdisplay);
-		height = max(height, mode->vdisplay);
+		highres_mode = get_highres_mode(output);
+		igt_require(highres_mode != NULL);
+		mode_hi[count] = *highres_mode;
 
-		mode_hi = get_highres_mode(output);
-		igt_require(mode_hi != NULL);
+		lowres_mode = get_lowres_mode(output);
+		mode_lo[count] = *lowres_mode;
 
-		igt_output_set_pipe(output, i);
-		igt_output_override_mode(output, mode_hi);
-		i++;
-	}
-	igt_require(intel_pipe_output_combo_valid(display));
-	igt_display_reset(display);
+		if (mode_hi[count].hdisplay == mode_lo[count].hdisplay &&
+		    mode_hi[count].vdisplay == mode_lo[count].vdisplay) {
+			igt_debug("Highest and lowest mode resolutions are same; no transition\n");
+			continue;
+		}
 
-	igt_create_pattern_fb(data->drm_fd, width, height, DRM_FORMAT_XRGB8888,
-			      DRM_FORMAT_MOD_LINEAR, &fb);
-	i = 0;
-	for_each_connected_output(display, output) {
-		pipe = &display->pipes[i];
-		plane = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
+		valid_outputs[count++] = output;
+	}
 
-		mode = NULL;
+	igt_skip_on_f(count < MIN_DISPLAYS,
+		      "Number of valid outputs (%d) must be greater than or equal to the "
+		      "minimum required displays (%d)\n", count, MIN_DISPLAYS);
 
-		igt_output_set_pipe(output, i);
-		mode = igt_output_get_mode(output);
+	for (int i = 0; i < count; i++) {
+		mode = igt_output_get_mode(valid_outputs[i]);
 		igt_assert(mode);
 
-		mode_lo = get_lowres_mode(output);
+		width = max(width, mode->hdisplay);
+		height = max(height, mode->vdisplay);
 
-		igt_output_override_mode(output, mode_lo);
-		igt_plane_set_fb(plane, &fb);
-		igt_fb_set_size(&fb, plane, mode_lo->hdisplay, mode_lo->vdisplay);
-		igt_plane_set_size(plane, mode_lo->hdisplay, mode_lo->vdisplay);
-		i++;
+		igt_output_set_pipe(valid_outputs[i], i);
+		igt_output_override_mode(valid_outputs[i], &mode_hi[i]);
 	}
 
-	igt_display_commit2(display, COMMIT_ATOMIC);
-	cdclk_ref = get_current_cdclk_freq(debugfs_fd);
-
-	j = 0;
-	for_each_connected_output(display, output) {
-		pipe = &display->pipes[j];
-		plane = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
-
-		mode = NULL;
-
-		igt_output_set_pipe(output, j);
-		mode = igt_output_get_mode(output);
-		igt_assert(mode);
+	igt_require(intel_pipe_output_combo_valid(display));
 
-		mode_hi = get_highres_mode(output);
-		igt_require(mode_hi != NULL);
+	igt_create_pattern_fb(data->drm_fd, width, height, DRM_FORMAT_XRGB8888,
+			      DRM_FORMAT_MOD_LINEAR, &fb);
 
-		igt_output_override_mode(output, mode_hi);
-		igt_plane_set_fb(plane, &fb);
-		igt_fb_set_size(&fb, plane, mode_hi->hdisplay, mode_hi->vdisplay);
-		igt_plane_set_size(plane, mode_hi->hdisplay, mode_hi->vdisplay);
-		j++;
-	}
+	set_mode(data, mode_lo, count, fb, valid_outputs);
+	igt_display_commit2(display, COMMIT_ATOMIC);
+	cdclk_ref = get_current_cdclk_freq(debugfs_fd);
 
+	set_mode(data, mode_hi, count, fb, valid_outputs);
 	igt_display_commit2(display, COMMIT_ATOMIC);
 	cdclk_new = get_current_cdclk_freq(debugfs_fd);
 	igt_info("CD clock frequency %d -> %d\n", cdclk_ref, cdclk_new);
@@ -360,7 +357,6 @@ static void test_mode_transition_on_all_outputs(data_t *data)
 	/* cdclk should bump */
 	igt_assert_lt(cdclk_ref, cdclk_new);
 
-	igt_plane_set_fb(plane, NULL);
 	do_cleanup_display(display);
 	igt_remove_fb(data->drm_fd, &fb);
 }
-- 
2.25.1