Re: [igt-dev] [PATCH i-g-t 06/10] lib/psr: Make psr_wait_entry and psr_wait_update aware of the PSR version tested

Dhinakaran Pandiyan
2019-01-16T05:19:33Z

On Fri, 2019-01-11 at 17:46 -0800, José Roberto de Souza wrote:
> This way we can test both PSR version separated.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> lib/igt_psr.c | 26 ++++++++++++++++++--------
> lib/igt_psr.h | 9 +++++++--
> tests/kms_frontbuffer_tracking.c | 4 ++--
> tests/kms_psr.c | 4 ++--
> 4 files changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index 5edec6a2..c6638c2c 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -25,26 +25,36 @@
> #include "igt_sysfs.h"
> #include <errno.h>
>
> -static bool psr_active(int debugfs_fd, bool check_active)
> +static bool psr_state_check(int debugfs_fd, const char *state)
nit: Is there a plan to use this function for anything else? Making
this function check only for the active state is more straightforward
IMO

-static bool psr_state_check(int debugfs_fd, const char *state)
+static bool psr_active_check(int debugfs_fd, enum psr_mode mode)
{
char buf[PSR_STATUS_MAX_LEN];
+ char *state;

+ state = mode == PSR_MODE_1 ? "SRDENT" : "DEEP_SLEEP";
igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
sizeof(buf));

return strstr(buf, state);
}
> {
> - bool active;
> char buf[PSR_STATUS_MAX_LEN];
>
> igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> sizeof(buf));
>
> - active = strstr(buf, "SRDENT") || strstr(buf, "DEEP_SLEEP");
> - return check_active ? active : !active;
> + return strstr(buf, state);
> }
>
> -bool psr_wait_entry(int debugfs_fd)
> +static inline const char *psr_active_state_get(enum psr_mode mode)
> {
> - return igt_wait(psr_active(debugfs_fd, true), 500, 20);
> + return mode == PSR_MODE_1 ? "SRDENT" : "DEEP_SLEEP";
> }
>
> -bool psr_wait_update(int debugfs_fd)
> +/*
> + * For PSR1, we wait until PSR is active. We wait until DEEP_SLEEP
> for PSR2.
> + */
> +bool psr_wait_entry(int debugfs_fd, enum psr_mode mode)
> {
> - return igt_wait(psr_active(debugfs_fd, false), 40, 10);
> + const char *state = psr_active_state_get(mode);
> +
> + return igt_wait(psr_state_check(debugfs_fd, state), 500, 20);
> +}
> +
> +bool psr_wait_update(int debugfs_fd, enum psr_mode mode)
> +{
> + const char *state = psr_active_state_get(mode);
> +
> + return igt_wait(!psr_state_check(debugfs_fd, state), 40, 10);
> }
>
> static ssize_t psr_write(int debugfs_fd, const char *buf)
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index 62c680ee..4fff77ec 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -30,8 +30,13 @@
>
> #define PSR_STATUS_MAX_LEN 512
>
> -bool psr_wait_entry(int debugfs_fd);
> -bool psr_wait_update(int debugfs_fd);
> +enum psr_mode {
Add a
PSR_MODE_DISABLE, too?
> + PSR_MODE_1,
> + PSR_MODE_2
> +};
> +
> +bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
> +bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
> bool psr_enable(int debugfs_fd);
> bool psr_disable(int debugfs_fd);
> bool psr_sink_support(int debugfs_fd);
> diff --git a/tests/kms_frontbuffer_tracking.c
> b/tests/kms_frontbuffer_tracking.c
> index cd7dda91..ed9a039a 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -1622,10 +1622,10 @@ static void do_status_assertions(int flags)
> }
>
> if (flags & ASSERT_PSR_ENABLED)
> - igt_assert_f(psr_wait_entry(drm.debugfs),
> + igt_assert_f(psr_wait_entry(drm.debugfs, PSR_MODE_1),
> "PSR still disabled\n");
> else if (flags & ASSERT_PSR_DISABLED)
> - igt_assert_f(psr_wait_update(drm.debugfs),
> + igt_assert_f(psr_wait_update(drm.debugfs, PSR_MODE_1),
> "PSR still enabled\n");
> }
>
> diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> index fb3bd7a4..23d000bd 100644
> --- a/tests/kms_psr.c
> +++ b/tests/kms_psr.c
> @@ -199,7 +199,7 @@ static bool psr_wait_entry_if_enabled(data_t
> *data)
> if (data->with_psr_disabled)
> return true;
>
> - return psr_wait_entry(data->debugfs_fd);
> + return psr_wait_entry(data->debugfs_fd, PSR_MODE_1);
> }
>
> static inline void manual(const char *expected)
> @@ -289,7 +289,7 @@ static void run_test(data_t *data)
> expected = "screen GREEN";
> break;
> }
> - igt_assert(psr_wait_update(data->debugfs_fd));
> + igt_assert(psr_wait_update(data->debugfs_fd, PSR_MODE_1));
> manual(expected);
> }