diff mbox series

[v2,1/2] drm: bridge: adv7511: fix reading edid segments

Message ID 20231027122214.599067-1-emas@bang-olufsen.dk (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] drm: bridge: adv7511: fix reading edid segments | expand

Commit Message

Emil Abildgaard Svendsen Oct. 27, 2023, 12:22 p.m. UTC
Currently reading EDID only works because usually only two EDID blocks
of 128 bytes is used. Where an EDID segment holds 256 bytes or two EDID
blocks. And the first EDID segment read works fine but E-EDID specifies
up to 128 segments.

The logic is broken so change EDID segment index to multiple of 256
bytes and not 128 (block size).

Fixes: 9c8af882bf12 ("drm: Add adv7511 encoder driver")

Signed-off-by: Emil Svendsen <emas@bang-olufsen.dk>
---
v2:
 - Split into two patches.
 - Add Fixes tag.

v1: https://lore.kernel.org/all/20231026113029.575846-1-emas@bang-olufsen.dk/

 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Frieder Schrempf Nov. 30, 2023, 3:57 p.m. UTC | #1
Hi Emil,

On 27.10.23 14:22, Emil Abildgaard Svendsen wrote:
> [Sie erhalten nicht häufig E-Mails von emas@bang-olufsen.dk. Weitere Informationen, warum dies wichtig ist, finden Sie unter https://aka.ms/LearnAboutSenderIdentification ]
> 
> Currently reading EDID only works because usually only two EDID blocks
> of 128 bytes is used. Where an EDID segment holds 256 bytes or two EDID
> blocks. And the first EDID segment read works fine but E-EDID specifies
> up to 128 segments.
> 
> The logic is broken so change EDID segment index to multiple of 256
> bytes and not 128 (block size).
> 
> Fixes: 9c8af882bf12 ("drm: Add adv7511 encoder driver")
> 
> Signed-off-by: Emil Svendsen <emas@bang-olufsen.dk>
> ---
> v2:
>  - Split into two patches.
>  - Add Fixes tag.
> 
> v1: https://lore.kernel.org/all/20231026113029.575846-1-emas@bang-olufsen.dk/

Unfortunately this v2 series breaks my setup in the same way as I
reported for v1 [1].

Also, please add me to CC for future versions otherwise I will likely
miss them.

Thanks
Frieder

[1]
https://patchwork.kernel.org/project/dri-devel/patch/20231026113029.575846-1-emas@bang-olufsen.dk/#25572510
Frieder Schrempf Nov. 30, 2023, 4:03 p.m. UTC | #2
On 30.11.23 16:57, Frieder Schrempf wrote:
> Hi Emil,
> 
> On 27.10.23 14:22, Emil Abildgaard Svendsen wrote:
>> [Sie erhalten nicht häufig E-Mails von emas@bang-olufsen.dk. Weitere Informationen, warum dies wichtig ist, finden Sie unter https://aka.ms/LearnAboutSenderIdentification ]
>>
>> Currently reading EDID only works because usually only two EDID blocks
>> of 128 bytes is used. Where an EDID segment holds 256 bytes or two EDID
>> blocks. And the first EDID segment read works fine but E-EDID specifies
>> up to 128 segments.
>>
>> The logic is broken so change EDID segment index to multiple of 256
>> bytes and not 128 (block size).
>>
>> Fixes: 9c8af882bf12 ("drm: Add adv7511 encoder driver")
>>
>> Signed-off-by: Emil Svendsen <emas@bang-olufsen.dk>
>> ---
>> v2:
>>  - Split into two patches.
>>  - Add Fixes tag.
>>
>> v1: https://lore.kernel.org/all/20231026113029.575846-1-emas@bang-olufsen.dk/
> 
> Unfortunately this v2 series breaks my setup in the same way as I
> reported for v1 [1].

To be more specific: It's actually patch 2/2 that breaks it. Patch 1/2
alone seems to work fine in my case.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 8be235144f6d..9b6294120516 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -537,6 +537,7 @@  static int adv7511_get_edid_block(void *data, u8 *buf, unsigned int block,
 				  size_t len)
 {
 	struct adv7511 *adv7511 = data;
+	int edid_segment = block / 2;
 	struct i2c_msg xfer[2];
 	uint8_t offset;
 	unsigned int i;
@@ -545,7 +546,7 @@  static int adv7511_get_edid_block(void *data, u8 *buf, unsigned int block,
 	if (len > 128)
 		return -EINVAL;
 
-	if (adv7511->current_edid_segment != block / 2) {
+	if (adv7511->current_edid_segment != edid_segment) {
 		unsigned int status;
 
 		ret = regmap_read(adv7511->regmap, ADV7511_REG_DDC_STATUS,
@@ -556,7 +557,7 @@  static int adv7511_get_edid_block(void *data, u8 *buf, unsigned int block,
 		if (status != 2) {
 			adv7511->edid_read = false;
 			regmap_write(adv7511->regmap, ADV7511_REG_EDID_SEGMENT,
-				     block);
+				     edid_segment);
 			ret = adv7511_wait_for_edid(adv7511, 200);
 			if (ret < 0)
 				return ret;
@@ -589,7 +590,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 = edid_segment;
 	}
 
 	if (block % 2 == 0)