| Message ID | 1403110353-6137-3-git-send-email-thomas.wood@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 18, 2014 at 12:52 PM, Thomas Wood <thomas.wood@intel.com> wrote: > Add a file to debugfs for each connector to allow the EDID to be > overridden. > > v2: Copy ubuf before accessing it and reject invalid length data. (David > Herrmann) > Ensure override_edid is reset when a new EDID value is written. > (David Herrmann) > Fix the debugfs file permissions. (David Herrmann) > > Signed-off-by: Thomas Wood <thomas.wood@intel.com> One minor nit below, but either way: Reviewed-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/drm_crtc.c | 4 +++ > drivers/gpu/drm/drm_debugfs.c | 68 ++++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_probe_helper.c | 9 ++++- > include/drm/drm_crtc.h | 1 + > 4 files changed, 81 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index fdb69f7..4b3bbc4 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -3916,6 +3916,10 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector, > struct drm_device *dev = connector->dev; > int ret, size; > > + /* ignore requests to set edid when overridden */ > + if (connector->override_edid) > + return 0; > + > if (connector->edid_blob_ptr) > drm_property_destroy_blob(dev, connector->edid_blob_ptr); > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > index 8ab3f3e..13bd429 100644 > --- a/drivers/gpu/drm/drm_debugfs.c > +++ b/drivers/gpu/drm/drm_debugfs.c > @@ -35,6 +35,7 @@ > #include <linux/slab.h> > #include <linux/export.h> > #include <drm/drmP.h> > +#include <drm/drm_edid.h> > > #if defined(CONFIG_DEBUG_FS) > > @@ -304,6 +305,67 @@ static ssize_t connector_write(struct file *file, const char __user *ubuf, > return len; > } > > +static int edid_show(struct seq_file *m, void *data) > +{ > + struct drm_connector *connector = m->private; > + struct drm_property_blob *edid = connector->edid_blob_ptr; > + > + if (connector->override_edid && edid) > + seq_write(m, edid->data, edid->length); > + > + return 0; > +} > + > +static int edid_open(struct inode *inode, struct file *file) > +{ > + struct drm_connector *dev = inode->i_private; > + > + return single_open(file, edid_show, dev); > +} > + > +static ssize_t edid_write(struct file *file, const char __user *ubuf, > + size_t len, loff_t *offp) > +{ > + struct seq_file *m = file->private_data; > + struct drm_connector *connector = m->private; > + char *buf; > + struct edid *edid; > + int ret; > + > + buf = memdup_user(ubuf, len); > + if (IS_ERR(buf)) > + return PTR_ERR(buf); > + > + edid = (struct edid *) buf; > + > + if (len == 5 && !strncmp(buf, "reset", 5)) { > + connector->override_edid = false; > + ret = drm_mode_connector_update_edid_property(connector, NULL); > + } else if (len < EDID_LENGTH || > + EDID_LENGTH * (1 + edid->extensions) > len) > + ret = -EINVAL; > + else { > + connector->override_edid = false; Might be worth doing some minimal validation of the EDID (e.g., make sure it has a valid header). Alex > + ret = drm_mode_connector_update_edid_property(connector, edid); > + if (!ret) > + connector->override_edid = true; > + } > + > + kfree(buf); > + > + return (ret) ? ret : len; > +} > + > +static const struct file_operations drm_edid_fops = { > + .owner = THIS_MODULE, > + .open = edid_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > + .write = edid_write > +}; > + > + > static const struct file_operations drm_connector_fops = { > .owner = THIS_MODULE, > .open = connector_open, > @@ -333,6 +395,12 @@ int drm_debugfs_connector_add(struct drm_connector *connector) > if (!ent) > goto error; > > + /* edid */ > + ent = debugfs_create_file("edid_override", S_IRUGO | S_IWUSR, root, > + connector, &drm_edid_fops); > + if (!ent) > + goto error; > + > return 0; > > error: > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > index d22676b..db7d250 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -130,7 +130,14 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect > count = drm_load_edid_firmware(connector); > if (count == 0) > #endif > - count = (*connector_funcs->get_modes)(connector); > + { > + if (connector->override_edid) { > + struct edid *edid = (struct edid *) connector->edid_blob_ptr->data; > + > + count = drm_add_edid_modes(connector, edid); > + } else > + count = (*connector_funcs->get_modes)(connector); > + } > > if (count == 0 && connector->status == connector_status_connected) > count = drm_add_modes_noedid(connector, 1024, 768); > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 67a33bc..d09b471 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -537,6 +537,7 @@ struct drm_connector { > > /* forced on connector */ > enum drm_connector_force force; > + bool override_edid; > uint32_t encoder_ids[DRM_CONNECTOR_MAX_ENCODER]; > struct drm_encoder *encoder; /* currently active encoder */ > > -- > 1.9.3 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Jun 18, 2014 at 03:37:30PM -0400, Alex Deucher wrote: > On Wed, Jun 18, 2014 at 12:52 PM, Thomas Wood <thomas.wood@intel.com> wrote: > > +static ssize_t edid_write(struct file *file, const char __user *ubuf, > > + size_t len, loff_t *offp) > > +{ > > + struct seq_file *m = file->private_data; > > + struct drm_connector *connector = m->private; > > + char *buf; > > + struct edid *edid; > > + int ret; > > + > > + buf = memdup_user(ubuf, len); > > + if (IS_ERR(buf)) > > + return PTR_ERR(buf); > > + > > + edid = (struct edid *) buf; > > + > > + if (len == 5 && !strncmp(buf, "reset", 5)) { > > + connector->override_edid = false; > > + ret = drm_mode_connector_update_edid_property(connector, NULL); > > + } else if (len < EDID_LENGTH || > > + EDID_LENGTH * (1 + edid->extensions) > len) > > + ret = -EINVAL; > > + else { > > + connector->override_edid = false; > > Might be worth doing some minimal validation of the EDID (e.g., make > sure it has a valid header). Actually we also have plans to abuse this for a bit of nasty EDID injection to exercise our parser. So at most we should do just enough checking to make sure the claimed edid length field agrees with the edid itself (which we have), but beyond that any kind of garbage should be allowed imo. -Daniel
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index fdb69f7..4b3bbc4 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3916,6 +3916,10 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector, struct drm_device *dev = connector->dev; int ret, size; + /* ignore requests to set edid when overridden */ + if (connector->override_edid) + return 0; + if (connector->edid_blob_ptr) drm_property_destroy_blob(dev, connector->edid_blob_ptr); diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index 8ab3f3e..13bd429 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -35,6 +35,7 @@ #include <linux/slab.h> #include <linux/export.h> #include <drm/drmP.h> +#include <drm/drm_edid.h> #if defined(CONFIG_DEBUG_FS) @@ -304,6 +305,67 @@ static ssize_t connector_write(struct file *file, const char __user *ubuf, return len; } +static int edid_show(struct seq_file *m, void *data) +{ + struct drm_connector *connector = m->private; + struct drm_property_blob *edid = connector->edid_blob_ptr; + + if (connector->override_edid && edid) + seq_write(m, edid->data, edid->length); + + return 0; +} + +static int edid_open(struct inode *inode, struct file *file) +{ + struct drm_connector *dev = inode->i_private; + + return single_open(file, edid_show, dev); +} + +static ssize_t edid_write(struct file *file, const char __user *ubuf, + size_t len, loff_t *offp) +{ + struct seq_file *m = file->private_data; + struct drm_connector *connector = m->private; + char *buf; + struct edid *edid; + int ret; + + buf = memdup_user(ubuf, len); + if (IS_ERR(buf)) + return PTR_ERR(buf); + + edid = (struct edid *) buf; + + if (len == 5 && !strncmp(buf, "reset", 5)) { + connector->override_edid = false; + ret = drm_mode_connector_update_edid_property(connector, NULL); + } else if (len < EDID_LENGTH || + EDID_LENGTH * (1 + edid->extensions) > len) + ret = -EINVAL; + else { + connector->override_edid = false; + ret = drm_mode_connector_update_edid_property(connector, edid); + if (!ret) + connector->override_edid = true; + } + + kfree(buf); + + return (ret) ? ret : len; +} + +static const struct file_operations drm_edid_fops = { + .owner = THIS_MODULE, + .open = edid_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, + .write = edid_write +}; + + static const struct file_operations drm_connector_fops = { .owner = THIS_MODULE, .open = connector_open, @@ -333,6 +395,12 @@ int drm_debugfs_connector_add(struct drm_connector *connector) if (!ent) goto error; + /* edid */ + ent = debugfs_create_file("edid_override", S_IRUGO | S_IWUSR, root, + connector, &drm_edid_fops); + if (!ent) + goto error; + return 0; error: diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index d22676b..db7d250 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -130,7 +130,14 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect count = drm_load_edid_firmware(connector); if (count == 0) #endif - count = (*connector_funcs->get_modes)(connector); + { + if (connector->override_edid) { + struct edid *edid = (struct edid *) connector->edid_blob_ptr->data; + + count = drm_add_edid_modes(connector, edid); + } else + count = (*connector_funcs->get_modes)(connector); + } if (count == 0 && connector->status == connector_status_connected) count = drm_add_modes_noedid(connector, 1024, 768); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 67a33bc..d09b471 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -537,6 +537,7 @@ struct drm_connector { /* forced on connector */ enum drm_connector_force force; + bool override_edid; uint32_t encoder_ids[DRM_CONNECTOR_MAX_ENCODER]; struct drm_encoder *encoder; /* currently active encoder */
Add a file to debugfs for each connector to allow the EDID to be overridden. v2: Copy ubuf before accessing it and reject invalid length data. (David Herrmann) Ensure override_edid is reset when a new EDID value is written. (David Herrmann) Fix the debugfs file permissions. (David Herrmann) Signed-off-by: Thomas Wood <thomas.wood@intel.com> --- drivers/gpu/drm/drm_crtc.c | 4 +++ drivers/gpu/drm/drm_debugfs.c | 68 ++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_probe_helper.c | 9 ++++- include/drm/drm_crtc.h | 1 + 4 files changed, 81 insertions(+), 1 deletion(-)