| Message ID | 1443483936-2348-3-git-send-email-rafael.antognolli@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Sep 28, 2015 at 04:45:36PM -0700, Rafael Antognolli wrote: > This module is heavily based on i2c-dev. Once loaded, it provides one > dev node per DP AUX channel, named drm_dp_auxN, where N is an integer. > > It's possible to know which connector owns this aux channel by looking > at the respective sysfs /sys/class/drm_aux_dev/drm_dp_auxN/connector, if > the connector device pointer was correctly set in the aux helper struct. > > Two main operations are provided on the registers read and write. The > address of the register to be read or written is given using lseek. The > seek position is updated upon read or write. > > Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com> > --- > drivers/gpu/drm/Kconfig | 8 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/drm_dp_aux_dev.c | 357 +++++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_dp_helper.c | 5 + > include/drm/drm_dp_aux_dev.h | 50 ++++++ > 5 files changed, 421 insertions(+) > create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c > create mode 100644 include/drm/drm_dp_aux_dev.h > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 1a0a8df..64fa0f4 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -25,6 +25,14 @@ config DRM_MIPI_DSI > bool > depends on DRM > > +config DRM_DP_AUX_CHARDEV > + bool "DRM DP AUX Interface" > + depends on DRM > + help > + Choose this option to enable a /dev/drm_dp_auxN node that allows to > + read and write values to arbitrary DPCD registers on the DP aux > + channel. > + > config DRM_KMS_HELPER > tristate > depends on DRM > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 45e7719..e5ae7f0 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -25,6 +25,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \ > drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o > drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o > drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o > +drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o > > obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o > > diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c > new file mode 100644 > index 0000000..e337081 > --- /dev/null > +++ b/drivers/gpu/drm/drm_dp_aux_dev.c > @@ -0,0 +1,357 @@ > +/* > + * Copyright © 2015 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + * > + * Authors: > + * Rafael Antognolli <rafael.antognolli@intel.com> > + * > + */ > + > +#include <linux/device.h> > +#include <linux/fs.h> > +#include <linux/slab.h> > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <asm/uaccess.h> > +#include <drm/drm_dp_helper.h> > +#include <drm/drm_crtc.h> > + > +struct drm_dp_aux_dev { > + struct list_head list; > + unsigned index; > + struct drm_dp_aux *aux; > + struct device *dev; > +}; > + > +#define DRM_AUX_MINORS 256 > +static int aux_max_offset = 1 << 20; People seem to prefer defines for such things. > +static int drm_dp_aux_dev_count = 0; > +static LIST_HEAD(drm_dp_aux_dev_list); > +static DEFINE_SPINLOCK(drm_dp_aux_dev_list_lock); > + > +static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_minor(unsigned index) > +{ > + struct drm_dp_aux_dev *aux_dev; > + > + spin_lock(&drm_dp_aux_dev_list_lock); > + list_for_each_entry(aux_dev, &drm_dp_aux_dev_list, list) { > + if (aux_dev->index == index) > + goto found; > + } > + > + aux_dev = NULL; > +found: > + spin_unlock(&drm_dp_aux_dev_list_lock); > + return aux_dev; > +} > + > +static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_aux(struct drm_dp_aux *aux) > +{ > + struct drm_dp_aux_dev *aux_dev; > + > + spin_lock(&drm_dp_aux_dev_list_lock); > + list_for_each_entry(aux_dev, &drm_dp_aux_dev_list, list) { > + if (aux_dev->aux == aux) > + goto found; > + } > + > + aux_dev = NULL; > +found: > + spin_unlock(&drm_dp_aux_dev_list_lock); > + return aux_dev; > +} > + > +static struct drm_dp_aux_dev *get_free_drm_dp_aux_dev(struct drm_dp_aux *aux) > +{ > + struct drm_dp_aux_dev *aux_dev; > + int index; > + > + spin_lock(&drm_dp_aux_dev_list_lock); > + index = drm_dp_aux_dev_count; > + spin_unlock(&drm_dp_aux_dev_list_lock); > + if (index >= DRM_AUX_MINORS) { > + printk(KERN_ERR "drm_dp_aux_dev: Out of device minors (%d)\n", > + index); > + return ERR_PTR(-ENODEV); > + } > + > + aux_dev = kzalloc(sizeof(*aux_dev), GFP_KERNEL); > + if (!aux_dev) > + return ERR_PTR(-ENOMEM); > + aux_dev->aux = aux; > + aux_dev->index = index; > + > + spin_lock(&drm_dp_aux_dev_list_lock); > + drm_dp_aux_dev_count++; That's racy. Someone else could have snuck in and pushed drm_dp_aux_dev_count over the limit already. Maybe we want to use an idr or something to allocate the index? > + list_add_tail(&aux_dev->list, &drm_dp_aux_dev_list); > + spin_unlock(&drm_dp_aux_dev_list_lock); > + return aux_dev; > +} > + > +static void return_drm_dp_aux_dev(struct drm_dp_aux_dev *aux_dev) > +{ > + spin_lock(&drm_dp_aux_dev_list_lock); > + list_del(&aux_dev->list); > + spin_unlock(&drm_dp_aux_dev_list_lock); > + kfree(aux_dev); > +} I'm not a fan of the get_free & return names. Maybe just alloc/free? > + > +static ssize_t name_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct drm_dp_aux_dev *aux_dev = > + drm_dp_aux_dev_get_by_minor(MINOR(dev->devt)); > + > + if (!aux_dev) > + return -ENODEV; > + return sprintf(buf, "%s\n", aux_dev->aux->name); > +} > +static DEVICE_ATTR_RO(name); > + > +static ssize_t connector_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct drm_dp_aux_dev *aux_dev = > + drm_dp_aux_dev_get_by_minor(MINOR(dev->devt)); > + struct drm_dp_aux *aux; > + struct device *conn_dev; > + struct drm_connector *connector = NULL; > + > + if (!aux_dev) > + return -ENODEV; > + aux = aux_dev->aux; > + conn_dev = aux->connector; > + if (!conn_dev) > + return sprintf(buf, "unknown\n"); > + > + connector = dev_get_drvdata(aux->connector); > + > + return sprintf(buf, "%s\n", connector->name); > +} > +static DEVICE_ATTR_RO(connector); > + > +static struct attribute *drm_dp_aux_attrs[] = { > + &dev_attr_name.attr, > + &dev_attr_connector.attr, > + NULL, > +}; > +ATTRIBUTE_GROUPS(drm_dp_aux); > + > +static int auxdev_open(struct inode *inode, struct file *file) > +{ > + unsigned int minor = iminor(inode); > + struct drm_dp_aux_dev *aux_dev; > + > + aux_dev = drm_dp_aux_dev_get_by_minor(minor); > + if (!aux_dev) > + return -ENODEV; > + > + file->private_data = aux_dev; > + return 0; > +} > + > +static loff_t auxdev_llseek(struct file *file, loff_t offset, int whence) > +{ > + return fixed_size_llseek(file, offset, whence, aux_max_offset); > +} > + > +static ssize_t auxdev_read(struct file *file, char __user *buf, size_t count, > + loff_t *offset) > +{ > + ssize_t bytes_pending, num_bytes_processed = 0; > + struct drm_dp_aux_dev *aux_dev = file->private_data; > + > + bytes_pending = aux_max_offset - *offset; > + bytes_pending = count < bytes_pending ? count : bytes_pending; I would write it as bytes_pending = min(count, aux_max_offset - *offset); > + if (bytes_pending < 0) > + return -EFAULT; Can this actually happen somehow? > + > + if (!access_ok(VERIFY_WRITE, buf, bytes_pending)) > + return -EFAULT; > + > + while (bytes_pending > 0) { > + uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES]; > + ssize_t todo = sizeof(localbuf); > + ssize_t res; > + todo = bytes_pending < todo ? bytes_pending : todo; Would write it as todo = min(bytes_pending, sizeof(localbuf)); > + > + res = drm_dp_dpcd_read(aux_dev->aux, *offset, localbuf, todo); > + if (res <= 0) { > + return num_bytes_processed ? num_bytes_processed : res; > + } Could drop the extra {} > + if (__copy_to_user(buf + num_bytes_processed, localbuf, res)) { > + return num_bytes_processed ? > + num_bytes_processed : -EFAULT; > + } ditto > + bytes_pending -= res; > + *offset += res; > + num_bytes_processed += res; > + } > + > + return num_bytes_processed; > +} > + > +static ssize_t auxdev_write(struct file *file, const char __user *buf, > + size_t count, loff_t *offset) > +{ > + ssize_t bytes_pending, num_bytes_processed = 0; > + struct drm_dp_aux_dev *aux_dev = file->private_data; > + > + bytes_pending = aux_max_offset - *offset; > + bytes_pending = count < bytes_pending ? count : bytes_pending; > + if (bytes_pending < 0) > + return -EFAULT; > + > + if (!access_ok(VERIFY_READ, buf, bytes_pending)) > + return -EFAULT; > + > + while (bytes_pending > 0) { > + uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES]; > + ssize_t todo = sizeof(localbuf); > + ssize_t res; > + todo = bytes_pending < todo ? bytes_pending : todo; > + if (__copy_from_user(localbuf, > + buf + num_bytes_processed, todo)) { > + return num_bytes_processed ? > + num_bytes_processed : -EFAULT; > + } > + > + res = drm_dp_dpcd_write(aux_dev->aux, *offset, localbuf, todo); > + if (res <= 0) { > + return num_bytes_processed ? num_bytes_processed : res; > + } same comments for write as for read. > + bytes_pending -= res; > + *offset += res; > + num_bytes_processed += res; > + } > + > + return num_bytes_processed; > +} > + > +static const struct file_operations auxdev_fops = { > + .owner = THIS_MODULE, > + .llseek = auxdev_llseek, > + .read = auxdev_read, > + .write = auxdev_write, > + .open = auxdev_open, > +}; > + > +static struct class *drm_dp_aux_dev_class; > +static int drm_dev_major = -1; > + > +#define to_auxdev(d) container_of(d, struct drm_dp_aux_dev, aux) > + > +/** > + * drm_dp_aux_register_devnode() - register a devnode for this aux channel > + * @aux: DisplayPort AUX channel > + * > + * Returns 0 on success or a negative error code on failure. > + */ > +int drm_dp_aux_register_devnode(struct drm_dp_aux *aux) > +{ > + struct drm_dp_aux_dev *aux_dev; > + int res; > + > + aux_dev = get_free_drm_dp_aux_dev(aux); > + if (IS_ERR(aux_dev)) > + return PTR_ERR(aux_dev); > + > + aux_dev->dev = device_create(drm_dp_aux_dev_class, aux->dev, > + MKDEV(drm_dev_major, aux_dev->index), NULL, > + "drm_dp_aux%d", aux_dev->index); > + if (IS_ERR(aux_dev->dev)) { > + res = PTR_ERR(aux_dev->dev); > + goto error; > + } > + > + pr_debug("drm_dp_aux_dev: aux [%s] registered as minor %d\n", > + aux->name, aux_dev->index); > + return 0; > +error: > + return_drm_dp_aux_dev(aux_dev); > + return res; > + > +} > + > +/** > + * drm_dp_aux_unregister_devnode() - unregister a devnode for this aux channel > + * @aux: DisplayPort AUX channel > + * > + * Returns 0 on success or a negative error code on failure. > + */ > +int drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux) > +{ > + int minor; > + struct drm_dp_aux_dev *aux_dev; > + > + aux_dev = drm_dp_aux_dev_get_by_aux(aux); > + if (!aux_dev) /* attach must have failed */ > + return 0; > + > + minor = aux_dev->index; > + return_drm_dp_aux_dev(aux_dev); > + device_destroy(drm_dp_aux_dev_class, MKDEV(drm_dev_major, minor)); device_destroy before you free the auxdev seem safer. What happens if someone has the device still open? Do we need to kref the aux_dev and add some kind of dead flag or something? > + > + pr_debug("drm_dp_aux_dev: aux [%s] unregistered\n", aux->name); > + return 0; > +} > + > +static int __init drm_dp_aux_dev_init(void) > +{ > + int res; > + > + printk(KERN_INFO "drm dp aux /dev entries driver\n"); I don't see much point in this printk. > + > + res = register_chrdev(0, "aux", &auxdev_fops); > + if (res < 0) > + goto out; > + drm_dev_major = res; > + > + drm_dp_aux_dev_class = class_create(THIS_MODULE, "drm_dp_aux_dev"); > + if (IS_ERR(drm_dp_aux_dev_class)) { > + res = PTR_ERR(drm_dp_aux_dev_class); > + goto out_unreg_chrdev; > + } > + drm_dp_aux_dev_class->dev_groups = drm_dp_aux_groups; > + > + return 0; > + > +out_unreg_chrdev: > + unregister_chrdev(drm_dev_major, "aux"); > +out: > + printk(KERN_ERR "%s: Driver Initialisation failed\n", __FILE__); > + return res; > +} > + > +static void __exit drm_dp_aux_dev_exit(void) > +{ > + printk(KERN_INFO "drm dp aux /dev entries driver - unloading\n"); ditto > + class_destroy(drm_dp_aux_dev_class); > + unregister_chrdev(drm_dev_major, "aux"); > +} > + > +MODULE_AUTHOR("Rafael Antognolli <rafael.antognolli@intel.com>"); > +MODULE_DESCRIPTION("DRM DP AUX /dev entries driver"); > +MODULE_LICENSE("GPL"); Doesn't match your license boilerplate. > + > +module_init(drm_dp_aux_dev_init); > +module_exit(drm_dp_aux_dev_exit); > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 9535c5b..f3a2eb0 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -28,6 +28,7 @@ > #include <linux/sched.h> > #include <linux/i2c.h> > #include <drm/drm_dp_helper.h> > +#include <drm/drm_dp_aux_dev.h> > #include <drm/drmP.h> > > /** > @@ -768,6 +769,9 @@ int drm_dp_aux_register(struct drm_dp_aux *aux) > strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(aux->dev), > sizeof(aux->ddc.name)); > > + > + drm_dp_aux_register_devnode(aux); > + > return i2c_add_adapter(&aux->ddc); > } > EXPORT_SYMBOL(drm_dp_aux_register); > @@ -778,6 +782,7 @@ EXPORT_SYMBOL(drm_dp_aux_register); > */ > void drm_dp_aux_unregister(struct drm_dp_aux *aux) > { > + drm_dp_aux_unregister_devnode(aux); > i2c_del_adapter(&aux->ddc); > } > EXPORT_SYMBOL(drm_dp_aux_unregister); > diff --git a/include/drm/drm_dp_aux_dev.h b/include/drm/drm_dp_aux_dev.h > new file mode 100644 > index 0000000..77534e8 > --- /dev/null > +++ b/include/drm/drm_dp_aux_dev.h > @@ -0,0 +1,50 @@ > +/* > + * Copyright © 2015 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + * > + * Authors: > + * Rafael Antognolli <rafael.antognolli@intel.com> > + * > + */ > + > +#ifndef _UAPI_LINUX_DRM_DP_AUX_DEV_ > +#define _UAPI_LINUX_DRM_DP_AUX_DEV_ Not uapi. #ifndef DRM_DP_AUX_DEV_H #define ... would seem line enough. > + > +#ifdef CONFIG_DRM_DP_AUX_CHARDEV > + > +int drm_dp_aux_register_devnode(struct drm_dp_aux *aux); > +int drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux); > + > +#else > + > +static inline int drm_dp_aux_register_devnode(struct drm_dp_aux *aux) > +{ > + return 0; > +} > + > +static inline int drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux) > +{ > + return 0; > +} > + > +#endif > + > +#endif > -- > 2.4.3 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Thanks Ville for the review, I'm addressing all the issues but have some questions on the ones mentioned below: On Tue, Sep 29, 2015 at 05:09:23PM +0300, Ville Syrjälä wrote: > On Mon, Sep 28, 2015 at 04:45:36PM -0700, Rafael Antognolli wrote: > > This module is heavily based on i2c-dev. Once loaded, it provides one > > dev node per DP AUX channel, named drm_dp_auxN, where N is an integer. > > > > It's possible to know which connector owns this aux channel by looking > > at the respective sysfs /sys/class/drm_aux_dev/drm_dp_auxN/connector, if > > the connector device pointer was correctly set in the aux helper struct. > > > > Two main operations are provided on the registers read and write. The > > address of the register to be read or written is given using lseek. The > > seek position is updated upon read or write. > > > > Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com> > > --- > > drivers/gpu/drm/Kconfig | 8 + > > drivers/gpu/drm/Makefile | 1 + > > drivers/gpu/drm/drm_dp_aux_dev.c | 357 +++++++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/drm_dp_helper.c | 5 + > > include/drm/drm_dp_aux_dev.h | 50 ++++++ > > 5 files changed, 421 insertions(+) > > create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c > > create mode 100644 include/drm/drm_dp_aux_dev.h > > > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > > index 1a0a8df..64fa0f4 100644 > > --- a/drivers/gpu/drm/Kconfig > > +++ b/drivers/gpu/drm/Kconfig > > @@ -25,6 +25,14 @@ config DRM_MIPI_DSI > > bool > > depends on DRM > > > > +config DRM_DP_AUX_CHARDEV > > + bool "DRM DP AUX Interface" > > + depends on DRM > > + help > > + Choose this option to enable a /dev/drm_dp_auxN node that allows to > > + read and write values to arbitrary DPCD registers on the DP aux > > + channel. > > + > > config DRM_KMS_HELPER > > tristate > > depends on DRM > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > > index 45e7719..e5ae7f0 100644 > > --- a/drivers/gpu/drm/Makefile > > +++ b/drivers/gpu/drm/Makefile > > @@ -25,6 +25,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \ > > drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o > > drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o > > drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o > > +drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o > > > > obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o > > > > diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c > > new file mode 100644 > > index 0000000..e337081 > > --- /dev/null > > +++ b/drivers/gpu/drm/drm_dp_aux_dev.c > > @@ -0,0 +1,357 @@ > > +/* > > + * Copyright © 2015 Intel Corporation > > + * > > + * Permission is hereby granted, free of charge, to any person obtaining a > > + * copy of this software and associated documentation files (the "Software"), > > + * to deal in the Software without restriction, including without limitation > > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > > + * and/or sell copies of the Software, and to permit persons to whom the > > + * Software is furnished to do so, subject to the following conditions: > > + * > > + * The above copyright notice and this permission notice (including the next > > + * paragraph) shall be included in all copies or substantial portions of the > > + * Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > > + * IN THE SOFTWARE. > > + * > > + * Authors: > > + * Rafael Antognolli <rafael.antognolli@intel.com> > > + * > > + */ > > + > > +#include <linux/device.h> > > +#include <linux/fs.h> > > +#include <linux/slab.h> > > +#include <linux/init.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <asm/uaccess.h> > > +#include <drm/drm_dp_helper.h> > > +#include <drm/drm_crtc.h> > > + > > +struct drm_dp_aux_dev { > > + struct list_head list; > > + unsigned index; > > + struct drm_dp_aux *aux; > > + struct device *dev; > > +}; > > + > > +#define DRM_AUX_MINORS 256 > > +static int aux_max_offset = 1 << 20; > > People seem to prefer defines for such things. > > > +static int drm_dp_aux_dev_count = 0; > > +static LIST_HEAD(drm_dp_aux_dev_list); > > +static DEFINE_SPINLOCK(drm_dp_aux_dev_list_lock); > > + > > +static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_minor(unsigned index) > > +{ > > + struct drm_dp_aux_dev *aux_dev; > > + > > + spin_lock(&drm_dp_aux_dev_list_lock); > > + list_for_each_entry(aux_dev, &drm_dp_aux_dev_list, list) { > > + if (aux_dev->index == index) > > + goto found; > > + } > > + > > + aux_dev = NULL; > > +found: > > + spin_unlock(&drm_dp_aux_dev_list_lock); > > + return aux_dev; > > +} > > + > > +static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_aux(struct drm_dp_aux *aux) > > +{ > > + struct drm_dp_aux_dev *aux_dev; > > + > > + spin_lock(&drm_dp_aux_dev_list_lock); > > + list_for_each_entry(aux_dev, &drm_dp_aux_dev_list, list) { > > + if (aux_dev->aux == aux) > > + goto found; > > + } > > + > > + aux_dev = NULL; > > +found: > > + spin_unlock(&drm_dp_aux_dev_list_lock); > > + return aux_dev; > > +} > > + > > +static struct drm_dp_aux_dev *get_free_drm_dp_aux_dev(struct drm_dp_aux *aux) > > +{ > > + struct drm_dp_aux_dev *aux_dev; > > + int index; > > + > > + spin_lock(&drm_dp_aux_dev_list_lock); > > + index = drm_dp_aux_dev_count; > > + spin_unlock(&drm_dp_aux_dev_list_lock); > > + if (index >= DRM_AUX_MINORS) { > > + printk(KERN_ERR "drm_dp_aux_dev: Out of device minors (%d)\n", > > + index); > > + return ERR_PTR(-ENODEV); > > + } > > + > > + aux_dev = kzalloc(sizeof(*aux_dev), GFP_KERNEL); > > + if (!aux_dev) > > + return ERR_PTR(-ENOMEM); > > + aux_dev->aux = aux; > > + aux_dev->index = index; > > + > > + spin_lock(&drm_dp_aux_dev_list_lock); > > + drm_dp_aux_dev_count++; > > That's racy. Someone else could have snuck in and pushed > drm_dp_aux_dev_count over the limit already. > > Maybe we want to use an idr or something to allocate the > index? > > > + list_add_tail(&aux_dev->list, &drm_dp_aux_dev_list); > > + spin_unlock(&drm_dp_aux_dev_list_lock); > > + return aux_dev; > > +} > > + > > +static void return_drm_dp_aux_dev(struct drm_dp_aux_dev *aux_dev) > > +{ > > + spin_lock(&drm_dp_aux_dev_list_lock); > > + list_del(&aux_dev->list); > > + spin_unlock(&drm_dp_aux_dev_list_lock); > > + kfree(aux_dev); > > +} > > I'm not a fan of the get_free & return names. Maybe just alloc/free? > > > + > > +static ssize_t name_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + struct drm_dp_aux_dev *aux_dev = > > + drm_dp_aux_dev_get_by_minor(MINOR(dev->devt)); > > + > > + if (!aux_dev) > > + return -ENODEV; > > + return sprintf(buf, "%s\n", aux_dev->aux->name); > > +} > > +static DEVICE_ATTR_RO(name); > > + > > +static ssize_t connector_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + struct drm_dp_aux_dev *aux_dev = > > + drm_dp_aux_dev_get_by_minor(MINOR(dev->devt)); > > + struct drm_dp_aux *aux; > > + struct device *conn_dev; > > + struct drm_connector *connector = NULL; > > + > > + if (!aux_dev) > > + return -ENODEV; > > + aux = aux_dev->aux; > > + conn_dev = aux->connector; > > + if (!conn_dev) > > + return sprintf(buf, "unknown\n"); > > + > > + connector = dev_get_drvdata(aux->connector); > > + > > + return sprintf(buf, "%s\n", connector->name); > > +} > > +static DEVICE_ATTR_RO(connector); > > + > > +static struct attribute *drm_dp_aux_attrs[] = { > > + &dev_attr_name.attr, > > + &dev_attr_connector.attr, > > + NULL, > > +}; > > +ATTRIBUTE_GROUPS(drm_dp_aux); > > + > > +static int auxdev_open(struct inode *inode, struct file *file) > > +{ > > + unsigned int minor = iminor(inode); > > + struct drm_dp_aux_dev *aux_dev; > > + > > + aux_dev = drm_dp_aux_dev_get_by_minor(minor); > > + if (!aux_dev) > > + return -ENODEV; > > + > > + file->private_data = aux_dev; > > + return 0; > > +} > > + > > +static loff_t auxdev_llseek(struct file *file, loff_t offset, int whence) > > +{ > > + return fixed_size_llseek(file, offset, whence, aux_max_offset); > > +} > > + > > +static ssize_t auxdev_read(struct file *file, char __user *buf, size_t count, > > + loff_t *offset) > > +{ > > + ssize_t bytes_pending, num_bytes_processed = 0; > > + struct drm_dp_aux_dev *aux_dev = file->private_data; > > + > > + bytes_pending = aux_max_offset - *offset; > > + bytes_pending = count < bytes_pending ? count : bytes_pending; > > I would write it as > bytes_pending = min(count, aux_max_offset - *offset); > > > + if (bytes_pending < 0) > > + return -EFAULT; > > Can this actually happen somehow? I believe the "count" argument could be negative, not sure if the kernel does any checks on that. Still have to look that up. But if that's the case, I could just check for count < 0 earlier on, right? > > + > > + if (!access_ok(VERIFY_WRITE, buf, bytes_pending)) > > + return -EFAULT; > > + > > + while (bytes_pending > 0) { > > + uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES]; > > + ssize_t todo = sizeof(localbuf); > > + ssize_t res; > > + todo = bytes_pending < todo ? bytes_pending : todo; > > Would write it as > todo = min(bytes_pending, sizeof(localbuf)); > > > + > > + res = drm_dp_dpcd_read(aux_dev->aux, *offset, localbuf, todo); > > + if (res <= 0) { > > + return num_bytes_processed ? num_bytes_processed : res; > > + } > > Could drop the extra {} > > > + if (__copy_to_user(buf + num_bytes_processed, localbuf, res)) { > > + return num_bytes_processed ? > > + num_bytes_processed : -EFAULT; > > + } > > ditto > > > + bytes_pending -= res; > > + *offset += res; > > + num_bytes_processed += res; > > + } > > + > > + return num_bytes_processed; > > +} > > + > > +static ssize_t auxdev_write(struct file *file, const char __user *buf, > > + size_t count, loff_t *offset) > > +{ > > + ssize_t bytes_pending, num_bytes_processed = 0; > > + struct drm_dp_aux_dev *aux_dev = file->private_data; > > + > > + bytes_pending = aux_max_offset - *offset; > > + bytes_pending = count < bytes_pending ? count : bytes_pending; > > + if (bytes_pending < 0) > > + return -EFAULT; > > + > > + if (!access_ok(VERIFY_READ, buf, bytes_pending)) > > + return -EFAULT; > > + > > + while (bytes_pending > 0) { > > + uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES]; > > + ssize_t todo = sizeof(localbuf); > > + ssize_t res; > > + todo = bytes_pending < todo ? bytes_pending : todo; > > + if (__copy_from_user(localbuf, > > + buf + num_bytes_processed, todo)) { > > + return num_bytes_processed ? > > + num_bytes_processed : -EFAULT; > > + } > > + > > + res = drm_dp_dpcd_write(aux_dev->aux, *offset, localbuf, todo); > > + if (res <= 0) { > > + return num_bytes_processed ? num_bytes_processed : res; > > + } > > same comments for write as for read. > > > + bytes_pending -= res; > > + *offset += res; > > + num_bytes_processed += res; > > + } > > + > > + return num_bytes_processed; > > +} > > + > > +static const struct file_operations auxdev_fops = { > > + .owner = THIS_MODULE, > > + .llseek = auxdev_llseek, > > + .read = auxdev_read, > > + .write = auxdev_write, > > + .open = auxdev_open, > > +}; > > + > > +static struct class *drm_dp_aux_dev_class; > > +static int drm_dev_major = -1; > > + > > +#define to_auxdev(d) container_of(d, struct drm_dp_aux_dev, aux) > > + > > +/** > > + * drm_dp_aux_register_devnode() - register a devnode for this aux channel > > + * @aux: DisplayPort AUX channel > > + * > > + * Returns 0 on success or a negative error code on failure. > > + */ > > +int drm_dp_aux_register_devnode(struct drm_dp_aux *aux) > > +{ > > + struct drm_dp_aux_dev *aux_dev; > > + int res; > > + > > + aux_dev = get_free_drm_dp_aux_dev(aux); > > + if (IS_ERR(aux_dev)) > > + return PTR_ERR(aux_dev); > > + > > + aux_dev->dev = device_create(drm_dp_aux_dev_class, aux->dev, > > + MKDEV(drm_dev_major, aux_dev->index), NULL, > > + "drm_dp_aux%d", aux_dev->index); > > + if (IS_ERR(aux_dev->dev)) { > > + res = PTR_ERR(aux_dev->dev); > > + goto error; > > + } > > + > > + pr_debug("drm_dp_aux_dev: aux [%s] registered as minor %d\n", > > + aux->name, aux_dev->index); > > + return 0; > > +error: > > + return_drm_dp_aux_dev(aux_dev); > > + return res; > > + > > +} > > + > > +/** > > + * drm_dp_aux_unregister_devnode() - unregister a devnode for this aux channel > > + * @aux: DisplayPort AUX channel > > + * > > + * Returns 0 on success or a negative error code on failure. > > + */ > > +int drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux) > > +{ > > + int minor; > > + struct drm_dp_aux_dev *aux_dev; > > + > > + aux_dev = drm_dp_aux_dev_get_by_aux(aux); > > + if (!aux_dev) /* attach must have failed */ > > + return 0; > > + > > + minor = aux_dev->index; > > + return_drm_dp_aux_dev(aux_dev); > > + device_destroy(drm_dp_aux_dev_class, MKDEV(drm_dev_major, minor)); > > device_destroy before you free the auxdev seem safer. > > What happens if someone has the device still open? Do we need to kref the > aux_dev and add some kind of dead flag or something? Yeah, I couldn't find anything that says the device would stay around while the file is still open, so I guess you are right. So, I would need to put some locks around the aux_dev usage too, right? > > + > > + pr_debug("drm_dp_aux_dev: aux [%s] unregistered\n", aux->name); > > + return 0; > > +} > > + > > +static int __init drm_dp_aux_dev_init(void) > > +{ > > + int res; > > + > > + printk(KERN_INFO "drm dp aux /dev entries driver\n"); > > I don't see much point in this printk. > > > + > > + res = register_chrdev(0, "aux", &auxdev_fops); > > + if (res < 0) > > + goto out; > > + drm_dev_major = res; > > + > > + drm_dp_aux_dev_class = class_create(THIS_MODULE, "drm_dp_aux_dev"); > > + if (IS_ERR(drm_dp_aux_dev_class)) { > > + res = PTR_ERR(drm_dp_aux_dev_class); > > + goto out_unreg_chrdev; > > + } > > + drm_dp_aux_dev_class->dev_groups = drm_dp_aux_groups; > > + > > + return 0; > > + > > +out_unreg_chrdev: > > + unregister_chrdev(drm_dev_major, "aux"); > > +out: > > + printk(KERN_ERR "%s: Driver Initialisation failed\n", __FILE__); > > + return res; > > +} > > + > > +static void __exit drm_dp_aux_dev_exit(void) > > +{ > > + printk(KERN_INFO "drm dp aux /dev entries driver - unloading\n"); > > ditto > > > + class_destroy(drm_dp_aux_dev_class); > > + unregister_chrdev(drm_dev_major, "aux"); > > +} > > + > > +MODULE_AUTHOR("Rafael Antognolli <rafael.antognolli@intel.com>"); > > +MODULE_DESCRIPTION("DRM DP AUX /dev entries driver"); > > +MODULE_LICENSE("GPL"); > > Doesn't match your license boilerplate. So, should it be "GPL and additional rights"? > > + > > +module_init(drm_dp_aux_dev_init); > > +module_exit(drm_dp_aux_dev_exit); > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > > index 9535c5b..f3a2eb0 100644 > > --- a/drivers/gpu/drm/drm_dp_helper.c > > +++ b/drivers/gpu/drm/drm_dp_helper.c > > @@ -28,6 +28,7 @@ > > #include <linux/sched.h> > > #include <linux/i2c.h> > > #include <drm/drm_dp_helper.h> > > +#include <drm/drm_dp_aux_dev.h> > > #include <drm/drmP.h> > > > > /** > > @@ -768,6 +769,9 @@ int drm_dp_aux_register(struct drm_dp_aux *aux) > > strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(aux->dev), > > sizeof(aux->ddc.name)); > > > > + > > + drm_dp_aux_register_devnode(aux); > > + > > return i2c_add_adapter(&aux->ddc); > > } > > EXPORT_SYMBOL(drm_dp_aux_register); > > @@ -778,6 +782,7 @@ EXPORT_SYMBOL(drm_dp_aux_register); > > */ > > void drm_dp_aux_unregister(struct drm_dp_aux *aux) > > { > > + drm_dp_aux_unregister_devnode(aux); > > i2c_del_adapter(&aux->ddc); > > } > > EXPORT_SYMBOL(drm_dp_aux_unregister); > > diff --git a/include/drm/drm_dp_aux_dev.h b/include/drm/drm_dp_aux_dev.h > > new file mode 100644 > > index 0000000..77534e8 > > --- /dev/null > > +++ b/include/drm/drm_dp_aux_dev.h > > @@ -0,0 +1,50 @@ > > +/* > > + * Copyright © 2015 Intel Corporation > > + * > > + * Permission is hereby granted, free of charge, to any person obtaining a > > + * copy of this software and associated documentation files (the "Software"), > > + * to deal in the Software without restriction, including without limitation > > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > > + * and/or sell copies of the Software, and to permit persons to whom the > > + * Software is furnished to do so, subject to the following conditions: > > + * > > + * The above copyright notice and this permission notice (including the next > > + * paragraph) shall be included in all copies or substantial portions of the > > + * Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > > + * IN THE SOFTWARE. > > + * > > + * Authors: > > + * Rafael Antognolli <rafael.antognolli@intel.com> > > + * > > + */ > > + > > +#ifndef _UAPI_LINUX_DRM_DP_AUX_DEV_ > > +#define _UAPI_LINUX_DRM_DP_AUX_DEV_ > > Not uapi. > > #ifndef DRM_DP_AUX_DEV_H > #define ... > would seem line enough. > > > + > > +#ifdef CONFIG_DRM_DP_AUX_CHARDEV > > + > > +int drm_dp_aux_register_devnode(struct drm_dp_aux *aux); > > +int drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux); > > + > > +#else > > + > > +static inline int drm_dp_aux_register_devnode(struct drm_dp_aux *aux) > > +{ > > + return 0; > > +} > > + > > +static inline int drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux) > > +{ > > + return 0; > > +} > > + > > +#endif > > + > > +#endif > > -- > > 2.4.3 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel Thanks, Rafael
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 1a0a8df..64fa0f4 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -25,6 +25,14 @@ config DRM_MIPI_DSI bool depends on DRM +config DRM_DP_AUX_CHARDEV + bool "DRM DP AUX Interface" + depends on DRM + help + Choose this option to enable a /dev/drm_dp_auxN node that allows to + read and write values to arbitrary DPCD registers on the DP aux + channel. + config DRM_KMS_HELPER tristate depends on DRM diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 45e7719..e5ae7f0 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -25,6 +25,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \ drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o +drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c new file mode 100644 index 0000000..e337081 --- /dev/null +++ b/drivers/gpu/drm/drm_dp_aux_dev.c @@ -0,0 +1,357 @@ +/* + * Copyright © 2015 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + * Rafael Antognolli <rafael.antognolli@intel.com> + * + */ + +#include <linux/device.h> +#include <linux/fs.h> +#include <linux/slab.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <asm/uaccess.h> +#include <drm/drm_dp_helper.h> +#include <drm/drm_crtc.h> + +struct drm_dp_aux_dev { + struct list_head list; + unsigned index; + struct drm_dp_aux *aux; + struct device *dev; +}; + +#define DRM_AUX_MINORS 256 +static int aux_max_offset = 1 << 20; +static int drm_dp_aux_dev_count = 0; +static LIST_HEAD(drm_dp_aux_dev_list); +static DEFINE_SPINLOCK(drm_dp_aux_dev_list_lock); + +static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_minor(unsigned index) +{ + struct drm_dp_aux_dev *aux_dev; + + spin_lock(&drm_dp_aux_dev_list_lock); + list_for_each_entry(aux_dev, &drm_dp_aux_dev_list, list) { + if (aux_dev->index == index) + goto found; + } + + aux_dev = NULL; +found: + spin_unlock(&drm_dp_aux_dev_list_lock); + return aux_dev; +} + +static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_aux(struct drm_dp_aux *aux) +{ + struct drm_dp_aux_dev *aux_dev; + + spin_lock(&drm_dp_aux_dev_list_lock); + list_for_each_entry(aux_dev, &drm_dp_aux_dev_list, list) { + if (aux_dev->aux == aux) + goto found; + } + + aux_dev = NULL; +found: + spin_unlock(&drm_dp_aux_dev_list_lock); + return aux_dev; +} + +static struct drm_dp_aux_dev *get_free_drm_dp_aux_dev(struct drm_dp_aux *aux) +{ + struct drm_dp_aux_dev *aux_dev; + int index; + + spin_lock(&drm_dp_aux_dev_list_lock); + index = drm_dp_aux_dev_count; + spin_unlock(&drm_dp_aux_dev_list_lock); + if (index >= DRM_AUX_MINORS) { + printk(KERN_ERR "drm_dp_aux_dev: Out of device minors (%d)\n", + index); + return ERR_PTR(-ENODEV); + } + + aux_dev = kzalloc(sizeof(*aux_dev), GFP_KERNEL); + if (!aux_dev) + return ERR_PTR(-ENOMEM); + aux_dev->aux = aux; + aux_dev->index = index; + + spin_lock(&drm_dp_aux_dev_list_lock); + drm_dp_aux_dev_count++; + list_add_tail(&aux_dev->list, &drm_dp_aux_dev_list); + spin_unlock(&drm_dp_aux_dev_list_lock); + return aux_dev; +} + +static void return_drm_dp_aux_dev(struct drm_dp_aux_dev *aux_dev) +{ + spin_lock(&drm_dp_aux_dev_list_lock); + list_del(&aux_dev->list); + spin_unlock(&drm_dp_aux_dev_list_lock); + kfree(aux_dev); +} + +static ssize_t name_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct drm_dp_aux_dev *aux_dev = + drm_dp_aux_dev_get_by_minor(MINOR(dev->devt)); + + if (!aux_dev) + return -ENODEV; + return sprintf(buf, "%s\n", aux_dev->aux->name); +} +static DEVICE_ATTR_RO(name); + +static ssize_t connector_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct drm_dp_aux_dev *aux_dev = + drm_dp_aux_dev_get_by_minor(MINOR(dev->devt)); + struct drm_dp_aux *aux; + struct device *conn_dev; + struct drm_connector *connector = NULL; + + if (!aux_dev) + return -ENODEV; + aux = aux_dev->aux; + conn_dev = aux->connector; + if (!conn_dev) + return sprintf(buf, "unknown\n"); + + connector = dev_get_drvdata(aux->connector); + + return sprintf(buf, "%s\n", connector->name); +} +static DEVICE_ATTR_RO(connector); + +static struct attribute *drm_dp_aux_attrs[] = { + &dev_attr_name.attr, + &dev_attr_connector.attr, + NULL, +}; +ATTRIBUTE_GROUPS(drm_dp_aux); + +static int auxdev_open(struct inode *inode, struct file *file) +{ + unsigned int minor = iminor(inode); + struct drm_dp_aux_dev *aux_dev; + + aux_dev = drm_dp_aux_dev_get_by_minor(minor); + if (!aux_dev) + return -ENODEV; + + file->private_data = aux_dev; + return 0; +} + +static loff_t auxdev_llseek(struct file *file, loff_t offset, int whence) +{ + return fixed_size_llseek(file, offset, whence, aux_max_offset); +} + +static ssize_t auxdev_read(struct file *file, char __user *buf, size_t count, + loff_t *offset) +{ + ssize_t bytes_pending, num_bytes_processed = 0; + struct drm_dp_aux_dev *aux_dev = file->private_data; + + bytes_pending = aux_max_offset - *offset; + bytes_pending = count < bytes_pending ? count : bytes_pending; + if (bytes_pending < 0) + return -EFAULT; + + if (!access_ok(VERIFY_WRITE, buf, bytes_pending)) + return -EFAULT; + + while (bytes_pending > 0) { + uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES]; + ssize_t todo = sizeof(localbuf); + ssize_t res; + todo = bytes_pending < todo ? bytes_pending : todo; + + res = drm_dp_dpcd_read(aux_dev->aux, *offset, localbuf, todo); + if (res <= 0) { + return num_bytes_processed ? num_bytes_processed : res; + } + if (__copy_to_user(buf + num_bytes_processed, localbuf, res)) { + return num_bytes_processed ? + num_bytes_processed : -EFAULT; + } + bytes_pending -= res; + *offset += res; + num_bytes_processed += res; + } + + return num_bytes_processed; +} + +static ssize_t auxdev_write(struct file *file, const char __user *buf, + size_t count, loff_t *offset) +{ + ssize_t bytes_pending, num_bytes_processed = 0; + struct drm_dp_aux_dev *aux_dev = file->private_data; + + bytes_pending = aux_max_offset - *offset; + bytes_pending = count < bytes_pending ? count : bytes_pending; + if (bytes_pending < 0) + return -EFAULT; + + if (!access_ok(VERIFY_READ, buf, bytes_pending)) + return -EFAULT; + + while (bytes_pending > 0) { + uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES]; + ssize_t todo = sizeof(localbuf); + ssize_t res; + todo = bytes_pending < todo ? bytes_pending : todo; + if (__copy_from_user(localbuf, + buf + num_bytes_processed, todo)) { + return num_bytes_processed ? + num_bytes_processed : -EFAULT; + } + + res = drm_dp_dpcd_write(aux_dev->aux, *offset, localbuf, todo); + if (res <= 0) { + return num_bytes_processed ? num_bytes_processed : res; + } + bytes_pending -= res; + *offset += res; + num_bytes_processed += res; + } + + return num_bytes_processed; +} + +static const struct file_operations auxdev_fops = { + .owner = THIS_MODULE, + .llseek = auxdev_llseek, + .read = auxdev_read, + .write = auxdev_write, + .open = auxdev_open, +}; + +static struct class *drm_dp_aux_dev_class; +static int drm_dev_major = -1; + +#define to_auxdev(d) container_of(d, struct drm_dp_aux_dev, aux) + +/** + * drm_dp_aux_register_devnode() - register a devnode for this aux channel + * @aux: DisplayPort AUX channel + * + * Returns 0 on success or a negative error code on failure. + */ +int drm_dp_aux_register_devnode(struct drm_dp_aux *aux) +{ + struct drm_dp_aux_dev *aux_dev; + int res; + + aux_dev = get_free_drm_dp_aux_dev(aux); + if (IS_ERR(aux_dev)) + return PTR_ERR(aux_dev); + + aux_dev->dev = device_create(drm_dp_aux_dev_class, aux->dev, + MKDEV(drm_dev_major, aux_dev->index), NULL, + "drm_dp_aux%d", aux_dev->index); + if (IS_ERR(aux_dev->dev)) { + res = PTR_ERR(aux_dev->dev); + goto error; + } + + pr_debug("drm_dp_aux_dev: aux [%s] registered as minor %d\n", + aux->name, aux_dev->index); + return 0; +error: + return_drm_dp_aux_dev(aux_dev); + return res; + +} + +/** + * drm_dp_aux_unregister_devnode() - unregister a devnode for this aux channel + * @aux: DisplayPort AUX channel + * + * Returns 0 on success or a negative error code on failure. + */ +int drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux) +{ + int minor; + struct drm_dp_aux_dev *aux_dev; + + aux_dev = drm_dp_aux_dev_get_by_aux(aux); + if (!aux_dev) /* attach must have failed */ + return 0; + + minor = aux_dev->index; + return_drm_dp_aux_dev(aux_dev); + device_destroy(drm_dp_aux_dev_class, MKDEV(drm_dev_major, minor)); + + pr_debug("drm_dp_aux_dev: aux [%s] unregistered\n", aux->name); + return 0; +} + +static int __init drm_dp_aux_dev_init(void) +{ + int res; + + printk(KERN_INFO "drm dp aux /dev entries driver\n"); + + res = register_chrdev(0, "aux", &auxdev_fops); + if (res < 0) + goto out; + drm_dev_major = res; + + drm_dp_aux_dev_class = class_create(THIS_MODULE, "drm_dp_aux_dev"); + if (IS_ERR(drm_dp_aux_dev_class)) { + res = PTR_ERR(drm_dp_aux_dev_class); + goto out_unreg_chrdev; + } + drm_dp_aux_dev_class->dev_groups = drm_dp_aux_groups; + + return 0; + +out_unreg_chrdev: + unregister_chrdev(drm_dev_major, "aux"); +out: + printk(KERN_ERR "%s: Driver Initialisation failed\n", __FILE__); + return res; +} + +static void __exit drm_dp_aux_dev_exit(void) +{ + printk(KERN_INFO "drm dp aux /dev entries driver - unloading\n"); + class_destroy(drm_dp_aux_dev_class); + unregister_chrdev(drm_dev_major, "aux"); +} + +MODULE_AUTHOR("Rafael Antognolli <rafael.antognolli@intel.com>"); +MODULE_DESCRIPTION("DRM DP AUX /dev entries driver"); +MODULE_LICENSE("GPL"); + +module_init(drm_dp_aux_dev_init); +module_exit(drm_dp_aux_dev_exit); diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 9535c5b..f3a2eb0 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -28,6 +28,7 @@ #include <linux/sched.h> #include <linux/i2c.h> #include <drm/drm_dp_helper.h> +#include <drm/drm_dp_aux_dev.h> #include <drm/drmP.h> /** @@ -768,6 +769,9 @@ int drm_dp_aux_register(struct drm_dp_aux *aux) strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(aux->dev), sizeof(aux->ddc.name)); + + drm_dp_aux_register_devnode(aux); + return i2c_add_adapter(&aux->ddc); } EXPORT_SYMBOL(drm_dp_aux_register); @@ -778,6 +782,7 @@ EXPORT_SYMBOL(drm_dp_aux_register); */ void drm_dp_aux_unregister(struct drm_dp_aux *aux) { + drm_dp_aux_unregister_devnode(aux); i2c_del_adapter(&aux->ddc); } EXPORT_SYMBOL(drm_dp_aux_unregister); diff --git a/include/drm/drm_dp_aux_dev.h b/include/drm/drm_dp_aux_dev.h new file mode 100644 index 0000000..77534e8 --- /dev/null +++ b/include/drm/drm_dp_aux_dev.h @@ -0,0 +1,50 @@ +/* + * Copyright © 2015 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + * Rafael Antognolli <rafael.antognolli@intel.com> + * + */ + +#ifndef _UAPI_LINUX_DRM_DP_AUX_DEV_ +#define _UAPI_LINUX_DRM_DP_AUX_DEV_ + +#ifdef CONFIG_DRM_DP_AUX_CHARDEV + +int drm_dp_aux_register_devnode(struct drm_dp_aux *aux); +int drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux); + +#else + +static inline int drm_dp_aux_register_devnode(struct drm_dp_aux *aux) +{ + return 0; +} + +static inline int drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux) +{ + return 0; +} + +#endif + +#endif
This module is heavily based on i2c-dev. Once loaded, it provides one dev node per DP AUX channel, named drm_dp_auxN, where N is an integer. It's possible to know which connector owns this aux channel by looking at the respective sysfs /sys/class/drm_aux_dev/drm_dp_auxN/connector, if the connector device pointer was correctly set in the aux helper struct. Two main operations are provided on the registers read and write. The address of the register to be read or written is given using lseek. The seek position is updated upon read or write. Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com> --- drivers/gpu/drm/Kconfig | 8 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_dp_aux_dev.c | 357 +++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_dp_helper.c | 5 + include/drm/drm_dp_aux_dev.h | 50 ++++++ 5 files changed, 421 insertions(+) create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c create mode 100644 include/drm/drm_dp_aux_dev.h