[PATCH RFC v2 3/3] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
Ville Syrjälä
ville.syrjala at linux.intel.com
Tue Sep 22 11:09:51 PDT 2015
On Tue, Sep 22, 2015 at 10:48:06AM -0700, Rafael Antognolli wrote:
> On Tue, Sep 22, 2015 at 10:59:51AM +0200, Daniel Vetter wrote:
> > On Tue, Sep 15, 2015 at 04:55:04PM -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_aux-N.
> > >
> > > It's possible to know which connector owns this aux channel by looking
> > > at the respective sysfs /sys/class/drm_aux-dev/drm_aux-N/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.
> > > Reading or writing does not update the offset of the file.
> >
> > I think not updating the read position is very surprising. Would it be
> > hard to fix that?
>
> No, not hard at all. But I assume then I should update the write
> position too, right?
>
> BTW, i2c-dev doesn't update either of them, but I'm not sure why.
I think there's just no standard definition of what an offset would mean
for an i2c device. For standard eeproms it would work as one would expect,
but generally it's device specific.
>
> > >
> > > Signed-off-by: Rafael Antognolli <rafael.antognolli at intel.com>
> > > ---
> > > drivers/gpu/drm/Kconfig | 4 +
> > > drivers/gpu/drm/Makefile | 1 +
> > > drivers/gpu/drm/drm_aux-dev.c | 326 ++++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 331 insertions(+)
> > > create mode 100644 drivers/gpu/drm/drm_aux-dev.c
> > >
> > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > > index 1a0a8df..eae847c 100644
> > > --- a/drivers/gpu/drm/Kconfig
> > > +++ b/drivers/gpu/drm/Kconfig
> > > @@ -25,6 +25,10 @@ config DRM_MIPI_DSI
> > > bool
> > > depends on DRM
> > >
> > > +config DRM_AUX_CHARDEV
> > > + tristate "DRM DP AUX Interface"
> > > + depends on DRM
> > > +
> > > config DRM_KMS_HELPER
> > > tristate
> > > depends on DRM
> > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > > index 45e7719..a1a94306 100644
> > > --- a/drivers/gpu/drm/Makefile
> > > +++ b/drivers/gpu/drm/Makefile
> > > @@ -32,6 +32,7 @@ CFLAGS_drm_trace_points.o := -I$(src)
> > >
> > > obj-$(CONFIG_DRM) += drm.o
> > > obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
> > > +obj-$(CONFIG_DRM_AUX_CHARDEV) += drm_aux-dev.o
> > > obj-$(CONFIG_DRM_TTM) += ttm/
> > > obj-$(CONFIG_DRM_TDFX) += tdfx/
> > > obj-$(CONFIG_DRM_R128) += r128/
> > > diff --git a/drivers/gpu/drm/drm_aux-dev.c b/drivers/gpu/drm/drm_aux-dev.c
> > > new file mode 100644
> > > index 0000000..fcc334a
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/drm_aux-dev.c
> > > @@ -0,0 +1,326 @@
> > > +/*
> > > + * 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 at 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_aux_dev {
> > > + struct list_head list;
> > > + unsigned index;
> > > + struct drm_dp_aux *aux;
> > > + struct device *dev;
> > > +};
> > > +
> > > +#define DRM_AUX_MINORS 256
> > > +static int drm_aux_dev_count = 0;
> > > +static LIST_HEAD(drm_aux_dev_list);
> > > +static DEFINE_SPINLOCK(drm_aux_dev_list_lock);
> > > +
> > > +static struct drm_aux_dev *drm_aux_dev_get_by_minor(unsigned index)
> > > +{
> > > + struct drm_aux_dev *aux_dev;
> > > +
> > > + spin_lock(&drm_aux_dev_list_lock);
> > > + list_for_each_entry(aux_dev, &drm_aux_dev_list, list) {
> > > + if (aux_dev->index == index)
> > > + goto found;
> > > + }
> > > +
> > > + aux_dev = NULL;
> > > +found:
> > > + spin_unlock(&drm_aux_dev_list_lock);
> > > + return aux_dev;
> > > +}
> > > +
> > > +static struct drm_aux_dev *drm_aux_dev_get_by_aux(struct drm_dp_aux *aux)
> > > +{
> > > + struct drm_aux_dev *aux_dev;
> > > +
> > > + spin_lock(&drm_aux_dev_list_lock);
> > > + list_for_each_entry(aux_dev, &drm_aux_dev_list, list) {
> > > + if (aux_dev->aux == aux)
> > > + goto found;
> > > + }
> > > +
> > > + aux_dev = NULL;
> > > +found:
> > > + spin_unlock(&drm_aux_dev_list_lock);
> > > + return aux_dev;
> > > +}
> > > +
> > > +static struct drm_aux_dev *get_free_drm_aux_dev(struct drm_dp_aux *aux)
> > > +{
> > > + struct drm_aux_dev *aux_dev;
> > > + int index;
> > > +
> > > + spin_lock(&drm_aux_dev_list_lock);
> > > + index = drm_aux_dev_count;
> > > + spin_unlock(&drm_aux_dev_list_lock);
> > > + if (index >= DRM_AUX_MINORS) {
> > > + printk(KERN_ERR "i2c-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_aux_dev_list_lock);
> > > + drm_aux_dev_count++;
> > > + list_add_tail(&aux_dev->list, &drm_aux_dev_list);
> > > + spin_unlock(&drm_aux_dev_list_lock);
> > > + return aux_dev;
> > > +}
> > > +
> > > +static void return_drm_aux_dev(struct drm_aux_dev *aux_dev)
> > > +{
> > > + spin_lock(&drm_aux_dev_list_lock);
> > > + list_del(&aux_dev->list);
> > > + spin_unlock(&drm_aux_dev_list_lock);
> > > + kfree(aux_dev);
> > > +}
> > > +
> > > +static ssize_t name_show(struct device *dev,
> > > + struct device_attribute *attr, char *buf)
> > > +{
> > > + struct drm_aux_dev *aux_dev = drm_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_aux_dev *aux_dev = drm_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_aux_attrs[] = {
> > > + &dev_attr_name.attr,
> > > + &dev_attr_connector.attr,
> > > + NULL,
> > > +};
> > > +ATTRIBUTE_GROUPS(drm_aux);
> > > +
> > > +static int auxdev_open(struct inode *inode, struct file *file)
> > > +{
> > > + unsigned int minor = iminor(inode);
> > > + struct drm_aux_dev *aux_dev;
> > > +
> > > + aux_dev = drm_aux_dev_get_by_minor(minor);
> > > + if (!aux_dev)
> > > + return -ENODEV;
> > > +
> > > + file->private_data = aux_dev;
> > > + return 0;
> > > +}
> > > +
> > > +static ssize_t auxdev_read(struct file *file, char __user *buf, size_t count,
> > > + loff_t *offset)
> > > +{
> > > + char *localbuf;
> > > + ssize_t res;
> > > + struct drm_aux_dev *aux_dev = file->private_data;
> > > +
> > > + localbuf = memdup_user(buf, count);
> > > + if (IS_ERR(localbuf))
> > > + return PTR_ERR(buf);
> > > +
> > > + res = drm_dp_dpcd_read(aux_dev->aux, *offset, localbuf, count);
> > > + if (res < 0)
> > > + goto finish;
> > > +
> > > + if (copy_to_user(buf, localbuf, res))
> > > + res = -EFAULT;
> > > +
> > > +finish:
> > > + kfree(localbuf);
> > > + return res;
> > > +}
> > > +
> > > +static ssize_t auxdev_write(struct file *file, const char __user *buf,
> > > + size_t count, loff_t *offset)
> > > +{
> > > + char *localbuf;
> > > + ssize_t res;
> > > + struct drm_aux_dev *aux_dev = file->private_data;
> > > +
> > > + localbuf = memdup_user(buf, count);
> > > + if (IS_ERR(localbuf))
> > > + return PTR_ERR(buf);
> > > +
> > > + res = drm_dp_dpcd_write(aux_dev->aux, *offset, localbuf, count);
> > > + kfree(localbuf);
> > > +
> > > + return res;
> > > +}
> > > +
> > > +static const struct file_operations auxdev_fops = {
> > > + .owner = THIS_MODULE,
> > > + .llseek = generic_file_llseek,
> > > + .read = auxdev_read,
> > > + .write = auxdev_write,
> > > + .open = auxdev_open,
> > > +};
> > > +
> > > +static struct class *drm_aux_dev_class;
> > > +static int drm_dev_major = -1;
> > > +
> > > +#define to_auxdev(d) container_of(d, struct drm_aux_dev, aux)
> > > +
> > > +static int auxdev_attach_aux(struct drm_dp_aux *aux, void *data)
> > > +{
> > > + int *major = data;
> > > + struct drm_aux_dev *aux_dev;
> > > + int res;
> > > +
> > > + aux_dev = get_free_drm_aux_dev(aux);
> > > + if (IS_ERR(aux_dev))
> > > + return PTR_ERR(aux_dev);
> > > +
> > > + aux_dev->dev = device_create(drm_aux_dev_class, aux->dev,
> > > + MKDEV(*major, aux_dev->index), NULL,
> > > + "drm_aux-%d", aux_dev->index);
> > > + if (IS_ERR(aux_dev->dev)) {
> > > + res = PTR_ERR(aux_dev->dev);
> > > + goto error;
> > > + }
> > > +
> > > + pr_debug("drm_aux-dev: aux [%s] registered as minor %d\n",
> > > + aux->name, aux_dev->index);
> > > + return 0;
> > > +error:
> > > + return_drm_aux_dev(aux_dev);
> > > + return res;
> > > +
> > > +}
> > > +
> > > +static int auxdev_detach_aux(struct drm_dp_aux *aux, void *data)
> > > +{
> > > + int *major = data;
> > > + int minor;
> > > + struct drm_aux_dev *aux_dev;
> > > +
> > > + aux_dev = drm_aux_dev_get_by_aux(aux);
> > > + if (!aux_dev) /* attach must have failed */
> > > + return 0;
> > > +
> > > + minor = aux_dev->index;
> > > + return_drm_aux_dev(aux_dev);
> > > + device_destroy(drm_aux_dev_class, MKDEV(*major, minor));
> > > +
> > > + pr_debug("drm_aux-dev: aux [%s] unregistered\n", aux->name);
> > > + return 0;
> > > +}
> > > +
> > > +static int auxdev_inform_cb(int action, struct drm_dp_aux *aux)
> > > +{
> > > + switch (action) {
> > > + case DRM_DP_ADD_AUX:
> > > + return auxdev_attach_aux(aux, &drm_dev_major);
> > > + case DRM_DP_DEL_AUX:
> > > + return auxdev_detach_aux(aux, &drm_dev_major);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int __init drm_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_aux_dev_class = class_create(THIS_MODULE, "drm_aux-dev");
> > > + if (IS_ERR(drm_aux_dev_class)) {
> > > + res = PTR_ERR(drm_aux_dev_class);
> > > + goto out_unreg_chrdev;
> > > + }
> > > + drm_aux_dev_class->dev_groups = drm_aux_groups;
> > > +
> > > + /* Keep track of DP aux that will be added or removed later */
> > > + drm_dp_aux_set_aux_dev(&auxdev_inform_cb);
> > > +
> > > + /* Bind to already existing DP aux */
> > > + res = drm_dp_aux_for_each(&drm_dev_major, auxdev_attach_aux);
> > > +
> > > + 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_aux_dev_exit(void)
> > > +{
> > > + printk(KERN_INFO "drm dp aux /dev entries driver - unloading\n");
> > > + drm_dp_aux_set_aux_dev(NULL);
> > > + drm_dp_aux_for_each(&drm_dev_major, auxdev_detach_aux);
> > > + class_destroy(drm_aux_dev_class);
> > > + unregister_chrdev(drm_dev_major, "aux");
> > > +}
> > > +
> > > +MODULE_AUTHOR("Rafael Antognolli <rafael.antognolli at intel.com>");
> > > +MODULE_DESCRIPTION("DRM DP AUX /dev entries driver");
> > > +MODULE_LICENSE("GPL");
> > > +
> > > +module_init(drm_aux_dev_init);
> > > +module_exit(drm_aux_dev_exit);
> > > --
> > > 2.4.3
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
More information about the dri-devel
mailing list