diff mbox

[igt,1/4] lib: add igt_wait()

Message ID 1418055164-1536-1-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni Dec. 8, 2014, 4:12 p.m. UTC
From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Just a little helper for code that needs to wait for a certain
condition to happen. It has the nice advantage that it can survive the
signal helper.

Despite the callers added in this patch, there is another that will go
in a separate patch, and another in a new IGT test file that I plan to
push later.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 lib/igt_aux.c  | 18 +-----------------
 lib/igt_aux.h  | 40 ++++++++++++++++++++++++++++++++++++++++
 tests/pm_rpm.c | 28 +++++-----------------------
 3 files changed, 46 insertions(+), 40 deletions(-)

Comments

Daniel Vetter Dec. 8, 2014, 4:40 p.m. UTC | #1
On Mon, Dec 08, 2014 at 02:12:41PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Just a little helper for code that needs to wait for a certain
> condition to happen. It has the nice advantage that it can survive the
> signal helper.
> 
> Despite the callers added in this patch, there is another that will go
> in a separate patch, and another in a new IGT test file that I plan to
> push later.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

A library unit test in lib/tests would be awesome for igt_wait. You can
run them with $ make check. If you're a bit bored, that is ;-)
-Daniel

> ---
>  lib/igt_aux.c  | 18 +-----------------
>  lib/igt_aux.h  | 40 ++++++++++++++++++++++++++++++++++++++++
>  tests/pm_rpm.c | 28 +++++-----------------------
>  3 files changed, 46 insertions(+), 40 deletions(-)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 3051d84..180141e 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -518,29 +518,13 @@ enum igt_runtime_pm_status igt_get_runtime_pm_status(void)
>   * Waits until for the driver to switch to into the desired runtime PM status,
>   * with a 10 second timeout.
>   *
> - * Some subtests call this function while the signal helper is active, so we
> - * can't assume each usleep() call will sleep for 100ms.
> - *
>   * Returns:
>   * True if the desired runtime PM status was attained, false if the operation
>   * timed out.
>   */
>  bool igt_wait_for_pm_status(enum igt_runtime_pm_status status)
>  {
> -	struct timeval start, end, diff;
> -
> -	igt_assert(gettimeofday(&start, NULL) == 0);
> -	do {
> -		if (igt_get_runtime_pm_status() == status)
> -			return true;
> -
> -		usleep(100 * 1000);
> -
> -		igt_assert(gettimeofday(&end, NULL) == 0);
> -		timersub(&end, &start, &diff);
> -	} while (diff.tv_sec < 10);
> -
> -	return false;
> +	return igt_wait(igt_get_runtime_pm_status() == status, 10000, 100);
>  }
>  
>  /* Functions with prefix kmstest_ independent of cairo library are pulled out
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 6c83c53..48c495b 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -30,6 +30,7 @@
>  
>  #include <intel_bufmgr.h>
>  #include <stdbool.h>
> +#include <sys/time.h>
>  
>  extern drm_intel_bo **trash_bos;
>  extern int num_trash_bos;
> @@ -90,4 +91,43 @@ void intel_require_memory(uint32_t count, uint32_t size, unsigned mode);
>  #define min(a, b) ((a) < (b) ? (a) : (b))
>  #define max(a, b) ((a) > (b) ? (a) : (b))
>  
> +/**
> + * igt_wait:
> + * @COND: condition to wait
> + * @timeout_ms: timeout in milliseconds
> + * @interval_ms: amount of time we try to sleep between COND checks
> + *
> + * Waits until COND evaluates to true or the timeout passes.
> + *
> + * It is safe to call this macro if the signal helper is active. The only
> + * problem is that the usleep() calls will return early, making us evaluate COND
> + * too often, possibly eating valuable CPU cycles.
> + *
> + * Returns:
> + * True of COND evaluated to true, false otherwise.
> + */
> +#define igt_wait(COND, timeout_ms, interval_ms) ({			\
> +	struct timeval start_, end_, diff_;				\
> +	int elapsed_ms_;						\
> +	bool ret_ = false;						\
> +									\
> +	igt_assert(gettimeofday(&start_, NULL) == 0);			\
> +	do {								\
> +		if (COND) {						\
> +			ret_ = true;					\
> +			break;						\
> +		}							\
> +									\
> +		usleep(interval_ms * 1000);				\
> +									\
> +		igt_assert(gettimeofday(&end_, NULL) == 0);		\
> +		timersub(&end_, &start_, &diff_);			\
> +									\
> +		elapsed_ms_ = diff_.tv_sec * 1000 +			\
> +			      diff_.tv_usec / 1000;			\
> +	} while (elapsed_ms_ < timeout_ms);				\
> +									\
> +	ret_;								\
> +})
> +
>  #endif /* IGT_AUX_H */
> diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
> index c120d75..b7f9635 100644
> --- a/tests/pm_rpm.c
> +++ b/tests/pm_rpm.c
> @@ -153,24 +153,16 @@ static uint64_t get_residency(uint32_t type)
>  
>  static bool pc8_plus_residency_changed(unsigned int timeout_sec)
>  {
> -	unsigned int i;
>  	uint64_t res_pc8, res_pc9, res_pc10;
> -	int to_sleep = 100 * 1000;
>  
>  	res_pc8 = get_residency(MSR_PC8_RES);
>  	res_pc9 = get_residency(MSR_PC9_RES);
>  	res_pc10 = get_residency(MSR_PC10_RES);
>  
> -	for (i = 0; i < timeout_sec * 1000 * 1000; i += to_sleep) {
> -		if (res_pc8 != get_residency(MSR_PC8_RES) ||
> -		    res_pc9 != get_residency(MSR_PC9_RES) ||
> -		    res_pc10 != get_residency(MSR_PC10_RES)) {
> -			return true;
> -		}
> -		usleep(to_sleep);
> -	}
> -
> -	return false;
> +	return igt_wait(res_pc8 != get_residency(MSR_PC8_RES) ||
> +			res_pc9 != get_residency(MSR_PC9_RES) ||
> +			res_pc10 != get_residency(MSR_PC10_RES),
> +			timeout_sec * 1000, 100);
>  }
>  
>  static enum pc8_status get_pc8_status(void)
> @@ -191,17 +183,7 @@ static enum pc8_status get_pc8_status(void)
>  
>  static bool wait_for_pc8_status(enum pc8_status status)
>  {
> -	int i;
> -	int hundred_ms = 100 * 1000, ten_s = 10 * 1000 * 1000;
> -
> -	for (i = 0; i < ten_s; i += hundred_ms) {
> -		if (get_pc8_status() == status)
> -			return true;
> -
> -		usleep(hundred_ms);
> -	}
> -
> -	return false;
> +	return igt_wait(get_pc8_status() == status, 10000, 100);
>  }
>  
>  static bool wait_for_suspended(void)
> -- 
> 2.1.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 3051d84..180141e 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -518,29 +518,13 @@  enum igt_runtime_pm_status igt_get_runtime_pm_status(void)
  * Waits until for the driver to switch to into the desired runtime PM status,
  * with a 10 second timeout.
  *
- * Some subtests call this function while the signal helper is active, so we
- * can't assume each usleep() call will sleep for 100ms.
- *
  * Returns:
  * True if the desired runtime PM status was attained, false if the operation
  * timed out.
  */
 bool igt_wait_for_pm_status(enum igt_runtime_pm_status status)
 {
-	struct timeval start, end, diff;
-
-	igt_assert(gettimeofday(&start, NULL) == 0);
-	do {
-		if (igt_get_runtime_pm_status() == status)
-			return true;
-
-		usleep(100 * 1000);
-
-		igt_assert(gettimeofday(&end, NULL) == 0);
-		timersub(&end, &start, &diff);
-	} while (diff.tv_sec < 10);
-
-	return false;
+	return igt_wait(igt_get_runtime_pm_status() == status, 10000, 100);
 }
 
 /* Functions with prefix kmstest_ independent of cairo library are pulled out
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 6c83c53..48c495b 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -30,6 +30,7 @@ 
 
 #include <intel_bufmgr.h>
 #include <stdbool.h>
+#include <sys/time.h>
 
 extern drm_intel_bo **trash_bos;
 extern int num_trash_bos;
@@ -90,4 +91,43 @@  void intel_require_memory(uint32_t count, uint32_t size, unsigned mode);
 #define min(a, b) ((a) < (b) ? (a) : (b))
 #define max(a, b) ((a) > (b) ? (a) : (b))
 
+/**
+ * igt_wait:
+ * @COND: condition to wait
+ * @timeout_ms: timeout in milliseconds
+ * @interval_ms: amount of time we try to sleep between COND checks
+ *
+ * Waits until COND evaluates to true or the timeout passes.
+ *
+ * It is safe to call this macro if the signal helper is active. The only
+ * problem is that the usleep() calls will return early, making us evaluate COND
+ * too often, possibly eating valuable CPU cycles.
+ *
+ * Returns:
+ * True of COND evaluated to true, false otherwise.
+ */
+#define igt_wait(COND, timeout_ms, interval_ms) ({			\
+	struct timeval start_, end_, diff_;				\
+	int elapsed_ms_;						\
+	bool ret_ = false;						\
+									\
+	igt_assert(gettimeofday(&start_, NULL) == 0);			\
+	do {								\
+		if (COND) {						\
+			ret_ = true;					\
+			break;						\
+		}							\
+									\
+		usleep(interval_ms * 1000);				\
+									\
+		igt_assert(gettimeofday(&end_, NULL) == 0);		\
+		timersub(&end_, &start_, &diff_);			\
+									\
+		elapsed_ms_ = diff_.tv_sec * 1000 +			\
+			      diff_.tv_usec / 1000;			\
+	} while (elapsed_ms_ < timeout_ms);				\
+									\
+	ret_;								\
+})
+
 #endif /* IGT_AUX_H */
diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index c120d75..b7f9635 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -153,24 +153,16 @@  static uint64_t get_residency(uint32_t type)
 
 static bool pc8_plus_residency_changed(unsigned int timeout_sec)
 {
-	unsigned int i;
 	uint64_t res_pc8, res_pc9, res_pc10;
-	int to_sleep = 100 * 1000;
 
 	res_pc8 = get_residency(MSR_PC8_RES);
 	res_pc9 = get_residency(MSR_PC9_RES);
 	res_pc10 = get_residency(MSR_PC10_RES);
 
-	for (i = 0; i < timeout_sec * 1000 * 1000; i += to_sleep) {
-		if (res_pc8 != get_residency(MSR_PC8_RES) ||
-		    res_pc9 != get_residency(MSR_PC9_RES) ||
-		    res_pc10 != get_residency(MSR_PC10_RES)) {
-			return true;
-		}
-		usleep(to_sleep);
-	}
-
-	return false;
+	return igt_wait(res_pc8 != get_residency(MSR_PC8_RES) ||
+			res_pc9 != get_residency(MSR_PC9_RES) ||
+			res_pc10 != get_residency(MSR_PC10_RES),
+			timeout_sec * 1000, 100);
 }
 
 static enum pc8_status get_pc8_status(void)
@@ -191,17 +183,7 @@  static enum pc8_status get_pc8_status(void)
 
 static bool wait_for_pc8_status(enum pc8_status status)
 {
-	int i;
-	int hundred_ms = 100 * 1000, ten_s = 10 * 1000 * 1000;
-
-	for (i = 0; i < ten_s; i += hundred_ms) {
-		if (get_pc8_status() == status)
-			return true;
-
-		usleep(hundred_ms);
-	}
-
-	return false;
+	return igt_wait(get_pc8_status() == status, 10000, 100);
 }
 
 static bool wait_for_suspended(void)