* [PATCH] drm/i915: Increase the response time for slow SDVO devices @ 2012-11-23 8:44 Chris Wilson 2012-11-23 11:42 ` Jani Nikula 0 siblings, 1 reply; 8+ messages in thread From: Chris Wilson @ 2012-11-23 8:44 UTC (permalink / raw) To: intel-gfx 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: bo.b.wang@intel.com References: https://bugs.freedesktop.org/show_bug.cgi?id=36997 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- 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); 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Increase the response time for slow SDVO devices 2012-11-23 8:44 [PATCH] drm/i915: Increase the response time for slow SDVO devices Chris Wilson @ 2012-11-23 11:42 ` Jani Nikula 2012-11-23 11:47 ` Chris Wilson 2012-11-23 11:57 ` Chris Wilson 0 siblings, 2 replies; 8+ messages in thread From: Jani Nikula @ 2012-11-23 11:42 UTC (permalink / raw) To: Chris Wilson, intel-gfx On Fri, 23 Nov 2012, Chris Wilson <chris@chris-wilson.co.uk> 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: bo.b.wang@intel.com > References: https://bugs.freedesktop.org/show_bug.cgi?id=36997 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > 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 > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Increase the response time for slow SDVO devices 2012-11-23 11:42 ` Jani Nikula @ 2012-11-23 11:47 ` Chris Wilson 2012-11-23 11:57 ` Chris Wilson 1 sibling, 0 replies; 8+ messages in thread From: Chris Wilson @ 2012-11-23 11:47 UTC (permalink / raw) To: Jani Nikula, intel-gfx On Fri, 23 Nov 2012 13:42:38 +0200, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Fri, 23 Nov 2012, Chris Wilson <chris@chris-wilson.co.uk> 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: bo.b.wang@intel.com > > References: https://bugs.freedesktop.org/show_bug.cgi?id=36997 > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > 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: Sure, looks better. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] drm/i915: Increase the response time for slow SDVO devices 2012-11-23 11:42 ` Jani Nikula 2012-11-23 11:47 ` Chris Wilson @ 2012-11-23 11:57 ` Chris Wilson 2012-11-23 12:21 ` Jani Nikula 2012-11-26 18:45 ` Daniel Vetter 1 sibling, 2 replies; 8+ messages in thread From: Chris Wilson @ 2012-11-23 11:57 UTC (permalink / raw) To: intel-gfx 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... v2: Simplify the loop constructs as suggested by Jani Nikula. References: https://bugs.freedesktop.org/show_bug.cgi?id=36997 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_sdvo.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index d85ebb0..cff3c0b 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -509,7 +509,7 @@ out: static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo, void *response, int response_len) { - u8 retry = 5; + u8 retry = 15; /* 5 quick checks, followed by 10 long checks */ u8 status; int i; @@ -522,14 +522,27 @@ 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. + * Originally (in the DDX long ago), the delay was only ever 15ms + * with an additional delay of 30ms applied for TVs added later after + * many experiments. To accommodate both sets of delays, we do a + * sequence of slow checks if the device is falling behind and fails + * to reply within 5*15µs. */ if (!intel_sdvo_read_byte(intel_sdvo, SDVO_I2C_CMD_STATUS, &status)) goto log_fail; - while (status == SDVO_CMD_STATUS_PENDING && retry--) { - udelay(15); + while (status == SDVO_CMD_STATUS_PENDING && --retry) { + if (retry < 10) + msleep(15); + else + udelay(15); + if (!intel_sdvo_read_byte(intel_sdvo, SDVO_I2C_CMD_STATUS, &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 Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Increase the response time for slow SDVO devices 2012-11-23 11:57 ` Chris Wilson @ 2012-11-23 12:21 ` Jani Nikula 2012-11-23 12:47 ` Chris Wilson 2012-11-26 18:45 ` Daniel Vetter 1 sibling, 1 reply; 8+ messages in thread From: Jani Nikula @ 2012-11-23 12:21 UTC (permalink / raw) To: Chris Wilson, intel-gfx On Fri, 23 Nov 2012, Chris Wilson <chris@chris-wilson.co.uk> 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... > > v2: Simplify the loop constructs as suggested by Jani Nikula. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=36997 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/intel_sdvo.c | 31 +++++++++++++++++++------------ > 1 file changed, 19 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c > index d85ebb0..cff3c0b 100644 > --- a/drivers/gpu/drm/i915/intel_sdvo.c > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > @@ -509,7 +509,7 @@ out: > static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo, > void *response, int response_len) > { > - u8 retry = 5; > + u8 retry = 15; /* 5 quick checks, followed by 10 long checks */ > u8 status; > int i; > > @@ -522,14 +522,27 @@ 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. > + * Originally (in the DDX long ago), the delay was only ever 15ms > + * with an additional delay of 30ms applied for TVs added later after > + * many experiments. To accommodate both sets of delays, we do a > + * sequence of slow checks if the device is falling behind and fails > + * to reply within 5*15µs. > */ > if (!intel_sdvo_read_byte(intel_sdvo, > SDVO_I2C_CMD_STATUS, > &status)) > goto log_fail; > > - while (status == SDVO_CMD_STATUS_PENDING && retry--) { > - udelay(15); > + while (status == SDVO_CMD_STATUS_PENDING && --retry) { Hey, why did you switch from post to pre decrement? It will now retry only retry-1 times. Or is this about the semantics of retries vs. tries? ;) Otherwise, Reviewed-by: Jani Nikula <jani.nikula@intel.com> > + if (retry < 10) > + msleep(15); > + else > + udelay(15); > + > if (!intel_sdvo_read_byte(intel_sdvo, > SDVO_I2C_CMD_STATUS, > &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 > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Increase the response time for slow SDVO devices 2012-11-23 12:21 ` Jani Nikula @ 2012-11-23 12:47 ` Chris Wilson 2012-11-23 13:19 ` Jani Nikula 0 siblings, 1 reply; 8+ messages in thread From: Chris Wilson @ 2012-11-23 12:47 UTC (permalink / raw) To: Jani Nikula, intel-gfx [-- Attachment #1: Type: text/plain, Size: 2696 bytes --] On Fri, 23 Nov 2012 14:21:58 +0200, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Fri, 23 Nov 2012, Chris Wilson <chris@chris-wilson.co.uk> 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... > > > > v2: Simplify the loop constructs as suggested by Jani Nikula. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=36997 > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/intel_sdvo.c | 31 +++++++++++++++++++------------ > > 1 file changed, 19 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c > > index d85ebb0..cff3c0b 100644 > > --- a/drivers/gpu/drm/i915/intel_sdvo.c > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > > @@ -509,7 +509,7 @@ out: > > static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo, > > void *response, int response_len) > > { > > - u8 retry = 5; > > + u8 retry = 15; /* 5 quick checks, followed by 10 long checks */ > > u8 status; > > int i; > > > > @@ -522,14 +522,27 @@ 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. > > + * Originally (in the DDX long ago), the delay was only ever 15ms > > + * with an additional delay of 30ms applied for TVs added later after > > + * many experiments. To accommodate both sets of delays, we do a > > + * sequence of slow checks if the device is falling behind and fails > > + * to reply within 5*15µs. > > */ > > if (!intel_sdvo_read_byte(intel_sdvo, > > SDVO_I2C_CMD_STATUS, > > &status)) > > goto log_fail; > > > > - while (status == SDVO_CMD_STATUS_PENDING && retry--) { > > - udelay(15); > > + while (status == SDVO_CMD_STATUS_PENDING && --retry) { > > Hey, why did you switch from post to pre decrement? It will now retry > only retry-1 times. Or is this about the semantics of retries vs. tries? > ;) Because on the last go through, inside the loop retry would be 255 and we would not get the final 15ms sleep. -Chris -- Chris Wilson, Intel Open Source Technology Centre [-- Attachment #2: Type: text/plain, Size: 159 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Increase the response time for slow SDVO devices 2012-11-23 12:47 ` Chris Wilson @ 2012-11-23 13:19 ` Jani Nikula 0 siblings, 0 replies; 8+ messages in thread From: Jani Nikula @ 2012-11-23 13:19 UTC (permalink / raw) To: Chris Wilson, intel-gfx On Fri, 23 Nov 2012, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Fri, 23 Nov 2012 14:21:58 +0200, Jani Nikula <jani.nikula@linux.intel.com> wrote: >> On Fri, 23 Nov 2012, Chris Wilson <chris@chris-wilson.co.uk> 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... >> > >> > v2: Simplify the loop constructs as suggested by Jani Nikula. >> > >> > References: https://bugs.freedesktop.org/show_bug.cgi?id=36997 >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> > --- >> > drivers/gpu/drm/i915/intel_sdvo.c | 31 +++++++++++++++++++------------ >> > 1 file changed, 19 insertions(+), 12 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c >> > index d85ebb0..cff3c0b 100644 >> > --- a/drivers/gpu/drm/i915/intel_sdvo.c >> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c >> > @@ -509,7 +509,7 @@ out: >> > static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo, >> > void *response, int response_len) >> > { >> > - u8 retry = 5; >> > + u8 retry = 15; /* 5 quick checks, followed by 10 long checks */ >> > u8 status; >> > int i; >> > >> > @@ -522,14 +522,27 @@ 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. >> > + * Originally (in the DDX long ago), the delay was only ever 15ms >> > + * with an additional delay of 30ms applied for TVs added later after >> > + * many experiments. To accommodate both sets of delays, we do a >> > + * sequence of slow checks if the device is falling behind and fails >> > + * to reply within 5*15µs. >> > */ >> > if (!intel_sdvo_read_byte(intel_sdvo, >> > SDVO_I2C_CMD_STATUS, >> > &status)) >> > goto log_fail; >> > >> > - while (status == SDVO_CMD_STATUS_PENDING && retry--) { >> > - udelay(15); >> > + while (status == SDVO_CMD_STATUS_PENDING && --retry) { >> >> Hey, why did you switch from post to pre decrement? It will now retry >> only retry-1 times. Or is this about the semantics of retries vs. tries? >> ;) > > Because on the last go through, inside the loop retry would be 255 and > we would not get the final 15ms sleep. Right. The r-b applies. Jani. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Increase the response time for slow SDVO devices 2012-11-23 11:57 ` Chris Wilson 2012-11-23 12:21 ` Jani Nikula @ 2012-11-26 18:45 ` Daniel Vetter 1 sibling, 0 replies; 8+ messages in thread From: Daniel Vetter @ 2012-11-26 18:45 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx On Fri, Nov 23, 2012 at 11:57:56AM +0000, Chris Wilson 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... > > v2: Simplify the loop constructs as suggested by Jani Nikula. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=36997 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-11-26 18:44 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-11-23 8:44 [PATCH] drm/i915: Increase the response time for slow SDVO devices Chris Wilson 2012-11-23 11:42 ` Jani Nikula 2012-11-23 11:47 ` Chris Wilson 2012-11-23 11:57 ` Chris Wilson 2012-11-23 12:21 ` Jani Nikula 2012-11-23 12:47 ` Chris Wilson 2012-11-23 13:19 ` Jani Nikula 2012-11-26 18:45 ` Daniel Vetter
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.