[PATCH v10 0/4] Add drm_dp_aux chardev support.

[PATCH v10 0/4] Add drm_dp_aux chardev support.

[Rafael Antognolli]

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.

Lukas Wunner (1):
  drm/radeon: Fix WARN_ON if DRM_DP_AUX_CHARDEV is enabled

Rafael Antognolli (3):
  drm/kms_helper: Add a common place to call init and exit functions.
  drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
  drm/i915: Set aux.dev to the drm_connector device, instead of
    drm_device.

 drivers/gpu/drm/Kconfig                 |   8 +
 drivers/gpu/drm/Makefile                |   5 +-
 drivers/gpu/drm/drm_crtc_helper.c       |   3 -
 drivers/gpu/drm/drm_dp_aux_dev.c        | 368 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_dp_helper.c         |  16 +-
 drivers/gpu/drm/drm_fb_helper.c         |   9 +-
 drivers/gpu/drm/drm_kms_helper_common.c |  60 ++++++
 drivers/gpu/drm/i915/intel_dp.c         |  18 +-
 drivers/gpu/drm/radeon/radeon_display.c |   5 +-
 include/drm/drm_dp_aux_dev.h            |  62 ++++++
 include/drm/drm_fb_helper.h             |   6 +
 11 files changed, 532 insertions(+), 28 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c
 create mode 100644 drivers/gpu/drm/drm_kms_helper_common.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

[PATCH v10 1/4] drm/kms_helper: Add a common place to call init and exit functions.

[Rafael Antognolli]

The module_init and module_exit functions will start here, and call the
subsequent init's and exit's.

v10:
 - Keep __init on drm_fb_helper init function.
 - Move MODULE_* macros to the common file.

Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
---
 drivers/gpu/drm/Makefile                |  4 ++-
 drivers/gpu/drm/drm_crtc_helper.c       |  3 ---
 drivers/gpu/drm/drm_fb_helper.c         |  9 +++----
 drivers/gpu/drm/drm_kms_helper_common.c | 47 +++++++++++++++++++++++++++++++++
 include/drm/drm_fb_helper.h             |  6 +++++
 5 files changed, 60 insertions(+), 9 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_kms_helper_common.c

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index f858aa2..dfe513f 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -24,7 +24,9 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o
 drm-y += $(drm-m)
 
 drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
-		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o
+		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
+		drm_kms_helper_common.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
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 5d4bc64..baac181 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -73,9 +73,6 @@
  * &drm_crtc_helper_funcs, struct &drm_encoder_helper_funcs and struct
  * &drm_connector_helper_funcs.
  */
-MODULE_AUTHOR("David Airlie, Jesse Barnes");
-MODULE_DESCRIPTION("DRM KMS helper");
-MODULE_LICENSE("GPL and additional rights");
 
 /**
  * drm_helper_move_panel_connectors_to_head() - move panels to the front in the
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 1e103c4..c27b964 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2175,9 +2175,9 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
  * but the module doesn't depend on any fb console symbols.  At least
  * attempt to load fbcon to avoid leaving the system without a usable console.
  */
-#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
-static int __init drm_fb_helper_modinit(void)
+int __init drm_fb_helper_modinit(void)
 {
+#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
 	const char *name = "fbcon";
 	struct module *fbcon;
 
@@ -2187,8 +2187,7 @@ static int __init drm_fb_helper_modinit(void)
 
 	if (!fbcon)
 		request_module_nowait(name);
+#endif
 	return 0;
 }
-
-module_init(drm_fb_helper_modinit);
-#endif
+EXPORT_SYMBOL(drm_fb_helper_modinit);
diff --git a/drivers/gpu/drm/drm_kms_helper_common.c b/drivers/gpu/drm/drm_kms_helper_common.c
new file mode 100644
index 0000000..d361005
--- /dev/null
+++ b/drivers/gpu/drm/drm_kms_helper_common.c
@@ -0,0 +1,47 @@
+/*
+ * 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 <drm/drmP.h>
+#include <drm/drm_fb_helper.h>
+
+MODULE_AUTHOR("David Airlie, Jesse Barnes");
+MODULE_DESCRIPTION("DRM KMS helper");
+MODULE_LICENSE("GPL and additional rights");
+
+static int __init drm_kms_helper_init(void)
+{
+	/* Call init functions from specific kms helpers here */
+	return drm_fb_helper_modinit();
+}
+
+static void __exit drm_kms_helper_exit(void)
+{
+	/* Call exit functions from specific kms helpers here */
+}
+
+module_init(drm_kms_helper_init);
+module_exit(drm_kms_helper_exit);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index d8a40df..062723b 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -219,6 +219,7 @@ struct drm_fb_helper {
 };
 
 #ifdef CONFIG_DRM_FBDEV_EMULATION
+int drm_fb_helper_modinit(void);
 void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
 			   const struct drm_fb_helper_funcs *funcs);
 int drm_fb_helper_init(struct drm_device *dev,
@@ -283,6 +284,11 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_
 int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 				       struct drm_connector *connector);
 #else
+static inline int drm_fb_helper_modinit(void)
+{
+	return 0;
+}
+
 static inline void drm_fb_helper_prepare(struct drm_device *dev,
 					struct drm_fb_helper *helper,
 					const struct drm_fb_helper_funcs *funcs)
-- 
2.4.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v10 2/4] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.

[Rafael Antognolli]

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.

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

v6:
 - Use atomic_t for usage count
 - Use a mutex instead of spinlock for idr lock
 - Destroy chardev immediately on unregister
 - other minor suggestions from Ville

v7:
 - style fixes
 - error handling fixes

v8:
 - more error handling fixes

v9:
 - remove module_init and module_exit, and add drm_dp_aux_dev_init/exit
 to drm_kms_helper_init/exit.

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        | 368 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_dp_helper.c         |  16 +-
 drivers/gpu/drm/drm_kms_helper_common.c |  15 +-
 include/drm/drm_dp_aux_dev.h            |  62 ++++++
 6 files changed, 468 insertions(+), 2 deletions(-)
 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 59babd5..dff87ca 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 dfe513f..424fcb7 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -30,6 +30,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..f73b38b
--- /dev/null
+++ b/drivers/gpu/drm/drm_dp_aux_dev.c
@@ -0,0 +1,368 @@
+/*
+ * 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 <linux/uaccess.h>
+#include <drm/drm_dp_helper.h>
+#include <drm/drm_crtc.h>
+#include <drm/drmP.h>
+
+struct drm_dp_aux_dev {
+	unsigned index;
+	struct drm_dp_aux *aux;
+	struct device *dev;
+	struct kref refcount;
+	atomic_t usecount;
+};
+
+#define DRM_AUX_MINORS	256
+#define AUX_MAX_OFFSET	(1 << 20)
+static DEFINE_IDR(aux_idr);
+static DEFINE_MUTEX(aux_idr_mutex);
+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;
+
+	mutex_lock(&aux_idr_mutex);
+	aux_dev = idr_find(&aux_idr, index);
+	if (!kref_get_unless_zero(&aux_dev->refcount))
+		aux_dev = NULL;
+	mutex_unlock(&aux_idr_mutex);
+
+	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;
+	atomic_set(&aux_dev->usecount, 1);
+	kref_init(&aux_dev->refcount);
+
+	mutex_lock(&aux_idr_mutex);
+	index = idr_alloc_cyclic(&aux_idr, aux_dev, 0, DRM_AUX_MINORS,
+				 GFP_KERNEL);
+	mutex_unlock(&aux_idr_mutex);
+	if (index < 0) {
+		kfree(aux_dev);
+		return ERR_PTR(index);
+	}
+	aux_dev->index = index;
+
+	return aux_dev;
+}
+
+static void release_drm_dp_aux_dev(struct kref *ref)
+{
+	struct drm_dp_aux_dev *aux_dev =
+		container_of(ref, struct drm_dp_aux_dev, refcount);
+
+	kfree(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;
+	ssize_t res = 0;
+
+	if (!atomic_inc_not_zero(&aux_dev->usecount))
+		return -ENODEV;
+
+	bytes_pending = min((loff_t)count, AUX_MAX_OFFSET - (*offset));
+
+	if (!access_ok(VERIFY_WRITE, buf, bytes_pending)) {
+		res = -EFAULT;
+		goto out;
+	}
+
+	while (bytes_pending > 0) {
+		uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES];
+		ssize_t todo = min_t(size_t, bytes_pending, sizeof(localbuf));
+
+		res = drm_dp_dpcd_read(aux_dev->aux, *offset, localbuf, todo);
+		if (res <= 0) {
+			res = num_bytes_processed ? num_bytes_processed : res;
+			goto out;
+		}
+		if (__copy_to_user(buf + num_bytes_processed, localbuf, res)) {
+			res = num_bytes_processed ?
+				num_bytes_processed : -EFAULT;
+			goto out;
+		}
+		bytes_pending -= res;
+		*offset += res;
+		num_bytes_processed += res;
+		res = num_bytes_processed;
+	}
+
+out:
+	atomic_dec(&aux_dev->usecount);
+	wake_up_atomic_t(&aux_dev->usecount);
+	return res;
+}
+
+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;
+	ssize_t res = 0;
+
+	if (!atomic_inc_not_zero(&aux_dev->usecount))
+		return -ENODEV;
+
+	bytes_pending = min((loff_t)count, AUX_MAX_OFFSET - *offset);
+
+	if (!access_ok(VERIFY_READ, buf, bytes_pending)) {
+		res = -EFAULT;
+		goto out;
+	}
+
+	while (bytes_pending > 0) {
+		uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES];
+		ssize_t todo = min_t(size_t, bytes_pending, sizeof(localbuf));
+
+		if (__copy_from_user(localbuf,
+				     buf + num_bytes_processed, todo)) {
+			res = num_bytes_processed ?
+				num_bytes_processed : -EFAULT;
+			goto out;
+		}
+
+		res = drm_dp_dpcd_write(aux_dev->aux, *offset, localbuf, todo);
+		if (res <= 0) {
+			res = num_bytes_processed ? num_bytes_processed : res;
+			goto out;
+		}
+		bytes_pending -= res;
+		*offset += res;
+		num_bytes_processed += res;
+		res = num_bytes_processed;
+	}
+
+out:
+	atomic_dec(&aux_dev->usecount);
+	wake_up_atomic_t(&aux_dev->usecount);
+	return res;
+}
+
+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)
+
+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
+	 */
+	mutex_lock(&aux_idr_mutex);
+	idr_for_each_entry(&aux_idr, iter, id) {
+		if (iter->aux == aux) {
+			aux_dev = iter;
+			break;
+		}
+	}
+	mutex_unlock(&aux_idr_mutex);
+	return aux_dev;
+}
+
+static int auxdev_wait_atomic_t(atomic_t *p)
+{
+	schedule();
+	return 0;
+}
+/**
+ * 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.
+ */
+void drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux)
+{
+	struct drm_dp_aux_dev *aux_dev;
+	unsigned int minor;
+
+	aux_dev = drm_dp_aux_dev_get_by_aux(aux);
+	if (!aux_dev) /* attach must have failed */
+		return;
+
+	mutex_lock(&aux_idr_mutex);
+	idr_remove(&aux_idr, aux_dev->index);
+	mutex_unlock(&aux_idr_mutex);
+
+	atomic_dec(&aux_dev->usecount);
+	wait_on_atomic_t(&aux_dev->usecount, auxdev_wait_atomic_t,
+			 TASK_UNINTERRUPTIBLE);
+
+	minor = aux_dev->index;
+	if (aux_dev->dev)
+		device_destroy(drm_dp_aux_dev_class,
+			       MKDEV(drm_dev_major, minor));
+
+	DRM_DEBUG("drm_dp_aux_dev: aux [%s] unregistering\n", aux->name);
+	kref_put(&aux_dev->refcount, release_drm_dp_aux_dev);
+}
+EXPORT_SYMBOL(drm_dp_aux_unregister_devnode);
+
+/**
+ * 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);
+		aux_dev->dev = NULL;
+		goto error;
+	}
+
+	DRM_DEBUG("drm_dp_aux_dev: aux [%s] registered as minor %d\n",
+		  aux->name, aux_dev->index);
+	return 0;
+error:
+	drm_dp_aux_unregister_devnode(aux);
+	return res;
+}
+EXPORT_SYMBOL(drm_dp_aux_register_devnode);
+
+int drm_dp_aux_dev_init(void)
+{
+	int 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;
+	}
+	drm_dp_aux_dev_class->dev_groups = drm_dp_aux_groups;
+
+	res = register_chrdev(0, "aux", &auxdev_fops);
+	if (res < 0)
+		goto out;
+	drm_dev_major = res;
+
+	return 0;
+out:
+	class_destroy(drm_dp_aux_dev_class);
+	return res;
+}
+EXPORT_SYMBOL(drm_dp_aux_dev_init);
+
+void drm_dp_aux_dev_exit(void)
+{
+	unregister_chrdev(drm_dev_major, "aux");
+	class_destroy(drm_dp_aux_dev_class);
+}
+EXPORT_SYMBOL(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..7d58f59 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>
 
 /**
@@ -754,6 +755,8 @@ static const struct i2c_algorithm drm_dp_i2c_algo = {
  */
 int drm_dp_aux_register(struct drm_dp_aux *aux)
 {
+	int ret;
+
 	mutex_init(&aux->hw_mutex);
 
 	aux->ddc.algo = &drm_dp_i2c_algo;
@@ -768,7 +771,17 @@ 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));
 
-	return i2c_add_adapter(&aux->ddc);
+	ret = drm_dp_aux_register_devnode(aux);
+	if (ret)
+		return ret;
+
+	ret = i2c_add_adapter(&aux->ddc);
+	if (ret) {
+		drm_dp_aux_unregister_devnode(aux);
+		return ret;
+	}
+
+	return 0;
 }
 EXPORT_SYMBOL(drm_dp_aux_register);
 
@@ -778,6 +791,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/drivers/gpu/drm/drm_kms_helper_common.c b/drivers/gpu/drm/drm_kms_helper_common.c
index d361005..3187c4b 100644
--- a/drivers/gpu/drm/drm_kms_helper_common.c
+++ b/drivers/gpu/drm/drm_kms_helper_common.c
@@ -27,6 +27,7 @@
 
 #include <drm/drmP.h>
 #include <drm/drm_fb_helper.h>
+#include <drm/drm_dp_aux_dev.h>
 
 MODULE_AUTHOR("David Airlie, Jesse Barnes");
 MODULE_DESCRIPTION("DRM KMS helper");
@@ -34,13 +35,25 @@ MODULE_LICENSE("GPL and additional rights");
 
 static int __init drm_kms_helper_init(void)
 {
+	int ret;
+
 	/* Call init functions from specific kms helpers here */
-	return drm_fb_helper_modinit();
+	ret = drm_fb_helper_modinit();
+	if (ret < 0)
+		goto out;
+
+	ret = drm_dp_aux_dev_init();
+	if (ret < 0)
+		goto out;
+
+out:
+	return ret;
 }
 
 static void __exit drm_kms_helper_exit(void)
 {
 	/* Call exit functions from specific kms helpers here */
+	drm_dp_aux_dev_exit();
 }
 
 module_init(drm_kms_helper_init);
diff --git a/include/drm/drm_dp_aux_dev.h b/include/drm/drm_dp_aux_dev.h
new file mode 100644
index 0000000..1b76d99
--- /dev/null
+++ b/include/drm/drm_dp_aux_dev.h
@@ -0,0 +1,62 @@
+/*
+ * 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
+
+#include <drm/drm_dp_helper.h>
+
+#ifdef CONFIG_DRM_DP_AUX_CHARDEV
+
+int drm_dp_aux_dev_init(void);
+void drm_dp_aux_dev_exit(void);
+int drm_dp_aux_register_devnode(struct drm_dp_aux *aux);
+void drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux);
+
+#else
+
+static inline int drm_dp_aux_dev_init(void)
+{
+	return 0;
+}
+
+static inline void drm_dp_aux_dev_exit(void)
+{
+}
+
+static inline int drm_dp_aux_register_devnode(struct drm_dp_aux *aux)
+{
+	return 0;
+}
+
+static inline void drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux)
+{
+}
+
+#endif
+
+#endif
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v10 3/4] drm/i915: Set aux.dev to the drm_connector device, instead of drm_device.

[Rafael Antognolli]

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.

v9:
 - As a side effect, drm_dp_aux_unregister() must be called before
 intel_connector_unregister(), as both the aux.dev and the i2c adapter
 dev are children of the drm_connector device now. Calling
 drm_dp_aux_unregister() before prevents them from being destroyed
 twice.

v10:
 - move aux_fini() to connector_unregister(), instead of moving
 drm_dp_aux_unregister() outside of connector_register().

Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e2bea710..da704c6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1188,7 +1188,6 @@ intel_dp_aux_fini(struct intel_dp *intel_dp)
 static int
 intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 {
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	enum port port = intel_dig_port->port;
 	int ret;
@@ -1199,7 +1198,7 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 	if (!intel_dp->aux.name)
 		return -ENOMEM;
 
-	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",
@@ -1214,16 +1213,6 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 		return ret;
 	}
 
-	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",
-			  intel_dp->aux.name, ret);
-		intel_dp_aux_fini(intel_dp);
-		return ret;
-	}
-
 	return 0;
 }
 
@@ -1232,9 +1221,7 @@ 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_dp_aux_fini(intel_dp);
 	intel_connector_unregister(intel_connector);
 }
 
@@ -4868,7 +4855,6 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
 
-	intel_dp_aux_fini(intel_dp);
 	intel_dp_mst_encoder_cleanup(intel_dig_port);
 	if (is_edp(intel_dp)) {
 		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v10 4/4] drm/radeon: Fix WARN_ON if DRM_DP_AUX_CHARDEV is enabled

[Rafael Antognolli]

From: Lukas Wunner <lukas@wunner.de>

Rafael Antognolli's new DRM_DP_AUX_CHARDEV feature causes a WARN_ON
if drm_dp_aux->dev == drm_connector->kdev and drm_dp_aux_unregister()
is called after drm_connector_unregister(). radeon is the only driver
affected by this besides i915. (amdgpu calls drm_dp_aux_unregister()
before drm_connector_unregister().)

Cc: Rafael Antognolli <rafael.antognolli@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/radeon/radeon_display.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index b3bb923..a885dae 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -1684,6 +1684,9 @@ void radeon_modeset_fini(struct radeon_device *rdev)
 	radeon_fbdev_fini(rdev);
 	kfree(rdev->mode_info.bios_hardcoded_edid);
 
+	/* free i2c buses */
+	radeon_i2c_fini(rdev);
+
 	if (rdev->mode_info.mode_config_initialized) {
 		radeon_afmt_fini(rdev);
 		drm_kms_helper_poll_fini(rdev->ddev);
@@ -1691,8 +1694,6 @@ void radeon_modeset_fini(struct radeon_device *rdev)
 		drm_mode_config_cleanup(rdev->ddev);
 		rdev->mode_info.mode_config_initialized = false;
 	}
-	/* free i2c buses */
-	radeon_i2c_fini(rdev);
 }
 
 static bool is_hdtv_mode(const struct drm_display_mode *mode)
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for Add drm_dp_aux chardev support. (rev5)

[Patchwork]

== Summary ==

Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest

Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (ilk-hp8440p)
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (skl-i7k-2)
                pass       -> DMESG-WARN (ilk-hp8440p)

bdw-nuci7        total:140  pass:131  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:143  pass:137  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:143  pass:119  dwarn:0   dfail:0   fail:0   skip:24 
byt-nuc          total:143  pass:128  dwarn:0   dfail:0   fail:0   skip:15 
hsw-brixbox      total:143  pass:136  dwarn:0   dfail:0   fail:0   skip:7  
ilk-hp8440p      total:143  pass:102  dwarn:3   dfail:0   fail:0   skip:38 
ivb-t430s        total:143  pass:137  dwarn:0   dfail:0   fail:0   skip:6  
skl-i5k-2        total:143  pass:134  dwarn:1   dfail:0   fail:0   skip:8  
skl-i7k-2        total:143  pass:133  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps      total:143  pass:129  dwarn:0   dfail:0   fail:0   skip:14 
snb-x220t        total:143  pass:129  dwarn:0   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1244/

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v10 1/4] drm/kms_helper: Add a common place to call init and exit functions.

[Ville Syrjälä]

On Thu, Jan 21, 2016 at 03:10:18PM -0800, Rafael Antognolli wrote:
> The module_init and module_exit functions will start here, and call the
> subsequent init's and exit's.
> 
> v10:
>  - Keep __init on drm_fb_helper init function.
>  - Move MODULE_* macros to the common file.
> 
> Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>

Looks all right to me.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/Makefile                |  4 ++-
>  drivers/gpu/drm/drm_crtc_helper.c       |  3 ---
>  drivers/gpu/drm/drm_fb_helper.c         |  9 +++----
>  drivers/gpu/drm/drm_kms_helper_common.c | 47 +++++++++++++++++++++++++++++++++
>  include/drm/drm_fb_helper.h             |  6 +++++
>  5 files changed, 60 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_kms_helper_common.c
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index f858aa2..dfe513f 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -24,7 +24,9 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o
>  drm-y += $(drm-m)
>  
>  drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
> -		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o
> +		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
> +		drm_kms_helper_common.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
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 5d4bc64..baac181 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -73,9 +73,6 @@
>   * &drm_crtc_helper_funcs, struct &drm_encoder_helper_funcs and struct
>   * &drm_connector_helper_funcs.
>   */
> -MODULE_AUTHOR("David Airlie, Jesse Barnes");
> -MODULE_DESCRIPTION("DRM KMS helper");
> -MODULE_LICENSE("GPL and additional rights");
>  
>  /**
>   * drm_helper_move_panel_connectors_to_head() - move panels to the front in the
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 1e103c4..c27b964 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2175,9 +2175,9 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
>   * but the module doesn't depend on any fb console symbols.  At least
>   * attempt to load fbcon to avoid leaving the system without a usable console.
>   */
> -#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
> -static int __init drm_fb_helper_modinit(void)
> +int __init drm_fb_helper_modinit(void)
>  {
> +#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
>  	const char *name = "fbcon";
>  	struct module *fbcon;
>  
> @@ -2187,8 +2187,7 @@ static int __init drm_fb_helper_modinit(void)
>  
>  	if (!fbcon)
>  		request_module_nowait(name);
> +#endif
>  	return 0;
>  }
> -
> -module_init(drm_fb_helper_modinit);
> -#endif
> +EXPORT_SYMBOL(drm_fb_helper_modinit);
> diff --git a/drivers/gpu/drm/drm_kms_helper_common.c b/drivers/gpu/drm/drm_kms_helper_common.c
> new file mode 100644
> index 0000000..d361005
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_kms_helper_common.c
> @@ -0,0 +1,47 @@
> +/*
> + * 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 <drm/drmP.h>
> +#include <drm/drm_fb_helper.h>
> +
> +MODULE_AUTHOR("David Airlie, Jesse Barnes");
> +MODULE_DESCRIPTION("DRM KMS helper");
> +MODULE_LICENSE("GPL and additional rights");
> +
> +static int __init drm_kms_helper_init(void)
> +{
> +	/* Call init functions from specific kms helpers here */
> +	return drm_fb_helper_modinit();
> +}
> +
> +static void __exit drm_kms_helper_exit(void)
> +{
> +	/* Call exit functions from specific kms helpers here */
> +}
> +
> +module_init(drm_kms_helper_init);
> +module_exit(drm_kms_helper_exit);
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index d8a40df..062723b 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -219,6 +219,7 @@ struct drm_fb_helper {
>  };
>  
>  #ifdef CONFIG_DRM_FBDEV_EMULATION
> +int drm_fb_helper_modinit(void);
>  void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
>  			   const struct drm_fb_helper_funcs *funcs);
>  int drm_fb_helper_init(struct drm_device *dev,
> @@ -283,6 +284,11 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_
>  int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
>  				       struct drm_connector *connector);
>  #else
> +static inline int drm_fb_helper_modinit(void)
> +{
> +	return 0;
> +}
> +
>  static inline void drm_fb_helper_prepare(struct drm_device *dev,
>  					struct drm_fb_helper *helper,
>  					const struct drm_fb_helper_funcs *funcs)
> -- 
> 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
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v10 2/4] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.

[Ville Syrjälä]

On Thu, Jan 21, 2016 at 03:10:19PM -0800, 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.
> 
> 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
> 
> v6:
>  - Use atomic_t for usage count
>  - Use a mutex instead of spinlock for idr lock
>  - Destroy chardev immediately on unregister
>  - other minor suggestions from Ville
> 
> v7:
>  - style fixes
>  - error handling fixes
> 
> v8:
>  - more error handling fixes
> 
> v9:
>  - remove module_init and module_exit, and add drm_dp_aux_dev_init/exit
>  to drm_kms_helper_init/exit.
> 
> Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>

Only checked the init/exit stuff since I should have read the rest 
many times by now.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/Kconfig                 |   8 +
>  drivers/gpu/drm/Makefile                |   1 +
>  drivers/gpu/drm/drm_dp_aux_dev.c        | 368 ++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_dp_helper.c         |  16 +-
>  drivers/gpu/drm/drm_kms_helper_common.c |  15 +-
>  include/drm/drm_dp_aux_dev.h            |  62 ++++++
>  6 files changed, 468 insertions(+), 2 deletions(-)
>  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 59babd5..dff87ca 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 dfe513f..424fcb7 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -30,6 +30,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..f73b38b
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> @@ -0,0 +1,368 @@
> +/*
> + * 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 <linux/uaccess.h>
> +#include <drm/drm_dp_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drmP.h>
> +
> +struct drm_dp_aux_dev {
> +	unsigned index;
> +	struct drm_dp_aux *aux;
> +	struct device *dev;
> +	struct kref refcount;
> +	atomic_t usecount;
> +};
> +
> +#define DRM_AUX_MINORS	256
> +#define AUX_MAX_OFFSET	(1 << 20)
> +static DEFINE_IDR(aux_idr);
> +static DEFINE_MUTEX(aux_idr_mutex);
> +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;
> +
> +	mutex_lock(&aux_idr_mutex);
> +	aux_dev = idr_find(&aux_idr, index);
> +	if (!kref_get_unless_zero(&aux_dev->refcount))
> +		aux_dev = NULL;
> +	mutex_unlock(&aux_idr_mutex);
> +
> +	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;
> +	atomic_set(&aux_dev->usecount, 1);
> +	kref_init(&aux_dev->refcount);
> +
> +	mutex_lock(&aux_idr_mutex);
> +	index = idr_alloc_cyclic(&aux_idr, aux_dev, 0, DRM_AUX_MINORS,
> +				 GFP_KERNEL);
> +	mutex_unlock(&aux_idr_mutex);
> +	if (index < 0) {
> +		kfree(aux_dev);
> +		return ERR_PTR(index);
> +	}
> +	aux_dev->index = index;
> +
> +	return aux_dev;
> +}
> +
> +static void release_drm_dp_aux_dev(struct kref *ref)
> +{
> +	struct drm_dp_aux_dev *aux_dev =
> +		container_of(ref, struct drm_dp_aux_dev, refcount);
> +
> +	kfree(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;
> +	ssize_t res = 0;
> +
> +	if (!atomic_inc_not_zero(&aux_dev->usecount))
> +		return -ENODEV;
> +
> +	bytes_pending = min((loff_t)count, AUX_MAX_OFFSET - (*offset));
> +
> +	if (!access_ok(VERIFY_WRITE, buf, bytes_pending)) {
> +		res = -EFAULT;
> +		goto out;
> +	}
> +
> +	while (bytes_pending > 0) {
> +		uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES];
> +		ssize_t todo = min_t(size_t, bytes_pending, sizeof(localbuf));
> +
> +		res = drm_dp_dpcd_read(aux_dev->aux, *offset, localbuf, todo);
> +		if (res <= 0) {
> +			res = num_bytes_processed ? num_bytes_processed : res;
> +			goto out;
> +		}
> +		if (__copy_to_user(buf + num_bytes_processed, localbuf, res)) {
> +			res = num_bytes_processed ?
> +				num_bytes_processed : -EFAULT;
> +			goto out;
> +		}
> +		bytes_pending -= res;
> +		*offset += res;
> +		num_bytes_processed += res;
> +		res = num_bytes_processed;
> +	}
> +
> +out:
> +	atomic_dec(&aux_dev->usecount);
> +	wake_up_atomic_t(&aux_dev->usecount);
> +	return res;
> +}
> +
> +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;
> +	ssize_t res = 0;
> +
> +	if (!atomic_inc_not_zero(&aux_dev->usecount))
> +		return -ENODEV;
> +
> +	bytes_pending = min((loff_t)count, AUX_MAX_OFFSET - *offset);
> +
> +	if (!access_ok(VERIFY_READ, buf, bytes_pending)) {
> +		res = -EFAULT;
> +		goto out;
> +	}
> +
> +	while (bytes_pending > 0) {
> +		uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES];
> +		ssize_t todo = min_t(size_t, bytes_pending, sizeof(localbuf));
> +
> +		if (__copy_from_user(localbuf,
> +				     buf + num_bytes_processed, todo)) {
> +			res = num_bytes_processed ?
> +				num_bytes_processed : -EFAULT;
> +			goto out;
> +		}
> +
> +		res = drm_dp_dpcd_write(aux_dev->aux, *offset, localbuf, todo);
> +		if (res <= 0) {
> +			res = num_bytes_processed ? num_bytes_processed : res;
> +			goto out;
> +		}
> +		bytes_pending -= res;
> +		*offset += res;
> +		num_bytes_processed += res;
> +		res = num_bytes_processed;
> +	}
> +
> +out:
> +	atomic_dec(&aux_dev->usecount);
> +	wake_up_atomic_t(&aux_dev->usecount);
> +	return res;
> +}
> +
> +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)
> +
> +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
> +	 */
> +	mutex_lock(&aux_idr_mutex);
> +	idr_for_each_entry(&aux_idr, iter, id) {
> +		if (iter->aux == aux) {
> +			aux_dev = iter;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&aux_idr_mutex);
> +	return aux_dev;
> +}
> +
> +static int auxdev_wait_atomic_t(atomic_t *p)
> +{
> +	schedule();
> +	return 0;
> +}
> +/**
> + * 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.
> + */
> +void drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux)
> +{
> +	struct drm_dp_aux_dev *aux_dev;
> +	unsigned int minor;
> +
> +	aux_dev = drm_dp_aux_dev_get_by_aux(aux);
> +	if (!aux_dev) /* attach must have failed */
> +		return;
> +
> +	mutex_lock(&aux_idr_mutex);
> +	idr_remove(&aux_idr, aux_dev->index);
> +	mutex_unlock(&aux_idr_mutex);
> +
> +	atomic_dec(&aux_dev->usecount);
> +	wait_on_atomic_t(&aux_dev->usecount, auxdev_wait_atomic_t,
> +			 TASK_UNINTERRUPTIBLE);
> +
> +	minor = aux_dev->index;
> +	if (aux_dev->dev)
> +		device_destroy(drm_dp_aux_dev_class,
> +			       MKDEV(drm_dev_major, minor));
> +
> +	DRM_DEBUG("drm_dp_aux_dev: aux [%s] unregistering\n", aux->name);
> +	kref_put(&aux_dev->refcount, release_drm_dp_aux_dev);
> +}
> +EXPORT_SYMBOL(drm_dp_aux_unregister_devnode);
> +
> +/**
> + * 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);
> +		aux_dev->dev = NULL;
> +		goto error;
> +	}
> +
> +	DRM_DEBUG("drm_dp_aux_dev: aux [%s] registered as minor %d\n",
> +		  aux->name, aux_dev->index);
> +	return 0;
> +error:
> +	drm_dp_aux_unregister_devnode(aux);
> +	return res;
> +}
> +EXPORT_SYMBOL(drm_dp_aux_register_devnode);
> +
> +int drm_dp_aux_dev_init(void)
> +{
> +	int 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;
> +	}
> +	drm_dp_aux_dev_class->dev_groups = drm_dp_aux_groups;
> +
> +	res = register_chrdev(0, "aux", &auxdev_fops);
> +	if (res < 0)
> +		goto out;
> +	drm_dev_major = res;
> +
> +	return 0;
> +out:
> +	class_destroy(drm_dp_aux_dev_class);
> +	return res;
> +}
> +EXPORT_SYMBOL(drm_dp_aux_dev_init);
> +
> +void drm_dp_aux_dev_exit(void)
> +{
> +	unregister_chrdev(drm_dev_major, "aux");
> +	class_destroy(drm_dp_aux_dev_class);
> +}
> +EXPORT_SYMBOL(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..7d58f59 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>
>  
>  /**
> @@ -754,6 +755,8 @@ static const struct i2c_algorithm drm_dp_i2c_algo = {
>   */
>  int drm_dp_aux_register(struct drm_dp_aux *aux)
>  {
> +	int ret;
> +
>  	mutex_init(&aux->hw_mutex);
>  
>  	aux->ddc.algo = &drm_dp_i2c_algo;
> @@ -768,7 +771,17 @@ 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));
>  
> -	return i2c_add_adapter(&aux->ddc);
> +	ret = drm_dp_aux_register_devnode(aux);
> +	if (ret)
> +		return ret;
> +
> +	ret = i2c_add_adapter(&aux->ddc);
> +	if (ret) {
> +		drm_dp_aux_unregister_devnode(aux);
> +		return ret;
> +	}
> +
> +	return 0;
>  }
>  EXPORT_SYMBOL(drm_dp_aux_register);
>  
> @@ -778,6 +791,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/drivers/gpu/drm/drm_kms_helper_common.c b/drivers/gpu/drm/drm_kms_helper_common.c
> index d361005..3187c4b 100644
> --- a/drivers/gpu/drm/drm_kms_helper_common.c
> +++ b/drivers/gpu/drm/drm_kms_helper_common.c
> @@ -27,6 +27,7 @@
>  
>  #include <drm/drmP.h>
>  #include <drm/drm_fb_helper.h>
> +#include <drm/drm_dp_aux_dev.h>
>  
>  MODULE_AUTHOR("David Airlie, Jesse Barnes");
>  MODULE_DESCRIPTION("DRM KMS helper");
> @@ -34,13 +35,25 @@ MODULE_LICENSE("GPL and additional rights");
>  
>  static int __init drm_kms_helper_init(void)
>  {
> +	int ret;
> +
>  	/* Call init functions from specific kms helpers here */
> -	return drm_fb_helper_modinit();
> +	ret = drm_fb_helper_modinit();
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = drm_dp_aux_dev_init();
> +	if (ret < 0)
> +		goto out;
> +
> +out:
> +	return ret;
>  }
>  
>  static void __exit drm_kms_helper_exit(void)
>  {
>  	/* Call exit functions from specific kms helpers here */
> +	drm_dp_aux_dev_exit();
>  }
>  
>  module_init(drm_kms_helper_init);
> diff --git a/include/drm/drm_dp_aux_dev.h b/include/drm/drm_dp_aux_dev.h
> new file mode 100644
> index 0000000..1b76d99
> --- /dev/null
> +++ b/include/drm/drm_dp_aux_dev.h
> @@ -0,0 +1,62 @@
> +/*
> + * 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
> +
> +#include <drm/drm_dp_helper.h>
> +
> +#ifdef CONFIG_DRM_DP_AUX_CHARDEV
> +
> +int drm_dp_aux_dev_init(void);
> +void drm_dp_aux_dev_exit(void);
> +int drm_dp_aux_register_devnode(struct drm_dp_aux *aux);
> +void drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux);
> +
> +#else
> +
> +static inline int drm_dp_aux_dev_init(void)
> +{
> +	return 0;
> +}
> +
> +static inline void drm_dp_aux_dev_exit(void)
> +{
> +}
> +
> +static inline int drm_dp_aux_register_devnode(struct drm_dp_aux *aux)
> +{
> +	return 0;
> +}
> +
> +static inline void drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux)
> +{
> +}
> +
> +#endif
> +
> +#endif
> -- 
> 2.4.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: warning for Add drm_dp_aux chardev support. (rev5)

[Ville Syrjälä]

On Fri, Jan 22, 2016 at 09:15:31AM -0000, Patchwork wrote:
> == Summary ==
> 
> Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest
> 
> Test kms_flip:
>         Subgroup basic-flip-vs-dpms:
>                 pass       -> DMESG-WARN (ilk-hp8440p)

The usual ilk underrun
https://bugs.freedesktop.org/show_bug.cgi?id=93787

>         Subgroup basic-flip-vs-modeset:
>                 pass       -> DMESG-WARN (skl-i7k-2)

This is the known "DMC doesn't know what state it's in" problem.

>                 pass       -> DMESG-WARN (ilk-hp8440p)

Another ilk underrun.

> 
> bdw-nuci7        total:140  pass:131  dwarn:0   dfail:0   fail:0   skip:9  
> bdw-ultra        total:143  pass:137  dwarn:0   dfail:0   fail:0   skip:6  
> bsw-nuc-2        total:143  pass:119  dwarn:0   dfail:0   fail:0   skip:24 
> byt-nuc          total:143  pass:128  dwarn:0   dfail:0   fail:0   skip:15 
> hsw-brixbox      total:143  pass:136  dwarn:0   dfail:0   fail:0   skip:7  
> ilk-hp8440p      total:143  pass:102  dwarn:3   dfail:0   fail:0   skip:38 
> ivb-t430s        total:143  pass:137  dwarn:0   dfail:0   fail:0   skip:6  
> skl-i5k-2        total:143  pass:134  dwarn:1   dfail:0   fail:0   skip:8  
> skl-i7k-2        total:143  pass:133  dwarn:2   dfail:0   fail:0   skip:8  
> snb-dellxps      total:143  pass:129  dwarn:0   dfail:0   fail:0   skip:14 
> snb-x220t        total:143  pass:129  dwarn:0   dfail:0   fail:1   skip:13 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1244/
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v10 2/4] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.

[Daniel Vetter]

On Fri, Feb 12, 2016 at 01:53:10PM +0200, Ville Syrjälä wrote:
> On Thu, Jan 21, 2016 at 03:10:19PM -0800, 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.
> > 
> > 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
> > 
> > v6:
> >  - Use atomic_t for usage count
> >  - Use a mutex instead of spinlock for idr lock
> >  - Destroy chardev immediately on unregister
> >  - other minor suggestions from Ville
> > 
> > v7:
> >  - style fixes
> >  - error handling fixes
> > 
> > v8:
> >  - more error handling fixes
> > 
> > v9:
> >  - remove module_init and module_exit, and add drm_dp_aux_dev_init/exit
> >  to drm_kms_helper_init/exit.
> > 
> > Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
> 
> Only checked the init/exit stuff since I should have read the rest 
> many times by now.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Merged the first 2 patches to drm-misc, thanks.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/Kconfig                 |   8 +
> >  drivers/gpu/drm/Makefile                |   1 +
> >  drivers/gpu/drm/drm_dp_aux_dev.c        | 368 ++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/drm_dp_helper.c         |  16 +-
> >  drivers/gpu/drm/drm_kms_helper_common.c |  15 +-
> >  include/drm/drm_dp_aux_dev.h            |  62 ++++++
> >  6 files changed, 468 insertions(+), 2 deletions(-)
> >  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 59babd5..dff87ca 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 dfe513f..424fcb7 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -30,6 +30,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..f73b38b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> > @@ -0,0 +1,368 @@
> > +/*
> > + * 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 <linux/uaccess.h>
> > +#include <drm/drm_dp_helper.h>
> > +#include <drm/drm_crtc.h>
> > +#include <drm/drmP.h>
> > +
> > +struct drm_dp_aux_dev {
> > +	unsigned index;
> > +	struct drm_dp_aux *aux;
> > +	struct device *dev;
> > +	struct kref refcount;
> > +	atomic_t usecount;
> > +};
> > +
> > +#define DRM_AUX_MINORS	256
> > +#define AUX_MAX_OFFSET	(1 << 20)
> > +static DEFINE_IDR(aux_idr);
> > +static DEFINE_MUTEX(aux_idr_mutex);
> > +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;
> > +
> > +	mutex_lock(&aux_idr_mutex);
> > +	aux_dev = idr_find(&aux_idr, index);
> > +	if (!kref_get_unless_zero(&aux_dev->refcount))
> > +		aux_dev = NULL;
> > +	mutex_unlock(&aux_idr_mutex);
> > +
> > +	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;
> > +	atomic_set(&aux_dev->usecount, 1);
> > +	kref_init(&aux_dev->refcount);
> > +
> > +	mutex_lock(&aux_idr_mutex);
> > +	index = idr_alloc_cyclic(&aux_idr, aux_dev, 0, DRM_AUX_MINORS,
> > +				 GFP_KERNEL);
> > +	mutex_unlock(&aux_idr_mutex);
> > +	if (index < 0) {
> > +		kfree(aux_dev);
> > +		return ERR_PTR(index);
> > +	}
> > +	aux_dev->index = index;
> > +
> > +	return aux_dev;
> > +}
> > +
> > +static void release_drm_dp_aux_dev(struct kref *ref)
> > +{
> > +	struct drm_dp_aux_dev *aux_dev =
> > +		container_of(ref, struct drm_dp_aux_dev, refcount);
> > +
> > +	kfree(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;
> > +	ssize_t res = 0;
> > +
> > +	if (!atomic_inc_not_zero(&aux_dev->usecount))
> > +		return -ENODEV;
> > +
> > +	bytes_pending = min((loff_t)count, AUX_MAX_OFFSET - (*offset));
> > +
> > +	if (!access_ok(VERIFY_WRITE, buf, bytes_pending)) {
> > +		res = -EFAULT;
> > +		goto out;
> > +	}
> > +
> > +	while (bytes_pending > 0) {
> > +		uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES];
> > +		ssize_t todo = min_t(size_t, bytes_pending, sizeof(localbuf));
> > +
> > +		res = drm_dp_dpcd_read(aux_dev->aux, *offset, localbuf, todo);
> > +		if (res <= 0) {
> > +			res = num_bytes_processed ? num_bytes_processed : res;
> > +			goto out;
> > +		}
> > +		if (__copy_to_user(buf + num_bytes_processed, localbuf, res)) {
> > +			res = num_bytes_processed ?
> > +				num_bytes_processed : -EFAULT;
> > +			goto out;
> > +		}
> > +		bytes_pending -= res;
> > +		*offset += res;
> > +		num_bytes_processed += res;
> > +		res = num_bytes_processed;
> > +	}
> > +
> > +out:
> > +	atomic_dec(&aux_dev->usecount);
> > +	wake_up_atomic_t(&aux_dev->usecount);
> > +	return res;
> > +}
> > +
> > +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;
> > +	ssize_t res = 0;
> > +
> > +	if (!atomic_inc_not_zero(&aux_dev->usecount))
> > +		return -ENODEV;
> > +
> > +	bytes_pending = min((loff_t)count, AUX_MAX_OFFSET - *offset);
> > +
> > +	if (!access_ok(VERIFY_READ, buf, bytes_pending)) {
> > +		res = -EFAULT;
> > +		goto out;
> > +	}
> > +
> > +	while (bytes_pending > 0) {
> > +		uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES];
> > +		ssize_t todo = min_t(size_t, bytes_pending, sizeof(localbuf));
> > +
> > +		if (__copy_from_user(localbuf,
> > +				     buf + num_bytes_processed, todo)) {
> > +			res = num_bytes_processed ?
> > +				num_bytes_processed : -EFAULT;
> > +			goto out;
> > +		}
> > +
> > +		res = drm_dp_dpcd_write(aux_dev->aux, *offset, localbuf, todo);
> > +		if (res <= 0) {
> > +			res = num_bytes_processed ? num_bytes_processed : res;
> > +			goto out;
> > +		}
> > +		bytes_pending -= res;
> > +		*offset += res;
> > +		num_bytes_processed += res;
> > +		res = num_bytes_processed;
> > +	}
> > +
> > +out:
> > +	atomic_dec(&aux_dev->usecount);
> > +	wake_up_atomic_t(&aux_dev->usecount);
> > +	return res;
> > +}
> > +
> > +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)
> > +
> > +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
> > +	 */
> > +	mutex_lock(&aux_idr_mutex);
> > +	idr_for_each_entry(&aux_idr, iter, id) {
> > +		if (iter->aux == aux) {
> > +			aux_dev = iter;
> > +			break;
> > +		}
> > +	}
> > +	mutex_unlock(&aux_idr_mutex);
> > +	return aux_dev;
> > +}
> > +
> > +static int auxdev_wait_atomic_t(atomic_t *p)
> > +{
> > +	schedule();
> > +	return 0;
> > +}
> > +/**
> > + * 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.
> > + */
> > +void drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux)
> > +{
> > +	struct drm_dp_aux_dev *aux_dev;
> > +	unsigned int minor;
> > +
> > +	aux_dev = drm_dp_aux_dev_get_by_aux(aux);
> > +	if (!aux_dev) /* attach must have failed */
> > +		return;
> > +
> > +	mutex_lock(&aux_idr_mutex);
> > +	idr_remove(&aux_idr, aux_dev->index);
> > +	mutex_unlock(&aux_idr_mutex);
> > +
> > +	atomic_dec(&aux_dev->usecount);
> > +	wait_on_atomic_t(&aux_dev->usecount, auxdev_wait_atomic_t,
> > +			 TASK_UNINTERRUPTIBLE);
> > +
> > +	minor = aux_dev->index;
> > +	if (aux_dev->dev)
> > +		device_destroy(drm_dp_aux_dev_class,
> > +			       MKDEV(drm_dev_major, minor));
> > +
> > +	DRM_DEBUG("drm_dp_aux_dev: aux [%s] unregistering\n", aux->name);
> > +	kref_put(&aux_dev->refcount, release_drm_dp_aux_dev);
> > +}
> > +EXPORT_SYMBOL(drm_dp_aux_unregister_devnode);
> > +
> > +/**
> > + * 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);
> > +		aux_dev->dev = NULL;
> > +		goto error;
> > +	}
> > +
> > +	DRM_DEBUG("drm_dp_aux_dev: aux [%s] registered as minor %d\n",
> > +		  aux->name, aux_dev->index);
> > +	return 0;
> > +error:
> > +	drm_dp_aux_unregister_devnode(aux);
> > +	return res;
> > +}
> > +EXPORT_SYMBOL(drm_dp_aux_register_devnode);
> > +
> > +int drm_dp_aux_dev_init(void)
> > +{
> > +	int 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;
> > +	}
> > +	drm_dp_aux_dev_class->dev_groups = drm_dp_aux_groups;
> > +
> > +	res = register_chrdev(0, "aux", &auxdev_fops);
> > +	if (res < 0)
> > +		goto out;
> > +	drm_dev_major = res;
> > +
> > +	return 0;
> > +out:
> > +	class_destroy(drm_dp_aux_dev_class);
> > +	return res;
> > +}
> > +EXPORT_SYMBOL(drm_dp_aux_dev_init);
> > +
> > +void drm_dp_aux_dev_exit(void)
> > +{
> > +	unregister_chrdev(drm_dev_major, "aux");
> > +	class_destroy(drm_dp_aux_dev_class);
> > +}
> > +EXPORT_SYMBOL(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..7d58f59 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>
> >  
> >  /**
> > @@ -754,6 +755,8 @@ static const struct i2c_algorithm drm_dp_i2c_algo = {
> >   */
> >  int drm_dp_aux_register(struct drm_dp_aux *aux)
> >  {
> > +	int ret;
> > +
> >  	mutex_init(&aux->hw_mutex);
> >  
> >  	aux->ddc.algo = &drm_dp_i2c_algo;
> > @@ -768,7 +771,17 @@ 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));
> >  
> > -	return i2c_add_adapter(&aux->ddc);
> > +	ret = drm_dp_aux_register_devnode(aux);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = i2c_add_adapter(&aux->ddc);
> > +	if (ret) {
> > +		drm_dp_aux_unregister_devnode(aux);
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> >  }
> >  EXPORT_SYMBOL(drm_dp_aux_register);
> >  
> > @@ -778,6 +791,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/drivers/gpu/drm/drm_kms_helper_common.c b/drivers/gpu/drm/drm_kms_helper_common.c
> > index d361005..3187c4b 100644
> > --- a/drivers/gpu/drm/drm_kms_helper_common.c
> > +++ b/drivers/gpu/drm/drm_kms_helper_common.c
> > @@ -27,6 +27,7 @@
> >  
> >  #include <drm/drmP.h>
> >  #include <drm/drm_fb_helper.h>
> > +#include <drm/drm_dp_aux_dev.h>
> >  
> >  MODULE_AUTHOR("David Airlie, Jesse Barnes");
> >  MODULE_DESCRIPTION("DRM KMS helper");
> > @@ -34,13 +35,25 @@ MODULE_LICENSE("GPL and additional rights");
> >  
> >  static int __init drm_kms_helper_init(void)
> >  {
> > +	int ret;
> > +
> >  	/* Call init functions from specific kms helpers here */
> > -	return drm_fb_helper_modinit();
> > +	ret = drm_fb_helper_modinit();
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +	ret = drm_dp_aux_dev_init();
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +out:
> > +	return ret;
> >  }
> >  
> >  static void __exit drm_kms_helper_exit(void)
> >  {
> >  	/* Call exit functions from specific kms helpers here */
> > +	drm_dp_aux_dev_exit();
> >  }
> >  
> >  module_init(drm_kms_helper_init);
> > diff --git a/include/drm/drm_dp_aux_dev.h b/include/drm/drm_dp_aux_dev.h
> > new file mode 100644
> > index 0000000..1b76d99
> > --- /dev/null
> > +++ b/include/drm/drm_dp_aux_dev.h
> > @@ -0,0 +1,62 @@
> > +/*
> > + * 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
> > +
> > +#include <drm/drm_dp_helper.h>
> > +
> > +#ifdef CONFIG_DRM_DP_AUX_CHARDEV
> > +
> > +int drm_dp_aux_dev_init(void);
> > +void drm_dp_aux_dev_exit(void);
> > +int drm_dp_aux_register_devnode(struct drm_dp_aux *aux);
> > +void drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux);
> > +
> > +#else
> > +
> > +static inline int drm_dp_aux_dev_init(void)
> > +{
> > +	return 0;
> > +}
> > +
> > +static inline void drm_dp_aux_dev_exit(void)
> > +{
> > +}
> > +
> > +static inline int drm_dp_aux_register_devnode(struct drm_dp_aux *aux)
> > +{
> > +	return 0;
> > +}
> > +
> > +static inline void drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux)
> > +{
> > +}
> > +
> > +#endif
> > +
> > +#endif
> > -- 
> > 2.4.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v10 3/4] drm/i915: Set aux.dev to the drm_connector device, instead of drm_device.

[Imre Deak]

On Thu, 2016-01-21 at 15:10 -0800, Rafael Antognolli wrote:
> 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.

Yes, having the i2c-dev only at one place under the connector is more
logical and what we want imo. It's an ABI change but if other drivers
already have it in this way then I assume it's fine. For HDMI
connectors we still have them under the drm_device (and there is no
symlink for them under the connector device) but this was inconsistent
already before this patch and can be fixed up later.

Adding Jani, in case he has comments on this.

Below one more comment:

> v9:
>  - As a side effect, drm_dp_aux_unregister() must be called before
>  intel_connector_unregister(), as both the aux.dev and the i2c
> adapter
>  dev are children of the drm_connector device now. Calling
>  drm_dp_aux_unregister() before prevents them from being destroyed
>  twice.
> 
> v10:
>  - move aux_fini() to connector_unregister(), instead of moving
>  drm_dp_aux_unregister() outside of connector_register().
> 
> Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 18 ++----------------
>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index e2bea710..da704c6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1188,7 +1188,6 @@ intel_dp_aux_fini(struct intel_dp *intel_dp)
>  static int
>  intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector
> *connector)
>  {
> -	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>  	struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
>  	enum port port = intel_dig_port->port;
>  	int ret;
> @@ -1199,7 +1198,7 @@ intel_dp_aux_init(struct intel_dp *intel_dp,
> struct intel_connector *connector)
>  	if (!intel_dp->aux.name)
>  		return -ENOMEM;
>  
> -	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",
> @@ -1214,16 +1213,6 @@ intel_dp_aux_init(struct intel_dp *intel_dp,
> struct intel_connector *connector)
>  		return ret;
>  	}
>  
> -	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",
> -			  intel_dp->aux.name, ret);
> -		intel_dp_aux_fini(intel_dp);
> -		return ret;
> -	}
> -
>  	return 0;
>  }
>  
> @@ -1232,9 +1221,7 @@ 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_dp_aux_fini(intel_dp);

Hrm, the mst_port check seems to have been misplaced at some point,
this function isn't called for virtual MST connectors. So I think it's
fine to remove it. The patch looks ok:

Reviewed-by: Imre Deak <imre.deak@intel.com>

>  	intel_connector_unregister(intel_connector);
>  }
>  
> @@ -4868,7 +4855,6 @@ void intel_dp_encoder_destroy(struct
> drm_encoder *encoder)
>  	struct intel_digital_port *intel_dig_port =
> enc_to_dig_port(encoder);
>  	struct intel_dp *intel_dp = &intel_dig_port->dp;
>  
> -	intel_dp_aux_fini(intel_dp);
>  	intel_dp_mst_encoder_cleanup(intel_dig_port);
>  	if (is_edp(intel_dp)) {
>  		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v10 4/4] drm/radeon: Fix WARN_ON if DRM_DP_AUX_CHARDEV is enabled

[Alex Deucher]

On Thu, Jan 21, 2016 at 6:10 PM, Rafael Antognolli
<rafael.antognolli@intel.com> wrote:
> From: Lukas Wunner <lukas@wunner.de>
>
> Rafael Antognolli's new DRM_DP_AUX_CHARDEV feature causes a WARN_ON
> if drm_dp_aux->dev == drm_connector->kdev and drm_dp_aux_unregister()
> is called after drm_connector_unregister(). radeon is the only driver
> affected by this besides i915. (amdgpu calls drm_dp_aux_unregister()
> before drm_connector_unregister().)
>
> Cc: Rafael Antognolli <rafael.antognolli@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>

Applied.  thanks!

Alex

> ---
>  drivers/gpu/drm/radeon/radeon_display.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
> index b3bb923..a885dae 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -1684,6 +1684,9 @@ void radeon_modeset_fini(struct radeon_device *rdev)
>         radeon_fbdev_fini(rdev);
>         kfree(rdev->mode_info.bios_hardcoded_edid);
>
> +       /* free i2c buses */
> +       radeon_i2c_fini(rdev);
> +
>         if (rdev->mode_info.mode_config_initialized) {
>                 radeon_afmt_fini(rdev);
>                 drm_kms_helper_poll_fini(rdev->ddev);
> @@ -1691,8 +1694,6 @@ void radeon_modeset_fini(struct radeon_device *rdev)
>                 drm_mode_config_cleanup(rdev->ddev);
>                 rdev->mode_info.mode_config_initialized = false;
>         }
> -       /* free i2c buses */
> -       radeon_i2c_fini(rdev);
>  }
>
>  static bool is_hdtv_mode(const struct drm_display_mode *mode)
> --
> 2.4.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: warning for Add drm_dp_aux chardev support. (rev5)

[Rafael Antognolli]

On Fri, Feb 12, 2016 at 02:01:11PM +0200, Ville Syrjälä wrote:
> On Fri, Jan 22, 2016 at 09:15:31AM -0000, Patchwork wrote:
> > == Summary ==
> > 
> > Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest
> > 
> > Test kms_flip:
> >         Subgroup basic-flip-vs-dpms:
> >                 pass       -> DMESG-WARN (ilk-hp8440p)
> 
> The usual ilk underrun
> https://bugs.freedesktop.org/show_bug.cgi?id=93787
> 
> >         Subgroup basic-flip-vs-modeset:
> >                 pass       -> DMESG-WARN (skl-i7k-2)
> 
> This is the known "DMC doesn't know what state it's in" problem.

Does it mean that I have to fix it on my patch, or is it something
expected from this test?

> >                 pass       -> DMESG-WARN (ilk-hp8440p)
> 
> Another ilk underrun.
> 
> > 
> > bdw-nuci7        total:140  pass:131  dwarn:0   dfail:0   fail:0   skip:9  
> > bdw-ultra        total:143  pass:137  dwarn:0   dfail:0   fail:0   skip:6  
> > bsw-nuc-2        total:143  pass:119  dwarn:0   dfail:0   fail:0   skip:24 
> > byt-nuc          total:143  pass:128  dwarn:0   dfail:0   fail:0   skip:15 
> > hsw-brixbox      total:143  pass:136  dwarn:0   dfail:0   fail:0   skip:7  
> > ilk-hp8440p      total:143  pass:102  dwarn:3   dfail:0   fail:0   skip:38 
> > ivb-t430s        total:143  pass:137  dwarn:0   dfail:0   fail:0   skip:6  
> > skl-i5k-2        total:143  pass:134  dwarn:1   dfail:0   fail:0   skip:8  
> > skl-i7k-2        total:143  pass:133  dwarn:2   dfail:0   fail:0   skip:8  
> > snb-dellxps      total:143  pass:129  dwarn:0   dfail:0   fail:0   skip:14 
> > snb-x220t        total:143  pass:129  dwarn:0   dfail:0   fail:1   skip:13 
> > 
> > Results at /archive/results/CI_IGT_test/Patchwork_1244/
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: warning for Add drm_dp_aux chardev support. (rev5)

[Ville Syrjälä]

On Fri, Feb 12, 2016 at 01:21:33PM -0800, Rafael Antognolli wrote:
> On Fri, Feb 12, 2016 at 02:01:11PM +0200, Ville Syrjälä wrote:
> > On Fri, Jan 22, 2016 at 09:15:31AM -0000, Patchwork wrote:
> > > == Summary ==
> > > 
> > > Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest
> > > 
> > > Test kms_flip:
> > >         Subgroup basic-flip-vs-dpms:
> > >                 pass       -> DMESG-WARN (ilk-hp8440p)
> > 
> > The usual ilk underrun
> > https://bugs.freedesktop.org/show_bug.cgi?id=93787
> > 
> > >         Subgroup basic-flip-vs-modeset:
> > >                 pass       -> DMESG-WARN (skl-i7k-2)
> > 
> > This is the known "DMC doesn't know what state it's in" problem.
> 
> Does it mean that I have to fix it on my patch, or is it something
> expected from this test?

It's unrelated. Hopefully Mika will have applied enough duct tape
soon to hide it better.

> 
> > >                 pass       -> DMESG-WARN (ilk-hp8440p)
> > 
> > Another ilk underrun.
> > 
> > > 
> > > bdw-nuci7        total:140  pass:131  dwarn:0   dfail:0   fail:0   skip:9  
> > > bdw-ultra        total:143  pass:137  dwarn:0   dfail:0   fail:0   skip:6  
> > > bsw-nuc-2        total:143  pass:119  dwarn:0   dfail:0   fail:0   skip:24 
> > > byt-nuc          total:143  pass:128  dwarn:0   dfail:0   fail:0   skip:15 
> > > hsw-brixbox      total:143  pass:136  dwarn:0   dfail:0   fail:0   skip:7  
> > > ilk-hp8440p      total:143  pass:102  dwarn:3   dfail:0   fail:0   skip:38 
> > > ivb-t430s        total:143  pass:137  dwarn:0   dfail:0   fail:0   skip:6  
> > > skl-i5k-2        total:143  pass:134  dwarn:1   dfail:0   fail:0   skip:8  
> > > skl-i7k-2        total:143  pass:133  dwarn:2   dfail:0   fail:0   skip:8  
> > > snb-dellxps      total:143  pass:129  dwarn:0   dfail:0   fail:0   skip:14 
> > > snb-x220t        total:143  pass:129  dwarn:0   dfail:0   fail:1   skip:13 
> > > 
> > > Results at /archive/results/CI_IGT_test/Patchwork_1244/
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v10 3/4] drm/i915: Set aux.dev to the drm_connector device, instead of drm_device.

[Daniel Vetter]

On Fri, Feb 12, 2016 at 10:28:28PM +0200, Imre Deak wrote:
> On Thu, 2016-01-21 at 15:10 -0800, Rafael Antognolli wrote:
> > 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.
> 
> Yes, having the i2c-dev only at one place under the connector is more
> logical and what we want imo. It's an ABI change but if other drivers
> already have it in this way then I assume it's fine. For HDMI
> connectors we still have them under the drm_device (and there is no
> symlink for them under the connector device) but this was inconsistent
> already before this patch and can be fixed up later.
> 
> Adding Jani, in case he has comments on this.
> 
> Below one more comment:
> 
> > v9:
> >  - As a side effect, drm_dp_aux_unregister() must be called before
> >  intel_connector_unregister(), as both the aux.dev and the i2c
> > adapter
> >  dev are children of the drm_connector device now. Calling
> >  drm_dp_aux_unregister() before prevents them from being destroyed
> >  twice.
> > 
> > v10:
> >  - move aux_fini() to connector_unregister(), instead of moving
> >  drm_dp_aux_unregister() outside of connector_register().
> > 
> > Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 18 ++----------------
> >  1 file changed, 2 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index e2bea710..da704c6 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1188,7 +1188,6 @@ intel_dp_aux_fini(struct intel_dp *intel_dp)
> >  static int
> >  intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector
> > *connector)
> >  {
> > -	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >  	struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> >  	enum port port = intel_dig_port->port;
> >  	int ret;
> > @@ -1199,7 +1198,7 @@ intel_dp_aux_init(struct intel_dp *intel_dp,
> > struct intel_connector *connector)
> >  	if (!intel_dp->aux.name)
> >  		return -ENOMEM;
> >  
> > -	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",
> > @@ -1214,16 +1213,6 @@ intel_dp_aux_init(struct intel_dp *intel_dp,
> > struct intel_connector *connector)
> >  		return ret;
> >  	}
> >  
> > -	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",
> > -			  intel_dp->aux.name, ret);
> > -		intel_dp_aux_fini(intel_dp);
> > -		return ret;
> > -	}
> > -
> >  	return 0;
> >  }
> >  
> > @@ -1232,9 +1221,7 @@ 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_dp_aux_fini(intel_dp);
> 
> Hrm, the mst_port check seems to have been misplaced at some point,
> this function isn't called for virtual MST connectors. So I think it's
> fine to remove it. The patch looks ok:
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Added to drm-misc, thanks.
-Daniel

> 
> >  	intel_connector_unregister(intel_connector);
> >  }
> >  
> > @@ -4868,7 +4855,6 @@ void intel_dp_encoder_destroy(struct
> > drm_encoder *encoder)
> >  	struct intel_digital_port *intel_dig_port =
> > enc_to_dig_port(encoder);
> >  	struct intel_dp *intel_dp = &intel_dig_port->dp;
> >  
> > -	intel_dp_aux_fini(intel_dp);
> >  	intel_dp_mst_encoder_cleanup(intel_dig_port);
> >  	if (is_edp(intel_dp)) {
> >  		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx