drm: Try loading builtin EDIDs first

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>

diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 271b42b..9081172 100644 (file)
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -32,7 +32,7 @@ MODULE_PARM_DESC(edid_firmware, "Do not probe monitor, use specified EDID blob "
        "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",
@@ -40,7 +40,7 @@ static char *generic_edid_name[GENERIC_EDIDS] = {
        "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,
@@ -133,63 +133,68 @@ static u8 generic_edid[GENERIC_EDIDS][128] = {
        },
 };
 
+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;
 
-       if (err) {
-               DRM_ERROR("Requesting EDID firmware \"%s\" failed (err=%d)\n",
-                   name, err);
-               goto out;
-       }
+               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);
+               }
+
+               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)) {
@@ -197,8 +202,8 @@ static u8 *edid_load(struct drm_connector *connector, const char *name,
                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++) {
@@ -210,19 +215,18 @@ static u8 *edid_load(struct drm_connector *connector, const char *name,
        }
 
        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 "
@@ -230,13 +234,9 @@ static u8 *edid_load(struct drm_connector *connector, const char *name,
            "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;
 }