Re: [PATCH] drm/i915: Increase the response time for slow SDVO devices

From: Jani Nikula <jani.nikula@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Increase the response time for slow SDVO devices
Date: Fri, 23 Nov 2012 15:19:48 +0200	[thread overview]
Message-ID: <87y5hsfocr.fsf@intel.com> (raw)
In-Reply-To: <275ffc$7hgj4p@fmsmga002.fm.intel.com>

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