From 9066f83c055edd0e42b0ecb57a27b308bde64687 Mon Sep 17 00:00:00 2001 From: Chris Wilson <chris@chris-wilson.co.uk> Date: Wed, 2 Oct 2013 11:12:53 +0100 Subject: drm: Try loading builtin EDIDs first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the firmware is not builtin and userspace is not yet running, we can stall the boot process for a minute whilst the firmware loader times out. This is contrary to expectations of providing a builtin EDID! In the process, we can rearrange the code to make the error handling more resilient and prevent gcc warning about unitialised variables along the error paths. v2: Load builtins first, fix gcc second (Jani) and cosmetics (Ville). v3: Verify that we do not read beyond the end of the fwdata (Ville) Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Reviewed-by: Jani Nikula <jani.nikula@intel.com> Signed-off-by: Dave Airlie <airlied@redhat.com> --- drivers/gpu/drm/drm_edid_load.c | 108 ++++++++++++++++++++-------------------- 1 file changed, 54 insertions(+), 54 deletions(-) Index: linux-3.12.24-rt37/drivers/gpu/drm/drm_edid_load.c =================================================================== @ linux-3.12.24-rt37/drivers/gpu/drm/drm_edid_load.c:35 @ MODULE_PARM_DESC(edid_firmware, "Do not "from built-in data or /lib/firmware instead. "); #define GENERIC_EDIDS 5 -static char *generic_edid_name[GENERIC_EDIDS] = { +static const char *generic_edid_name[GENERIC_EDIDS] = { "edid/1024x768.bin", "edid/1280x1024.bin", "edid/1600x1200.bin", @ linux-3.12.24-rt37/drivers/gpu/drm/drm_edid_load.c:43 @ static char *generic_edid_name[GENERIC_E "edid/1920x1080.bin", }; -static u8 generic_edid[GENERIC_EDIDS][128] = { +static const u8 generic_edid[GENERIC_EDIDS][128] = { { 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, @ linux-3.12.24-rt37/drivers/gpu/drm/drm_edid_load.c:136 @ static u8 generic_edid[GENERIC_EDIDS][12 }, }; +static int edid_size(const u8 *edid, int data_size) +{ + if (data_size < EDID_LENGTH) + return 0; + + return (edid[0x7e] + 1) * EDID_LENGTH; +} + static u8 *edid_load(struct drm_connector *connector, const char *name, const char *connector_name) { - const struct firmware *fw; - struct platform_device *pdev; - u8 *fwdata = NULL, *edid, *new_edid; - int fwsize, expected; - int builtin = 0, err = 0; + const struct firmware *fw = NULL; + const u8 *fwdata; + u8 *edid; + int fwsize, builtin; int i, valid_extensions = 0; bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS); - pdev = platform_device_register_simple(connector_name, -1, NULL, 0); - if (IS_ERR(pdev)) { - DRM_ERROR("Failed to register EDID firmware platform device " - "for connector \"%s\"\n", connector_name); - err = -EINVAL; - goto out; - } - - err = request_firmware(&fw, name, &pdev->dev); - platform_device_unregister(pdev); - - if (err) { - i = 0; - while (i < GENERIC_EDIDS && strcmp(name, generic_edid_name[i])) - i++; - if (i < GENERIC_EDIDS) { - err = 0; - builtin = 1; + builtin = 0; + for (i = 0; i < GENERIC_EDIDS; i++) { + if (strcmp(name, generic_edid_name[i]) == 0) { fwdata = generic_edid[i]; fwsize = sizeof(generic_edid[i]); + builtin = 1; + break; } } + if (!builtin) { + struct platform_device *pdev; + int err; + + pdev = platform_device_register_simple(connector_name, -1, NULL, 0); + if (IS_ERR(pdev)) { + DRM_ERROR("Failed to register EDID firmware platform device " + "for connector \"%s\"\n", connector_name); + return ERR_CAST(pdev); + } - if (err) { - DRM_ERROR("Requesting EDID firmware \"%s\" failed (err=%d)\n", - name, err); - goto out; - } + err = request_firmware(&fw, name, &pdev->dev); + platform_device_unregister(pdev); + if (err) { + DRM_ERROR("Requesting EDID firmware \"%s\" failed (err=%d)\n", + name, err); + return ERR_PTR(err); + } - if (fwdata == NULL) { - fwdata = (u8 *) fw->data; + fwdata = fw->data; fwsize = fw->size; } - expected = (fwdata[0x7e] + 1) * EDID_LENGTH; - if (expected != fwsize) { + if (edid_size(fwdata, fwsize) != fwsize) { DRM_ERROR("Size of EDID firmware \"%s\" is invalid " - "(expected %d, got %d)\n", name, expected, (int) fwsize); - err = -EINVAL; - goto relfw_out; + "(expected %d, got %d\n", name, + edid_size(fwdata, fwsize), (int)fwsize); + edid = ERR_PTR(-EINVAL); + goto out; } edid = kmemdup(fwdata, fwsize, GFP_KERNEL); if (edid == NULL) { - err = -ENOMEM; - goto relfw_out; + edid = ERR_PTR(-ENOMEM); + goto out; } if (!drm_edid_block_valid(edid, 0, print_bad_edid)) { @ linux-3.12.24-rt37/drivers/gpu/drm/drm_edid_load.c:205 @ static u8 *edid_load(struct drm_connecto DRM_ERROR("Base block of EDID firmware \"%s\" is invalid ", name); kfree(edid); - err = -EINVAL; - goto relfw_out; + edid = ERR_PTR(-EINVAL); + goto out; } for (i = 1; i <= edid[0x7e]; i++) { @ linux-3.12.24-rt37/drivers/gpu/drm/drm_edid_load.c:218 @ static u8 *edid_load(struct drm_connecto } if (valid_extensions != edid[0x7e]) { + u8 *new_edid; + edid[EDID_LENGTH-1] += edid[0x7e] - valid_extensions; DRM_INFO("Found %d valid extensions instead of %d in EDID data " "\"%s\" for connector \"%s\"\n", valid_extensions, edid[0x7e], name, connector_name); edid[0x7e] = valid_extensions; + new_edid = krealloc(edid, (valid_extensions + 1) * EDID_LENGTH, - GFP_KERNEL); - if (new_edid == NULL) { - err = -ENOMEM; - kfree(edid); - goto relfw_out; - } - edid = new_edid; + GFP_KERNEL); + if (new_edid) + edid = new_edid; } DRM_INFO("Got %s EDID base block and %d extension%s from " @ linux-3.12.24-rt37/drivers/gpu/drm/drm_edid_load.c:237 @ static u8 *edid_load(struct drm_connecto "external", valid_extensions, valid_extensions == 1 ? "" : "s", name, connector_name); -relfw_out: - release_firmware(fw); - out: - if (err) - return ERR_PTR(err); - + if (fw) + release_firmware(fw); return edid; }