[PATCH] drm/bridge: adv7511: fix support for large EDIDs
Niklas Söderlund
niklas.soderlund at ragnatech.se
Fri Mar 26 00:39:13 UTC 2021
Hi Hans,
Thanks for your patch.
On 2021-03-24 09:53:32 +0100, Hans Verkuil wrote:
> While testing support for large (> 256 bytes) EDIDs on the Renesas
> Koelsch board I noticed that the adv7511 bridge driver only read the
> first two blocks.
>
> The media V4L2 version for the adv7511 (drivers/media/i2c/adv7511-v4l2.c)
> handled this correctly.
>
> Besides a simple bug when setting the segment register (it was set to the
> block number instead of block / 2), the logic of the code was also weird.
> In particular reading the DDC_STATUS is odd: this is unrelated to EDID
> reading.
>
> The reworked code just waits for any EDID segment reads to finish (this
> does nothing if the a segment is already read), checks if the desired
> segment matches the read segment, and if not, then it requests the new
> segment and waits again for the EDID segment to be read.
>
> Finally it checks if the currently buffered EDID segment contains the
> desired EDID block, and if not it will update the EDID buffer from
> the adv7511.
>
> Tested with my Koelsch board and with EDIDs of 1, 2, 3 and 4 blocks.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco at xs4all.nl>
Tested-by: Niklas Söderlund <niklas.soderlund+renesas at ragnatech.se>
> ---
> Testing on the Renesas board also requires these two adv7604 patches
> if you want to test with an HDMI cable between the HDMI input and output:
>
> https://patchwork.linuxtv.org/project/linux-media/patch/00882808-472a-d429-c565-a701da579ead@xs4all.nl/
> https://patchwork.linuxtv.org/project/linux-media/patch/c7093e76-ffb4-b19c-f576-b264f935a3ce@xs4all.nl/
> ---
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 76555ae64e9c..9e8db1c60167 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -328,6 +328,7 @@ static void adv7511_set_link_config(struct adv7511 *adv7511,
> static void __adv7511_power_on(struct adv7511 *adv7511)
> {
> adv7511->current_edid_segment = -1;
> + adv7511->edid_read = false;
>
> regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER,
> ADV7511_POWER_POWER_DOWN, 0);
> @@ -529,29 +530,35 @@ static int adv7511_get_edid_block(void *data, u8 *buf, unsigned int block,
> struct adv7511 *adv7511 = data;
> struct i2c_msg xfer[2];
> uint8_t offset;
> + unsigned int cur_segment;
> unsigned int i;
> int ret;
>
> if (len > 128)
> return -EINVAL;
>
> - if (adv7511->current_edid_segment != block / 2) {
> - unsigned int status;
> + /* wait for any EDID segment reads to finish */
> + adv7511_wait_for_edid(adv7511, 200);
>
> - ret = regmap_read(adv7511->regmap, ADV7511_REG_DDC_STATUS,
> - &status);
> + ret = regmap_read(adv7511->regmap, ADV7511_REG_EDID_SEGMENT, &cur_segment);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * If the current read segment does not match what we need, then
> + * write the new segment and wait for it to be read.
> + */
> + if (cur_segment != block / 2) {
> + adv7511->edid_read = false;
> + cur_segment = block / 2;
> + regmap_write(adv7511->regmap, ADV7511_REG_EDID_SEGMENT,
> + cur_segment);
> + ret = adv7511_wait_for_edid(adv7511, 200);
> if (ret < 0)
> return ret;
> + }
>
> - if (status != 2) {
> - adv7511->edid_read = false;
> - regmap_write(adv7511->regmap, ADV7511_REG_EDID_SEGMENT,
> - block);
> - ret = adv7511_wait_for_edid(adv7511, 200);
> - if (ret < 0)
> - return ret;
> - }
> -
> + if (adv7511->current_edid_segment != cur_segment) {
> /* Break this apart, hopefully more I2C controllers will
> * support 64 byte transfers than 256 byte transfers
> */
> @@ -579,7 +586,7 @@ static int adv7511_get_edid_block(void *data, u8 *buf, unsigned int block,
> offset += 64;
> }
>
> - adv7511->current_edid_segment = block / 2;
> + adv7511->current_edid_segment = cur_segment;
> }
>
> if (block % 2 == 0)
--
Regards,
Niklas Söderlund
More information about the dri-devel
mailing list