All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] Idleness DRRS test
@ 2016-08-31  8:16 Nautiyal Ankit
  2016-09-07 14:53 ` Marius Vlad
  0 siblings, 1 reply; 12+ messages in thread
From: Nautiyal Ankit @ 2016-08-31  8:16 UTC (permalink / raw)
  To: intel-gfx, daniel.vetter, chris, rodrigo.vivi; +Cc: aknautiy, paulo.r.zanoni

From: aknautiy <ankit.k.nautiyal@intel.com>

Idleness DRRS:
	By default the DRRS state will be at DRRS_HIGH_RR. When a Display
	content is Idle for more than 1Sec Idleness will be declared and
	DRRS_LOW_RR will be invoked, changing the refresh rate to the
	lower most refresh rate supported by the panel. As soon as there
	is a display content change there will be a DRRS state transition
	as DRRS_LOW_RR--> DRRS_HIGH_RR, changing the refresh rate to the
	highest refresh rate supported by the panel.

To test this, Idleness DRRS IGT will probe the DRRS state at below
instances and compare with the expected state.

	Instance					Expected State
1. Immediately after rendering the still image		DRRS_HIGH_RR
2. After a delay of 1.2Sec				DRRS_LOW_RR
3. After changing the frame buffer			DRRS_HIGH_RR
4. After a delay of 1.2Sec				DRRS_LOW_RR
5. After changing the frame buffer			DRRS_HIGH_RR
6. After a delay of 1.2Sec				DRRS_LOW_RR

The test checks the driver DRRS state from the debugfs entry. To check the
actual refresh-rate, a separate thread counts the number of vblanks
received per sec. The refresh-rate calculated is checked against the
expected refresh-rate with a tolerance value of 2.

This patch is a continuation of the earlier work
https://patchwork.freedesktop.org/patch/45472/ towards igt for idleness

DRRS. The code is tested on Broxton BXT_T platform.

Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com>
---
 tests/Makefile.sources |   1 +
 tests/kms_drrs.c       | 614 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 615 insertions(+)
 create mode 100644 tests/kms_drrs.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index a837977..5f31521 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -91,6 +91,7 @@ TESTS_progs_M = \
 	kms_cursor_crc \
 	kms_cursor_legacy \
 	kms_draw_crc \
+	kms_drrs \
 	kms_fbc_crc \
 	kms_fbcon_fbt \
 	kms_flip \
diff --git a/tests/kms_drrs.c b/tests/kms_drrs.c
new file mode 100644
index 0000000..fbe78c5
--- /dev/null
+++ b/tests/kms_drrs.c
@@ -0,0 +1,614 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "drmtest.h"
+#include "igt_debugfs.h"
+#include "igt_kms.h"
+#include "intel_chipset.h"
+#include "intel_batchbuffer.h"
+#include "ioctl_wrappers.h"
+#include <time.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <sys/time.h>
+IGT_TEST_DESCRIPTION(
+"Performs write operations and then waits for DRRS to invoke the"
+"Low Refresh Rate and then disturbs the contents of the screen once"
+"again hence DRRS revert back to High Refresh Rate(Default).");
+
+#define DRRS_STATUS_BYTES_CNT	1000
+#define SET	1
+#define RESET	0
+
+/*
+ * Structure to store data to create 2 framebuffers, fb[0] and fb[1] on a given
+ * display. To disturb the content of the screen, we replace fb[0] by fb[1] and
+ * vice versa.
+ */
+typedef struct {
+	int drm_fd;
+	uint32_t devid;
+	uint32_t handle[2];
+	igt_display_t display;
+	igt_output_t *output;
+	enum pipe pipe;
+	igt_plane_t *primary;
+	struct igt_fb fb[2];
+	uint32_t fb_id[2];
+} data_t;
+
+/*
+ * Structure to count vblank and note the starting time of the counter
+ */
+typedef struct {
+	unsigned int vbl_count;
+	struct timeval start;
+} vbl_info;
+
+/*
+ * Condition variables,mutex, and shared variables for thread synchronization
+ */
+pthread_mutex_t rr_mutex;
+pthread_cond_t cv_rr_calc_requested;
+pthread_cond_t cv_rr_calc_completed;
+int rr_calc_requested = RESET;
+int rr_calc_completed = RESET;
+int thread_flag = SET;
+int refresh_rate_shared = -1;
+
+/*
+ * Structure for refresh rate type
+ */
+typedef struct{
+	int rate;
+	const char *name;
+} refresh_rate_t;
+
+/*
+ * vblank_handler :
+ *	User defined vblank event handler, to count vblanks.
+ *	The control reaches this function after a vblank event is registered
+ */
+static void vblank_handler(int fd, unsigned int frame,
+			unsigned int sec, unsigned int usec, void *data)
+{
+	/* The control reaches this function after a vblank event is
+	 * registered
+	 */
+	vbl_info *info = data;
+
+	if (info == NULL) {
+		igt_info("ERROR: Invalid Data passed to the vblank handler\n");
+		return;
+	}
+	info->vbl_count++;
+}
+
+
+/*
+ * read_drrs_status :
+ *	Func to read the DRRS status from debugfs
+ */
+static bool read_drrs_status(data_t *data, char *str)
+{
+	FILE *status;
+	int cnt;
+
+	status = igt_debugfs_fopen("i915_drrs_status", "r");
+	igt_assert(status);
+
+	cnt = fread(str, DRRS_STATUS_BYTES_CNT - 1, 1, status);
+	if (!cnt) {
+		if (!feof(status)) {
+			igt_info("ERROR: %d at fread\n", ferror(status));
+			return false;
+		}
+		clearerr(status);
+	}
+	fclose(status);
+	return true;
+}
+
+/*
+ * is_drrs_enabled :
+ *	Func to check for DRRS support
+ */
+static bool is_drrs_enabled(data_t *data)
+{
+	char str[DRRS_STATUS_BYTES_CNT] = {};
+
+	if (!read_drrs_status(data, str)) {
+		igt_info("ERROR: Debugfs read FAILED\n");
+		return false;
+	}
+	return strstr(str, "DRRS Supported: Yes") != NULL;
+}
+
+/*
+ * prepare_crtc :
+ *	Func to prepare crtc for the given display
+ */
+static bool prepare_crtc(data_t *data)
+{
+	if (data == NULL)
+		return false;
+	igt_display_t *display = &data->display;
+	igt_output_t *output = data->output;
+
+	/* select the pipe we want to use */
+	igt_output_set_pipe(output, data->pipe);
+	igt_display_commit(display);
+
+	if (!output->valid) {
+		igt_output_set_pipe(output, PIPE_ANY);
+		igt_display_commit(display);
+		return false;
+	}
+
+	return true;
+}
+
+/*
+ * calculate_refresh_rate :
+ *	Func to calculate the refresh rate by counting
+ *	vblanks in 1 sec. It returns the calculated refresh rate on success.
+ *	Returns -1 in case of error.
+ */
+int calculate_refresh_rate(data_t *data)
+{
+	drmEventContext evctx;
+	drmVBlank vbl;
+	vbl_info handler_info;
+	struct timeval start_time, curr_time;
+	double time_elapsed;
+	int refresh_rate = 0, vbl_count = 0;
+	/*set up event context for handling vblank events*/
+	memset(&evctx, 0, sizeof(evctx));
+	evctx.version = DRM_EVENT_CONTEXT_VERSION;
+	evctx.vblank_handler = vblank_handler;
+	evctx.page_flip_handler = NULL;
+
+	/*initialize the vbl count to zero and set the starting time*/
+	handler_info.vbl_count = 0;
+	gettimeofday(&start_time, NULL);
+	handler_info.start = start_time;
+	curr_time = start_time;
+
+	time_elapsed = 0.0;
+	/* To count vblanks in 1 sec. Loop till
+	 * (current-time - starting time) <= 1.0 sec
+	 */
+	while (time_elapsed <= 1.0) {
+		int ret;
+
+		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
+		vbl.request.sequence = 1;
+		vbl.request.signal = (unsigned long)&handler_info;
+		ret = drmWaitVBlank(data->drm_fd, &vbl);
+		if (ret != 0) {
+			igt_info("ERROR : drmWaitVBlank FAILED :%d\n", ret);
+			return -1;
+		}
+		/*call drmHandleEvent() to handle vblank event */
+		ret = drmHandleEvent(data->drm_fd, &evctx);
+		if (ret != 0) {
+			igt_info("ERROR : drmHandleEvent FAILED %d\n", ret);
+			return -1;
+		}
+
+		gettimeofday(&curr_time, NULL);
+		time_elapsed = curr_time.tv_sec + curr_time.tv_usec * 1e-6 -
+			(start_time.tv_sec + start_time.tv_usec * 1e-6);
+	}
+	if (!time_elapsed) {
+		igt_info("ERROR: Incorrect measurement of vblank duration\n");
+		return -1;
+	}
+	vbl_count = handler_info.vbl_count;
+	/* calculate the refresh rate; rr=vblank_count/time_taken */
+	refresh_rate = vbl_count / time_elapsed;
+	return refresh_rate;
+}
+
+/*
+ * worker_thread_func :
+ *	Func which is run by a worker thread to calculate the refresh_rate,
+ *	when signalled by the master thread.
+ */
+void *worker_thread_func(void *ptr)
+{
+	data_t *data = ptr;
+
+	while (1) {
+		int refresh_rate = 0;
+		/*wait for signal from master*/
+		pthread_mutex_lock(&rr_mutex);
+		while (!rr_calc_requested)
+			pthread_cond_wait(&cv_rr_calc_requested, &rr_mutex);
+
+		/* checkpoint for thread termination */
+		if (thread_flag == RESET) {
+			pthread_mutex_unlock(&rr_mutex);
+			pthread_exit(NULL);
+		}
+		rr_calc_requested = RESET;
+		pthread_mutex_unlock(&rr_mutex);
+		/*calculate refresh_rate*/
+		refresh_rate = calculate_refresh_rate(data);
+
+		/* signal the master */
+		pthread_mutex_lock(&rr_mutex);
+		refresh_rate_shared = refresh_rate;
+		rr_calc_completed = SET;
+		pthread_mutex_unlock(&rr_mutex);
+		pthread_cond_signal(&cv_rr_calc_completed);
+	}
+	return NULL;
+}
+
+/*
+ * check_refresh_rate :
+ *	Func to check the refresh rate calculated, against the expected refresh
+ *	rate. It signals the worker thread to calculate the refresh-
+ *	rate, and checks if the correct refresh rate is switched.
+ *	Returns 1 in case of pass , 0 in case of failure, and -1 for error.
+ */
+
+static int check_refresh_rate(data_t *data, refresh_rate_t *expected_rr)
+{
+/*
+ * The calculated refresh-rate is an approximation to estimated refresh-rate.
+ * The tolerance threshold for the difference is set to 2. Difference more than
+ * 2 is considered as failure for the check_refresh_rate.
+ */
+#define TOLERANCE_THRESHOLD		2
+	int ret, refresh_rate = -1;
+	char drrs_status[DRRS_STATUS_BYTES_CNT] = {};
+
+	/* Signal worker thread to calculate refresh rate */
+	pthread_mutex_lock(&rr_mutex);
+	rr_calc_requested = SET;
+	pthread_mutex_unlock(&rr_mutex);
+	pthread_cond_signal(&cv_rr_calc_requested);
+
+	/* Get the DRRS Status from the debugfs entry */
+	if (!read_drrs_status(data, drrs_status)) {
+		igt_info("ERROR : DRRS debugfs read FAILED\n");
+		return -1;
+	}
+	/* Wait for the refresh rate calculator thread */
+	pthread_mutex_lock(&rr_mutex);
+	while (!rr_calc_completed)
+		pthread_cond_wait(&cv_rr_calc_completed, &rr_mutex);
+	rr_calc_completed = RESET;
+	refresh_rate = refresh_rate_shared;
+	pthread_mutex_unlock(&rr_mutex);
+	if (refresh_rate < 0) {
+		igt_info("ERROR : Refresh rate calculation FAILED\n");
+		return -1;
+	}
+	igt_info("INFO : Actual Refresh_rate based on Vblank Counts/sec = %d\n"
+							, refresh_rate);
+
+	/* Compare with Expected Refresh Rate*/
+	igt_info("INFO: Expected Refresh-Rate %d\n", expected_rr->rate);
+	if (strstr(drrs_status, expected_rr->name) != NULL
+	&& abs(refresh_rate - expected_rr->rate) <= TOLERANCE_THRESHOLD) {
+		igt_info("INFO: Expected %s : PASSED\n", expected_rr->name);
+		return 1;
+	}
+
+	igt_info("INFO : Expected %s : FAILED\n", expected_rr->name);
+	return 0;
+}
+
+/*
+ * clean_up :
+ *	Func to stop the worker thread and destroy thread syncronization
+ *	structures.
+ */
+void clean_up(pthread_t rr_thread)
+{
+	/* Stop the worker thread */
+	pthread_mutex_lock(&rr_mutex);
+	thread_flag = RESET;
+	rr_calc_requested = SET;
+	pthread_mutex_unlock(&rr_mutex);
+	pthread_cond_signal(&cv_rr_calc_requested);
+	pthread_join(rr_thread, NULL);
+
+	/* destroy the condition variables and mutex */
+	pthread_cond_destroy(&cv_rr_calc_requested);
+	pthread_cond_destroy(&cv_rr_calc_completed);
+	pthread_mutex_destroy(&rr_mutex);
+}
+
+/*
+ * execute_test:
+ *	This function
+ *	1. Initializes the thread sync structures.
+ *	2. Creates a worker thread which can be signaled to estimate
+ *	the refresh rates based on counting vblanks per sec.
+ *	3. Creates two different Framebuffer fb0, fb1
+ *	and apply them in the interval of 1.2Sec. So between FB change
+ *	DRRS should toggle from DRRS_HIGH_RR -> DRRS_LOW_RR.
+ *	And on next FB change DRRS_LOW_RR -> DRRS_HIGH_RR.
+ *
+ *	These expected transistions are verified using the kernel debugfs
+ *	and the refresh-rate, calculated by counting vblanks per sec by the
+ *	worker thread.
+ */
+static bool execute_test(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output = data->output;
+	drmModeModeInfo *mode, *supported_modes;
+	bool test_failed = false;
+	int ret, i, count_modes;
+	refresh_rate_t high_rr, low_rr;
+	pthread_t rr_thread;
+
+	/*initialize the condition variables and  mutex*/
+	pthread_cond_init(&cv_rr_calc_requested, NULL);
+	pthread_cond_init(&cv_rr_calc_completed, NULL);
+	pthread_mutex_init(&rr_mutex, NULL);
+	thread_flag = SET;
+	rr_calc_requested = RESET;
+	rr_calc_completed = RESET;
+	/* Create a thread for calculating the refresh rate */
+	ret = pthread_create(&rr_thread, NULL, worker_thread_func,
+							(void *) data);
+	if (ret == -1) {
+		igt_info("ERROR : Refresh rate thread creation FAILED\n");
+		clean_up(rr_thread);
+		return false;
+	}
+	/*close the write end of the pipe as it will be used for reading only*/
+
+	high_rr.name = "DRRS_HIGH_RR";
+	high_rr.rate = 0;
+	low_rr.name = "DRRS_LOW_RR";
+	low_rr.rate = 0;
+	/* get the max and min supported refresh rates for the display */
+	supported_modes = output->config.connector->modes;
+	count_modes = output->config.connector->count_modes;
+	/* minimum 2 modes are required for DRRS */
+	if (count_modes < 2) {
+		igt_info("ERROR: Minimum 2 modes required for DRRS\n");
+		return false;
+	}
+	for (i = 0; i < count_modes; i++) {
+		int rr = supported_modes[i].vrefresh;
+
+		if (i == 0) {
+			high_rr.rate = low_rr.rate = rr;
+			continue;
+		}
+		if (high_rr.rate < rr)
+			high_rr.rate = rr;
+		if (low_rr.rate > rr)
+			low_rr.rate = rr;
+	}
+	data->primary = igt_output_get_plane(data->output, IGT_PLANE_PRIMARY);
+	mode = igt_output_get_mode(data->output);
+	data->fb_id[0] = igt_create_color_fb(data->drm_fd, mode->hdisplay,
+			mode->vdisplay,
+			DRM_FORMAT_XRGB8888,
+			LOCAL_DRM_FORMAT_MOD_NONE,
+			0.0, 100.1, 0.0, &data->fb[0]);
+	igt_assert(data->fb_id[0]);
+	data->fb_id[1] = igt_create_color_fb(data->drm_fd, mode->hdisplay,
+			mode->vdisplay,
+			DRM_FORMAT_XRGB8888,
+			LOCAL_DRM_FORMAT_MOD_NONE,
+			100.1, 0.0, 0.0,
+			&data->fb[1]);
+	igt_assert(data->fb_id[1]);
+
+	data->handle[0] = data->fb[0].gem_handle;
+	data->handle[1] = data->fb[1].gem_handle;
+
+	/* scanout = fb[1] */
+	igt_plane_set_fb(data->primary, &data->fb[1]);
+	igt_display_commit(display);
+
+	if (!is_drrs_enabled(data)) {
+		igt_info("INFO : DRRS not enabled\n");
+		igt_plane_set_fb(data->primary, NULL);
+		igt_output_set_pipe(output, PIPE_ANY);
+		igt_display_commit(display);
+
+		igt_remove_fb(data->drm_fd, &data->fb[0]);
+		igt_remove_fb(data->drm_fd, &data->fb[1]);
+		clean_up(rr_thread);
+		return false;
+	}
+
+	/* expecting High RR */
+	ret =  check_refresh_rate(data, &high_rr);
+	if (ret != 1)
+		test_failed = true;
+	/* 1.2Sec to Enter the Idleness and check for Low Refresh Rate */
+	usleep(1200000);
+	ret =  check_refresh_rate(data, &low_rr);
+	if (ret != 1)
+		test_failed = true;
+
+	/* scanout = fb[0] and check for High Refresh Rate*/
+	igt_plane_set_fb(data->primary, &data->fb[0]);
+	igt_display_commit(display);
+	ret =  check_refresh_rate(data, &high_rr);
+	if (ret != 1)
+		test_failed = true;
+
+	/* 1.2Sec to Enter the Idleness and check for Low Refresh Rate */
+	usleep(1200000);
+	ret =  check_refresh_rate(data, &low_rr);
+	if (ret != 1)
+		test_failed = true;
+
+	/* scanout = fb[1] and check for High Refresh Rate*/
+	igt_plane_set_fb(data->primary, &data->fb[1]);
+	igt_display_commit(display);
+	ret =  check_refresh_rate(data, &high_rr);
+	if (ret != 1)
+		test_failed = true;
+
+	/* 1.2Sec to Enter the Idleness */
+	usleep(1200000);
+	ret =  check_refresh_rate(data, &low_rr);
+	if (ret != 1)
+		test_failed = true;
+
+	igt_assert(!test_failed);
+	clean_up(rr_thread);
+	return true;
+}
+
+/*
+ * finish_crtc :
+ *	Func to free the framebuffers after the test completion.
+ */
+static void finish_crtc(data_t *data)
+{
+	igt_plane_set_fb(data->primary, NULL);
+	igt_output_set_pipe(data->output, PIPE_ANY);
+	igt_display_commit(&data->display);
+
+	igt_remove_fb(data->drm_fd, &data->fb[0]);
+	igt_remove_fb(data->drm_fd, &data->fb[1]);
+}
+
+/*
+ * reset_display :
+ *	Func to reset the display structures after the test completion.
+ */
+static void reset_display(data_t *data)
+{
+	igt_display_t *display = &data->display;
+
+	for_each_connected_output(display, data->output) {
+		if (data->output->valid) {
+			data->primary =  igt_output_get_plane(data->output,
+							IGT_PLANE_PRIMARY);
+			igt_plane_set_fb(data->primary, NULL);
+		}
+		igt_output_set_pipe(data->output, PIPE_ANY);
+	}
+}
+/*
+ * run_test :
+ *	Func to run the test for the eDP display.
+ */
+static void run_test(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	int valid_tests = 0;
+
+	reset_display(data);
+	/* Check for eDP Connector on PIPE A ,as eDP goes normally on PIPE_A */
+	for_each_connected_output(display, data->output) {
+		drmModeConnectorPtr c = data->output->config.connector;
+
+		if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
+			c->connection != DRM_MODE_CONNECTED)
+			continue;
+			data->pipe = PIPE_A;
+			prepare_crtc(data);
+			igt_info("INFO : Beginning %s on pipe %s,connector %s\n",
+						igt_subtest_name(),
+						kmstest_pipe_name(data->pipe),
+						igt_output_name(data->output));
+
+			if (!execute_test(data)) {
+				igt_info("INFO :%s on pipe %s, connector %s:SKIPPED\n",
+						igt_subtest_name(),
+						kmstest_pipe_name(data->pipe),
+						igt_output_name(data->output));
+				continue;
+			}
+
+			valid_tests++;
+
+			igt_info("INFO :%s on pipe %s, connector %s: PASSED\n",
+					igt_subtest_name(),
+					kmstest_pipe_name(data->pipe),
+					igt_output_name(data->output));
+
+			finish_crtc(data);
+	}
+}
+
+igt_main
+{
+	data_t data = {};
+
+	igt_skip_on_simulation();
+
+	igt_fixture {
+		char buf[64];
+		int cnt;
+		FILE *status;
+
+		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+		if (data.drm_fd == -1) {
+			igt_info("ERROR : invalid fd\n");
+			return;
+		}
+
+		data.devid = intel_get_drm_devid(data.drm_fd);
+		if (data.devid < 0) {
+			igt_info("ERROR : invalid dev id\n");
+			return;
+		}
+
+		status = igt_debugfs_fopen("i915_drrs_status", "r");
+		if (status == NULL) {
+			igt_info("ERROR : debugfs open FAILED\n");
+			return;
+		}
+		igt_require_f(status, "No i915_drrs_status found\n");
+		cnt = fread(buf, sizeof(buf), 1, status);
+		if (!cnt) {
+			igt_info("ERROR : fread FAILED\n");
+			return;
+		}
+		fclose(status);
+		buf[sizeof(buf) - 1] = '\0';
+		igt_require_f(!strstr(buf, "disabled"),
+				"DRRS not supported:check VBT/panel caps\n");
+
+		/* Check if the DRRS is supported.
+		 * If yes call the Idleness DRRS test
+		 */
+
+		igt_display_init(&data.display, data.drm_fd);
+	}
+	igt_subtest_f("Idleness_DRRS")
+		run_test(&data);
+
+	igt_fixture {
+		igt_display_fini(&data.display);
+	}
+}
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC] Idleness DRRS test
  2016-08-31  8:16 [RFC] Idleness DRRS test Nautiyal Ankit
@ 2016-09-07 14:53 ` Marius Vlad
  2016-09-23 12:07   ` Nautiyal, Ankit K
  0 siblings, 1 reply; 12+ messages in thread
From: Marius Vlad @ 2016-09-07 14:53 UTC (permalink / raw)
  To: Nautiyal Ankit; +Cc: daniel.vetter, intel-gfx, rodrigo.vivi, paulo.r.zanoni


[-- Attachment #1.1: Type: text/plain, Size: 22698 bytes --]

On Wed, Aug 31, 2016 at 01:46:03PM +0530, Nautiyal Ankit wrote:
> From: aknautiy <ankit.k.nautiyal@intel.com>
> 
> Idleness DRRS:
> 	By default the DRRS state will be at DRRS_HIGH_RR. When a Display
> 	content is Idle for more than 1Sec Idleness will be declared and
> 	DRRS_LOW_RR will be invoked, changing the refresh rate to the
> 	lower most refresh rate supported by the panel. As soon as there
> 	is a display content change there will be a DRRS state transition
> 	as DRRS_LOW_RR--> DRRS_HIGH_RR, changing the refresh rate to the
> 	highest refresh rate supported by the panel.
> 
> To test this, Idleness DRRS IGT will probe the DRRS state at below
> instances and compare with the expected state.
> 
> 	Instance					Expected State
> 1. Immediately after rendering the still image		DRRS_HIGH_RR
> 2. After a delay of 1.2Sec				DRRS_LOW_RR
> 3. After changing the frame buffer			DRRS_HIGH_RR
> 4. After a delay of 1.2Sec				DRRS_LOW_RR
> 5. After changing the frame buffer			DRRS_HIGH_RR
> 6. After a delay of 1.2Sec				DRRS_LOW_RR
> 
> The test checks the driver DRRS state from the debugfs entry. To check the
> actual refresh-rate, a separate thread counts the number of vblanks
> received per sec. The refresh-rate calculated is checked against the
> expected refresh-rate with a tolerance value of 2.
> 
> This patch is a continuation of the earlier work
> https://patchwork.freedesktop.org/patch/45472/ towards igt for idleness
> 
> DRRS. The code is tested on Broxton BXT_T platform.
> 
> Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com>
> ---
>  tests/Makefile.sources |   1 +
>  tests/kms_drrs.c       | 614 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 615 insertions(+)
>  create mode 100644 tests/kms_drrs.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index a837977..5f31521 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -91,6 +91,7 @@ TESTS_progs_M = \
>  	kms_cursor_crc \
>  	kms_cursor_legacy \
>  	kms_draw_crc \
> +	kms_drrs \
>  	kms_fbc_crc \
>  	kms_fbcon_fbt \
>  	kms_flip \
> diff --git a/tests/kms_drrs.c b/tests/kms_drrs.c
> new file mode 100644
> index 0000000..fbe78c5
> --- /dev/null
> +++ b/tests/kms_drrs.c
> @@ -0,0 +1,614 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "drmtest.h"
> +#include "igt_debugfs.h"
> +#include "igt_kms.h"
> +#include "intel_chipset.h"
> +#include "intel_batchbuffer.h"
> +#include "ioctl_wrappers.h"
> +#include <time.h>
> +#include <pthread.h>
> +#include <stdlib.h>
> +#include <sys/time.h>
> +IGT_TEST_DESCRIPTION(
> +"Performs write operations and then waits for DRRS to invoke the"
> +"Low Refresh Rate and then disturbs the contents of the screen once"
> +"again hence DRRS revert back to High Refresh Rate(Default).");
> +
> +#define DRRS_STATUS_BYTES_CNT	1000
> +#define SET	1
> +#define RESET	0
> +
> +/*
> + * Structure to store data to create 2 framebuffers, fb[0] and fb[1] on a given
> + * display. To disturb the content of the screen, we replace fb[0] by fb[1] and
> + * vice versa.
> + */
> +typedef struct {
> +	int drm_fd;
> +	uint32_t devid;
> +	uint32_t handle[2];
> +	igt_display_t display;
> +	igt_output_t *output;
> +	enum pipe pipe;
> +	igt_plane_t *primary;
> +	struct igt_fb fb[2];
> +	uint32_t fb_id[2];
> +} data_t;
> +
> +/*
> + * Structure to count vblank and note the starting time of the counter
> + */
> +typedef struct {
> +	unsigned int vbl_count;
> +	struct timeval start;
> +} vbl_info;
> +
> +/*
> + * Condition variables,mutex, and shared variables for thread synchronization
> + */
> +pthread_mutex_t rr_mutex;
> +pthread_cond_t cv_rr_calc_requested;
> +pthread_cond_t cv_rr_calc_completed;
> +int rr_calc_requested = RESET;
> +int rr_calc_completed = RESET;
> +int thread_flag = SET;
> +int refresh_rate_shared = -1;
> +
> +/*
> + * Structure for refresh rate type
> + */
> +typedef struct{
> +	int rate;
> +	const char *name;
> +} refresh_rate_t;
> +
> +/*
> + * vblank_handler :
> + *	User defined vblank event handler, to count vblanks.
> + *	The control reaches this function after a vblank event is registered
> + */
> +static void vblank_handler(int fd, unsigned int frame,
> +			unsigned int sec, unsigned int usec, void *data)
> +{
> +	/* The control reaches this function after a vblank event is
> +	 * registered
> +	 */
> +	vbl_info *info = data;
Doesn't this yell a warning?
> +
> +	if (info == NULL) {
> +		igt_info("ERROR: Invalid Data passed to the vblank handler\n");
> +		return;
> +	}
> +	info->vbl_count++;
> +}
> +
> +
> +/*
> + * read_drrs_status :
> + *	Func to read the DRRS status from debugfs
> + */
> +static bool read_drrs_status(data_t *data, char *str)
> +{
> +	FILE *status;
> +	int cnt;
> +
> +	status = igt_debugfs_fopen("i915_drrs_status", "r");
> +	igt_assert(status);
> +
> +	cnt = fread(str, DRRS_STATUS_BYTES_CNT - 1, 1, status);
> +	if (!cnt) {
> +		if (!feof(status)) {
> +			igt_info("ERROR: %d at fread\n", ferror(status));
> +			return false;
> +		}
> +		clearerr(status);
> +	}
> +	fclose(status);
> +	return true;
> +}
Can't find where data is used here.
> +
> +/*
> + * is_drrs_enabled :
> + *	Func to check for DRRS support
> + */
> +static bool is_drrs_enabled(data_t *data)
> +{
> +	char str[DRRS_STATUS_BYTES_CNT] = {};
> +
> +	if (!read_drrs_status(data, str)) {
> +		igt_info("ERROR: Debugfs read FAILED\n");
> +		return false;
> +	}
> +	return strstr(str, "DRRS Supported: Yes") != NULL;
No need for data here.
> +}
> +
> +/*
> + * prepare_crtc :
> + *	Func to prepare crtc for the given display
> + */
> +static bool prepare_crtc(data_t *data)
> +{
> +	if (data == NULL)
> +		return false;
> +	igt_display_t *display = &data->display;
> +	igt_output_t *output = data->output;
> +
> +	/* select the pipe we want to use */
> +	igt_output_set_pipe(output, data->pipe);
> +	igt_display_commit(display);
> +
> +	if (!output->valid) {
> +		igt_output_set_pipe(output, PIPE_ANY);
> +		igt_display_commit(display);
> +		return false;
> +	}
> +
> +	return true;
> +}
The return value isn't used anywhere.
> +
> +/*
> + * calculate_refresh_rate :
> + *	Func to calculate the refresh rate by counting
> + *	vblanks in 1 sec. It returns the calculated refresh rate on success.
> + *	Returns -1 in case of error.
> + */
> +int calculate_refresh_rate(data_t *data)
> +{
> +	drmEventContext evctx;
> +	drmVBlank vbl;
> +	vbl_info handler_info;
> +	struct timeval start_time, curr_time;
> +	double time_elapsed;
> +	int refresh_rate = 0, vbl_count = 0;
> +	/*set up event context for handling vblank events*/
> +	memset(&evctx, 0, sizeof(evctx));
> +	evctx.version = DRM_EVENT_CONTEXT_VERSION;
> +	evctx.vblank_handler = vblank_handler;
> +	evctx.page_flip_handler = NULL;
> +
> +	/*initialize the vbl count to zero and set the starting time*/
> +	handler_info.vbl_count = 0;
> +	gettimeofday(&start_time, NULL);
> +	handler_info.start = start_time;
> +	curr_time = start_time;
> +
> +	time_elapsed = 0.0;
> +	/* To count vblanks in 1 sec. Loop till
> +	 * (current-time - starting time) <= 1.0 sec
> +	 */
> +	while (time_elapsed <= 1.0) {
> +		int ret;
> +
> +		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
> +		vbl.request.sequence = 1;
> +		vbl.request.signal = (unsigned long)&handler_info;
> +		ret = drmWaitVBlank(data->drm_fd, &vbl);
> +		if (ret != 0) {
> +			igt_info("ERROR : drmWaitVBlank FAILED :%d\n", ret);
> +			return -1;
> +		}
> +		/*call drmHandleEvent() to handle vblank event */
> +		ret = drmHandleEvent(data->drm_fd, &evctx);
> +		if (ret != 0) {
> +			igt_info("ERROR : drmHandleEvent FAILED %d\n", ret);
> +			return -1;
> +		}
> +
> +		gettimeofday(&curr_time, NULL);
> +		time_elapsed = curr_time.tv_sec + curr_time.tv_usec * 1e-6 -
> +			(start_time.tv_sec + start_time.tv_usec * 1e-6);
> +	}
> +	if (!time_elapsed) {
> +		igt_info("ERROR: Incorrect measurement of vblank duration\n");
> +		return -1;
> +	}
> +	vbl_count = handler_info.vbl_count;
> +	/* calculate the refresh rate; rr=vblank_count/time_taken */
> +	refresh_rate = vbl_count / time_elapsed;
> +	return refresh_rate;
> +}
> +
> +/*
> + * worker_thread_func :
> + *	Func which is run by a worker thread to calculate the refresh_rate,
> + *	when signalled by the master thread.
> + */
> +void *worker_thread_func(void *ptr)
> +{
> +	data_t *data = ptr;
Isn't a typecast needed here?
> +
> +	while (1) {
> +		int refresh_rate = 0;
> +		/*wait for signal from master*/
> +		pthread_mutex_lock(&rr_mutex);
> +		while (!rr_calc_requested)
> +			pthread_cond_wait(&cv_rr_calc_requested, &rr_mutex);
> +
> +		/* checkpoint for thread termination */
> +		if (thread_flag == RESET) {
> +			pthread_mutex_unlock(&rr_mutex);
> +			pthread_exit(NULL);
> +		}
> +		rr_calc_requested = RESET;
> +		pthread_mutex_unlock(&rr_mutex);
> +		/*calculate refresh_rate*/
> +		refresh_rate = calculate_refresh_rate(data);
> +
> +		/* signal the master */
> +		pthread_mutex_lock(&rr_mutex);
> +		refresh_rate_shared = refresh_rate;
> +		rr_calc_completed = SET;
> +		pthread_mutex_unlock(&rr_mutex);
> +		pthread_cond_signal(&cv_rr_calc_completed);
> +	}
> +	return NULL;
> +}
> +
> +/*
> + * check_refresh_rate :
> + *	Func to check the refresh rate calculated, against the expected refresh
> + *	rate. It signals the worker thread to calculate the refresh-
> + *	rate, and checks if the correct refresh rate is switched.
> + *	Returns 1 in case of pass , 0 in case of failure, and -1 for error.
> + */
> +
> +static int check_refresh_rate(data_t *data, refresh_rate_t *expected_rr)
> +{
> +/*
> + * The calculated refresh-rate is an approximation to estimated refresh-rate.
> + * The tolerance threshold for the difference is set to 2. Difference more than
> + * 2 is considered as failure for the check_refresh_rate.
> + */
> +#define TOLERANCE_THRESHOLD		2
> +	int ret, refresh_rate = -1;
> +	char drrs_status[DRRS_STATUS_BYTES_CNT] = {};
> +
> +	/* Signal worker thread to calculate refresh rate */
> +	pthread_mutex_lock(&rr_mutex);
> +	rr_calc_requested = SET;
> +	pthread_mutex_unlock(&rr_mutex);
> +	pthread_cond_signal(&cv_rr_calc_requested);
> +
> +	/* Get the DRRS Status from the debugfs entry */
> +	if (!read_drrs_status(data, drrs_status)) {
> +		igt_info("ERROR : DRRS debugfs read FAILED\n");
> +		return -1;
> +	}
> +	/* Wait for the refresh rate calculator thread */
> +	pthread_mutex_lock(&rr_mutex);
> +	while (!rr_calc_completed)
> +		pthread_cond_wait(&cv_rr_calc_completed, &rr_mutex);
> +	rr_calc_completed = RESET;
> +	refresh_rate = refresh_rate_shared;
> +	pthread_mutex_unlock(&rr_mutex);
> +	if (refresh_rate < 0) {
> +		igt_info("ERROR : Refresh rate calculation FAILED\n");
> +		return -1;
> +	}
> +	igt_info("INFO : Actual Refresh_rate based on Vblank Counts/sec = %d\n"
> +							, refresh_rate);
> +
> +	/* Compare with Expected Refresh Rate*/
> +	igt_info("INFO: Expected Refresh-Rate %d\n", expected_rr->rate);
> +	if (strstr(drrs_status, expected_rr->name) != NULL
> +	&& abs(refresh_rate - expected_rr->rate) <= TOLERANCE_THRESHOLD) {
> +		igt_info("INFO: Expected %s : PASSED\n", expected_rr->name);
> +		return 1;
> +	}
> +
> +	igt_info("INFO : Expected %s : FAILED\n", expected_rr->name);
> +	return 0;
> +}
> +
> +/*
> + * clean_up :
> + *	Func to stop the worker thread and destroy thread syncronization
> + *	structures.
> + */
> +void clean_up(pthread_t rr_thread)
> +{
> +	/* Stop the worker thread */
> +	pthread_mutex_lock(&rr_mutex);
> +	thread_flag = RESET;
> +	rr_calc_requested = SET;
> +	pthread_mutex_unlock(&rr_mutex);
> +	pthread_cond_signal(&cv_rr_calc_requested);
> +	pthread_join(rr_thread, NULL);
> +
> +	/* destroy the condition variables and mutex */
> +	pthread_cond_destroy(&cv_rr_calc_requested);
> +	pthread_cond_destroy(&cv_rr_calc_completed);
> +	pthread_mutex_destroy(&rr_mutex);
> +}
> +
> +/*
> + * execute_test:
> + *	This function
> + *	1. Initializes the thread sync structures.
> + *	2. Creates a worker thread which can be signaled to estimate
> + *	the refresh rates based on counting vblanks per sec.
> + *	3. Creates two different Framebuffer fb0, fb1
> + *	and apply them in the interval of 1.2Sec. So between FB change
> + *	DRRS should toggle from DRRS_HIGH_RR -> DRRS_LOW_RR.
> + *	And on next FB change DRRS_LOW_RR -> DRRS_HIGH_RR.
> + *
> + *	These expected transistions are verified using the kernel debugfs
> + *	and the refresh-rate, calculated by counting vblanks per sec by the
> + *	worker thread.
> + */
> +static bool execute_test(data_t *data)
> +{
> +	igt_display_t *display = &data->display;
> +	igt_output_t *output = data->output;
> +	drmModeModeInfo *mode, *supported_modes;
> +	bool test_failed = false;
> +	int ret, i, count_modes;
> +	refresh_rate_t high_rr, low_rr;
> +	pthread_t rr_thread;
> +
> +	/*initialize the condition variables and  mutex*/
> +	pthread_cond_init(&cv_rr_calc_requested, NULL);
> +	pthread_cond_init(&cv_rr_calc_completed, NULL);
> +	pthread_mutex_init(&rr_mutex, NULL);
> +	thread_flag = SET;
> +	rr_calc_requested = RESET;
> +	rr_calc_completed = RESET;
> +	/* Create a thread for calculating the refresh rate */
> +	ret = pthread_create(&rr_thread, NULL, worker_thread_func,
> +							(void *) data);
> +	if (ret == -1) {
> +		igt_info("ERROR : Refresh rate thread creation FAILED\n");
> +		clean_up(rr_thread);
> +		return false;
> +	}
> +	/*close the write end of the pipe as it will be used for reading only*/
Where is the write-end of the pipe?
> +
> +	high_rr.name = "DRRS_HIGH_RR";
> +	high_rr.rate = 0;
> +	low_rr.name = "DRRS_LOW_RR";
> +	low_rr.rate = 0;
> +	/* get the max and min supported refresh rates for the display */
> +	supported_modes = output->config.connector->modes;
> +	count_modes = output->config.connector->count_modes;
> +	/* minimum 2 modes are required for DRRS */
> +	if (count_modes < 2) {
> +		igt_info("ERROR: Minimum 2 modes required for DRRS\n");
> +		return false;
> +	}
> +	for (i = 0; i < count_modes; i++) {
> +		int rr = supported_modes[i].vrefresh;
> +
> +		if (i == 0) {
> +			high_rr.rate = low_rr.rate = rr;
> +			continue;
> +		}
> +		if (high_rr.rate < rr)
> +			high_rr.rate = rr;
> +		if (low_rr.rate > rr)
> +			low_rr.rate = rr;
> +	}
> +	data->primary = igt_output_get_plane(data->output, IGT_PLANE_PRIMARY);
> +	mode = igt_output_get_mode(data->output);
> +	data->fb_id[0] = igt_create_color_fb(data->drm_fd, mode->hdisplay,
> +			mode->vdisplay,
> +			DRM_FORMAT_XRGB8888,
> +			LOCAL_DRM_FORMAT_MOD_NONE,
> +			0.0, 100.1, 0.0, &data->fb[0]);
> +	igt_assert(data->fb_id[0]);
> +	data->fb_id[1] = igt_create_color_fb(data->drm_fd, mode->hdisplay,
> +			mode->vdisplay,
> +			DRM_FORMAT_XRGB8888,
> +			LOCAL_DRM_FORMAT_MOD_NONE,
> +			100.1, 0.0, 0.0,
> +			&data->fb[1]);
> +	igt_assert(data->fb_id[1]);
> +
> +	data->handle[0] = data->fb[0].gem_handle;
> +	data->handle[1] = data->fb[1].gem_handle;
> +
> +	/* scanout = fb[1] */
> +	igt_plane_set_fb(data->primary, &data->fb[1]);
> +	igt_display_commit(display);
> +

I assume that the behaviour is that once you have committed your
frame-buffer, DRRS should be enabled by the kernel, hence the check here
again. You do this (albeit manually) in igt_fixture block.

> +	if (!is_drrs_enabled(data)) {
> +		igt_info("INFO : DRRS not enabled\n");
> +		igt_plane_set_fb(data->primary, NULL);
> +		igt_output_set_pipe(output, PIPE_ANY);
> +		igt_display_commit(display);
> +
> +		igt_remove_fb(data->drm_fd, &data->fb[0]);
> +		igt_remove_fb(data->drm_fd, &data->fb[1]);
> +		clean_up(rr_thread);
> +		return false;
> +	}
> +
> +	/* expecting High RR */
> +	ret =  check_refresh_rate(data, &high_rr);
> +	if (ret != 1)
> +		test_failed = true;
> +	/* 1.2Sec to Enter the Idleness and check for Low Refresh Rate */
> +	usleep(1200000);
> +	ret =  check_refresh_rate(data, &low_rr);
> +	if (ret != 1)
> +		test_failed = true;
> +
> +	/* scanout = fb[0] and check for High Refresh Rate*/
> +	igt_plane_set_fb(data->primary, &data->fb[0]);
> +	igt_display_commit(display);
> +	ret =  check_refresh_rate(data, &high_rr);
> +	if (ret != 1)
> +		test_failed = true;
> +
> +	/* 1.2Sec to Enter the Idleness and check for Low Refresh Rate */
> +	usleep(1200000);
> +	ret =  check_refresh_rate(data, &low_rr);
> +	if (ret != 1)
> +		test_failed = true;
> +
> +	/* scanout = fb[1] and check for High Refresh Rate*/
> +	igt_plane_set_fb(data->primary, &data->fb[1]);
> +	igt_display_commit(display);
> +	ret =  check_refresh_rate(data, &high_rr);
> +	if (ret != 1)
> +		test_failed = true;
> +
> +	/* 1.2Sec to Enter the Idleness */
> +	usleep(1200000);
> +	ret =  check_refresh_rate(data, &low_rr);
> +	if (ret != 1)
> +		test_failed = true;
> +
> +	igt_assert(!test_failed);
Do you really want to exit with an assert if DRRS test failed?

This function returns false when DRRS is not present and bails out
earlier if test has failed. You might want to have returned an int with
-1 for errors, 0 for test succeeded, 1 when test not succeeded (you seem
to be using this notation in one of the functions above).

> +	clean_up(rr_thread);
> +	return true;
> +}
> +
> +/*
> + * finish_crtc :
> + *	Func to free the framebuffers after the test completion.
> + */
> +static void finish_crtc(data_t *data)
> +{
> +	igt_plane_set_fb(data->primary, NULL);
> +	igt_output_set_pipe(data->output, PIPE_ANY);
> +	igt_display_commit(&data->display);
> +
> +	igt_remove_fb(data->drm_fd, &data->fb[0]);
> +	igt_remove_fb(data->drm_fd, &data->fb[1]);
> +}
> +
> +/*
> + * reset_display :
> + *	Func to reset the display structures after the test completion.
> + */
> +static void reset_display(data_t *data)
> +{
> +	igt_display_t *display = &data->display;
> +
> +	for_each_connected_output(display, data->output) {
> +		if (data->output->valid) {
> +			data->primary =  igt_output_get_plane(data->output,
> +							IGT_PLANE_PRIMARY);
> +			igt_plane_set_fb(data->primary, NULL);
> +		}
> +		igt_output_set_pipe(data->output, PIPE_ANY);
> +	}
> +}
> +/*
> + * run_test :
> + *	Func to run the test for the eDP display.
> + */
> +static void run_test(data_t *data)
> +{
> +	igt_display_t *display = &data->display;
> +	int valid_tests = 0;
> +
> +	reset_display(data);
> +	/* Check for eDP Connector on PIPE A ,as eDP goes normally on PIPE_A */
> +	for_each_connected_output(display, data->output) {
> +		drmModeConnectorPtr c = data->output->config.connector;
> +
> +		if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
> +			c->connection != DRM_MODE_CONNECTED)
> +			continue;
> +			data->pipe = PIPE_A;
> +			prepare_crtc(data);
prepare_crtc() returns a bool.
> +			igt_info("INFO : Beginning %s on pipe %s,connector %s\n",
> +						igt_subtest_name(),
> +						kmstest_pipe_name(data->pipe),
> +						igt_output_name(data->output));
> +
> +			if (!execute_test(data)) {
> +				igt_info("INFO :%s on pipe %s, connector %s:SKIPPED\n",
> +						igt_subtest_name(),
> +						kmstest_pipe_name(data->pipe),
> +						igt_output_name(data->output));
> +				continue;
> +			}
> +
> +			valid_tests++;
> +
> +			igt_info("INFO :%s on pipe %s, connector %s: PASSED\n",
> +					igt_subtest_name(),
> +					kmstest_pipe_name(data->pipe),
> +					igt_output_name(data->output));
> +
> +			finish_crtc(data);
> +	}
> +}
> +
> +igt_main
Might want to use igt_simple_main for only one test.
> +{
> +	data_t data = {};
> +
> +	igt_skip_on_simulation();
> +
> +	igt_fixture {
> +		char buf[64];
> +		int cnt;
> +		FILE *status;
> +
> +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> +		if (data.drm_fd == -1) {
> +			igt_info("ERROR : invalid fd\n");
> +			return;
> +		}
> +
> +		data.devid = intel_get_drm_devid(data.drm_fd);
> +		if (data.devid < 0) {
> +			igt_info("ERROR : invalid dev id\n");
> +			return;
> +		}
> +
> +		status = igt_debugfs_fopen("i915_drrs_status", "r");
> +		if (status == NULL) {
> +			igt_info("ERROR : debugfs open FAILED\n");
> +			return;
> +		}
> +		igt_require_f(status, "No i915_drrs_status found\n");
> +		cnt = fread(buf, sizeof(buf), 1, status);
> +		if (!cnt) {
> +			igt_info("ERROR : fread FAILED\n");
> +			return;
> +		}
> +		fclose(status);
> +		buf[sizeof(buf) - 1] = '\0';
> +		igt_require_f(!strstr(buf, "disabled"),
> +				"DRRS not supported:check VBT/panel caps\n");
You have a couple functions to test DRRS presence bou do it (again)
manually. Also you check for each connected output in you do it
(again) manually.
> +		/* Check if the DRRS is supported.
> +		 * If yes call the Idleness DRRS test
> +		 */
This comments seems to belong in a different place.
> +
> +		igt_display_init(&data.display, data.drm_fd);
> +	}
> +	igt_subtest_f("Idleness_DRRS")
> +		run_test(&data);
> +
> +	igt_fixture {
> +		igt_display_fini(&data.display);
> +	}
> +}
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] Idleness DRRS test
  2016-09-07 14:53 ` Marius Vlad
@ 2016-09-23 12:07   ` Nautiyal, Ankit K
  2016-09-23 12:40     ` [PATCH v2] " Nautiyal Ankit
  0 siblings, 1 reply; 12+ messages in thread
From: Nautiyal, Ankit K @ 2016-09-23 12:07 UTC (permalink / raw)
  To: intel-gfx, daniel.vetter, chris, rodrigo.vivi, paulo.r.zanoni


[-- Attachment #1.1: Type: text/plain, Size: 24060 bytes --]

Hi Marius,

Thanks for the comments and recommendations. I will be submitting the 
next version of the patch shortly with the changes.

PFB the actions taken/explanations inline.


On 9/7/2016 8:23 PM, Marius Vlad wrote:
> On Wed, Aug 31, 2016 at 01:46:03PM +0530, Nautiyal Ankit wrote:
>> From: aknautiy <ankit.k.nautiyal@intel.com>
>>
>> Idleness DRRS:
>> 	By default the DRRS state will be at DRRS_HIGH_RR. When a Display
>> 	content is Idle for more than 1Sec Idleness will be declared and
>> 	DRRS_LOW_RR will be invoked, changing the refresh rate to the
>> 	lower most refresh rate supported by the panel. As soon as there
>> 	is a display content change there will be a DRRS state transition
>> 	as DRRS_LOW_RR--> DRRS_HIGH_RR, changing the refresh rate to the
>> 	highest refresh rate supported by the panel.
>>
>> To test this, Idleness DRRS IGT will probe the DRRS state at below
>> instances and compare with the expected state.
>>
>> 	Instance					Expected State
>> 1. Immediately after rendering the still image		DRRS_HIGH_RR
>> 2. After a delay of 1.2Sec				DRRS_LOW_RR
>> 3. After changing the frame buffer			DRRS_HIGH_RR
>> 4. After a delay of 1.2Sec				DRRS_LOW_RR
>> 5. After changing the frame buffer			DRRS_HIGH_RR
>> 6. After a delay of 1.2Sec				DRRS_LOW_RR
>>
>> The test checks the driver DRRS state from the debugfs entry. To check the
>> actual refresh-rate, a separate thread counts the number of vblanks
>> received per sec. The refresh-rate calculated is checked against the
>> expected refresh-rate with a tolerance value of 2.
>>
>> This patch is a continuation of the earlier work
>> https://patchwork.freedesktop.org/patch/45472/ towards igt for idleness
>>
>> DRRS. The code is tested on Broxton BXT_T platform.
>>
>> Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com>
>> ---
>>   tests/Makefile.sources |   1 +
>>   tests/kms_drrs.c       | 614 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 615 insertions(+)
>>   create mode 100644 tests/kms_drrs.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index a837977..5f31521 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -91,6 +91,7 @@ TESTS_progs_M = \
>>   	kms_cursor_crc \
>>   	kms_cursor_legacy \
>>   	kms_draw_crc \
>> +	kms_drrs \
>>   	kms_fbc_crc \
>>   	kms_fbcon_fbt \
>>   	kms_flip \
>> diff --git a/tests/kms_drrs.c b/tests/kms_drrs.c
>> new file mode 100644
>> index 0000000..fbe78c5
>> --- /dev/null
>> +++ b/tests/kms_drrs.c
>> @@ -0,0 +1,614 @@
>> +/*
>> + * Copyright © 2016 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + */
>> +
>> +#include "drmtest.h"
>> +#include "igt_debugfs.h"
>> +#include "igt_kms.h"
>> +#include "intel_chipset.h"
>> +#include "intel_batchbuffer.h"
>> +#include "ioctl_wrappers.h"
>> +#include <time.h>
>> +#include <pthread.h>
>> +#include <stdlib.h>
>> +#include <sys/time.h>
>> +IGT_TEST_DESCRIPTION(
>> +"Performs write operations and then waits for DRRS to invoke the"
>> +"Low Refresh Rate and then disturbs the contents of the screen once"
>> +"again hence DRRS revert back to High Refresh Rate(Default).");
>> +
>> +#define DRRS_STATUS_BYTES_CNT	1000
>> +#define SET	1
>> +#define RESET	0
>> +
>> +/*
>> + * Structure to store data to create 2 framebuffers, fb[0] and fb[1] on a given
>> + * display. To disturb the content of the screen, we replace fb[0] by fb[1] and
>> + * vice versa.
>> + */
>> +typedef struct {
>> +	int drm_fd;
>> +	uint32_t devid;
>> +	uint32_t handle[2];
>> +	igt_display_t display;
>> +	igt_output_t *output;
>> +	enum pipe pipe;
>> +	igt_plane_t *primary;
>> +	struct igt_fb fb[2];
>> +	uint32_t fb_id[2];
>> +} data_t;
>> +
>> +/*
>> + * Structure to count vblank and note the starting time of the counter
>> + */
>> +typedef struct {
>> +	unsigned int vbl_count;
>> +	struct timeval start;
>> +} vbl_info;
>> +
>> +/*
>> + * Condition variables,mutex, and shared variables for thread synchronization
>> + */
>> +pthread_mutex_t rr_mutex;
>> +pthread_cond_t cv_rr_calc_requested;
>> +pthread_cond_t cv_rr_calc_completed;
>> +int rr_calc_requested = RESET;
>> +int rr_calc_completed = RESET;
>> +int thread_flag = SET;
>> +int refresh_rate_shared = -1;
>> +
>> +/*
>> + * Structure for refresh rate type
>> + */
>> +typedef struct{
>> +	int rate;
>> +	const char *name;
>> +} refresh_rate_t;
>> +
>> +/*
>> + * vblank_handler :
>> + *	User defined vblank event handler, to count vblanks.
>> + *	The control reaches this function after a vblank event is registered
>> + */
>> +static void vblank_handler(int fd, unsigned int frame,
>> +			unsigned int sec, unsigned int usec, void *data)
>> +{
>> +	/* The control reaches this function after a vblank event is
>> +	 * registered
>> +	 */
>> +	vbl_info *info = data;
> Doesn't this yell a warning?

Ankit- The type conversion is taken care of in the next patchset.  

>> +
>> +	if (info == NULL) {
>> +		igt_info("ERROR: Invalid Data passed to the vblank handler\n");
>> +		return;
>> +	}
>> +	info->vbl_count++;
>> +}
>> +
>> +
>> +/*
>> + * read_drrs_status :
>> + *	Func to read the DRRS status from debugfs
>> + */
>> +static bool read_drrs_status(data_t *data, char *str)
>> +{
>> +	FILE *status;
>> +	int cnt;
>> +
>> +	status = igt_debugfs_fopen("i915_drrs_status", "r");
>> +	igt_assert(status);
>> +
>> +	cnt = fread(str, DRRS_STATUS_BYTES_CNT - 1, 1, status);
>> +	if (!cnt) {
>> +		if (!feof(status)) {
>> +			igt_info("ERROR: %d at fread\n", ferror(status));
>> +			return false;
>> +		}
>> +		clearerr(status);
>> +	}
>> +	fclose(status);
>> +	return true;
>> +}
> Can't find where data is used here.

Ankit – Removed the unnecessary parameter :data_t from the function.

>> +
>> +/*
>> + * is_drrs_enabled :
>> + *	Func to check for DRRS support
>> + */
>> +static bool is_drrs_enabled(data_t *data)
>> +{
>> +	char str[DRRS_STATUS_BYTES_CNT] = {};
>> +
>> +	if (!read_drrs_status(data, str)) {
>> +		igt_info("ERROR: Debugfs read FAILED\n");
>> +		return false;
>> +	}
>> +	return strstr(str, "DRRS Supported: Yes") != NULL;
> No need for data here.

Ankit – Removed the unnecessary parameter :data_t from the function.

>> +}
>> +
>> +/*
>> + * prepare_crtc :
>> + *	Func to prepare crtc for the given display
>> + */
>> +static bool prepare_crtc(data_t *data)
>> +{
>> +	if (data == NULL)
>> +		return false;
>> +	igt_display_t *display = &data->display;
>> +	igt_output_t *output = data->output;
>> +
>> +	/* select the pipe we want to use */
>> +	igt_output_set_pipe(output, data->pipe);
>> +	igt_display_commit(display);
>> +
>> +	if (!output->valid) {
>> +		igt_output_set_pipe(output, PIPE_ANY);
>> +		igt_display_commit(display);
>> +		return false;
>> +	}
>> +
>> +	return true;
>> +}
> The return value isn't used anywhere.

Ankit- the return value is checked for successful prepare_crtc()

>> +
>> +/*
>> + * calculate_refresh_rate :
>> + *	Func to calculate the refresh rate by counting
>> + *	vblanks in 1 sec. It returns the calculated refresh rate on success.
>> + *	Returns -1 in case of error.
>> + */
>> +int calculate_refresh_rate(data_t *data)
>> +{
>> +	drmEventContext evctx;
>> +	drmVBlank vbl;
>> +	vbl_info handler_info;
>> +	struct timeval start_time, curr_time;
>> +	double time_elapsed;
>> +	int refresh_rate = 0, vbl_count = 0;
>> +	/*set up event context for handling vblank events*/
>> +	memset(&evctx, 0, sizeof(evctx));
>> +	evctx.version = DRM_EVENT_CONTEXT_VERSION;
>> +	evctx.vblank_handler = vblank_handler;
>> +	evctx.page_flip_handler = NULL;
>> +
>> +	/*initialize the vbl count to zero and set the starting time*/
>> +	handler_info.vbl_count = 0;
>> +	gettimeofday(&start_time, NULL);
>> +	handler_info.start = start_time;
>> +	curr_time = start_time;
>> +
>> +	time_elapsed = 0.0;
>> +	/* To count vblanks in 1 sec. Loop till
>> +	 * (current-time - starting time) <= 1.0 sec
>> +	 */
>> +	while (time_elapsed <= 1.0) {
>> +		int ret;
>> +
>> +		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
>> +		vbl.request.sequence = 1;
>> +		vbl.request.signal = (unsigned long)&handler_info;
>> +		ret = drmWaitVBlank(data->drm_fd, &vbl);
>> +		if (ret != 0) {
>> +			igt_info("ERROR : drmWaitVBlank FAILED :%d\n", ret);
>> +			return -1;
>> +		}
>> +		/*call drmHandleEvent() to handle vblank event */
>> +		ret = drmHandleEvent(data->drm_fd, &evctx);
>> +		if (ret != 0) {
>> +			igt_info("ERROR : drmHandleEvent FAILED %d\n", ret);
>> +			return -1;
>> +		}
>> +
>> +		gettimeofday(&curr_time, NULL);
>> +		time_elapsed = curr_time.tv_sec + curr_time.tv_usec * 1e-6 -
>> +			(start_time.tv_sec + start_time.tv_usec * 1e-6);
>> +	}
>> +	if (!time_elapsed) {
>> +		igt_info("ERROR: Incorrect measurement of vblank duration\n");
>> +		return -1;
>> +	}
>> +	vbl_count = handler_info.vbl_count;
>> +	/* calculate the refresh rate; rr=vblank_count/time_taken */
>> +	refresh_rate = vbl_count / time_elapsed;
>> +	return refresh_rate;
>> +}
>> +
>> +/*
>> + * worker_thread_func :
>> + *	Func which is run by a worker thread to calculate the refresh_rate,
>> + *	when signalled by the master thread.
>> + */
>> +void *worker_thread_func(void *ptr)
>> +{
>> +	data_t *data = ptr;
> Isn't a typecast needed here?

Ankit- Typecasted to data_t *

>> +
>> +	while (1) {
>> +		int refresh_rate = 0;
>> +		/*wait for signal from master*/
>> +		pthread_mutex_lock(&rr_mutex);
>> +		while (!rr_calc_requested)
>> +			pthread_cond_wait(&cv_rr_calc_requested, &rr_mutex);
>> +
>> +		/* checkpoint for thread termination */
>> +		if (thread_flag == RESET) {
>> +			pthread_mutex_unlock(&rr_mutex);
>> +			pthread_exit(NULL);
>> +		}
>> +		rr_calc_requested = RESET;
>> +		pthread_mutex_unlock(&rr_mutex);
>> +		/*calculate refresh_rate*/
>> +		refresh_rate = calculate_refresh_rate(data);
>> +
>> +		/* signal the master */
>> +		pthread_mutex_lock(&rr_mutex);
>> +		refresh_rate_shared = refresh_rate;
>> +		rr_calc_completed = SET;
>> +		pthread_mutex_unlock(&rr_mutex);
>> +		pthread_cond_signal(&cv_rr_calc_completed);
>> +	}
>> +	return NULL;
>> +}
>> +
>> +/*
>> + * check_refresh_rate :
>> + *	Func to check the refresh rate calculated, against the expected refresh
>> + *	rate. It signals the worker thread to calculate the refresh-
>> + *	rate, and checks if the correct refresh rate is switched.
>> + *	Returns 1 in case of pass , 0 in case of failure, and -1 for error.
>> + */
>> +
>> +static int check_refresh_rate(data_t *data, refresh_rate_t *expected_rr)
>> +{
>> +/*
>> + * The calculated refresh-rate is an approximation to estimated refresh-rate.
>> + * The tolerance threshold for the difference is set to 2. Difference more than
>> + * 2 is considered as failure for the check_refresh_rate.
>> + */
>> +#define TOLERANCE_THRESHOLD		2
>> +	int ret, refresh_rate = -1;
>> +	char drrs_status[DRRS_STATUS_BYTES_CNT] = {};
>> +
>> +	/* Signal worker thread to calculate refresh rate */
>> +	pthread_mutex_lock(&rr_mutex);
>> +	rr_calc_requested = SET;
>> +	pthread_mutex_unlock(&rr_mutex);
>> +	pthread_cond_signal(&cv_rr_calc_requested);
>> +
>> +	/* Get the DRRS Status from the debugfs entry */
>> +	if (!read_drrs_status(data, drrs_status)) {
>> +		igt_info("ERROR : DRRS debugfs read FAILED\n");
>> +		return -1;
>> +	}
>> +	/* Wait for the refresh rate calculator thread */
>> +	pthread_mutex_lock(&rr_mutex);
>> +	while (!rr_calc_completed)
>> +		pthread_cond_wait(&cv_rr_calc_completed, &rr_mutex);
>> +	rr_calc_completed = RESET;
>> +	refresh_rate = refresh_rate_shared;
>> +	pthread_mutex_unlock(&rr_mutex);
>> +	if (refresh_rate < 0) {
>> +		igt_info("ERROR : Refresh rate calculation FAILED\n");
>> +		return -1;
>> +	}
>> +	igt_info("INFO : Actual Refresh_rate based on Vblank Counts/sec = %d\n"
>> +							, refresh_rate);
>> +
>> +	/* Compare with Expected Refresh Rate*/
>> +	igt_info("INFO: Expected Refresh-Rate %d\n", expected_rr->rate);
>> +	if (strstr(drrs_status, expected_rr->name) != NULL
>> +	&& abs(refresh_rate - expected_rr->rate) <= TOLERANCE_THRESHOLD) {
>> +		igt_info("INFO: Expected %s : PASSED\n", expected_rr->name);
>> +		return 1;
>> +	}
>> +
>> +	igt_info("INFO : Expected %s : FAILED\n", expected_rr->name);
>> +	return 0;
>> +}
>> +
>> +/*
>> + * clean_up :
>> + *	Func to stop the worker thread and destroy thread syncronization
>> + *	structures.
>> + */
>> +void clean_up(pthread_t rr_thread)
>> +{
>> +	/* Stop the worker thread */
>> +	pthread_mutex_lock(&rr_mutex);
>> +	thread_flag = RESET;
>> +	rr_calc_requested = SET;
>> +	pthread_mutex_unlock(&rr_mutex);
>> +	pthread_cond_signal(&cv_rr_calc_requested);
>> +	pthread_join(rr_thread, NULL);
>> +
>> +	/* destroy the condition variables and mutex */
>> +	pthread_cond_destroy(&cv_rr_calc_requested);
>> +	pthread_cond_destroy(&cv_rr_calc_completed);
>> +	pthread_mutex_destroy(&rr_mutex);
>> +}
>> +
>> +/*
>> + * execute_test:
>> + *	This function
>> + *	1. Initializes the thread sync structures.
>> + *	2. Creates a worker thread which can be signaled to estimate
>> + *	the refresh rates based on counting vblanks per sec.
>> + *	3. Creates two different Framebuffer fb0, fb1
>> + *	and apply them in the interval of 1.2Sec. So between FB change
>> + *	DRRS should toggle from DRRS_HIGH_RR -> DRRS_LOW_RR.
>> + *	And on next FB change DRRS_LOW_RR -> DRRS_HIGH_RR.
>> + *
>> + *	These expected transistions are verified using the kernel debugfs
>> + *	and the refresh-rate, calculated by counting vblanks per sec by the
>> + *	worker thread.
>> + */
>> +static bool execute_test(data_t *data)
>> +{
>> +	igt_display_t *display = &data->display;
>> +	igt_output_t *output = data->output;
>> +	drmModeModeInfo *mode, *supported_modes;
>> +	bool test_failed = false;
>> +	int ret, i, count_modes;
>> +	refresh_rate_t high_rr, low_rr;
>> +	pthread_t rr_thread;
>> +
>> +	/*initialize the condition variables and  mutex*/
>> +	pthread_cond_init(&cv_rr_calc_requested, NULL);
>> +	pthread_cond_init(&cv_rr_calc_completed, NULL);
>> +	pthread_mutex_init(&rr_mutex, NULL);
>> +	thread_flag = SET;
>> +	rr_calc_requested = RESET;
>> +	rr_calc_completed = RESET;
>> +	/* Create a thread for calculating the refresh rate */
>> +	ret = pthread_create(&rr_thread, NULL, worker_thread_func,
>> +							(void *) data);
>> +	if (ret == -1) {
>> +		igt_info("ERROR : Refresh rate thread creation FAILED\n");
>> +		clean_up(rr_thread);
>> +		return false;
>> +	}
>> +	/*close the write end of the pipe as it will be used for reading only*/
> Where is the write-end of the pipe?

Ankit- Removed this comment.

>> +
>> +	high_rr.name = "DRRS_HIGH_RR";
>> +	high_rr.rate = 0;
>> +	low_rr.name = "DRRS_LOW_RR";
>> +	low_rr.rate = 0;
>> +	/* get the max and min supported refresh rates for the display */
>> +	supported_modes = output->config.connector->modes;
>> +	count_modes = output->config.connector->count_modes;
>> +	/* minimum 2 modes are required for DRRS */
>> +	if (count_modes < 2) {
>> +		igt_info("ERROR: Minimum 2 modes required for DRRS\n");
>> +		return false;
>> +	}
>> +	for (i = 0; i < count_modes; i++) {
>> +		int rr = supported_modes[i].vrefresh;
>> +
>> +		if (i == 0) {
>> +			high_rr.rate = low_rr.rate = rr;
>> +			continue;
>> +		}
>> +		if (high_rr.rate < rr)
>> +			high_rr.rate = rr;
>> +		if (low_rr.rate > rr)
>> +			low_rr.rate = rr;
>> +	}
>> +	data->primary = igt_output_get_plane(data->output, IGT_PLANE_PRIMARY);
>> +	mode = igt_output_get_mode(data->output);
>> +	data->fb_id[0] = igt_create_color_fb(data->drm_fd, mode->hdisplay,
>> +			mode->vdisplay,
>> +			DRM_FORMAT_XRGB8888,
>> +			LOCAL_DRM_FORMAT_MOD_NONE,
>> +			0.0, 100.1, 0.0, &data->fb[0]);
>> +	igt_assert(data->fb_id[0]);
>> +	data->fb_id[1] = igt_create_color_fb(data->drm_fd, mode->hdisplay,
>> +			mode->vdisplay,
>> +			DRM_FORMAT_XRGB8888,
>> +			LOCAL_DRM_FORMAT_MOD_NONE,
>> +			100.1, 0.0, 0.0,
>> +			&data->fb[1]);
>> +	igt_assert(data->fb_id[1]);
>> +
>> +	data->handle[0] = data->fb[0].gem_handle;
>> +	data->handle[1] = data->fb[1].gem_handle;
>> +
>> +	/* scanout = fb[1] */
>> +	igt_plane_set_fb(data->primary, &data->fb[1]);
>> +	igt_display_commit(display);
>> +
> I assume that the behaviour is that once you have committed your
> frame-buffer, DRRS should be enabled by the kernel, hence the check here
> again. You do this (albeit manually) in igt_fixture block.

Ankit– Yes you are right. There are two things to be checked. 1st is 
whether DRRS is supported from VBT/Panel side, and this is checked in 
igt_simple_main. The 2nd thing is whether DRRS is enabled by the driver, 
checked once the framebuffer is committed.

>
>> +	if (!is_drrs_enabled(data)) {
>> +		igt_info("INFO : DRRS not enabled\n");
>> +		igt_plane_set_fb(data->primary, NULL);
>> +		igt_output_set_pipe(output, PIPE_ANY);
>> +		igt_display_commit(display);
>> +
>> +		igt_remove_fb(data->drm_fd, &data->fb[0]);
>> +		igt_remove_fb(data->drm_fd, &data->fb[1]);
>> +		clean_up(rr_thread);
>> +		return false;
>> +	}
>> +
>> +	/* expecting High RR */
>> +	ret =  check_refresh_rate(data, &high_rr);
>> +	if (ret != 1)
>> +		test_failed = true;
>> +	/* 1.2Sec to Enter the Idleness and check for Low Refresh Rate */
>> +	usleep(1200000);
>> +	ret =  check_refresh_rate(data, &low_rr);
>> +	if (ret != 1)
>> +		test_failed = true;
>> +
>> +	/* scanout = fb[0] and check for High Refresh Rate*/
>> +	igt_plane_set_fb(data->primary, &data->fb[0]);
>> +	igt_display_commit(display);
>> +	ret =  check_refresh_rate(data, &high_rr);
>> +	if (ret != 1)
>> +		test_failed = true;
>> +
>> +	/* 1.2Sec to Enter the Idleness and check for Low Refresh Rate */
>> +	usleep(1200000);
>> +	ret =  check_refresh_rate(data, &low_rr);
>> +	if (ret != 1)
>> +		test_failed = true;
>> +
>> +	/* scanout = fb[1] and check for High Refresh Rate*/
>> +	igt_plane_set_fb(data->primary, &data->fb[1]);
>> +	igt_display_commit(display);
>> +	ret =  check_refresh_rate(data, &high_rr);
>> +	if (ret != 1)
>> +		test_failed = true;
>> +
>> +	/* 1.2Sec to Enter the Idleness */
>> +	usleep(1200000);
>> +	ret =  check_refresh_rate(data, &low_rr);
>> +	if (ret != 1)
>> +		test_failed = true;
>> +
>> +	igt_assert(!test_failed);
> Do you really want to exit with an assert if DRRS test failed?
>
> This function returns false when DRRS is not present and bails out
> earlier if test has failed. You might want to have returned an int with
> -1 for errors, 0 for test succeeded, 1 when test not succeeded (you seem
> to be using this notation in one of the functions above).

Ankit- Returning int here, -1 for errors, 0 for test success and 1 for 
failure.

>
>> +	clean_up(rr_thread);
>> +	return true;
>> +}
>> +
>> +/*
>> + * finish_crtc :
>> + *	Func to free the framebuffers after the test completion.
>> + */
>> +static void finish_crtc(data_t *data)
>> +{
>> +	igt_plane_set_fb(data->primary, NULL);
>> +	igt_output_set_pipe(data->output, PIPE_ANY);
>> +	igt_display_commit(&data->display);
>> +
>> +	igt_remove_fb(data->drm_fd, &data->fb[0]);
>> +	igt_remove_fb(data->drm_fd, &data->fb[1]);
>> +}
>> +
>> +/*
>> + * reset_display :
>> + *	Func to reset the display structures after the test completion.
>> + */
>> +static void reset_display(data_t *data)
>> +{
>> +	igt_display_t *display = &data->display;
>> +
>> +	for_each_connected_output(display, data->output) {
>> +		if (data->output->valid) {
>> +			data->primary =  igt_output_get_plane(data->output,
>> +							IGT_PLANE_PRIMARY);
>> +			igt_plane_set_fb(data->primary, NULL);
>> +		}
>> +		igt_output_set_pipe(data->output, PIPE_ANY);
>> +	}
>> +}
>> +/*
>> + * run_test :
>> + *	Func to run the test for the eDP display.
>> + */
>> +static void run_test(data_t *data)
>> +{
>> +	igt_display_t *display = &data->display;
>> +	int valid_tests = 0;
>> +
>> +	reset_display(data);
>> +	/* Check for eDP Connector on PIPE A ,as eDP goes normally on PIPE_A */
>> +	for_each_connected_output(display, data->output) {
>> +		drmModeConnectorPtr c = data->output->config.connector;
>> +
>> +		if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
>> +			c->connection != DRM_MODE_CONNECTED)
>> +			continue;
>> +			data->pipe = PIPE_A;
>> +			prepare_crtc(data);
> prepare_crtc() returns a bool.

Ankit- Return value is handled properly.

>> +			igt_info("INFO : Beginning %s on pipe %s,connector %s\n",
>> +						igt_subtest_name(),
>> +						kmstest_pipe_name(data->pipe),
>> +						igt_output_name(data->output));
>> +
>> +			if (!execute_test(data)) {
>> +				igt_info("INFO :%s on pipe %s, connector %s:SKIPPED\n",
>> +						igt_subtest_name(),
>> +						kmstest_pipe_name(data->pipe),
>> +						igt_output_name(data->output));
>> +				continue;
>> +			}
>> +
>> +			valid_tests++;
>> +
>> +			igt_info("INFO :%s on pipe %s, connector %s: PASSED\n",
>> +					igt_subtest_name(),
>> +					kmstest_pipe_name(data->pipe),
>> +					igt_output_name(data->output));
>> +
>> +			finish_crtc(data);
>> +	}
>> +}
>> +
>> +igt_main
> Might want to use igt_simple_main for only one test.

Ankit – igt_simple_main is used.

>> +{
>> +	data_t data = {};
>> +
>> +	igt_skip_on_simulation();
>> +
>> +	igt_fixture {
>> +		char buf[64];
>> +		int cnt;
>> +		FILE *status;
>> +
>> +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
>> +		if (data.drm_fd == -1) {
>> +			igt_info("ERROR : invalid fd\n");
>> +			return;
>> +		}
>> +
>> +		data.devid = intel_get_drm_devid(data.drm_fd);
>> +		if (data.devid < 0) {
>> +			igt_info("ERROR : invalid dev id\n");
>> +			return;
>> +		}
>> +
>> +		status = igt_debugfs_fopen("i915_drrs_status", "r");
>> +		if (status == NULL) {
>> +			igt_info("ERROR : debugfs open FAILED\n");
>> +			return;
>> +		}
>> +		igt_require_f(status, "No i915_drrs_status found\n");
>> +		cnt = fread(buf, sizeof(buf), 1, status);
>> +		if (!cnt) {
>> +			igt_info("ERROR : fread FAILED\n");
>> +			return;
>> +		}
>> +		fclose(status);
>> +		buf[sizeof(buf) - 1] = '\0';
>> +		igt_require_f(!strstr(buf, "disabled"),
>> +				"DRRS not supported:check VBT/panel caps\n");
> You have a couple functions to test DRRS presence bou do it (again)
> manually. Also you check for each connected output in you do it
> (again) manually.

Ankit- Reusing the existing function is_drrs_enabled() for checking the 
DRRS support. However the function name was a misnomer, so changed it to 
is_drrs_supported.

>> +		/* Check if the DRRS is supported.
>> +		 * If yes call the Idleness DRRS test
>> +		 */
> This comments seems to belong in a different place.

Ankit- Added the comments in proper place.

>> +
>> +		igt_display_init(&data.display, data.drm_fd);
>> +	}
>> +	igt_subtest_f("Idleness_DRRS")
>> +		run_test(&data);
>> +
>> +	igt_fixture {
>> +		igt_display_fini(&data.display);
>> +	}
>> +}
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Thanks for the comments , suggestions pointing out the misses. I will 
address these comments and send new version of the patch.

Regards,
Ankit

[-- Attachment #1.2: Type: text/html, Size: 449265 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] Idleness DRRS test
  2016-09-23 12:07   ` Nautiyal, Ankit K
@ 2016-09-23 12:40     ` Nautiyal Ankit
  2016-09-23 13:18       ` Chris Wilson
  0 siblings, 1 reply; 12+ messages in thread
From: Nautiyal Ankit @ 2016-09-23 12:40 UTC (permalink / raw)
  To: intel-gfx, daniel.vetter, chris, rodrigo.vivi; +Cc: paulo.r.zanoni

From: Ramalingam C <ramalingam.c@intel.com>

Idleness DRRS:
	By default the DRRS state will be at DRRS_HIGH_RR. When a Display
	content is Idle for more than 1Sec Idleness will be declared and
	DRRS_LOW_RR will be invoked, changing the refresh rate to the
	lower most refresh rate supported by the panel. As soon as there
	is a display content change there will be a DRRS state transition
	as DRRS_LOW_RR--> DRRS_HIGH_RR, changing the refresh rate to the
	highest refresh rate supported by the panel.

To test this, Idleness DRRS IGT will probe the DRRS state at below
instances and compare with the expected state.

	Instance					Expected State
1. Immediately after rendering the still image		DRRS_HIGH_RR
2. After a delay of 1.2Sec				DRRS_LOW_RR
3. After changing the frame buffer			DRRS_HIGH_RR
4. After a delay of 1.2Sec				DRRS_LOW_RR
5. After changing the frame buffer			DRRS_HIGH_RR
6. After a delay of 1.2Sec				DRRS_LOW_RR

The test checks the driver DRRS state from the debugfs entry. To check the
actual refresh-rate, a separate thread counts the number of vblanks
received per sec. The refresh-rate calculated is checked against the
expected refresh-rate with a tolerance value of 2.

This patch is a continuation of the earlier work
https://patchwork.freedesktop.org/patch/45472/ towards igt for idleness

DRRS. The code is tested on Broxton BXT_T platform.

v2: Addressed the comments and suggestions from Vlad, Marius.
The signoff details from the earlier work are also included.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com>
---
 tests/Makefile.sources |   1 +
 tests/kms_drrs.c       | 612 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 613 insertions(+)
 create mode 100644 tests/kms_drrs.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index a837977..5f31521 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -91,6 +91,7 @@ TESTS_progs_M = \
 	kms_cursor_crc \
 	kms_cursor_legacy \
 	kms_draw_crc \
+	kms_drrs \
 	kms_fbc_crc \
 	kms_fbcon_fbt \
 	kms_flip \
diff --git a/tests/kms_drrs.c b/tests/kms_drrs.c
new file mode 100644
index 0000000..69f8b06
--- /dev/null
+++ b/tests/kms_drrs.c
@@ -0,0 +1,612 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "drmtest.h"
+#include "igt_debugfs.h"
+#include "igt_kms.h"
+#include "intel_chipset.h"
+#include "intel_batchbuffer.h"
+#include "ioctl_wrappers.h"
+#include <time.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <sys/time.h>
+IGT_TEST_DESCRIPTION(
+"Performs write operations and then waits for DRRS to invoke the"
+"Low Refresh Rate and then disturbs the contents of the screen once"
+"again hence DRRS revert back to High Refresh Rate(Default).");
+
+#define DRRS_STATUS_BYTES_CNT	1000
+#define SET	1
+#define RESET	0
+
+/*
+ * Structure to store data to create 2 framebuffers, fb[0] and fb[1] on a given
+ * display. To disturb the content of the screen, we replace fb[0] by fb[1] and
+ * vice versa.
+ */
+typedef struct {
+	int drm_fd;
+	uint32_t devid;
+	uint32_t handle[2];
+	igt_display_t display;
+	igt_output_t *output;
+	enum pipe pipe;
+	igt_plane_t *primary;
+	struct igt_fb fb[2];
+	uint32_t fb_id[2];
+} data_t;
+
+/*
+ * Structure to count vblank and note the starting time of the counter
+ */
+typedef struct {
+	unsigned int vbl_count;
+	struct timeval start;
+} vbl_info;
+
+/*
+ * Condition variables,mutex, and shared variables for thread synchronization
+ */
+pthread_mutex_t rr_mutex;
+pthread_cond_t cv_rr_calc_requested;
+pthread_cond_t cv_rr_calc_completed;
+int rr_calc_requested = RESET;
+int rr_calc_completed = RESET;
+int thread_flag = SET;
+int refresh_rate_shared = -1;
+
+/*
+ * Structure for refresh rate type
+ */
+typedef struct{
+	int rate;
+	const char *name;
+} refresh_rate_t;
+
+/*
+ * vblank_handler :
+ *	User defined vblank event handler, to count vblanks.
+ *	The control reaches this function after a vblank event is registered
+ */
+static void vblank_handler(int fd, unsigned int frame,
+			unsigned int sec, unsigned int usec, void *data)
+{
+	/* The control reaches this function after a vblank event is
+	 * registered
+	 */
+	vbl_info *info = (vbl_info *) data;
+
+	if (info == NULL) {
+		igt_info("ERROR: Invalid Data passed to the vblank handler\n");
+		return;
+	}
+	info->vbl_count++;
+}
+
+
+/*
+ * read_drrs_status :
+ *	Func to read the DRRS status from debugfs
+ */
+static bool read_drrs_status(char *str)
+{
+	FILE *status;
+	int cnt;
+
+	status = igt_debugfs_fopen("i915_drrs_status", "r");
+	igt_assert(status);
+
+	cnt = fread(str, DRRS_STATUS_BYTES_CNT - 1, 1, status);
+	if (!cnt) {
+		if (!feof(status)) {
+			igt_info("ERROR: %d at fread\n", ferror(status));
+			return false;
+		}
+		clearerr(status);
+	}
+	fclose(status);
+	return true;
+}
+
+/*
+ * is_drrs_supported :
+ *	Func to check for DRRS support
+ */
+static bool is_drrs_supported(void)
+{
+	char str[DRRS_STATUS_BYTES_CNT] = {};
+
+	if (!read_drrs_status(str)) {
+		igt_info("ERROR: Debugfs read FAILED\n");
+		return false;
+	}
+	return strstr(str, "DRRS Supported: Yes") != NULL;
+}
+
+/*
+ * is_drrs_enabled :
+ *	Func to check if DRRS is enabled by driver
+ */
+static bool is_drrs_enabled(void)
+{
+	char str[DRRS_STATUS_BYTES_CNT] = {};
+
+	if (!read_drrs_status(str)) {
+		igt_info("ERROR: Debugfs read FAILED\n");
+		return false;
+	}
+	return strstr(str, "Idleness DRRS: Disabled") == NULL;
+}
+
+/*
+ * prepare_crtc :
+ *	Func to prepare crtc for the given display
+ */
+static bool prepare_crtc(data_t *data)
+{
+	if (data == NULL)
+		return false;
+	igt_display_t *display = &data->display;
+	igt_output_t *output = data->output;
+
+	/* select the pipe we want to use */
+	igt_output_set_pipe(output, data->pipe);
+	igt_display_commit(display);
+
+	if (!output->valid) {
+		igt_output_set_pipe(output, PIPE_ANY);
+		igt_display_commit(display);
+		return false;
+	}
+
+	return true;
+}
+
+/*
+ * calculate_refresh_rate :
+ *	Func to calculate the refresh rate by counting
+ *	vblanks in 1 sec. It returns the calculated refresh rate on success.
+ *	Returns -1 in case of error.
+ */
+int calculate_refresh_rate(data_t *data)
+{
+	drmEventContext evctx;
+	drmVBlank vbl;
+	vbl_info handler_info;
+	struct timeval start_time, curr_time;
+	double time_elapsed;
+	int refresh_rate = 0, vbl_count = 0;
+	/*set up event context for handling vblank events*/
+	memset(&evctx, 0, sizeof(evctx));
+	evctx.version = DRM_EVENT_CONTEXT_VERSION;
+	evctx.vblank_handler = vblank_handler;
+	evctx.page_flip_handler = NULL;
+
+	/*initialize the vbl count to zero and set the starting time*/
+	handler_info.vbl_count = 0;
+	gettimeofday(&start_time, NULL);
+	handler_info.start = start_time;
+	curr_time = start_time;
+
+	time_elapsed = 0.0;
+	/* To count vblanks in 1 sec. Loop till
+	 * (current-time - starting time) <= 1.0 sec
+	 */
+	while (time_elapsed <= 1.0) {
+		int ret;
+
+		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
+		vbl.request.sequence = 1;
+		vbl.request.signal = (unsigned long)&handler_info;
+		ret = drmWaitVBlank(data->drm_fd, &vbl);
+		if (ret != 0) {
+			igt_info("ERROR : drmWaitVBlank FAILED :%d\n", ret);
+			return -1;
+		}
+		/*call drmHandleEvent() to handle vblank event */
+		ret = drmHandleEvent(data->drm_fd, &evctx);
+		if (ret != 0) {
+			igt_info("ERROR : drmHandleEvent FAILED %d\n", ret);
+			return -1;
+		}
+
+		gettimeofday(&curr_time, NULL);
+		time_elapsed = curr_time.tv_sec + curr_time.tv_usec * 1e-6 -
+			(start_time.tv_sec + start_time.tv_usec * 1e-6);
+	}
+	if (!time_elapsed) {
+		igt_info("ERROR: Incorrect measurement of vblank duration\n");
+		return -1;
+	}
+	vbl_count = handler_info.vbl_count;
+	/* calculate the refresh rate; rr=vblank_count/time_taken */
+	refresh_rate = vbl_count / time_elapsed;
+	return refresh_rate;
+}
+
+/*
+ * worker_thread_func :
+ *	Func which is run by a worker thread to calculate the refresh_rate,
+ *	when signalled by the master thread.
+ */
+void *worker_thread_func(void *ptr)
+{
+	data_t *data = (data_t *) ptr;
+
+	while (1) {
+		int refresh_rate = 0;
+		/*wait for signal from master*/
+		pthread_mutex_lock(&rr_mutex);
+		while (!rr_calc_requested)
+			pthread_cond_wait(&cv_rr_calc_requested, &rr_mutex);
+
+		/* checkpoint for thread termination */
+		if (thread_flag == RESET) {
+			pthread_mutex_unlock(&rr_mutex);
+			pthread_exit(NULL);
+		}
+		rr_calc_requested = RESET;
+		pthread_mutex_unlock(&rr_mutex);
+		/*calculate refresh_rate*/
+		refresh_rate = calculate_refresh_rate(data);
+
+		/* signal the master */
+		pthread_mutex_lock(&rr_mutex);
+		refresh_rate_shared = refresh_rate;
+		rr_calc_completed = SET;
+		pthread_mutex_unlock(&rr_mutex);
+		pthread_cond_signal(&cv_rr_calc_completed);
+	}
+	return NULL;
+}
+
+/*
+ * check_refresh_rate :
+ *	Func to check the refresh rate calculated, against the expected refresh
+ *	rate. It signals the worker thread to calculate the refresh-
+ *	rate, and checks if the correct refresh rate is switched.
+ *	Returns 0 in case of pass , 1 in case of failure, and -1 for error.
+ */
+
+static int check_refresh_rate(data_t *data, refresh_rate_t *expected_rr)
+{
+/*
+ * The calculated refresh-rate is an approximation to estimated refresh-rate.
+ * The tolerance threshold for the difference is set to 2. Difference more than
+ * 2 is considered as failure for the check_refresh_rate.
+ */
+#define TOLERANCE_THRESHOLD		2
+	int refresh_rate = -1;
+	char drrs_status[DRRS_STATUS_BYTES_CNT] = {};
+
+	/* Signal worker thread to calculate refresh rate */
+	pthread_mutex_lock(&rr_mutex);
+	rr_calc_requested = SET;
+	pthread_mutex_unlock(&rr_mutex);
+	pthread_cond_signal(&cv_rr_calc_requested);
+
+	/* Get the DRRS Status from the debugfs entry */
+	if (!read_drrs_status(drrs_status)) {
+		igt_info("ERROR : DRRS debugfs read FAILED\n");
+		return -1;
+	}
+	/* Wait for the refresh rate calculator thread */
+	pthread_mutex_lock(&rr_mutex);
+	while (!rr_calc_completed)
+		pthread_cond_wait(&cv_rr_calc_completed, &rr_mutex);
+	rr_calc_completed = RESET;
+	refresh_rate = refresh_rate_shared;
+	pthread_mutex_unlock(&rr_mutex);
+	if (refresh_rate < 0) {
+		igt_info("ERROR : Refresh rate calculation FAILED\n");
+		return -1;
+	}
+	igt_info("INFO : Actual Refresh_rate based on Vblank Counts/sec = %d\n"
+							, refresh_rate);
+
+	/* Compare with Expected Refresh Rate*/
+	igt_info("INFO: Expected Refresh-Rate %d\n", expected_rr->rate);
+	if (strstr(drrs_status, expected_rr->name) != NULL
+	&& abs(refresh_rate - expected_rr->rate) <= TOLERANCE_THRESHOLD) {
+		igt_info("INFO: Expected %s : PASSED\n", expected_rr->name);
+		return 0;
+	}
+
+	igt_info("INFO : Expected %s : FAILED.\n", expected_rr->name);
+	return 1;
+}
+
+/*
+ * clean_up :
+ *	Func to stop the worker thread and destroy thread syncronization
+ *	structures.
+ */
+void clean_up(pthread_t rr_thread)
+{
+	/* Stop the worker thread */
+	pthread_mutex_lock(&rr_mutex);
+	thread_flag = RESET;
+	rr_calc_requested = SET;
+	pthread_mutex_unlock(&rr_mutex);
+	pthread_cond_signal(&cv_rr_calc_requested);
+	pthread_join(rr_thread, NULL);
+
+	/* destroy the condition variables and mutex */
+	pthread_cond_destroy(&cv_rr_calc_requested);
+	pthread_cond_destroy(&cv_rr_calc_completed);
+	pthread_mutex_destroy(&rr_mutex);
+}
+
+/*
+ * execute_test:
+ *	This function
+ *	1. Initializes the thread sync structures.
+ *	2. Creates a worker thread which can be signaled to estimate
+ *	the refresh rates based on counting vblanks per sec.
+ *	3. Creates two different Framebuffer fb0, fb1
+ *	and apply them in the interval of 1.2Sec. So between FB change
+ *	DRRS should toggle from DRRS_HIGH_RR -> DRRS_LOW_RR.
+ *	And on next FB change DRRS_LOW_RR -> DRRS_HIGH_RR.
+ *
+ *	These expected transitions are verified using the kernel debugfs
+ *	and the refresh-rate, calculated by counting vblanks per sec by the
+ *	worker thread.
+ */
+static int execute_test(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output = data->output;
+	drmModeModeInfo *mode, *supported_modes;
+	int test_failed = 0;
+	int ret, i, count_modes;
+	refresh_rate_t high_rr, low_rr;
+	pthread_t rr_thread;
+
+	/*initialize the condition variables and  mutex*/
+	pthread_cond_init(&cv_rr_calc_requested, NULL);
+	pthread_cond_init(&cv_rr_calc_completed, NULL);
+	pthread_mutex_init(&rr_mutex, NULL);
+	thread_flag = SET;
+	rr_calc_requested = RESET;
+	rr_calc_completed = RESET;
+	/* create a thread for calculating the refresh rate */
+	ret = pthread_create(&rr_thread, NULL, worker_thread_func,
+							(void *) data);
+	if (ret == -1) {
+		igt_info("ERROR : Refresh rate thread creation FAILED\n");
+		clean_up(rr_thread);
+		return -1;
+	}
+
+	high_rr.name = "DRRS_HIGH_RR";
+	high_rr.rate = 0;
+	low_rr.name = "DRRS_LOW_RR";
+	low_rr.rate = 0;
+	/* get the max and min supported refresh rates for the display */
+	supported_modes = output->config.connector->modes;
+	count_modes = output->config.connector->count_modes;
+	/* minimum 2 modes are required for DRRS */
+	if (count_modes < 2) {
+		igt_info("ERROR: Minimum 2 modes required for DRRS\n");
+		return -1;
+	}
+	for (i = 0; i < count_modes; i++) {
+		int rr = supported_modes[i].vrefresh;
+
+		if (i == 0) {
+			high_rr.rate = low_rr.rate = rr;
+			continue;
+		}
+		if (high_rr.rate < rr)
+			high_rr.rate = rr;
+		if (low_rr.rate > rr)
+			low_rr.rate = rr;
+	}
+	data->primary = igt_output_get_plane(data->output, IGT_PLANE_PRIMARY);
+	mode = igt_output_get_mode(data->output);
+	data->fb_id[0] = igt_create_color_fb(data->drm_fd, mode->hdisplay,
+			mode->vdisplay,
+			DRM_FORMAT_XRGB8888,
+			LOCAL_DRM_FORMAT_MOD_NONE,
+			0.0, 100.1, 0.0, &data->fb[0]);
+	igt_assert(data->fb_id[0]);
+	data->fb_id[1] = igt_create_color_fb(data->drm_fd, mode->hdisplay,
+			mode->vdisplay,
+			DRM_FORMAT_XRGB8888,
+			LOCAL_DRM_FORMAT_MOD_NONE,
+			100.1, 0.0, 0.0,
+			&data->fb[1]);
+	igt_assert(data->fb_id[1]);
+
+	data->handle[0] = data->fb[0].gem_handle;
+	data->handle[1] = data->fb[1].gem_handle;
+
+	/* scanout = fb[1] */
+	igt_plane_set_fb(data->primary, &data->fb[1]);
+	igt_display_commit(display);
+	if (!is_drrs_enabled()) {
+		igt_info("INFO : DRRS not enabled\n");
+		igt_plane_set_fb(data->primary, NULL);
+		igt_output_set_pipe(output, PIPE_ANY);
+		igt_display_commit(display);
+		igt_remove_fb(data->drm_fd, &data->fb[0]);
+		igt_remove_fb(data->drm_fd, &data->fb[1]);
+		clean_up(rr_thread);
+		return -1;
+	}
+
+	/* expecting High RR */
+	ret =  check_refresh_rate(data, &high_rr);
+	if (ret == -1)
+		return -1;
+	if (ret == 1)
+		test_failed = 1;
+	/* 1.2Sec to Enter the Idleness and check for Low Refresh Rate */
+	usleep(1200000);
+	ret =  check_refresh_rate(data, &low_rr);
+	if (ret == -1)
+		return -1;
+	if (ret == 1)
+		test_failed = 1;
+
+	/* scanout = fb[0] and check for High Refresh Rate*/
+	igt_plane_set_fb(data->primary, &data->fb[0]);
+	igt_display_commit(display);
+	ret =  check_refresh_rate(data, &high_rr);
+	if (ret == -1)
+		return -1;
+	if (ret == 1)
+		test_failed = 1;
+
+	/* 1.2Sec to Enter the Idleness and check for Low Refresh Rate */
+	usleep(1200000);
+	ret =  check_refresh_rate(data, &low_rr);
+	if (ret == -1)
+		return -1;
+	if (ret == 1)
+		test_failed = 1;
+
+	/* scanout = fb[1] and check for High Refresh Rate*/
+	igt_plane_set_fb(data->primary, &data->fb[1]);
+	igt_display_commit(display);
+	ret =  check_refresh_rate(data, &high_rr);
+	if (ret == -1)
+		return -1;
+	if (ret == 1)
+		test_failed = 1;
+
+	/* 1.2Sec to Enter the Idleness */
+	usleep(1200000);
+	ret =  check_refresh_rate(data, &low_rr);
+	if (ret == -1)
+		return -1;
+	if (ret == 1)
+		test_failed = 1;
+
+	clean_up(rr_thread);
+	return test_failed;
+}
+
+/*
+ * finish_crtc :
+ *	Func to free the framebuffers after the test completion.
+ */
+static void finish_crtc(data_t *data)
+{
+	igt_plane_set_fb(data->primary, NULL);
+	igt_output_set_pipe(data->output, PIPE_ANY);
+	igt_display_commit(&data->display);
+
+	igt_remove_fb(data->drm_fd, &data->fb[0]);
+	igt_remove_fb(data->drm_fd, &data->fb[1]);
+}
+
+/*
+ * reset_display :
+ *	Func to reset the display structures after the test completion.
+ */
+static void reset_display(data_t *data)
+{
+	igt_display_t *display = &data->display;
+
+	for_each_connected_output(display, data->output) {
+		if (data->output->valid) {
+			data->primary =  igt_output_get_plane(data->output,
+							IGT_PLANE_PRIMARY);
+			igt_plane_set_fb(data->primary, NULL);
+		}
+		igt_output_set_pipe(data->output, PIPE_ANY);
+	}
+}
+/*
+ * run_test :
+ *	Func to run the test for the eDP display.
+ */
+static void run_test(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	int ret;
+
+	reset_display(data);
+	/* Check for eDP Connector on PIPE A ,as eDP goes normally on PIPE_A */
+	for_each_connected_output(display, data->output) {
+		drmModeConnectorPtr c = data->output->config.connector;
+
+		if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
+			c->connection != DRM_MODE_CONNECTED)
+			continue;
+		data->pipe = PIPE_A;
+		igt_assert_f(prepare_crtc(data), "Failed to prepare CRTC\n");
+		igt_info("INFO : Beginning %s on pipe %s,connector %s\n",
+					igt_test_name(),
+					kmstest_pipe_name(data->pipe),
+					igt_output_name(data->output));
+
+		ret = execute_test(data);
+		igt_skip_on_f(ret == -1,
+				"INFO :%s on pipe %s, connector %s:SKIPPED\n",
+					igt_test_name(),
+					kmstest_pipe_name(data->pipe),
+					igt_output_name(data->output));
+		igt_fail_on_f(ret == 1,
+				"INFO :%s on pipe %s, connector %s:FAILED\n",
+					igt_test_name(),
+					kmstest_pipe_name(data->pipe),
+					igt_output_name(data->output));
+
+		igt_assert_f(ret == 0,
+				"INFO :%s on pipe %s, connector %s: PASSED\n",
+					igt_test_name(),
+					kmstest_pipe_name(data->pipe),
+					igt_output_name(data->output));
+
+		finish_crtc(data);
+	}
+}
+
+igt_simple_main
+{
+	data_t data = {};
+
+	igt_skip_on_simulation();
+	data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+	if (data.drm_fd == -1) {
+		igt_info("ERROR : invalid fd\n");
+		return;
+	}
+	data.devid = intel_get_drm_devid(data.drm_fd);
+	if (data.devid < 0) {
+		igt_info("ERROR : invalid dev id\n");
+		return;
+	}
+	/* Check if the DRRS is supported.
+	 * If yes call the Idleness DRRS test
+	 */
+	igt_require_f(is_drrs_supported(),
+			"DRRS not supported:check VBT/panel caps\n");
+	igt_display_init(&data.display, data.drm_fd);
+	run_test(&data);
+	igt_display_fini(&data.display);
+}
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] Idleness DRRS test
  2016-09-23 12:40     ` [PATCH v2] " Nautiyal Ankit
@ 2016-09-23 13:18       ` Chris Wilson
  2016-09-27 11:19         ` Nautiyal, Ankit K
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2016-09-23 13:18 UTC (permalink / raw)
  To: Nautiyal Ankit; +Cc: paulo.r.zanoni, intel-gfx, rodrigo.vivi, daniel.vetter

On Fri, Sep 23, 2016 at 06:10:29PM +0530, Nautiyal Ankit wrote:
> From: Ramalingam C <ramalingam.c@intel.com>
> 
> Idleness DRRS:
> 	By default the DRRS state will be at DRRS_HIGH_RR. When a Display
> 	content is Idle for more than 1Sec Idleness will be declared and
> 	DRRS_LOW_RR will be invoked, changing the refresh rate to the
> 	lower most refresh rate supported by the panel. As soon as there
> 	is a display content change there will be a DRRS state transition
> 	as DRRS_LOW_RR--> DRRS_HIGH_RR, changing the refresh rate to the
> 	highest refresh rate supported by the panel.
> 
> To test this, Idleness DRRS IGT will probe the DRRS state at below
> instances and compare with the expected state.
> 
> 	Instance					Expected State
> 1. Immediately after rendering the still image		DRRS_HIGH_RR
> 2. After a delay of 1.2Sec				DRRS_LOW_RR
> 3. After changing the frame buffer			DRRS_HIGH_RR
> 4. After a delay of 1.2Sec				DRRS_LOW_RR
> 5. After changing the frame buffer			DRRS_HIGH_RR
> 6. After a delay of 1.2Sec				DRRS_LOW_RR
> 
> The test checks the driver DRRS state from the debugfs entry. To check the
> actual refresh-rate, a separate thread counts the number of vblanks
> received per sec. The refresh-rate calculated is checked against the
> expected refresh-rate with a tolerance value of 2.
> 
> This patch is a continuation of the earlier work
> https://patchwork.freedesktop.org/patch/45472/ towards igt for idleness
> 
> DRRS. The code is tested on Broxton BXT_T platform.
> 
> v2: Addressed the comments and suggestions from Vlad, Marius.
> The signoff details from the earlier work are also included.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com>
> ---
>  tests/Makefile.sources |   1 +
>  tests/kms_drrs.c       | 612 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 613 insertions(+)
>  create mode 100644 tests/kms_drrs.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index a837977..5f31521 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -91,6 +91,7 @@ TESTS_progs_M = \
>  	kms_cursor_crc \
>  	kms_cursor_legacy \
>  	kms_draw_crc \
> +	kms_drrs \
>  	kms_fbc_crc \
>  	kms_fbcon_fbt \
>  	kms_flip \
> diff --git a/tests/kms_drrs.c b/tests/kms_drrs.c
> new file mode 100644
> index 0000000..69f8b06
> --- /dev/null
> +++ b/tests/kms_drrs.c
> @@ -0,0 +1,612 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "drmtest.h"
> +#include "igt_debugfs.h"
> +#include "igt_kms.h"
> +#include "intel_chipset.h"
> +#include "intel_batchbuffer.h"
> +#include "ioctl_wrappers.h"
> +#include <time.h>
> +#include <pthread.h>
> +#include <stdlib.h>
> +#include <sys/time.h>
> +IGT_TEST_DESCRIPTION(
> +"Performs write operations and then waits for DRRS to invoke the"
> +"Low Refresh Rate and then disturbs the contents of the screen once"
> +"again hence DRRS revert back to High Refresh Rate(Default).");
> +
> +#define DRRS_STATUS_BYTES_CNT	1000
> +#define SET	1
> +#define RESET	0
> +
> +/*
> + * Structure to store data to create 2 framebuffers, fb[0] and fb[1] on a given
> + * display. To disturb the content of the screen, we replace fb[0] by fb[1] and
> + * vice versa.
> + */
> +typedef struct {
> +	int drm_fd;
> +	uint32_t devid;
> +	uint32_t handle[2];
> +	igt_display_t display;
> +	igt_output_t *output;
> +	enum pipe pipe;
> +	igt_plane_t *primary;
> +	struct igt_fb fb[2];
> +	uint32_t fb_id[2];
> +} data_t;
> +
> +/*
> + * Structure to count vblank and note the starting time of the counter
> + */
> +typedef struct {
> +	unsigned int vbl_count;
> +	struct timeval start;
> +} vbl_info;
> +
> +/*
> + * Condition variables,mutex, and shared variables for thread synchronization
> + */
> +pthread_mutex_t rr_mutex;
> +pthread_cond_t cv_rr_calc_requested;
> +pthread_cond_t cv_rr_calc_completed;
> +int rr_calc_requested = RESET;
> +int rr_calc_completed = RESET;
> +int thread_flag = SET;
> +int refresh_rate_shared = -1;
> +
> +/*
> + * Structure for refresh rate type
> + */
> +typedef struct{
> +	int rate;
> +	const char *name;
> +} refresh_rate_t;
> +
> +/*
> + * vblank_handler :
> + *	User defined vblank event handler, to count vblanks.
> + *	The control reaches this function after a vblank event is registered
> + */
> +static void vblank_handler(int fd, unsigned int frame,
> +			unsigned int sec, unsigned int usec, void *data)
> +{
> +	/* The control reaches this function after a vblank event is
> +	 * registered
> +	 */
> +	vbl_info *info = (vbl_info *) data;
> +
> +	if (info == NULL) {
> +		igt_info("ERROR: Invalid Data passed to the vblank handler\n");
> +		return;
> +	}
> +	info->vbl_count++;
> +}
> +
> +
> +/*
> + * read_drrs_status :
> + *	Func to read the DRRS status from debugfs
> + */
> +static bool read_drrs_status(char *str)
> +{
> +	FILE *status;
> +	int cnt;
> +
> +	status = igt_debugfs_fopen("i915_drrs_status", "r");
> +	igt_assert(status);
> +
> +	cnt = fread(str, DRRS_STATUS_BYTES_CNT - 1, 1, status);
> +	if (!cnt) {
> +		if (!feof(status)) {
> +			igt_info("ERROR: %d at fread\n", ferror(status));
> +			return false;
> +		}
> +		clearerr(status);
> +	}
> +	fclose(status);
> +	return true;
> +}
> +
> +/*
> + * is_drrs_supported :
> + *	Func to check for DRRS support
> + */
> +static bool is_drrs_supported(void)
> +{
> +	char str[DRRS_STATUS_BYTES_CNT] = {};
> +
> +	if (!read_drrs_status(str)) {
> +		igt_info("ERROR: Debugfs read FAILED\n");
> +		return false;
> +	}
> +	return strstr(str, "DRRS Supported: Yes") != NULL;
> +}
> +
> +/*
> + * is_drrs_enabled :
> + *	Func to check if DRRS is enabled by driver
> + */
> +static bool is_drrs_enabled(void)
> +{
> +	char str[DRRS_STATUS_BYTES_CNT] = {};
> +
> +	if (!read_drrs_status(str)) {
> +		igt_info("ERROR: Debugfs read FAILED\n");
> +		return false;
> +	}
> +	return strstr(str, "Idleness DRRS: Disabled") == NULL;
> +}
> +
> +/*
> + * prepare_crtc :
> + *	Func to prepare crtc for the given display
> + */
> +static bool prepare_crtc(data_t *data)
> +{
> +	if (data == NULL)
> +		return false;
> +	igt_display_t *display = &data->display;
> +	igt_output_t *output = data->output;
> +
> +	/* select the pipe we want to use */
> +	igt_output_set_pipe(output, data->pipe);
> +	igt_display_commit(display);
> +
> +	if (!output->valid) {
> +		igt_output_set_pipe(output, PIPE_ANY);
> +		igt_display_commit(display);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +/*
> + * calculate_refresh_rate :
> + *	Func to calculate the refresh rate by counting
> + *	vblanks in 1 sec. It returns the calculated refresh rate on success.
> + *	Returns -1 in case of error.
> + */
> +int calculate_refresh_rate(data_t *data)
> +{
> +	drmEventContext evctx;
> +	drmVBlank vbl;
> +	vbl_info handler_info;
> +	struct timeval start_time, curr_time;
> +	double time_elapsed;
> +	int refresh_rate = 0, vbl_count = 0;
> +	/*set up event context for handling vblank events*/
> +	memset(&evctx, 0, sizeof(evctx));
> +	evctx.version = DRM_EVENT_CONTEXT_VERSION;
> +	evctx.vblank_handler = vblank_handler;
> +	evctx.page_flip_handler = NULL;
> +
> +	/*initialize the vbl count to zero and set the starting time*/
> +	handler_info.vbl_count = 0;
> +	gettimeofday(&start_time, NULL);
> +	handler_info.start = start_time;
> +	curr_time = start_time;
> +
> +	time_elapsed = 0.0;
> +	/* To count vblanks in 1 sec. Loop till
> +	 * (current-time - starting time) <= 1.0 sec
> +	 */
> +	while (time_elapsed <= 1.0) {
> +		int ret;
> +
> +		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
> +		vbl.request.sequence = 1;
> +		vbl.request.signal = (unsigned long)&handler_info;
> +		ret = drmWaitVBlank(data->drm_fd, &vbl);
> +		if (ret != 0) {
> +			igt_info("ERROR : drmWaitVBlank FAILED :%d\n", ret);
> +			return -1;
> +		}
> +		/*call drmHandleEvent() to handle vblank event */
> +		ret = drmHandleEvent(data->drm_fd, &evctx);

Looks quite complicated. What you want is just

static double vbl_rate(const union drm_wait_vblank *start,
		       const union drm_wait_vblank *end)
{
	double s, e;

	s = start->reply.tval_sec + 1e-6*start->reply.tval_usec;
	e = end->reply.tval_sec + 1e-6*end->reply.tval_usec;

	return (end->reply.sequence - start->reply.sequnce) / (e - s);
}

static int calculate_refresh_rate(int fd, int pipe, int period_ms)
{
	union drm_wait_vblank start, end;
	struct timeval tv;

	memset(&start, 0, sizeof(start));
	start.request.type = DRM_VBLANK_RELATIVE | pipe_select(pipe);
	igt_assert(drmWaitVblank(fb, &start) == 0);

	while (igt_milliseconds_elapsed(&tv) < period_ms) {
		memset(&end, 0, sizeof(end));
		end.request.type = DRM_VBLANK_RELATIVE | pipe_select(pipe);
		end.sequence = 1;
		igt_assert(drmWaitVblank(fb, &end) == 0);
	}

	return vbl_rate(&start, &end);
}

right?

Call that igt_pipe_measure_vrefresh(int fd, enum pipe pipe, int min_period_ms);

or igt_crtc_measure_vrefresh(with igt_pipe_t instead of enum pipe).

and be done.

> +		if (ret != 0) {
> +			igt_info("ERROR : drmHandleEvent FAILED %d\n", ret);
> +			return -1;
> +		}
> +
> +		gettimeofday(&curr_time, NULL);
> +		time_elapsed = curr_time.tv_sec + curr_time.tv_usec * 1e-6 -
> +			(start_time.tv_sec + start_time.tv_usec * 1e-6);
> +	}
> +	if (!time_elapsed) {
> +		igt_info("ERROR: Incorrect measurement of vblank duration\n");
> +		return -1;
> +	}
> +	vbl_count = handler_info.vbl_count;
> +	/* calculate the refresh rate; rr=vblank_count/time_taken */
> +	refresh_rate = vbl_count / time_elapsed;
> +	return refresh_rate;
> +}
> +
> +/*
> + * worker_thread_func :
> + *	Func which is run by a worker thread to calculate the refresh_rate,
> + *	when signalled by the master thread.
> + */
> +void *worker_thread_func(void *ptr)
> +{
> +	data_t *data = (data_t *) ptr;
> +
> +	while (1) {
> +		int refresh_rate = 0;
> +		/*wait for signal from master*/
> +		pthread_mutex_lock(&rr_mutex);
> +		while (!rr_calc_requested)
> +			pthread_cond_wait(&cv_rr_calc_requested, &rr_mutex);
> +
> +		/* checkpoint for thread termination */
> +		if (thread_flag == RESET) {
> +			pthread_mutex_unlock(&rr_mutex);
> +			pthread_exit(NULL);
> +		}
> +		rr_calc_requested = RESET;
> +		pthread_mutex_unlock(&rr_mutex);
> +		/*calculate refresh_rate*/
> +		refresh_rate = calculate_refresh_rate(data);
> +
> +		/* signal the master */
> +		pthread_mutex_lock(&rr_mutex);
> +		refresh_rate_shared = refresh_rate;
> +		rr_calc_completed = SET;
> +		pthread_mutex_unlock(&rr_mutex);
> +		pthread_cond_signal(&cv_rr_calc_completed);
> +	}
> +	return NULL;
> +}

You are calculating the refresh rate synchronously, so what's the point
of the thread?

You are serialising on this thread

> +static int execute_test(data_t *data)
> +{

Split up this monolith into separate subtests and groups thereof.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] Idleness DRRS test
  2016-09-23 13:18       ` Chris Wilson
@ 2016-09-27 11:19         ` Nautiyal, Ankit K
  2016-10-21  3:52           ` [PATCH v3] " Nautiyal Ankit
  0 siblings, 1 reply; 12+ messages in thread
From: Nautiyal, Ankit K @ 2016-09-27 11:19 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx, daniel.vetter, rodrigo.vivi,
	paulo.r.zanoni, ramalingam.c, shashank.sharma

Hi Chris,

Thanks for the comments.

I will try out the changes you have mentioned for counting vblanks.

For other comments please find the responses inline.


On 9/23/2016 6:48 PM, Chris Wilson wrote:
> On Fri, Sep 23, 2016 at 06:10:29PM +0530, Nautiyal Ankit wrote:
>> From: Ramalingam C <ramalingam.c@intel.com>
>>
>> Idleness DRRS:
>> 	By default the DRRS state will be at DRRS_HIGH_RR. When a Display
>> 	content is Idle for more than 1Sec Idleness will be declared and
>> 	DRRS_LOW_RR will be invoked, changing the refresh rate to the
>> 	lower most refresh rate supported by the panel. As soon as there
>> 	is a display content change there will be a DRRS state transition
>> 	as DRRS_LOW_RR--> DRRS_HIGH_RR, changing the refresh rate to the
>> 	highest refresh rate supported by the panel.
>>
>> To test this, Idleness DRRS IGT will probe the DRRS state at below
>> instances and compare with the expected state.
>>
>> 	Instance					Expected State
>> 1. Immediately after rendering the still image		DRRS_HIGH_RR
>> 2. After a delay of 1.2Sec				DRRS_LOW_RR
>> 3. After changing the frame buffer			DRRS_HIGH_RR
>> 4. After a delay of 1.2Sec				DRRS_LOW_RR
>> 5. After changing the frame buffer			DRRS_HIGH_RR
>> 6. After a delay of 1.2Sec				DRRS_LOW_RR
>>
>> The test checks the driver DRRS state from the debugfs entry. To check the
>> actual refresh-rate, a separate thread counts the number of vblanks
>> received per sec. The refresh-rate calculated is checked against the
>> expected refresh-rate with a tolerance value of 2.
>>
>> This patch is a continuation of the earlier work
>> https://patchwork.freedesktop.org/patch/45472/ towards igt for idleness
>>
>> DRRS. The code is tested on Broxton BXT_T platform.
>>
>> v2: Addressed the comments and suggestions from Vlad, Marius.
>> The signoff details from the earlier work are also included.
>>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com>
>> ---
>>   tests/Makefile.sources |   1 +
>>   tests/kms_drrs.c       | 612 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 613 insertions(+)
>>   create mode 100644 tests/kms_drrs.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index a837977..5f31521 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -91,6 +91,7 @@ TESTS_progs_M = \
>>   	kms_cursor_crc \
>>   	kms_cursor_legacy \
>>   	kms_draw_crc \
>> +	kms_drrs \
>>   	kms_fbc_crc \
>>   	kms_fbcon_fbt \
>>   	kms_flip \
>> diff --git a/tests/kms_drrs.c b/tests/kms_drrs.c
>> new file mode 100644
>> index 0000000..69f8b06
>> --- /dev/null
>> +++ b/tests/kms_drrs.c
>> @@ -0,0 +1,612 @@
>> +/*
>> + * Copyright © 2016 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + */
>> +
>> +#include "drmtest.h"
>> +#include "igt_debugfs.h"
>> +#include "igt_kms.h"
>> +#include "intel_chipset.h"
>> +#include "intel_batchbuffer.h"
>> +#include "ioctl_wrappers.h"
>> +#include <time.h>
>> +#include <pthread.h>
>> +#include <stdlib.h>
>> +#include <sys/time.h>
>> +IGT_TEST_DESCRIPTION(
>> +"Performs write operations and then waits for DRRS to invoke the"
>> +"Low Refresh Rate and then disturbs the contents of the screen once"
>> +"again hence DRRS revert back to High Refresh Rate(Default).");
>> +
>> +#define DRRS_STATUS_BYTES_CNT	1000
>> +#define SET	1
>> +#define RESET	0
>> +
>> +/*
>> + * Structure to store data to create 2 framebuffers, fb[0] and fb[1] on a given
>> + * display. To disturb the content of the screen, we replace fb[0] by fb[1] and
>> + * vice versa.
>> + */
>> +typedef struct {
>> +	int drm_fd;
>> +	uint32_t devid;
>> +	uint32_t handle[2];
>> +	igt_display_t display;
>> +	igt_output_t *output;
>> +	enum pipe pipe;
>> +	igt_plane_t *primary;
>> +	struct igt_fb fb[2];
>> +	uint32_t fb_id[2];
>> +} data_t;
>> +
>> +/*
>> + * Structure to count vblank and note the starting time of the counter
>> + */
>> +typedef struct {
>> +	unsigned int vbl_count;
>> +	struct timeval start;
>> +} vbl_info;
>> +
>> +/*
>> + * Condition variables,mutex, and shared variables for thread synchronization
>> + */
>> +pthread_mutex_t rr_mutex;
>> +pthread_cond_t cv_rr_calc_requested;
>> +pthread_cond_t cv_rr_calc_completed;
>> +int rr_calc_requested = RESET;
>> +int rr_calc_completed = RESET;
>> +int thread_flag = SET;
>> +int refresh_rate_shared = -1;
>> +
>> +/*
>> + * Structure for refresh rate type
>> + */
>> +typedef struct{
>> +	int rate;
>> +	const char *name;
>> +} refresh_rate_t;
>> +
>> +/*
>> + * vblank_handler :
>> + *	User defined vblank event handler, to count vblanks.
>> + *	The control reaches this function after a vblank event is registered
>> + */
>> +static void vblank_handler(int fd, unsigned int frame,
>> +			unsigned int sec, unsigned int usec, void *data)
>> +{
>> +	/* The control reaches this function after a vblank event is
>> +	 * registered
>> +	 */
>> +	vbl_info *info = (vbl_info *) data;
>> +
>> +	if (info == NULL) {
>> +		igt_info("ERROR: Invalid Data passed to the vblank handler\n");
>> +		return;
>> +	}
>> +	info->vbl_count++;
>> +}
>> +
>> +
>> +/*
>> + * read_drrs_status :
>> + *	Func to read the DRRS status from debugfs
>> + */
>> +static bool read_drrs_status(char *str)
>> +{
>> +	FILE *status;
>> +	int cnt;
>> +
>> +	status = igt_debugfs_fopen("i915_drrs_status", "r");
>> +	igt_assert(status);
>> +
>> +	cnt = fread(str, DRRS_STATUS_BYTES_CNT - 1, 1, status);
>> +	if (!cnt) {
>> +		if (!feof(status)) {
>> +			igt_info("ERROR: %d at fread\n", ferror(status));
>> +			return false;
>> +		}
>> +		clearerr(status);
>> +	}
>> +	fclose(status);
>> +	return true;
>> +}
>> +
>> +/*
>> + * is_drrs_supported :
>> + *	Func to check for DRRS support
>> + */
>> +static bool is_drrs_supported(void)
>> +{
>> +	char str[DRRS_STATUS_BYTES_CNT] = {};
>> +
>> +	if (!read_drrs_status(str)) {
>> +		igt_info("ERROR: Debugfs read FAILED\n");
>> +		return false;
>> +	}
>> +	return strstr(str, "DRRS Supported: Yes") != NULL;
>> +}
>> +
>> +/*
>> + * is_drrs_enabled :
>> + *	Func to check if DRRS is enabled by driver
>> + */
>> +static bool is_drrs_enabled(void)
>> +{
>> +	char str[DRRS_STATUS_BYTES_CNT] = {};
>> +
>> +	if (!read_drrs_status(str)) {
>> +		igt_info("ERROR: Debugfs read FAILED\n");
>> +		return false;
>> +	}
>> +	return strstr(str, "Idleness DRRS: Disabled") == NULL;
>> +}
>> +
>> +/*
>> + * prepare_crtc :
>> + *	Func to prepare crtc for the given display
>> + */
>> +static bool prepare_crtc(data_t *data)
>> +{
>> +	if (data == NULL)
>> +		return false;
>> +	igt_display_t *display = &data->display;
>> +	igt_output_t *output = data->output;
>> +
>> +	/* select the pipe we want to use */
>> +	igt_output_set_pipe(output, data->pipe);
>> +	igt_display_commit(display);
>> +
>> +	if (!output->valid) {
>> +		igt_output_set_pipe(output, PIPE_ANY);
>> +		igt_display_commit(display);
>> +		return false;
>> +	}
>> +
>> +	return true;
>> +}
>> +
>> +/*
>> + * calculate_refresh_rate :
>> + *	Func to calculate the refresh rate by counting
>> + *	vblanks in 1 sec. It returns the calculated refresh rate on success.
>> + *	Returns -1 in case of error.
>> + */
>> +int calculate_refresh_rate(data_t *data)
>> +{
>> +	drmEventContext evctx;
>> +	drmVBlank vbl;
>> +	vbl_info handler_info;
>> +	struct timeval start_time, curr_time;
>> +	double time_elapsed;
>> +	int refresh_rate = 0, vbl_count = 0;
>> +	/*set up event context for handling vblank events*/
>> +	memset(&evctx, 0, sizeof(evctx));
>> +	evctx.version = DRM_EVENT_CONTEXT_VERSION;
>> +	evctx.vblank_handler = vblank_handler;
>> +	evctx.page_flip_handler = NULL;
>> +
>> +	/*initialize the vbl count to zero and set the starting time*/
>> +	handler_info.vbl_count = 0;
>> +	gettimeofday(&start_time, NULL);
>> +	handler_info.start = start_time;
>> +	curr_time = start_time;
>> +
>> +	time_elapsed = 0.0;
>> +	/* To count vblanks in 1 sec. Loop till
>> +	 * (current-time - starting time) <= 1.0 sec
>> +	 */
>> +	while (time_elapsed <= 1.0) {
>> +		int ret;
>> +
>> +		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
>> +		vbl.request.sequence = 1;
>> +		vbl.request.signal = (unsigned long)&handler_info;
>> +		ret = drmWaitVBlank(data->drm_fd, &vbl);
>> +		if (ret != 0) {
>> +			igt_info("ERROR : drmWaitVBlank FAILED :%d\n", ret);
>> +			return -1;
>> +		}
>> +		/*call drmHandleEvent() to handle vblank event */
>> +		ret = drmHandleEvent(data->drm_fd, &evctx);
> Looks quite complicated. What you want is just
>
> static double vbl_rate(const union drm_wait_vblank *start,
> 		       const union drm_wait_vblank *end)
> {
> 	double s, e;
>
> 	s = start->reply.tval_sec + 1e-6*start->reply.tval_usec;
> 	e = end->reply.tval_sec + 1e-6*end->reply.tval_usec;
>
> 	return (end->reply.sequence - start->reply.sequnce) / (e - s);
> }
>
> static int calculate_refresh_rate(int fd, int pipe, int period_ms)
> {
> 	union drm_wait_vblank start, end;
> 	struct timeval tv;
>
> 	memset(&start, 0, sizeof(start));
> 	start.request.type = DRM_VBLANK_RELATIVE | pipe_select(pipe);
> 	igt_assert(drmWaitVblank(fb, &start) == 0);
>
> 	while (igt_milliseconds_elapsed(&tv) < period_ms) {
> 		memset(&end, 0, sizeof(end));
> 		end.request.type = DRM_VBLANK_RELATIVE | pipe_select(pipe);
> 		end.sequence = 1;
> 		igt_assert(drmWaitVblank(fb, &end) == 0);
> 	}
>
> 	return vbl_rate(&start, &end);
> }
>
> right?
>
> Call that igt_pipe_measure_vrefresh(int fd, enum pipe pipe, int min_period_ms);
>
> or igt_crtc_measure_vrefresh(with igt_pipe_t instead of enum pipe).
>
> and be done.

I will try this out and get back to you.

>> +		if (ret != 0) {
>> +			igt_info("ERROR : drmHandleEvent FAILED %d\n", ret);
>> +			return -1;
>> +		}
>> +
>> +		gettimeofday(&curr_time, NULL);
>> +		time_elapsed = curr_time.tv_sec + curr_time.tv_usec * 1e-6 -
>> +			(start_time.tv_sec + start_time.tv_usec * 1e-6);
>> +	}
>> +	if (!time_elapsed) {
>> +		igt_info("ERROR: Incorrect measurement of vblank duration\n");
>> +		return -1;
>> +	}
>> +	vbl_count = handler_info.vbl_count;
>> +	/* calculate the refresh rate; rr=vblank_count/time_taken */
>> +	refresh_rate = vbl_count / time_elapsed;
>> +	return refresh_rate;
>> +}
>> +
>> +/*
>> + * worker_thread_func :
>> + *	Func which is run by a worker thread to calculate the refresh_rate,
>> + *	when signalled by the master thread.
>> + */
>> +void *worker_thread_func(void *ptr)
>> +{
>> +	data_t *data = (data_t *) ptr;
>> +
>> +	while (1) {
>> +		int refresh_rate = 0;
>> +		/*wait for signal from master*/
>> +		pthread_mutex_lock(&rr_mutex);
>> +		while (!rr_calc_requested)
>> +			pthread_cond_wait(&cv_rr_calc_requested, &rr_mutex);
>> +
>> +		/* checkpoint for thread termination */
>> +		if (thread_flag == RESET) {
>> +			pthread_mutex_unlock(&rr_mutex);
>> +			pthread_exit(NULL);
>> +		}
>> +		rr_calc_requested = RESET;
>> +		pthread_mutex_unlock(&rr_mutex);
>> +		/*calculate refresh_rate*/
>> +		refresh_rate = calculate_refresh_rate(data);
>> +
>> +		/* signal the master */
>> +		pthread_mutex_lock(&rr_mutex);
>> +		refresh_rate_shared = refresh_rate;
>> +		rr_calc_completed = SET;
>> +		pthread_mutex_unlock(&rr_mutex);
>> +		pthread_cond_signal(&cv_rr_calc_completed);
>> +	}
>> +	return NULL;
>> +}
> You are calculating the refresh rate synchronously, so what's the point
> of the thread?
>
> You are serialising on this thread

The DRRS status is verified in 2 ways.

One is through debugfs entry which checks the driver state [DRRS_HIGH / DRRS_LOW].

the other is by counting vblanks/sec for approximate refresh-rate.

As soon as we do a flip, the state is at DRRS_HIGH status, and panel displays at higher refresh rate.

The state goes to DRRS_LOW after waiting for 1.2 sec.

In this window of 1.2sec the main thread checks the state through debugfs, while the worker thread

counts the vblanks and calculates the approximate refresh rate in parallel.

The threads are synchronized so that master (after determining the driver state from debugfs),

waits for the worker thread to complete the refresh rate calculation, before proceeding with the test.

>> +static int execute_test(data_t *data)
>> +{
> Split up this monolith into separate subtests and groups thereof.

There are no sub-tests here, as we are flipping and checking the transition from high to low and low to high refresh rate.
Flip [1.2 sec ](HIGH->LOW); Flip (LOW->HIGH); Flip [1.2 sec] (HIGH->LOW). This constitutes a single test.

> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3] Idleness DRRS test
  2016-09-27 11:19         ` Nautiyal, Ankit K
@ 2016-10-21  3:52           ` Nautiyal Ankit
  2016-11-14 12:44             ` Petri Latvala
  0 siblings, 1 reply; 12+ messages in thread
From: Nautiyal Ankit @ 2016-10-21  3:52 UTC (permalink / raw)
  To: intel-gfx, daniel.vetter, chris, rodrigo.vivi; +Cc: paulo.r.zanoni

From: Ramalingam C <ramalingam.c@intel.com>

Idleness DRRS:
	By default the DRRS state will be at DRRS_HIGH_RR. When a Display
	content is Idle for more than 1Sec Idleness will be declared and
	DRRS_LOW_RR will be invoked, changing the refresh rate to the
	lower most refresh rate supported by the panel. As soon as there
	is a display content change there will be a DRRS state transition
	as DRRS_LOW_RR--> DRRS_HIGH_RR, changing the refresh rate to the
	highest refresh rate supported by the panel.

To test this, Idleness DRRS IGT will probe the DRRS state at below
instances and compare with the expected state.

	Instance					Expected State
1. Immediately after rendering the still image		DRRS_HIGH_RR
2. After a delay of 1.2Sec				DRRS_LOW_RR
3. After changing the frame buffer			DRRS_HIGH_RR
4. After a delay of 1.2Sec				DRRS_LOW_RR
5. After changing the frame buffer			DRRS_HIGH_RR
6. After a delay of 1.2Sec				DRRS_LOW_RR

The test checks the driver DRRS state from the debugfs entry. To check the
actual refresh-rate, a separate thread counts the number of vblanks
received per sec. The refresh-rate calculated is checked against the
expected refresh-rate with a tolerance value of 2.

This patch is a continuation of the earlier work
https://patchwork.freedesktop.org/patch/45472/ towards igt for idleness

DRRS. The code is tested on Broxton BXT_T platform.

v2: Addressed the comments and suggestions from Vlad, Marius.
The signoff details from the earlier work are also included.

v3: Modified vblank rate calculation by using reply-sequence, provided by
drmWaitVBlank, as suggested by Chris Wilson.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com>
---
 tests/Makefile.sources |   1 +
 tests/kms_drrs.c       | 599 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 600 insertions(+)
 create mode 100644 tests/kms_drrs.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index a837977..5f31521 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -91,6 +91,7 @@ TESTS_progs_M = \
 	kms_cursor_crc \
 	kms_cursor_legacy \
 	kms_draw_crc \
+	kms_drrs \
 	kms_fbc_crc \
 	kms_fbcon_fbt \
 	kms_flip \
diff --git a/tests/kms_drrs.c b/tests/kms_drrs.c
new file mode 100644
index 0000000..bd5a135
--- /dev/null
+++ b/tests/kms_drrs.c
@@ -0,0 +1,599 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "drmtest.h"
+#include "igt_debugfs.h"
+#include "igt_kms.h"
+#include "intel_chipset.h"
+#include "intel_batchbuffer.h"
+#include "ioctl_wrappers.h"
+#include <time.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <sys/time.h>
+IGT_TEST_DESCRIPTION(
+"Performs write operations and then waits for DRRS to invoke the"
+"Low Refresh Rate and then disturbs the contents of the screen once"
+"again hence DRRS revert back to High Refresh Rate(Default).");
+
+#define DRRS_STATUS_BYTES_CNT	1000
+#define SET	1
+#define RESET	0
+
+/*
+ * Structure to store data to create 2 framebuffers, fb[0] and fb[1] on a given
+ * display. To disturb the content of the screen, we replace fb[0] by fb[1] and
+ * vice versa.
+ */
+typedef struct {
+	int drm_fd;
+	uint32_t devid;
+	uint32_t handle[2];
+	igt_display_t display;
+	igt_output_t *output;
+	enum pipe pipe;
+	igt_plane_t *primary;
+	struct igt_fb fb[2];
+	uint32_t fb_id[2];
+} data_t;
+
+/*
+ * Structure to count vblank and note the starting time of the counter
+ */
+typedef struct {
+	unsigned int vbl_count;
+	struct timeval start;
+} vbl_info;
+
+/*
+ * Condition variables,mutex, and shared variables for thread synchronization
+ */
+pthread_mutex_t rr_mutex;
+pthread_cond_t cv_rr_calc_requested;
+pthread_cond_t cv_rr_calc_completed;
+int rr_calc_requested = RESET;
+int rr_calc_completed = RESET;
+int thread_flag = SET;
+int refresh_rate_shared = -1;
+
+/*
+ * Structure for refresh rate type
+ */
+typedef struct{
+	int rate;
+	const char *name;
+} refresh_rate_t;
+
+static inline uint32_t pipe_select(int pipe)
+{
+	if (pipe > 1)
+		return pipe << DRM_VBLANK_HIGH_CRTC_SHIFT;
+	else if (pipe > 0)
+		return DRM_VBLANK_SECONDARY;
+	else
+		return 0;
+}
+
+/*
+ * igt_millisec_elapsed:
+ *	Calculates the no. of millisec elapsed since timeval start
+ */
+static double igt_millisec_elapsed(const struct timeval *start)
+{
+	struct timeval curr;
+	double res;
+
+	gettimeofday(&curr, NULL);
+	return (1e6*(curr.tv_sec - start->tv_sec) +
+				(curr.tv_usec - start->tv_usec)/1000.00);
+}
+
+/*
+ * read_drrs_status :
+ *	Func to read the DRRS status from debugfs
+ */
+static bool read_drrs_status(char *str)
+{
+	FILE *status;
+	int cnt;
+
+	status = igt_debugfs_fopen("i915_drrs_status", "r");
+	igt_assert(status);
+
+	cnt = fread(str, DRRS_STATUS_BYTES_CNT - 1, 1, status);
+	if (!cnt) {
+		if (!feof(status)) {
+			igt_info("ERROR: %d at fread\n", ferror(status));
+			return false;
+		}
+		clearerr(status);
+	}
+	fclose(status);
+	return true;
+}
+
+/*
+ * is_drrs_supported :
+ *	Func to check for DRRS support
+ */
+static bool is_drrs_supported(void)
+{
+	char str[DRRS_STATUS_BYTES_CNT] = {};
+
+	if (!read_drrs_status(str)) {
+		igt_info("ERROR: Debugfs read FAILED\n");
+		return false;
+	}
+	return strstr(str, "DRRS Supported: Yes") != NULL;
+}
+
+/*
+ * is_drrs_enabled :
+ *	Func to check if DRRS is enabled by driver
+ */
+static bool is_drrs_enabled(void)
+{
+	char str[DRRS_STATUS_BYTES_CNT] = {};
+
+	if (!read_drrs_status(str)) {
+		igt_info("ERROR: Debugfs read FAILED\n");
+		return false;
+	}
+	return strstr(str, "Idleness DRRS: Disabled") == NULL;
+}
+
+/*
+ * prepare_crtc :
+ *	Func to prepare crtc for the given display
+ */
+static bool prepare_crtc(data_t *data)
+{
+	if (data == NULL)
+		return false;
+
+	igt_display_t *display = &data->display;
+	igt_output_t *output = data->output;
+
+	/* select the pipe we want to use */
+	igt_output_set_pipe(output, data->pipe);
+	igt_display_commit(display);
+
+	if (!output->valid) {
+		igt_output_set_pipe(output, PIPE_ANY);
+		igt_display_commit(display);
+		return false;
+	}
+
+	return true;
+}
+/*
+ * vbl_rate :
+ *	calculates the vblank rate by counting vblanks between start and end
+ *	period, divided by time between start and end.
+ */
+static double vbl_rate(const union drm_wait_vblank *start,
+				const union drm_wait_vblank *end) {
+	double s, e;
+
+	s = start->reply.tval_sec + 1e-6 * start->reply.tval_usec;
+	e = end->reply.tval_sec + 1e-6 * end->reply.tval_usec;
+	return ((end->reply.sequence - start->reply.sequence) / (e - s));
+}
+
+/*
+ * calculate_refresh_rate :
+ *	Func to calculate the refresh rate by counting
+ *	vblanks in 1 sec (1000 ms). It returns the calculated refresh rate on
+ *	success. Returns -1 in case of error.
+ */
+int calculate_refresh_rate(data_t *data)
+{
+#define PERIOD_MS 1000
+	union drm_wait_vblank start, end;
+	struct timeval start_tv;
+
+	memset(&start, 0, sizeof(start));
+	start.request.type = DRM_VBLANK_RELATIVE | pipe_select(data->pipe);
+	if (drmWaitVBlank(data->drm_fd, &start) != 0) {
+		igt_info("ERROR : drmWaitVBlank Failed!\n");
+		return -1;
+	}
+	gettimeofday(&start_tv, NULL);
+	while (igt_millisec_elapsed(&start_tv) < PERIOD_MS) {
+		memset(&end, 0, sizeof(end));
+		end.request.type = DRM_VBLANK_RELATIVE |
+						pipe_select(data->pipe);
+		end.request.sequence = 1;
+		if (drmWaitVBlank(data->drm_fd, &end) != 0) {
+			igt_info("ERROR : drmWaitVBlank Failed!\n");
+			return -1;
+		}
+	}
+	return vbl_rate(&start, &end);
+}
+
+/*
+ * worker_thread_func :
+ *	Func which is run by a worker thread to calculate the refresh_rate,
+ *	when signalled by the master thread.
+ */
+void *worker_thread_func(void *ptr)
+{
+	data_t *data = (data_t *) ptr;
+
+	while (1) {
+		int refresh_rate = 0;
+		/*wait for signal from master*/
+		pthread_mutex_lock(&rr_mutex);
+		while (!rr_calc_requested)
+			pthread_cond_wait(&cv_rr_calc_requested, &rr_mutex);
+
+		/* checkpoint for thread termination */
+		if (thread_flag == RESET) {
+			pthread_mutex_unlock(&rr_mutex);
+			pthread_exit(NULL);
+		}
+		rr_calc_requested = RESET;
+		pthread_mutex_unlock(&rr_mutex);
+		/*calculate refresh_rate*/
+		refresh_rate = calculate_refresh_rate(data);
+
+		/* signal the master */
+		pthread_mutex_lock(&rr_mutex);
+		refresh_rate_shared = refresh_rate;
+		rr_calc_completed = SET;
+		pthread_mutex_unlock(&rr_mutex);
+		pthread_cond_signal(&cv_rr_calc_completed);
+	}
+	return NULL;
+}
+
+/*
+ * check_refresh_rate :
+ *	Func to check the refresh rate calculated, against the expected refresh
+ *	rate. It signals the worker thread to calculate the refresh-
+ *	rate, and checks if the correct refresh rate is switched.
+ *	Returns 0 in case of pass , 1 in case of failure, and -1 for error.
+ */
+
+static int check_refresh_rate(data_t *data, refresh_rate_t *expected_rr)
+{
+/*
+ * The calculated refresh-rate is an approximation to estimated refresh-rate.
+ * The tolerance threshold for the difference is set to 2. Difference more than
+ * 2 is considered as failure for the check_refresh_rate.
+ */
+#define TOLERANCE_THRESHOLD		2
+	int refresh_rate = -1;
+	char drrs_status[DRRS_STATUS_BYTES_CNT] = {};
+
+	/* Signal worker thread to calculate refresh rate */
+	pthread_mutex_lock(&rr_mutex);
+	rr_calc_requested = SET;
+	pthread_mutex_unlock(&rr_mutex);
+	pthread_cond_signal(&cv_rr_calc_requested);
+
+	/* Get the DRRS Status from the debugfs entry */
+	if (!read_drrs_status(drrs_status)) {
+		igt_info("ERROR : DRRS debugfs read FAILED\n");
+		return -1;
+	}
+	/* Wait for the refresh rate calculator thread */
+	pthread_mutex_lock(&rr_mutex);
+	while (!rr_calc_completed)
+		pthread_cond_wait(&cv_rr_calc_completed, &rr_mutex);
+	rr_calc_completed = RESET;
+	refresh_rate = refresh_rate_shared;
+	pthread_mutex_unlock(&rr_mutex);
+	if (refresh_rate < 0) {
+		igt_info("ERROR : Refresh rate calculation FAILED\n");
+		return -1;
+	}
+	igt_info("INFO : Actual Refresh_rate based on Vblank Counts/sec = %d\n"
+							, refresh_rate);
+
+	/* Compare with Expected Refresh Rate*/
+	igt_info("INFO: Expected Refresh-Rate %d\n", expected_rr->rate);
+	if (strstr(drrs_status, expected_rr->name) != NULL
+	&& abs(refresh_rate - expected_rr->rate) <= TOLERANCE_THRESHOLD) {
+		igt_info("INFO: Expected %s : PASSED\n", expected_rr->name);
+		return 0;
+	}
+
+	igt_info("INFO : Expected %s : FAILED.\n", expected_rr->name);
+	return 1;
+}
+
+/*
+ * clean_up :
+ *	Func to stop the worker thread and destroy thread syncronization
+ *	structures.
+ */
+void clean_up(pthread_t rr_thread)
+{
+	/* Stop the worker thread */
+	pthread_mutex_lock(&rr_mutex);
+	thread_flag = RESET;
+	rr_calc_requested = SET;
+	pthread_mutex_unlock(&rr_mutex);
+	pthread_cond_signal(&cv_rr_calc_requested);
+	pthread_join(rr_thread, NULL);
+
+	/* destroy the condition variables and mutex */
+	pthread_cond_destroy(&cv_rr_calc_requested);
+	pthread_cond_destroy(&cv_rr_calc_completed);
+	pthread_mutex_destroy(&rr_mutex);
+}
+
+/*
+ * execute_test:
+ *	This function
+ *	1. Initializes the thread sync structures.
+ *	2. Creates a worker thread which can be signaled to estimate
+ *	the refresh rates based on counting vblanks per sec.
+ *	3. Creates two different Framebuffer fb0, fb1
+ *	and apply them in the interval of 1.2Sec. So between FB change
+ *	DRRS should toggle from DRRS_HIGH_RR -> DRRS_LOW_RR.
+ *	And on next FB change DRRS_LOW_RR -> DRRS_HIGH_RR.
+ *
+ *	These expected transitions are verified using the kernel debugfs
+ *	and the refresh-rate, calculated by counting vblanks per sec by the
+ *	worker thread.
+ */
+static int execute_test(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output = data->output;
+	drmModeModeInfo *mode, *supported_modes;
+	int test_failed = 0;
+	int ret, i, count_modes;
+	refresh_rate_t high_rr, low_rr;
+	pthread_t rr_thread;
+
+	/*initialize the condition variables and  mutex*/
+	pthread_cond_init(&cv_rr_calc_requested, NULL);
+	pthread_cond_init(&cv_rr_calc_completed, NULL);
+	pthread_mutex_init(&rr_mutex, NULL);
+	thread_flag = SET;
+	rr_calc_requested = RESET;
+	rr_calc_completed = RESET;
+	/* create a thread for calculating the refresh rate */
+	ret = pthread_create(&rr_thread, NULL, worker_thread_func,
+							(void *) data);
+	if (ret == -1) {
+		igt_info("ERROR : Refresh rate thread creation FAILED\n");
+		clean_up(rr_thread);
+		return -1;
+	}
+
+	high_rr.name = "DRRS_HIGH_RR";
+	high_rr.rate = 0;
+	low_rr.name = "DRRS_LOW_RR";
+	low_rr.rate = 0;
+	/* get the max and min supported refresh rates for the display */
+	supported_modes = output->config.connector->modes;
+	count_modes = output->config.connector->count_modes;
+	/* minimum 2 modes are required for DRRS */
+	if (count_modes < 2) {
+		igt_info("ERROR: Minimum 2 modes required for DRRS\n");
+		return -1;
+	}
+	for (i = 0; i < count_modes; i++) {
+		int rr = supported_modes[i].vrefresh;
+
+		if (i == 0) {
+			high_rr.rate = low_rr.rate = rr;
+			continue;
+		}
+		if (high_rr.rate < rr)
+			high_rr.rate = rr;
+		if (low_rr.rate > rr)
+			low_rr.rate = rr;
+	}
+	data->primary = igt_output_get_plane(data->output, IGT_PLANE_PRIMARY);
+	mode = igt_output_get_mode(data->output);
+	data->fb_id[0] = igt_create_color_fb(data->drm_fd, mode->hdisplay,
+			mode->vdisplay,
+			DRM_FORMAT_XRGB8888,
+			LOCAL_DRM_FORMAT_MOD_NONE,
+			0.0, 100.1, 0.0, &data->fb[0]);
+	igt_assert(data->fb_id[0]);
+	data->fb_id[1] = igt_create_color_fb(data->drm_fd, mode->hdisplay,
+			mode->vdisplay,
+			DRM_FORMAT_XRGB8888,
+			LOCAL_DRM_FORMAT_MOD_NONE,
+			100.1, 0.0, 0.0,
+			&data->fb[1]);
+	igt_assert(data->fb_id[1]);
+
+	data->handle[0] = data->fb[0].gem_handle;
+	data->handle[1] = data->fb[1].gem_handle;
+
+	/* scanout = fb[1] */
+	igt_plane_set_fb(data->primary, &data->fb[1]);
+	igt_display_commit(display);
+	if (!is_drrs_enabled()) {
+		igt_info("INFO : DRRS not enabled\n");
+		igt_plane_set_fb(data->primary, NULL);
+		igt_output_set_pipe(output, PIPE_ANY);
+		igt_display_commit(display);
+		igt_remove_fb(data->drm_fd, &data->fb[0]);
+		igt_remove_fb(data->drm_fd, &data->fb[1]);
+		clean_up(rr_thread);
+		return -1;
+	}
+
+	/* expecting High RR */
+	ret =  check_refresh_rate(data, &high_rr);
+	if (ret == -1)
+		return -1;
+	if (ret == 1)
+		test_failed = 1;
+	/* 1.2Sec to Enter the Idleness and check for Low Refresh Rate */
+	usleep(1200000);
+	ret =  check_refresh_rate(data, &low_rr);
+	if (ret == -1)
+		return -1;
+	if (ret == 1)
+		test_failed = 1;
+
+	/* scanout = fb[0] and check for High Refresh Rate*/
+	igt_plane_set_fb(data->primary, &data->fb[0]);
+	igt_display_commit(display);
+	ret =  check_refresh_rate(data, &high_rr);
+	if (ret == -1)
+		return -1;
+	if (ret == 1)
+		test_failed = 1;
+
+	/* 1.2Sec to Enter the Idleness and check for Low Refresh Rate */
+	usleep(1200000);
+	ret =  check_refresh_rate(data, &low_rr);
+	if (ret == -1)
+		return -1;
+	if (ret == 1)
+		test_failed = 1;
+
+	/* scanout = fb[1] and check for High Refresh Rate*/
+	igt_plane_set_fb(data->primary, &data->fb[1]);
+	igt_display_commit(display);
+	ret =  check_refresh_rate(data, &high_rr);
+	if (ret == -1)
+		return -1;
+	if (ret == 1)
+		test_failed = 1;
+
+	/* 1.2Sec to Enter the Idleness */
+	usleep(1200000);
+	ret =  check_refresh_rate(data, &low_rr);
+	if (ret == -1)
+		return -1;
+	if (ret == 1)
+		test_failed = 1;
+
+	clean_up(rr_thread);
+	return test_failed;
+}
+
+/*
+ * finish_crtc :
+ *	Func to free the framebuffers after the test completion.
+ */
+static void finish_crtc(data_t *data)
+{
+	igt_plane_set_fb(data->primary, NULL);
+	igt_output_set_pipe(data->output, PIPE_ANY);
+	igt_display_commit(&data->display);
+
+	igt_remove_fb(data->drm_fd, &data->fb[0]);
+	igt_remove_fb(data->drm_fd, &data->fb[1]);
+}
+
+/*
+ * reset_display :
+ *	Func to reset the display structures after the test completion.
+ */
+static void reset_display(data_t *data)
+{
+	igt_display_t *display = &data->display;
+
+	for_each_connected_output(display, data->output) {
+		if (data->output->valid) {
+			data->primary =  igt_output_get_plane(data->output,
+							IGT_PLANE_PRIMARY);
+			igt_plane_set_fb(data->primary, NULL);
+		}
+		igt_output_set_pipe(data->output, PIPE_ANY);
+	}
+}
+/*
+ * run_test :
+ *	Func to run the test for the eDP display.
+ */
+static void run_test(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	int ret;
+
+	reset_display(data);
+	/* Check for eDP Connector on PIPE A ,as eDP goes normally on PIPE_A */
+	for_each_connected_output(display, data->output) {
+		drmModeConnectorPtr c = data->output->config.connector;
+
+		if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
+			c->connection != DRM_MODE_CONNECTED)
+			continue;
+		data->pipe = PIPE_A;
+		igt_assert_f(prepare_crtc(data), "Failed to prepare CRTC\n");
+		igt_info("INFO : Beginning %s on pipe %s,connector %s\n",
+					igt_test_name(),
+					kmstest_pipe_name(data->pipe),
+					igt_output_name(data->output));
+
+		ret = execute_test(data);
+		igt_skip_on_f(ret == -1,
+				"INFO :%s on pipe %s, connector %s:SKIPPED\n",
+					igt_test_name(),
+					kmstest_pipe_name(data->pipe),
+					igt_output_name(data->output));
+		igt_fail_on_f(ret == 1,
+				"INFO :%s on pipe %s, connector %s:FAILED\n",
+					igt_test_name(),
+					kmstest_pipe_name(data->pipe),
+					igt_output_name(data->output));
+
+		igt_assert_f(ret == 0,
+				"INFO :%s on pipe %s, connector %s: PASSED\n",
+					igt_test_name(),
+					kmstest_pipe_name(data->pipe),
+					igt_output_name(data->output));
+
+		finish_crtc(data);
+	}
+}
+
+igt_simple_main
+{
+	data_t data = {};
+
+	igt_skip_on_simulation();
+	data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+	if (data.drm_fd == -1) {
+		igt_info("ERROR : invalid fd\n");
+		return;
+	}
+	data.devid = intel_get_drm_devid(data.drm_fd);
+	if (data.devid < 0) {
+		igt_info("ERROR : invalid dev id\n");
+		return;
+	}
+	/* Check if the DRRS is supported.
+	 * If yes call the Idleness DRRS test
+	 */
+	igt_require_f(is_drrs_supported(),
+			"DRRS not supported:check VBT/panel caps\n");
+	igt_display_init(&data.display, data.drm_fd);
+	run_test(&data);
+	igt_display_fini(&data.display);
+}
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] Idleness DRRS test
  2016-10-21  3:52           ` [PATCH v3] " Nautiyal Ankit
@ 2016-11-14 12:44             ` Petri Latvala
  2016-11-14 13:06               ` Chris Wilson
  0 siblings, 1 reply; 12+ messages in thread
From: Petri Latvala @ 2016-11-14 12:44 UTC (permalink / raw)
  To: chris
  Cc: daniel.vetter, intel-gfx, Nautiyal Ankit, paulo.r.zanoni, rodrigo.vivi

Chris, happy with this revision?


On Fri, Oct 21, 2016 at 09:22:13AM +0530, Nautiyal Ankit wrote:
> From: Ramalingam C <ramalingam.c@intel.com>
> 
> Idleness DRRS:
> 	By default the DRRS state will be at DRRS_HIGH_RR. When a Display
> 	content is Idle for more than 1Sec Idleness will be declared and
> 	DRRS_LOW_RR will be invoked, changing the refresh rate to the
> 	lower most refresh rate supported by the panel. As soon as there
> 	is a display content change there will be a DRRS state transition
> 	as DRRS_LOW_RR--> DRRS_HIGH_RR, changing the refresh rate to the
> 	highest refresh rate supported by the panel.
> 
> To test this, Idleness DRRS IGT will probe the DRRS state at below
> instances and compare with the expected state.
> 
> 	Instance					Expected State
> 1. Immediately after rendering the still image		DRRS_HIGH_RR
> 2. After a delay of 1.2Sec				DRRS_LOW_RR
> 3. After changing the frame buffer			DRRS_HIGH_RR
> 4. After a delay of 1.2Sec				DRRS_LOW_RR
> 5. After changing the frame buffer			DRRS_HIGH_RR
> 6. After a delay of 1.2Sec				DRRS_LOW_RR
> 
> The test checks the driver DRRS state from the debugfs entry. To check the
> actual refresh-rate, a separate thread counts the number of vblanks
> received per sec. The refresh-rate calculated is checked against the
> expected refresh-rate with a tolerance value of 2.
> 
> This patch is a continuation of the earlier work
> https://patchwork.freedesktop.org/patch/45472/ towards igt for idleness
> 
> DRRS. The code is tested on Broxton BXT_T platform.
> 
> v2: Addressed the comments and suggestions from Vlad, Marius.
> The signoff details from the earlier work are also included.
> 
> v3: Modified vblank rate calculation by using reply-sequence, provided by
> drmWaitVBlank, as suggested by Chris Wilson.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com>
> ---
>  tests/Makefile.sources |   1 +
>  tests/kms_drrs.c       | 599 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 600 insertions(+)
>  create mode 100644 tests/kms_drrs.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index a837977..5f31521 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -91,6 +91,7 @@ TESTS_progs_M = \
>  	kms_cursor_crc \
>  	kms_cursor_legacy \
>  	kms_draw_crc \
> +	kms_drrs \
>  	kms_fbc_crc \
>  	kms_fbcon_fbt \
>  	kms_flip \
> diff --git a/tests/kms_drrs.c b/tests/kms_drrs.c
> new file mode 100644
> index 0000000..bd5a135
> --- /dev/null
> +++ b/tests/kms_drrs.c
> @@ -0,0 +1,599 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "drmtest.h"
> +#include "igt_debugfs.h"
> +#include "igt_kms.h"
> +#include "intel_chipset.h"
> +#include "intel_batchbuffer.h"
> +#include "ioctl_wrappers.h"
> +#include <time.h>
> +#include <pthread.h>
> +#include <stdlib.h>
> +#include <sys/time.h>
> +IGT_TEST_DESCRIPTION(
> +"Performs write operations and then waits for DRRS to invoke the"
> +"Low Refresh Rate and then disturbs the contents of the screen once"
> +"again hence DRRS revert back to High Refresh Rate(Default).");
> +
> +#define DRRS_STATUS_BYTES_CNT	1000
> +#define SET	1
> +#define RESET	0
> +
> +/*
> + * Structure to store data to create 2 framebuffers, fb[0] and fb[1] on a given
> + * display. To disturb the content of the screen, we replace fb[0] by fb[1] and
> + * vice versa.
> + */
> +typedef struct {
> +	int drm_fd;
> +	uint32_t devid;
> +	uint32_t handle[2];
> +	igt_display_t display;
> +	igt_output_t *output;
> +	enum pipe pipe;
> +	igt_plane_t *primary;
> +	struct igt_fb fb[2];
> +	uint32_t fb_id[2];
> +} data_t;
> +
> +/*
> + * Structure to count vblank and note the starting time of the counter
> + */
> +typedef struct {
> +	unsigned int vbl_count;
> +	struct timeval start;
> +} vbl_info;
> +
> +/*
> + * Condition variables,mutex, and shared variables for thread synchronization
> + */
> +pthread_mutex_t rr_mutex;
> +pthread_cond_t cv_rr_calc_requested;
> +pthread_cond_t cv_rr_calc_completed;
> +int rr_calc_requested = RESET;
> +int rr_calc_completed = RESET;
> +int thread_flag = SET;
> +int refresh_rate_shared = -1;
> +
> +/*
> + * Structure for refresh rate type
> + */
> +typedef struct{
> +	int rate;
> +	const char *name;
> +} refresh_rate_t;
> +
> +static inline uint32_t pipe_select(int pipe)
> +{
> +	if (pipe > 1)
> +		return pipe << DRM_VBLANK_HIGH_CRTC_SHIFT;
> +	else if (pipe > 0)
> +		return DRM_VBLANK_SECONDARY;
> +	else
> +		return 0;
> +}
> +
> +/*
> + * igt_millisec_elapsed:
> + *	Calculates the no. of millisec elapsed since timeval start
> + */
> +static double igt_millisec_elapsed(const struct timeval *start)
> +{
> +	struct timeval curr;
> +	double res;
> +
> +	gettimeofday(&curr, NULL);
> +	return (1e6*(curr.tv_sec - start->tv_sec) +
> +				(curr.tv_usec - start->tv_usec)/1000.00);
> +}
> +
> +/*
> + * read_drrs_status :
> + *	Func to read the DRRS status from debugfs
> + */
> +static bool read_drrs_status(char *str)
> +{
> +	FILE *status;
> +	int cnt;
> +
> +	status = igt_debugfs_fopen("i915_drrs_status", "r");
> +	igt_assert(status);
> +
> +	cnt = fread(str, DRRS_STATUS_BYTES_CNT - 1, 1, status);
> +	if (!cnt) {
> +		if (!feof(status)) {
> +			igt_info("ERROR: %d at fread\n", ferror(status));
> +			return false;
> +		}
> +		clearerr(status);
> +	}
> +	fclose(status);
> +	return true;
> +}
> +
> +/*
> + * is_drrs_supported :
> + *	Func to check for DRRS support
> + */
> +static bool is_drrs_supported(void)
> +{
> +	char str[DRRS_STATUS_BYTES_CNT] = {};
> +
> +	if (!read_drrs_status(str)) {
> +		igt_info("ERROR: Debugfs read FAILED\n");
> +		return false;
> +	}
> +	return strstr(str, "DRRS Supported: Yes") != NULL;
> +}
> +
> +/*
> + * is_drrs_enabled :
> + *	Func to check if DRRS is enabled by driver
> + */
> +static bool is_drrs_enabled(void)
> +{
> +	char str[DRRS_STATUS_BYTES_CNT] = {};
> +
> +	if (!read_drrs_status(str)) {
> +		igt_info("ERROR: Debugfs read FAILED\n");
> +		return false;
> +	}
> +	return strstr(str, "Idleness DRRS: Disabled") == NULL;
> +}
> +
> +/*
> + * prepare_crtc :
> + *	Func to prepare crtc for the given display
> + */
> +static bool prepare_crtc(data_t *data)
> +{
> +	if (data == NULL)
> +		return false;
> +
> +	igt_display_t *display = &data->display;
> +	igt_output_t *output = data->output;
> +
> +	/* select the pipe we want to use */
> +	igt_output_set_pipe(output, data->pipe);
> +	igt_display_commit(display);
> +
> +	if (!output->valid) {
> +		igt_output_set_pipe(output, PIPE_ANY);
> +		igt_display_commit(display);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +/*
> + * vbl_rate :
> + *	calculates the vblank rate by counting vblanks between start and end
> + *	period, divided by time between start and end.
> + */
> +static double vbl_rate(const union drm_wait_vblank *start,
> +				const union drm_wait_vblank *end) {
> +	double s, e;
> +
> +	s = start->reply.tval_sec + 1e-6 * start->reply.tval_usec;
> +	e = end->reply.tval_sec + 1e-6 * end->reply.tval_usec;
> +	return ((end->reply.sequence - start->reply.sequence) / (e - s));
> +}
> +
> +/*
> + * calculate_refresh_rate :
> + *	Func to calculate the refresh rate by counting
> + *	vblanks in 1 sec (1000 ms). It returns the calculated refresh rate on
> + *	success. Returns -1 in case of error.
> + */
> +int calculate_refresh_rate(data_t *data)
> +{
> +#define PERIOD_MS 1000
> +	union drm_wait_vblank start, end;
> +	struct timeval start_tv;
> +
> +	memset(&start, 0, sizeof(start));
> +	start.request.type = DRM_VBLANK_RELATIVE | pipe_select(data->pipe);
> +	if (drmWaitVBlank(data->drm_fd, &start) != 0) {
> +		igt_info("ERROR : drmWaitVBlank Failed!\n");
> +		return -1;
> +	}
> +	gettimeofday(&start_tv, NULL);
> +	while (igt_millisec_elapsed(&start_tv) < PERIOD_MS) {
> +		memset(&end, 0, sizeof(end));
> +		end.request.type = DRM_VBLANK_RELATIVE |
> +						pipe_select(data->pipe);
> +		end.request.sequence = 1;
> +		if (drmWaitVBlank(data->drm_fd, &end) != 0) {
> +			igt_info("ERROR : drmWaitVBlank Failed!\n");
> +			return -1;
> +		}
> +	}
> +	return vbl_rate(&start, &end);
> +}
> +
> +/*
> + * worker_thread_func :
> + *	Func which is run by a worker thread to calculate the refresh_rate,
> + *	when signalled by the master thread.
> + */
> +void *worker_thread_func(void *ptr)
> +{
> +	data_t *data = (data_t *) ptr;
> +
> +	while (1) {
> +		int refresh_rate = 0;
> +		/*wait for signal from master*/
> +		pthread_mutex_lock(&rr_mutex);
> +		while (!rr_calc_requested)
> +			pthread_cond_wait(&cv_rr_calc_requested, &rr_mutex);
> +
> +		/* checkpoint for thread termination */
> +		if (thread_flag == RESET) {
> +			pthread_mutex_unlock(&rr_mutex);
> +			pthread_exit(NULL);
> +		}
> +		rr_calc_requested = RESET;
> +		pthread_mutex_unlock(&rr_mutex);
> +		/*calculate refresh_rate*/
> +		refresh_rate = calculate_refresh_rate(data);
> +
> +		/* signal the master */
> +		pthread_mutex_lock(&rr_mutex);
> +		refresh_rate_shared = refresh_rate;
> +		rr_calc_completed = SET;
> +		pthread_mutex_unlock(&rr_mutex);
> +		pthread_cond_signal(&cv_rr_calc_completed);
> +	}
> +	return NULL;
> +}
> +
> +/*
> + * check_refresh_rate :
> + *	Func to check the refresh rate calculated, against the expected refresh
> + *	rate. It signals the worker thread to calculate the refresh-
> + *	rate, and checks if the correct refresh rate is switched.
> + *	Returns 0 in case of pass , 1 in case of failure, and -1 for error.
> + */
> +
> +static int check_refresh_rate(data_t *data, refresh_rate_t *expected_rr)
> +{
> +/*
> + * The calculated refresh-rate is an approximation to estimated refresh-rate.
> + * The tolerance threshold for the difference is set to 2. Difference more than
> + * 2 is considered as failure for the check_refresh_rate.
> + */
> +#define TOLERANCE_THRESHOLD		2
> +	int refresh_rate = -1;
> +	char drrs_status[DRRS_STATUS_BYTES_CNT] = {};
> +
> +	/* Signal worker thread to calculate refresh rate */
> +	pthread_mutex_lock(&rr_mutex);
> +	rr_calc_requested = SET;
> +	pthread_mutex_unlock(&rr_mutex);
> +	pthread_cond_signal(&cv_rr_calc_requested);
> +
> +	/* Get the DRRS Status from the debugfs entry */
> +	if (!read_drrs_status(drrs_status)) {
> +		igt_info("ERROR : DRRS debugfs read FAILED\n");
> +		return -1;
> +	}
> +	/* Wait for the refresh rate calculator thread */
> +	pthread_mutex_lock(&rr_mutex);
> +	while (!rr_calc_completed)
> +		pthread_cond_wait(&cv_rr_calc_completed, &rr_mutex);
> +	rr_calc_completed = RESET;
> +	refresh_rate = refresh_rate_shared;
> +	pthread_mutex_unlock(&rr_mutex);
> +	if (refresh_rate < 0) {
> +		igt_info("ERROR : Refresh rate calculation FAILED\n");
> +		return -1;
> +	}
> +	igt_info("INFO : Actual Refresh_rate based on Vblank Counts/sec = %d\n"
> +							, refresh_rate);
> +
> +	/* Compare with Expected Refresh Rate*/
> +	igt_info("INFO: Expected Refresh-Rate %d\n", expected_rr->rate);
> +	if (strstr(drrs_status, expected_rr->name) != NULL
> +	&& abs(refresh_rate - expected_rr->rate) <= TOLERANCE_THRESHOLD) {
> +		igt_info("INFO: Expected %s : PASSED\n", expected_rr->name);
> +		return 0;
> +	}
> +
> +	igt_info("INFO : Expected %s : FAILED.\n", expected_rr->name);
> +	return 1;
> +}
> +
> +/*
> + * clean_up :
> + *	Func to stop the worker thread and destroy thread syncronization
> + *	structures.
> + */
> +void clean_up(pthread_t rr_thread)
> +{
> +	/* Stop the worker thread */
> +	pthread_mutex_lock(&rr_mutex);
> +	thread_flag = RESET;
> +	rr_calc_requested = SET;
> +	pthread_mutex_unlock(&rr_mutex);
> +	pthread_cond_signal(&cv_rr_calc_requested);
> +	pthread_join(rr_thread, NULL);
> +
> +	/* destroy the condition variables and mutex */
> +	pthread_cond_destroy(&cv_rr_calc_requested);
> +	pthread_cond_destroy(&cv_rr_calc_completed);
> +	pthread_mutex_destroy(&rr_mutex);
> +}
> +
> +/*
> + * execute_test:
> + *	This function
> + *	1. Initializes the thread sync structures.
> + *	2. Creates a worker thread which can be signaled to estimate
> + *	the refresh rates based on counting vblanks per sec.
> + *	3. Creates two different Framebuffer fb0, fb1
> + *	and apply them in the interval of 1.2Sec. So between FB change
> + *	DRRS should toggle from DRRS_HIGH_RR -> DRRS_LOW_RR.
> + *	And on next FB change DRRS_LOW_RR -> DRRS_HIGH_RR.
> + *
> + *	These expected transitions are verified using the kernel debugfs
> + *	and the refresh-rate, calculated by counting vblanks per sec by the
> + *	worker thread.
> + */
> +static int execute_test(data_t *data)
> +{
> +	igt_display_t *display = &data->display;
> +	igt_output_t *output = data->output;
> +	drmModeModeInfo *mode, *supported_modes;
> +	int test_failed = 0;
> +	int ret, i, count_modes;
> +	refresh_rate_t high_rr, low_rr;
> +	pthread_t rr_thread;
> +
> +	/*initialize the condition variables and  mutex*/
> +	pthread_cond_init(&cv_rr_calc_requested, NULL);
> +	pthread_cond_init(&cv_rr_calc_completed, NULL);
> +	pthread_mutex_init(&rr_mutex, NULL);
> +	thread_flag = SET;
> +	rr_calc_requested = RESET;
> +	rr_calc_completed = RESET;
> +	/* create a thread for calculating the refresh rate */
> +	ret = pthread_create(&rr_thread, NULL, worker_thread_func,
> +							(void *) data);
> +	if (ret == -1) {
> +		igt_info("ERROR : Refresh rate thread creation FAILED\n");
> +		clean_up(rr_thread);
> +		return -1;
> +	}
> +
> +	high_rr.name = "DRRS_HIGH_RR";
> +	high_rr.rate = 0;
> +	low_rr.name = "DRRS_LOW_RR";
> +	low_rr.rate = 0;
> +	/* get the max and min supported refresh rates for the display */
> +	supported_modes = output->config.connector->modes;
> +	count_modes = output->config.connector->count_modes;
> +	/* minimum 2 modes are required for DRRS */
> +	if (count_modes < 2) {
> +		igt_info("ERROR: Minimum 2 modes required for DRRS\n");
> +		return -1;
> +	}
> +	for (i = 0; i < count_modes; i++) {
> +		int rr = supported_modes[i].vrefresh;
> +
> +		if (i == 0) {
> +			high_rr.rate = low_rr.rate = rr;
> +			continue;
> +		}
> +		if (high_rr.rate < rr)
> +			high_rr.rate = rr;
> +		if (low_rr.rate > rr)
> +			low_rr.rate = rr;
> +	}
> +	data->primary = igt_output_get_plane(data->output, IGT_PLANE_PRIMARY);
> +	mode = igt_output_get_mode(data->output);
> +	data->fb_id[0] = igt_create_color_fb(data->drm_fd, mode->hdisplay,
> +			mode->vdisplay,
> +			DRM_FORMAT_XRGB8888,
> +			LOCAL_DRM_FORMAT_MOD_NONE,
> +			0.0, 100.1, 0.0, &data->fb[0]);
> +	igt_assert(data->fb_id[0]);
> +	data->fb_id[1] = igt_create_color_fb(data->drm_fd, mode->hdisplay,
> +			mode->vdisplay,
> +			DRM_FORMAT_XRGB8888,
> +			LOCAL_DRM_FORMAT_MOD_NONE,
> +			100.1, 0.0, 0.0,
> +			&data->fb[1]);
> +	igt_assert(data->fb_id[1]);
> +
> +	data->handle[0] = data->fb[0].gem_handle;
> +	data->handle[1] = data->fb[1].gem_handle;
> +
> +	/* scanout = fb[1] */
> +	igt_plane_set_fb(data->primary, &data->fb[1]);
> +	igt_display_commit(display);
> +	if (!is_drrs_enabled()) {
> +		igt_info("INFO : DRRS not enabled\n");
> +		igt_plane_set_fb(data->primary, NULL);
> +		igt_output_set_pipe(output, PIPE_ANY);
> +		igt_display_commit(display);
> +		igt_remove_fb(data->drm_fd, &data->fb[0]);
> +		igt_remove_fb(data->drm_fd, &data->fb[1]);
> +		clean_up(rr_thread);
> +		return -1;
> +	}
> +
> +	/* expecting High RR */
> +	ret =  check_refresh_rate(data, &high_rr);
> +	if (ret == -1)
> +		return -1;
> +	if (ret == 1)
> +		test_failed = 1;
> +	/* 1.2Sec to Enter the Idleness and check for Low Refresh Rate */
> +	usleep(1200000);
> +	ret =  check_refresh_rate(data, &low_rr);
> +	if (ret == -1)
> +		return -1;
> +	if (ret == 1)
> +		test_failed = 1;
> +
> +	/* scanout = fb[0] and check for High Refresh Rate*/
> +	igt_plane_set_fb(data->primary, &data->fb[0]);
> +	igt_display_commit(display);
> +	ret =  check_refresh_rate(data, &high_rr);
> +	if (ret == -1)
> +		return -1;
> +	if (ret == 1)
> +		test_failed = 1;
> +
> +	/* 1.2Sec to Enter the Idleness and check for Low Refresh Rate */
> +	usleep(1200000);
> +	ret =  check_refresh_rate(data, &low_rr);
> +	if (ret == -1)
> +		return -1;
> +	if (ret == 1)
> +		test_failed = 1;
> +
> +	/* scanout = fb[1] and check for High Refresh Rate*/
> +	igt_plane_set_fb(data->primary, &data->fb[1]);
> +	igt_display_commit(display);
> +	ret =  check_refresh_rate(data, &high_rr);
> +	if (ret == -1)
> +		return -1;
> +	if (ret == 1)
> +		test_failed = 1;
> +
> +	/* 1.2Sec to Enter the Idleness */
> +	usleep(1200000);
> +	ret =  check_refresh_rate(data, &low_rr);
> +	if (ret == -1)
> +		return -1;
> +	if (ret == 1)
> +		test_failed = 1;
> +
> +	clean_up(rr_thread);
> +	return test_failed;
> +}
> +
> +/*
> + * finish_crtc :
> + *	Func to free the framebuffers after the test completion.
> + */
> +static void finish_crtc(data_t *data)
> +{
> +	igt_plane_set_fb(data->primary, NULL);
> +	igt_output_set_pipe(data->output, PIPE_ANY);
> +	igt_display_commit(&data->display);
> +
> +	igt_remove_fb(data->drm_fd, &data->fb[0]);
> +	igt_remove_fb(data->drm_fd, &data->fb[1]);
> +}
> +
> +/*
> + * reset_display :
> + *	Func to reset the display structures after the test completion.
> + */
> +static void reset_display(data_t *data)
> +{
> +	igt_display_t *display = &data->display;
> +
> +	for_each_connected_output(display, data->output) {
> +		if (data->output->valid) {
> +			data->primary =  igt_output_get_plane(data->output,
> +							IGT_PLANE_PRIMARY);
> +			igt_plane_set_fb(data->primary, NULL);
> +		}
> +		igt_output_set_pipe(data->output, PIPE_ANY);
> +	}
> +}
> +/*
> + * run_test :
> + *	Func to run the test for the eDP display.
> + */
> +static void run_test(data_t *data)
> +{
> +	igt_display_t *display = &data->display;
> +	int ret;
> +
> +	reset_display(data);
> +	/* Check for eDP Connector on PIPE A ,as eDP goes normally on PIPE_A */
> +	for_each_connected_output(display, data->output) {
> +		drmModeConnectorPtr c = data->output->config.connector;
> +
> +		if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
> +			c->connection != DRM_MODE_CONNECTED)
> +			continue;
> +		data->pipe = PIPE_A;
> +		igt_assert_f(prepare_crtc(data), "Failed to prepare CRTC\n");
> +		igt_info("INFO : Beginning %s on pipe %s,connector %s\n",
> +					igt_test_name(),
> +					kmstest_pipe_name(data->pipe),
> +					igt_output_name(data->output));
> +
> +		ret = execute_test(data);
> +		igt_skip_on_f(ret == -1,
> +				"INFO :%s on pipe %s, connector %s:SKIPPED\n",
> +					igt_test_name(),
> +					kmstest_pipe_name(data->pipe),
> +					igt_output_name(data->output));
> +		igt_fail_on_f(ret == 1,
> +				"INFO :%s on pipe %s, connector %s:FAILED\n",
> +					igt_test_name(),
> +					kmstest_pipe_name(data->pipe),
> +					igt_output_name(data->output));
> +
> +		igt_assert_f(ret == 0,
> +				"INFO :%s on pipe %s, connector %s: PASSED\n",
> +					igt_test_name(),
> +					kmstest_pipe_name(data->pipe),
> +					igt_output_name(data->output));
> +
> +		finish_crtc(data);
> +	}
> +}
> +
> +igt_simple_main
> +{
> +	data_t data = {};
> +
> +	igt_skip_on_simulation();
> +	data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> +	if (data.drm_fd == -1) {
> +		igt_info("ERROR : invalid fd\n");
> +		return;
> +	}
> +	data.devid = intel_get_drm_devid(data.drm_fd);
> +	if (data.devid < 0) {
> +		igt_info("ERROR : invalid dev id\n");
> +		return;
> +	}
> +	/* Check if the DRRS is supported.
> +	 * If yes call the Idleness DRRS test
> +	 */
> +	igt_require_f(is_drrs_supported(),
> +			"DRRS not supported:check VBT/panel caps\n");
> +	igt_display_init(&data.display, data.drm_fd);
> +	run_test(&data);
> +	igt_display_fini(&data.display);
> +}
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] Idleness DRRS test
  2016-11-14 12:44             ` Petri Latvala
@ 2016-11-14 13:06               ` Chris Wilson
  2016-11-15  8:58                 ` Daniel Vetter
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2016-11-14 13:06 UTC (permalink / raw)
  To: Petri Latvala
  Cc: daniel.vetter, intel-gfx, Nautiyal Ankit, paulo.r.zanoni, rodrigo.vivi

On Mon, Nov 14, 2016 at 02:44:35PM +0200, Petri Latvala wrote:
> Chris, happy with this revision?

Me? No. It still uses a thread instead of events, so I don't think it
qualifies as a good example for anyone else wanting to do the same thing.
Lots of hardcoded expectations (specific sleep patterns, rather than
waiting for the kernel to change with a timeout for failure). It doesn't
check all the possible ways that the output maybe changed (and if they
are irrelevent, that too also needs to be tested to establish expected
behaviour and catch future regressions). Important question, how is
userspace expected to know that the vrefresh changed? How do get around
that userspace is expecting a fixed vblank frequency?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] Idleness DRRS test
  2016-11-14 13:06               ` Chris Wilson
@ 2016-11-15  8:58                 ` Daniel Vetter
  2016-11-29  9:36                   ` Nautiyal, Ankit K
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2016-11-15  8:58 UTC (permalink / raw)
  To: Chris Wilson, Petri Latvala, intel-gfx, daniel.vetter,
	rodrigo.vivi, paulo.r.zanoni, Nautiyal Ankit

On Mon, Nov 14, 2016 at 01:06:09PM +0000, Chris Wilson wrote:
> On Mon, Nov 14, 2016 at 02:44:35PM +0200, Petri Latvala wrote:
> > Chris, happy with this revision?
> 
> Me? No. It still uses a thread instead of events, so I don't think it
> qualifies as a good example for anyone else wanting to do the same thing.
> Lots of hardcoded expectations (specific sleep patterns, rather than
> waiting for the kernel to change with a timeout for failure). It doesn't
> check all the possible ways that the output maybe changed (and if they
> are irrelevent, that too also needs to be tested to establish expected
> behaviour and catch future regressions). Important question, how is
> userspace expected to know that the vrefresh changed? How do get around
> that userspace is expecting a fixed vblank frequency?

for display power testing we already have the kms_frontbuffer_tracking
testcase, adding a DRRS variants to that one should resolve all this.

And then kms_drrs would be empty, except when we (if ever) do explicit
drrs through modeset requests. And that would just be checking that the
observed timings match the requested timings.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] Idleness DRRS test
  2016-11-15  8:58                 ` Daniel Vetter
@ 2016-11-29  9:36                   ` Nautiyal, Ankit K
  2016-11-30  9:22                     ` Daniel Vetter
  0 siblings, 1 reply; 12+ messages in thread
From: Nautiyal, Ankit K @ 2016-11-29  9:36 UTC (permalink / raw)
  To: Daniel Vetter, Chris Wilson, Petri Latvala, intel-gfx,
	daniel.vetter, rodrigo.vivi, paulo.r.zanoni

As per discussion with Chris, on IRC following were the suggestions :

- Chris has suggested an event based approach to avoid using pthreads.

- Avoid using kernel-specific info like transitional delays and instead 
use either polling or wait-for-event approach. Need to focus on 
user-observable behavior rather than the kernel standpoint.

- Check for transition latency is unnecessary in this test, as this is 
more of a performance/power benchmark.


I will try the event based mechanism to do away with pthreads, and 
incorporate these suggestions in the next patch-set.

-Ankit


On 11/15/2016 2:28 PM, Daniel Vetter wrote:
> On Mon, Nov 14, 2016 at 01:06:09PM +0000, Chris Wilson wrote:
>> On Mon, Nov 14, 2016 at 02:44:35PM +0200, Petri Latvala wrote:
>>> Chris, happy with this revision?
>> Me? No. It still uses a thread instead of events, so I don't think it
>> qualifies as a good example for anyone else wanting to do the same thing.
>> Lots of hardcoded expectations (specific sleep patterns, rather than
>> waiting for the kernel to change with a timeout for failure). It doesn't
>> check all the possible ways that the output maybe changed (and if they
>> are irrelevent, that too also needs to be tested to establish expected
>> behaviour and catch future regressions). Important question, how is
>> userspace expected to know that the vrefresh changed? How do get around
>> that userspace is expecting a fixed vblank frequency?
> for display power testing we already have the kms_frontbuffer_tracking
> testcase, adding a DRRS variants to that one should resolve all this.
>
> And then kms_drrs would be empty, except when we (if ever) do explicit
> drrs through modeset requests. And that would just be checking that the
> observed timings match the requested timings.
> -Daniel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] Idleness DRRS test
  2016-11-29  9:36                   ` Nautiyal, Ankit K
@ 2016-11-30  9:22                     ` Daniel Vetter
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2016-11-30  9:22 UTC (permalink / raw)
  To: Nautiyal, Ankit K; +Cc: daniel.vetter, intel-gfx, rodrigo.vivi, paulo.r.zanoni

On Tue, Nov 29, 2016 at 03:06:17PM +0530, Nautiyal, Ankit K wrote:
> As per discussion with Chris, on IRC following were the suggestions :
> 
> - Chris has suggested an event based approach to avoid using pthreads.
> 
> - Avoid using kernel-specific info like transitional delays and instead use
> either polling or wait-for-event approach. Need to focus on user-observable
> behavior rather than the kernel standpoint.
> 
> - Check for transition latency is unnecessary in this test, as this is more
> of a performance/power benchmark.
> 
> 
> I will try the event based mechanism to do away with pthreads, and
> incorporate these suggestions in the next patch-set.

Again, why is kms_frontbuffer_tracking not considered? Reinventing wheels
is not good, and kms_frontbuffer_tracking implements all of the above
stuff afaik.
-Daniel

> 
> -Ankit
> 
> 
> On 11/15/2016 2:28 PM, Daniel Vetter wrote:
> > On Mon, Nov 14, 2016 at 01:06:09PM +0000, Chris Wilson wrote:
> > > On Mon, Nov 14, 2016 at 02:44:35PM +0200, Petri Latvala wrote:
> > > > Chris, happy with this revision?
> > > Me? No. It still uses a thread instead of events, so I don't think it
> > > qualifies as a good example for anyone else wanting to do the same thing.
> > > Lots of hardcoded expectations (specific sleep patterns, rather than
> > > waiting for the kernel to change with a timeout for failure). It doesn't
> > > check all the possible ways that the output maybe changed (and if they
> > > are irrelevent, that too also needs to be tested to establish expected
> > > behaviour and catch future regressions). Important question, how is
> > > userspace expected to know that the vrefresh changed? How do get around
> > > that userspace is expecting a fixed vblank frequency?
> > for display power testing we already have the kms_frontbuffer_tracking
> > testcase, adding a DRRS variants to that one should resolve all this.
> > 
> > And then kms_drrs would be empty, except when we (if ever) do explicit
> > drrs through modeset requests. And that would just be checking that the
> > observed timings match the requested timings.
> > -Daniel
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-11-30  9:22 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-31  8:16 [RFC] Idleness DRRS test Nautiyal Ankit
2016-09-07 14:53 ` Marius Vlad
2016-09-23 12:07   ` Nautiyal, Ankit K
2016-09-23 12:40     ` [PATCH v2] " Nautiyal Ankit
2016-09-23 13:18       ` Chris Wilson
2016-09-27 11:19         ` Nautiyal, Ankit K
2016-10-21  3:52           ` [PATCH v3] " Nautiyal Ankit
2016-11-14 12:44             ` Petri Latvala
2016-11-14 13:06               ` Chris Wilson
2016-11-15  8:58                 ` Daniel Vetter
2016-11-29  9:36                   ` Nautiyal, Ankit K
2016-11-30  9:22                     ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.