Em Sex, 2017-06-30 às 12:12 -0700, Jim Bride escreveu:
> v2: * Minor functional tweaks and bug fixes
> * Rebase
>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> Signed-off-by: Jim Bride <jim.bride@xxxxxxxxxxxxxxx>
> ---
> tests/kms_frontbuffer_tracking.c | 119 +++++++--------------------
> ------------
> 1 file changed, 19 insertions(+), 100 deletions(-)
>
> diff --git a/tests/kms_frontbuffer_tracking.c
> b/tests/kms_frontbuffer_tracking.c
> index c24e4a8..3a8b754 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -183,7 +183,7 @@ struct {
> };
>
>
> -#define SINK_CRC_SIZE 12
> +#define SINK_CRC_SIZE 6
> typedef struct {
> char data[SINK_CRC_SIZE];
> } sink_crc_t;
> @@ -327,28 +327,6 @@ drmModeModeInfo std_1024_mode = {
> .name = "Custom 1024x768",
> };
>
> -static drmModeModeInfoPtr
> get_connector_smallest_mode(drmModeConnectorPtr c)
> -{
> - int i;
> - drmModeModeInfoPtr smallest = NULL;
> -
> - for (i = 0; i < c->count_modes; i++) {
> - drmModeModeInfoPtr mode = &c->modes[i];
> -
> - if (!smallest)
> - smallest = mode;
> -
> - if (mode->hdisplay * mode->vdisplay <
> - smallest->hdisplay * smallest->vdisplay)
> - smallest = mode;
> - }
> -
> - if (c->connector_type == DRM_MODE_CONNECTOR_eDP)
> - smallest = &std_1024_mode;
> -
> - return smallest;
> -}
> -
> static drmModeConnectorPtr get_connector(uint32_t id)
> {
> int i;
> @@ -421,30 +399,6 @@ static void init_mode_params(struct
> modeset_params *params, uint32_t crtc_id,
> params->sprite.h = 64;
> }
>
> -static bool connector_get_mode(drmModeConnectorPtr c,
> drmModeModeInfoPtr *mode)
> -{
> - *mode = NULL;
> -
> - if (c->connection != DRM_MODE_CONNECTED || !c->count_modes)
> - return false;
> -
> - if (c->connector_type == DRM_MODE_CONNECTOR_eDP &&
> opt.no_edp)
> - return false;
> -
> - if (opt.small_modes)
> - *mode = get_connector_smallest_mode(c);
> - else
> - *mode = &c->modes[0];
> -
> - /* On HSW the CRC WA is so awful that it makes you think
> everything is
> - * bugged. */
> - if (IS_HASWELL(intel_get_drm_devid(drm.fd)) &&
> - c->connector_type == DRM_MODE_CONNECTOR_eDP)
> - *mode = &std_1024_mode;
> -
> - return true;
> -}
> -
> static bool connector_supports_pipe_a(drmModeConnectorPtr connector)
> {
> int i;
> @@ -473,7 +427,7 @@ static bool find_connector(bool edp_only, bool
> pipe_a, uint32_t forbidden_id,
> continue;
> if (c->connector_id == forbidden_id)
> continue;
> - if (!connector_get_mode(c, &mode))
> + if (!igt_psr_find_good_mode(c, &mode))
> continue;
>
> *ret_connector = c;
> @@ -804,23 +758,6 @@ static void fbc_print_status(void)
> igt_info("FBC status:\n%s\n", buf);
> }
>
> -static bool psr_is_enabled(void)
> -{
> - char buf[256];
> -
> - debugfs_read("i915_edp_psr_status", buf);
> - return strstr(buf, "\nActive: yes\n") &&
> - strstr(buf, "\nHW Enabled & Active bit: yes\n");
> -}
> -
> -static void psr_print_status(void)
> -{
> - char buf[256];
> -
> - debugfs_read("i915_edp_psr_status", buf);
> - igt_info("PSR status:\n%s\n", buf);
> -}
> -
> static struct timespec fbc_get_last_action(void)
> {
> struct timespec ret = { 0, 0 };
> @@ -926,44 +863,31 @@ static bool fbc_wait_until_enabled(void)
> return igt_wait(fbc_is_enabled(), 2000, 1);
> }
>
> -static bool psr_wait_until_enabled(void)
> -{
> - return igt_wait(psr_is_enabled(), 5000, 1);
Changing the timeout from 5s to 10s is going to significantly increase
the runtime for kms_frontbuffer_tracking.
> -}
> -
> #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)
>
> static void get_sink_crc(sink_crc_t *crc, bool mandatory)
> {
> - int rc, errno_;
> + int rc;
>
> if (!sink_crc.supported) {
> memcpy(crc, "unsupported!", SINK_CRC_SIZE);
> return;
> }
>
> - lseek(sink_crc.fd, 0, SEEK_SET);
> -
> - rc = read(sink_crc.fd, crc->data, SINK_CRC_SIZE);
> - errno_ = errno;
> -
> - if (rc == -1 && errno_ == ENOTTY) {
> + rc = igt_psr_get_sink_crc(sink_crc.fd, crc->data);
> + if (rc == ENOTTY) {
> igt_info("Sink CRC not supported: panel doesn't
> support it\n");
> sink_crc.supported = false;
> - } else if (rc == -1 && errno_ == ETIMEDOUT) {
> - if (sink_crc.reliable) {
> - igt_info("Sink CRC is unreliable on this
> machine.\n");
> + } else if (rc == ETIMEDOUT) {
> + if (sink_crc.reliable)
> sink_crc.reliable = false;
> - }
> +
>
> if (mandatory)
> igt_skip("Sink CRC is unreliable on this
> machine.\n");
> } else {
> - igt_assert_f(rc != -1, "Unexpected error: %d\n",
> errno_);
> - igt_assert(rc == SINK_CRC_SIZE);
> + igt_assert_f(rc == 0, "Unexpected error: %d\n", rc);
> }
> }
>
> @@ -1180,7 +1104,7 @@ static void disable_features(const struct
> test_mode *t)
> return;
>
> fbc_disable();
> - psr_disable();
> + igt_psr_disable();
> }
>
> static void *busy_thread_func(void *data)
> @@ -1432,7 +1356,7 @@ static void setup_sink_crc(void)
> fill_fb_region(&prim_mode_params.fb, COLOR_PRIM_BG);
> set_mode_for_params(&prim_mode_params);
>
> - sink_crc.fd = openat(drm.debugfs, "i915_sink_crc_eDP1",
> O_RDONLY);
> + sink_crc.fd = igt_psr_open_sink_crc(drm.debugfs);
> igt_assert_lte(0, sink_crc.fd);
>
> /* Do a first read to try to detect if it's supported. */
> @@ -1547,14 +1471,6 @@ static void teardown_fbc(void)
> {
> }
>
> -static bool psr_sink_has_support(void)
> -{
> - char buf[256];
> -
> - debugfs_read("i915_edp_psr_status", buf);
> - return strstr(buf, "Sink_Support: yes\n");
> -}
> -
> static void setup_psr(void)
> {
> if (get_connector(prim_mode_params.connector_id)-
> >connector_type !=
> @@ -1563,7 +1479,7 @@ static void setup_psr(void)
> return;
> }
>
> - if (!psr_sink_has_support()) {
> + if (!igt_psr_sink_support(drm.fd)) {
> igt_info("Can't test PSR: not supported by
> sink.\n");
> return;
> }
> @@ -1717,12 +1633,15 @@ static int adjust_assertion_flags(const
> struct test_mode *t, int flags)
> }
> \
>
> \
> if (flags_ & ASSERT_PSR_ENABLED) {
> \
> - if (!psr_wait_until_enabled()) {
> \
> - psr_print_status();
> \
> + if (!igt_psr_await_status(drm.fd, true)) {
> \
> + igt_psr_print_status(drm.fd);
> \
> igt_assert_f(false, "PSR disabled\n");
> \
> }
> \
> } else if (flags_ & ASSERT_PSR_DISABLED) {
> \
> - igt_assert(!psr_wait_until_enabled());
> \
> + if (!igt_psr_await_status(drm.fd, false)) {
There's a difference between "wait until disabled" and "wait until
enabled". If we're expecting PSR to be disabled and we call
wait_until_enabled(), we guarantee that PSR is going to be disabled
during the whole timeout. And that's the point: guarantee that PSR
stays disabled during the entire time, i.e., nothing enables it at any
point in the next $timeout seconds. It could be the case that PSR got
disabled and then re-enabled due to some delayed work function, and we
want to prevent that by staying the whole timeout re-checking its
status.
So here, if we just assert that PSR got disabled at some point during
the timeout, we're changing the behavior of the test and we're losing
the original purpose of the assertion.
> \
> + igt_psr_print_status(drm.fd);
> \
> + igt_assert_f(false, "PSR enabled\n");
> \
> + }
> \
> }
> \
> } while (0)
>
> @@ -1822,7 +1741,7 @@ static void enable_features_for_test(const
> struct test_mode *t)
> if (t->feature & FEATURE_FBC)
> fbc_enable();
> if (t->feature & FEATURE_PSR)
> - psr_enable();
> + igt_psr_enable();
> }
>
> static void check_test_requirements(const struct test_mode *t)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
