[Intel-gfx] [PATCH] drm/i915: Increase the response time for slow SDVO devices

[Chris Wilson]

On Fri, 23 Nov 2012 14:21:58 +0200, Jani Nikula <jani.nikula at linux.intel.com> wrote:
> On Fri, 23 Nov 2012, Chris Wilson <chris at 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 at 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