Re: [PATCH 1/8] drm/i915/dp: retry link status read 3 times on failure

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Fri, 01 Jul 2011 16:41:06 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Fri,  1 Jul 2011 15:22:51 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > Especially after a hotplug or power status change, the sink may not
> > reply immediately to a link status query.  So retry 3 times per the spec
> > to really make sure nothing is there.
> 
> There's no 'false' return path here. I think you want:
> > 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c |   17 +++++++++++++----
> >  1 files changed, 13 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 391b55f..1829ecc 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1019,13 +1019,22 @@ intel_dp_dpms(struct drm_encoder *encoder, int mode)
> >  static bool
> >  intel_dp_get_link_status(struct intel_dp *intel_dp)
> >  {
> > -	int ret;
> > +	int ret, i;
> > +
> > +	/* Must try AUX reads for this at least 3 times */
> > +	for (i = 0; i < 3; i++) {
> > +		ret = intel_dp_aux_native_read(intel_dp,
> > +					       DP_LANE0_1_STATUS,
> > +					       intel_dp->link_status,
> > +					       DP_LINK_STATUS_SIZE);
> > +		if (ret == DP_LINK_STATUS_SIZE)
> > +			break;
> 
>                         return true;
> > +		msleep(1);
> > +	}
> >  
> > -	ret = intel_dp_aux_native_read(intel_dp,
> > -				       DP_LANE0_1_STATUS,
> > -				       intel_dp->link_status, DP_LINK_STATUS_SIZE);
> >  	if (ret != DP_LINK_STATUS_SIZE)
> >  		return false;

I think we'd hit the "return false" above if the loop failed.  But
simply returning true from the loop and false otherwise is clearer and
fewer lines.  Will fix.

-- 
Jesse Barnes, Intel Open Source Technology Center