* [PATCH v5 0/2] Add drm_dp_aux chardev support.
@ 2015-10-09 21:48 Rafael Antognolli
2015-10-09 21:48 ` [PATCH v5 1/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers Rafael Antognolli
2015-10-09 21:48 ` [PATCH v5 2/2] drm/dp: Set aux.dev to the drm_connector device, instead of drm_device Rafael Antognolli
0 siblings, 2 replies; 5+ messages in thread
From: Rafael Antognolli @ 2015-10-09 21:48 UTC (permalink / raw)
To: intel-gfx, dri-devel
This series implement support to a drm_dp_aux chardev that allows reading and
writing an arbitrary amount of bytes to arbitrary dpcd register addresses using
regular read, write and lseek operations.
v2:
- lseek is used to select the register to read/write
- read/write are used instead of ioctl
- no blocking_notifier is used, just a direct callback
v3:
- use drm_dp_aux_dev prefix for public functions
- chardev is named drm_dp_auxN
- read/write don't allocate a buffer anymore, and transfer up to 16 bytes a
time
- remove notifier list from the implementation
- option on menuconfig is now a boolean
- add inline stub functions to avoid breakage when this option is disabled
v4:
- fix build system changes - actually disable this module when not selected.
v5:
- Use kref to avoid device closing while still in use
- Don't use list, use an idr for storing aux_dev
- Remove "connector" attribute
- set aux.dev to the connector drm_connector device, instead of
drm_device
Rafael Antognolli (2):
drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
drm/dp: Set aux.dev to the drm_connector device, instead of
drm_device.
drivers/gpu/drm/Kconfig | 8 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/drm_dp_aux_dev.c | 373 +++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/drm_dp_helper.c | 7 +
drivers/gpu/drm/i915/intel_dp.c | 14 +-
include/drm/drm_dp_aux_dev.h | 50 ++++++
6 files changed, 441 insertions(+), 12 deletions(-)
create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c
create mode 100644 include/drm/drm_dp_aux_dev.h
--
2.4.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v5 1/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
2015-10-09 21:48 [PATCH v5 0/2] Add drm_dp_aux chardev support Rafael Antognolli
@ 2015-10-09 21:48 ` Rafael Antognolli
2015-10-10 0:49 ` [Intel-gfx] " kbuild test robot
2015-10-20 16:05 ` Ville Syrjälä
2015-10-09 21:48 ` [PATCH v5 2/2] drm/dp: Set aux.dev to the drm_connector device, instead of drm_device Rafael Antognolli
1 sibling, 2 replies; 5+ messages in thread
From: Rafael Antognolli @ 2015-10-09 21:48 UTC (permalink / raw)
To: intel-gfx, dri-devel
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 | 373 +++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/drm_dp_helper.c | 7 +
include/drm/drm_dp_aux_dev.h | 50 ++++++
5 files changed, 439 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 e814517..e5bc0ca 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -28,6 +28,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..a2861e2
--- /dev/null
+++ b/drivers/gpu/drm/drm_dp_aux_dev.c
@@ -0,0 +1,373 @@
+/*
+ * 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 {
+ unsigned index;
+ struct drm_dp_aux *aux;
+ struct device *dev;
+ struct kref refcount;
+ bool removed;
+ spinlock_t removed_lock;
+};
+
+#define DRM_AUX_MINORS 256
+#define AUX_MAX_OFFSET (1 << 20)
+static DEFINE_IDR(aux_idr);
+static DEFINE_SPINLOCK(aux_idr_lock);
+static struct class *drm_dp_aux_dev_class;
+static int drm_dev_major = -1;
+
+static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_minor(unsigned index)
+{
+ struct drm_dp_aux_dev *aux_dev = NULL;
+
+ spin_lock(&aux_idr_lock);
+ aux_dev = idr_find(&aux_idr, index);
+ if (!kref_get_unless_zero(&aux_dev->refcount))
+ aux_dev = NULL;
+ spin_unlock(&aux_idr_lock);
+
+ return aux_dev;
+}
+
+static struct drm_dp_aux_dev *alloc_drm_dp_aux_dev(struct drm_dp_aux *aux)
+{
+ struct drm_dp_aux_dev *aux_dev;
+ int index;
+
+
+ aux_dev = kzalloc(sizeof(*aux_dev), GFP_KERNEL);
+ if (!aux_dev)
+ return ERR_PTR(-ENOMEM);
+ aux_dev->aux = aux;
+ aux_dev->removed = false;
+ spin_lock_init(&aux_dev->removed_lock);
+ kref_init(&aux_dev->refcount);
+
+ idr_preload(GFP_KERNEL);
+ spin_lock(&aux_idr_lock);
+ index = idr_alloc_cyclic(&aux_idr, aux_dev, 0, DRM_AUX_MINORS,
+ GFP_NOWAIT);
+ spin_unlock(&aux_idr_lock);
+ idr_preload_end();
+ if (index < 0) {
+ kfree(aux_dev);
+ return ERR_PTR(-ENOMEM);
+ }
+ aux_dev->index = index;
+
+ return aux_dev;
+}
+
+static void free_drm_dp_aux_dev(struct drm_dp_aux_dev *aux_dev)
+{
+ spin_lock(&aux_idr_lock);
+ idr_remove(&aux_idr, aux_dev->index);
+ spin_unlock(&aux_idr_lock);
+ kfree(aux_dev);
+}
+
+static void release_drm_dp_aux_dev(struct kref *ref)
+{
+ int minor;
+ struct drm_dp_aux_dev *aux_dev =
+ container_of(ref, struct drm_dp_aux_dev, refcount);
+ minor = aux_dev->index;
+ device_destroy(drm_dp_aux_dev_class, MKDEV(drm_dev_major, minor));
+
+ free_drm_dp_aux_dev(aux_dev);
+}
+
+static ssize_t name_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ ssize_t res;
+ struct drm_dp_aux_dev *aux_dev =
+ drm_dp_aux_dev_get_by_minor(MINOR(dev->devt));
+
+ if (!aux_dev)
+ return -ENODEV;
+
+ res = sprintf(buf, "%s\n", aux_dev->aux->name);
+ kref_put(&aux_dev->refcount, release_drm_dp_aux_dev);
+
+ return res;
+}
+static DEVICE_ATTR_RO(name);
+
+static struct attribute *drm_dp_aux_attrs[] = {
+ &dev_attr_name.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)
+{
+ size_t bytes_pending, num_bytes_processed = 0;
+ struct drm_dp_aux_dev *aux_dev = file->private_data;
+ bool aux_removed;
+
+ if (count < 0)
+ return -EINVAL;
+
+ spin_lock(&aux_dev->removed_lock);
+ aux_removed = aux_dev->removed;
+ spin_unlock(&aux_dev->removed_lock);
+ if (aux_removed)
+ return -ENODEV;
+
+ bytes_pending = min((loff_t)count, AUX_MAX_OFFSET - (*offset));
+
+ if (!access_ok(VERIFY_WRITE, buf, bytes_pending))
+ return -EFAULT;
+
+ while (bytes_pending > 0) {
+ uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES];
+ ssize_t res;
+ ssize_t 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;
+ 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)
+{
+ size_t bytes_pending, num_bytes_processed = 0;
+ struct drm_dp_aux_dev *aux_dev = file->private_data;
+ bool aux_removed;
+
+ if (count < 0)
+ return -EINVAL;
+
+ spin_lock(&aux_dev->removed_lock);
+ aux_removed = aux_dev->removed;
+ spin_unlock(&aux_dev->removed_lock);
+ if (aux_removed)
+ return -ENODEV;
+
+ bytes_pending = min((loff_t)count, AUX_MAX_OFFSET - *offset);
+
+ if (!access_ok(VERIFY_READ, buf, bytes_pending))
+ return -EFAULT;
+
+ while (bytes_pending > 0) {
+ uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES];
+ ssize_t res;
+ ssize_t todo = min(bytes_pending, sizeof(localbuf));
+
+ 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 int auxdev_release(struct inode *inode, struct file *file)
+{
+ struct drm_dp_aux_dev *aux_dev = file->private_data;
+ kref_put(&aux_dev->refcount, release_drm_dp_aux_dev);
+ return 0;
+}
+
+static const struct file_operations auxdev_fops = {
+ .owner = THIS_MODULE,
+ .llseek = auxdev_llseek,
+ .read = auxdev_read,
+ .write = auxdev_write,
+ .open = auxdev_open,
+ .release = auxdev_release,
+};
+
+#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 = alloc_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:
+ free_drm_dp_aux_dev(aux_dev);
+ return res;
+}
+EXPORT_SYMBOL(drm_dp_aux_register_devnode);
+
+static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_aux(struct drm_dp_aux *aux)
+{
+ struct drm_dp_aux_dev *iter, *aux_dev = NULL;
+ int id;
+
+ /* don't increase kref count here because this function should only be
+ * used by drm_dp_aux_unregister_devnode. Thus, it will always have at
+ * least one reference - the one that drm_dp_aux_register_devnode
+ * created */
+ spin_lock(&aux_idr_lock);
+ idr_for_each_entry(&aux_idr, iter, id) {
+ if (iter->aux == aux) {
+ aux_dev = iter;
+ break;
+ }
+ }
+ spin_unlock(&aux_idr_lock);
+ return aux_dev;
+}
+
+/**
+ * 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)
+{
+ 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;
+
+ spin_lock(&aux_dev->removed_lock);
+ aux_dev->removed = true;
+ spin_unlock(&aux_dev->removed_lock);
+
+ kref_put(&aux_dev->refcount, release_drm_dp_aux_dev);
+ pr_debug("drm_dp_aux_dev: aux [%s] unregistered\n", aux->name);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_aux_unregister_devnode);
+
+static int __init drm_dp_aux_dev_init(void)
+{
+ int res;
+
+ 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;
+
+ idr_init(&aux_idr);
+
+ 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)
+{
+ 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 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..e50f65d 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,11 @@ 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));
+
+ if (drm_dp_aux_register_devnode(aux))
+ printk(KERN_ERR "%s: Could not register drm_dp_aux_dev.\n",
+ __FILE__);
+
return i2c_add_adapter(&aux->ddc);
}
EXPORT_SYMBOL(drm_dp_aux_register);
@@ -778,6 +784,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..35409a2
--- /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 DRM_DP_AUX_DEV
+#define 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
--
2.4.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v5 2/2] drm/dp: Set aux.dev to the drm_connector device, instead of drm_device.
2015-10-09 21:48 [PATCH v5 0/2] Add drm_dp_aux chardev support Rafael Antognolli
2015-10-09 21:48 ` [PATCH v5 1/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers Rafael Antognolli
@ 2015-10-09 21:48 ` Rafael Antognolli
1 sibling, 0 replies; 5+ messages in thread
From: Rafael Antognolli @ 2015-10-09 21:48 UTC (permalink / raw)
To: intel-gfx, dri-devel
So far, the i915 driver and some other drivers set it to the drm_device,
which doesn't allow one to know which DP a given aux channel is related
to. Changing this to be the drm_connector provides proper nesting, still
allowing one to get the drm_device from it. Some drivers already set it
to the drm_connector.
This also removes the need to add a sysfs link for the i2c device under
the connector, as it will already be there.
Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 19 ++-----------------
1 file changed, 2 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 18bcfbe..0acdf0f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1078,36 +1078,21 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
intel_dp->aux.name = name;
- intel_dp->aux.dev = dev->dev;
+ intel_dp->aux.dev = connector->base.kdev;
intel_dp->aux.transfer = intel_dp_aux_transfer;
DRM_DEBUG_KMS("registering %s bus for %s\n", name,
connector->base.kdev->kobj.name);
ret = drm_dp_aux_register(&intel_dp->aux);
- if (ret < 0) {
+ if (ret < 0)
DRM_ERROR("drm_dp_aux_register() for %s failed (%d)\n",
name, ret);
- return;
- }
-
- ret = sysfs_create_link(&connector->base.kdev->kobj,
- &intel_dp->aux.ddc.dev.kobj,
- intel_dp->aux.ddc.dev.kobj.name);
- if (ret < 0) {
- DRM_ERROR("sysfs_create_link() for %s failed (%d)\n", name, ret);
- drm_dp_aux_unregister(&intel_dp->aux);
- }
}
static void
intel_dp_connector_unregister(struct intel_connector *intel_connector)
{
- struct intel_dp *intel_dp = intel_attached_dp(&intel_connector->base);
-
- if (!intel_connector->mst_port)
- sysfs_remove_link(&intel_connector->base.kdev->kobj,
- intel_dp->aux.ddc.dev.kobj.name);
intel_connector_unregister(intel_connector);
}
--
2.4.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
2015-10-09 21:48 ` [PATCH v5 1/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers Rafael Antognolli
@ 2015-10-10 0:49 ` kbuild test robot
2015-10-20 16:05 ` Ville Syrjälä
1 sibling, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2015-10-10 0:49 UTC (permalink / raw)
To: Rafael Antognolli; +Cc: intel-gfx, kbuild-all, dri-devel
[-- Attachment #1: Type: text/plain, Size: 6475 bytes --]
Hi Rafael,
[auto build test WARNING on drm/drm-next -- if it's inappropriate base, please ignore]
config: mn10300-allyesconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mn10300
All warnings (new ones prefixed by >>):
In file included from include/linux/list.h:8:0,
from include/linux/kobject.h:20,
from include/linux/device.h:17,
from drivers/gpu/drm/drm_dp_aux_dev.c:28:
drivers/gpu/drm/drm_dp_aux_dev.c: In function 'auxdev_read':
include/linux/kernel.h:722:17: warning: comparison of distinct pointer types lacks a cast
(void) (&_min1 == &_min2); \
^
>> drivers/gpu/drm/drm_dp_aux_dev.c:180:18: note: in expansion of macro 'min'
ssize_t todo = min(bytes_pending, sizeof(localbuf));
^
drivers/gpu/drm/drm_dp_aux_dev.c: In function 'auxdev_write':
include/linux/kernel.h:722:17: warning: comparison of distinct pointer types lacks a cast
(void) (&_min1 == &_min2); \
^
drivers/gpu/drm/drm_dp_aux_dev.c:220:18: note: in expansion of macro 'min'
ssize_t todo = min(bytes_pending, sizeof(localbuf));
^
vim +/min +180 drivers/gpu/drm/drm_dp_aux_dev.c
22 *
23 * Authors:
24 * Rafael Antognolli <rafael.antognolli@intel.com>
25 *
26 */
27
> 28 #include <linux/device.h>
29 #include <linux/fs.h>
30 #include <linux/slab.h>
31 #include <linux/init.h>
32 #include <linux/kernel.h>
33 #include <linux/module.h>
34 #include <asm/uaccess.h>
35 #include <drm/drm_dp_helper.h>
36 #include <drm/drm_crtc.h>
37
38 struct drm_dp_aux_dev {
39 unsigned index;
40 struct drm_dp_aux *aux;
41 struct device *dev;
42 struct kref refcount;
43 bool removed;
44 spinlock_t removed_lock;
45 };
46
47 #define DRM_AUX_MINORS 256
48 #define AUX_MAX_OFFSET (1 << 20)
49 static DEFINE_IDR(aux_idr);
50 static DEFINE_SPINLOCK(aux_idr_lock);
51 static struct class *drm_dp_aux_dev_class;
52 static int drm_dev_major = -1;
53
54 static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_minor(unsigned index)
55 {
56 struct drm_dp_aux_dev *aux_dev = NULL;
57
58 spin_lock(&aux_idr_lock);
59 aux_dev = idr_find(&aux_idr, index);
60 if (!kref_get_unless_zero(&aux_dev->refcount))
61 aux_dev = NULL;
62 spin_unlock(&aux_idr_lock);
63
64 return aux_dev;
65 }
66
67 static struct drm_dp_aux_dev *alloc_drm_dp_aux_dev(struct drm_dp_aux *aux)
68 {
69 struct drm_dp_aux_dev *aux_dev;
70 int index;
71
72
73 aux_dev = kzalloc(sizeof(*aux_dev), GFP_KERNEL);
74 if (!aux_dev)
75 return ERR_PTR(-ENOMEM);
76 aux_dev->aux = aux;
77 aux_dev->removed = false;
78 spin_lock_init(&aux_dev->removed_lock);
79 kref_init(&aux_dev->refcount);
80
81 idr_preload(GFP_KERNEL);
82 spin_lock(&aux_idr_lock);
83 index = idr_alloc_cyclic(&aux_idr, aux_dev, 0, DRM_AUX_MINORS,
84 GFP_NOWAIT);
85 spin_unlock(&aux_idr_lock);
86 idr_preload_end();
87 if (index < 0) {
88 kfree(aux_dev);
89 return ERR_PTR(-ENOMEM);
90 }
91 aux_dev->index = index;
92
93 return aux_dev;
94 }
95
96 static void free_drm_dp_aux_dev(struct drm_dp_aux_dev *aux_dev)
97 {
98 spin_lock(&aux_idr_lock);
99 idr_remove(&aux_idr, aux_dev->index);
100 spin_unlock(&aux_idr_lock);
101 kfree(aux_dev);
102 }
103
104 static void release_drm_dp_aux_dev(struct kref *ref)
105 {
106 int minor;
107 struct drm_dp_aux_dev *aux_dev =
108 container_of(ref, struct drm_dp_aux_dev, refcount);
109 minor = aux_dev->index;
110 device_destroy(drm_dp_aux_dev_class, MKDEV(drm_dev_major, minor));
111
112 free_drm_dp_aux_dev(aux_dev);
113 }
114
115 static ssize_t name_show(struct device *dev,
116 struct device_attribute *attr, char *buf)
117 {
118 ssize_t res;
119 struct drm_dp_aux_dev *aux_dev =
120 drm_dp_aux_dev_get_by_minor(MINOR(dev->devt));
121
122 if (!aux_dev)
123 return -ENODEV;
124
125 res = sprintf(buf, "%s\n", aux_dev->aux->name);
126 kref_put(&aux_dev->refcount, release_drm_dp_aux_dev);
127
128 return res;
129 }
130 static DEVICE_ATTR_RO(name);
131
132 static struct attribute *drm_dp_aux_attrs[] = {
133 &dev_attr_name.attr,
134 NULL,
135 };
136 ATTRIBUTE_GROUPS(drm_dp_aux);
137
138 static int auxdev_open(struct inode *inode, struct file *file)
139 {
140 unsigned int minor = iminor(inode);
141 struct drm_dp_aux_dev *aux_dev;
142
143 aux_dev = drm_dp_aux_dev_get_by_minor(minor);
144 if (!aux_dev)
145 return -ENODEV;
146
147 file->private_data = aux_dev;
148 return 0;
149 }
150
151 static loff_t auxdev_llseek(struct file *file, loff_t offset, int whence)
152 {
153 return fixed_size_llseek(file, offset, whence, AUX_MAX_OFFSET);
154 }
155
156 static ssize_t auxdev_read(struct file *file, char __user *buf, size_t count,
157 loff_t *offset)
158 {
159 size_t bytes_pending, num_bytes_processed = 0;
160 struct drm_dp_aux_dev *aux_dev = file->private_data;
161 bool aux_removed;
162
163 if (count < 0)
164 return -EINVAL;
165
166 spin_lock(&aux_dev->removed_lock);
167 aux_removed = aux_dev->removed;
168 spin_unlock(&aux_dev->removed_lock);
169 if (aux_removed)
170 return -ENODEV;
171
172 bytes_pending = min((loff_t)count, AUX_MAX_OFFSET - (*offset));
173
174 if (!access_ok(VERIFY_WRITE, buf, bytes_pending))
175 return -EFAULT;
176
177 while (bytes_pending > 0) {
178 uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES];
179 ssize_t res;
> 180 ssize_t todo = min(bytes_pending, sizeof(localbuf));
181
182 res = drm_dp_dpcd_read(aux_dev->aux, *offset, localbuf, todo);
183 if (res <= 0)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 36227 bytes --]
[-- Attachment #3: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v5 1/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
2015-10-09 21:48 ` [PATCH v5 1/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers Rafael Antognolli
2015-10-10 0:49 ` [Intel-gfx] " kbuild test robot
@ 2015-10-20 16:05 ` Ville Syrjälä
1 sibling, 0 replies; 5+ messages in thread
From: Ville Syrjälä @ 2015-10-20 16:05 UTC (permalink / raw)
To: Rafael Antognolli; +Cc: intel-gfx, dri-devel
On Fri, Oct 09, 2015 at 02:48:33PM -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 | 373 +++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_dp_helper.c | 7 +
> include/drm/drm_dp_aux_dev.h | 50 ++++++
> 5 files changed, 439 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 e814517..e5bc0ca 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -28,6 +28,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..a2861e2
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> @@ -0,0 +1,373 @@
> +/*
> + * 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 {
> + unsigned index;
> + struct drm_dp_aux *aux;
> + struct device *dev;
> + struct kref refcount;
> + bool removed;
> + spinlock_t removed_lock;
> +};
> +
> +#define DRM_AUX_MINORS 256
> +#define AUX_MAX_OFFSET (1 << 20)
> +static DEFINE_IDR(aux_idr);
> +static DEFINE_SPINLOCK(aux_idr_lock);
> +static struct class *drm_dp_aux_dev_class;
> +static int drm_dev_major = -1;
> +
> +static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_minor(unsigned index)
> +{
> + struct drm_dp_aux_dev *aux_dev = NULL;
> +
> + spin_lock(&aux_idr_lock);
> + aux_dev = idr_find(&aux_idr, index);
> + if (!kref_get_unless_zero(&aux_dev->refcount))
> + aux_dev = NULL;
> + spin_unlock(&aux_idr_lock);
> +
> + return aux_dev;
> +}
> +
> +static struct drm_dp_aux_dev *alloc_drm_dp_aux_dev(struct drm_dp_aux *aux)
> +{
> + struct drm_dp_aux_dev *aux_dev;
> + int index;
> +
> +
> + aux_dev = kzalloc(sizeof(*aux_dev), GFP_KERNEL);
> + if (!aux_dev)
> + return ERR_PTR(-ENOMEM);
> + aux_dev->aux = aux;
> + aux_dev->removed = false;
> + spin_lock_init(&aux_dev->removed_lock);
> + kref_init(&aux_dev->refcount);
> +
> + idr_preload(GFP_KERNEL);
> + spin_lock(&aux_idr_lock);
> + index = idr_alloc_cyclic(&aux_idr, aux_dev, 0, DRM_AUX_MINORS,
> + GFP_NOWAIT);
> + spin_unlock(&aux_idr_lock);
> + idr_preload_end();
With a mutex things would be simpler.
> + if (index < 0) {
> + kfree(aux_dev);
> + return ERR_PTR(-ENOMEM);
Can just pass the error value from idr_alloc_cyclic() upwards I think.
> + }
> + aux_dev->index = index;
> +
> + return aux_dev;
> +}
> +
> +static void free_drm_dp_aux_dev(struct drm_dp_aux_dev *aux_dev)
> +{
> + spin_lock(&aux_idr_lock);
> + idr_remove(&aux_idr, aux_dev->index);
> + spin_unlock(&aux_idr_lock);
I think the idr_remove should be done when unregistering the aux_dev,
that way no one can open it anymore after it's gone.
> + kfree(aux_dev);
> +}
> +
> +static void release_drm_dp_aux_dev(struct kref *ref)
> +{
> + int minor;
> + struct drm_dp_aux_dev *aux_dev =
> + container_of(ref, struct drm_dp_aux_dev, refcount);
> + minor = aux_dev->index;
> + device_destroy(drm_dp_aux_dev_class, MKDEV(drm_dev_major, minor));
device_destroy() should also be done when unregisteing I think.
> +
> + free_drm_dp_aux_dev(aux_dev);
> +}
> +
> +static ssize_t name_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + ssize_t res;
> + struct drm_dp_aux_dev *aux_dev =
> + drm_dp_aux_dev_get_by_minor(MINOR(dev->devt));
> +
> + if (!aux_dev)
> + return -ENODEV;
> +
> + res = sprintf(buf, "%s\n", aux_dev->aux->name);
> + kref_put(&aux_dev->refcount, release_drm_dp_aux_dev);
> +
> + return res;
> +}
> +static DEVICE_ATTR_RO(name);
> +
> +static struct attribute *drm_dp_aux_attrs[] = {
> + &dev_attr_name.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)
> +{
> + size_t bytes_pending, num_bytes_processed = 0;
> + struct drm_dp_aux_dev *aux_dev = file->private_data;
> + bool aux_removed;
> +
> + if (count < 0)
> + return -EINVAL;
> +
> + spin_lock(&aux_dev->removed_lock);
> + aux_removed = aux_dev->removed;
> + spin_unlock(&aux_dev->removed_lock);
> + if (aux_removed)
> + return -ENODEV;
Hmm. This seems racy still. aux_dev will now stay around until the file
is closed, but the actual aux helper backing it may still disappear at
any time. I don't think we want to hold a lock around the whole thing,
cause I have a feeling that way lies lockdep issues. So maybe we need
some kinda atomic use counter here, or something?
read()
{
if (atomic_inc_unless_zero(usecount))
return -...;
...
atomic_dec();
wake_up();
}
unregister()
{
idr_remove();
atomic_dec(usecount);
wait_event(usecount == 0);
kref_put();
}
register()
{
...
atomic_set(usecount, 1);
idr_alloc_cyclic();
}
I guess another option would to just wait for all fds to be closed in a
similar fashion. But that seems like a more open ended wait.
> +
> + bytes_pending = min((loff_t)count, AUX_MAX_OFFSET - (*offset));
> +
> + if (!access_ok(VERIFY_WRITE, buf, bytes_pending))
> + return -EFAULT;
> +
> + while (bytes_pending > 0) {
> + uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES];
> + ssize_t res;
> + ssize_t 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;
> + 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)
> +{
> + size_t bytes_pending, num_bytes_processed = 0;
> + struct drm_dp_aux_dev *aux_dev = file->private_data;
> + bool aux_removed;
> +
> + if (count < 0)
> + return -EINVAL;
> +
> + spin_lock(&aux_dev->removed_lock);
> + aux_removed = aux_dev->removed;
> + spin_unlock(&aux_dev->removed_lock);
> + if (aux_removed)
> + return -ENODEV;
> +
> + bytes_pending = min((loff_t)count, AUX_MAX_OFFSET - *offset);
> +
> + if (!access_ok(VERIFY_READ, buf, bytes_pending))
> + return -EFAULT;
> +
> + while (bytes_pending > 0) {
> + uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES];
> + ssize_t res;
> + ssize_t todo = min(bytes_pending, sizeof(localbuf));
> +
> + 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 int auxdev_release(struct inode *inode, struct file *file)
> +{
> + struct drm_dp_aux_dev *aux_dev = file->private_data;
> + kref_put(&aux_dev->refcount, release_drm_dp_aux_dev);
> + return 0;
> +}
> +
> +static const struct file_operations auxdev_fops = {
> + .owner = THIS_MODULE,
> + .llseek = auxdev_llseek,
> + .read = auxdev_read,
> + .write = auxdev_write,
> + .open = auxdev_open,
> + .release = auxdev_release,
> +};
> +
> +#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 = alloc_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:
> + free_drm_dp_aux_dev(aux_dev);
> + return res;
> +}
> +EXPORT_SYMBOL(drm_dp_aux_register_devnode);
> +
> +static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_aux(struct drm_dp_aux *aux)
> +{
> + struct drm_dp_aux_dev *iter, *aux_dev = NULL;
> + int id;
> +
> + /* don't increase kref count here because this function should only be
> + * used by drm_dp_aux_unregister_devnode. Thus, it will always have at
> + * least one reference - the one that drm_dp_aux_register_devnode
> + * created */
> + spin_lock(&aux_idr_lock);
> + idr_for_each_entry(&aux_idr, iter, id) {
> + if (iter->aux == aux) {
> + aux_dev = iter;
> + break;
> + }
> + }
> + spin_unlock(&aux_idr_lock);
> + return aux_dev;
> +}
> +
> +/**
> + * 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)
> +{
> + 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;
> +
> + spin_lock(&aux_dev->removed_lock);
> + aux_dev->removed = true;
> + spin_unlock(&aux_dev->removed_lock);
> +
> + kref_put(&aux_dev->refcount, release_drm_dp_aux_dev);
> + pr_debug("drm_dp_aux_dev: aux [%s] unregistered\n", aux->name);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_aux_unregister_devnode);
> +
> +static int __init drm_dp_aux_dev_init(void)
> +{
> + int res;
> +
> + res = register_chrdev(0, "aux", &auxdev_fops);
As soon as this returns someone can open the device, so seems like this
should be the last thing we do.
> + 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;
> +
> + idr_init(&aux_idr);
AFAIK no need to init it since you used DEFINE_IDR()
> +
> + 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)
> +{
> + 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 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..e50f65d 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,11 @@ 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));
>
> +
Looks like an extra newline we don't need.
> + if (drm_dp_aux_register_devnode(aux))
> + printk(KERN_ERR "%s: Could not register drm_dp_aux_dev.\n",
> + __FILE__);
> +
> return i2c_add_adapter(&aux->ddc);
> }
> EXPORT_SYMBOL(drm_dp_aux_register);
> @@ -778,6 +784,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..35409a2
> --- /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 DRM_DP_AUX_DEV
> +#define 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
> --
> 2.4.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-10-20 16:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-09 21:48 [PATCH v5 0/2] Add drm_dp_aux chardev support Rafael Antognolli
2015-10-09 21:48 ` [PATCH v5 1/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers Rafael Antognolli
2015-10-10 0:49 ` [Intel-gfx] " kbuild test robot
2015-10-20 16:05 ` Ville Syrjälä
2015-10-09 21:48 ` [PATCH v5 2/2] drm/dp: Set aux.dev to the drm_connector device, instead of drm_device Rafael Antognolli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox