From: Matthieu CHARETTE <matthieu.charette@gmail.com>
To: matthieu.charette@gmail.com
Cc: tzimmermann@suse.de, airlied@linux.ie,
linux-kernel@vger.kernel.org, matthieu.charette@gmail.com,
dri-devel@lists.freedesktop.org
Subject: [PATCH] drm: Fix EDID firmware load on resume
Date: Wed, 06 Jul 2022 16:58:08 +0200 [thread overview]
Message-ID: <WKTLER.NFH5CX2O259Q@gmail.com> (raw)
Loading an EDID using drm.edid_firmware parameter makes resume to fail
after firmware cache is being cleaned. This is because edid_load() use a
temporary device to request the firmware. This cause the EDID firmware
not to be cached from suspend. And, requesting the EDID firmware return
an error during resume.
So the request_firmware() call should use a permanent device for each
connector. Also, we should cache the EDID even if no monitor is
connected, in case it's plugged while suspended.
Signed-off-by: Matthieu CHARETTE <matthieu.charette@gmail.com>
---
drivers/gpu/drm/drm_connector.c | 9 ++++
drivers/gpu/drm/drm_edid_load.c | 81 ++++++++++++++++++++++++++++-----
include/drm/drm_connector.h | 12 +++++
include/drm/drm_edid.h | 3 ++
4 files changed, 94 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c
b/drivers/gpu/drm/drm_connector.c
index 1c48d162c77e..e8819ebf1c4b 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -31,6 +31,7 @@
#include <drm/drm_privacy_screen_consumer.h>
#include <drm/drm_sysfs.h>
+#include <linux/platform_device.h>
#include <linux/uaccess.h>
#include "drm_crtc_internal.h"
@@ -289,6 +290,9 @@ int drm_connector_init(struct drm_device *dev,
drm_connector_get_cmdline_mode(connector);
+ connector->edid_load_pdev = NULL;
+ drm_cache_edid_firmware(connector);
+
/* We should add connectors at the end to avoid upsetting the
connector
* index too much.
*/
@@ -473,6 +477,11 @@ void drm_connector_cleanup(struct drm_connector
*connector)
connector->tile_group = NULL;
}
+ if (connector->edid_load_pdev) {
+ platform_device_unregister(connector->edid_load_pdev);
+ connector->edid_load_pdev = NULL;
+ }
+
list_for_each_entry_safe(mode, t, &connector->probed_modes, head)
drm_mode_remove(connector, mode);
diff --git a/drivers/gpu/drm/drm_edid_load.c
b/drivers/gpu/drm/drm_edid_load.c
index 37d8ba3ddb46..5a82be9917ec 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -167,6 +167,19 @@ static int edid_size(const u8 *edid, int data_size)
return (edid[0x7e] + 1) * EDID_LENGTH;
}
+static struct platform_device *edid_pdev(const char *connector_name)
+{
+ struct platform_device *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);
+ }
+
+ return pdev;
+}
+
static void *edid_load(struct drm_connector *connector, const char
*name,
const char *connector_name)
{
@@ -182,18 +195,17 @@ static void *edid_load(struct drm_connector
*connector, const char *name,
fwdata = generic_edid[builtin];
fwsize = sizeof(generic_edid[builtin]);
} else {
- struct platform_device *pdev;
+ struct platform_device *pdev = connector->edid_load_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 (WARN_ON(!pdev)) {
+ pdev = edid_pdev(connector_name);
+ if (IS_ERR(pdev))
+ return ERR_CAST(pdev);
+ connector->edid_load_pdev = 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);
@@ -263,11 +275,9 @@ static void *edid_load(struct drm_connector
*connector, const char *name,
return edid;
}
-struct edid *drm_load_edid_firmware(struct drm_connector *connector)
+static char *edid_name(const char *connector_name)
{
- const char *connector_name = connector->name;
char *edidname, *last, *colon, *fwstr, *edidstr, *fallback = NULL;
- struct edid *edid;
if (edid_firmware[0] == '\0')
return ERR_PTR(-ENOENT);
@@ -310,8 +320,57 @@ struct edid *drm_load_edid_firmware(struct
drm_connector *connector)
if (*last == '\n')
*last = '\0';
- edid = edid_load(connector, edidname, connector_name);
+ edidname = kstrdup(edidname, GFP_KERNEL);
+ if (!edidname) {
+ kfree(fwstr);
+ return ERR_PTR(-ENOMEM);
+ }
+
kfree(fwstr);
+ return edidname;
+}
+
+void drm_cache_edid_firmware(struct drm_connector *connector)
+{
+ const char *connector_name = connector->name;
+ const char *edidname = edid_name(connector_name);
+ struct platform_device *pdev;
+ int err;
+
+ if (IS_ERR(edidname))
+ return;
+
+ if (match_string(generic_edid_name, GENERIC_EDIDS, edidname) >= 0) {
+ kfree(edidname);
+ return;
+ }
+
+ pdev = edid_pdev(connector_name);
+ if (IS_ERR(pdev)) {
+ kfree(edidname);
+ return;
+ }
+ connector->edid_load_pdev = pdev;
+
+ err = firmware_request_cache(&pdev->dev, edidname);
+ if (err)
+ DRM_ERROR("Requesting EDID firmware cache \"%s\" failed (err=%d)\n",
+ edidname, err);
+
+ kfree(edidname);
+}
+
+struct edid *drm_load_edid_firmware(struct drm_connector *connector)
+{
+ const char *connector_name = connector->name;
+ const char *edidname = edid_name(connector_name);
+ struct edid *edid;
+
+ if (IS_ERR(edidname))
+ return ERR_CAST(edidname);
+
+ edid = edid_load(connector, edidname, connector_name);
+ kfree(edidname);
return edid;
}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 3ac4bf87f257..47c84741517e 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1573,6 +1573,18 @@ struct drm_connector {
*/
struct i2c_adapter *ddc;
+ /**
+ * @edid_load_pdev: Platform device for loading EDID via firmware.
+ *
+ * The platform device is registered in drm_connector_init() in case a
+ * custom EDID firmware is used with `edid_firmware` parameter.
Otherwise,
+ * it is set to NULL.
+ *
+ * Platform device is unregistered in drm_connector_cleanup() if it
+ * is not NULL.
+ */
+ struct platform_device *edid_load_pdev;
+
/**
* @null_edid_counter: track sinks that give us all zeros for the
EDID.
* Needed to workaround some HW bugs where we get all 0s
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index b2756753370b..b54cea3f1107 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -378,10 +378,13 @@ int drm_av_sync_delay(struct drm_connector
*connector,
const struct drm_display_mode *mode);
#ifdef CONFIG_DRM_LOAD_EDID_FIRMWARE
+void drm_cache_edid_firmware(struct drm_connector *connector);
struct edid *drm_load_edid_firmware(struct drm_connector *connector);
int __drm_set_edid_firmware_path(const char *path);
int __drm_get_edid_firmware_path(char *buf, size_t bufsize);
#else
+static inline void
+drm_cache_edid_firmware(struct drm_connector *connector);
static inline struct edid *
drm_load_edid_firmware(struct drm_connector *connector)
{
--
2.36.1
WARNING: multiple messages have this Message-ID (diff)
From: Matthieu CHARETTE <matthieu.charette@gmail.com>
To: matthieu.charette@gmail.com
Cc: matthieu.charette@gmail.com, maarten.lankhorst@linux.intel.com,
mripard@kernel.org, tzimmermann@suse.de, airlied@linux.ie,
daniel@ffwll.ch, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org
Subject: [PATCH] drm: Fix EDID firmware load on resume
Date: Wed, 06 Jul 2022 16:58:08 +0200 [thread overview]
Message-ID: <WKTLER.NFH5CX2O259Q@gmail.com> (raw)
Loading an EDID using drm.edid_firmware parameter makes resume to fail
after firmware cache is being cleaned. This is because edid_load() use a
temporary device to request the firmware. This cause the EDID firmware
not to be cached from suspend. And, requesting the EDID firmware return
an error during resume.
So the request_firmware() call should use a permanent device for each
connector. Also, we should cache the EDID even if no monitor is
connected, in case it's plugged while suspended.
Signed-off-by: Matthieu CHARETTE <matthieu.charette@gmail.com>
---
drivers/gpu/drm/drm_connector.c | 9 ++++
drivers/gpu/drm/drm_edid_load.c | 81 ++++++++++++++++++++++++++++-----
include/drm/drm_connector.h | 12 +++++
include/drm/drm_edid.h | 3 ++
4 files changed, 94 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c
b/drivers/gpu/drm/drm_connector.c
index 1c48d162c77e..e8819ebf1c4b 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -31,6 +31,7 @@
#include <drm/drm_privacy_screen_consumer.h>
#include <drm/drm_sysfs.h>
+#include <linux/platform_device.h>
#include <linux/uaccess.h>
#include "drm_crtc_internal.h"
@@ -289,6 +290,9 @@ int drm_connector_init(struct drm_device *dev,
drm_connector_get_cmdline_mode(connector);
+ connector->edid_load_pdev = NULL;
+ drm_cache_edid_firmware(connector);
+
/* We should add connectors at the end to avoid upsetting the
connector
* index too much.
*/
@@ -473,6 +477,11 @@ void drm_connector_cleanup(struct drm_connector
*connector)
connector->tile_group = NULL;
}
+ if (connector->edid_load_pdev) {
+ platform_device_unregister(connector->edid_load_pdev);
+ connector->edid_load_pdev = NULL;
+ }
+
list_for_each_entry_safe(mode, t, &connector->probed_modes, head)
drm_mode_remove(connector, mode);
diff --git a/drivers/gpu/drm/drm_edid_load.c
b/drivers/gpu/drm/drm_edid_load.c
index 37d8ba3ddb46..5a82be9917ec 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -167,6 +167,19 @@ static int edid_size(const u8 *edid, int data_size)
return (edid[0x7e] + 1) * EDID_LENGTH;
}
+static struct platform_device *edid_pdev(const char *connector_name)
+{
+ struct platform_device *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);
+ }
+
+ return pdev;
+}
+
static void *edid_load(struct drm_connector *connector, const char
*name,
const char *connector_name)
{
@@ -182,18 +195,17 @@ static void *edid_load(struct drm_connector
*connector, const char *name,
fwdata = generic_edid[builtin];
fwsize = sizeof(generic_edid[builtin]);
} else {
- struct platform_device *pdev;
+ struct platform_device *pdev = connector->edid_load_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 (WARN_ON(!pdev)) {
+ pdev = edid_pdev(connector_name);
+ if (IS_ERR(pdev))
+ return ERR_CAST(pdev);
+ connector->edid_load_pdev = 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);
@@ -263,11 +275,9 @@ static void *edid_load(struct drm_connector
*connector, const char *name,
return edid;
}
-struct edid *drm_load_edid_firmware(struct drm_connector *connector)
+static char *edid_name(const char *connector_name)
{
- const char *connector_name = connector->name;
char *edidname, *last, *colon, *fwstr, *edidstr, *fallback = NULL;
- struct edid *edid;
if (edid_firmware[0] == '\0')
return ERR_PTR(-ENOENT);
@@ -310,8 +320,57 @@ struct edid *drm_load_edid_firmware(struct
drm_connector *connector)
if (*last == '\n')
*last = '\0';
- edid = edid_load(connector, edidname, connector_name);
+ edidname = kstrdup(edidname, GFP_KERNEL);
+ if (!edidname) {
+ kfree(fwstr);
+ return ERR_PTR(-ENOMEM);
+ }
+
kfree(fwstr);
+ return edidname;
+}
+
+void drm_cache_edid_firmware(struct drm_connector *connector)
+{
+ const char *connector_name = connector->name;
+ const char *edidname = edid_name(connector_name);
+ struct platform_device *pdev;
+ int err;
+
+ if (IS_ERR(edidname))
+ return;
+
+ if (match_string(generic_edid_name, GENERIC_EDIDS, edidname) >= 0) {
+ kfree(edidname);
+ return;
+ }
+
+ pdev = edid_pdev(connector_name);
+ if (IS_ERR(pdev)) {
+ kfree(edidname);
+ return;
+ }
+ connector->edid_load_pdev = pdev;
+
+ err = firmware_request_cache(&pdev->dev, edidname);
+ if (err)
+ DRM_ERROR("Requesting EDID firmware cache \"%s\" failed (err=%d)\n",
+ edidname, err);
+
+ kfree(edidname);
+}
+
+struct edid *drm_load_edid_firmware(struct drm_connector *connector)
+{
+ const char *connector_name = connector->name;
+ const char *edidname = edid_name(connector_name);
+ struct edid *edid;
+
+ if (IS_ERR(edidname))
+ return ERR_CAST(edidname);
+
+ edid = edid_load(connector, edidname, connector_name);
+ kfree(edidname);
return edid;
}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 3ac4bf87f257..47c84741517e 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1573,6 +1573,18 @@ struct drm_connector {
*/
struct i2c_adapter *ddc;
+ /**
+ * @edid_load_pdev: Platform device for loading EDID via firmware.
+ *
+ * The platform device is registered in drm_connector_init() in case a
+ * custom EDID firmware is used with `edid_firmware` parameter.
Otherwise,
+ * it is set to NULL.
+ *
+ * Platform device is unregistered in drm_connector_cleanup() if it
+ * is not NULL.
+ */
+ struct platform_device *edid_load_pdev;
+
/**
* @null_edid_counter: track sinks that give us all zeros for the
EDID.
* Needed to workaround some HW bugs where we get all 0s
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index b2756753370b..b54cea3f1107 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -378,10 +378,13 @@ int drm_av_sync_delay(struct drm_connector
*connector,
const struct drm_display_mode *mode);
#ifdef CONFIG_DRM_LOAD_EDID_FIRMWARE
+void drm_cache_edid_firmware(struct drm_connector *connector);
struct edid *drm_load_edid_firmware(struct drm_connector *connector);
int __drm_set_edid_firmware_path(const char *path);
int __drm_get_edid_firmware_path(char *buf, size_t bufsize);
#else
+static inline void
+drm_cache_edid_firmware(struct drm_connector *connector);
static inline struct edid *
drm_load_edid_firmware(struct drm_connector *connector)
{
--
2.36.1
next reply other threads:[~2022-07-06 17:03 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-06 14:58 Matthieu CHARETTE [this message]
2022-07-06 14:58 ` [PATCH] drm: Fix EDID firmware load on resume Matthieu CHARETTE
2022-07-08 7:14 ` Matthieu CHARETTE
2022-07-08 7:14 ` Matthieu CHARETTE
2022-07-14 14:23 ` André Almeida
2022-07-14 14:23 ` André Almeida
2022-07-15 9:03 ` Matthieu CHARETTE
2022-07-15 9:03 ` Matthieu CHARETTE
2022-07-15 9:22 ` Matthieu CHARETTE
2022-07-15 9:22 ` Matthieu CHARETTE
2022-07-16 19:04 ` kernel test robot
2022-07-16 19:04 ` kernel test robot
2022-07-16 22:08 ` kernel test robot
2022-07-16 22:08 ` kernel test robot
2022-07-16 22:39 ` kernel test robot
2022-07-16 22:39 ` kernel test robot
2022-07-17 12:18 ` kernel test robot
2022-07-17 12:18 ` kernel test robot
2022-07-27 7:41 ` Matthieu CHARETTE
2022-07-27 7:41 ` Matthieu CHARETTE
2022-07-27 7:41 ` Matthieu CHARETTE
2022-07-27 8:18 ` Takashi Iwai
2022-07-27 8:18 ` Takashi Iwai
2022-07-27 8:18 ` Takashi Iwai
2022-07-27 13:53 ` Matthieu CHARETTE
2022-07-27 13:53 ` Matthieu CHARETTE
2022-07-27 13:53 ` Matthieu CHARETTE
2022-08-02 14:29 ` Jani Nikula
2022-08-02 14:29 ` Jani Nikula
2022-08-08 15:15 ` Matthieu CHARETTE
2022-08-08 15:15 ` Matthieu CHARETTE
2022-08-08 17:19 ` Jani Nikula
2022-08-08 17:19 ` Jani Nikula
2022-09-12 16:25 ` Matthieu CHARETTE
2022-09-12 16:25 ` Matthieu CHARETTE
2022-09-12 16:25 ` Matthieu CHARETTE
2022-10-06 22:23 ` Jani Nikula
2022-10-06 22:23 ` Jani Nikula
2022-08-08 15:17 ` Matthieu CHARETTE
2022-08-08 15:17 ` Matthieu CHARETTE
2022-08-08 15:17 ` Matthieu CHARETTE
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=WKTLER.NFH5CX2O259Q@gmail.com \
--to=matthieu.charette@gmail.com \
--cc=airlied@linux.ie \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.