On Fri, 23 Nov 2012, Chris Wilson <[email protected]> wrote:
> Some devices may respond very slowly and only flag that the reply is
> pending within the first 15us response window. Be kind to such devices
> and wait a further 15ms, before checking for the pending reply. This
> moves the existing special case delay of 30ms down from the detection
> routine into the common path and pretends to explain it...
>
> Tested-by: [email protected]
> References: https://bugs.freedesktop.org/show_bug.cgi?id=36997
> Signed-off-by: Chris Wilson <[email protected]>
> ---
> drivers/gpu/drm/i915/intel_sdvo.c | 39
> ++++++++++++++++++++++---------------
> 1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> b/drivers/gpu/drm/i915/intel_sdvo.c
> index d85ebb0..f0a1a6f 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -522,19 +522,32 @@ static bool intel_sdvo_read_response(struct intel_sdvo
> *intel_sdvo,
> * command to be complete.
> *
> * Check 5 times in case the hardware failed to read the docs.
> + *
> + * Also beware that the first response by many devices is to
> + * reply PENDING and stall for time. TVs are notorious for
> + * requiring longer than specified to complete their replies.
> */
> if (!intel_sdvo_read_byte(intel_sdvo,
> SDVO_I2C_CMD_STATUS,
> &status))
> goto log_fail;
>
> - while (status == SDVO_CMD_STATUS_PENDING && retry--) {
> - udelay(15);
> - if (!intel_sdvo_read_byte(intel_sdvo,
> - SDVO_I2C_CMD_STATUS,
> - &status))
> - goto log_fail;
> - }
> + do {
> + int quick = 5;
> +
> + while (status == SDVO_CMD_STATUS_PENDING && quick--) {
> + udelay(15);
> + if (!intel_sdvo_read_byte(intel_sdvo,
> + SDVO_I2C_CMD_STATUS,
> + &status))
> + goto log_fail;
> + }
> +
> + if (status != SDVO_CMD_STATUS_PENDING || --retry == 0)
> + break;
> +
> + msleep(15);
> + } while (1);
Is your intention to have 5 quick retries nested in 5 slow retries,
resulting in 25 retries total? What do the quick retries buy you after
the first msleep(15)? In other words, why not just do something simple
like:
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
b/drivers/gpu/drm/i915/intel_sdvo.c
index 30f1752..3b2eddc 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -504,7 +504,7 @@ out:
static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
void *response, int response_len)
{
- u8 retry = 5;
+ u8 retry = 2*5;
u8 status;
int i;
@@ -524,7 +524,10 @@ static bool intel_sdvo_read_response(struct intel_sdvo
*intel_sdvo,
goto log_fail;
while (status == SDVO_CMD_STATUS_PENDING && retry--) {
- udelay(15);
+ if (retry >= 5)
+ udelay(15);
+ else
+ msleep(15);
if (!intel_sdvo_read_byte(intel_sdvo,
SDVO_I2C_CMD_STATUS,
&status))
BR,
Jani.
>
> if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
> DRM_LOG_KMS("(%s)", cmd_status_names[status]);
> @@ -1535,15 +1548,9 @@ intel_sdvo_detect(struct drm_connector *connector,
> bool force)
> struct intel_sdvo_connector *intel_sdvo_connector =
> to_intel_sdvo_connector(connector);
> enum drm_connector_status ret;
>
> - if (!intel_sdvo_write_cmd(intel_sdvo,
> - SDVO_CMD_GET_ATTACHED_DISPLAYS, NULL, 0))
> - return connector_status_unknown;
> -
> - /* add 30ms delay when the output type might be TV */
> - if (intel_sdvo->caps.output_flags & SDVO_TV_MASK)
> - msleep(30);
> -
> - if (!intel_sdvo_read_response(intel_sdvo, &response, 2))
> + if (!intel_sdvo_get_value(intel_sdvo,
> + SDVO_CMD_GET_ATTACHED_DISPLAYS,
> + &response, 2))
> return connector_status_unknown;
>
> DRM_DEBUG_KMS("SDVO response %d %d [%x]\n",
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
