[Intel-gfx] [PATCH] drm/i915/sdvo: Poll command status 5 times without delay on read

[Chris Wilson]

The documentation says that an SDVO command takes a maximum of 15us to be
processed by the device, and that it is sufficient to read the status byte
3 times (whilst the command is still in the PENDING state) for the driver
to be confident that sufficient time has elapsed.

We err on the safe side and try 5 times before giving up.

The only question that remains: was the old behaviour derived by
experiments with real hardware?

A look into the murky history of UMS, implies that the behaviour was
accidental and the current retry mechanism was solely designed to catch
the status byte indicating PENDING with no reference to hardware
behaviour. (commit ac9181c014638dbeb334b40b4029d0ccb2b7a0fc in
xf86-video-intel)

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_sdvo.c |   70 ++++++++++++++++++------------------
 1 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index f84ae87..eb62a73 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -461,54 +461,54 @@ static const char *cmd_status_names[] = {
 	"Scaling not supported"
 };
 
-static void intel_sdvo_debug_response(struct intel_sdvo *intel_sdvo,
-				      void *response, int response_len,
-				      u8 status)
+static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
+				     void *response, int response_len)
 {
+	u8 retry = 5;
+	u8 status;
 	int i;
 
+	/*
+	 * The documentation states that all commands will be
+	 * processed within 15µs, and that we need only poll
+	 * the status byte a maximum of 3 times in order for the
+	 * command to be complete.
+	 *
+	 * Check 5 times in case the hardware failed to read the docs.
+	 */
+	do {
+		if (!intel_sdvo_read_byte(intel_sdvo,
+					  SDVO_I2C_CMD_STATUS,
+					  &status))
+			return false;
+	} while (status == SDVO_CMD_STATUS_PENDING && --retry);
+
 	DRM_DEBUG_KMS("%s: R: ", SDVO_NAME(intel_sdvo));
-	for (i = 0; i < response_len; i++)
-		DRM_LOG_KMS("%02X ", ((u8 *)response)[i]);
-	for (; i < 8; i++)
-		DRM_LOG_KMS("   ");
 	if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
 		DRM_LOG_KMS("(%s)", cmd_status_names[status]);
 	else
 		DRM_LOG_KMS("(??? %d)", status);
-	DRM_LOG_KMS("\n");
-}
 
-static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
-				     void *response, int response_len)
-{
-	int i;
-	u8 status;
-	u8 retry = 50;
-
-	while (retry--) {
-		/* Read the command response */
-		for (i = 0; i < response_len; i++) {
-			if (!intel_sdvo_read_byte(intel_sdvo,
-						  SDVO_I2C_RETURN_0 + i,
-						  &((u8 *)response)[i]))
-				return false;
-		}
+	if (status != SDVO_CMD_STATUS_SUCCESS)
+		goto log_fail;
 
-		/* read the return status */
-		if (!intel_sdvo_read_byte(intel_sdvo, SDVO_I2C_CMD_STATUS,
-					  &status))
-			return false;
+	/* Read the command response */
+	for (i = 0; i < response_len; i++) {
+		if (!intel_sdvo_read_byte(intel_sdvo,
+					  SDVO_I2C_RETURN_0 + i,
+					  &((u8 *)response)[i]))
+			goto log_fail;
+		DRM_LOG_KMS("%02X ", ((u8 *)response)[i]);
+	}
 
-		intel_sdvo_debug_response(intel_sdvo, response, response_len,
-					  status);
-		if (status != SDVO_CMD_STATUS_PENDING)
-			break;
+	for (; i < 8; i++)
+		DRM_LOG_KMS("   ");
 
-		mdelay(50);
-	}
+	return true;
 
-	return status == SDVO_CMD_STATUS_SUCCESS;
+log_fail:
+	DRM_LOG_KMS("\n");
+	return false;
 }
 
 static int intel_sdvo_get_pixel_multiplier(struct drm_display_mode *mode)
-- 
1.7.1