Hi all,
The patchset contains few fixes for sii8620 driver. I2C recovery will be effective only if I2C adapter support I2C recovery, patches for HSI2C (I2C adapter used by sii8620 device on TM2/TM2e platforms) are already sent [1].
[1]: https://marc.info/?l=linux-i2c&m=151205223005257
Regards Andrzej
Andrzej Hajda (3): drm/bridge/sii8620: perform I2C bus recovery after reset drm/bridge/sii8620: simplify hardware reset procedure drm/bridge/sii8620: fix loops in EDID fetch logic
drivers/gpu/drm/bridge/sil-sii8620.c | 44 ++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 17 deletions(-)
Chip has known bug which causes I2C client state machine to frequently enter non-idle mode after chip reset. Let's ask I2C adapter to perform bus recovery to mitigate this bug.
Signed-off-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/bridge/sil-sii8620.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c index 86789f8918a4..db93e5e0497c 100644 --- a/drivers/gpu/drm/bridge/sil-sii8620.c +++ b/drivers/gpu/drm/bridge/sil-sii8620.c @@ -978,6 +978,8 @@ static int sii8620_hw_off(struct sii8620 *ctx)
static void sii8620_hw_reset(struct sii8620 *ctx) { + struct i2c_client *i2c = to_i2c_client(ctx->dev); + usleep_range(10000, 20000); gpiod_set_value(ctx->gpio_reset, 0); usleep_range(5000, 20000); @@ -985,6 +987,9 @@ static void sii8620_hw_reset(struct sii8620 *ctx) usleep_range(10000, 20000); gpiod_set_value(ctx->gpio_reset, 0); msleep(300); + + /* I2C bus recovery prevents I2C errors due to known bug in the chip */ + i2c_recover_bus(i2c->adapter); }
static void sii8620_cbus_reset(struct sii8620 *ctx)
On 15.01.2018 18:33, Andrzej Hajda wrote:
Chip has known bug which causes I2C client state machine to frequently enter non-idle mode after chip reset. Let's ask I2C adapter to perform bus recovery to mitigate this bug.
Signed-off-by: Andrzej Hajda a.hajda@samsung.com
drivers/gpu/drm/bridge/sil-sii8620.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c index 86789f8918a4..db93e5e0497c 100644 --- a/drivers/gpu/drm/bridge/sil-sii8620.c +++ b/drivers/gpu/drm/bridge/sil-sii8620.c @@ -978,6 +978,8 @@ static int sii8620_hw_off(struct sii8620 *ctx)
static void sii8620_hw_reset(struct sii8620 *ctx) {
- struct i2c_client *i2c = to_i2c_client(ctx->dev);
- usleep_range(10000, 20000); gpiod_set_value(ctx->gpio_reset, 0); usleep_range(5000, 20000);
@@ -985,6 +987,9 @@ static void sii8620_hw_reset(struct sii8620 *ctx) usleep_range(10000, 20000); gpiod_set_value(ctx->gpio_reset, 0); msleep(300);
- /* I2C bus recovery prevents I2C errors due to known bug in the chip */
- i2c_recover_bus(i2c->adapter);
}
static void sii8620_cbus_reset(struct sii8620 *ctx)
I have just posted patch which deals with the problem directly in i2c adapter, so this patch can be dropped[1].
[1]: https://marc.info/?l=linux-i2c&m=151696860906805
Regards
Andrzej
There is no need to flip reset pin twice. Also delays can be changed to values present in vendor's code.
Signed-off-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/bridge/sil-sii8620.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c index db93e5e0497c..7c46847fef18 100644 --- a/drivers/gpu/drm/bridge/sil-sii8620.c +++ b/drivers/gpu/drm/bridge/sil-sii8620.c @@ -980,13 +980,9 @@ static void sii8620_hw_reset(struct sii8620 *ctx) { struct i2c_client *i2c = to_i2c_client(ctx->dev);
- usleep_range(10000, 20000); - gpiod_set_value(ctx->gpio_reset, 0); - usleep_range(5000, 20000); - gpiod_set_value(ctx->gpio_reset, 1); - usleep_range(10000, 20000); + msleep(100); gpiod_set_value(ctx->gpio_reset, 0); - msleep(300); + msleep(100);
/* I2C bus recovery prevents I2C errors due to known bug in the chip */ i2c_recover_bus(i2c->adapter);
Function should constantly check if cable is connected and finish in finite time.
Signed-off-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/bridge/sil-sii8620.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c index 7c46847fef18..f65e14836c0e 100644 --- a/drivers/gpu/drm/bridge/sil-sii8620.c +++ b/drivers/gpu/drm/bridge/sil-sii8620.c @@ -801,6 +801,7 @@ static void sii8620_burst_rx_all(struct sii8620 *ctx) static void sii8620_fetch_edid(struct sii8620 *ctx) { u8 lm_ddc, ddc_cmd, int3, cbus; + unsigned long timeout; int fetched, i; int edid_len = EDID_LENGTH; u8 *edid; @@ -850,23 +851,31 @@ static void sii8620_fetch_edid(struct sii8620 *ctx) REG_DDC_CMD, ddc_cmd | VAL_DDC_CMD_ENH_DDC_READ_NO_ACK );
- do { - int3 = sii8620_readb(ctx, REG_INTR3); + int3 = 0; + timeout = jiffies + msecs_to_jiffies(200); + for (;;) { cbus = sii8620_readb(ctx, REG_CBUS_STATUS); - - if (int3 & BIT_DDC_CMD_DONE) - break; - - if (!(cbus & BIT_CBUS_STATUS_CBUS_CONNECTED)) { + if (~cbus & BIT_CBUS_STATUS_CBUS_CONNECTED) { + kfree(edid); + edid = NULL; + goto end; + } + if (int3 & BIT_DDC_CMD_DONE) { + if (sii8620_readb(ctx, REG_DDC_DOUT_CNT) + >= FETCH_SIZE) + break; + } else { + int3 = sii8620_readb(ctx, REG_INTR3); + } + if (time_is_before_jiffies(timeout)) { + ctx->error = -ETIMEDOUT; + dev_err(ctx->dev, "timeout during EDID read\n"); kfree(edid); edid = NULL; goto end; } - } while (1); - - sii8620_readb(ctx, REG_DDC_STATUS); - while (sii8620_readb(ctx, REG_DDC_DOUT_CNT) < FETCH_SIZE) usleep_range(10, 20); + }
sii8620_read_buf(ctx, REG_DDC_DATA, edid + fetched, FETCH_SIZE); if (fetched + FETCH_SIZE == EDID_LENGTH) {
Hi Andrzej,
On 2018-01-15 18:33, Andrzej Hajda wrote:
Function should constantly check if cable is connected and finish in finite time.
Signed-off-by: Andrzej Hajda a.hajda@samsung.com
Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/sil-sii8620.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c index 7c46847fef18..f65e14836c0e 100644 --- a/drivers/gpu/drm/bridge/sil-sii8620.c +++ b/drivers/gpu/drm/bridge/sil-sii8620.c @@ -801,6 +801,7 @@ static void sii8620_burst_rx_all(struct sii8620 *ctx) static void sii8620_fetch_edid(struct sii8620 *ctx) { u8 lm_ddc, ddc_cmd, int3, cbus;
- unsigned long timeout; int fetched, i; int edid_len = EDID_LENGTH; u8 *edid;
@@ -850,23 +851,31 @@ static void sii8620_fetch_edid(struct sii8620 *ctx) REG_DDC_CMD, ddc_cmd | VAL_DDC_CMD_ENH_DDC_READ_NO_ACK );
do {int3 = sii8620_readb(ctx, REG_INTR3);
int3 = 0;timeout = jiffies + msecs_to_jiffies(200);for (;;) { cbus = sii8620_readb(ctx, REG_CBUS_STATUS);
if (int3 & BIT_DDC_CMD_DONE)break;if (!(cbus & BIT_CBUS_STATUS_CBUS_CONNECTED)) {
if (~cbus & BIT_CBUS_STATUS_CBUS_CONNECTED) {kfree(edid);edid = NULL;goto end;}if (int3 & BIT_DDC_CMD_DONE) {if (sii8620_readb(ctx, REG_DDC_DOUT_CNT)>= FETCH_SIZE)break;} else {int3 = sii8620_readb(ctx, REG_INTR3);}if (time_is_before_jiffies(timeout)) {ctx->error = -ETIMEDOUT;dev_err(ctx->dev, "timeout during EDID read\n"); kfree(edid); edid = NULL; goto end; }
} while (1);sii8620_readb(ctx, REG_DDC_STATUS);while (sii8620_readb(ctx, REG_DDC_DOUT_CNT) < FETCH_SIZE) usleep_range(10, 20);
}sii8620_read_buf(ctx, REG_DDC_DATA, edid + fetched, FETCH_SIZE); if (fetched + FETCH_SIZE == EDID_LENGTH) {
Best regards
Hi Andrzej,
On 01/15/2018 06:33 PM, Andrzej Hajda wrote:
Function should constantly check if cable is connected and finish in finite time.
Signed-off-by: Andrzej Hajda a.hajda@samsung.com
Looks fine to me.
Reviewed-by: Maciej Purski m.purski@samsung.com
drivers/gpu/drm/bridge/sil-sii8620.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c index 7c46847fef18..f65e14836c0e 100644 --- a/drivers/gpu/drm/bridge/sil-sii8620.c +++ b/drivers/gpu/drm/bridge/sil-sii8620.c @@ -801,6 +801,7 @@ static void sii8620_burst_rx_all(struct sii8620 *ctx) static void sii8620_fetch_edid(struct sii8620 *ctx) { u8 lm_ddc, ddc_cmd, int3, cbus;
- unsigned long timeout; int fetched, i; int edid_len = EDID_LENGTH; u8 *edid;
@@ -850,23 +851,31 @@ static void sii8620_fetch_edid(struct sii8620 *ctx) REG_DDC_CMD, ddc_cmd | VAL_DDC_CMD_ENH_DDC_READ_NO_ACK );
do {int3 = sii8620_readb(ctx, REG_INTR3);
int3 = 0;timeout = jiffies + msecs_to_jiffies(200);for (;;) { cbus = sii8620_readb(ctx, REG_CBUS_STATUS);
if (int3 & BIT_DDC_CMD_DONE)break;if (!(cbus & BIT_CBUS_STATUS_CBUS_CONNECTED)) {
if (~cbus & BIT_CBUS_STATUS_CBUS_CONNECTED) {kfree(edid);edid = NULL;goto end;}if (int3 & BIT_DDC_CMD_DONE) {if (sii8620_readb(ctx, REG_DDC_DOUT_CNT)>= FETCH_SIZE)break;} else {int3 = sii8620_readb(ctx, REG_INTR3);}if (time_is_before_jiffies(timeout)) {ctx->error = -ETIMEDOUT;dev_err(ctx->dev, "timeout during EDID read\n"); kfree(edid); edid = NULL; goto end; }
} while (1);sii8620_readb(ctx, REG_DDC_STATUS);while (sii8620_readb(ctx, REG_DDC_DOUT_CNT) < FETCH_SIZE) usleep_range(10, 20);
}sii8620_read_buf(ctx, REG_DDC_DATA, edid + fetched, FETCH_SIZE); if (fetched + FETCH_SIZE == EDID_LENGTH) {
dri-devel@lists.freedesktop.org
-
Andrzej Hajda -
Maciej Purski -
Marek Szyprowski