public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks
@ 2026-05-17 18:14 vishnu.saini
  2026-05-17 20:58 ` Dmitry Baryshkov
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: vishnu.saini @ 2026-05-17 18:14 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: dri-devel, linux-kernel, prahlad.valluru, Ravi Agola,
	Vishnu Saini

From: Ravi Agola <raviagol@qti.qualcomm.com>

The LT9611UXC bridge can fetch only 2 EDID blocks at a time, which
previously limited EDID reading to 2 blocks and prevented support
for displays exposing more than 2 EDID blocks.

Extend the driver to support up to 4 EDID blocks by re-triggering
EDID access after the first 2 blocks are read. For block 2, clear
the EDID ready flag in 0xb02a so the bridge can expose the remaining
blocks, then retry the read until the expected EDID is returned.

Also cache the full EDID blob in the driver and reuse it until the
next HPD disconnect event, so repeated EDID reads do not re-query
the bridge.

Signed-off-by: Ravi Agola <raviagol@qti.qualcomm.com>
Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
---
 drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 84 ++++++++++++++++++++++++++----
 1 file changed, 73 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
index 11aab07d88df..b5a9f62b9fe3 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
@@ -28,7 +28,7 @@
 #include <drm/display/drm_hdmi_audio_helper.h>
 
 #define EDID_BLOCK_SIZE	128
-#define EDID_NUM_BLOCKS	2
+#define EDID_NUM_BLOCKS	4
 
 #define FW_FILE "lt9611uxc_fw.bin"
 
@@ -61,6 +61,8 @@ struct lt9611uxc {
 	/* can be accessed from different threads, so protect this with ocm_lock */
 	bool hdmi_connected;
 	uint8_t fw_version;
+
+	const struct drm_edid *edid_data;
 };
 
 #define LT9611_PAGE_CONTROL	0xff
@@ -170,6 +172,12 @@ static void lt9611uxc_hpd_work(struct work_struct *work)
 	connected = lt9611uxc->hdmi_connected;
 	mutex_unlock(&lt9611uxc->ocm_lock);
 
+	if (!connected) {
+		lt9611uxc->edid_read = false;
+		drm_edid_free(lt9611uxc->edid_data);
+		lt9611uxc->edid_data = NULL;
+	}
+
 	drm_bridge_hpd_notify(&lt9611uxc->bridge,
 			      connected ?
 			      connector_status_connected :
@@ -384,10 +392,34 @@ static int lt9611uxc_wait_for_edid(struct lt9611uxc *lt9611uxc)
 			msecs_to_jiffies(500));
 }
 
+static int lt9611uxc_read_edid_block(struct lt9611uxc *lt9611uxc, unsigned int block,
+				     u8 *buf,  size_t len)
+{
+	int ret;
+
+	lt9611uxc_lock(lt9611uxc);
+
+	regmap_write(lt9611uxc->regmap, 0xb00a, (block%2) * EDID_BLOCK_SIZE);
+
+	ret = regmap_noinc_read(lt9611uxc->regmap, 0xb0b0, buf, len);
+	if (ret) {
+		dev_err(lt9611uxc->dev, "edid block %d read failed: %d\n", block, ret);
+		lt9611uxc_unlock(lt9611uxc);
+		return -EINVAL;
+	}
+	lt9611uxc_unlock(lt9611uxc);
+
+	return ret;
+}
+
 static int lt9611uxc_get_edid_block(void *data, u8 *buf, unsigned int block, size_t len)
 {
 	struct lt9611uxc *lt9611uxc = data;
-	int ret;
+	int ret = 0;
+	int retry_cnt = 10;
+	int edid_ext_block;
+	const u8 edid_header[8] = { 0x00, 0xFF, 0xFF, 0xFF,
+				    0xFF, 0xFF, 0xFF, 0x00 };
 
 	if (len > EDID_BLOCK_SIZE)
 		return -EINVAL;
@@ -395,19 +427,39 @@ static int lt9611uxc_get_edid_block(void *data, u8 *buf, unsigned int block, siz
 	if (block >= EDID_NUM_BLOCKS)
 		return -EINVAL;
 
-	lt9611uxc_lock(lt9611uxc);
+	if (block == 2) {
+		lt9611uxc_lock(lt9611uxc);
 
-	regmap_write(lt9611uxc->regmap, 0xb00b, 0x10);
+		/* Read number of block available in EDID data */
+		ret = regmap_read(lt9611uxc->regmap, 0xb02a, &edid_ext_block);
+		if (ret) {
+			dev_err(lt9611uxc->dev, "edid block read failed: %d\n", ret);
+			lt9611uxc_unlock(lt9611uxc);
+			return ret;
+		}
 
-	regmap_write(lt9611uxc->regmap, 0xb00a, block * EDID_BLOCK_SIZE);
+		/* Reset EDID ready flag so that lt9611uxc can read 2nd and 3rd block */
+		regmap_write(lt9611uxc->regmap, 0xb02a, (edid_ext_block & (~BIT(3))));
 
-	ret = regmap_noinc_read(lt9611uxc->regmap, 0xb0b0, buf, len);
-	if (ret)
-		dev_err(lt9611uxc->dev, "edid read failed: %d\n", ret);
+		lt9611uxc_unlock(lt9611uxc);
 
-	lt9611uxc_unlock(lt9611uxc);
+		msleep(100);
 
-	return 0;
+		ret = lt9611uxc_read_edid_block(lt9611uxc, block, buf, len);
+
+		/*
+		 * Compare first 8 bytes of EDID header (0th block) and 2nd block to confirm
+		 * that 2nd EDID block data is read successfully by lt9611uxc
+		 */
+		while (!ret && 0 == memcmp(&edid_header, &buf, 8) && retry_cnt-- > 0) {
+			msleep(100);
+			ret = lt9611uxc_read_edid_block(lt9611uxc, block, buf, len);
+		}
+	} else {
+		ret = lt9611uxc_read_edid_block(lt9611uxc, block, buf, len);
+	}
+
+	return ret;
 };
 
 static const struct drm_edid *lt9611uxc_bridge_edid_read(struct drm_bridge *bridge,
@@ -425,7 +477,17 @@ static const struct drm_edid *lt9611uxc_bridge_edid_read(struct drm_bridge *brid
 		return NULL;
 	}
 
-	return drm_edid_read_custom(connector, lt9611uxc_get_edid_block, lt9611uxc);
+	/* If EDID is read once, provide same EDID data till next HPD event */
+	if (lt9611uxc->edid_data == NULL) {
+		lt9611uxc->edid_data = drm_edid_read_custom(connector, lt9611uxc_get_edid_block,
+							   lt9611uxc);
+	}
+
+	/*
+	 * Copy the EDID data and return the copied value which will be freed by DRM.
+	 * The original EDID data is cached in the driver until the next HPD event.
+	 */
+	return drm_edid_dup(lt9611uxc->edid_data);
 }
 
 static void lt9611uxc_bridge_hpd_notify(struct drm_bridge *bridge,

---
base-commit: 4c26e162947f91aa78ba57dd4fddd38fc80e7d60
change-id: 20260517-lt9611usc_edid34_misc_next-b02592de0b25

Best regards,
-- 
Vishnu Saini <vishnu.saini@oss.qualcomm.com>


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks
  2026-05-17 18:14 [PATCH] drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks vishnu.saini
@ 2026-05-17 20:58 ` Dmitry Baryshkov
  2026-05-18  0:16 ` kernel test robot
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Dmitry Baryshkov @ 2026-05-17 20:58 UTC (permalink / raw)
  To: vishnu.saini
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	dri-devel, linux-kernel, prahlad.valluru, Ravi Agola

On Sun, May 17, 2026 at 11:44:49PM +0530, vishnu.saini@oss.qualcomm.com wrote:
> From: Ravi Agola <raviagol@qti.qualcomm.com>
> 
> The LT9611UXC bridge can fetch only 2 EDID blocks at a time, which
> previously limited EDID reading to 2 blocks and prevented support
> for displays exposing more than 2 EDID blocks.
> 
> Extend the driver to support up to 4 EDID blocks by re-triggering
> EDID access after the first 2 blocks are read. For block 2, clear
> the EDID ready flag in 0xb02a so the bridge can expose the remaining
> blocks, then retry the read until the expected EDID is returned.

Won't this affect re-reading of the EDID? If so, we might need to reset
the flag at the block0 path too.

> 
> Also cache the full EDID blob in the driver and reuse it until the
> next HPD disconnect event, so repeated EDID reads do not re-query
> the bridge.

Separte commit, separate justification. Most of the drivers don't use
the cache, so, I'd say, most likely no.

> 
> Signed-off-by: Ravi Agola <raviagol@qti.qualcomm.com>
> Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 84 ++++++++++++++++++++++++++----
>  1 file changed, 73 insertions(+), 11 deletions(-)
> 
-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks
  2026-05-17 18:14 [PATCH] drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks vishnu.saini
  2026-05-17 20:58 ` Dmitry Baryshkov
@ 2026-05-18  0:16 ` kernel test robot
  2026-05-18  0:36 ` kernel test robot
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2026-05-18  0:16 UTC (permalink / raw)
  To: vishnu.saini, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter
  Cc: oe-kbuild-all, dri-devel, linux-kernel, prahlad.valluru,
	Ravi Agola, Vishnu Saini

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 4c26e162947f91aa78ba57dd4fddd38fc80e7d60]

url:    https://github.com/intel-lab-lkp/linux/commits/vishnu-saini-oss-qualcomm-com/drm-bridge-lt9611uxc-support-displays-with-up-to-4-EDID-blocks/20260518-021632
base:   4c26e162947f91aa78ba57dd4fddd38fc80e7d60
patch link:    https://lore.kernel.org/r/20260517-lt9611usc_edid34_misc_next-v1-1-5e2fd8c6399b%40oss.qualcomm.com
patch subject: [PATCH] drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks
config: csky-randconfig-001-20260518 (https://download.01.org/0day-ci/archive/20260518/202605180850.uN2x73KR-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260518/202605180850.uN2x73KR-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605180850.uN2x73KR-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/bridge/lontium-lt9611uxc.c: In function 'lt9611uxc_get_edid_block':
>> drivers/gpu/drm/bridge/lontium-lt9611uxc.c:454:37: warning: 'memcmp' specified bound 8 exceeds source size 4 [-Wstringop-overread]
     454 |                 while (!ret && 0 == memcmp(&edid_header, &buf, 8) && retry_cnt-- > 0) {
         |                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/bridge/lontium-lt9611uxc.c:415:53: note: source object allocated here
     415 | static int lt9611uxc_get_edid_block(void *data, u8 *buf, unsigned int block, size_t len)
         |                                                 ~~~~^~~


vim +/memcmp +454 drivers/gpu/drm/bridge/lontium-lt9611uxc.c

   414	
   415	static int lt9611uxc_get_edid_block(void *data, u8 *buf, unsigned int block, size_t len)
   416	{
   417		struct lt9611uxc *lt9611uxc = data;
   418		int ret = 0;
   419		int retry_cnt = 10;
   420		int edid_ext_block;
   421		const u8 edid_header[8] = { 0x00, 0xFF, 0xFF, 0xFF,
   422					    0xFF, 0xFF, 0xFF, 0x00 };
   423	
   424		if (len > EDID_BLOCK_SIZE)
   425			return -EINVAL;
   426	
   427		if (block >= EDID_NUM_BLOCKS)
   428			return -EINVAL;
   429	
   430		if (block == 2) {
   431			lt9611uxc_lock(lt9611uxc);
   432	
   433			/* Read number of block available in EDID data */
   434			ret = regmap_read(lt9611uxc->regmap, 0xb02a, &edid_ext_block);
   435			if (ret) {
   436				dev_err(lt9611uxc->dev, "edid block read failed: %d\n", ret);
   437				lt9611uxc_unlock(lt9611uxc);
   438				return ret;
   439			}
   440	
   441			/* Reset EDID ready flag so that lt9611uxc can read 2nd and 3rd block */
   442			regmap_write(lt9611uxc->regmap, 0xb02a, (edid_ext_block & (~BIT(3))));
   443	
   444			lt9611uxc_unlock(lt9611uxc);
   445	
   446			msleep(100);
   447	
   448			ret = lt9611uxc_read_edid_block(lt9611uxc, block, buf, len);
   449	
   450			/*
   451			 * Compare first 8 bytes of EDID header (0th block) and 2nd block to confirm
   452			 * that 2nd EDID block data is read successfully by lt9611uxc
   453			 */
 > 454			while (!ret && 0 == memcmp(&edid_header, &buf, 8) && retry_cnt-- > 0) {
   455				msleep(100);
   456				ret = lt9611uxc_read_edid_block(lt9611uxc, block, buf, len);
   457			}
   458		} else {
   459			ret = lt9611uxc_read_edid_block(lt9611uxc, block, buf, len);
   460		}
   461	
   462		return ret;
   463	};
   464	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks
  2026-05-17 18:14 [PATCH] drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks vishnu.saini
  2026-05-17 20:58 ` Dmitry Baryshkov
  2026-05-18  0:16 ` kernel test robot
@ 2026-05-18  0:36 ` kernel test robot
  2026-05-18  6:00 ` Claude review: " Claude Code Review Bot
  2026-05-18  6:00 ` Claude Code Review Bot
  4 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2026-05-18  0:36 UTC (permalink / raw)
  To: vishnu.saini, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter
  Cc: oe-kbuild-all, dri-devel, linux-kernel, prahlad.valluru,
	Ravi Agola, Vishnu Saini

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on 4c26e162947f91aa78ba57dd4fddd38fc80e7d60]

url:    https://github.com/intel-lab-lkp/linux/commits/vishnu-saini-oss-qualcomm-com/drm-bridge-lt9611uxc-support-displays-with-up-to-4-EDID-blocks/20260518-021632
base:   4c26e162947f91aa78ba57dd4fddd38fc80e7d60
patch link:    https://lore.kernel.org/r/20260517-lt9611usc_edid34_misc_next-v1-1-5e2fd8c6399b%40oss.qualcomm.com
patch subject: [PATCH] drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks
config: i386-randconfig-014-20260518 (https://download.01.org/0day-ci/archive/20260518/202605180800.luZQCJZK-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260518/202605180800.luZQCJZK-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605180800.luZQCJZK-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/string.h:386,
                    from include/linux/bitmap.h:13,
                    from include/linux/cpumask.h:11,
                    from arch/x86/include/asm/paravirt.h:19,
                    from arch/x86/include/asm/irqflags.h:100,
                    from include/linux/irqflags.h:18,
                    from include/linux/spinlock.h:59,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:7,
                    from include/linux/firmware.h:8,
                    from drivers/gpu/drm/bridge/lontium-lt9611uxc.c:7:
   In function 'memcmp',
       inlined from 'lt9611uxc_get_edid_block' at drivers/gpu/drm/bridge/lontium-lt9611uxc.c:454:23:
>> include/linux/fortify-string.h:719:25: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object (2nd parameter)
     719 |                         __read_overflow2();
         |                         ^~~~~~~~~~~~~~~~~~


vim +/__read_overflow2 +719 include/linux/fortify-string.h

a28a6e860c6cf23 Francis Laniel 2021-02-25  708  
92df138a8d663ce Kees Cook      2022-02-08  709  __FORTIFY_INLINE __diagnose_as(__builtin_memcmp, 1, 2, 3)
281d0c962752fb4 Kees Cook      2022-02-08  710  int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t size)
a28a6e860c6cf23 Francis Laniel 2021-02-25  711  {
21a2c74b0a2a784 Kees Cook      2023-04-07  712  	const size_t p_size = __struct_size(p);
21a2c74b0a2a784 Kees Cook      2023-04-07  713  	const size_t q_size = __struct_size(q);
a28a6e860c6cf23 Francis Laniel 2021-02-25  714  
a28a6e860c6cf23 Francis Laniel 2021-02-25  715  	if (__builtin_constant_p(size)) {
fa35198f39571bb Kees Cook      2022-09-19  716  		if (__compiletime_lessthan(p_size, size))
a28a6e860c6cf23 Francis Laniel 2021-02-25  717  			__read_overflow();
fa35198f39571bb Kees Cook      2022-09-19  718  		if (__compiletime_lessthan(q_size, size))
a28a6e860c6cf23 Francis Laniel 2021-02-25 @719  			__read_overflow2();
a28a6e860c6cf23 Francis Laniel 2021-02-25  720  	}
3d965b33e40d973 Kees Cook      2023-04-07  721  	if (p_size < size)
3d965b33e40d973 Kees Cook      2023-04-07  722  		fortify_panic(FORTIFY_FUNC_memcmp, FORTIFY_READ, p_size, size, INT_MIN);
3d965b33e40d973 Kees Cook      2023-04-07  723  	else if (q_size < size)
3d965b33e40d973 Kees Cook      2023-04-07  724  		fortify_panic(FORTIFY_FUNC_memcmp, FORTIFY_READ, q_size, size, INT_MIN);
a28a6e860c6cf23 Francis Laniel 2021-02-25  725  	return __underlying_memcmp(p, q, size);
a28a6e860c6cf23 Francis Laniel 2021-02-25  726  }
a28a6e860c6cf23 Francis Laniel 2021-02-25  727  

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Claude review: drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks
  2026-05-17 18:14 [PATCH] drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks vishnu.saini
                   ` (3 preceding siblings ...)
  2026-05-18  6:00 ` Claude review: " Claude Code Review Bot
@ 2026-05-18  6:00 ` Claude Code Review Bot
  4 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-18  6:00 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks
Author: vishnu.saini@oss.qualcomm.com
Patches: 4
Reviewed: 2026-05-18T16:00:28.122513

---

This single-patch series extends the LT9611UXC bridge driver to read up to 4 EDID blocks (previously 2), and caches the EDID until the next HPD disconnect. The goal is reasonable — many modern displays expose CTA extension blocks beyond the first two — but the implementation has a **critical memory corruption bug** in the retry loop, several concurrency concerns, a missing cleanup in `lt9611uxc_remove`, and some code quality issues.

**Verdict: Needs revision.** The memcmp bug alone would cause undefined behavior in every retry path.

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Claude review: drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks
  2026-05-17 18:14 [PATCH] drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks vishnu.saini
                   ` (2 preceding siblings ...)
  2026-05-18  0:36 ` kernel test robot
@ 2026-05-18  6:00 ` Claude Code Review Bot
  2026-05-18  6:00 ` Claude Code Review Bot
  4 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-18  6:00 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**BUG (critical): `memcmp` compares wrong addresses**

```c
while (!ret && 0 == memcmp(&edid_header, &buf, 8) && retry_cnt-- > 0) {
```

`buf` is a `u8 *` (pointer). Taking `&buf` gives the address of the pointer variable on the stack, **not** the address of the buffer contents. This means `memcmp` compares 8 bytes of stack memory (the pointer value + whatever follows) against the EDID header, which will almost never match. The retry loop will effectively never execute. Similarly, `&edid_header` works by accident here since `edid_header` is an array (so `&edid_header == edid_header` address-wise), but for consistency both should just be `edid_header` and `buf`:

```c
while (!ret && memcmp(edid_header, buf, 8) == 0 && retry_cnt-- > 0) {
```

Also, the Yoda-style `0 == memcmp(...)` is not idiomatic kernel style.

**BUG: Missing `edid_data` cleanup in `lt9611uxc_remove`**

The patch adds `edid_data` caching but never frees it in `lt9611uxc_remove()`. If the device is removed while a display is connected (and EDID is cached), the `drm_edid` allocation leaks. `lt9611uxc_remove` needs:

```c
drm_edid_free(lt9611uxc->edid_data);
```

**Issue: Missing `regmap_write(0xb00b, 0x10)` for blocks 2–3**

The original code writes `regmap_write(lt9611uxc->regmap, 0xb00b, 0x10)` before reading an EDID block. The new `lt9611uxc_read_edid_block()` helper omits this write entirely. For blocks 0 and 1 this may work because the old code path previously set it, but the intent of that register write is unclear and dropping it silently could cause issues. If register `0xb00b` is needed to set up EDID reading, it should be included in `lt9611uxc_read_edid_block`. If it's not needed, its removal from the block 0/1 path should be explicitly noted in the commit message.

**Issue: Concurrency on `edid_data` cache**

`edid_data` is read/written from `lt9611uxc_bridge_edid_read()` and `lt9611uxc_hpd_work()` without any locking. The HPD work function clears `edid_data` while `edid_read` could be simultaneously accessing it. The `ocm_lock` mutex is held for `hdmi_connected` access but the new `edid_data` manipulation in `lt9611uxc_hpd_work` happens *outside* that lock:

```c
mutex_unlock(&lt9611uxc->ocm_lock);

if (!connected) {
    lt9611uxc->edid_read = false;
    drm_edid_free(lt9611uxc->edid_data);
    lt9611uxc->edid_data = NULL;
}
```

This creates a TOCTOU race: `lt9611uxc_bridge_edid_read` could see `edid_data != NULL`, then HPD work frees it, then `edid_read` calls `drm_edid_dup` on freed memory. The cache operations need to be protected (either extend the existing `ocm_lock` scope or add separate locking).

**Issue: `edid_read` flag set outside lock**

```c
lt9611uxc->edid_read = false;
```

This is set in `lt9611uxc_hpd_work` without holding `ocm_lock`, but it's set by the IRQ handler under `ocm_lock` (line 150 in the original). Inconsistent locking on a shared flag.

**Nit: Coding style**

Missing space before modulo operator:
```c
regmap_write(lt9611uxc->regmap, 0xb00a, (block%2) * EDID_BLOCK_SIZE);
```
Should be `(block % 2)`.

**Nit: Unnecessary double-space**

```c
u8 *buf,  size_t len)
```

Two spaces before `size_t`.

**Nit: Return value masking**

```c
ret = regmap_noinc_read(lt9611uxc->regmap, 0xb0b0, buf, len);
if (ret) {
    dev_err(lt9611uxc->dev, "edid block %d read failed: %d\n", block, ret);
    lt9611uxc_unlock(lt9611uxc);
    return -EINVAL;
}
```

This replaces `ret` (which is the actual regmap error code) with `-EINVAL`. It should just `return ret` to propagate the real error. Alternatively, use a `goto` with a single unlock path to avoid the duplicated `lt9611uxc_unlock` call.

**Design: Retry logic is fragile**

The retry checks whether block 2 data still looks like block 0 (EDID header) as a staleness indicator. This is clever but fragile — if block 2 legitimately starts with `00 FF FF FF FF FF FF 00` (very unlikely for a CEA extension but not formally impossible), the retry loop would spin all 10 iterations pointlessly. A comment explaining the hardware behavior would help. Also, the 10 retries × 100ms = 1 second total, plus the initial 100ms, gives 1.1 seconds maximum. It would be good to document whether this is sufficient for the hardware.

**Summary of required changes:**
1. Fix `&buf` → `buf` in the `memcmp` call (critical bug)
2. Add `drm_edid_free(lt9611uxc->edid_data)` in `lt9611uxc_remove` (leak)
3. Add locking around `edid_data` accesses (race condition)
4. Clarify or restore the `0xb00b` register write
5. Minor style fixes

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-05-18  6:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-17 18:14 [PATCH] drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks vishnu.saini
2026-05-17 20:58 ` Dmitry Baryshkov
2026-05-18  0:16 ` kernel test robot
2026-05-18  0:36 ` kernel test robot
2026-05-18  6:00 ` Claude review: " Claude Code Review Bot
2026-05-18  6:00 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox