Bad EDID causing audio failure

[JamesH65]

Placeholder issue.

A customer has a monitor that is passing back a badly formatted EDID. The CTA block section has 4 blocks, which take up a certain number of bytes, but the field defining the total length is incorrect (2 bytes shorter than the total bytes of the blocks). This causes the last block to not be read when passed up to DRM, and means that subsequent data in the EDID is corrupted. Final result is that although there is enough valid data to start up the display, it thinks it's DVT, not CEA, so audio is disabled. This is due to the final CTA block which contains the vendor ID flag which indicates CEA, not being correctly parsed due to the block length error.

During investigation it was discovered that the edid-decode application was failing to detect this error, so this issue is being used to provide easy access to the information required to fix it.

[JamesH65]

EDID in base64 (use base64 -d to convert to binary)

AP///////wBjGAAAAAAAABcYAQOAAAAACtelollKliQUUFSjCADRwLMAgQCBgIFAgcABAQEBBHQA MPJwWoCwWIoARBd0AAAefyFWqlEAHjBGjzMAP0MhAAAeAAAA/QAyTB5QEAAKICAgICAgAAAA/ABB QUEKICAgICAgICAgAQwCAy50UJAFBAMHAgYBHxQTEhYRFSApCQcDFQdQVwcAgwEAAG0DDAAQAAA8 IABgAQIDAR0AvFLQHiC4KFVAxI4hAAAeAR2A0HIcFiAQLCWAxI4hAACejArQiiDgLRAQPpYAE44h AAAYjArQkCBAMSAMQFUAE44hAAAYAAAAAAAAYA==

[JamesH65]

Patch that fixes edid-decode

> From 94f60d95087ac92746dd148cdab29159b2e3409f Mon Sep 17 00:00:00 2001

From: James Hughes <james.hughes@raspberrypi.org>
Date: Tue, 10 Sep 2019 11:23:09 +0100
Subject: [PATCH] Add check for CTA block lengths

---
 edid-decode.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/edid-decode.c b/edid-decode.c
index 7442f8a..04fdcc6 100644
--- a/edid-decode.c
+++ b/edid-decode.c
@@ -2168,12 +2168,17 @@ static void cta_hdmi_audio_block(const unsigned char *x, unsigned int length)
    }
 }

-static void cta_block(const unsigned char *x)
+static void cta_block(const unsigned char *x, int max_length)
 {
    static int last_block_was_hdmi_vsdb;
    unsigned int length = x[0] & 0x1f;
    unsigned int oui;

+   if (length > max_length) {
+       printf("CTA Block is truncated!\n");
+       return;
+   }
+
    switch ((x[0] & 0xe0) >> 5) {
    case 0x01:
        printf("  Audio data block\n");
@@ -2314,7 +2319,7 @@ static int parse_cta(const unsigned char *x)
            int i;
            printf("%d bytes of CTA data\n", offset - 4);
            for (i = 4; i < offset; i += (x[i] & 0x1f) + 1) {
-               cta_block(x + i);
+               cta_block(x + i, offset - i);
            }
        }

-- 
2.17.1

[JamesH65]

This is a won't fix from RPI point of view - we haven't thought of a sane way of fixing up bad EDID's - should be fixed at the TV end.

[popcornmix]

hdmi_edid_file=edid.dat would be a workaround for customer (either fixing up their own edid, or capturing one from a broadly similar tv).

[pelwell]

While you don't want to be reading off into invalid data, is it not clear in this case (based on the lengths of the components) that the overall length is wrong in such a way that it can be patched up?

[ghost]

This issue is specific to Pi 4 - on Pi 3 it seems the firmware can cope with this slightly borked EDID - see https://www.raspberrypi.org/forums/viewtopic.php?f=28&t=248094

[JamesH65]

@pelwell In this particular case, I think it would be possible to automagically fix up and redo the checksum, but there comes a point of diminishing returns - how many bad EDID's should WE be fixing...

@andrum99 At the moment that is the case, but a PR to allow detailed timings to be added to the available resolutions is in the pipelines, and this would be badly affected by this sort of bad EDID since the detailed timings are after this particular block. And that PR would apply to all models.

@popcornmix We do have a EDID @6by9 manually fixed up that appears to work.

[JamesH65]

This is the fixed edid

MPJwWoCwWIoARBd0AAAefyFWqlEAHjBGjzMAP0MhAAAeAAAA/QAyTB5QEAAKICAgICAgAAAA/ABB
QUEKICAgICAgICAgAQwCAzF0UJAFBAMHAgYBHxQTEhYRFSApCQcDFQdQVwcAgwEAAG0DDAAQAAA8
IABgAQIDAR0AvFLQHiC4KFVAxI4hAAAeAR2A0HIcFiAQLCWAxI4hAACejArQiiDgLRAQPpYAE44h
AAAYjArQkCBAMSAMQFUAE44hAAAYAAAAAAAAXQ==

[6by9]

While you don't want to be reading off into invalid data, is it not clear in this case (based on the lengths of the components) that the overall length is wrong in such a way that it can be patched up?

Not really.

Working through the CEA extension block, byte 2 has the offset to the Detailed Timing Descriptors (DTDs). Starting at byte 4 you then get the Data Collection Blocks, each of which has a length field. In this case, the offset at byte 2 is reported as 0x2e (46). From byte 4 we have 4 blocks, lengths 17, 10, 4, and 14 for a total of 45 bytes. Add in the 4 byte header at the start of the block and you have 49 (0x31) bytes.

Some of the DCBs are variable length, so is it the length of the last DCB that is wrong, or the offset to the DTDs? In this case it is the offset, but short of trying to sanity check the DTD timings there's no easy way to tell which condition is applicable for a randomly incorrect EDID.

I'll add that all the EDID parsing is common DRM code, so we'd be either hacking the raw EDID within the firmware, or trying to get weird hacks for dodgy EDIDs merged into mainline.

[6by9]

@JamesH65 Your base64 of the fixed EDID missed out the first line

AP///////wBjGAAAAAAAABcYAQOAAAAACtelollKliQUUFSjCADRwLMAgQCBgIFAgcABAQEBBHQA
MPJwWoCwWIoARBd0AAAefyFWqlEAHjBGjzMAP0MhAAAeAAAA/QAyTB5QEAAKICAgICAgAAAA/ABB
QUEKICAgICAgICAgAQwCAzF0UJAFBAMHAgYBHxQTEhYRFSApCQcDFQdQVwcAgwEAAG0DDAAQAAA8
IABgAQIDAR0AvFLQHiC4KFVAxI4hAAAeAR2A0HIcFiAQLCWAxI4hAACejArQiiDgLRAQPpYAE44h
AAAYjArQkCBAMSAMQFUAE44hAAAYAAAAAAAAXQ==

[sebres]

FWIW (no idea it is also EDID of my TV, but possible it'd help someone with similar issue) - in my case (PI 4 model B and Philips TV 47PFL7008K), I successfully got an audio adding following entries in boot/config.txt:

hdmi_drive=2
hdmi_force_hotplug=1

And in my case it must be necessarily connected on HDMI 1 socket of PI (via HDMI 2 still no sound on TV).

Setting of hdmi_force_edid_audio=-1 as well as an attempt to switch the audio output to HDMI (using raspi-config or amixer cset numid=3 2) did not work on my side.

And here is my EDID just in case...

``` AP///////wBBDAAAAQEBASoWAQOAaDp4CuaSo1RKmSYPSkwhCACzAJUAqUCQQIEAgYCBQAEBAjqA GHE4LUBYLEUAANBSAAAeAjqA0HI4LUAQLEWAANBSAAAeAAAA/ABQaGlsaXBzIEZUVgogAAAA/QAw Pg9GEQAKICAgICAgAb8CAzrxUhAfICIhBRQEExIDEQIWBxUGASYJHwcVB1CDAQAAcAMMABAAOC2v TEzQBAFAAf/jBQMB4gBJAR2APnM4LUB+LEWAANBSAAAeAR2A0HIcFiAQLCWAANBSAACeAR0AvFLQ HiC4KFVAANBSAAAeAAAAAAAAAAAAAAAAAAAASg== ```