| Message ID | 1515595620-23742-1-git-send-email-lohith.bs@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 10, 2018 at 02:47:00PM +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]. > > v13: En/Dis-able DRRS only when DRRS is capable on the setup. > > Signed-off-by: Lohith BS <lohith.bs@intel.com> > Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com> > --- > tests/kms_frontbuffer_tracking.c | 172 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 164 insertions(+), 8 deletions(-) > > diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c > index 1601cab..6b2299b 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 > @@ -182,6 +183,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 { > @@ -790,7 +798,14 @@ static void __debugfs_read(const char *param, char *buf, int len) > buf[len] = '\0'; > } > > +static void __debugfs_write(const char *param, char *buf, int len) > +{ > + igt_assert_eq(igt_sysfs_write(drm.debugfs, param, buf, len - 1), > + len - 1); > +} > + > #define debugfs_read(p, arr) __debugfs_read(p, arr, sizeof(arr)) > +#define debugfs_write(p, arr) __debugfs_write(p, arr, sizeof(arr)) > > static bool fbc_is_enabled(void) > { > @@ -825,6 +840,62 @@ static void psr_print_status(void) > igt_info("PSR status:\n%s\n", buf); > } > > +void drrs_set(unsigned int val) > +{ > + char buf[2]; > + > + if (!drrs.can_test) > + return; Can you please elaborate more about this solution here? I couldn't find the logs anymore. What was the issue that CI caught? What happens if a regular user on HSW with no DRRS monitor (same configuration as CI) write to this debugfs enabling DRRS? What I wonder is if this is the right place for this solution or on the kernel side. Also if it is a testcase I wonder if it shouldn't have a skip handling somewhere instead of a blind return.... Thanks, Rodrigo. > + > + igt_debug("Manually %sabling DRRS. %llu\n", val ? "en" : "dis", val); > + snprintf(buf, sizeof(buf), "%d", val); > + debugfs_write("i915_drrs_ctl", 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 +1006,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 +1262,7 @@ static void disable_features(const struct test_mode *t) > > fbc_disable(); > psr_disable(); > + drrs_disable(); > } > > static void *busy_thread_func(void *data) > @@ -1577,6 +1656,22 @@ 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; > + } > + > + drrs.can_test = true; > +} > + > static void setup_environment(void) > { > setup_drm(); > @@ -1584,6 +1679,7 @@ static void setup_environment(void) > > setup_fbc(); > setup_psr(); > + setup_drrs(); > > setup_crcs(); > } > @@ -1662,6 +1758,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 +1770,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 +1812,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 +1956,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 +1977,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 +2110,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 +2202,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 +2417,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 +3030,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 +3519,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 +3791,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 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wednesday 10 January 2018 11:45 PM, Rodrigo Vivi wrote: > On Wed, Jan 10, 2018 at 02:47:00PM +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]. >> >> v13: En/Dis-able DRRS only when DRRS is capable on the setup. >> >> Signed-off-by: Lohith BS <lohith.bs@intel.com> >> Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com> >> --- >> tests/kms_frontbuffer_tracking.c | 172 +++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 164 insertions(+), 8 deletions(-) >> >> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c >> index 1601cab..6b2299b 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 >> @@ -182,6 +183,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 { >> @@ -790,7 +798,14 @@ static void __debugfs_read(const char *param, char *buf, int len) >> buf[len] = '\0'; >> } >> >> +static void __debugfs_write(const char *param, char *buf, int len) >> +{ >> + igt_assert_eq(igt_sysfs_write(drm.debugfs, param, buf, len - 1), >> + len - 1); >> +} >> + >> #define debugfs_read(p, arr) __debugfs_read(p, arr, sizeof(arr)) >> +#define debugfs_write(p, arr) __debugfs_write(p, arr, sizeof(arr)) >> >> static bool fbc_is_enabled(void) >> { >> @@ -825,6 +840,62 @@ static void psr_print_status(void) >> igt_info("PSR status:\n%s\n", buf); >> } >> >> +void drrs_set(unsigned int val) >> +{ >> + char buf[2]; >> + >> + if (!drrs.can_test) >> + return; > Can you please elaborate more about this solution here? > > I couldn't find the logs anymore. What was the issue that CI caught? CI failure were caused due to two reasons: * Error from kernel on debugfs write when the platform doesn't support DRRS (<gen7). This can be addressed either from kernel by returning success from i915_drrs_ctl_set instead of -ENODEV for <gen7, or by not En/Disable the drrs feature when you already knew that feature is not supported. * Non Availability of debugfs_fd for debugfs write for DRRS when eDP panel was not there. This is handled by adding debugfs_write. Which will open and close the file as and when it is required using igt_sysfs_write. Similar to already existing debugfs_read. > > What happens if a regular user on HSW with no DRRS monitor (same configuration as CI) > write to this debugfs enabling DRRS? Basically this debugfs writein kernel on >=gen7 for drrs enable req: will enable drrs if there is any DRRS capable eDP panel. for drrs disable req: will disable drrs if there is any DRRS capable eDP panel in enabled state. on <gen7 returns -ENODEV. Not sure whether we should just skip all operations and return success on <gen7 also. This patch skips any write into the drrs ctrl debugfs. > > What I wonder is if this is the right place for this solution or on the > kernel side. > > Also if it is a testcase I wonder if it shouldn't have a skip handling > somewhere instead of a blind return.... In this IGT before every subtest, all the features are disabled and only the features required for that subtest is enabled. So we though its good to validate the en/Disable call at the implementation of drrs_set than at calling place. Please suggest if you think otherway. -Ram > > Thanks, > Rodrigo. > >> + >> + igt_debug("Manually %sabling DRRS. %llu\n", val ? "en" : "dis", val); >> + snprintf(buf, sizeof(buf), "%d", val); >> + debugfs_write("i915_drrs_ctl", 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 +1006,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 +1262,7 @@ static void disable_features(const struct test_mode *t) >> >> fbc_disable(); >> psr_disable(); >> + drrs_disable(); >> } >> >> static void *busy_thread_func(void *data) >> @@ -1577,6 +1656,22 @@ 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; >> + } >> + >> + drrs.can_test = true; >> +} >> + >> static void setup_environment(void) >> { >> setup_drm(); >> @@ -1584,6 +1679,7 @@ static void setup_environment(void) >> >> setup_fbc(); >> setup_psr(); >> + setup_drrs(); >> >> setup_crcs(); >> } >> @@ -1662,6 +1758,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 +1770,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 +1812,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 +1956,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 +1977,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 +2110,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 +2202,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 +2417,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 +3030,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 +3519,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 +3791,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 >> >> _______________________________________________ >> 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
On Wed, Jan 10, 2018 at 08:17:00PM +0530, 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]. > > v13: En/Dis-able DRRS only when DRRS is capable on the setup. > > Signed-off-by: Lohith BS <lohith.bs@intel.com> > Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com> Btw pls resubmit new versions as a new thread, it tends to confuse patchwork a bit if you in-reply-to forever. in-reply-to is kinda just to update individual patches in a big patch series while it's being discussed. Thanks, Daniel > --- > tests/kms_frontbuffer_tracking.c | 172 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 164 insertions(+), 8 deletions(-) > > diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c > index 1601cab..6b2299b 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 > @@ -182,6 +183,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 { > @@ -790,7 +798,14 @@ static void __debugfs_read(const char *param, char *buf, int len) > buf[len] = '\0'; > } > > +static void __debugfs_write(const char *param, char *buf, int len) > +{ > + igt_assert_eq(igt_sysfs_write(drm.debugfs, param, buf, len - 1), > + len - 1); > +} > + > #define debugfs_read(p, arr) __debugfs_read(p, arr, sizeof(arr)) > +#define debugfs_write(p, arr) __debugfs_write(p, arr, sizeof(arr)) > > static bool fbc_is_enabled(void) > { > @@ -825,6 +840,62 @@ static void psr_print_status(void) > igt_info("PSR status:\n%s\n", buf); > } > > +void drrs_set(unsigned int val) > +{ > + char buf[2]; > + > + if (!drrs.can_test) > + return; > + > + igt_debug("Manually %sabling DRRS. %llu\n", val ? "en" : "dis", val); > + snprintf(buf, sizeof(buf), "%d", val); > + debugfs_write("i915_drrs_ctl", 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 +1006,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 +1262,7 @@ static void disable_features(const struct test_mode *t) > > fbc_disable(); > psr_disable(); > + drrs_disable(); > } > > static void *busy_thread_func(void *data) > @@ -1577,6 +1656,22 @@ 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; > + } > + > + drrs.can_test = true; > +} > + > static void setup_environment(void) > { > setup_drm(); > @@ -1584,6 +1679,7 @@ static void setup_environment(void) > > setup_fbc(); > setup_psr(); > + setup_drrs(); > > setup_crcs(); > } > @@ -1662,6 +1758,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 +1770,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 +1812,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 +1956,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 +1977,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 +2110,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 +2202,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 +2417,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 +3030,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 +3519,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 +3791,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 >
On Thu, Jan 11, 2018 at 05:27:52AM +0000, Ramalingam C wrote: > > > On Wednesday 10 January 2018 11:45 PM, Rodrigo Vivi wrote: > > On Wed, Jan 10, 2018 at 02:47:00PM +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]. > > v13: En/Dis-able DRRS only when DRRS is capable on the setup. > > Signed-off-by: Lohith BS <lohith.bs@intel.com> > Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com> > --- > tests/kms_frontbuffer_tracking.c | 172 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 164 insertions(+), 8 deletions(-) > > diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c > index 1601cab..6b2299b 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 > @@ -182,6 +183,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 { > @@ -790,7 +798,14 @@ static void __debugfs_read(const char *param, char *buf, int len) > buf[len] = '\0'; > } > > +static void __debugfs_write(const char *param, char *buf, int len) > +{ > + igt_assert_eq(igt_sysfs_write(drm.debugfs, param, buf, len - 1), > + len - 1); > +} > + > #define debugfs_read(p, arr) __debugfs_read(p, arr, sizeof(arr)) > +#define debugfs_write(p, arr) __debugfs_write(p, arr, sizeof(arr)) > > static bool fbc_is_enabled(void) > { > @@ -825,6 +840,62 @@ static void psr_print_status(void) > igt_info("PSR status:\n%s\n", buf); > } > > +void drrs_set(unsigned int val) > +{ > + char buf[2]; > + > + if (!drrs.can_test) > + return; > > Can you please elaborate more about this solution here? > > I couldn't find the logs anymore. What was the issue that CI caught? > > > CI failure were caused due to two reasons: > > • Error from kernel on debugfs write when the platform doesn't support DRRS (<gen7). > > This can be addressed either from kernel by returning success from > i915_drrs_ctl_set instead of -ENODEV for <gen7, -ENODEV is better, but then igt test skip when handling this -ENODEV explicitly instead of a random return on !can_test. > or by not En/Disable the drrs feature when you already knew that feature is > not supported. > > • Non Availability of debugfs_fd for debugfs write for DRRS when eDP panel was not > there. > > This is handled by adding debugfs_write. Which will open and close the file > as and when it is required using igt_sysfs_write. > Similar to already existing debugfs_read. > > > What happens if a regular user on HSW with no DRRS monitor (same configuration as CI) > write to this debugfs enabling DRRS? > > > Basically this debugfs write in kernel > on >=gen7 > for drrs enable req: will enable drrs if there is any DRRS capable eDP panel. > for drrs disable req: will disable drrs if there is any DRRS capable eDP > panel in enabled state. > on <gen7 > returns -ENODEV. Not sure whether we should just skip all operations and > return success on <gen7 also. > > This patch skips any write into the drrs ctrl debugfs. > > > What I wonder is if this is the right place for this solution or on the > kernel side. > > Also if it is a testcase I wonder if it shouldn't have a skip handling > somewhere instead of a blind return.... > > In this IGT before every subtest, all the features are disabled and only the features > required for that subtest is enabled. > So we though its good to validate the en/Disable call at the implementation of drrs_set > than at calling place. Please suggest if you think otherway. my suggestion is to keep -ENODEV and igt check explicitly for this return value and skip testcase if -ENODEV is returned. > > -Ram > > > Thanks, > Rodrigo. > > > + > + igt_debug("Manually %sabling DRRS. %llu\n", val ? "en" : "dis", val); > + snprintf(buf, sizeof(buf), "%d", val); > + debugfs_write("i915_drrs_ctl", 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 +1006,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 +1262,7 @@ static void disable_features(const struct test_mode *t) > > fbc_disable(); > psr_disable(); > + drrs_disable(); > } > > static void *busy_thread_func(void *data) > @@ -1577,6 +1656,22 @@ 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; > + } > + > + drrs.can_test = true; > +} > + > static void setup_environment(void) > { > setup_drm(); > @@ -1584,6 +1679,7 @@ static void setup_environment(void) > > setup_fbc(); > setup_psr(); > + setup_drrs(); > > setup_crcs(); > } > @@ -1662,6 +1758,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 +1770,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 +1812,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 +1956,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 +1977,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 +2110,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 +2202,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 +2417,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 +3030,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 +3519,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 +3791,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 > > _______________________________________________ > 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 > >
Summary from IRC discussions with Rodrigo: drrs_disable will get called on all test setups(including the drrs non-capable). So we cant fail/skip the sub_test based on the return value from drrs_set(0) drrs_enable will get called only when the test setup is capable of drrs. And also after the request for enabling drrs, drrs state is verified through i915_drrs_status. So functionally it is not necessary to validate the debugfs_write return value. But no harm in double checking the error state for drrs_set(1). Rodrigo, as per above understanding i have mentioned the expected deltas inline. If you confirm, we will submit the next version. Thanks for the time. On Wednesday 10 January 2018 08:17 PM, 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]. > > v13: En/Dis-able DRRS only when DRRS is capable on the setup. > > Signed-off-by: Lohith BS <lohith.bs@intel.com> > Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com> > --- > tests/kms_frontbuffer_tracking.c | 172 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 164 insertions(+), 8 deletions(-) > > diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c > index 1601cab..6b2299b 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 > @@ -182,6 +183,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 { > @@ -790,7 +798,14 @@ static void __debugfs_read(const char *param, char *buf, int len) > buf[len] = '\0'; > } > > +static void __debugfs_write(const char *param, char *buf, int len) > +{ > + igt_assert_eq(igt_sysfs_write(drm.debugfs, param, buf, len - 1), > + len - 1); > +} > + redefining the __debugfs_write() as below: static int __debugfs_write(const char *param, char *buf, int len) { return igt_sysfs_write(drm.debugfs, param, buf, len - 1); } > #define debugfs_read(p, arr) __debugfs_read(p, arr, sizeof(arr)) > +#define debugfs_write(p, arr) __debugfs_write(p, arr, sizeof(arr)) > > static bool fbc_is_enabled(void) > { > @@ -825,6 +840,62 @@ static void psr_print_status(void) > igt_info("PSR status:\n%s\n", buf); > } > > +void drrs_set(unsigned int val) > +{ > + char buf[2]; > + > + if (!drrs.can_test) > + return; removing above two lines. > + > + igt_debug("Manually %sabling DRRS. %llu\n", val ? "en" : "dis", val); > + snprintf(buf, sizeof(buf), "%d", val); > + debugfs_write("i915_drrs_ctl", buf); replacing above line with below code ret = debugfs_write("i915_drrs_ctl", buf); /* drrs_enable can't fail, as that is called on drrs capable setup only. */ if (val) igt_assert_eq(ret, sizeof(buf) - 1); --Ram > +} > + > +static bool is_drrs_high(void) > +{ > + char buf[MAX_DRRS_STATUS_BUF_LEN]; >
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c index 1601cab..6b2299b 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 @@ -182,6 +183,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 { @@ -790,7 +798,14 @@ static void __debugfs_read(const char *param, char *buf, int len) buf[len] = '\0'; } +static void __debugfs_write(const char *param, char *buf, int len) +{ + igt_assert_eq(igt_sysfs_write(drm.debugfs, param, buf, len - 1), + len - 1); +} + #define debugfs_read(p, arr) __debugfs_read(p, arr, sizeof(arr)) +#define debugfs_write(p, arr) __debugfs_write(p, arr, sizeof(arr)) static bool fbc_is_enabled(void) { @@ -825,6 +840,62 @@ static void psr_print_status(void) igt_info("PSR status:\n%s\n", buf); } +void drrs_set(unsigned int val) +{ + char buf[2]; + + if (!drrs.can_test) + return; + + igt_debug("Manually %sabling DRRS. %llu\n", val ? "en" : "dis", val); + snprintf(buf, sizeof(buf), "%d", val); + debugfs_write("i915_drrs_ctl", 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 +1006,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 +1262,7 @@ static void disable_features(const struct test_mode *t) fbc_disable(); psr_disable(); + drrs_disable(); } static void *busy_thread_func(void *data) @@ -1577,6 +1656,22 @@ 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; + } + + drrs.can_test = true; +} + static void setup_environment(void) { setup_drm(); @@ -1584,6 +1679,7 @@ static void setup_environment(void) setup_fbc(); setup_psr(); + setup_drrs(); setup_crcs(); } @@ -1662,6 +1758,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 +1770,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 +1812,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 +1956,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 +1977,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 +2110,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 +2202,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 +2417,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 +3030,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 +3519,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 +3791,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);
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]. v13: En/Dis-able DRRS only when DRRS is capable on the setup. Signed-off-by: Lohith BS <lohith.bs@intel.com> Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com> --- tests/kms_frontbuffer_tracking.c | 172 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 164 insertions(+), 8 deletions(-)