Skip to contentSkip to conversation

drm: i2c: tda998x: Retry fetching the EDID if it fails first time.

PATCH In review Andrew Jackson 2 participants 5 messages Nov 18, 2014 – Dec 1, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drm: i2c: tda998x: Retry fetching the EDID if it fails first time.

Fetching the EDID from a connected monitor is an automated thing with NXP TDA19988. But on some boards the fetching fails for the first time silently without any indication that an error has occured. More than that, subsequent fetches of the EDID succeed until the monitor(s) are unplugged.

Add a function to validate the read EDID and retry if the block retrieved is not valid.

---
 drivers/gpu/drm/i2c/tda998x_drv.c |   36 ++++++++++++++++++++++++++++--------
 1 files changed, 28 insertions(+), 8 deletions(-)
drivers/gpu/drm/i2c/tda998x_drv.c +28 -8
index d476279..025adc0 100644
@@ -1066,11 +1066,36 @@ static int read_edid_block(struct tda998x_priv *priv, uint8_t *buf, int blk)
 	return 0;
 }
 
+static int
+read_validate_edid_block(struct tda998x_priv *priv, uint8_t *buf, int blk)
+{
+	bool print_bad_edid = drm_debug & DRM_UT_KMS;
+	int ret;
+	int retries = 1;
+
+	do
+	{
+		bool print_bad = print_bad_edid && (retries == 0);
+
+		ret = read_edid_block(priv, buf, blk);
+		/* Fail on I2C error */
+		if (ret)
+			break;
+
+		/* But retry checksum errored blocks */
+		if (drm_edid_block_valid(buf, blk, print_bad))
+			break;
+		else
+			ret = -EINVAL;
+	} while (retries-- > 0);
+
+	return ret;
+}
+
 static uint8_t *do_get_edid(struct tda998x_priv *priv)
 {
 	int j, valid_extensions = 0;
 	uint8_t *block, *new;
-	bool print_bad_edid = drm_debug & DRM_UT_KMS;
 
 	if ((block = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
 		return NULL;
@@ -1079,10 +1104,7 @@ static uint8_t *do_get_edid(struct tda998x_priv *priv)
 		reg_clear(priv, REG_TX4, TX4_PD_RAM);
 
 	/* base block fetch */
-	if (read_edid_block(priv, block, 0))
-		goto fail;
-
-	if (!drm_edid_block_valid(block, 0, print_bad_edid))
+	if (read_validate_edid_block(priv, block, 0))
 		goto fail;
 
 	/* if there's no extensions, we're done */
@@ -1096,10 +1118,8 @@ static uint8_t *do_get_edid(struct tda998x_priv *priv)
 
 	for (j = 1; j <= block[0x7e]; j++) {
 		uint8_t *ext_block = block + (valid_extensions + 1) * EDID_LENGTH;
-		if (read_edid_block(priv, ext_block, j))
-			goto fail;
 
-		if (!drm_edid_block_valid(ext_block, j, print_bad_edid))
+		if (read_validate_edid_block(priv, ext_block, j))
 			goto fail;
 
 		valid_extensions++;
Signature · 1 line
1.7.1
#1 lore raw
Quoted 11 lines · On Tue, 18 Nov 2014 17:41:26 +0000 Andrew Jackson <[email protected]>…

Fetching the EDID from a connected monitor is an automated thing with NXP TDA19988. But on some boards the fetching fails for the first time silently without any indication that an error has occured. More than that, subsequent fetches of the EDID succeed until the monitor(s) are unplugged.

Add a function to validate the read EDID and retry if the block retrieved is not valid.

It seems that your patch is deprecated by Laurent Pinchart's

	[PATCH] drm: tda998x: Use drm_do_get_edid()

http://lists.freedesktop.org/archives/dri-devel/2014-November/072906.html

Signature · 2 lines
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
#2 lore raw
Quoted 12 lines · Jean-Francois Moine
Quoted 9 lines · On Tue, 18 Nov 2014 17:41:26 +0000 Andrew Jackson <[email protected]>…

Fetching the EDID from a connected monitor is an automated thing with NXP TDA19988. But on some boards the fetching fails for the first time silently without any indication that an error has occured. More than that, subsequent fetches of the EDID succeed until the monitor(s) are unplugged.

Add a function to validate the read EDID and retry if the block retrieved is not valid.

Quoted 7 lines
Quoted 2 lines

It seems that your patch is deprecated by Laurent Pinchart's

	[PATCH] drm: tda998x: Use drm_do_get_edid()

http://lists.freedesktop.org/archives/dri-devel/2014-November/072906.html

Thank you for the heads-up, I'd not seen that. I'll consider how my patch might be modified to suit the new infrastructure.

Andrew

#3 lore raw
Quoted 6 lines · On Fri, 28 Nov 2014 09:02:39 +0000 Andrew Jackson <[email protected]>…
Quoted 4 lines

It seems that your patch is deprecated by Laurent Pinchart's

	[PATCH] drm: tda998x: Use drm_do_get_edid()

http://lists.freedesktop.org/archives/dri-devel/2014-November/072906.html

Thank you for the heads-up, I'd not seen that. I'll consider how my patch might be modified to suit the new infrastructure.

You don't need any patch: drm_do_get_edid() already loops on reading the EDID.

Signature · 2 lines
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
#4 lore raw
Quoted 8 lines · Jean-Francois Moine
Quoted 5 lines · On Fri, 28 Nov 2014 09:02:39 +0000 Andrew Jackson <[email protected]>…
Quoted 4 lines

It seems that your patch is deprecated by Laurent Pinchart's

	[PATCH] drm: tda998x: Use drm_do_get_edid()

http://lists.freedesktop.org/archives/dri-devel/2014-November/072906.html

Quoted 5 lines
Quoted 1 line

Thank you for the heads-up, I'd not seen that. I'll consider how my patch might be modified to suit the new infrastructure.

You don't need any patch: drm_do_get_edid() already loops on reading the EDID.

Thank you again: that saves me work!

Andrew