All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Paul <seanpaul@chromium.org>
To: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org, daniel.vetter@intel.com,
	Sean Paul <seanpaul@chromium.org>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	David Airlie <airlied@linux.ie>
Subject: [PATCH v2 2/8] drm/i915: Add more control to wait_for routines
Date: Fri,  1 Dec 2017 12:20:24 -0500	[thread overview]
Message-ID: <20171201172032.47357-3-seanpaul@chromium.org> (raw)
In-Reply-To: <20171201172032.47357-1-seanpaul@chromium.org>

This patch adds a little more control to a couple wait_for routines such
that we can avoid open-coding read/wait/timeout patterns which:
 - need the value of the register after the wait_for
 - run arbitrary operation for the read portion

This patch also chooses the correct sleep function (based on
timers-howto.txt) for the polling interval the caller specifies.

Changes in v2:
- Added to the series

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/intel_drv.h    | 17 ++++++++++++-----
 drivers/gpu/drm/i915/intel_uncore.c | 23 ++++++++++++++++-------
 drivers/gpu/drm/i915/intel_uncore.h | 14 +++++++++++++-
 3 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 69aab324aaa1..191c80fc4314 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -41,19 +41,21 @@
 #include <drm/drm_atomic.h>
 
 /**
- * _wait_for - magic (register) wait macro
+ * __wait_for - magic 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
+ * Macro to help avoid open coding check/wait/timeout patterns, will do the
+ * right think wrt to choosing msleep vs usleep_range based on how long the wait
+ * interval is. Note that it's important that we check the condition again after
  * having timed out, since the timeout could be due to preemption or similar and
  * we've never had a chance to check the condition before the timeout.
  */
-#define _wait_for(COND, US, W) ({ \
+#define __wait_for(OP, COND, US, W) ({ \
 	unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1;	\
 	int ret__;							\
 	might_sleep();							\
 	for (;;) {							\
 		bool expired__ = time_after(jiffies, timeout__);	\
+		OP;							\
 		if (COND) {						\
 			ret__ = 0;					\
 			break;						\
@@ -62,11 +64,16 @@
 			ret__ = -ETIMEDOUT;				\
 			break;						\
 		}							\
-		usleep_range((W), (W) * 2);				\
+		if (W > (20 * 1000))					\
+			msleep(W / 1000);				\
+		else							\
+			usleep_range((W), (W) * 2);			\
 	}								\
 	ret__;								\
 })
 
+#define _wait_for(COND, US, W)		__wait_for(;,(COND), US, W)
+
 #define wait_for(COND, MS)	  	_wait_for((COND), (MS) * 1000, 1000)
 
 /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index b4621271e7a2..c851b0c0602d 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1770,12 +1770,14 @@ int __intel_wait_for_register_fw(struct drm_i915_private *dev_priv,
 }
 
 /**
- * intel_wait_for_register - wait until register matches expected state
+ * __intel_wait_for_register - wait until register matches expected state
  * @dev_priv: the i915 device
  * @reg: the register to read
  * @mask: mask to apply to register value
  * @value: expected value
- * @timeout_ms: timeout in millisecond
+ * @fast_timeout_us: fast timeout in microsecond for atomic/tight wait
+ * @slow_timeout_ms: slow timeout in millisecond
+ * @out_value: optional placeholder to hold registry value
  *
  * This routine waits until the target register @reg contains the expected
  * @value after applying the @mask, i.e. it waits until ::
@@ -1786,15 +1788,18 @@ int __intel_wait_for_register_fw(struct drm_i915_private *dev_priv,
  *
  * Returns 0 if the register matches the desired condition, or -ETIMEOUT.
  */
-int intel_wait_for_register(struct drm_i915_private *dev_priv,
+int __intel_wait_for_register(struct drm_i915_private *dev_priv,
 			    i915_reg_t reg,
 			    u32 mask,
 			    u32 value,
-			    unsigned int timeout_ms)
+			    unsigned int fast_timeout_us,
+			    unsigned int slow_timeout_ms,
+			    u32 *out_value)
 {
 	unsigned fw =
 		intel_uncore_forcewake_for_reg(dev_priv, reg, FW_REG_READ);
 	int ret;
+	u32 reg_value;
 
 	might_sleep();
 
@@ -1803,14 +1808,18 @@ int intel_wait_for_register(struct drm_i915_private *dev_priv,
 
 	ret = __intel_wait_for_register_fw(dev_priv,
 					   reg, mask, value,
-					   2, 0, NULL);
+					   fast_timeout_us, 0, &reg_value);
 
 	intel_uncore_forcewake_put__locked(dev_priv, fw);
 	spin_unlock_irq(&dev_priv->uncore.lock);
 
 	if (ret)
-		ret = wait_for((I915_READ_NOTRACE(reg) & mask) == value,
-			       timeout_ms);
+		ret = __wait_for(reg_value = I915_READ_NOTRACE(reg),
+			         (reg_value & mask) == value,
+			         slow_timeout_ms * 1000, 1000);
+
+	if (out_value)
+		*out_value = reg_value;
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index 9ce079b5dd0d..bed019ef000f 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -163,11 +163,23 @@ void intel_uncore_forcewake_put__locked(struct drm_i915_private *dev_priv,
 void intel_uncore_forcewake_user_get(struct drm_i915_private *dev_priv);
 void intel_uncore_forcewake_user_put(struct drm_i915_private *dev_priv);
 
+int __intel_wait_for_register(struct drm_i915_private *dev_priv,
+			      i915_reg_t reg,
+			      u32 mask,
+			      u32 value,
+			      unsigned int fast_timeout_us,
+			      unsigned int slow_timeout_ms,
+			      u32 *out_value);
+static inline
 int intel_wait_for_register(struct drm_i915_private *dev_priv,
 			    i915_reg_t reg,
 			    u32 mask,
 			    u32 value,
-			    unsigned int timeout_ms);
+			    unsigned int timeout_ms)
+{
+	return __intel_wait_for_register(dev_priv, reg, mask, value, 2,
+					 timeout_ms, NULL);
+}
 int __intel_wait_for_register_fw(struct drm_i915_private *dev_priv,
 				 i915_reg_t reg,
 				 u32 mask,
-- 
2.15.0.531.g2ccb3012c9-goog

WARNING: multiple messages have this Message-ID (diff)
From: Sean Paul <seanpaul@chromium.org>
To: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Cc: David Airlie <airlied@linux.ie>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	linux-kernel@vger.kernel.org,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	daniel.vetter@intel.com
Subject: [PATCH v2 2/8] drm/i915: Add more control to wait_for routines
Date: Fri,  1 Dec 2017 12:20:24 -0500	[thread overview]
Message-ID: <20171201172032.47357-3-seanpaul@chromium.org> (raw)
In-Reply-To: <20171201172032.47357-1-seanpaul@chromium.org>

This patch adds a little more control to a couple wait_for routines such
that we can avoid open-coding read/wait/timeout patterns which:
 - need the value of the register after the wait_for
 - run arbitrary operation for the read portion

This patch also chooses the correct sleep function (based on
timers-howto.txt) for the polling interval the caller specifies.

Changes in v2:
- Added to the series

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/i915/intel_drv.h    | 17 ++++++++++++-----
 drivers/gpu/drm/i915/intel_uncore.c | 23 ++++++++++++++++-------
 drivers/gpu/drm/i915/intel_uncore.h | 14 +++++++++++++-
 3 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 69aab324aaa1..191c80fc4314 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -41,19 +41,21 @@
 #include <drm/drm_atomic.h>
 
 /**
- * _wait_for - magic (register) wait macro
+ * __wait_for - magic 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
+ * Macro to help avoid open coding check/wait/timeout patterns, will do the
+ * right think wrt to choosing msleep vs usleep_range based on how long the wait
+ * interval is. Note that it's important that we check the condition again after
  * having timed out, since the timeout could be due to preemption or similar and
  * we've never had a chance to check the condition before the timeout.
  */
-#define _wait_for(COND, US, W) ({ \
+#define __wait_for(OP, COND, US, W) ({ \
 	unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1;	\
 	int ret__;							\
 	might_sleep();							\
 	for (;;) {							\
 		bool expired__ = time_after(jiffies, timeout__);	\
+		OP;							\
 		if (COND) {						\
 			ret__ = 0;					\
 			break;						\
@@ -62,11 +64,16 @@
 			ret__ = -ETIMEDOUT;				\
 			break;						\
 		}							\
-		usleep_range((W), (W) * 2);				\
+		if (W > (20 * 1000))					\
+			msleep(W / 1000);				\
+		else							\
+			usleep_range((W), (W) * 2);			\
 	}								\
 	ret__;								\
 })
 
+#define _wait_for(COND, US, W)		__wait_for(;,(COND), US, W)
+
 #define wait_for(COND, MS)	  	_wait_for((COND), (MS) * 1000, 1000)
 
 /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index b4621271e7a2..c851b0c0602d 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1770,12 +1770,14 @@ int __intel_wait_for_register_fw(struct drm_i915_private *dev_priv,
 }
 
 /**
- * intel_wait_for_register - wait until register matches expected state
+ * __intel_wait_for_register - wait until register matches expected state
  * @dev_priv: the i915 device
  * @reg: the register to read
  * @mask: mask to apply to register value
  * @value: expected value
- * @timeout_ms: timeout in millisecond
+ * @fast_timeout_us: fast timeout in microsecond for atomic/tight wait
+ * @slow_timeout_ms: slow timeout in millisecond
+ * @out_value: optional placeholder to hold registry value
  *
  * This routine waits until the target register @reg contains the expected
  * @value after applying the @mask, i.e. it waits until ::
@@ -1786,15 +1788,18 @@ int __intel_wait_for_register_fw(struct drm_i915_private *dev_priv,
  *
  * Returns 0 if the register matches the desired condition, or -ETIMEOUT.
  */
-int intel_wait_for_register(struct drm_i915_private *dev_priv,
+int __intel_wait_for_register(struct drm_i915_private *dev_priv,
 			    i915_reg_t reg,
 			    u32 mask,
 			    u32 value,
-			    unsigned int timeout_ms)
+			    unsigned int fast_timeout_us,
+			    unsigned int slow_timeout_ms,
+			    u32 *out_value)
 {
 	unsigned fw =
 		intel_uncore_forcewake_for_reg(dev_priv, reg, FW_REG_READ);
 	int ret;
+	u32 reg_value;
 
 	might_sleep();
 
@@ -1803,14 +1808,18 @@ int intel_wait_for_register(struct drm_i915_private *dev_priv,
 
 	ret = __intel_wait_for_register_fw(dev_priv,
 					   reg, mask, value,
-					   2, 0, NULL);
+					   fast_timeout_us, 0, &reg_value);
 
 	intel_uncore_forcewake_put__locked(dev_priv, fw);
 	spin_unlock_irq(&dev_priv->uncore.lock);
 
 	if (ret)
-		ret = wait_for((I915_READ_NOTRACE(reg) & mask) == value,
-			       timeout_ms);
+		ret = __wait_for(reg_value = I915_READ_NOTRACE(reg),
+			         (reg_value & mask) == value,
+			         slow_timeout_ms * 1000, 1000);
+
+	if (out_value)
+		*out_value = reg_value;
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index 9ce079b5dd0d..bed019ef000f 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -163,11 +163,23 @@ void intel_uncore_forcewake_put__locked(struct drm_i915_private *dev_priv,
 void intel_uncore_forcewake_user_get(struct drm_i915_private *dev_priv);
 void intel_uncore_forcewake_user_put(struct drm_i915_private *dev_priv);
 
+int __intel_wait_for_register(struct drm_i915_private *dev_priv,
+			      i915_reg_t reg,
+			      u32 mask,
+			      u32 value,
+			      unsigned int fast_timeout_us,
+			      unsigned int slow_timeout_ms,
+			      u32 *out_value);
+static inline
 int intel_wait_for_register(struct drm_i915_private *dev_priv,
 			    i915_reg_t reg,
 			    u32 mask,
 			    u32 value,
-			    unsigned int timeout_ms);
+			    unsigned int timeout_ms)
+{
+	return __intel_wait_for_register(dev_priv, reg, mask, value, 2,
+					 timeout_ms, NULL);
+}
 int __intel_wait_for_register_fw(struct drm_i915_private *dev_priv,
 				 i915_reg_t reg,
 				 u32 mask,
-- 
2.15.0.531.g2ccb3012c9-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2017-12-01 17:20 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-01 17:20 [PATCH v2 0/8] drm/i915: Implement HDCP Sean Paul
2017-12-01 17:20 ` Sean Paul
2017-12-01 17:20 ` [PATCH v2 1/8] drm: Fix link-status kerneldoc line lengths Sean Paul
2017-12-01 17:20   ` Sean Paul
2017-12-01 17:20 ` Sean Paul [this message]
2017-12-01 17:20   ` [PATCH v2 2/8] drm/i915: Add more control to wait_for routines Sean Paul
2017-12-01 17:44   ` [Intel-gfx] " Chris Wilson
2017-12-01 17:48     ` Sean Paul
2017-12-01 17:48       ` Sean Paul
2017-12-01 17:55       ` Chris Wilson
2017-12-01 17:57         ` Chris Wilson
2017-12-01 18:00           ` [Intel-gfx] " Sean Paul
2017-12-01 18:00             ` Sean Paul
2017-12-01 17:20 ` [PATCH v2 3/8] drm: Add Content Protection property Sean Paul
2017-12-01 17:20   ` Sean Paul
2017-12-01 17:20 ` [PATCH v2 4/8] drm: Add some HDCP related #defines Sean Paul
2017-12-01 17:20   ` Sean Paul
2017-12-01 17:20 ` [PATCH v2 5/8] drm/i915: Add HDCP framework + base implementation Sean Paul
2017-12-01 17:20   ` Sean Paul
2017-12-01 17:20 ` [PATCH v2 6/8] drm/i915: Add function to output Aksv over GMBUS Sean Paul
2017-12-01 17:20   ` Sean Paul
2017-12-01 19:06   ` Ville Syrjälä
2017-12-01 19:06     ` Ville Syrjälä
2017-12-01 19:17     ` Sean Paul
2017-12-01 19:17       ` Sean Paul
2017-12-01 20:03       ` Ville Syrjälä
2017-12-01 20:03         ` Ville Syrjälä
2017-12-01 17:20 ` [PATCH v2 7/8] drm/i915: Implement HDCP for HDMI Sean Paul
2017-12-01 17:20 ` [PATCH v2 8/8] drm/i915: Implement HDCP for DisplayPort Sean Paul
2017-12-01 17:23 ` ✗ Fi.CI.BAT: failure for drm/i915: Implement HDCP (rev2) Patchwork
2017-12-01 18:47 ` [PATCH v2 0/8] drm/i915: Implement HDCP Hans Verkuil
2017-12-01 18:47   ` Hans Verkuil
2017-12-01 18:58   ` Sean Paul
2017-12-01 18:58     ` Sean Paul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171201172032.47357-3-seanpaul@chromium.org \
    --to=seanpaul@chromium.org \
    --cc=airlied@linux.ie \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rodrigo.vivi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.