[Intel-gfx] [PATCH i-g-t v12] tests/kms_frontbuffer_tracking: Including DRRS test coverage

Ramalingam C ramalingam.c at intel.com
Sat Jan 6 10:48:02 UTC 2018



On Friday 05 January 2018 11:25 PM, Rodrigo Vivi wrote:
> On Fri, Jan 05, 2018 at 11:40:29AM +0000, Lohith BS wrote:
>> Dynamic Refresh Rate Switch(DRRS) is used to switch the panel's
>> refresh rate to the lowest vrefresh supported by panel, when frame is
>> not flipped for more than a Sec.
>>
>> In kernel, DRRS uses the front buffer tracking infrastructure.
>> Hence DRRS test coverage is added along with other frontbuffer tracking
>> based features such as FBC and PSR tests.
>>
>> Here, we are testing DRRS with other features in all possible
>> combinations, in all required test cases, to capture any possible
>> regression.
>>
>> 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.
>>
>> v4: As suggested from Chris Wilson and Daniel Vetter
>>      1) Avoided using pthread for calculating vblank refresh rate,
>>         instead used drmWaitVBlank reply sequence.
>>      2) Avoided using kernel-specific info like transitional delays,
>>         instead polling mechanism with timeout is used.
>>      3) Included edp-DRRS as a subtest in kms_frontbuffer_tracking.c,
>>         instead of having a separate test.
>>
>> v5: This patch adds DRRS as a new feature in the
>>      kms_frontbuffer_tracking IGT.
>>      DRRS switch to lower vrefresh rate is tested at slow-draw subtest.
>>
>>      Note:
>>      1) Currently kernel doesn't have support to enable and disable
>>         the DRRS feature dynamically(as in case of PSR). Hence if the
>>         panel supports DRRS it will be enabled by default.
>>
>>      This is in continuation of last patch
>> 	"https://patchwork.freedesktop.org/patch/162726/"
>>
>> v6: This patch adds runtime enable and disable feature for testing DRRS
>>
>> v7: This patch adds runtime enable and disable feature for testing DRRS
>>      through debugfs entry "i915_drrs_ctl".
>>
>> v8: Commit message is updated to reflect current implementation.
>>
>> v9: Addressed Paulo Zanoni comments.
>>      Check for DRRS_LOW at tests with OFFSCREEN + FBS_INDIVIDUAL.
>>
>> v10: Corrected DRRS state expectation in suspend related subtests.
>>
>> v11: Removing the global flag is_psr_drrs_combo [Rodrigo].
>>
>> v12: Rewriting the DRRS inactive deduction [Rodrigo].
>>
>> Signed-off-by: Lohith BS <lohith.bs at intel.com>
>> Signed-off-by: aknautiy <ankit.k.nautiyal at intel.com>
> I was almost adding the rv-b here, but I will step back while CI is not
> fully happy with the patch:
>
> https://patchwork.freedesktop.org/series/32888/
>
> regressed many sub-tests on snb and hsw...
Rodrigo, please share the logs for regressions. We will analyze it.

Thanks
-Ram
>
>> ---
>>   tests/kms_frontbuffer_tracking.c | 178 +++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 170 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
>> index 1601cab..4b87273 100644
>> --- a/tests/kms_frontbuffer_tracking.c
>> +++ b/tests/kms_frontbuffer_tracking.c
>> @@ -34,7 +34,7 @@
>>   
>>   
>>   IGT_TEST_DESCRIPTION("Test the Kernel's frontbuffer tracking mechanism and "
>> -		     "its related features: FBC and PSR");
>> +		     "its related features: FBC, PSR and DRRS");
>>   
>>   /*
>>    * One of the aspects of this test is that, for every subtest, we try different
>> @@ -105,8 +105,9 @@ struct test_mode {
>>   		FEATURE_NONE  = 0,
>>   		FEATURE_FBC   = 1,
>>   		FEATURE_PSR   = 2,
>> -		FEATURE_COUNT = 4,
>> -		FEATURE_DEFAULT = 4,
>> +		FEATURE_DRRS  = 4,
>> +		FEATURE_COUNT = 8,
>> +		FEATURE_DEFAULT = 8,
>>   	} feature;
>>   
>>   	/* Possible pixel formats. We just use FORMAT_DEFAULT for most tests and
>> @@ -156,6 +157,7 @@ struct rect {
>>   struct {
>>   	int fd;
>>   	int debugfs;
>> +	int drrs_debugfs_fd;
>>   	drmModeResPtr res;
>>   	drmModeConnectorPtr connectors[MAX_CONNECTORS];
>>   	drmModeEncoderPtr encoders[MAX_ENCODERS];
>> @@ -182,6 +184,13 @@ struct {
>>   	.can_test = false,
>>   };
>>   
>> +#define MAX_DRRS_STATUS_BUF_LEN 256
>> +
>> +struct {
>> +	bool can_test;
>> +} drrs = {
>> +	.can_test = false,
>> +};
>>   
>>   #define SINK_CRC_SIZE 12
>>   typedef struct {
>> @@ -825,6 +834,60 @@ static void psr_print_status(void)
>>   	igt_info("PSR status:\n%s\n", buf);
>>   }
>>   
>> +void drrs_set(unsigned int val)
>> +{
>> +	char buf[2];
>> +
>> +	igt_debug("Manually %sabling DRRS. %llu\n", val ? "en" : "dis", val);
>> +	snprintf(buf, sizeof(buf), "%d", val);
>> +	igt_assert_eq(write(drm.drrs_debugfs_fd,
>> +		      buf, strlen(buf)), strlen(buf));
>> +}
>> +
>> +static bool is_drrs_high(void)
>> +{
>> +	char buf[MAX_DRRS_STATUS_BUF_LEN];
>> +
>> +	debugfs_read("i915_drrs_status", buf);
>> +	return strstr(buf, "DRRS_HIGH_RR");
>> +}
>> +
>> +static bool is_drrs_low(void)
>> +{
>> +	char buf[MAX_DRRS_STATUS_BUF_LEN];
>> +
>> +	debugfs_read("i915_drrs_status", buf);
>> +	return strstr(buf, "DRRS_LOW_RR");
>> +}
>> +
>> +static bool is_drrs_supported(void)
>> +{
>> +	char buf[MAX_DRRS_STATUS_BUF_LEN];
>> +
>> +	debugfs_read("i915_drrs_status", buf);
>> +	return strstr(buf, "DRRS Supported: Yes");
>> +}
>> +
>> +static bool is_drrs_inactive(void)
>> +{
>> +	char buf[MAX_DRRS_STATUS_BUF_LEN];
>> +
>> +	debugfs_read("i915_drrs_status", buf);
>> +
>> +	if (strstr(buf, "DRRS_State: "))
>> +		return false;
>> +
>> +	return true;
>> +}
>> +
>> +static void drrs_print_status(void)
>> +{
>> +	char buf[MAX_DRRS_STATUS_BUF_LEN];
>> +
>> +	debugfs_read("i915_drrs_status", buf);
>> +	igt_info("DRRS STATUS :\n%s\n", buf);
>> +}
>> +
>>   static struct timespec fbc_get_last_action(void)
>>   {
>>   	struct timespec ret = { 0, 0 };
>> @@ -935,10 +998,17 @@ static bool psr_wait_until_enabled(void)
>>   	return igt_wait(psr_is_enabled(), 5000, 1);
>>   }
>>   
>> +static bool drrs_wait_until_rr_switch_to_low(void)
>> +{
>> +	return igt_wait(is_drrs_low(), 5000, 1);
>> +}
>> +
>>   #define fbc_enable() igt_set_module_param_int("enable_fbc", 1)
>>   #define fbc_disable() igt_set_module_param_int("enable_fbc", 0)
>>   #define psr_enable() igt_set_module_param_int("enable_psr", 1)
>>   #define psr_disable() igt_set_module_param_int("enable_psr", 0)
>> +#define drrs_enable()	drrs_set(1)
>> +#define drrs_disable()	drrs_set(0)
>>   
>>   static void get_sink_crc(sink_crc_t *crc, bool mandatory)
>>   {
>> @@ -1184,6 +1254,7 @@ static void disable_features(const struct test_mode *t)
>>   
>>   	fbc_disable();
>>   	psr_disable();
>> +	drrs_disable();
>>   }
>>   
>>   static void *busy_thread_func(void *data)
>> @@ -1508,6 +1579,12 @@ static void teardown_crcs(void)
>>   	igt_pipe_crc_free(pipe_crc);
>>   }
>>   
>> +static void teardown_drrs(void)
>> +{
>> +	if (drm.drrs_debugfs_fd != -1)
>> +		close(drm.drrs_debugfs_fd);
>> +}
>> +
>>   static bool fbc_supported_on_chipset(void)
>>   {
>>   	char buf[128];
>> @@ -1577,6 +1654,29 @@ static void teardown_psr(void)
>>   {
>>   }
>>   
>> +static void setup_drrs(void)
>> +{
>> +	if (get_connector(prim_mode_params.connector_id)->connector_type !=
>> +	    DRM_MODE_CONNECTOR_eDP) {
>> +		igt_info("Can't test DRRS: no usable eDP screen.\n");
>> +		return;
>> +	}
>> +
>> +	if (!is_drrs_supported()) {
>> +		igt_info("Can't test DRRS: Not supported.\n");
>> +		return;
>> +	}
>> +
>> +	drm.drrs_debugfs_fd = openat(drm.debugfs, "i915_drrs_ctl", O_WRONLY);
>> +
>> +	if (drm.drrs_debugfs_fd > 0) {
>> +		drrs.can_test = true;
>> +	} else {
>> +		igt_info("Can't test DRRS: Debugfs entry i915_drrs_ctl not found.\n");
>> +		drrs.can_test = false;
>> +	}
>> +}
>> +
>>   static void setup_environment(void)
>>   {
>>   	setup_drm();
>> @@ -1584,6 +1684,7 @@ static void setup_environment(void)
>>   
>>   	setup_fbc();
>>   	setup_psr();
>> +	setup_drrs();
>>   
>>   	setup_crcs();
>>   }
>> @@ -1596,6 +1697,7 @@ static void teardown_environment(void)
>>   	teardown_psr();
>>   	teardown_fbc();
>>   	teardown_modeset();
>> +	teardown_drrs();
>>   	teardown_drm();
>>   }
>>   
>> @@ -1662,6 +1764,11 @@ static void do_flush(const struct test_mode *t)
>>   #define ASSERT_PSR_ENABLED		(1 << 6)
>>   #define ASSERT_PSR_DISABLED		(1 << 7)
>>   
>> +#define DRRS_ASSERT_FLAGS		(7 << 8)
>> +#define ASSERT_DRRS_HIGH		(1 << 8)
>> +#define ASSERT_DRRS_LOW		(1 << 9)
>> +#define ASSERT_DRRS_INACTIVE		(1 << 10)
>> +
>>   static int adjust_assertion_flags(const struct test_mode *t, int flags)
>>   {
>>   	if (!(flags & DONT_ASSERT_FEATURE_STATUS)) {
>> @@ -1669,12 +1776,17 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
>>   			flags |= ASSERT_FBC_ENABLED;
>>   		if (!(flags & ASSERT_PSR_DISABLED))
>>   			flags |= ASSERT_PSR_ENABLED;
>> +		if (!((flags & ASSERT_DRRS_LOW) ||
>> +		    (flags & ASSERT_DRRS_INACTIVE)))
>> +			flags |= ASSERT_DRRS_HIGH;
>>   	}
>>   
>>   	if ((t->feature & FEATURE_FBC) == 0)
>>   		flags &= ~FBC_ASSERT_FLAGS;
>>   	if ((t->feature & FEATURE_PSR) == 0)
>>   		flags &= ~PSR_ASSERT_FLAGS;
>> +	if ((t->feature & FEATURE_DRRS) == 0)
>> +		flags &= ~DRRS_ASSERT_FLAGS;
>>   
>>   	return flags;
>>   }
>> @@ -1706,6 +1818,23 @@ static void do_status_assertions(int flags)
>>   		return;
>>   	}
>>   
>> +	if (flags & ASSERT_DRRS_HIGH) {
>> +		if (!is_drrs_high()) {
>> +			drrs_print_status();
>> +			igt_assert_f(false, "DRRS HIGH\n");
>> +		}
>> +	} else if (flags & ASSERT_DRRS_LOW) {
>> +		if (!drrs_wait_until_rr_switch_to_low()) {
>> +			drrs_print_status();
>> +			igt_assert_f(false, "DRRS LOW\n");
>> +		}
>> +	} else if (flags & ASSERT_DRRS_INACTIVE) {
>> +		if (!is_drrs_inactive()) {
>> +			drrs_print_status();
>> +			igt_assert_f(false, "DRRS INACTIVE\n");
>> +		}
>> +	}
>> +
>>   	if (flags & ASSERT_FBC_ENABLED) {
>>   		igt_require(!fbc_not_enough_stolen());
>>   		igt_require(!fbc_stride_not_supported());
>> @@ -1833,6 +1962,8 @@ static void enable_features_for_test(const struct test_mode *t)
>>   		fbc_enable();
>>   	if (t->feature & FEATURE_PSR)
>>   		psr_enable();
>> +	if (t->feature & FEATURE_DRRS)
>> +		drrs_enable();
>>   }
>>   
>>   static void check_test_requirements(const struct test_mode *t)
>> @@ -1852,6 +1983,18 @@ static void check_test_requirements(const struct test_mode *t)
>>   			      "Can't test PSR without sink CRCs\n");
>>   	}
>>   
>> +	if (t->feature & FEATURE_DRRS)
>> +		igt_require_f(drrs.can_test,
>> +			      "Can't test DRRS with the current outputs\n");
>> +
>> +	/*
>> +	 * In kernel, When PSR is enabled, DRRS will be disabled. So If a test
>> +	 * case needs DRRS + PSR enabled, that will be skipped.
>> +	 */
>> +	igt_require_f(!((t->feature & FEATURE_PSR) &&
>> +		      (t->feature & FEATURE_DRRS)),
>> +		      "Can't test PSR and DRRS together\n");
>> +
>>   	if (opt.only_pipes != PIPE_COUNT)
>>   		igt_require(t->pipes == opt.only_pipes);
>>   }
>> @@ -1973,7 +2116,7 @@ static void rte_subtest(const struct test_mode *t)
>>   
>>   	unset_all_crtcs();
>>   	do_assertions(ASSERT_FBC_DISABLED | ASSERT_PSR_DISABLED |
>> -		      DONT_ASSERT_CRC);
>> +		      DONT_ASSERT_CRC | ASSERT_DRRS_INACTIVE);
>>   
>>   	enable_prim_screen_and_wait(t);
>>   	set_cursor_for_test(t, &prim_mode_params);
>> @@ -2065,6 +2208,13 @@ static void draw_subtest(const struct test_mode *t)
>>   	if (op_disables_psr(t, t->method))
>>   		assertions |= ASSERT_PSR_DISABLED;
>>   
>> +	/*
>> +	 * On FBS_INDIVIDUAL, write to offscreen plane will not touch the
>> +	 * current frambuffer. Hence assert for DRRS_LOW.
>> +	 */
>> +	if ((t->fbs == FBS_INDIVIDUAL) && (t->screen == SCREEN_OFFSCREEN))
>> +		assertions |= ASSERT_DRRS_LOW;
>> +
>>   	prepare_subtest(t, pattern);
>>   	target = pick_target(t, params);
>>   
>> @@ -2273,7 +2423,11 @@ static void slow_draw_subtest(const struct test_mode *t)
>>   		sleep(2);
>>   
>>   		update_wanted_crc(t, &pattern->crcs[t->format][r]);
>> -		do_assertions(0);
>> +
>> +		if (t->feature & FEATURE_DRRS)
>> +			do_assertions(ASSERT_DRRS_LOW);
>> +		else
>> +			do_assertions(0);
>>   	}
>>   }
>>   
>> @@ -2882,14 +3036,14 @@ static void suspend_subtest(const struct test_mode *t)
>>   	sleep(5);
>>   	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
>>   	sleep(5);
>> -	do_assertions(0);
>> +	do_assertions(ASSERT_DRRS_LOW);
>>   
>>   	unset_all_crtcs();
>>   	sleep(5);
>>   	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
>>   	sleep(5);
>>   	do_assertions(ASSERT_FBC_DISABLED | ASSERT_PSR_DISABLED |
>> -		      DONT_ASSERT_CRC);
>> +		      DONT_ASSERT_CRC | ASSERT_DRRS_INACTIVE);
>>   
>>   	set_mode_for_params(params);
>>   	do_assertions(0);
>> @@ -3371,6 +3525,14 @@ static const char *feature_str(int feature)
>>   		return "psr";
>>   	case FEATURE_FBC | FEATURE_PSR:
>>   		return "fbcpsr";
>> +	case FEATURE_DRRS:
>> +		return "drrs";
>> +	case FEATURE_FBC | FEATURE_DRRS:
>> +		return "fbcdrrs";
>> +	case FEATURE_PSR | FEATURE_DRRS:
>> +		return "psrdrrs";
>> +	case FEATURE_FBC | FEATURE_PSR | FEATURE_DRRS:
>> +		return "fbcpsrdrrs";
>>   	default:
>>   		igt_assert(false);
>>   	}
>> @@ -3635,7 +3797,7 @@ int main(int argc, char *argv[])
>>   				tilingchange_subtest(&t);
>>   		}
>>   
>> -		if (t.feature & FEATURE_PSR)
>> +		if ((t.feature & FEATURE_PSR) || (t.feature & FEATURE_DRRS))
>>   			igt_subtest_f("%s-slowdraw", feature_str(t.feature))
>>   				slow_draw_subtest(&t);
>>   
>> -- 
>> 1.9.1
>>



More information about the Intel-gfx mailing list