Re: [PATCH v4] drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/bridge: lt9611uxc: race condition in EDID SRAM polling causes torn reads
- [Medium] drm/bridge: lt9611uxc: strict EDID header check breaks repairable displays
--

commit 6559cd85aa7ae1a652dd056a93747b35998d927d
Author: Ravi Agola <raviagol@qti.qualcomm.com>

drm/bridge: lt9611uxc: support displays with up to 4 EDID blocks

This commit adds support for fetching up to 4 EDID blocks in the LT9611UXC
bridge driver by re-triggering access for blocks beyond the first two. It
introduces a polling mechanism for blocks 0 and 2, checking for a valid EDID
header match to verify read success. It also increases the EDID wait time to
handle slower platform interrupts.

> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> index b3bb7f2cebb3b..a74077a65f62b 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
[ ... ]
> @@ -394,20 +422,48 @@ 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 == 0 || block == 2) {
>  
> -	regmap_write(lt9611uxc->regmap, 0xb00b, 0x10);
> +		lt9611uxc_lock(lt9611uxc);
>  
> -	regmap_write(lt9611uxc->regmap, 0xb00a, block * EDID_BLOCK_SIZE);
> +		edid_ready_flag = (block == 0) ? BIT(0) : BIT(1);
>  
> -	ret = regmap_noinc_read(lt9611uxc->regmap, 0xb0b0, buf, len);
> -	if (ret)
> -		dev_err(lt9611uxc->dev, "edid read failed: %d\n", ret);
> +		/*
> +		 * Set the EDID ready flag so that lt9611uxc can fetch correct EDID block
> +		 */
> +		regmap_write(lt9611uxc->regmap, 0xb028, edid_ready_flag);
>  
> -	lt9611uxc_unlock(lt9611uxc);
> +		lt9611uxc_unlock(lt9611uxc);
>  
> -	return 0;
> -};
> +		do {
> +			msleep(100);
> +			ret = lt9611uxc_read_edid_block(lt9611uxc, block, buf, len);
> +			if (ret)
> +				break;
> +			/*
> +			 * Compare first 8 bytes of EDID header for block 0 and block 2
> +			 * to confirm EDID read successfully
> +			 */
> +			header_matched = (memcmp(edid_header, buf, 8) == 0);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Will this strict exact match check inadvertently drop blocks with repairable
header corruptions?

If the display provides a corrupted header that the DRM core is natively
designed to fix via edid_header_fix(), it seems this strict memcmp will never
match and the polling loop will exhaust its retries, returning -ETIMEDOUT.
Does this break initialization for displays with non-compliant but repairable
EDID headers?

> +			edid_valid = (block == 0 && header_matched) ||
> +				     (block == 2 && !header_matched);
> +			if (edid_valid)
> +				break;
                                ^^^^^

[Severity: High]
Does this condition break the wait loop too early, ignoring that the rest of
the block is still being written by the MCU?

When the DRM core requests block 2, the bridge driver writes to 0xb028 to
trigger the MCU fetch, and then polls the SRAM. The SRAM initially contains
block 0 (which has a valid header). As soon as the MCU overwrites the first
byte of SRAM with block 2 data, the !header_matched condition evaluates to
true.

Could this cause the driver to immediately stop polling and read the rest of
the block, which might still contain stale data from block 0?

> +
> +		} while (retry_cnt-- > 0);
> +
> +		if (!ret && !edid_valid)
> +			ret = -ETIMEDOUT;
> +	} else {
> +		ret = lt9611uxc_read_edid_block(lt9611uxc, block, buf, len);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Blocks 1 and 3 are read immediately here without any synchronization. Does
this guarantee reading torn data if the firmware is still fetching them over
I2C?

> +
> +		lt9611uxc_lock(lt9611uxc);
> +		regmap_write(lt9611uxc->regmap, 0xb028, 0x00);
> +		lt9611uxc_unlock(lt9611uxc);
> +	}
> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922-lt9611usc_edid34_misc_next-v4-1-47d5315821fe@oss.qualcomm.com?part=1