[PATCH] drm/edid/firmware: Remove built-in EDIDs
Thomas Zimmermann
tzimmermann at suse.de
Tue Feb 20 16:45:21 UTC 2024
Am 20.02.24 um 17:10 schrieb Maxime Ripard:
> The EDID firmware loading mechanism introduced a few built-in EDIDs that
> could be forced on any connector, bypassing the EDIDs it exposes.
>
> While convenient, this limited set of EDIDs doesn't take into account
> the connector type, and we can end up with an EDID that is completely
> invalid for a given connector.
>
> For example, the edid/800x600.bin file matches the following EDID:
>
> edid-decode (hex):
>
> 00 ff ff ff ff ff ff 00 31 d8 00 00 00 00 00 00
> 05 16 01 03 6d 1b 14 78 ea 5e c0 a4 59 4a 98 25
> 20 50 54 01 00 00 45 40 01 01 01 01 01 01 01 01
> 01 01 01 01 01 01 a0 0f 20 00 31 58 1c 20 28 80
> 14 00 15 d0 10 00 00 1e 00 00 00 ff 00 4c 69 6e
> 75 78 20 23 30 0a 20 20 20 20 00 00 00 fd 00 3b
> 3d 24 26 05 00 0a 20 20 20 20 20 20 00 00 00 fc
> 00 4c 69 6e 75 78 20 53 56 47 41 0a 20 20 00 c2
>
> ----------------
>
> Block 0, Base EDID:
> EDID Structure Version & Revision: 1.3
> Vendor & Product Identification:
> Manufacturer: LNX
> Model: 0
> Made in: week 5 of 2012
> Basic Display Parameters & Features:
> Analog display
> Signal Level Standard: 0.700 : 0.000 : 0.700 V p-p
> Blank level equals black level
> Sync: Separate Composite Serration
> Maximum image size: 27 cm x 20 cm
> Gamma: 2.20
> DPMS levels: Standby Suspend Off
> RGB color display
> First detailed timing is the preferred timing
> Color Characteristics:
> Red : 0.6416, 0.3486
> Green: 0.2919, 0.5957
> Blue : 0.1474, 0.1250
> White: 0.3125, 0.3281
> Established Timings I & II:
> DMT 0x09: 800x600 60.316541 Hz 4:3 37.879 kHz 40.000000 MHz
> Standard Timings:
> DMT 0x09: 800x600 60.316541 Hz 4:3 37.879 kHz 40.000000 MHz
> Detailed Timing Descriptors:
> DTD 1: 800x600 60.316541 Hz 4:3 37.879 kHz 40.000000 MHz (277 mm x 208 mm)
> Hfront 40 Hsync 128 Hback 88 Hpol P
> Vfront 1 Vsync 4 Vback 23 Vpol P
> Display Product Serial Number: 'Linux #0'
> Display Range Limits:
> Monitor ranges (GTF): 59-61 Hz V, 36-38 kHz H, max dotclock 50 MHz
> Display Product Name: 'Linux SVGA'
> Checksum: 0xc2
>
> So, an analog monitor EDID. However, if the connector was an HDMI
> monitor for example, it breaks the HDMI specification that requires,
> among other things, a digital display, the VIC 1 mode and an HDMI Forum
> Vendor Specific Data Block in an CTA-861 extension.
>
> We thus end up with a completely invalid EDID, which thus might confuse
> HDMI-related code that could parse it.
>
> After some discussions on IRC, we identified mainly two ways to fix
> this:
>
> - We can either create more EDIDs for each connector type to provide
> a built-in EDID that matches the resolution passed in the name, and
> still be a sensible EDID for that connector type;
>
> - Or we can just prevent the EDID to be exposed to userspace if it's
> built-in.
>
> Or possibly both.
>
> However, the conclusion was that maybe we just don't need the built-in
> EDIDs at all and we should just get rid of them. So here we are.
>
> Signed-off-by: Maxime Ripard <mripard at kernel.org>
Acked-by: Thomas Zimmermann <tzimmermann at suse.de>
> ---
> drivers/gpu/drm/drm_edid_load.c | 160 +++-----------------------------
> 1 file changed, 13 insertions(+), 147 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index 60fcb80bce61..d1c7e8298702 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -20,162 +20,28 @@
>
> static char edid_firmware[PATH_MAX];
> module_param_string(edid_firmware, edid_firmware, sizeof(edid_firmware), 0644);
> -MODULE_PARM_DESC(edid_firmware, "Do not probe monitor, use specified EDID blob "
> - "from built-in data or /lib/firmware instead. ");
> -
> -#define GENERIC_EDIDS 6
> -static const char * const generic_edid_name[GENERIC_EDIDS] = {
> - "edid/800x600.bin",
> - "edid/1024x768.bin",
> - "edid/1280x1024.bin",
> - "edid/1600x1200.bin",
> - "edid/1680x1050.bin",
> - "edid/1920x1080.bin",
> -};
> -
> -static const u8 generic_edid[GENERIC_EDIDS][128] = {
> - {
> - 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00,
> - 0x31, 0xd8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> - 0x05, 0x16, 0x01, 0x03, 0x6d, 0x1b, 0x14, 0x78,
> - 0xea, 0x5e, 0xc0, 0xa4, 0x59, 0x4a, 0x98, 0x25,
> - 0x20, 0x50, 0x54, 0x01, 0x00, 0x00, 0x45, 0x40,
> - 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
> - 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0xa0, 0x0f,
> - 0x20, 0x00, 0x31, 0x58, 0x1c, 0x20, 0x28, 0x80,
> - 0x14, 0x00, 0x15, 0xd0, 0x10, 0x00, 0x00, 0x1e,
> - 0x00, 0x00, 0x00, 0xff, 0x00, 0x4c, 0x69, 0x6e,
> - 0x75, 0x78, 0x20, 0x23, 0x30, 0x0a, 0x20, 0x20,
> - 0x20, 0x20, 0x00, 0x00, 0x00, 0xfd, 0x00, 0x3b,
> - 0x3d, 0x24, 0x26, 0x05, 0x00, 0x0a, 0x20, 0x20,
> - 0x20, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0xfc,
> - 0x00, 0x4c, 0x69, 0x6e, 0x75, 0x78, 0x20, 0x53,
> - 0x56, 0x47, 0x41, 0x0a, 0x20, 0x20, 0x00, 0xc2,
> - },
> - {
> - 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00,
> - 0x31, 0xd8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> - 0x05, 0x16, 0x01, 0x03, 0x6d, 0x23, 0x1a, 0x78,
> - 0xea, 0x5e, 0xc0, 0xa4, 0x59, 0x4a, 0x98, 0x25,
> - 0x20, 0x50, 0x54, 0x00, 0x08, 0x00, 0x61, 0x40,
> - 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
> - 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x64, 0x19,
> - 0x00, 0x40, 0x41, 0x00, 0x26, 0x30, 0x08, 0x90,
> - 0x36, 0x00, 0x63, 0x0a, 0x11, 0x00, 0x00, 0x18,
> - 0x00, 0x00, 0x00, 0xff, 0x00, 0x4c, 0x69, 0x6e,
> - 0x75, 0x78, 0x20, 0x23, 0x30, 0x0a, 0x20, 0x20,
> - 0x20, 0x20, 0x00, 0x00, 0x00, 0xfd, 0x00, 0x3b,
> - 0x3d, 0x2f, 0x31, 0x07, 0x00, 0x0a, 0x20, 0x20,
> - 0x20, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0xfc,
> - 0x00, 0x4c, 0x69, 0x6e, 0x75, 0x78, 0x20, 0x58,
> - 0x47, 0x41, 0x0a, 0x20, 0x20, 0x20, 0x00, 0x55,
> - },
> - {
> - 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00,
> - 0x31, 0xd8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> - 0x05, 0x16, 0x01, 0x03, 0x6d, 0x2c, 0x23, 0x78,
> - 0xea, 0x5e, 0xc0, 0xa4, 0x59, 0x4a, 0x98, 0x25,
> - 0x20, 0x50, 0x54, 0x00, 0x00, 0x00, 0x81, 0x80,
> - 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
> - 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x30, 0x2a,
> - 0x00, 0x98, 0x51, 0x00, 0x2a, 0x40, 0x30, 0x70,
> - 0x13, 0x00, 0xbc, 0x63, 0x11, 0x00, 0x00, 0x1e,
> - 0x00, 0x00, 0x00, 0xff, 0x00, 0x4c, 0x69, 0x6e,
> - 0x75, 0x78, 0x20, 0x23, 0x30, 0x0a, 0x20, 0x20,
> - 0x20, 0x20, 0x00, 0x00, 0x00, 0xfd, 0x00, 0x3b,
> - 0x3d, 0x3e, 0x40, 0x0b, 0x00, 0x0a, 0x20, 0x20,
> - 0x20, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0xfc,
> - 0x00, 0x4c, 0x69, 0x6e, 0x75, 0x78, 0x20, 0x53,
> - 0x58, 0x47, 0x41, 0x0a, 0x20, 0x20, 0x00, 0xa0,
> - },
> - {
> - 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00,
> - 0x31, 0xd8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> - 0x05, 0x16, 0x01, 0x03, 0x6d, 0x37, 0x29, 0x78,
> - 0xea, 0x5e, 0xc0, 0xa4, 0x59, 0x4a, 0x98, 0x25,
> - 0x20, 0x50, 0x54, 0x00, 0x00, 0x00, 0xa9, 0x40,
> - 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
> - 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x48, 0x3f,
> - 0x40, 0x30, 0x62, 0xb0, 0x32, 0x40, 0x40, 0xc0,
> - 0x13, 0x00, 0x2b, 0xa0, 0x21, 0x00, 0x00, 0x1e,
> - 0x00, 0x00, 0x00, 0xff, 0x00, 0x4c, 0x69, 0x6e,
> - 0x75, 0x78, 0x20, 0x23, 0x30, 0x0a, 0x20, 0x20,
> - 0x20, 0x20, 0x00, 0x00, 0x00, 0xfd, 0x00, 0x3b,
> - 0x3d, 0x4a, 0x4c, 0x11, 0x00, 0x0a, 0x20, 0x20,
> - 0x20, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0xfc,
> - 0x00, 0x4c, 0x69, 0x6e, 0x75, 0x78, 0x20, 0x55,
> - 0x58, 0x47, 0x41, 0x0a, 0x20, 0x20, 0x00, 0x9d,
> - },
> - {
> - 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00,
> - 0x31, 0xd8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> - 0x05, 0x16, 0x01, 0x03, 0x6d, 0x2b, 0x1b, 0x78,
> - 0xea, 0x5e, 0xc0, 0xa4, 0x59, 0x4a, 0x98, 0x25,
> - 0x20, 0x50, 0x54, 0x00, 0x00, 0x00, 0xb3, 0x00,
> - 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
> - 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x21, 0x39,
> - 0x90, 0x30, 0x62, 0x1a, 0x27, 0x40, 0x68, 0xb0,
> - 0x36, 0x00, 0xb5, 0x11, 0x11, 0x00, 0x00, 0x1e,
> - 0x00, 0x00, 0x00, 0xff, 0x00, 0x4c, 0x69, 0x6e,
> - 0x75, 0x78, 0x20, 0x23, 0x30, 0x0a, 0x20, 0x20,
> - 0x20, 0x20, 0x00, 0x00, 0x00, 0xfd, 0x00, 0x3b,
> - 0x3d, 0x40, 0x42, 0x0f, 0x00, 0x0a, 0x20, 0x20,
> - 0x20, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0xfc,
> - 0x00, 0x4c, 0x69, 0x6e, 0x75, 0x78, 0x20, 0x57,
> - 0x53, 0x58, 0x47, 0x41, 0x0a, 0x20, 0x00, 0x26,
> - },
> - {
> - 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00,
> - 0x31, 0xd8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> - 0x05, 0x16, 0x01, 0x03, 0x6d, 0x32, 0x1c, 0x78,
> - 0xea, 0x5e, 0xc0, 0xa4, 0x59, 0x4a, 0x98, 0x25,
> - 0x20, 0x50, 0x54, 0x00, 0x00, 0x00, 0xd1, 0xc0,
> - 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
> - 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x02, 0x3a,
> - 0x80, 0x18, 0x71, 0x38, 0x2d, 0x40, 0x58, 0x2c,
> - 0x45, 0x00, 0xf4, 0x19, 0x11, 0x00, 0x00, 0x1e,
> - 0x00, 0x00, 0x00, 0xff, 0x00, 0x4c, 0x69, 0x6e,
> - 0x75, 0x78, 0x20, 0x23, 0x30, 0x0a, 0x20, 0x20,
> - 0x20, 0x20, 0x00, 0x00, 0x00, 0xfd, 0x00, 0x3b,
> - 0x3d, 0x42, 0x44, 0x0f, 0x00, 0x0a, 0x20, 0x20,
> - 0x20, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0xfc,
> - 0x00, 0x4c, 0x69, 0x6e, 0x75, 0x78, 0x20, 0x46,
> - 0x48, 0x44, 0x0a, 0x20, 0x20, 0x20, 0x00, 0x05,
> - },
> -};
> +MODULE_PARM_DESC(edid_firmware,
> + "Do not probe monitor, use specified EDID blob from /lib/firmware instead.");
>
> static const struct drm_edid *edid_load(struct drm_connector *connector, const char *name)
> {
> const struct firmware *fw = NULL;
> - const u8 *fwdata;
> const struct drm_edid *drm_edid;
> - int fwsize, builtin;
> + int err;
>
> - builtin = match_string(generic_edid_name, GENERIC_EDIDS, name);
> - if (builtin >= 0) {
> - fwdata = generic_edid[builtin];
> - fwsize = sizeof(generic_edid[builtin]);
> - } else {
> - int err;
> -
> - err = request_firmware(&fw, name, connector->dev->dev);
> - if (err) {
> - drm_err(connector->dev,
> - "[CONNECTOR:%d:%s] Requesting EDID firmware \"%s\" failed (err=%d)\n",
> - connector->base.id, connector->name,
> - name, err);
> - return ERR_PTR(err);
> - }
> -
> - fwdata = fw->data;
> - fwsize = fw->size;
> + err = request_firmware(&fw, name, connector->dev->dev);
> + if (err) {
> + drm_err(connector->dev,
> + "[CONNECTOR:%d:%s] Requesting EDID firmware \"%s\" failed (err=%d)\n",
> + connector->base.id, connector->name,
> + name, err);
> + return ERR_PTR(err);
> }
>
> - drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] Loaded %s firmware EDID \"%s\"\n",
> - connector->base.id, connector->name,
> - builtin >= 0 ? "built-in" : "external", name);
> + drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] Loaded external firmware EDID \"%s\"\n",
> + connector->base.id, connector->name, name);
>
> - drm_edid = drm_edid_alloc(fwdata, fwsize);
> + drm_edid = drm_edid_alloc(fw->data, fw->size);
> if (!drm_edid_valid(drm_edid)) {
> drm_err(connector->dev, "Invalid firmware EDID \"%s\"\n", name);
> drm_edid_free(drm_edid);
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
More information about the dri-devel
mailing list