[PATCH 0/7] wait_for and wait_until_reg

[PATCH 0/7] wait_for and wait_until_reg

[Mika Kuoppala]

I resurrected this series as there was irc discussion about wait_for
and it shortcomings. I have wait_until_reg conversions also 
as individual patches per file if that makes things easier to digest.

If we decide to go this route,

text	   data	    bss	    dec	    hex	filename
1166505	  24385	    920	1191810	 122f82	drivers/gpu/drm/i915/i915.ko.nightly
1155137	  24385	    920	1180442	 12031a	drivers/gpu/drm/i915/i915.ko

we save 11368 bytes.

Mika Kuoppala (7):
  drm/i915: Remove the wait_for_us macro
  drm/i915: Use milliseconds in _wait_for macro
  drm/i915: Spin opportunistically in wait_for
  drm/i915: Take longer naps in wait_for
  drm/i915: Introduce wait_until_reg
  drm/i915: Use wait_until_reg macros
  drm/i915/debug: Warn when waiting on condition timeouts

 drivers/gpu/drm/i915/i915_drv.c         | 19 +++++-----------
 drivers/gpu/drm/i915/i915_drv.h         | 40 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h         |  2 +-
 drivers/gpu/drm/i915/intel_crt.c        | 15 +++++++------
 drivers/gpu/drm/i915/intel_ddi.c        |  8 ++++---
 drivers/gpu/drm/i915/intel_display.c    | 38 ++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_dp.c         |  6 ++---
 drivers/gpu/drm/i915/intel_dp_mst.c     |  3 +--
 drivers/gpu/drm/i915/intel_dpll_mgr.c   |  2 +-
 drivers/gpu/drm/i915/intel_drv.h        | 33 +++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_dsi.c        | 19 +++++++---------
 drivers/gpu/drm/i915/intel_dsi_pll.c    |  5 ++---
 drivers/gpu/drm/i915/intel_fbc.c        |  3 +--
 drivers/gpu/drm/i915/intel_lrc.c        |  3 ++-
 drivers/gpu/drm/i915/intel_lvds.c       |  4 ++--
 drivers/gpu/drm/i915/intel_pm.c         |  6 ++---
 drivers/gpu/drm/i915/intel_psr.c        | 19 ++++++++--------
 drivers/gpu/drm/i915/intel_ringbuffer.c | 12 +++++-----
 drivers/gpu/drm/i915/intel_runtime_pm.c | 25 ++++++++++++---------
 drivers/gpu/drm/i915/intel_sideband.c   | 20 ++++++++---------
 drivers/gpu/drm/i915/intel_uncore.c     | 15 +++++++++----
 21 files changed, 175 insertions(+), 122 deletions(-)

-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/7] drm/i915: Remove the wait_for_us macro

[Mika Kuoppala]

We use jiffies based resolution for timeout detection. So
the promise that in the event of timeout, we would return in the 1us
resolution is false. And with quite large margin.

Remove the wait_for_us macro to prevent further broken promises
and convert the 3 callsites. The wait time will grow to 1ms but
this will be amended somewhat when wait_for gets enhanced with
adaptive backoff.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     | 4 ++--
 drivers/gpu/drm/i915/intel_display.c | 8 ++++----
 drivers/gpu/drm/i915/intel_drv.h     | 1 -
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c454744dda0b..a62c60c6fb0b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1826,8 +1826,8 @@ static void broxton_phy_init(struct drm_i915_private *dev_priv,
 	 * HW team confirmed that the time to reach phypowergood status is
 	 * anywhere between 50 us and 100us.
 	 */
-	if (wait_for_us(((I915_READ(BXT_PORT_CL1CM_DW0(phy)) &
-		(PHY_RESERVED | PHY_POWER_GOOD)) == PHY_POWER_GOOD), 100)) {
+	if (wait_for(((I915_READ(BXT_PORT_CL1CM_DW0(phy)) &
+		(PHY_RESERVED | PHY_POWER_GOOD)) == PHY_POWER_GOOD), 1)) {
 		DRM_ERROR("timeout during PHY%d power on\n", phy);
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4777087326f6..05d4a2b365f8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9619,8 +9619,8 @@ static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
 	val |= LCPLL_CD_SOURCE_FCLK;
 	I915_WRITE(LCPLL_CTL, val);
 
-	if (wait_for_us(I915_READ(LCPLL_CTL) &
-			LCPLL_CD_SOURCE_FCLK_DONE, 1))
+	if (wait_for(I915_READ(LCPLL_CTL) &
+		     LCPLL_CD_SOURCE_FCLK_DONE, 1))
 		DRM_ERROR("Switching to FCLK failed\n");
 
 	val = I915_READ(LCPLL_CTL);
@@ -9654,8 +9654,8 @@ static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
 	val &= ~LCPLL_CD_SOURCE_FCLK;
 	I915_WRITE(LCPLL_CTL, val);
 
-	if (wait_for_us((I915_READ(LCPLL_CTL) &
-			LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
+	if (wait_for((I915_READ(LCPLL_CTL) &
+		      LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
 		DRM_ERROR("Switching back to LCPLL failed\n");
 
 	mutex_lock(&dev_priv->rps.hw_lock);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3536292babe0..0fc8579e7b2e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -69,7 +69,6 @@
 })
 
 #define wait_for(COND, MS)	  	_wait_for((COND), (MS) * 1000, 1000)
-#define wait_for_us(COND, US)	  	_wait_for((COND), (US), 1)
 
 /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
 #if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/7] drm/i915: Use milliseconds in _wait_for macro

[Mika Kuoppala]

Promising 1us accuracy where we timeout using jiffies is
misleading. Convert the _wait_for macro to use milliseconds
and convert the 2 callsites.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  |  4 ++--
 drivers/gpu/drm/i915/intel_drv.h | 13 +++++++------
 drivers/gpu/drm/i915/intel_psr.c |  6 +++---
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 36330026ceff..a32617469816 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1699,8 +1699,8 @@ static void wait_panel_status(struct intel_dp *intel_dp,
 			I915_READ(pp_stat_reg),
 			I915_READ(pp_ctrl_reg));
 
-	if (_wait_for((I915_READ(pp_stat_reg) & mask) == value,
-		      5 * USEC_PER_SEC, 10 * USEC_PER_MSEC))
+	if (_wait_for_ms((I915_READ(pp_stat_reg) & mask) == value,
+			 5 * MSEC_PER_SEC, 10 * USEC_PER_MSEC))
 		DRM_ERROR("Panel status timeout: status %08x control %08x\n",
 				I915_READ(pp_stat_reg),
 				I915_READ(pp_ctrl_reg));
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0fc8579e7b2e..488141929a7a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -39,7 +39,7 @@
 #include <drm/drm_atomic.h>
 
 /**
- * _wait_for - magic (register) wait macro
+ * _wait_for_ms - magic (register) wait macro
  *
  * Does the right thing for modeset paths when run under kdgb or similar atomic
  * contexts. Note that it's important that we check the condition again after
@@ -50,8 +50,9 @@
  * drm_can_sleep() can be removed and in_atomic()/!in_atomic() asserts
  * added.
  */
-#define _wait_for(COND, US, W) ({ \
-	unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1;	\
+#define _wait_for_ms(COND, TIMEOUT_MS, SLEEP_US) ({ \
+	const unsigned long timeout__ =					\
+		jiffies + msecs_to_jiffies(TIMEOUT_MS) + 1;		\
 	int ret__ = 0;							\
 	while (!(COND)) {						\
 		if (time_after(jiffies, timeout__)) {			\
@@ -59,8 +60,8 @@
 				ret__ = -ETIMEDOUT;			\
 			break;						\
 		}							\
-		if ((W) && drm_can_sleep()) {				\
-			usleep_range((W), (W)*2);			\
+		if ((SLEEP_US) && drm_can_sleep()) {			\
+			usleep_range((SLEEP_US), (SLEEP_US) * 2);	\
 		} else {						\
 			cpu_relax();					\
 		}							\
@@ -68,7 +69,7 @@
 	ret__;								\
 })
 
-#define wait_for(COND, MS)	  	_wait_for((COND), (MS) * 1000, 1000)
+#define wait_for(COND, MS)	 _wait_for_ms((COND), (MS), 1 * USEC_PER_MSEC)
 
 /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
 #if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index c3abae4bc596..0ceb2026835e 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -506,9 +506,9 @@ static void hsw_psr_disable(struct intel_dp *intel_dp)
 			   I915_READ(EDP_PSR_CTL) & ~EDP_PSR_ENABLE);
 
 		/* Wait till PSR is idle */
-		if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
-			       EDP_PSR_STATUS_STATE_MASK) == 0,
-			       2 * USEC_PER_SEC, 10 * USEC_PER_MSEC))
+		if (_wait_for_ms((I915_READ(EDP_PSR_STATUS_CTL) &
+				  EDP_PSR_STATUS_STATE_MASK) == 0,
+				 2 * MSEC_PER_SEC, 10 * USEC_PER_MSEC))
 			DRM_ERROR("Timed out waiting for PSR Idle State\n");
 
 		dev_priv->psr.active = false;
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/7] drm/i915: Spin opportunistically in wait_for

[Mika Kuoppala]

Usually the condition we are after appears within very short time.
Spin few times before going into sleep. With this approximately
half of the wait_for in init path will take the fast path without
sleeping.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 488141929a7a..c225605c727c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -39,7 +39,7 @@
 #include <drm/drm_atomic.h>
 
 /**
- * _wait_for_ms - magic (register) wait macro
+ * __wait_for_ms - magic (register) wait macro
  *
  * Does the right thing for modeset paths when run under kdgb or similar atomic
  * contexts. Note that it's important that we check the condition again after
@@ -50,17 +50,22 @@
  * drm_can_sleep() can be removed and in_atomic()/!in_atomic() asserts
  * added.
  */
-#define _wait_for_ms(COND, TIMEOUT_MS, SLEEP_US) ({ \
+#define __wait_for_ms(COND, TIMEOUT_MS, SLEEP_US, SPIN_COUNT) ({	\
 	const unsigned long timeout__ =					\
 		jiffies + msecs_to_jiffies(TIMEOUT_MS) + 1;		\
+	unsigned int c__ = 0;						\
 	int ret__ = 0;							\
+									\
 	while (!(COND)) {						\
 		if (time_after(jiffies, timeout__)) {			\
 			if (!(COND))					\
 				ret__ = -ETIMEDOUT;			\
 			break;						\
 		}							\
-		if ((SLEEP_US) && drm_can_sleep()) {			\
+									\
+		if (++c__ > (SPIN_COUNT) &&				\
+		    (SLEEP_US) &&					\
+		    drm_can_sleep()) {					\
 			usleep_range((SLEEP_US), (SLEEP_US) * 2);	\
 		} else {						\
 			cpu_relax();					\
@@ -69,7 +74,8 @@
 	ret__;								\
 })
 
-#define wait_for(COND, MS)	 _wait_for_ms((COND), (MS), 1 * USEC_PER_MSEC)
+#define wait_for(COND, MS)  __wait_for_ms((COND), (MS), 1 * USEC_PER_MSEC, 5)
+#define _wait_for_ms(COND, MS, US)  __wait_for_ms((COND), (MS), (US), 5)
 
 /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
 #if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/7] drm/i915: Take longer naps in wait_for

[Mika Kuoppala]

If the condition we are after doesn't happen, start to sleep
longer and longer periods to save power. But never sleep more than 1/5th
of the timeout value. Convert few remaining callsites to use this generic
macro instead of letting them specifying their own sleeping periods.
This results in only one generic wait_for across all callsites so
we can remove the macro specifying the sleep period.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  |  4 ++--
 drivers/gpu/drm/i915/intel_drv.h | 15 ++++++++++-----
 drivers/gpu/drm/i915/intel_psr.c |  6 +++---
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a32617469816..b14761f585e9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1699,8 +1699,8 @@ static void wait_panel_status(struct intel_dp *intel_dp,
 			I915_READ(pp_stat_reg),
 			I915_READ(pp_ctrl_reg));
 
-	if (_wait_for_ms((I915_READ(pp_stat_reg) & mask) == value,
-			 5 * MSEC_PER_SEC, 10 * USEC_PER_MSEC))
+	if (wait_for((I915_READ(pp_stat_reg) & mask) == value,
+		     5 * MSEC_PER_SEC))
 		DRM_ERROR("Panel status timeout: status %08x control %08x\n",
 				I915_READ(pp_stat_reg),
 				I915_READ(pp_ctrl_reg));
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c225605c727c..1b03461e75e4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -50,7 +50,7 @@
  * drm_can_sleep() can be removed and in_atomic()/!in_atomic() asserts
  * added.
  */
-#define __wait_for_ms(COND, TIMEOUT_MS, SLEEP_US, SPIN_COUNT) ({	\
+#define __wait_for_ms(COND, TIMEOUT_MS, SPIN_COUNT) ({			\
 	const unsigned long timeout__ =					\
 		jiffies + msecs_to_jiffies(TIMEOUT_MS) + 1;		\
 	unsigned int c__ = 0;						\
@@ -64,9 +64,15 @@
 		}							\
 									\
 		if (++c__ > (SPIN_COUNT) &&				\
-		    (SLEEP_US) &&					\
+		    (TIMEOUT_MS) &&					\
 		    drm_can_sleep()) {					\
-			usleep_range((SLEEP_US), (SLEEP_US) * 2);	\
+			/* Limit max nap to 1/4 of timeout */		\
+			unsigned int s__ =				\
+				clamp_val(c__ * 2 * USEC_PER_MSEC,	\
+				    TIMEOUT_MS * 250,			\
+				      MAX_UDELAY_MS);			\
+									\
+			usleep_range(s__ >> 1, s__);			\
 		} else {						\
 			cpu_relax();					\
 		}							\
@@ -74,8 +80,7 @@
 	ret__;								\
 })
 
-#define wait_for(COND, MS)  __wait_for_ms((COND), (MS), 1 * USEC_PER_MSEC, 5)
-#define _wait_for_ms(COND, MS, US)  __wait_for_ms((COND), (MS), (US), 5)
+#define wait_for(COND, MS)  __wait_for_ms((COND), (MS), 5)
 
 /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
 #if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0ceb2026835e..912312b79eda 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -506,9 +506,9 @@ static void hsw_psr_disable(struct intel_dp *intel_dp)
 			   I915_READ(EDP_PSR_CTL) & ~EDP_PSR_ENABLE);
 
 		/* Wait till PSR is idle */
-		if (_wait_for_ms((I915_READ(EDP_PSR_STATUS_CTL) &
-				  EDP_PSR_STATUS_STATE_MASK) == 0,
-				 2 * MSEC_PER_SEC, 10 * USEC_PER_MSEC))
+		if (wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
+			      EDP_PSR_STATUS_STATE_MASK) == 0,
+			     2 * MSEC_PER_SEC))
 			DRM_ERROR("Timed out waiting for PSR Idle State\n");
 
 		dev_priv->psr.active = false;
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/7] drm/i915: Introduce wait_until_reg

[Mika Kuoppala]

The most common usage pattern for wait_for() macro is to wait for some
register value. Instead of bloating all callsites, encapsulate this
complex wait_for macro into a helper function.

v2: s/for/until, macro readability (Chris)

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     | 25 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uncore.c |  9 +++++++++
 2 files changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 72f0b02a8372..ccf6747894b1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2873,6 +2873,31 @@ static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv)
 	return dev_priv->vgpu.active;
 }
 
+int intel_wait_until_register(struct drm_i915_private *dev_priv,
+			      i915_reg_t reg,
+			      u32 mask,
+			      u32 value,
+			      unsigned long timeout_ms);
+
+#define wait_until_reg(reg, mask, value, timeout_ms) ({ \
+	if (__builtin_constant_p(timeout_ms)) \
+		BUILD_BUG_ON((timeout_ms) > 60000); \
+	intel_wait_until_register(dev_priv, \
+				  (reg), (mask), (value), (timeout_ms)); \
+	})
+
+#define wait_until_reg_set(reg, v, timeout_ms) ({ \
+	if (__builtin_constant_p(v)) \
+		BUILD_BUG_ON_NOT_POWER_OF_2((u32)(v)); \
+	wait_until_reg(reg, v, v, timeout_ms); \
+	})
+
+#define wait_until_reg_clr(reg, v, timeout_ms) ({ \
+	if (__builtin_constant_p((u32)(v))) \
+		BUILD_BUG_ON_NOT_POWER_OF_2((u32)(v)); \
+	wait_until_reg(reg, v, 0, timeout_ms); \
+	})
+
 void
 i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
 		     u32 status_mask);
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 385114bca924..e217997a9fa0 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1860,3 +1860,12 @@ intel_uncore_forcewake_for_reg(struct drm_i915_private *dev_priv,
 
 	return fw_domains;
 }
+
+int intel_wait_until_register(struct drm_i915_private *dev_priv,
+			      i915_reg_t reg,
+			      u32 mask,
+			      u32 value,
+			      unsigned long timeout_ms)
+{
+	return wait_for((I915_READ(reg) & mask) == value, timeout_ms);
+}
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/7] drm/i915: Use wait_until_reg macros

[Mika Kuoppala]

Use wait_until_reg instead of generic wait_for macro on
the relevant callsites.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         | 19 ++++++-----------
 drivers/gpu/drm/i915/i915_reg.h         |  2 +-
 drivers/gpu/drm/i915/intel_crt.c        | 15 +++++++------
 drivers/gpu/drm/i915/intel_ddi.c        |  8 ++++---
 drivers/gpu/drm/i915/intel_display.c    | 38 +++++++++++++++------------------
 drivers/gpu/drm/i915/intel_dp.c         |  6 ++----
 drivers/gpu/drm/i915/intel_dp_mst.c     |  3 +--
 drivers/gpu/drm/i915/intel_dpll_mgr.c   |  2 +-
 drivers/gpu/drm/i915/intel_dsi.c        | 19 +++++++----------
 drivers/gpu/drm/i915/intel_dsi_pll.c    |  5 ++---
 drivers/gpu/drm/i915/intel_fbc.c        |  3 +--
 drivers/gpu/drm/i915/intel_lrc.c        |  3 ++-
 drivers/gpu/drm/i915/intel_lvds.c       |  4 ++--
 drivers/gpu/drm/i915/intel_pm.c         |  6 ++----
 drivers/gpu/drm/i915/intel_psr.c        | 19 +++++++++--------
 drivers/gpu/drm/i915/intel_ringbuffer.c | 12 +++++------
 drivers/gpu/drm/i915/intel_runtime_pm.c | 25 ++++++++++++----------
 drivers/gpu/drm/i915/intel_sideband.c   | 20 ++++++++---------
 drivers/gpu/drm/i915/intel_uncore.c     |  6 ++----
 19 files changed, 100 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index dba03c026151..4bd40ef3c9b8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1357,8 +1357,6 @@ int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
 	u32 val;
 	int err;
 
-#define COND (I915_READ(VLV_GTLC_SURVIVABILITY_REG) & VLV_GFX_CLK_STATUS_BIT)
-
 	val = I915_READ(VLV_GTLC_SURVIVABILITY_REG);
 	val &= ~VLV_GFX_CLK_FORCE_ON_BIT;
 	if (force_on)
@@ -1368,13 +1366,13 @@ int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
 	if (!force_on)
 		return 0;
 
-	err = wait_for(COND, 20);
+	err = wait_until_reg_set(VLV_GTLC_SURVIVABILITY_REG,
+				 VLV_GFX_CLK_STATUS_BIT, 20);
 	if (err)
 		DRM_ERROR("timeout waiting for GFX clock force-on (%08x)\n",
 			  I915_READ(VLV_GTLC_SURVIVABILITY_REG));
 
 	return err;
-#undef COND
 }
 
 static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow)
@@ -1389,13 +1387,12 @@ static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow)
 	I915_WRITE(VLV_GTLC_WAKE_CTRL, val);
 	POSTING_READ(VLV_GTLC_WAKE_CTRL);
 
-#define COND (!!(I915_READ(VLV_GTLC_PW_STATUS) & VLV_GTLC_ALLOWWAKEACK) == \
-	      allow)
-	err = wait_for(COND, 1);
+	err = wait_until_reg(VLV_GTLC_PW_STATUS,
+			     VLV_GTLC_ALLOWWAKEACK, allow, 1);
 	if (err)
 		DRM_ERROR("timeout disabling GT waking\n");
+
 	return err;
-#undef COND
 }
 
 static int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv,
@@ -1407,9 +1404,6 @@ static int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv,
 
 	mask = VLV_GTLC_PW_MEDIA_STATUS_MASK | VLV_GTLC_PW_RENDER_STATUS_MASK;
 	val = wait_for_on ? mask : 0;
-#define COND ((I915_READ(VLV_GTLC_PW_STATUS) & mask) == val)
-	if (COND)
-		return 0;
 
 	DRM_DEBUG_KMS("waiting for GT wells to go %s (%08x)\n",
 		      onoff(wait_for_on),
@@ -1419,13 +1413,12 @@ static int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv,
 	 * RC6 transitioning can be delayed up to 2 msec (see
 	 * valleyview_enable_rps), use 3 msec for safety.
 	 */
-	err = wait_for(COND, 3);
+	err = wait_until_reg(VLV_GTLC_PW_STATUS, mask, val, 3);
 	if (err)
 		DRM_ERROR("timeout waiting for GT wells to go %s\n",
 			  onoff(wait_for_on));
 
 	return err;
-#undef COND
 }
 
 static void vlv_check_no_gt_access(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 86fbf723eca7..5355d5272ad1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2222,7 +2222,7 @@ enum skl_disp_power_wells {
 #define IVB_FBC_RT_BASE			_MMIO(0x7020)
 
 #define IPS_CTL		_MMIO(0x43408)
-#define   IPS_ENABLE	(1 << 31)
+#define   IPS_ENABLE	(1ULL << 31)
 
 #define MSG_FBC_REND_STATE	_MMIO(0x50380)
 #define   FBC_REND_NUKE		(1<<2)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 3fbb6fc66451..c1c619505393 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -301,8 +301,8 @@ static bool intel_ironlake_crt_detect_hotplug(struct drm_connector *connector)
 
 		I915_WRITE(crt->adpa_reg, adpa);
 
-		if (wait_for((I915_READ(crt->adpa_reg) & ADPA_CRT_HOTPLUG_FORCE_TRIGGER) == 0,
-			     1000))
+		if (wait_until_reg_clr(crt->adpa_reg,
+				       ADPA_CRT_HOTPLUG_FORCE_TRIGGER, 1000))
 			DRM_DEBUG_KMS("timed out waiting for FORCE_TRIGGER");
 
 		if (turn_off_dac) {
@@ -338,8 +338,9 @@ static bool valleyview_crt_detect_hotplug(struct drm_connector *connector)
 
 	I915_WRITE(crt->adpa_reg, adpa);
 
-	if (wait_for((I915_READ(crt->adpa_reg) & ADPA_CRT_HOTPLUG_FORCE_TRIGGER) == 0,
-		     1000)) {
+	if (wait_until_reg_clr(crt->adpa_reg,
+			       ADPA_CRT_HOTPLUG_FORCE_TRIGGER,
+			       1000)) {
 		DRM_DEBUG_KMS("timed out waiting for FORCE_TRIGGER");
 		I915_WRITE(crt->adpa_reg, save_adpa);
 	}
@@ -394,9 +395,9 @@ static bool intel_crt_detect_hotplug(struct drm_connector *connector)
 					      CRT_HOTPLUG_FORCE_DETECT,
 					      CRT_HOTPLUG_FORCE_DETECT);
 		/* wait for FORCE_DETECT to go off */
-		if (wait_for((I915_READ(PORT_HOTPLUG_EN) &
-			      CRT_HOTPLUG_FORCE_DETECT) == 0,
-			     1000))
+		if (wait_until_reg_clr(PORT_HOTPLUG_EN,
+				       CRT_HOTPLUG_FORCE_DETECT,
+				       1000))
 			DRM_DEBUG_KMS("timed out waiting for FORCE_DETECT to go off");
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index a62c60c6fb0b..7234af90e0e7 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1783,7 +1783,7 @@ static u32 broxton_get_grc(struct drm_i915_private *dev_priv, enum dpio_phy phy)
 static void broxton_phy_wait_grc_done(struct drm_i915_private *dev_priv,
 				      enum dpio_phy phy)
 {
-	if (wait_for(I915_READ(BXT_PORT_REF_DW3(phy)) & GRC_DONE, 10))
+	if (wait_until_reg_set(BXT_PORT_REF_DW3(phy), GRC_DONE, 10))
 		DRM_ERROR("timeout waiting for PHY%d GRC\n", phy);
 }
 
@@ -1826,8 +1826,9 @@ static void broxton_phy_init(struct drm_i915_private *dev_priv,
 	 * HW team confirmed that the time to reach phypowergood status is
 	 * anywhere between 50 us and 100us.
 	 */
-	if (wait_for(((I915_READ(BXT_PORT_CL1CM_DW0(phy)) &
-		(PHY_RESERVED | PHY_POWER_GOOD)) == PHY_POWER_GOOD), 1)) {
+	if (wait_until_reg(BXT_PORT_CL1CM_DW0(phy),
+			   (PHY_RESERVED | PHY_POWER_GOOD),
+			   PHY_POWER_GOOD, 1)) {
 		DRM_ERROR("timeout during PHY%d power on\n", phy);
 	}
 
@@ -1899,6 +1900,7 @@ static void broxton_phy_init(struct drm_i915_private *dev_priv,
 		 * the corresponding calibrated value from PHY1, and disable
 		 * the automatic calibration on PHY0.
 		 */
+
 		broxton_phy_wait_grc_done(dev_priv, DPIO_PHY1);
 
 		val = dev_priv->bxt_phy_grc = broxton_get_grc(dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 05d4a2b365f8..b95519642e94 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1118,8 +1118,7 @@ static void intel_wait_for_pipe_off(struct intel_crtc *crtc)
 		i915_reg_t reg = PIPECONF(cpu_transcoder);
 
 		/* Wait for the Pipe State to go off */
-		if (wait_for((I915_READ(reg) & I965_PIPECONF_ACTIVE) == 0,
-			     100))
+		if (wait_until_reg_clr(reg, I965_PIPECONF_ACTIVE, 100))
 			WARN(1, "pipe_off wait timed out\n");
 	} else {
 		/* Wait for the display line to settle */
@@ -1538,7 +1537,7 @@ static void _vlv_enable_pll(struct intel_crtc *crtc,
 	POSTING_READ(DPLL(pipe));
 	udelay(150);
 
-	if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
+	if (wait_until_reg_set(DPLL(pipe), DPLL_LOCK_VLV, 1))
 		DRM_ERROR("DPLL %d failed to lock\n", pipe);
 }
 
@@ -1587,7 +1586,7 @@ static void _chv_enable_pll(struct intel_crtc *crtc,
 	I915_WRITE(DPLL(pipe), pipe_config->dpll_hw_state.dpll);
 
 	/* Check PLL is locked */
-	if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
+	if (wait_until_reg_set(DPLL(pipe), DPLL_LOCK_VLV, 1))
 		DRM_ERROR("PLL %d failed to lock\n", pipe);
 }
 
@@ -1807,7 +1806,7 @@ void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
 		BUG();
 	}
 
-	if (wait_for((I915_READ(dpll_reg) & port_mask) == expected_mask, 1000))
+	if (wait_until_reg(dpll_reg, port_mask, expected_mask, 1000))
 		WARN(1, "timed out waiting for port %c ready: got 0x%x, expected 0x%x\n",
 		     port_name(dport->port), I915_READ(dpll_reg) & port_mask, expected_mask);
 }
@@ -1865,7 +1864,7 @@ static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
 		val |= TRANS_PROGRESSIVE;
 
 	I915_WRITE(reg, val | TRANS_ENABLE);
-	if (wait_for(I915_READ(reg) & TRANS_STATE_ENABLE, 100))
+	if (wait_until_reg_set(reg, TRANS_STATE_ENABLE, 100))
 		DRM_ERROR("failed to enable transcoder %c\n", pipe_name(pipe));
 }
 
@@ -1893,7 +1892,7 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv,
 		val |= TRANS_PROGRESSIVE;
 
 	I915_WRITE(LPT_TRANSCONF, val);
-	if (wait_for(I915_READ(LPT_TRANSCONF) & TRANS_STATE_ENABLE, 100))
+	if (wait_until_reg_set(LPT_TRANSCONF, TRANS_STATE_ENABLE, 100))
 		DRM_ERROR("Failed to enable PCH transcoder\n");
 }
 
@@ -1916,7 +1915,7 @@ static void ironlake_disable_pch_transcoder(struct drm_i915_private *dev_priv,
 	val &= ~TRANS_ENABLE;
 	I915_WRITE(reg, val);
 	/* wait for PCH transcoder off, transcoder state */
-	if (wait_for((I915_READ(reg) & TRANS_STATE_ENABLE) == 0, 50))
+	if (wait_until_reg_clr(reg, TRANS_STATE_ENABLE, 50))
 		DRM_ERROR("failed to disable transcoder %c\n", pipe_name(pipe));
 
 	if (HAS_PCH_CPT(dev)) {
@@ -1936,7 +1935,7 @@ static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
 	val &= ~TRANS_ENABLE;
 	I915_WRITE(LPT_TRANSCONF, val);
 	/* wait for PCH transcoder off, transcoder state */
-	if (wait_for((I915_READ(LPT_TRANSCONF) & TRANS_STATE_ENABLE) == 0, 50))
+	if (wait_until_reg_clr(LPT_TRANSCONF, TRANS_STATE_ENABLE, 50))
 		DRM_ERROR("Failed to disable PCH transcoder\n");
 
 	/* Workaround: clear timing override bit. */
@@ -4461,7 +4460,7 @@ void hsw_disable_ips(struct intel_crtc *crtc)
 		WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, 0));
 		mutex_unlock(&dev_priv->rps.hw_lock);
 		/* wait for pcode to finish disabling IPS, which may take up to 42ms */
-		if (wait_for((I915_READ(IPS_CTL) & IPS_ENABLE) == 0, 42))
+		if (wait_until_reg_clr(IPS_CTL, IPS_ENABLE, 42))
 			DRM_ERROR("Timed out waiting for IPS disable\n");
 	} else {
 		I915_WRITE(IPS_CTL, 0);
@@ -5411,8 +5410,7 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
 	    current_cdclk == 624000) {
 		I915_WRITE(BXT_DE_PLL_ENABLE, ~BXT_DE_PLL_PLL_ENABLE);
 		/* Timeout 200us */
-		if (wait_for(!(I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK),
-			     1))
+		if (wait_until_reg_clr(BXT_DE_PLL_ENABLE, BXT_DE_PLL_LOCK, 1))
 			DRM_ERROR("timout waiting for DE PLL unlock\n");
 	}
 
@@ -5426,7 +5424,7 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
 
 		I915_WRITE(BXT_DE_PLL_ENABLE, BXT_DE_PLL_PLL_ENABLE);
 		/* Timeout 200us */
-		if (wait_for(I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK, 1))
+		if (wait_until_reg_set(BXT_DE_PLL_ENABLE, BXT_DE_PLL_LOCK, 1))
 			DRM_ERROR("timeout waiting for DE PLL lock\n");
 
 		val = divider | skl_cdclk_decimal(cdclk);
@@ -5596,7 +5594,7 @@ skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco)
 
 	I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) | LCPLL_PLL_ENABLE);
 
-	if (wait_for(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK, 5))
+	if (wait_until_reg_set(LCPLL1_CTL, LCPLL_PLL_LOCK, 5))
 		DRM_ERROR("DPLL0 not locked\n");
 }
 
@@ -5604,7 +5602,7 @@ static void
 skl_dpll0_disable(struct drm_i915_private *dev_priv)
 {
 	I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) & ~LCPLL_PLL_ENABLE);
-	if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
+	if (wait_until_reg_clr(LCPLL1_CTL, LCPLL_PLL_LOCK, 1))
 		DRM_ERROR("Couldn't disable DPLL0\n");
 }
 
@@ -9415,7 +9413,7 @@ static void hsw_disable_lcpll(struct drm_i915_private *dev_priv,
 	I915_WRITE(LCPLL_CTL, val);
 	POSTING_READ(LCPLL_CTL);
 
-	if (wait_for((I915_READ(LCPLL_CTL) & LCPLL_PLL_LOCK) == 0, 1))
+	if (wait_until_reg_clr(LCPLL_CTL, LCPLL_PLL_LOCK, 1))
 		DRM_ERROR("LCPLL still locked\n");
 
 	val = hsw_read_dcomp(dev_priv);
@@ -9470,7 +9468,7 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
 	val &= ~LCPLL_PLL_DISABLE;
 	I915_WRITE(LCPLL_CTL, val);
 
-	if (wait_for(I915_READ(LCPLL_CTL) & LCPLL_PLL_LOCK, 5))
+	if (wait_until_reg_set(LCPLL_CTL, LCPLL_PLL_LOCK, 5))
 		DRM_ERROR("LCPLL not locked yet\n");
 
 	if (val & LCPLL_CD_SOURCE_FCLK) {
@@ -9619,8 +9617,7 @@ static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
 	val |= LCPLL_CD_SOURCE_FCLK;
 	I915_WRITE(LCPLL_CTL, val);
 
-	if (wait_for(I915_READ(LCPLL_CTL) &
-		     LCPLL_CD_SOURCE_FCLK_DONE, 1))
+	if (wait_until_reg_set(LCPLL_CTL, LCPLL_CD_SOURCE_FCLK_DONE, 1))
 		DRM_ERROR("Switching to FCLK failed\n");
 
 	val = I915_READ(LCPLL_CTL);
@@ -9654,8 +9651,7 @@ static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
 	val &= ~LCPLL_CD_SOURCE_FCLK;
 	I915_WRITE(LCPLL_CTL, val);
 
-	if (wait_for((I915_READ(LCPLL_CTL) &
-		      LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
+	if (wait_until_reg_clr(LCPLL_CTL, LCPLL_CD_SOURCE_FCLK_DONE, 1))
 		DRM_ERROR("Switching back to LCPLL failed\n");
 
 	mutex_lock(&dev_priv->rps.hw_lock);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b14761f585e9..f32e5d0a971e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1699,8 +1699,7 @@ static void wait_panel_status(struct intel_dp *intel_dp,
 			I915_READ(pp_stat_reg),
 			I915_READ(pp_ctrl_reg));
 
-	if (wait_for((I915_READ(pp_stat_reg) & mask) == value,
-		     5 * MSEC_PER_SEC))
+	if (wait_until_reg(pp_stat_reg, mask, value, 5 * MSEC_PER_SEC))
 		DRM_ERROR("Panel status timeout: status %08x control %08x\n",
 				I915_READ(pp_stat_reg),
 				I915_READ(pp_ctrl_reg));
@@ -3252,8 +3251,7 @@ void intel_dp_set_idle_link_train(struct intel_dp *intel_dp)
 	if (port == PORT_A)
 		return;
 
-	if (wait_for((I915_READ(DP_TP_STATUS(port)) & DP_TP_STATUS_IDLE_DONE),
-		     1))
+	if (wait_until_reg_set(DP_TP_STATUS(port), DP_TP_STATUS_IDLE_DONE, 1))
 		DRM_ERROR("Timed out waiting for DP idle patterns\n");
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 7a34090cef34..8a066dd670c5 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -213,8 +213,7 @@ static void intel_mst_enable_dp(struct intel_encoder *encoder)
 
 	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
 
-	if (wait_for((I915_READ(DP_TP_STATUS(port)) & DP_TP_STATUS_ACT_SENT),
-		     1))
+	if (wait_until_reg_set(DP_TP_STATUS(port), DP_TP_STATUS_ACT_SENT, 1))
 		DRM_ERROR("Timed out waiting for ACT sent\n");
 
 	ret = drm_dp_check_act_status(&intel_dp->mst_mgr);
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index c283ba4babe8..9699cbcdef19 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -853,7 +853,7 @@ static void skl_ddi_pll_enable(struct drm_i915_private *dev_priv,
 	I915_WRITE(regs[pll->id].ctl,
 		   I915_READ(regs[pll->id].ctl) | LCPLL_PLL_ENABLE);
 
-	if (wait_for(I915_READ(DPLL_STATUS) & DPLL_LOCK(pll->id), 5))
+	if (wait_until_reg_set(DPLL_STATUS, DPLL_LOCK(pll->id), 5))
 		DRM_ERROR("DPLL %d not locked\n", pll->id);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 4009618a5b34..60d2f73d68d2 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -90,7 +90,7 @@ static void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
 	mask = LP_CTRL_FIFO_EMPTY | HS_CTRL_FIFO_EMPTY |
 		LP_DATA_FIFO_EMPTY | HS_DATA_FIFO_EMPTY;
 
-	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & mask) == mask, 100))
+	if (wait_until_reg(MIPI_GEN_FIFO_STAT(port), mask, mask, 100))
 		DRM_ERROR("DPI FIFOs are not empty\n");
 }
 
@@ -159,7 +159,7 @@ static ssize_t intel_dsi_host_transfer(struct mipi_dsi_host *host,
 	/* note: this is never true for reads */
 	if (packet.payload_length) {
 
-		if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & data_mask) == 0, 50))
+		if (wait_until_reg_clr(MIPI_GEN_FIFO_STAT(port), data_mask, 50))
 			DRM_ERROR("Timeout waiting for HS/LP DATA FIFO !full\n");
 
 		write_data(dev_priv, data_reg, packet.payload,
@@ -170,16 +170,15 @@ static ssize_t intel_dsi_host_transfer(struct mipi_dsi_host *host,
 		I915_WRITE(MIPI_INTR_STAT(port), GEN_READ_DATA_AVAIL);
 	}
 
-	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & ctrl_mask) == 0, 50)) {
+	if (wait_until_reg_clr(MIPI_GEN_FIFO_STAT(port), ctrl_mask, 50))
 		DRM_ERROR("Timeout waiting for HS/LP CTRL FIFO !full\n");
-	}
 
 	I915_WRITE(ctrl_reg, header[2] << 16 | header[1] << 8 | header[0]);
 
 	/* ->rx_len is set only for reads */
 	if (msg->rx_len) {
-		data_mask = GEN_READ_DATA_AVAIL;
-		if (wait_for((I915_READ(MIPI_INTR_STAT(port)) & data_mask) == data_mask, 50))
+		if (wait_until_reg_set(MIPI_INTR_STAT(port),
+				       GEN_READ_DATA_AVAIL, 50))
 			DRM_ERROR("Timeout waiting for read data.\n");
 
 		read_data(dev_priv, data_reg, msg->rx_buf, msg->rx_len);
@@ -251,7 +250,6 @@ static int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs,
 	struct drm_encoder *encoder = &intel_dsi->base.base;
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 mask;
 
 	/* XXX: pipe, hs */
 	if (hs)
@@ -268,8 +266,8 @@ static int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs,
 
 	I915_WRITE(MIPI_DPI_CONTROL(port), cmd);
 
-	mask = SPL_PKT_SENT_INTERRUPT;
-	if (wait_for((I915_READ(MIPI_INTR_STAT(port)) & mask) == mask, 100))
+	if (wait_until_reg_set(MIPI_INTR_STAT(port), SPL_PKT_SENT_INTERRUPT,
+			       100))
 		DRM_ERROR("Video mode command 0x%08x send failed.\n", cmd);
 
 	return 0;
@@ -667,8 +665,7 @@ static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
 		/* Wait till Clock lanes are in LP-00 state for MIPI Port A
 		 * only. MIPI Port C has no similar bit for checking
 		 */
-		if (wait_for(((I915_READ(port_ctrl) & AFE_LATCHOUT)
-						== 0x00000), 30))
+		if (wait_until_reg_clr(port_ctrl, AFE_LATCHOUT, 30))
 			DRM_ERROR("DSI LP not going Low\n");
 
 		/* Disable MIPI PHY transparent latch */
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 1765e6e18f2c..2feba836ee39 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -234,8 +234,7 @@ static void bxt_disable_dsi_pll(struct intel_encoder *encoder)
 	 * PLL lock should deassert within 200us.
 	 * Wait up to 1ms before timing out.
 	 */
-	if (wait_for((I915_READ(BXT_DSI_PLL_ENABLE)
-					& BXT_DSI_PLL_LOCKED) == 0, 1))
+	if (wait_until_reg_clr(BXT_DSI_PLL_ENABLE, BXT_DSI_PLL_LOCKED, 1))
 		DRM_ERROR("Timeout waiting for PLL lock deassertion\n");
 }
 
@@ -486,7 +485,7 @@ static void bxt_enable_dsi_pll(struct intel_encoder *encoder,
 	I915_WRITE(BXT_DSI_PLL_ENABLE, val);
 
 	/* Timeout and fail if PLL not locked */
-	if (wait_for(I915_READ(BXT_DSI_PLL_ENABLE) & BXT_DSI_PLL_LOCKED, 1)) {
+	if (wait_until_reg_set(BXT_DSI_PLL_ENABLE, BXT_DSI_PLL_LOCKED, 1)) {
 		DRM_ERROR("Timed out waiting for DSI PLL to lock\n");
 		return;
 	}
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 0dea5fbcd8aa..19ca799f4793 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -123,8 +123,7 @@ static void i8xx_fbc_deactivate(struct drm_i915_private *dev_priv)
 	fbc_ctl &= ~FBC_CTL_EN;
 	I915_WRITE(FBC_CONTROL, fbc_ctl);
 
-	/* Wait for compressing bit to clear */
-	if (wait_for((I915_READ(FBC_STATUS) & FBC_STAT_COMPRESSING) == 0, 10)) {
+	if (wait_until_reg_clr(FBC_STATUS, FBC_STAT_COMPRESSING, 10)) {
 		DRM_DEBUG_KMS("FBC idle timed out\n");
 		return;
 	}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index db10c961e0f4..62ae2e938085 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -911,7 +911,8 @@ void intel_logical_ring_stop(struct intel_engine_cs *engine)
 
 	/* TODO: Is this correct with Execlists enabled? */
 	I915_WRITE_MODE(engine, _MASKED_BIT_ENABLE(STOP_RING));
-	if (wait_for((I915_READ_MODE(engine) & MODE_IDLE) != 0, 1000)) {
+	if (wait_until_reg_set(RING_MI_MODE(engine->mmio_base), MODE_IDLE,
+			       1000)) {
 		DRM_ERROR("%s :timed out trying to stop ring\n", engine->name);
 		return;
 	}
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index d65fd945607a..8754f8bf5e00 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -231,7 +231,7 @@ static void intel_enable_lvds(struct intel_encoder *encoder)
 
 	I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
 	POSTING_READ(lvds_encoder->reg);
-	if (wait_for((I915_READ(stat_reg) & PP_ON) != 0, 1000))
+	if (wait_until_reg_set(stat_reg, PP_ON, 1000))
 		DRM_ERROR("timed out waiting for panel to power on\n");
 
 	intel_panel_enable_backlight(intel_connector);
@@ -253,7 +253,7 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
 	}
 
 	I915_WRITE(ctl_reg, I915_READ(ctl_reg) & ~POWER_TARGET_ON);
-	if (wait_for((I915_READ(stat_reg) & PP_ON) == 0, 1000))
+	if (wait_until_reg_clr(stat_reg, PP_ON, 1000))
 		DRM_ERROR("timed out waiting for panel to power off\n");
 
 	I915_WRITE(lvds_encoder->reg, I915_READ(lvds_encoder->reg) & ~LVDS_PORT_EN);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index adb64638f595..e72890210a6e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7431,8 +7431,7 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u32 mbox, u32 *val
 	I915_WRITE(GEN6_PCODE_DATA1, 0);
 	I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
 
-	if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
-		     500)) {
+	if (wait_until_reg_clr(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY, 500)) {
 		DRM_ERROR("timeout waiting for pcode read (%d) to finish\n", mbox);
 		return -ETIMEDOUT;
 	}
@@ -7455,8 +7454,7 @@ int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u32 mbox, u32 val
 	I915_WRITE(GEN6_PCODE_DATA, val);
 	I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
 
-	if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
-		     500)) {
+	if (wait_until_reg_clr(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY, 500)) {
 		DRM_ERROR("timeout waiting for pcode write (%d) to finish\n", mbox);
 		return -ETIMEDOUT;
 	}
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 912312b79eda..bb1ab0d13ee6 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -479,8 +479,8 @@ static void vlv_psr_disable(struct intel_dp *intel_dp)
 
 	if (dev_priv->psr.active) {
 		/* Put VLV PSR back to PSR_state 0 that is PSR Disabled. */
-		if (wait_for((I915_READ(VLV_PSRSTAT(intel_crtc->pipe)) &
-			      VLV_EDP_PSR_IN_TRANS) == 0, 1))
+		if (wait_until_reg_clr(VLV_PSRSTAT(intel_crtc->pipe),
+				       VLV_EDP_PSR_IN_TRANS, 1))
 			WARN(1, "PSR transition took longer than expected\n");
 
 		val = I915_READ(VLV_PSRCTL(intel_crtc->pipe));
@@ -506,9 +506,9 @@ static void hsw_psr_disable(struct intel_dp *intel_dp)
 			   I915_READ(EDP_PSR_CTL) & ~EDP_PSR_ENABLE);
 
 		/* Wait till PSR is idle */
-		if (wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
-			      EDP_PSR_STATUS_STATE_MASK) == 0,
-			     2 * MSEC_PER_SEC))
+		if (wait_until_reg(EDP_PSR_STATUS_CTL,
+				   EDP_PSR_STATUS_STATE_MASK, 0,
+				   2 * MSEC_PER_SEC))
 			DRM_ERROR("Timed out waiting for PSR Idle State\n");
 
 		dev_priv->psr.active = false;
@@ -563,15 +563,16 @@ static void intel_psr_work(struct work_struct *work)
 	 * PSR might take some time to get fully disabled
 	 * and be ready for re-enable.
 	 */
+
 	if (HAS_DDI(dev_priv)) {
-		if (wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
-			      EDP_PSR_STATUS_STATE_MASK) == 0, 50)) {
+		if (wait_until_reg(EDP_PSR_STATUS_CTL,
+				   EDP_PSR_STATUS_STATE_MASK, 0, 50)) {
 			DRM_ERROR("Timed out waiting for PSR Idle for re-enable\n");
 			return;
 		}
 	} else {
-		if (wait_for((I915_READ(VLV_PSRSTAT(pipe)) &
-			      VLV_EDP_PSR_IN_TRANS) == 0, 1)) {
+		if (wait_until_reg_clr(VLV_PSRSTAT(pipe),
+				       VLV_EDP_PSR_IN_TRANS, 1)) {
 			DRM_ERROR("Timed out waiting for PSR Idle for re-enable\n");
 			return;
 		}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8d35a3978f9b..46702594f93b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -515,8 +515,7 @@ static void intel_ring_setup_status_page(struct intel_engine_cs *engine)
 		I915_WRITE(reg,
 			   _MASKED_BIT_ENABLE(INSTPM_TLB_INVALIDATE |
 					      INSTPM_SYNC_FLUSH));
-		if (wait_for((I915_READ(reg) & INSTPM_SYNC_FLUSH) == 0,
-			     1000))
+		if (wait_until_reg_clr(reg, INSTPM_SYNC_FLUSH, 1000))
 			DRM_ERROR("%s: wait for SyncFlush to complete for TLB invalidation timed out\n",
 				  engine->name);
 	}
@@ -528,7 +527,8 @@ static bool stop_ring(struct intel_engine_cs *engine)
 
 	if (!IS_GEN2(dev_priv)) {
 		I915_WRITE_MODE(engine, _MASKED_BIT_ENABLE(STOP_RING));
-		if (wait_for((I915_READ_MODE(engine) & MODE_IDLE) != 0, 1000)) {
+		if (wait_until_reg_set(RING_MI_MODE(engine->mmio_base),
+				       MODE_IDLE, 1000)) {
 			DRM_ERROR("%s : timed out trying to stop ring\n",
 				  engine->name);
 			/* Sometimes we observe that the idle flag is not
@@ -2545,9 +2545,9 @@ static void gen6_bsd_ring_write_tail(struct intel_engine_cs *engine,
 	I915_WRITE64(GEN6_BSD_RNCID, 0x0);
 
 	/* Wait for the ring not to be idle, i.e. for it to wake up. */
-	if (wait_for((I915_READ(GEN6_BSD_SLEEP_PSMI_CONTROL) &
-		      GEN6_BSD_SLEEP_INDICATOR) == 0,
-		     50))
+	if (wait_until_reg_clr(GEN6_BSD_SLEEP_PSMI_CONTROL,
+			       GEN6_BSD_SLEEP_INDICATOR,
+			       50))
 		DRM_ERROR("timed out waiting for the BSD ring to wake up\n");
 
 	/* Now that the ring is fully powered up, update the tail */
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index b69b935516fb..a11342c9ddd4 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -345,8 +345,8 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 
 		if (!is_enabled) {
 			DRM_DEBUG_KMS("Enabling power well\n");
-			if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) &
-				      HSW_PWR_WELL_STATE_ENABLED), 20))
+			if (wait_until_reg_set(HSW_PWR_WELL_DRIVER,
+					       HSW_PWR_WELL_STATE_ENABLED, 20))
 				DRM_ERROR("Timeout enabling power well\n");
 			hsw_power_well_post_enable(dev_priv);
 		}
@@ -669,8 +669,8 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
 
 	switch (power_well->data) {
 	case SKL_DISP_PW_1:
-		if (wait_for((I915_READ(SKL_FUSE_STATUS) &
-			SKL_FUSE_PG0_DIST_STATUS), 1)) {
+		if (wait_until_reg_set(SKL_FUSE_STATUS,
+				       SKL_FUSE_PG0_DIST_STATUS, 1)) {
 			DRM_ERROR("PG0 not enabled\n");
 			return;
 		}
@@ -731,12 +731,12 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
 
 	if (check_fuse_status) {
 		if (power_well->data == SKL_DISP_PW_1) {
-			if (wait_for((I915_READ(SKL_FUSE_STATUS) &
-				SKL_FUSE_PG1_DIST_STATUS), 1))
+			if (wait_until_reg_set(SKL_FUSE_STATUS,
+					       SKL_FUSE_PG1_DIST_STATUS, 1))
 				DRM_ERROR("PG1 distributing status timeout\n");
 		} else if (power_well->data == SKL_DISP_PW_2) {
-			if (wait_for((I915_READ(SKL_FUSE_STATUS) &
-				SKL_FUSE_PG2_DIST_STATUS), 1))
+			if (wait_until_reg_set(SKL_FUSE_STATUS,
+					       SKL_FUSE_PG2_DIST_STATUS, 1))
 				DRM_ERROR("PG2 distributing status timeout\n");
 		}
 	}
@@ -1097,7 +1097,6 @@ static void assert_chv_phy_status(struct drm_i915_private *dev_priv)
 	u32 phy_control = dev_priv->chv_phy_control;
 	u32 phy_status = 0;
 	u32 phy_status_mask = 0xffffffff;
-	u32 tmp;
 
 	/*
 	 * The BIOS can leave the PHY is some weird state
@@ -1185,10 +1184,14 @@ static void assert_chv_phy_status(struct drm_i915_private *dev_priv)
 	 * The PHY may be busy with some initial calibration and whatnot,
 	 * so the power state can take a while to actually change.
 	 */
-	if (wait_for((tmp = I915_READ(DISPLAY_PHY_STATUS) & phy_status_mask) == phy_status, 10))
+	if (wait_until_reg(DISPLAY_PHY_STATUS,
+			   phy_status_mask, phy_status, 10)) {
+		u32 tmp = I915_READ(DISPLAY_PHY_STATUS);
+
 		WARN(phy_status != tmp,
 		     "Unexpected PHY_STATUS 0x%08x, expected 0x%08x (PHY_CONTROL=0x%08x)\n",
 		     tmp, phy_status, dev_priv->chv_phy_control);
+	}
 }
 
 #undef BITS_SET
@@ -1216,7 +1219,7 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
 	vlv_set_power_well(dev_priv, power_well, true);
 
 	/* Poll for phypwrgood signal */
-	if (wait_for(I915_READ(DISPLAY_PHY_STATUS) & PHY_POWERGOOD(phy), 1))
+	if (wait_until_reg_set(DISPLAY_PHY_STATUS, PHY_POWERGOOD(phy), 1))
 		DRM_ERROR("Display PHY %d is not power up\n", phy);
 
 	mutex_lock(&dev_priv->sb_lock);
diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
index c3998188cf35..2b5bb9ba160d 100644
--- a/drivers/gpu/drm/i915/intel_sideband.c
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -51,7 +51,7 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
 
 	WARN_ON(!mutex_is_locked(&dev_priv->sb_lock));
 
-	if (wait_for((I915_READ(VLV_IOSF_DOORBELL_REQ) & IOSF_SB_BUSY) == 0, 5)) {
+	if (wait_until_reg_clr(VLV_IOSF_DOORBELL_REQ, IOSF_SB_BUSY, 5)) {
 		DRM_DEBUG_DRIVER("IOSF sideband idle wait (%s) timed out\n",
 				 is_read ? "read" : "write");
 		return -EAGAIN;
@@ -62,7 +62,7 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
 		I915_WRITE(VLV_IOSF_DATA, *val);
 	I915_WRITE(VLV_IOSF_DOORBELL_REQ, cmd);
 
-	if (wait_for((I915_READ(VLV_IOSF_DOORBELL_REQ) & IOSF_SB_BUSY) == 0, 5)) {
+	if (wait_until_reg_clr(VLV_IOSF_DOORBELL_REQ, IOSF_SB_BUSY, 5)) {
 		DRM_DEBUG_DRIVER("IOSF sideband finish wait (%s) timed out\n",
 				 is_read ? "read" : "write");
 		return -ETIMEDOUT;
@@ -202,8 +202,7 @@ u32 intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg,
 	u32 value = 0;
 	WARN_ON(!mutex_is_locked(&dev_priv->sb_lock));
 
-	if (wait_for((I915_READ(SBI_CTL_STAT) & SBI_BUSY) == 0,
-				100)) {
+	if (wait_until_reg_clr(SBI_CTL_STAT, SBI_BUSY, 100)) {
 		DRM_ERROR("timeout waiting for SBI to become ready\n");
 		return 0;
 	}
@@ -216,8 +215,9 @@ u32 intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg,
 		value = SBI_CTL_DEST_MPHY | SBI_CTL_OP_IORD;
 	I915_WRITE(SBI_CTL_STAT, value | SBI_BUSY);
 
-	if (wait_for((I915_READ(SBI_CTL_STAT) & (SBI_BUSY | SBI_RESPONSE_FAIL)) == 0,
-				100)) {
+	if (wait_until_reg(SBI_CTL_STAT,
+			   (SBI_BUSY | SBI_RESPONSE_FAIL), 0,
+			   100)) {
 		DRM_ERROR("timeout waiting for SBI to complete read transaction\n");
 		return 0;
 	}
@@ -232,8 +232,7 @@ void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value,
 
 	WARN_ON(!mutex_is_locked(&dev_priv->sb_lock));
 
-	if (wait_for((I915_READ(SBI_CTL_STAT) & SBI_BUSY) == 0,
-				100)) {
+	if (wait_until_reg_clr(SBI_CTL_STAT, SBI_BUSY, 100)) {
 		DRM_ERROR("timeout waiting for SBI to become ready\n");
 		return;
 	}
@@ -247,8 +246,9 @@ void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value,
 		tmp = SBI_CTL_DEST_MPHY | SBI_CTL_OP_IOWR;
 	I915_WRITE(SBI_CTL_STAT, SBI_BUSY | tmp);
 
-	if (wait_for((I915_READ(SBI_CTL_STAT) & (SBI_BUSY | SBI_RESPONSE_FAIL)) == 0,
-				100)) {
+	if (wait_until_reg(SBI_CTL_STAT,
+			   (SBI_BUSY | SBI_RESPONSE_FAIL), 0,
+			   100)) {
 		DRM_ERROR("timeout waiting for SBI to complete write transaction\n");
 		return;
 	}
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index e217997a9fa0..f72420e00621 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1530,15 +1530,13 @@ static int ironlake_do_reset(struct drm_i915_private *dev_priv,
 
 	I915_WRITE(ILK_GDSR,
 		   ILK_GRDOM_RENDER | ILK_GRDOM_RESET_ENABLE);
-	ret = wait_for((I915_READ(ILK_GDSR) &
-			ILK_GRDOM_RESET_ENABLE) == 0, 500);
+	ret = wait_until_reg_clr(ILK_GDSR, ILK_GRDOM_RESET_ENABLE, 500);
 	if (ret)
 		return ret;
 
 	I915_WRITE(ILK_GDSR,
 		   ILK_GRDOM_MEDIA | ILK_GRDOM_RESET_ENABLE);
-	ret = wait_for((I915_READ(ILK_GDSR) &
-			ILK_GRDOM_RESET_ENABLE) == 0, 500);
+	ret = wait_until_reg_clr(ILK_GDSR, ILK_GRDOM_RESET_ENABLE, 500);
 	if (ret)
 		return ret;
 
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/7] drm/i915/debug: Warn when waiting on condition timeouts

[Mika Kuoppala]

Warn if we timeout on waiting register or other condition.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     | 25 ++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_drv.h    | 10 +++++++++-
 drivers/gpu/drm/i915/intel_uncore.c | 10 +++++-----
 3 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ccf6747894b1..3d03a17a8b7c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2873,11 +2873,26 @@ static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv)
 	return dev_priv->vgpu.active;
 }
 
-int intel_wait_until_register(struct drm_i915_private *dev_priv,
-			      i915_reg_t reg,
-			      u32 mask,
-			      u32 value,
-			      unsigned long timeout_ms);
+int __intel_wait_until_register(struct drm_i915_private *dev_priv,
+				i915_reg_t reg,
+				u32 mask,
+				u32 value,
+				unsigned long timeout_ms);
+
+#if defined(CONFIG_DRM_I915_DEBUG)
+#define intel_wait_until_register(dev_priv, reg, mask, value, timeout_ms) ({ \
+	int __ret; \
+	WARN_ON(__ret = __intel_wait_until_register((dev_priv), \
+						    (reg), (mask), \
+						    (value), \
+						    (timeout_ms))); \
+	__ret; \
+	})
+#else
+#define intel_wait_until_register(dev_priv, reg, mask, value, timeout_ms) \
+	__intel_wait_until_register((dev_priv), \
+				    (reg), (mask), (value), timeout_ms);
+#endif
 
 #define wait_until_reg(reg, mask, value, timeout_ms) ({ \
 	if (__builtin_constant_p(timeout_ms)) \
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1b03461e75e4..917b649f84b6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -80,7 +80,15 @@
 	ret__;								\
 })
 
-#define wait_for(COND, MS)  __wait_for_ms((COND), (MS), 5)
+#if defined(CONFIG_DRM_I915_DEBUG)
+#define wait_for(COND, MS)  ({ \
+	int r__; \
+	WARN_ON(r__ = __wait_for_ms((COND), (MS), 5)); \
+	r__;\
+	})
+#else
+#define wait_for(COND, MS) __wait_for_ms((COND), (MS), 5)
+#endif
 
 /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
 #if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index f72420e00621..def601ba93d4 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1859,11 +1859,11 @@ intel_uncore_forcewake_for_reg(struct drm_i915_private *dev_priv,
 	return fw_domains;
 }
 
-int intel_wait_until_register(struct drm_i915_private *dev_priv,
-			      i915_reg_t reg,
-			      u32 mask,
-			      u32 value,
-			      unsigned long timeout_ms)
+int __intel_wait_until_register(struct drm_i915_private *dev_priv,
+				i915_reg_t reg,
+				u32 mask,
+				u32 value,
+				unsigned long timeout_ms)
 {
 	return wait_for((I915_READ(reg) & mask) == value, timeout_ms);
 }
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Ro.CI.BAT: failure for wait_for and wait_until_reg

[Patchwork]

== Series Details ==

Series: wait_for and wait_until_reg
URL   : https://patchwork.freedesktop.org/series/7305/
State : failure

== Summary ==

Series 7305v1 wait_for and wait_until_reg
http://patchwork.freedesktop.org/api/1.0/series/7305/revisions/1/mbox

Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (ro-hsw-i7-4770r)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-c:
                pass       -> DMESG-WARN (ro-ivb-i7-3770)

fi-bdw-i7-5557u  total:219  pass:206  dwarn:0   dfail:0   fail:0   skip:13 
fi-bsw-n3050     total:218  pass:174  dwarn:0   dfail:0   fail:2   skip:42 
fi-hsw-i7-4770k  total:219  pass:198  dwarn:0   dfail:0   fail:0   skip:21 
fi-hsw-i7-4770r  total:219  pass:193  dwarn:0   dfail:0   fail:0   skip:26 
fi-kbl-y         total:219  pass:190  dwarn:2   dfail:0   fail:2   skip:25 
fi-skl-i7-6700k  total:219  pass:191  dwarn:0   dfail:0   fail:0   skip:28 
ro-bdw-i5-5250u  total:219  pass:181  dwarn:0   dfail:0   fail:0   skip:38 
ro-bdw-i7-5557U  total:219  pass:206  dwarn:0   dfail:0   fail:0   skip:13 
ro-bdw-i7-5600u  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-byt-n2820     total:218  pass:174  dwarn:0   dfail:0   fail:3   skip:41 
ro-hsw-i3-4010u  total:218  pass:193  dwarn:0   dfail:0   fail:0   skip:25 
ro-hsw-i7-4770r  total:219  pass:193  dwarn:0   dfail:0   fail:1   skip:25 
ro-ilk-i7-620lm  total:219  pass:151  dwarn:0   dfail:0   fail:1   skip:67 
ro-ilk1-i5-650   total:214  pass:151  dwarn:0   dfail:0   fail:2   skip:61 
ro-ivb-i7-3770   total:219  pass:182  dwarn:1   dfail:0   fail:0   skip:36 
ro-ivb2-i7-3770  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-skl-i7-6700hq total:214  pass:189  dwarn:0   dfail:0   fail:0   skip:25 
ro-snb-i7-2620M  total:219  pass:177  dwarn:0   dfail:0   fail:1   skip:41 
fi-byt-n2820 failed to connect after reboot
fi-snb-i7-2600 failed to connect after reboot
ro-bsw-n3050 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_924/

68c6a6d drm-intel-nightly: 2016y-05m-17d-13h-59m-27s UTC integration manifest
bfe366e drm/i915/debug: Warn when waiting on condition timeouts
eda7126 drm/i915: Use wait_until_reg macros
6064898 drm/i915: Introduce wait_until_reg
6a87001 drm/i915: Take longer naps in wait_for
2bc34cd drm/i915: Spin opportunistically in wait_for
003241f drm/i915: Use milliseconds in _wait_for macro
c98838e drm/i915: Remove the wait_for_us macro

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/7] drm/i915: Remove the wait_for_us macro

[Daniel Vetter]

On Tue, May 17, 2016 at 06:43:22PM +0300, Mika Kuoppala wrote:
> We use jiffies based resolution for timeout detection. So
> the promise that in the event of timeout, we would return in the 1us
> resolution is false. And with quite large margin.
> 
> Remove the wait_for_us macro to prevent further broken promises
> and convert the 3 callsites. The wait time will grow to 1ms but
> this will be amended somewhat when wait_for gets enhanced with
> adaptive backoff.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c     | 4 ++--
>  drivers/gpu/drm/i915/intel_display.c | 8 ++++----
>  drivers/gpu/drm/i915/intel_drv.h     | 1 -
>  3 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index c454744dda0b..a62c60c6fb0b 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1826,8 +1826,8 @@ static void broxton_phy_init(struct drm_i915_private *dev_priv,
>  	 * HW team confirmed that the time to reach phypowergood status is
>  	 * anywhere between 50 us and 100us.
>  	 */
> -	if (wait_for_us(((I915_READ(BXT_PORT_CL1CM_DW0(phy)) &
> -		(PHY_RESERVED | PHY_POWER_GOOD)) == PHY_POWER_GOOD), 100)) {
> +	if (wait_for(((I915_READ(BXT_PORT_CL1CM_DW0(phy)) &
> +		(PHY_RESERVED | PHY_POWER_GOOD)) == PHY_POWER_GOOD), 1)) {
>  		DRM_ERROR("timeout during PHY%d power on\n", phy);
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4777087326f6..05d4a2b365f8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9619,8 +9619,8 @@ static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
>  	val |= LCPLL_CD_SOURCE_FCLK;
>  	I915_WRITE(LCPLL_CTL, val);
>  
> -	if (wait_for_us(I915_READ(LCPLL_CTL) &
> -			LCPLL_CD_SOURCE_FCLK_DONE, 1))
> +	if (wait_for(I915_READ(LCPLL_CTL) &
> +		     LCPLL_CD_SOURCE_FCLK_DONE, 1))
>  		DRM_ERROR("Switching to FCLK failed\n");
>  
>  	val = I915_READ(LCPLL_CTL);
> @@ -9654,8 +9654,8 @@ static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
>  	val &= ~LCPLL_CD_SOURCE_FCLK;
>  	I915_WRITE(LCPLL_CTL, val);
>  
> -	if (wait_for_us((I915_READ(LCPLL_CTL) &
> -			LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
> +	if (wait_for((I915_READ(LCPLL_CTL) &
> +		      LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
>  		DRM_ERROR("Switching back to LCPLL failed\n");
>  
>  	mutex_lock(&dev_priv->rps.hw_lock);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3536292babe0..0fc8579e7b2e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -69,7 +69,6 @@
>  })
>  
>  #define wait_for(COND, MS)	  	_wait_for((COND), (MS) * 1000, 1000)
> -#define wait_for_us(COND, US)	  	_wait_for((COND), (US), 1)
>  
>  /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
>  #if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/7] drm/i915: Use milliseconds in _wait_for macro

[Daniel Vetter]

On Tue, May 17, 2016 at 06:43:23PM +0300, Mika Kuoppala wrote:
> Promising 1us accuracy where we timeout using jiffies is
> misleading. Convert the _wait_for macro to use milliseconds
> and convert the 2 callsites.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  |  4 ++--
>  drivers/gpu/drm/i915/intel_drv.h | 13 +++++++------
>  drivers/gpu/drm/i915/intel_psr.c |  6 +++---
>  3 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 36330026ceff..a32617469816 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1699,8 +1699,8 @@ static void wait_panel_status(struct intel_dp *intel_dp,
>  			I915_READ(pp_stat_reg),
>  			I915_READ(pp_ctrl_reg));
>  
> -	if (_wait_for((I915_READ(pp_stat_reg) & mask) == value,
> -		      5 * USEC_PER_SEC, 10 * USEC_PER_MSEC))
> +	if (_wait_for_ms((I915_READ(pp_stat_reg) & mask) == value,
> +			 5 * MSEC_PER_SEC, 10 * USEC_PER_MSEC))
>  		DRM_ERROR("Panel status timeout: status %08x control %08x\n",
>  				I915_READ(pp_stat_reg),
>  				I915_READ(pp_ctrl_reg));
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0fc8579e7b2e..488141929a7a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -39,7 +39,7 @@
>  #include <drm/drm_atomic.h>
>  
>  /**
> - * _wait_for - magic (register) wait macro
> + * _wait_for_ms - magic (register) wait macro
>   *
>   * Does the right thing for modeset paths when run under kdgb or similar atomic
>   * contexts. Note that it's important that we check the condition again after
> @@ -50,8 +50,9 @@
>   * drm_can_sleep() can be removed and in_atomic()/!in_atomic() asserts
>   * added.
>   */
> -#define _wait_for(COND, US, W) ({ \
> -	unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1;	\
> +#define _wait_for_ms(COND, TIMEOUT_MS, SLEEP_US) ({ \
> +	const unsigned long timeout__ =					\
> +		jiffies + msecs_to_jiffies(TIMEOUT_MS) + 1;		\

msec_to_jiffies_timeout()

Otherwise Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>


>  	int ret__ = 0;							\
>  	while (!(COND)) {						\
>  		if (time_after(jiffies, timeout__)) {			\
> @@ -59,8 +60,8 @@
>  				ret__ = -ETIMEDOUT;			\
>  			break;						\
>  		}							\
> -		if ((W) && drm_can_sleep()) {				\
> -			usleep_range((W), (W)*2);			\
> +		if ((SLEEP_US) && drm_can_sleep()) {			\
> +			usleep_range((SLEEP_US), (SLEEP_US) * 2);	\
>  		} else {						\
>  			cpu_relax();					\
>  		}							\
> @@ -68,7 +69,7 @@
>  	ret__;								\
>  })
>  
> -#define wait_for(COND, MS)	  	_wait_for((COND), (MS) * 1000, 1000)
> +#define wait_for(COND, MS)	 _wait_for_ms((COND), (MS), 1 * USEC_PER_MSEC)
>  
>  /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
>  #if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index c3abae4bc596..0ceb2026835e 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -506,9 +506,9 @@ static void hsw_psr_disable(struct intel_dp *intel_dp)
>  			   I915_READ(EDP_PSR_CTL) & ~EDP_PSR_ENABLE);
>  
>  		/* Wait till PSR is idle */
> -		if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
> -			       EDP_PSR_STATUS_STATE_MASK) == 0,
> -			       2 * USEC_PER_SEC, 10 * USEC_PER_MSEC))
> +		if (_wait_for_ms((I915_READ(EDP_PSR_STATUS_CTL) &
> +				  EDP_PSR_STATUS_STATE_MASK) == 0,
> +				 2 * MSEC_PER_SEC, 10 * USEC_PER_MSEC))
>  			DRM_ERROR("Timed out waiting for PSR Idle State\n");
>  
>  		dev_priv->psr.active = false;
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Spin opportunistically in wait_for

[Daniel Vetter]

On Tue, May 17, 2016 at 06:43:24PM +0300, Mika Kuoppala wrote:
> Usually the condition we are after appears within very short time.
> Spin few times before going into sleep. With this approximately
> half of the wait_for in init path will take the fast path without
> sleeping.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Some numbers on how much time this saved would be nice.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_drv.h | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 488141929a7a..c225605c727c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -39,7 +39,7 @@
>  #include <drm/drm_atomic.h>
>  
>  /**
> - * _wait_for_ms - magic (register) wait macro
> + * __wait_for_ms - magic (register) wait macro
>   *
>   * Does the right thing for modeset paths when run under kdgb or similar atomic
>   * contexts. Note that it's important that we check the condition again after
> @@ -50,17 +50,22 @@
>   * drm_can_sleep() can be removed and in_atomic()/!in_atomic() asserts
>   * added.
>   */
> -#define _wait_for_ms(COND, TIMEOUT_MS, SLEEP_US) ({ \
> +#define __wait_for_ms(COND, TIMEOUT_MS, SLEEP_US, SPIN_COUNT) ({	\
>  	const unsigned long timeout__ =					\
>  		jiffies + msecs_to_jiffies(TIMEOUT_MS) + 1;		\
> +	unsigned int c__ = 0;						\
>  	int ret__ = 0;							\
> +									\
>  	while (!(COND)) {						\
>  		if (time_after(jiffies, timeout__)) {			\
>  			if (!(COND))					\
>  				ret__ = -ETIMEDOUT;			\
>  			break;						\
>  		}							\
> -		if ((SLEEP_US) && drm_can_sleep()) {			\
> +									\
> +		if (++c__ > (SPIN_COUNT) &&				\
> +		    (SLEEP_US) &&					\
> +		    drm_can_sleep()) {					\
>  			usleep_range((SLEEP_US), (SLEEP_US) * 2);	\
>  		} else {						\
>  			cpu_relax();					\
> @@ -69,7 +74,8 @@
>  	ret__;								\
>  })
>  
> -#define wait_for(COND, MS)	 _wait_for_ms((COND), (MS), 1 * USEC_PER_MSEC)
> +#define wait_for(COND, MS)  __wait_for_ms((COND), (MS), 1 * USEC_PER_MSEC, 5)
> +#define _wait_for_ms(COND, MS, US)  __wait_for_ms((COND), (MS), (US), 5)
>  
>  /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
>  #if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915: Take longer naps in wait_for

[Daniel Vetter]

On Tue, May 17, 2016 at 06:43:25PM +0300, Mika Kuoppala wrote:
> If the condition we are after doesn't happen, start to sleep
> longer and longer periods to save power. But never sleep more than 1/5th
> of the timeout value. Convert few remaining callsites to use this generic
> macro instead of letting them specifying their own sleeping periods.
> This results in only one generic wait_for across all callsites so
> we can remove the macro specifying the sleep period.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  |  4 ++--
>  drivers/gpu/drm/i915/intel_drv.h | 15 ++++++++++-----
>  drivers/gpu/drm/i915/intel_psr.c |  6 +++---
>  3 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a32617469816..b14761f585e9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1699,8 +1699,8 @@ static void wait_panel_status(struct intel_dp *intel_dp,
>  			I915_READ(pp_stat_reg),
>  			I915_READ(pp_ctrl_reg));
>  
> -	if (_wait_for_ms((I915_READ(pp_stat_reg) & mask) == value,
> -			 5 * MSEC_PER_SEC, 10 * USEC_PER_MSEC))
> +	if (wait_for((I915_READ(pp_stat_reg) & mask) == value,
> +		     5 * MSEC_PER_SEC))
>  		DRM_ERROR("Panel status timeout: status %08x control %08x\n",
>  				I915_READ(pp_stat_reg),
>  				I915_READ(pp_ctrl_reg));
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c225605c727c..1b03461e75e4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -50,7 +50,7 @@
>   * drm_can_sleep() can be removed and in_atomic()/!in_atomic() asserts
>   * added.
>   */
> -#define __wait_for_ms(COND, TIMEOUT_MS, SLEEP_US, SPIN_COUNT) ({	\
> +#define __wait_for_ms(COND, TIMEOUT_MS, SPIN_COUNT) ({			\
>  	const unsigned long timeout__ =					\
>  		jiffies + msecs_to_jiffies(TIMEOUT_MS) + 1;		\
>  	unsigned int c__ = 0;						\
> @@ -64,9 +64,15 @@
>  		}							\
>  									\
>  		if (++c__ > (SPIN_COUNT) &&				\
> -		    (SLEEP_US) &&					\
> +		    (TIMEOUT_MS) &&					\
>  		    drm_can_sleep()) {					\
> -			usleep_range((SLEEP_US), (SLEEP_US) * 2);	\
> +			/* Limit max nap to 1/4 of timeout */		\
> +			unsigned int s__ =				\
> +				clamp_val(c__ * 2 * USEC_PER_MSEC,	\
> +				    TIMEOUT_MS * 250,			\
> +				      MAX_UDELAY_MS);			\

I don't think this does what you mean, or your commit description is off.
You claim that you'll never sleep more than 1/5 of the full timeout, but
TIMEOUT_MS*250 is 1/4th, and you use it as the minimum argument. I think
what you really want here is

min(min(c__* 2 * USEC_PER_MSEC, TIMEOUT_MS * 250),
	MAX_UDELAY_MS * USEC_PER_MSEC);


Also 2 * SPIN_COUNT ms sleeping for the first time seems a bit excessive
(and is much larger than the max usleep delay on may platforms), many of
our timeouts are in the 100usec range. Imo exponential backoff works
better,
i.e.

100 << (c__ - SPIN_COUNT)

That means sleep times of 100us, 200us, 400us, 800 us ...

Or do I completely misread the math here? Maybe also split up things into
a few lines, i.e. first compute the timout, then clamp with min as needed.
-Daniel

> +									\
> +			usleep_range(s__ >> 1, s__);			\
>  		} else {						\
>  			cpu_relax();					\
>  		}							\
> @@ -74,8 +80,7 @@
>  	ret__;								\
>  })
>  
> -#define wait_for(COND, MS)  __wait_for_ms((COND), (MS), 1 * USEC_PER_MSEC, 5)
> -#define _wait_for_ms(COND, MS, US)  __wait_for_ms((COND), (MS), (US), 5)
> +#define wait_for(COND, MS)  __wait_for_ms((COND), (MS), 5)
>  
>  /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
>  #if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 0ceb2026835e..912312b79eda 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -506,9 +506,9 @@ static void hsw_psr_disable(struct intel_dp *intel_dp)
>  			   I915_READ(EDP_PSR_CTL) & ~EDP_PSR_ENABLE);
>  
>  		/* Wait till PSR is idle */
> -		if (_wait_for_ms((I915_READ(EDP_PSR_STATUS_CTL) &
> -				  EDP_PSR_STATUS_STATE_MASK) == 0,
> -				 2 * MSEC_PER_SEC, 10 * USEC_PER_MSEC))
> +		if (wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
> +			      EDP_PSR_STATUS_STATE_MASK) == 0,
> +			     2 * MSEC_PER_SEC))
>  			DRM_ERROR("Timed out waiting for PSR Idle State\n");
>  
>  		dev_priv->psr.active = false;
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: Introduce wait_until_reg

[Daniel Vetter]

On Tue, May 17, 2016 at 06:43:26PM +0300, Mika Kuoppala wrote:
> The most common usage pattern for wait_for() macro is to wait for some
> register value. Instead of bloating all callsites, encapsulate this
> complex wait_for macro into a helper function.
> 
> v2: s/for/until, macro readability (Chris)
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     | 25 +++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_uncore.c |  9 +++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 72f0b02a8372..ccf6747894b1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2873,6 +2873,31 @@ static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv)
>  	return dev_priv->vgpu.active;
>  }
>  
> +int intel_wait_until_register(struct drm_i915_private *dev_priv,
> +			      i915_reg_t reg,
> +			      u32 mask,
> +			      u32 value,
> +			      unsigned long timeout_ms);
> +
> +#define wait_until_reg(reg, mask, value, timeout_ms) ({ \
> +	if (__builtin_constant_p(timeout_ms)) \
> +		BUILD_BUG_ON((timeout_ms) > 60000); \

Why the inconsistent use of __builtin_constant_p? Also the BUILD_BUG_ON
seems not that helpful ...

> +	intel_wait_until_register(dev_priv, \
> +				  (reg), (mask), (value), (timeout_ms)); \
> +	})
> +
> +#define wait_until_reg_set(reg, v, timeout_ms) ({ \
> +	if (__builtin_constant_p(v)) \
> +		BUILD_BUG_ON_NOT_POWER_OF_2((u32)(v)); \

Sometimes you might want to wait for multiple flags to get set. Not sure
this BUG_ON is valid. Same below. But I do see that very often you want to
have explicit value/mask in such a case, so seems ok.

Naming and all that makes sense.

> +	wait_until_reg(reg, v, v, timeout_ms); \
> +	})
> +
> +#define wait_until_reg_clr(reg, v, timeout_ms) ({ \
> +	if (__builtin_constant_p((u32)(v))) \
> +		BUILD_BUG_ON_NOT_POWER_OF_2((u32)(v)); \
> +	wait_until_reg(reg, v, 0, timeout_ms); \
> +	})
> +
>  void
>  i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>  		     u32 status_mask);
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 385114bca924..e217997a9fa0 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1860,3 +1860,12 @@ intel_uncore_forcewake_for_reg(struct drm_i915_private *dev_priv,
>  
>  	return fw_domains;
>  }
> +
> +int intel_wait_until_register(struct drm_i915_private *dev_priv,
> +			      i915_reg_t reg,
> +			      u32 mask,
> +			      u32 value,
> +			      unsigned long timeout_ms)
> +{
> +	return wait_for((I915_READ(reg) & mask) == value, timeout_ms);
> +}
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/7] drm/i915: Use wait_until_reg macros

[Daniel Vetter]

On Tue, May 17, 2016 at 06:43:27PM +0300, Mika Kuoppala wrote:
> Use wait_until_reg instead of generic wait_for macro on
> the relevant callsites.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Quick numbers on how much this shrinks object size would be neat.

I checked all the callers and didn't spot a mixup. Might have been
something cocci would have been better at, but there's a few cases so
would be some work to get them all.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_drv.c         | 19 ++++++-----------
>  drivers/gpu/drm/i915/i915_reg.h         |  2 +-
>  drivers/gpu/drm/i915/intel_crt.c        | 15 +++++++------
>  drivers/gpu/drm/i915/intel_ddi.c        |  8 ++++---
>  drivers/gpu/drm/i915/intel_display.c    | 38 +++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_dp.c         |  6 ++----
>  drivers/gpu/drm/i915/intel_dp_mst.c     |  3 +--
>  drivers/gpu/drm/i915/intel_dpll_mgr.c   |  2 +-
>  drivers/gpu/drm/i915/intel_dsi.c        | 19 +++++++----------
>  drivers/gpu/drm/i915/intel_dsi_pll.c    |  5 ++---
>  drivers/gpu/drm/i915/intel_fbc.c        |  3 +--
>  drivers/gpu/drm/i915/intel_lrc.c        |  3 ++-
>  drivers/gpu/drm/i915/intel_lvds.c       |  4 ++--
>  drivers/gpu/drm/i915/intel_pm.c         |  6 ++----
>  drivers/gpu/drm/i915/intel_psr.c        | 19 +++++++++--------
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 12 +++++------
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 25 ++++++++++++----------
>  drivers/gpu/drm/i915/intel_sideband.c   | 20 ++++++++---------
>  drivers/gpu/drm/i915/intel_uncore.c     |  6 ++----
>  19 files changed, 100 insertions(+), 115 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index dba03c026151..4bd40ef3c9b8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1357,8 +1357,6 @@ int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
>  	u32 val;
>  	int err;
>  
> -#define COND (I915_READ(VLV_GTLC_SURVIVABILITY_REG) & VLV_GFX_CLK_STATUS_BIT)
> -
>  	val = I915_READ(VLV_GTLC_SURVIVABILITY_REG);
>  	val &= ~VLV_GFX_CLK_FORCE_ON_BIT;
>  	if (force_on)
> @@ -1368,13 +1366,13 @@ int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
>  	if (!force_on)
>  		return 0;
>  
> -	err = wait_for(COND, 20);
> +	err = wait_until_reg_set(VLV_GTLC_SURVIVABILITY_REG,
> +				 VLV_GFX_CLK_STATUS_BIT, 20);
>  	if (err)
>  		DRM_ERROR("timeout waiting for GFX clock force-on (%08x)\n",
>  			  I915_READ(VLV_GTLC_SURVIVABILITY_REG));
>  
>  	return err;
> -#undef COND
>  }
>  
>  static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow)
> @@ -1389,13 +1387,12 @@ static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow)
>  	I915_WRITE(VLV_GTLC_WAKE_CTRL, val);
>  	POSTING_READ(VLV_GTLC_WAKE_CTRL);
>  
> -#define COND (!!(I915_READ(VLV_GTLC_PW_STATUS) & VLV_GTLC_ALLOWWAKEACK) == \
> -	      allow)
> -	err = wait_for(COND, 1);
> +	err = wait_until_reg(VLV_GTLC_PW_STATUS,
> +			     VLV_GTLC_ALLOWWAKEACK, allow, 1);
>  	if (err)
>  		DRM_ERROR("timeout disabling GT waking\n");
> +
>  	return err;
> -#undef COND
>  }
>  
>  static int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv,
> @@ -1407,9 +1404,6 @@ static int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv,
>  
>  	mask = VLV_GTLC_PW_MEDIA_STATUS_MASK | VLV_GTLC_PW_RENDER_STATUS_MASK;
>  	val = wait_for_on ? mask : 0;
> -#define COND ((I915_READ(VLV_GTLC_PW_STATUS) & mask) == val)
> -	if (COND)
> -		return 0;
>  
>  	DRM_DEBUG_KMS("waiting for GT wells to go %s (%08x)\n",
>  		      onoff(wait_for_on),
> @@ -1419,13 +1413,12 @@ static int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv,
>  	 * RC6 transitioning can be delayed up to 2 msec (see
>  	 * valleyview_enable_rps), use 3 msec for safety.
>  	 */
> -	err = wait_for(COND, 3);
> +	err = wait_until_reg(VLV_GTLC_PW_STATUS, mask, val, 3);
>  	if (err)
>  		DRM_ERROR("timeout waiting for GT wells to go %s\n",
>  			  onoff(wait_for_on));
>  
>  	return err;
> -#undef COND
>  }
>  
>  static void vlv_check_no_gt_access(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 86fbf723eca7..5355d5272ad1 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2222,7 +2222,7 @@ enum skl_disp_power_wells {
>  #define IVB_FBC_RT_BASE			_MMIO(0x7020)
>  
>  #define IPS_CTL		_MMIO(0x43408)
> -#define   IPS_ENABLE	(1 << 31)
> +#define   IPS_ENABLE	(1ULL << 31)
>  
>  #define MSG_FBC_REND_STATE	_MMIO(0x50380)
>  #define   FBC_REND_NUKE		(1<<2)
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 3fbb6fc66451..c1c619505393 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -301,8 +301,8 @@ static bool intel_ironlake_crt_detect_hotplug(struct drm_connector *connector)
>  
>  		I915_WRITE(crt->adpa_reg, adpa);
>  
> -		if (wait_for((I915_READ(crt->adpa_reg) & ADPA_CRT_HOTPLUG_FORCE_TRIGGER) == 0,
> -			     1000))
> +		if (wait_until_reg_clr(crt->adpa_reg,
> +				       ADPA_CRT_HOTPLUG_FORCE_TRIGGER, 1000))
>  			DRM_DEBUG_KMS("timed out waiting for FORCE_TRIGGER");
>  
>  		if (turn_off_dac) {
> @@ -338,8 +338,9 @@ static bool valleyview_crt_detect_hotplug(struct drm_connector *connector)
>  
>  	I915_WRITE(crt->adpa_reg, adpa);
>  
> -	if (wait_for((I915_READ(crt->adpa_reg) & ADPA_CRT_HOTPLUG_FORCE_TRIGGER) == 0,
> -		     1000)) {
> +	if (wait_until_reg_clr(crt->adpa_reg,
> +			       ADPA_CRT_HOTPLUG_FORCE_TRIGGER,
> +			       1000)) {
>  		DRM_DEBUG_KMS("timed out waiting for FORCE_TRIGGER");
>  		I915_WRITE(crt->adpa_reg, save_adpa);
>  	}
> @@ -394,9 +395,9 @@ static bool intel_crt_detect_hotplug(struct drm_connector *connector)
>  					      CRT_HOTPLUG_FORCE_DETECT,
>  					      CRT_HOTPLUG_FORCE_DETECT);
>  		/* wait for FORCE_DETECT to go off */
> -		if (wait_for((I915_READ(PORT_HOTPLUG_EN) &
> -			      CRT_HOTPLUG_FORCE_DETECT) == 0,
> -			     1000))
> +		if (wait_until_reg_clr(PORT_HOTPLUG_EN,
> +				       CRT_HOTPLUG_FORCE_DETECT,
> +				       1000))
>  			DRM_DEBUG_KMS("timed out waiting for FORCE_DETECT to go off");
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index a62c60c6fb0b..7234af90e0e7 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1783,7 +1783,7 @@ static u32 broxton_get_grc(struct drm_i915_private *dev_priv, enum dpio_phy phy)
>  static void broxton_phy_wait_grc_done(struct drm_i915_private *dev_priv,
>  				      enum dpio_phy phy)
>  {
> -	if (wait_for(I915_READ(BXT_PORT_REF_DW3(phy)) & GRC_DONE, 10))
> +	if (wait_until_reg_set(BXT_PORT_REF_DW3(phy), GRC_DONE, 10))
>  		DRM_ERROR("timeout waiting for PHY%d GRC\n", phy);
>  }
>  
> @@ -1826,8 +1826,9 @@ static void broxton_phy_init(struct drm_i915_private *dev_priv,
>  	 * HW team confirmed that the time to reach phypowergood status is
>  	 * anywhere between 50 us and 100us.
>  	 */
> -	if (wait_for(((I915_READ(BXT_PORT_CL1CM_DW0(phy)) &
> -		(PHY_RESERVED | PHY_POWER_GOOD)) == PHY_POWER_GOOD), 1)) {
> +	if (wait_until_reg(BXT_PORT_CL1CM_DW0(phy),
> +			   (PHY_RESERVED | PHY_POWER_GOOD),
> +			   PHY_POWER_GOOD, 1)) {
>  		DRM_ERROR("timeout during PHY%d power on\n", phy);
>  	}
>  
> @@ -1899,6 +1900,7 @@ static void broxton_phy_init(struct drm_i915_private *dev_priv,
>  		 * the corresponding calibrated value from PHY1, and disable
>  		 * the automatic calibration on PHY0.
>  		 */
> +
>  		broxton_phy_wait_grc_done(dev_priv, DPIO_PHY1);
>  
>  		val = dev_priv->bxt_phy_grc = broxton_get_grc(dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 05d4a2b365f8..b95519642e94 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1118,8 +1118,7 @@ static void intel_wait_for_pipe_off(struct intel_crtc *crtc)
>  		i915_reg_t reg = PIPECONF(cpu_transcoder);
>  
>  		/* Wait for the Pipe State to go off */
> -		if (wait_for((I915_READ(reg) & I965_PIPECONF_ACTIVE) == 0,
> -			     100))
> +		if (wait_until_reg_clr(reg, I965_PIPECONF_ACTIVE, 100))
>  			WARN(1, "pipe_off wait timed out\n");
>  	} else {
>  		/* Wait for the display line to settle */
> @@ -1538,7 +1537,7 @@ static void _vlv_enable_pll(struct intel_crtc *crtc,
>  	POSTING_READ(DPLL(pipe));
>  	udelay(150);
>  
> -	if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
> +	if (wait_until_reg_set(DPLL(pipe), DPLL_LOCK_VLV, 1))
>  		DRM_ERROR("DPLL %d failed to lock\n", pipe);
>  }
>  
> @@ -1587,7 +1586,7 @@ static void _chv_enable_pll(struct intel_crtc *crtc,
>  	I915_WRITE(DPLL(pipe), pipe_config->dpll_hw_state.dpll);
>  
>  	/* Check PLL is locked */
> -	if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
> +	if (wait_until_reg_set(DPLL(pipe), DPLL_LOCK_VLV, 1))
>  		DRM_ERROR("PLL %d failed to lock\n", pipe);
>  }
>  
> @@ -1807,7 +1806,7 @@ void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
>  		BUG();
>  	}
>  
> -	if (wait_for((I915_READ(dpll_reg) & port_mask) == expected_mask, 1000))
> +	if (wait_until_reg(dpll_reg, port_mask, expected_mask, 1000))
>  		WARN(1, "timed out waiting for port %c ready: got 0x%x, expected 0x%x\n",
>  		     port_name(dport->port), I915_READ(dpll_reg) & port_mask, expected_mask);
>  }
> @@ -1865,7 +1864,7 @@ static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
>  		val |= TRANS_PROGRESSIVE;
>  
>  	I915_WRITE(reg, val | TRANS_ENABLE);
> -	if (wait_for(I915_READ(reg) & TRANS_STATE_ENABLE, 100))
> +	if (wait_until_reg_set(reg, TRANS_STATE_ENABLE, 100))
>  		DRM_ERROR("failed to enable transcoder %c\n", pipe_name(pipe));
>  }
>  
> @@ -1893,7 +1892,7 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv,
>  		val |= TRANS_PROGRESSIVE;
>  
>  	I915_WRITE(LPT_TRANSCONF, val);
> -	if (wait_for(I915_READ(LPT_TRANSCONF) & TRANS_STATE_ENABLE, 100))
> +	if (wait_until_reg_set(LPT_TRANSCONF, TRANS_STATE_ENABLE, 100))
>  		DRM_ERROR("Failed to enable PCH transcoder\n");
>  }
>  
> @@ -1916,7 +1915,7 @@ static void ironlake_disable_pch_transcoder(struct drm_i915_private *dev_priv,
>  	val &= ~TRANS_ENABLE;
>  	I915_WRITE(reg, val);
>  	/* wait for PCH transcoder off, transcoder state */
> -	if (wait_for((I915_READ(reg) & TRANS_STATE_ENABLE) == 0, 50))
> +	if (wait_until_reg_clr(reg, TRANS_STATE_ENABLE, 50))
>  		DRM_ERROR("failed to disable transcoder %c\n", pipe_name(pipe));
>  
>  	if (HAS_PCH_CPT(dev)) {
> @@ -1936,7 +1935,7 @@ static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
>  	val &= ~TRANS_ENABLE;
>  	I915_WRITE(LPT_TRANSCONF, val);
>  	/* wait for PCH transcoder off, transcoder state */
> -	if (wait_for((I915_READ(LPT_TRANSCONF) & TRANS_STATE_ENABLE) == 0, 50))
> +	if (wait_until_reg_clr(LPT_TRANSCONF, TRANS_STATE_ENABLE, 50))
>  		DRM_ERROR("Failed to disable PCH transcoder\n");
>  
>  	/* Workaround: clear timing override bit. */
> @@ -4461,7 +4460,7 @@ void hsw_disable_ips(struct intel_crtc *crtc)
>  		WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, 0));
>  		mutex_unlock(&dev_priv->rps.hw_lock);
>  		/* wait for pcode to finish disabling IPS, which may take up to 42ms */
> -		if (wait_for((I915_READ(IPS_CTL) & IPS_ENABLE) == 0, 42))
> +		if (wait_until_reg_clr(IPS_CTL, IPS_ENABLE, 42))
>  			DRM_ERROR("Timed out waiting for IPS disable\n");
>  	} else {
>  		I915_WRITE(IPS_CTL, 0);
> @@ -5411,8 +5410,7 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
>  	    current_cdclk == 624000) {
>  		I915_WRITE(BXT_DE_PLL_ENABLE, ~BXT_DE_PLL_PLL_ENABLE);
>  		/* Timeout 200us */
> -		if (wait_for(!(I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK),
> -			     1))
> +		if (wait_until_reg_clr(BXT_DE_PLL_ENABLE, BXT_DE_PLL_LOCK, 1))
>  			DRM_ERROR("timout waiting for DE PLL unlock\n");
>  	}
>  
> @@ -5426,7 +5424,7 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
>  
>  		I915_WRITE(BXT_DE_PLL_ENABLE, BXT_DE_PLL_PLL_ENABLE);
>  		/* Timeout 200us */
> -		if (wait_for(I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK, 1))
> +		if (wait_until_reg_set(BXT_DE_PLL_ENABLE, BXT_DE_PLL_LOCK, 1))
>  			DRM_ERROR("timeout waiting for DE PLL lock\n");
>  
>  		val = divider | skl_cdclk_decimal(cdclk);
> @@ -5596,7 +5594,7 @@ skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco)
>  
>  	I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) | LCPLL_PLL_ENABLE);
>  
> -	if (wait_for(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK, 5))
> +	if (wait_until_reg_set(LCPLL1_CTL, LCPLL_PLL_LOCK, 5))
>  		DRM_ERROR("DPLL0 not locked\n");
>  }
>  
> @@ -5604,7 +5602,7 @@ static void
>  skl_dpll0_disable(struct drm_i915_private *dev_priv)
>  {
>  	I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) & ~LCPLL_PLL_ENABLE);
> -	if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
> +	if (wait_until_reg_clr(LCPLL1_CTL, LCPLL_PLL_LOCK, 1))
>  		DRM_ERROR("Couldn't disable DPLL0\n");
>  }
>  
> @@ -9415,7 +9413,7 @@ static void hsw_disable_lcpll(struct drm_i915_private *dev_priv,
>  	I915_WRITE(LCPLL_CTL, val);
>  	POSTING_READ(LCPLL_CTL);
>  
> -	if (wait_for((I915_READ(LCPLL_CTL) & LCPLL_PLL_LOCK) == 0, 1))
> +	if (wait_until_reg_clr(LCPLL_CTL, LCPLL_PLL_LOCK, 1))
>  		DRM_ERROR("LCPLL still locked\n");
>  
>  	val = hsw_read_dcomp(dev_priv);
> @@ -9470,7 +9468,7 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
>  	val &= ~LCPLL_PLL_DISABLE;
>  	I915_WRITE(LCPLL_CTL, val);
>  
> -	if (wait_for(I915_READ(LCPLL_CTL) & LCPLL_PLL_LOCK, 5))
> +	if (wait_until_reg_set(LCPLL_CTL, LCPLL_PLL_LOCK, 5))
>  		DRM_ERROR("LCPLL not locked yet\n");
>  
>  	if (val & LCPLL_CD_SOURCE_FCLK) {
> @@ -9619,8 +9617,7 @@ static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
>  	val |= LCPLL_CD_SOURCE_FCLK;
>  	I915_WRITE(LCPLL_CTL, val);
>  
> -	if (wait_for(I915_READ(LCPLL_CTL) &
> -		     LCPLL_CD_SOURCE_FCLK_DONE, 1))
> +	if (wait_until_reg_set(LCPLL_CTL, LCPLL_CD_SOURCE_FCLK_DONE, 1))
>  		DRM_ERROR("Switching to FCLK failed\n");
>  
>  	val = I915_READ(LCPLL_CTL);
> @@ -9654,8 +9651,7 @@ static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
>  	val &= ~LCPLL_CD_SOURCE_FCLK;
>  	I915_WRITE(LCPLL_CTL, val);
>  
> -	if (wait_for((I915_READ(LCPLL_CTL) &
> -		      LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
> +	if (wait_until_reg_clr(LCPLL_CTL, LCPLL_CD_SOURCE_FCLK_DONE, 1))
>  		DRM_ERROR("Switching back to LCPLL failed\n");
>  
>  	mutex_lock(&dev_priv->rps.hw_lock);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b14761f585e9..f32e5d0a971e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1699,8 +1699,7 @@ static void wait_panel_status(struct intel_dp *intel_dp,
>  			I915_READ(pp_stat_reg),
>  			I915_READ(pp_ctrl_reg));
>  
> -	if (wait_for((I915_READ(pp_stat_reg) & mask) == value,
> -		     5 * MSEC_PER_SEC))
> +	if (wait_until_reg(pp_stat_reg, mask, value, 5 * MSEC_PER_SEC))
>  		DRM_ERROR("Panel status timeout: status %08x control %08x\n",
>  				I915_READ(pp_stat_reg),
>  				I915_READ(pp_ctrl_reg));
> @@ -3252,8 +3251,7 @@ void intel_dp_set_idle_link_train(struct intel_dp *intel_dp)
>  	if (port == PORT_A)
>  		return;
>  
> -	if (wait_for((I915_READ(DP_TP_STATUS(port)) & DP_TP_STATUS_IDLE_DONE),
> -		     1))
> +	if (wait_until_reg_set(DP_TP_STATUS(port), DP_TP_STATUS_IDLE_DONE, 1))
>  		DRM_ERROR("Timed out waiting for DP idle patterns\n");
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 7a34090cef34..8a066dd670c5 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -213,8 +213,7 @@ static void intel_mst_enable_dp(struct intel_encoder *encoder)
>  
>  	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
>  
> -	if (wait_for((I915_READ(DP_TP_STATUS(port)) & DP_TP_STATUS_ACT_SENT),
> -		     1))
> +	if (wait_until_reg_set(DP_TP_STATUS(port), DP_TP_STATUS_ACT_SENT, 1))
>  		DRM_ERROR("Timed out waiting for ACT sent\n");
>  
>  	ret = drm_dp_check_act_status(&intel_dp->mst_mgr);
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index c283ba4babe8..9699cbcdef19 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -853,7 +853,7 @@ static void skl_ddi_pll_enable(struct drm_i915_private *dev_priv,
>  	I915_WRITE(regs[pll->id].ctl,
>  		   I915_READ(regs[pll->id].ctl) | LCPLL_PLL_ENABLE);
>  
> -	if (wait_for(I915_READ(DPLL_STATUS) & DPLL_LOCK(pll->id), 5))
> +	if (wait_until_reg_set(DPLL_STATUS, DPLL_LOCK(pll->id), 5))
>  		DRM_ERROR("DPLL %d not locked\n", pll->id);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 4009618a5b34..60d2f73d68d2 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -90,7 +90,7 @@ static void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
>  	mask = LP_CTRL_FIFO_EMPTY | HS_CTRL_FIFO_EMPTY |
>  		LP_DATA_FIFO_EMPTY | HS_DATA_FIFO_EMPTY;
>  
> -	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & mask) == mask, 100))
> +	if (wait_until_reg(MIPI_GEN_FIFO_STAT(port), mask, mask, 100))
>  		DRM_ERROR("DPI FIFOs are not empty\n");
>  }
>  
> @@ -159,7 +159,7 @@ static ssize_t intel_dsi_host_transfer(struct mipi_dsi_host *host,
>  	/* note: this is never true for reads */
>  	if (packet.payload_length) {
>  
> -		if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & data_mask) == 0, 50))
> +		if (wait_until_reg_clr(MIPI_GEN_FIFO_STAT(port), data_mask, 50))
>  			DRM_ERROR("Timeout waiting for HS/LP DATA FIFO !full\n");
>  
>  		write_data(dev_priv, data_reg, packet.payload,
> @@ -170,16 +170,15 @@ static ssize_t intel_dsi_host_transfer(struct mipi_dsi_host *host,
>  		I915_WRITE(MIPI_INTR_STAT(port), GEN_READ_DATA_AVAIL);
>  	}
>  
> -	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & ctrl_mask) == 0, 50)) {
> +	if (wait_until_reg_clr(MIPI_GEN_FIFO_STAT(port), ctrl_mask, 50))
>  		DRM_ERROR("Timeout waiting for HS/LP CTRL FIFO !full\n");
> -	}
>  
>  	I915_WRITE(ctrl_reg, header[2] << 16 | header[1] << 8 | header[0]);
>  
>  	/* ->rx_len is set only for reads */
>  	if (msg->rx_len) {
> -		data_mask = GEN_READ_DATA_AVAIL;
> -		if (wait_for((I915_READ(MIPI_INTR_STAT(port)) & data_mask) == data_mask, 50))
> +		if (wait_until_reg_set(MIPI_INTR_STAT(port),
> +				       GEN_READ_DATA_AVAIL, 50))
>  			DRM_ERROR("Timeout waiting for read data.\n");
>  
>  		read_data(dev_priv, data_reg, msg->rx_buf, msg->rx_len);
> @@ -251,7 +250,6 @@ static int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs,
>  	struct drm_encoder *encoder = &intel_dsi->base.base;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 mask;
>  
>  	/* XXX: pipe, hs */
>  	if (hs)
> @@ -268,8 +266,8 @@ static int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs,
>  
>  	I915_WRITE(MIPI_DPI_CONTROL(port), cmd);
>  
> -	mask = SPL_PKT_SENT_INTERRUPT;
> -	if (wait_for((I915_READ(MIPI_INTR_STAT(port)) & mask) == mask, 100))
> +	if (wait_until_reg_set(MIPI_INTR_STAT(port), SPL_PKT_SENT_INTERRUPT,
> +			       100))
>  		DRM_ERROR("Video mode command 0x%08x send failed.\n", cmd);
>  
>  	return 0;
> @@ -667,8 +665,7 @@ static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
>  		/* Wait till Clock lanes are in LP-00 state for MIPI Port A
>  		 * only. MIPI Port C has no similar bit for checking
>  		 */
> -		if (wait_for(((I915_READ(port_ctrl) & AFE_LATCHOUT)
> -						== 0x00000), 30))
> +		if (wait_until_reg_clr(port_ctrl, AFE_LATCHOUT, 30))
>  			DRM_ERROR("DSI LP not going Low\n");
>  
>  		/* Disable MIPI PHY transparent latch */
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index 1765e6e18f2c..2feba836ee39 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -234,8 +234,7 @@ static void bxt_disable_dsi_pll(struct intel_encoder *encoder)
>  	 * PLL lock should deassert within 200us.
>  	 * Wait up to 1ms before timing out.
>  	 */
> -	if (wait_for((I915_READ(BXT_DSI_PLL_ENABLE)
> -					& BXT_DSI_PLL_LOCKED) == 0, 1))
> +	if (wait_until_reg_clr(BXT_DSI_PLL_ENABLE, BXT_DSI_PLL_LOCKED, 1))
>  		DRM_ERROR("Timeout waiting for PLL lock deassertion\n");
>  }
>  
> @@ -486,7 +485,7 @@ static void bxt_enable_dsi_pll(struct intel_encoder *encoder,
>  	I915_WRITE(BXT_DSI_PLL_ENABLE, val);
>  
>  	/* Timeout and fail if PLL not locked */
> -	if (wait_for(I915_READ(BXT_DSI_PLL_ENABLE) & BXT_DSI_PLL_LOCKED, 1)) {
> +	if (wait_until_reg_set(BXT_DSI_PLL_ENABLE, BXT_DSI_PLL_LOCKED, 1)) {
>  		DRM_ERROR("Timed out waiting for DSI PLL to lock\n");
>  		return;
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 0dea5fbcd8aa..19ca799f4793 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -123,8 +123,7 @@ static void i8xx_fbc_deactivate(struct drm_i915_private *dev_priv)
>  	fbc_ctl &= ~FBC_CTL_EN;
>  	I915_WRITE(FBC_CONTROL, fbc_ctl);
>  
> -	/* Wait for compressing bit to clear */
> -	if (wait_for((I915_READ(FBC_STATUS) & FBC_STAT_COMPRESSING) == 0, 10)) {
> +	if (wait_until_reg_clr(FBC_STATUS, FBC_STAT_COMPRESSING, 10)) {
>  		DRM_DEBUG_KMS("FBC idle timed out\n");
>  		return;
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index db10c961e0f4..62ae2e938085 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -911,7 +911,8 @@ void intel_logical_ring_stop(struct intel_engine_cs *engine)
>  
>  	/* TODO: Is this correct with Execlists enabled? */
>  	I915_WRITE_MODE(engine, _MASKED_BIT_ENABLE(STOP_RING));
> -	if (wait_for((I915_READ_MODE(engine) & MODE_IDLE) != 0, 1000)) {
> +	if (wait_until_reg_set(RING_MI_MODE(engine->mmio_base), MODE_IDLE,
> +			       1000)) {
>  		DRM_ERROR("%s :timed out trying to stop ring\n", engine->name);
>  		return;
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index d65fd945607a..8754f8bf5e00 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -231,7 +231,7 @@ static void intel_enable_lvds(struct intel_encoder *encoder)
>  
>  	I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
>  	POSTING_READ(lvds_encoder->reg);
> -	if (wait_for((I915_READ(stat_reg) & PP_ON) != 0, 1000))
> +	if (wait_until_reg_set(stat_reg, PP_ON, 1000))
>  		DRM_ERROR("timed out waiting for panel to power on\n");
>  
>  	intel_panel_enable_backlight(intel_connector);
> @@ -253,7 +253,7 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
>  	}
>  
>  	I915_WRITE(ctl_reg, I915_READ(ctl_reg) & ~POWER_TARGET_ON);
> -	if (wait_for((I915_READ(stat_reg) & PP_ON) == 0, 1000))
> +	if (wait_until_reg_clr(stat_reg, PP_ON, 1000))
>  		DRM_ERROR("timed out waiting for panel to power off\n");
>  
>  	I915_WRITE(lvds_encoder->reg, I915_READ(lvds_encoder->reg) & ~LVDS_PORT_EN);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index adb64638f595..e72890210a6e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -7431,8 +7431,7 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u32 mbox, u32 *val
>  	I915_WRITE(GEN6_PCODE_DATA1, 0);
>  	I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
>  
> -	if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
> -		     500)) {
> +	if (wait_until_reg_clr(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY, 500)) {
>  		DRM_ERROR("timeout waiting for pcode read (%d) to finish\n", mbox);
>  		return -ETIMEDOUT;
>  	}
> @@ -7455,8 +7454,7 @@ int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u32 mbox, u32 val
>  	I915_WRITE(GEN6_PCODE_DATA, val);
>  	I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
>  
> -	if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
> -		     500)) {
> +	if (wait_until_reg_clr(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY, 500)) {
>  		DRM_ERROR("timeout waiting for pcode write (%d) to finish\n", mbox);
>  		return -ETIMEDOUT;
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 912312b79eda..bb1ab0d13ee6 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -479,8 +479,8 @@ static void vlv_psr_disable(struct intel_dp *intel_dp)
>  
>  	if (dev_priv->psr.active) {
>  		/* Put VLV PSR back to PSR_state 0 that is PSR Disabled. */
> -		if (wait_for((I915_READ(VLV_PSRSTAT(intel_crtc->pipe)) &
> -			      VLV_EDP_PSR_IN_TRANS) == 0, 1))
> +		if (wait_until_reg_clr(VLV_PSRSTAT(intel_crtc->pipe),
> +				       VLV_EDP_PSR_IN_TRANS, 1))
>  			WARN(1, "PSR transition took longer than expected\n");
>  
>  		val = I915_READ(VLV_PSRCTL(intel_crtc->pipe));
> @@ -506,9 +506,9 @@ static void hsw_psr_disable(struct intel_dp *intel_dp)
>  			   I915_READ(EDP_PSR_CTL) & ~EDP_PSR_ENABLE);
>  
>  		/* Wait till PSR is idle */
> -		if (wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
> -			      EDP_PSR_STATUS_STATE_MASK) == 0,
> -			     2 * MSEC_PER_SEC))
> +		if (wait_until_reg(EDP_PSR_STATUS_CTL,
> +				   EDP_PSR_STATUS_STATE_MASK, 0,
> +				   2 * MSEC_PER_SEC))
>  			DRM_ERROR("Timed out waiting for PSR Idle State\n");
>  
>  		dev_priv->psr.active = false;
> @@ -563,15 +563,16 @@ static void intel_psr_work(struct work_struct *work)
>  	 * PSR might take some time to get fully disabled
>  	 * and be ready for re-enable.
>  	 */
> +
>  	if (HAS_DDI(dev_priv)) {
> -		if (wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
> -			      EDP_PSR_STATUS_STATE_MASK) == 0, 50)) {
> +		if (wait_until_reg(EDP_PSR_STATUS_CTL,
> +				   EDP_PSR_STATUS_STATE_MASK, 0, 50)) {
>  			DRM_ERROR("Timed out waiting for PSR Idle for re-enable\n");
>  			return;
>  		}
>  	} else {
> -		if (wait_for((I915_READ(VLV_PSRSTAT(pipe)) &
> -			      VLV_EDP_PSR_IN_TRANS) == 0, 1)) {
> +		if (wait_until_reg_clr(VLV_PSRSTAT(pipe),
> +				       VLV_EDP_PSR_IN_TRANS, 1)) {
>  			DRM_ERROR("Timed out waiting for PSR Idle for re-enable\n");
>  			return;
>  		}
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 8d35a3978f9b..46702594f93b 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -515,8 +515,7 @@ static void intel_ring_setup_status_page(struct intel_engine_cs *engine)
>  		I915_WRITE(reg,
>  			   _MASKED_BIT_ENABLE(INSTPM_TLB_INVALIDATE |
>  					      INSTPM_SYNC_FLUSH));
> -		if (wait_for((I915_READ(reg) & INSTPM_SYNC_FLUSH) == 0,
> -			     1000))
> +		if (wait_until_reg_clr(reg, INSTPM_SYNC_FLUSH, 1000))
>  			DRM_ERROR("%s: wait for SyncFlush to complete for TLB invalidation timed out\n",
>  				  engine->name);
>  	}
> @@ -528,7 +527,8 @@ static bool stop_ring(struct intel_engine_cs *engine)
>  
>  	if (!IS_GEN2(dev_priv)) {
>  		I915_WRITE_MODE(engine, _MASKED_BIT_ENABLE(STOP_RING));
> -		if (wait_for((I915_READ_MODE(engine) & MODE_IDLE) != 0, 1000)) {
> +		if (wait_until_reg_set(RING_MI_MODE(engine->mmio_base),
> +				       MODE_IDLE, 1000)) {
>  			DRM_ERROR("%s : timed out trying to stop ring\n",
>  				  engine->name);
>  			/* Sometimes we observe that the idle flag is not
> @@ -2545,9 +2545,9 @@ static void gen6_bsd_ring_write_tail(struct intel_engine_cs *engine,
>  	I915_WRITE64(GEN6_BSD_RNCID, 0x0);
>  
>  	/* Wait for the ring not to be idle, i.e. for it to wake up. */
> -	if (wait_for((I915_READ(GEN6_BSD_SLEEP_PSMI_CONTROL) &
> -		      GEN6_BSD_SLEEP_INDICATOR) == 0,
> -		     50))
> +	if (wait_until_reg_clr(GEN6_BSD_SLEEP_PSMI_CONTROL,
> +			       GEN6_BSD_SLEEP_INDICATOR,
> +			       50))
>  		DRM_ERROR("timed out waiting for the BSD ring to wake up\n");
>  
>  	/* Now that the ring is fully powered up, update the tail */
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index b69b935516fb..a11342c9ddd4 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -345,8 +345,8 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  
>  		if (!is_enabled) {
>  			DRM_DEBUG_KMS("Enabling power well\n");
> -			if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) &
> -				      HSW_PWR_WELL_STATE_ENABLED), 20))
> +			if (wait_until_reg_set(HSW_PWR_WELL_DRIVER,
> +					       HSW_PWR_WELL_STATE_ENABLED, 20))
>  				DRM_ERROR("Timeout enabling power well\n");
>  			hsw_power_well_post_enable(dev_priv);
>  		}
> @@ -669,8 +669,8 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  
>  	switch (power_well->data) {
>  	case SKL_DISP_PW_1:
> -		if (wait_for((I915_READ(SKL_FUSE_STATUS) &
> -			SKL_FUSE_PG0_DIST_STATUS), 1)) {
> +		if (wait_until_reg_set(SKL_FUSE_STATUS,
> +				       SKL_FUSE_PG0_DIST_STATUS, 1)) {
>  			DRM_ERROR("PG0 not enabled\n");
>  			return;
>  		}
> @@ -731,12 +731,12 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  
>  	if (check_fuse_status) {
>  		if (power_well->data == SKL_DISP_PW_1) {
> -			if (wait_for((I915_READ(SKL_FUSE_STATUS) &
> -				SKL_FUSE_PG1_DIST_STATUS), 1))
> +			if (wait_until_reg_set(SKL_FUSE_STATUS,
> +					       SKL_FUSE_PG1_DIST_STATUS, 1))
>  				DRM_ERROR("PG1 distributing status timeout\n");
>  		} else if (power_well->data == SKL_DISP_PW_2) {
> -			if (wait_for((I915_READ(SKL_FUSE_STATUS) &
> -				SKL_FUSE_PG2_DIST_STATUS), 1))
> +			if (wait_until_reg_set(SKL_FUSE_STATUS,
> +					       SKL_FUSE_PG2_DIST_STATUS, 1))
>  				DRM_ERROR("PG2 distributing status timeout\n");
>  		}
>  	}
> @@ -1097,7 +1097,6 @@ static void assert_chv_phy_status(struct drm_i915_private *dev_priv)
>  	u32 phy_control = dev_priv->chv_phy_control;
>  	u32 phy_status = 0;
>  	u32 phy_status_mask = 0xffffffff;
> -	u32 tmp;
>  
>  	/*
>  	 * The BIOS can leave the PHY is some weird state
> @@ -1185,10 +1184,14 @@ static void assert_chv_phy_status(struct drm_i915_private *dev_priv)
>  	 * The PHY may be busy with some initial calibration and whatnot,
>  	 * so the power state can take a while to actually change.
>  	 */
> -	if (wait_for((tmp = I915_READ(DISPLAY_PHY_STATUS) & phy_status_mask) == phy_status, 10))
> +	if (wait_until_reg(DISPLAY_PHY_STATUS,
> +			   phy_status_mask, phy_status, 10)) {
> +		u32 tmp = I915_READ(DISPLAY_PHY_STATUS);
> +
>  		WARN(phy_status != tmp,
>  		     "Unexpected PHY_STATUS 0x%08x, expected 0x%08x (PHY_CONTROL=0x%08x)\n",
>  		     tmp, phy_status, dev_priv->chv_phy_control);
> +	}
>  }
>  
>  #undef BITS_SET
> @@ -1216,7 +1219,7 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
>  	vlv_set_power_well(dev_priv, power_well, true);
>  
>  	/* Poll for phypwrgood signal */
> -	if (wait_for(I915_READ(DISPLAY_PHY_STATUS) & PHY_POWERGOOD(phy), 1))
> +	if (wait_until_reg_set(DISPLAY_PHY_STATUS, PHY_POWERGOOD(phy), 1))
>  		DRM_ERROR("Display PHY %d is not power up\n", phy);
>  
>  	mutex_lock(&dev_priv->sb_lock);
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> index c3998188cf35..2b5bb9ba160d 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.c
> +++ b/drivers/gpu/drm/i915/intel_sideband.c
> @@ -51,7 +51,7 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
>  
>  	WARN_ON(!mutex_is_locked(&dev_priv->sb_lock));
>  
> -	if (wait_for((I915_READ(VLV_IOSF_DOORBELL_REQ) & IOSF_SB_BUSY) == 0, 5)) {
> +	if (wait_until_reg_clr(VLV_IOSF_DOORBELL_REQ, IOSF_SB_BUSY, 5)) {
>  		DRM_DEBUG_DRIVER("IOSF sideband idle wait (%s) timed out\n",
>  				 is_read ? "read" : "write");
>  		return -EAGAIN;
> @@ -62,7 +62,7 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
>  		I915_WRITE(VLV_IOSF_DATA, *val);
>  	I915_WRITE(VLV_IOSF_DOORBELL_REQ, cmd);
>  
> -	if (wait_for((I915_READ(VLV_IOSF_DOORBELL_REQ) & IOSF_SB_BUSY) == 0, 5)) {
> +	if (wait_until_reg_clr(VLV_IOSF_DOORBELL_REQ, IOSF_SB_BUSY, 5)) {
>  		DRM_DEBUG_DRIVER("IOSF sideband finish wait (%s) timed out\n",
>  				 is_read ? "read" : "write");
>  		return -ETIMEDOUT;
> @@ -202,8 +202,7 @@ u32 intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg,
>  	u32 value = 0;
>  	WARN_ON(!mutex_is_locked(&dev_priv->sb_lock));
>  
> -	if (wait_for((I915_READ(SBI_CTL_STAT) & SBI_BUSY) == 0,
> -				100)) {
> +	if (wait_until_reg_clr(SBI_CTL_STAT, SBI_BUSY, 100)) {
>  		DRM_ERROR("timeout waiting for SBI to become ready\n");
>  		return 0;
>  	}
> @@ -216,8 +215,9 @@ u32 intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg,
>  		value = SBI_CTL_DEST_MPHY | SBI_CTL_OP_IORD;
>  	I915_WRITE(SBI_CTL_STAT, value | SBI_BUSY);
>  
> -	if (wait_for((I915_READ(SBI_CTL_STAT) & (SBI_BUSY | SBI_RESPONSE_FAIL)) == 0,
> -				100)) {
> +	if (wait_until_reg(SBI_CTL_STAT,
> +			   (SBI_BUSY | SBI_RESPONSE_FAIL), 0,
> +			   100)) {
>  		DRM_ERROR("timeout waiting for SBI to complete read transaction\n");
>  		return 0;
>  	}
> @@ -232,8 +232,7 @@ void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value,
>  
>  	WARN_ON(!mutex_is_locked(&dev_priv->sb_lock));
>  
> -	if (wait_for((I915_READ(SBI_CTL_STAT) & SBI_BUSY) == 0,
> -				100)) {
> +	if (wait_until_reg_clr(SBI_CTL_STAT, SBI_BUSY, 100)) {
>  		DRM_ERROR("timeout waiting for SBI to become ready\n");
>  		return;
>  	}
> @@ -247,8 +246,9 @@ void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value,
>  		tmp = SBI_CTL_DEST_MPHY | SBI_CTL_OP_IOWR;
>  	I915_WRITE(SBI_CTL_STAT, SBI_BUSY | tmp);
>  
> -	if (wait_for((I915_READ(SBI_CTL_STAT) & (SBI_BUSY | SBI_RESPONSE_FAIL)) == 0,
> -				100)) {
> +	if (wait_until_reg(SBI_CTL_STAT,
> +			   (SBI_BUSY | SBI_RESPONSE_FAIL), 0,
> +			   100)) {
>  		DRM_ERROR("timeout waiting for SBI to complete write transaction\n");
>  		return;
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index e217997a9fa0..f72420e00621 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1530,15 +1530,13 @@ static int ironlake_do_reset(struct drm_i915_private *dev_priv,
>  
>  	I915_WRITE(ILK_GDSR,
>  		   ILK_GRDOM_RENDER | ILK_GRDOM_RESET_ENABLE);
> -	ret = wait_for((I915_READ(ILK_GDSR) &
> -			ILK_GRDOM_RESET_ENABLE) == 0, 500);
> +	ret = wait_until_reg_clr(ILK_GDSR, ILK_GRDOM_RESET_ENABLE, 500);
>  	if (ret)
>  		return ret;
>  
>  	I915_WRITE(ILK_GDSR,
>  		   ILK_GRDOM_MEDIA | ILK_GRDOM_RESET_ENABLE);
> -	ret = wait_for((I915_READ(ILK_GDSR) &
> -			ILK_GRDOM_RESET_ENABLE) == 0, 500);
> +	ret = wait_until_reg_clr(ILK_GDSR, ILK_GRDOM_RESET_ENABLE, 500);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Spin opportunistically in wait_for

[Ville Syrjälä]

On Tue, May 17, 2016 at 06:43:24PM +0300, Mika Kuoppala wrote:
> Usually the condition we are after appears within very short time.
> Spin few times before going into sleep. With this approximately
> half of the wait_for in init path will take the fast path without
> sleeping.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 488141929a7a..c225605c727c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -39,7 +39,7 @@
>  #include <drm/drm_atomic.h>
>  
>  /**
> - * _wait_for_ms - magic (register) wait macro
> + * __wait_for_ms - magic (register) wait macro
>   *
>   * Does the right thing for modeset paths when run under kdgb or similar atomic
>   * contexts. Note that it's important that we check the condition again after
> @@ -50,17 +50,22 @@
>   * drm_can_sleep() can be removed and in_atomic()/!in_atomic() asserts
>   * added.
>   */
> -#define _wait_for_ms(COND, TIMEOUT_MS, SLEEP_US) ({ \
> +#define __wait_for_ms(COND, TIMEOUT_MS, SLEEP_US, SPIN_COUNT) ({	\
>  	const unsigned long timeout__ =					\
>  		jiffies + msecs_to_jiffies(TIMEOUT_MS) + 1;		\
> +	unsigned int c__ = 0;						\
>  	int ret__ = 0;							\
> +									\
>  	while (!(COND)) {						\
>  		if (time_after(jiffies, timeout__)) {			\
>  			if (!(COND))					\
>  				ret__ = -ETIMEDOUT;			\
>  			break;						\
>  		}							\
> -		if ((SLEEP_US) && drm_can_sleep()) {			\
> +									\
> +		if (++c__ > (SPIN_COUNT) &&				\
> +		    (SLEEP_US) &&					\
> +		    drm_can_sleep()) {					\
>  			usleep_range((SLEEP_US), (SLEEP_US) * 2);	\
>  		} else {						\
>  			cpu_relax();					\
> @@ -69,7 +74,8 @@
>  	ret__;								\
>  })
>  
> -#define wait_for(COND, MS)	 _wait_for_ms((COND), (MS), 1 * USEC_PER_MSEC)
> +#define wait_for(COND, MS)  __wait_for_ms((COND), (MS), 1 * USEC_PER_MSEC, 5)
> +#define _wait_for_ms(COND, MS, US)  __wait_for_ms((COND), (MS), (US), 5)

Why 5? I did some histrograms on wait_for() on my BSW (just looking
at modeset paths really), and that showed ~66% of the time we got it
right on the first check, and ~33% was completed after one sleep
iteration. The sleep duration didn't make much of a difference here,
so the current 1-2 msec is probably not at all optimal. I also tested
doing a double check intially, and IIRC that reduced the second bin
by about half. I didn't check whether further spinnign would have
helped more.

>  
>  /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
>  #if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Spin opportunistically in wait_for

[Chris Wilson]

On Wed, May 18, 2016 at 10:20:22AM +0200, Daniel Vetter wrote:
> On Tue, May 17, 2016 at 06:43:24PM +0300, Mika Kuoppala wrote:
> > Usually the condition we are after appears within very short time.
> > Spin few times before going into sleep. With this approximately
> > half of the wait_for in init path will take the fast path without
> > sleeping.
> > 
> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> Some numbers on how much time this saved would be nice.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> > ---
> >  drivers/gpu/drm/i915/intel_drv.h | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 488141929a7a..c225605c727c 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -39,7 +39,7 @@
> >  #include <drm/drm_atomic.h>
> >  
> >  /**
> > - * _wait_for_ms - magic (register) wait macro
> > + * __wait_for_ms - magic (register) wait macro
> >   *
> >   * Does the right thing for modeset paths when run under kdgb or similar atomic
> >   * contexts. Note that it's important that we check the condition again after
> > @@ -50,17 +50,22 @@
> >   * drm_can_sleep() can be removed and in_atomic()/!in_atomic() asserts
> >   * added.
> >   */
> > -#define _wait_for_ms(COND, TIMEOUT_MS, SLEEP_US) ({ \
> > +#define __wait_for_ms(COND, TIMEOUT_MS, SLEEP_US, SPIN_COUNT) ({	\
> >  	const unsigned long timeout__ =					\
> >  		jiffies + msecs_to_jiffies(TIMEOUT_MS) + 1;		\
> > +	unsigned int c__ = 0;						\
> >  	int ret__ = 0;							\
> > +									\
> >  	while (!(COND)) {						\
> >  		if (time_after(jiffies, timeout__)) {			\
> >  			if (!(COND))					\
> >  				ret__ = -ETIMEDOUT;			\
> >  			break;						\
> >  		}							\
> > -		if ((SLEEP_US) && drm_can_sleep()) {			\
> > +									\
> > +		if (++c__ > (SPIN_COUNT) &&				\
> > +		    (SLEEP_US) &&					\

Could we kill SLEEP_US here in patch 0?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/7] drm/i915: Use milliseconds in _wait_for macro

[Chris Wilson]

On Tue, May 17, 2016 at 06:43:23PM +0300, Mika Kuoppala wrote:
> Promising 1us accuracy where we timeout using jiffies is
> misleading. Convert the _wait_for macro to use milliseconds
> and convert the 2 callsites.

Note this is not about timeout but the the sleep. The timeout here is
still as inaccurate as ever as it remains jiffie based.

(And I have no qualms about the timeout being too high, unless there is
a specific example that mandates high accuracy.)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/7] drm/i915: Use milliseconds in _wait_for macro

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> [ text/plain ]
> On Tue, May 17, 2016 at 06:43:23PM +0300, Mika Kuoppala wrote:
>> Promising 1us accuracy where we timeout using jiffies is
>> misleading. Convert the _wait_for macro to use milliseconds
>> and convert the 2 callsites.
>
> Note this is not about timeout but the the sleep. The timeout here is
> still as inaccurate as ever as it remains jiffie based.
>

Hmm no, the sleep is still in usecs at this stage, unmodified.

-Mika

> (And I have no qualms about the timeout being too high, unless there is
> a specific example that mandates high accuracy.)
> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Spin opportunistically in wait_for

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> [ text/plain ]
> On Wed, May 18, 2016 at 10:20:22AM +0200, Daniel Vetter wrote:
>> On Tue, May 17, 2016 at 06:43:24PM +0300, Mika Kuoppala wrote:
>> > Usually the condition we are after appears within very short time.
>> > Spin few times before going into sleep. With this approximately
>> > half of the wait_for in init path will take the fast path without
>> > sleeping.
>> > 
>> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> 
>> Some numbers on how much time this saved would be nice.
>> 
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> 
>> > ---
>> >  drivers/gpu/drm/i915/intel_drv.h | 14 ++++++++++----
>> >  1 file changed, 10 insertions(+), 4 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> > index 488141929a7a..c225605c727c 100644
>> > --- a/drivers/gpu/drm/i915/intel_drv.h
>> > +++ b/drivers/gpu/drm/i915/intel_drv.h
>> > @@ -39,7 +39,7 @@
>> >  #include <drm/drm_atomic.h>
>> >  
>> >  /**
>> > - * _wait_for_ms - magic (register) wait macro
>> > + * __wait_for_ms - magic (register) wait macro
>> >   *
>> >   * Does the right thing for modeset paths when run under kdgb or similar atomic
>> >   * contexts. Note that it's important that we check the condition again after
>> > @@ -50,17 +50,22 @@
>> >   * drm_can_sleep() can be removed and in_atomic()/!in_atomic() asserts
>> >   * added.
>> >   */
>> > -#define _wait_for_ms(COND, TIMEOUT_MS, SLEEP_US) ({ \
>> > +#define __wait_for_ms(COND, TIMEOUT_MS, SLEEP_US, SPIN_COUNT) ({	\
>> >  	const unsigned long timeout__ =					\
>> >  		jiffies + msecs_to_jiffies(TIMEOUT_MS) + 1;		\
>> > +	unsigned int c__ = 0;						\
>> >  	int ret__ = 0;							\
>> > +									\
>> >  	while (!(COND)) {						\
>> >  		if (time_after(jiffies, timeout__)) {			\
>> >  			if (!(COND))					\
>> >  				ret__ = -ETIMEDOUT;			\
>> >  			break;						\
>> >  		}							\
>> > -		if ((SLEEP_US) && drm_can_sleep()) {			\
>> > +									\
>> > +		if (++c__ > (SPIN_COUNT) &&				\
>> > +		    (SLEEP_US) &&					\
>
> Could we kill SLEEP_US here in patch 0?

Patch 1? Yes we could.
-Mika

> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/debug: Warn when waiting on condition timeouts

[Chris Wilson]

On Tue, May 17, 2016 at 06:43:28PM +0300, Mika Kuoppala wrote:
> Warn if we timeout on waiting register or other condition.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     | 25 ++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_drv.h    | 10 +++++++++-
>  drivers/gpu/drm/i915/intel_uncore.c | 10 +++++-----
>  3 files changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ccf6747894b1..3d03a17a8b7c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2873,11 +2873,26 @@ static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv)
>  	return dev_priv->vgpu.active;
>  }
>  
> -int intel_wait_until_register(struct drm_i915_private *dev_priv,
> -			      i915_reg_t reg,
> -			      u32 mask,
> -			      u32 value,
> -			      unsigned long timeout_ms);
> +int __intel_wait_until_register(struct drm_i915_private *dev_priv,
> +				i915_reg_t reg,
> +				u32 mask,
> +				u32 value,
> +				unsigned long timeout_ms);
> +
> +#if defined(CONFIG_DRM_I915_DEBUG)
> +#define intel_wait_until_register(dev_priv, reg, mask, value, timeout_ms) ({ \
> +	int __ret; \
> +	WARN_ON(__ret = __intel_wait_until_register((dev_priv), \
> +						    (reg), (mask), \
> +						    (value), \
> +						    (timeout_ms))); \
> +	__ret; \
> +	})
> +#else
> +#define intel_wait_until_register(dev_priv, reg, mask, value, timeout_ms) \
> +	__intel_wait_until_register((dev_priv), \
> +				    (reg), (mask), (value), timeout_ms);
> +#endif

Stuff it inside intel_wait_until_register. One more frame in the WARN
isn't going to confuse us? And make it a WARN() with more interesting
fmt string. Seems silly to undo the bloat you were fighting!

Good idea nevertheless.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/7] drm/i915: Use milliseconds in _wait_for macro

[Chris Wilson]

On Wed, May 18, 2016 at 02:07:53PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > [ text/plain ]
> > On Tue, May 17, 2016 at 06:43:23PM +0300, Mika Kuoppala wrote:
> >> Promising 1us accuracy where we timeout using jiffies is
> >> misleading. Convert the _wait_for macro to use milliseconds
> >> and convert the 2 callsites.
> >
> > Note this is not about timeout but the the sleep. The timeout here is
> > still as inaccurate as ever as it remains jiffie based.
> >
> 
> Hmm no, the sleep is still in usecs at this stage, unmodified.

Sleep, yes. Timeout, no. Commit log is talking about timeout.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx