On Fri, 04 Oct 2013, Chris Wilson chris@chris-wilson.co.uk wrote:
If the firmware is builtin and userspace is not yet running, we can stall the boot process for a minute whilst the firmware loader times out probing for a non-existent connector EDID. This is contrary to the 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) v4: Restore the lost parenthesis (Jani)
Thanks, looks good, Jani.
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@linux.intel.com
drivers/gpu/drm/drm_edid_load.c | 108 ++++++++++++++++++++-------------------- 1 file changed, 54 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c index 271b42bbfb72..75f204bbb888 100644 --- 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;
- 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);}
-- 1.8.4.rc3