[igt-dev] [PATCH igt-dev v14] tests/kms_frontbuffer_tracking: Including DRRS test coverage
Bs, Lohith
lohith.bs at intel.com
Wed Jan 31 12:17:15 UTC 2018
On 1/30/2018 11:32 PM, Rodrigo Vivi wrote:
> On Tue, Jan 30, 2018 at 05:57:37PM +0000, Rodrigo Vivi wrote:
>> On Tue, Jan 30, 2018 at 01:50:11PM +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.
>>>
>>> v14: Handle the error states of drrs_enable only [Rodrigo].
>>>
>>> Signed-off-by: Lohith BS <lohith.bs at intel.com>
>>> Signed-off-by: aknautiy <ankit.k.nautiyal at intel.com>
>>> ---
>>> tests/kms_frontbuffer_tracking.c | 177 +++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 169 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
>>> index 1601cab..046f16a 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,13 @@ static void __debugfs_read(const char *param, char *buf, int len)
>>> buf[len] = '\0';
>>> }
>>>
>>> +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 +839,68 @@ static void psr_print_status(void)
>>> igt_info("PSR status:\n%s\n", buf);
>>> }
>>>
>>> +void drrs_set(unsigned int val)
>>> +{
>>> + char buf[2];
>>> + int ret;
>>> +
>>> + igt_debug("Manually %sabling DRRS. %llu\n", val ? "en" : "dis", val);
>>> + snprintf(buf, sizeof(buf), "%d", val);
>>> + ret = debugfs_write("i915_drrs_ctl", buf);
>>> +
>>> + /*
>>> + * drrs_enable() is called on DRRS capable platform only,
>>> + * whereas drrs_disable() is called on all platforms.
>>> + * So handle the failure of debugfs_write only for drrs_enable().
>>> + */
>>> + if (val)
>>> + igt_assert_f(ret == (sizeof(buf) - 1), "debugfs_write failed");
>> I'd prefer the assert_neq(ret, -ENODEV) and in a way that we could kill the can_test
>> but I also cannot see that easy way on the current structure so it looks good for now
>>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> I take it back..
> it doesn't apply cleanly to igt master branch and after fixing conflicts meson build doesn't like it.
>
> Please make sure it applies and run properly on top of igt master branch and build cleanly with meson build.
>
> $ ./meson.sh
> ninja -C build
> ninja: Entering directory `build'
> [2/22] Compiling C object 'tests/kms_frontbuffer_tracking at exe/kms_frontbuffer_tracking.c.o'.
> In file included from ../lib/intel_batchbuffer.h:8:0,
> from ../lib/drmtest.h:39,
> from ../lib/igt.h:27,
> from ../tests/kms_frontbuffer_tracking.c:27:
> ../tests/kms_frontbuffer_tracking.c: In function ‘drrs_set’:
> ../tests/kms_frontbuffer_tracking.c:840:12: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 5 has type ‘unsigned int’ [-Wformat=]
> igt_debug("Manually %sabling DRRS. %llu\n", val ? "en" : "dis", val);
> ^
> ../lib/igt_core.h:795:64: note: in definition of macro ‘igt_debug’
> #define igt_debug(f...) igt_log(IGT_LOG_DOMAIN, IGT_LOG_DEBUG, f)
> ^
> [22/22] Generating igt_test_programs_gem_description.xml with a custom command.
Hi Rodrigo,
Thanks for reviewing.
Sorry I missed about the compiler warning. With the latest patch I have
re based and resolved all the compiler warnings.
Please consider the latest patch for the merge.
--Lohith
>
>> I'm going to merge now...
>>> +}
>>> +
>>> +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 +1011,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 +1267,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 +1661,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 +1684,7 @@ static void setup_environment(void)
>>>
>>> setup_fbc();
>>> setup_psr();
>>> + setup_drrs();
>>>
>>> setup_crcs();
>>> }
>>> @@ -1662,6 +1763,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 +1775,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 +1817,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 +1961,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 +1982,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 +2115,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 +2207,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 +2422,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 +3035,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 +3524,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 +3796,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 igt-dev
mailing list