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

2018-03-07 Thread Linus Walleij
On Mon, Mar 5, 2018 at 5:05 PM, Liviu Dudau  wrote:

>> Cc: Ville Syrjälä 
>> Cc: Liviu Dudau 
>
> I don't have a suitable configuration to test this, but looks goot to me.
>
> Reviewed-by: Liviu Dudau 

Thanks!

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

Very possible. I also suspect signal levels or something like
that, possibly no proper pull-up resistor on the line. Or
anything similar... it now falls back nicely to default EDID
modes though.

Yours,
Linus Walleij
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2018-03-05 Thread Liviu Dudau
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ä 
> Cc: Liviu Dudau 

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

Reviewed-by: Liviu Dudau 

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 
> ---
> 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