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 <dhinakaran.pandiyan@intel.com>]

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

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev