[PATCH v4 2/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
Thanks Ville for the review, I'm addressing all the issues but have some
questions on the ones mentioned below:
On Tue, Sep 29, 2015 at 05:09:23PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 28, 2015 at 04:45:36PM -0700, Rafael Antognolli wrote:
> > This module is heavily based on i2c-dev. Once loaded, it provides one
> > dev node per DP AUX channel, named drm_dp_auxN, where N is an integer.
> >
> > It's possible to know which connector owns this aux channel by looking
> > at the respective sysfs /sys/class/drm_aux_dev/drm_dp_auxN/connector, if
> > the connector device pointer was correctly set in the aux helper struct.
> >
> > Two main operations are provided on the registers read and write. The
> > address of the register to be read or written is given using lseek. The
> > seek position is updated upon read or write.
> >
> > Signed-off-by: Rafael Antognolli
> > ---
> > drivers/gpu/drm/Kconfig | 8 +
> > drivers/gpu/drm/Makefile | 1 +
> > drivers/gpu/drm/drm_dp_aux_dev.c | 357
> > +++
> > drivers/gpu/drm/drm_dp_helper.c | 5 +
> > include/drm/drm_dp_aux_dev.h | 50 ++
> > 5 files changed, 421 insertions(+)
> > create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c
> > create mode 100644 include/drm/drm_dp_aux_dev.h
> >
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index 1a0a8df..64fa0f4 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -25,6 +25,14 @@ config DRM_MIPI_DSI
> > bool
> > depends on DRM
> >
> > +config DRM_DP_AUX_CHARDEV
> > + bool "DRM DP AUX Interface"
> > + depends on DRM
> > + help
> > + Choose this option to enable a /dev/drm_dp_auxN node that allows to
> > + read and write values to arbitrary DPCD registers on the DP aux
> > + channel.
> > +
> > config DRM_KMS_HELPER
> > tristate
> > depends on DRM
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 45e7719..e5ae7f0 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -25,6 +25,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o
> > drm_probe_helper.o \
> > drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> > drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
> > drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
> > +drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
> >
> > obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
> >
> > diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c
> > b/drivers/gpu/drm/drm_dp_aux_dev.c
> > new file mode 100644
> > index 000..e337081
> > --- /dev/null
> > +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> > @@ -0,0 +1,357 @@
> > +/*
> > + * Copyright © 2015 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the
> > "Software"),
> > + * to deal in the Software without restriction, including without
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > next
> > + * paragraph) shall be included in all copies or substantial portions of
> > the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > DEALINGS
> > + * IN THE SOFTWARE.
> > + *
> > + * Authors:
> > + *Rafael Antognolli
> > + *
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +struct drm_dp_aux_dev {
> > + struct list_head list;
> > + unsigned index;
> > + struct drm_dp_aux *aux;
> > + struct device *dev;
> > +};
> > +
> > +#define DRM_AUX_MINORS 256
> > +static int aux_max_offset = 1 << 20;
>
> People seem to prefer defines for such things.
>
> > +static int drm_dp_aux_dev_count = 0;
> > +static LIST_HEAD(drm_dp_aux_dev_list);
> > +static DEFINE_SPINLOCK(drm_dp_aux_dev_list_lock);
> > +
> > +static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_minor(unsigned index)
> > +{
> > + struct drm_dp_aux_dev *aux_dev;
> > +
> > + spin_lock(_dp_aux_dev_list_lock);
> > + list_for_each_entry(aux_dev, _dp_aux_dev_list, list) {
> > + if (aux_dev->index == index)
[PATCH v4 2/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
On Mon, Sep 28, 2015 at 04:45:36PM -0700, Rafael Antognolli wrote:
> This module is heavily based on i2c-dev. Once loaded, it provides one
> dev node per DP AUX channel, named drm_dp_auxN, where N is an integer.
>
> It's possible to know which connector owns this aux channel by looking
> at the respective sysfs /sys/class/drm_aux_dev/drm_dp_auxN/connector, if
> the connector device pointer was correctly set in the aux helper struct.
>
> Two main operations are provided on the registers read and write. The
> address of the register to be read or written is given using lseek. The
> seek position is updated upon read or write.
>
> Signed-off-by: Rafael Antognolli
> ---
> drivers/gpu/drm/Kconfig | 8 +
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/drm_dp_aux_dev.c | 357
> +++
> drivers/gpu/drm/drm_dp_helper.c | 5 +
> include/drm/drm_dp_aux_dev.h | 50 ++
> 5 files changed, 421 insertions(+)
> create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c
> create mode 100644 include/drm/drm_dp_aux_dev.h
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 1a0a8df..64fa0f4 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -25,6 +25,14 @@ config DRM_MIPI_DSI
> bool
> depends on DRM
>
> +config DRM_DP_AUX_CHARDEV
> + bool "DRM DP AUX Interface"
> + depends on DRM
> + help
> + Choose this option to enable a /dev/drm_dp_auxN node that allows to
> + read and write values to arbitrary DPCD registers on the DP aux
> + channel.
> +
> config DRM_KMS_HELPER
> tristate
> depends on DRM
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 45e7719..e5ae7f0 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -25,6 +25,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o
> drm_probe_helper.o \
> drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
> drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
> +drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
>
> obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
>
> diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c
> b/drivers/gpu/drm/drm_dp_aux_dev.c
> new file mode 100644
> index 000..e337081
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> @@ -0,0 +1,357 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *Rafael Antognolli
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct drm_dp_aux_dev {
> + struct list_head list;
> + unsigned index;
> + struct drm_dp_aux *aux;
> + struct device *dev;
> +};
> +
> +#define DRM_AUX_MINORS 256
> +static int aux_max_offset = 1 << 20;
People seem to prefer defines for such things.
> +static int drm_dp_aux_dev_count = 0;
> +static LIST_HEAD(drm_dp_aux_dev_list);
> +static DEFINE_SPINLOCK(drm_dp_aux_dev_list_lock);
> +
> +static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_minor(unsigned index)
> +{
> + struct drm_dp_aux_dev *aux_dev;
> +
> + spin_lock(_dp_aux_dev_list_lock);
> + list_for_each_entry(aux_dev, _dp_aux_dev_list, list) {
> + if (aux_dev->index == index)
> + goto found;
> + }
> +
> + aux_dev = NULL;
> +found:
> + spin_unlock(_dp_aux_dev_list_lock);
> + return aux_dev;
> +}
> +
> +static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_aux(struct drm_dp_aux
> *aux)
> +{
> + struct drm_dp_aux_dev *aux_dev;
> +
> + spin_lock(_dp_aux_dev_list_lock);
> + list_for_each_entry(aux_dev, _dp_aux_dev_list, list) {
> +
[PATCH v4 2/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
This module is heavily based on i2c-dev. Once loaded, it provides one
dev node per DP AUX channel, named drm_dp_auxN, where N is an integer.
It's possible to know which connector owns this aux channel by looking
at the respective sysfs /sys/class/drm_aux_dev/drm_dp_auxN/connector, if
the connector device pointer was correctly set in the aux helper struct.
Two main operations are provided on the registers read and write. The
address of the register to be read or written is given using lseek. The
seek position is updated upon read or write.
Signed-off-by: Rafael Antognolli
---
drivers/gpu/drm/Kconfig | 8 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/drm_dp_aux_dev.c | 357 +++
drivers/gpu/drm/drm_dp_helper.c | 5 +
include/drm/drm_dp_aux_dev.h | 50 ++
5 files changed, 421 insertions(+)
create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c
create mode 100644 include/drm/drm_dp_aux_dev.h
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 1a0a8df..64fa0f4 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -25,6 +25,14 @@ config DRM_MIPI_DSI
bool
depends on DRM
+config DRM_DP_AUX_CHARDEV
+ bool "DRM DP AUX Interface"
+ depends on DRM
+ help
+ Choose this option to enable a /dev/drm_dp_auxN node that allows to
+ read and write values to arbitrary DPCD registers on the DP aux
+ channel.
+
config DRM_KMS_HELPER
tristate
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 45e7719..e5ae7f0 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -25,6 +25,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o
drm_probe_helper.o \
drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
+drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c
new file mode 100644
index 000..e337081
--- /dev/null
+++ b/drivers/gpu/drm/drm_dp_aux_dev.c
@@ -0,0 +1,357 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Rafael Antognolli
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct drm_dp_aux_dev {
+ struct list_head list;
+ unsigned index;
+ struct drm_dp_aux *aux;
+ struct device *dev;
+};
+
+#define DRM_AUX_MINORS 256
+static int aux_max_offset = 1 << 20;
+static int drm_dp_aux_dev_count = 0;
+static LIST_HEAD(drm_dp_aux_dev_list);
+static DEFINE_SPINLOCK(drm_dp_aux_dev_list_lock);
+
+static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_minor(unsigned index)
+{
+ struct drm_dp_aux_dev *aux_dev;
+
+ spin_lock(_dp_aux_dev_list_lock);
+ list_for_each_entry(aux_dev, _dp_aux_dev_list, list) {
+ if (aux_dev->index == index)
+ goto found;
+ }
+
+ aux_dev = NULL;
+found:
+ spin_unlock(_dp_aux_dev_list_lock);
+ return aux_dev;
+}
+
+static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_aux(struct drm_dp_aux *aux)
+{
+ struct drm_dp_aux_dev *aux_dev;
+
+ spin_lock(_dp_aux_dev_list_lock);
+ list_for_each_entry(aux_dev, _dp_aux_dev_list, list) {
+ if (aux_dev->aux == aux)
+ goto found;
+ }
+
+ aux_dev = NULL;
+found:
+ spin_unlock(_dp_aux_dev_list_lock);
+ return aux_dev;
+}
+
+static struct drm_dp_aux_dev *get_free_drm_dp_aux_dev(struct drm_dp_aux *aux)
+{
+ struct drm_dp_aux_dev *aux_dev;
+ int index;
+
+
