On Wed, 03 Sep 2025, syyang <syy...@lontium.com> wrote:
> +static int lt9611c_read_edid(struct lt9611c *lt9611c)
> +{
> + struct device *dev = lt9611c->dev;
> + int ret, i, bytes_to_copy, offset = 0;
> + u8 packets_num;
> + u8 read_edid_data_cmd[5] = {0x52, 0x48, 0x33, 0x3A, 0x00};
> + u8 return_edid_data[37];
> + u8 read_edid_byte_num_cmd[5] = {0x52, 0x48, 0x32, 0x3A, 0x00};
> + u8 return_edid_byte_num[6];
> +
> + ret = i2c_read_write_flow(lt9611c, read_edid_byte_num_cmd, 5,
> return_edid_byte_num, 6);
> + if (ret) {
> + dev_err(dev, "Failed to read EDID byte number\n");
> + lt9611c->edid_valid = false;
> + return ret;
> + }
> +
> + lt9611c->edid_len = (return_edid_byte_num[4] << 8) |
> return_edid_byte_num[5];
> +
> + if (!lt9611c->edid_buf || lt9611c->edid_len > (lt9611c->edid_valid ?
> + lt9611c->edid_len : 0)) {
> + kfree(lt9611c->edid_buf);
> + lt9611c->edid_buf = kzalloc(lt9611c->edid_len, GFP_KERNEL);
> + if (!lt9611c->edid_buf) {
> + dev_err(dev, "Failed to allocate EDID buffer\n");
> + lt9611c->edid_len = 0;
> + lt9611c->edid_valid = false;
> + return -ENOMEM;
> + }
> + }
If you want to do caching, store a struct drm_edid pointer at a higher
level, not dumb buffers at the low level. Might be easier to start off
without any caching.
> +
> + packets_num = (lt9611c->edid_len % 32) ? (lt9611c->edid_len / 32 + 1) :
> + (lt9611c->edid_len / 32);
> + for (i = 0; i < packets_num; i++) {
> + read_edid_data_cmd[4] = (u8)i;
> + ret = i2c_read_write_flow(lt9611c, read_edid_data_cmd, 5,
> return_edid_data, 37);
> + if (ret) {
> + dev_err(dev, "Failed to read EDID packet %d\n", i);
> + lt9611c->edid_valid = false;
> + return -EIO;
> + }
> + offset = i * 32;
> + bytes_to_copy = min(32, lt9611c->edid_len - offset);
> + memcpy(lt9611c->edid_buf + offset, &return_edid_data[5],
> bytes_to_copy);
> + }
And really, you wouldn't have to implement the custom get edid block at
all, if you added a proper i2c adapter implementation and passed that to
drm_edid_read_ddc().
> +
> + lt9611c->edid_valid = true;
> +
> + return ret;
> +}
> +
> +static int lt9611c_get_edid_block(void *data, u8 *buf, unsigned int block,
> size_t len)
> +{
> + struct lt9611c *lt9611c = data;
> + struct device *dev = lt9611c->dev;
> + unsigned int total_blocks;
> + int ret;
> +
> + if (len > 128)
> + return -EINVAL;
> +
> + guard(mutex)(<9611c->ocm_lock);
> + if (block == 0 || !lt9611c->edid_valid) {
> + ret = lt9611c_read_edid(lt9611c);
> + if (ret) {
> + dev_err(dev, "EDID read failed\n");
> + return ret;
> + }
> + }
> +
> + total_blocks = lt9611c->edid_len / 128;
> + if (!total_blocks) {
> + dev_err(dev, "No valid EDID blocks\n");
> + return -EIO;
> + }
> +
> + if (block >= total_blocks) {
> + dev_err(dev, "Requested block %u exceeds total blocks %u\n",
> + block, total_blocks);
> + return -EINVAL;
> + }
> +
> + memcpy(buf, lt9611c->edid_buf + block * 128, len);
The get edid block hook is supposed to read *one* block. Can't you
implement reading one block? This now reads the entire edid for every
block.
Again, better yet, i2c adapter implementation.
> +
> + return 0;
> +}
> +
> +static const struct drm_edid *lt9611c_bridge_edid_read(struct drm_bridge
> *bridge,
> + struct drm_connector
> *connector)
> +{
> + struct lt9611c *lt9611c = bridge_to_lt9611c(bridge);
> +
> + usleep_range(10000, 20000);
> + return drm_edid_read_custom(connector, lt9611c_get_edid_block, lt9611c);
> +}
--
Jani Nikula, Intel
