Re: [PATCH v2 i-g-t] kms_atomic: Measure speed of some plane ioctls

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 04/27/2015 10:20 PM, Chris Wilson wrote:
On Mon, Apr 27, 2015 at 04:34:05PM +0100, Tvrtko Ursulin wrote:
+static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
+			 igt_plane_t *plane)
+{
+	drmModeModeInfo *mode;
+	igt_display_t *display = &data->display;
+	int fb_id, fb_modeset_id;
+	unsigned int w, h;
+	uint64_t tiling = LOCAL_DRM_FORMAT_MOD_NONE;
+	uint32_t pixel_format = DRM_FORMAT_XRGB8888;
+	enum igt_commit_style commit = COMMIT_LEGACY;
+	igt_plane_t *primary;
+
+	igt_output_set_pipe(output, pipe);
+
+	mode = igt_output_get_mode(output);
+
+	w = mode->hdisplay;
+	h = mode->vdisplay;
+
+	fb_modeset_id = igt_create_fb(data->gfx_fd,
+				      w, h,
+				      pixel_format,
+				      tiling,
+				      &data->fb_modeset);
+	igt_assert(fb_modeset_id);
+
+	/*
+	 * With igt_display_commit2 and COMMIT_UNIVERSAL, we call just the
+	 * setplane without a modeset. So, to be able to call
+	 * igt_display_commit and ultimately setcrtc to do the first modeset,
+	 * we create an fb covering the crtc and call commit
+	 */
+
+	primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+	igt_plane_set_fb(primary, &data->fb_modeset);
+	igt_display_commit(display);
+
+	fb_id = igt_create_fb(data->gfx_fd,
+			      w, h,
+			      pixel_format,
+			      tiling,
+			      &data->fb);
+	igt_assert(fb_id);
+
+	igt_plane_set_fb(plane, &data->fb);
+

Please do commit = COMMIT_LEGACY here so I don't have to scan backwads.

Yeah makes sense, too much copy&paste.

+	if (plane->is_primary || plane->is_cursor) {
+		igt_require(data->display.has_universal_planes);
+		commit = COMMIT_UNIVERSAL;
+	}
+
+	igt_display_commit2(display, commit);
+}
+
+static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
+{
+	igt_display_t *display = &data->display;
+
+	igt_remove_fb(data->gfx_fd, &data->fb);
+	igt_remove_fb(data->gfx_fd, &data->fb_modeset);
+
+	/* XXX: see the note in prepare_crtc() */
+	if (!plane->is_primary) {
+		igt_plane_t *primary;
+
+		primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+		igt_plane_set_fb(primary, NULL);
+	}
+
+	igt_plane_set_fb(plane, NULL);
+	igt_output_set_pipe(output, PIPE_ANY);
+
+	igt_display_commit(display);
+}
+
+static double elapsed(const struct timeval *start,
+		      const struct timeval *end,
+		      int loop)
+{
+	return (1e6*(end->tv_sec - start->tv_sec) + (end->tv_usec - start->tv_usec))/loop;
+}
+
+static void test_commit_speed(data_t *data, enum igt_plane plane_type)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output;
+	enum pipe pipe;
+	enum igt_commit_style commit = COMMIT_LEGACY;
+	unsigned int i;
+	const unsigned int loops = 10000;
+	struct timeval start, end;
+	double e;
+

Same comment for commit = COMMIT_LEGACY here, mainly now for consistency
with before.

Here we want to be able to choose between commit types if we want to keep two subtests.

+	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
+		igt_require(data->display.has_universal_planes);
+		commit = COMMIT_UNIVERSAL;
+	}
+
+	for_each_connected_output(display, output) {
+		for_each_pipe(display, pipe) {
+			igt_plane_t *plane;
+
+			igt_output_set_pipe(output, pipe);
+			plane = igt_output_get_plane(output, plane_type);
+
+			prepare_crtc(data, output, pipe, plane);
+
+			igt_display_commit2(display, commit);
+
+			gettimeofday(&start, NULL);
+			for (i = loops; i > 0; i--) {
+				plane->position_changed = true;
+				igt_plane_commit(plane, output, commit, true);
+			}
+			gettimeofday(&end, NULL);
+			e = elapsed(&start, &end, loops);
+			igt_info("Pipe: %u Plane: %u Time: %7.3fµs\n",
+				 pipe, plane->index, e);
+			igt_require(e < 1000); /* 1ms, just so. */
+
+			kmstest_restore_vt_mode();
+			kmstest_set_vt_graphics_mode();

Do you really mean to switch VT between text/gfx on every display/pipe?
Why?

Because copy&paste is so easy. :)

+
+			cleanup_crtc(data, output, plane);
+		}
+	}
+}

Looks like magic to me. Is this enough variation to flush out future
bugs?

On which part does this apply?

Regards,

Tvrtko


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux