[Intel-gfx] [PATCH IGT v2 4/6] tests/kms_frontbuffer_tracking: Refactor to use IGT PSR library functions

Rodrigo Vivi rodrigo.vivi at gmail.com
Fri Jun 30 20:19:57 UTC 2017


I believe it would be better to create the psr lib with only one
function at time and on every patch that adds the new function already
removes that from here and from frontbuffer tracking test as well...

easier to review and to make sure that we are not changing the behaviour.

also...

On Fri, Jun 30, 2017 at 12:12 PM, Jim Bride <jim.bride at linux.intel.com> wrote:
> v2: * Minor functional tweaks and bug fixes
>     * Rebase
>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Signed-off-by: Jim Bride <jim.bride at linux.intel.com>
> ---
>  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

I believe this deserves a separated patch...

>  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);
> -}
> -
>  #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);

this doesn't fit anymore

>                 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)) {             \
> +                       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)
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br


More information about the Intel-gfx mailing list