On Thu, Aug 05, 2021 at 12:06:22PM -0400, Bindu Ramamurthy wrote:

i is already a valid PIPE_x here, no need to PIPE_A + i.

But beware of holes in your array. for_each_pipe goes through valid
pipes and the set of valid pipes can be PIPE_A, PIPE_C, leaving PIPE_B
(index 1) uninitialized.

+		data->pipe[i] = &data->display.pipes[data->pipe_id[i]];
+		data->primary[i] = igt_pipe_get_plane_type(
+			data->pipe[i], DRM_PLANE_TYPE_PRIMARY);
+		data->pipe_crc[i] =
+			igt_pipe_crc_new(data->fd, data->pipe_id[i], "auto");
+	}
+
+	for (i = 0; i < display->n_outputs && i < max_pipes; i++) {
+		igt_output_t *output = &display->outputs[i];
+
+		data->output[i] = output;
+
+		/* Only allow physically connected displays for the tests. */
+		if (!igt_output_is_connected(output))
+			continue;
+
+		igt_assert(kmstest_get_connector_default_mode(
+			data->fd, output->config.connector, &data->mode[i]));
+
+		data->w[i] = data->mode[i].hdisplay;
+		data->h[i] = data->mode[i].vdisplay;
+	}
+
+
+	igt_require(data->output[0]);
+	igt_display_reset(display);
+}
+
+static void test_fini(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	int i;
+
+	for_each_pipe(display, i) {
+		igt_pipe_crc_free(data->pipe_crc[i]);
+	}
+
+	igt_display_reset(display);
+	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, 0);
+}
+
+/* Forces a mode for a connector. */
+static void force_output_mode(data_t *d, igt_output_t *output,
+			      const drmModeModeInfo *mode)
+{
+	/* This allows us to create a virtual sink. */
+	if (!igt_output_is_connected(output)) {
+		kmstest_force_edid(d->fd, output->config.connector,
+				   igt_kms_get_4k_edid());
+
+		kmstest_force_connector(d->fd, output->config.connector,
+					FORCE_CONNECTOR_DIGITAL);
+	}
+
+	igt_output_override_mode(output, mode);
+}
+
+static void run_test_linear_tiling(data_t *data, int pipe, const drmModeModeInfo *mode) {
+	igt_display_t *display = &data->display;
+	igt_output_t *output;
+	struct igt_fb buffer[IGT_MAX_PIPES];
+	igt_crc_t zero, captured[IGT_MAX_PIPES];
+	int i = 0;
+
+
+	igt_skip_on_f(pipe >= igt_display_get_n_pipes(display),
+                      "ASIC does not have %d pipes\n", pipe);

The funky fused pipes support for i915 mentioned above has caused
igt_display_get_n_pipes to not quite do what you want to do here. It
always gives IGT_MAX_PIPES regardless of the amount of valid crtcs.

To get the amount of valid pipes you can do

enum pipe pipe;
num_pipes = 0;
for_each_pipe(display, pipe) num_pipes++;

+
+	test_init(data);
+
+	/* create buffers */
+	for (i = 0; i <= pipe; i++) {

Looping through the arrays in data this way accesses the holes
mentioned above.

+
+		output = data->output[i];
+		if (!output) {
+			continue;
+		}
+
+		force_output_mode(data, output, mode);
+
+		igt_create_color_fb(display->drm_fd, test_mode[0].hdisplay,
+				    test_mode[0].vdisplay, DRM_FORMAT_XRGB8888,
+				    DRM_FORMAT_MOD_NONE, 1.f, 0.f, 0.f,
+				    &buffer[i]);
+
+		igt_output_set_pipe(output, i);
+
+		igt_plane_set_fb(data->primary[i], &buffer[i]);
+	}
+
+	igt_display_commit_atomic(
+		display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+
+
+	for (i = 0; i <= pipe; i++) {
+		igt_pipe_crc_collect_crc(data->pipe_crc[i], &captured[i]);
+		igt_assert_f(!igt_check_crc_equal(&zero, &captured[i]),
+			     "CRC is zero\n");
+		igt_remove_fb(display->drm_fd, &buffer[i]);
+	}
+
+	test_fini(data);
+}
+
+igt_main
+{
+	data_t data;
+	int i = 0, j = 0;
+
+
+	memset(&data, 0, sizeof(data));
+
+	igt_fixture
+	{
+		data.fd = drm_open_driver_master(DRIVER_ANY);
+
+		kmstest_set_vt_graphics_mode();
+
+		igt_display_require(&data.display, data.fd);
+		igt_require(&data.display.is_atomic);
+		igt_display_require_output(&data.display);
+
+	}
+
+	for (i = 0; i < IGT_MAX_PIPES; i++)

I was going to suggest using for_each_pipe_static for this but
conceptually this makes more sense, as it's going for the _amount_ of
pipes.

Please add comment here that explains that.


-- 
Petri Latvala