[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