Re: [PATCH] drm/bridge: sii902x: Retry status read after DDI I2C

[Liviu Dudau <Liviu.Dudau@arm.com>]

On Mon, Mar 05, 2018 at 11:17:02AM +0100, Linus Walleij wrote:
> The following happens when connection a DVI output driven
> from the SiI9022 using a DVI-to-VGA adapter plug:
> 
> i2c i2c-0: sendbytes: NAK bailout.
> i2c i2c-0: sendbytes: NAK bailout.
> 
> Then no picture. Apparently the I2C engine inside the SiI9022
> is not smart enough to try to fall back to DDC I2C. Or the
> vendor have not integrated the electronics properly. I don't
> know which one it is.
> 
> After this, the I2C bus seems stalled and the first attempt to
> read the status register fails, and the code returns with
> negative return value, and the display fails to initialized.
> 
> Instead, retry status readout five times and continue even
> if this fails.
> 
> Tested on the ARM Versatile Express with a DVI-to-VGA
> connector, it now gives picture.
> 
> Introduce a helper struct device *dev variable to make
> the code more readable.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Liviu Dudau <Liviu.Dudau@arm.com>

I don't have a suitable configuration to test this, but looks goot to me.

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

BTW, Linus, could it be that your DVI-to-VGA connector doesn't wire
correctly the EDID pin?

Best regards,
Liviu

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Found the real problem instead of trying to explicitly
>   shoehorn EDID modes into the error path.
> ---
>  drivers/gpu/drm/bridge/sii902x.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index b1ab4ab09532..60373d7eb220 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -137,7 +137,9 @@ static int sii902x_get_modes(struct drm_connector *connector)
>  	struct sii902x *sii902x = connector_to_sii902x(connector);
>  	struct regmap *regmap = sii902x->regmap;
>  	u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> +	struct device *dev = &sii902x->i2c->dev;
>  	unsigned long timeout;
> +	unsigned int retries;
>  	unsigned int status;
>  	struct edid *edid;
>  	int num = 0;
> @@ -159,7 +161,7 @@ static int sii902x_get_modes(struct drm_connector *connector)
>  		 time_before(jiffies, timeout));
>  
>  	if (!(status & SII902X_SYS_CTRL_DDC_BUS_GRTD)) {
> -		dev_err(&sii902x->i2c->dev, "failed to acquire the i2c bus\n");
> +		dev_err(dev, "failed to acquire the i2c bus\n");
>  		return -ETIMEDOUT;
>  	}
>  
> @@ -179,9 +181,19 @@ static int sii902x_get_modes(struct drm_connector *connector)
>  	if (ret)
>  		return ret;
>  
> -	ret = regmap_read(regmap, SII902X_SYS_CTRL_DATA, &status);
> +	/*
> +	 * Sometimes the I2C bus can stall after failure to use the
> +	 * EDID channel. Retry a few times to see if things clear
> +	 * up, else continue anyway.
> +	 */
> +	retries = 5;
> +	do {
> +		ret = regmap_read(regmap, SII902X_SYS_CTRL_DATA,
> +				  &status);
> +		retries--;
> +	} while (ret && retries);
>  	if (ret)
> -		return ret;
> +		dev_err(dev, "failed to read status (%d)\n", ret);
>  
>  	ret = regmap_update_bits(regmap, SII902X_SYS_CTRL_DATA,
>  				 SII902X_SYS_CTRL_DDC_BUS_REQ |
> @@ -201,7 +213,7 @@ static int sii902x_get_modes(struct drm_connector *connector)
>  
>  	if (status & (SII902X_SYS_CTRL_DDC_BUS_REQ |
>  		      SII902X_SYS_CTRL_DDC_BUS_GRTD)) {
> -		dev_err(&sii902x->i2c->dev, "failed to release the i2c bus\n");
> +		dev_err(dev, "failed to release the i2c bus\n");
>  		return -ETIMEDOUT;
>  	}
>  
> -- 
> 2.14.3
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel