Re: [PATCH 2/4] drm/i915: Convert sandybridge_pcode_*() to use intel_wait_for_register()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 30/06/16 12:30, Chris Wilson wrote:
We want to replace the inline wait_for() with an out-of-line hybrid
busy/sleep wait_for() in the hopes of speeding up the communication wit
the PCode unit.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
  drivers/gpu/drm/i915/intel_pm.c | 43 +++++++++++++++++++++++++++--------------
  1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d7f8ba89d2a5..f84bf253d04a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7623,46 +7623,59 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u32 mbox, u32 *val
  {
  	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));

-	if (I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) {
+	/* GEN6_PCODE_* are outside of the forcewake domain, we can
+	 * use te fw I915_READ variants to reduce the amount of work
+	 * required when reading/writing.
+	 */
+
+	if (I915_READ_FW(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) {
  		DRM_DEBUG_DRIVER("warning: pcode (read) mailbox access failed\n");
  		return -EAGAIN;
  	}

-	I915_WRITE(GEN6_PCODE_DATA, *val);
-	I915_WRITE(GEN6_PCODE_DATA1, 0);
-	I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
+	I915_WRITE_FW(GEN6_PCODE_DATA, *val);
+	I915_WRITE_FW(GEN6_PCODE_DATA1, 0);
+	I915_WRITE_FW(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);

-	if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
-		     500)) {
+	if (intel_wait_for_register(dev_priv,
+				    GEN6_PCODE_MAILBOX, GEN6_PCODE_READY, 0,
+				    500)) {

Why not the _fw version?

Actually brings me to the point I glanced over in the previous patch - why would intel_wait_for_register_fw be not suitable for long waits? It looks like a concern for the calling code grabbing the fw outside it for a long period and not for the function itself. This call site being a case in point for that.

  		DRM_ERROR("timeout waiting for pcode read (%d) to finish\n", mbox);
  		return -ETIMEDOUT;
  	}

-	*val = I915_READ(GEN6_PCODE_DATA);
-	I915_WRITE(GEN6_PCODE_DATA, 0);
+	*val = I915_READ_FW(GEN6_PCODE_DATA);
+	I915_WRITE_FW(GEN6_PCODE_DATA, 0);

  	return 0;
  }

-int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u32 mbox, u32 val)
+int sandybridge_pcode_write(struct drm_i915_private *dev_priv,
+			       u32 mbox, u32 val)
  {
  	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));

-	if (I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) {
+	/* GEN6_PCODE_* are outside of the forcewake domain, we can
+	 * use te fw I915_READ variants to reduce the amount of work
+	 * required when reading/writing.
+	 */
+
+	if (I915_READ_FW(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) {
  		DRM_DEBUG_DRIVER("warning: pcode (write) mailbox access failed\n");
  		return -EAGAIN;
  	}

-	I915_WRITE(GEN6_PCODE_DATA, val);
-	I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
+	I915_WRITE_FW(GEN6_PCODE_DATA, val);
+	I915_WRITE_FW(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);

-	if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
-		     500)) {
+	if (intel_wait_for_register_fw(dev_priv,
+				       GEN6_PCODE_MAILBOX, GEN6_PCODE_READY, 0,
+				       500)) {

Here you use the _fw variant.

  		DRM_ERROR("timeout waiting for pcode write (%d) to finish\n", mbox);
  		return -ETIMEDOUT;
  	}

-	I915_WRITE(GEN6_PCODE_DATA, 0);
+	I915_WRITE_FW(GEN6_PCODE_DATA, 0);

  	return 0;
  }


Regards,

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux