Re: [RFCv1 PATCH] Expose controls in debugfs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Friday, March 02, 2012 14:36:16 Hans Verkuil wrote:
> Hi all,
> 
> I've resumed work on something that was discussed ages ago: exposing controls
> in debugfs. It's quite useful in pvrusb2, but that code is completely custom.
> Creating a general method for this is the goal of this patch.
> 
> The old thread can be found here:
> 
> http://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg17453.html
> 
> Although the original proposal in that thread was to do it in sysfs, the
> overall preference was to use debugfs instead.
> 
> This patch is quite simple and does not touch any driver code. All drivers
> that use the V4L2 control framework (which sadly does not include pvrusb2
> at the moment) will get it for free.
> 
> It even implements poll(), so you can poll on a control waiting for the value
> to change. It also honours the priority setting (VIDIOC_G/S_PRIORITY).
> 
> The control type information is not exposed. As mentioned in the thread above,
> I think adding debugfs entries with the type information will make it very messy.
> 
> One possible idea is to have the control files support VIDIOC_QUERYCTRL and
> VIDIOC_QUERYMENU and to create a small utility that will read out that
> information and output it in a manner that can be easily parsed. It would be
> trivial to do this.
> 
> This patch should also work fine for v4l2-subdevX nodes, although I don't
> have the hardware at the moment to check this.
> 
> The git tree containing this patch is here:
> 
> http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/debugfs
> 
> This patch only exposes controls, it does not add controls that emulate V4L2
> ioctls (as pvrusb2 does). I would have to experiment with that a bit. I'm not
> convinced it is that useful (as opposed to calling v4l2-ctl).

OK, here is a patch that implements a v4l2_input control as a front-end for
VIDIOC_G/S_INPUT and a v4l2_frequency control as a front-end for G/S_FREQUENCY.

This code doesn't touch any drivers, it is all done by the V4L2 core.

Right now the code that handles these two controls is part of v4l2-ctrls.c.
If we decide to go ahead with this, then this will be moved to a separate
source and it will also be put under a config option.

So for the vivi driver you have the following if debugfs is mounted under
/sys/kernel/debug:

$ l /sys/kernel/debug/video4linux/video1/controls/
bitmask  brightness  contrast  gain_automatic  integer_32_bits  menu        string      volume
boolean  button      gain      hue             integer_64_bits  saturation  v4l2_input

For ivtv it will look like this:

$ l /sys/kernel/debug/video4linux/video0/controls/
balance                           mpeg_insert_navigation_packets     mpeg_video_bitrate
brightness                        mpeg_median_chroma_filter_maximum  mpeg_video_bitrate_mode
chroma_agc                        mpeg_median_chroma_filter_minimum  mpeg_video_decoder_frame_count
chroma_gain                       mpeg_median_filter_type            mpeg_video_decoder_pts
contrast                          mpeg_median_luma_filter_maximum    mpeg_video_encoding
hue                               mpeg_median_luma_filter_minimum    mpeg_video_gop_closure
mpeg_audio_crc                    mpeg_spatial_chroma_filter_type    mpeg_video_gop_size
mpeg_audio_emphasis               mpeg_spatial_filter                mpeg_video_mute
mpeg_audio_encoding               mpeg_spatial_filter_mode           mpeg_video_mute_yuv
mpeg_audio_layer_ii_bitrate       mpeg_spatial_luma_filter_type      mpeg_video_peak_bitrate
mpeg_audio_multilingual_playback  mpeg_stream_type                   mpeg_video_temporal_decimation
mpeg_audio_mute                   mpeg_stream_vbi_format             mute
mpeg_audio_playback               mpeg_temporal_filter               saturation
mpeg_audio_sampling_frequency     mpeg_temporal_filter_mode          v4l2_frequency
mpeg_audio_stereo_mode            mpeg_video_aspect                  v4l2_input
mpeg_audio_stereo_mode_extension  mpeg_video_b_frames                volume

It's magic :-)

As a proof of concept this shows that it is quite easy to do this cleanly.
Whether this is actually something that should be followed-up is another
question.

Comments are welcome!

Regards,

	Hans

Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
---
 drivers/media/video/v4l2-ctrls.c |  164 ++++++++++++++++++++++++++++++--------
 drivers/media/video/v4l2-dev.c   |    5 +
 include/media/v4l2-ctrls.h       |    2 +
 include/media/v4l2-dev.h         |    4 +
 4 files changed, 142 insertions(+), 33 deletions(-)

diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index 4f49643..3cef2e2 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -2408,6 +2408,8 @@ EXPORT_SYMBOL(v4l2_ctrl_poll);
 static const char *get_class_prefix(u32 id)
 {
 	switch (V4L2_CTRL_ID2CLASS(id)) {
+	case 0:
+		return "v4l2_";
 	case V4L2_CTRL_CLASS_USER:
 		return "";
 	case V4L2_CTRL_CLASS_CAMERA:
@@ -2436,6 +2438,82 @@ static void make_debugfs_name(char *src)
 	*dst = 0;
 }
 
+#define DEBUGFS_CID_INPUT	1
+#define DEBUGFS_CID_FREQUENCY	2
+
+static void debugfs_v4l2_read(struct file *filp, struct video_device *vdev,
+		struct v4l2_ctrl *ctrl)
+{
+	switch (ctrl->id) {
+	case DEBUGFS_CID_INPUT:
+		vdev->ioctl_ops->vidioc_g_input(filp, filp->private_data,
+				&ctrl->cur.val);
+		break;
+
+	case DEBUGFS_CID_FREQUENCY: {
+		struct v4l2_frequency freq;
+
+		freq.tuner = 0;
+		freq.type = (vdev->vfl_type == VFL_TYPE_RADIO) ?
+			V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
+		memset(freq.reserved, 0, sizeof(freq.reserved));
+		vdev->ioctl_ops->vidioc_g_frequency(filp, filp->private_data,
+				&freq);
+		ctrl->cur.val = freq.frequency;
+		break;
+	}
+	}
+}
+
+static int debugfs_v4l2_write(struct file *filp, struct video_device *vdev,
+		struct v4l2_ctrl *ctrl)
+{
+	switch (ctrl->id) {
+	case DEBUGFS_CID_INPUT:
+		return vdev->ioctl_ops->vidioc_s_input(filp, filp->private_data,
+				ctrl->cur.val);
+
+	case DEBUGFS_CID_FREQUENCY: {
+		struct v4l2_frequency freq;
+
+		freq.tuner = 0;
+		freq.type = (vdev->vfl_type == VFL_TYPE_RADIO) ?
+			V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
+		freq.frequency = ctrl->cur.val;
+		memset(freq.reserved, 0, sizeof(freq.reserved));
+		return vdev->ioctl_ops->vidioc_s_frequency(filp, filp->private_data,
+				&freq);
+	}
+	}
+	return -EINVAL;
+}
+
+int v4l2_ctrl_debugfs_v4l2_controls(struct video_device *vdev)
+{
+	struct v4l2_ctrl_config cfg = {
+		.type = V4L2_CTRL_TYPE_INTEGER,
+		.max = INT_MAX,
+		.step = 1,
+	};
+	int ret = v4l2_ctrl_handler_init(&vdev->v4l2_hdl, 1);
+
+	if (ret)
+		return ret;
+	if (vdev->ioctl_ops->vidioc_enum_input) {
+		cfg.id = DEBUGFS_CID_INPUT;
+		cfg.name = "input";
+		if (!v4l2_ctrl_new_custom(&vdev->v4l2_hdl, &cfg, NULL))
+			return -ENOMEM;
+	}
+	if (vdev->ioctl_ops->vidioc_s_frequency) {
+		cfg.id = DEBUGFS_CID_FREQUENCY;
+		cfg.name = "frequency";
+		if (!v4l2_ctrl_new_custom(&vdev->v4l2_hdl, &cfg, NULL))
+			return -ENOMEM;
+	}
+	return 0;
+}
+
 static ssize_t debugfs_read(struct file *filp, char __user *buf,
 		size_t sz, loff_t *off)
 {
@@ -2454,6 +2532,9 @@ static ssize_t debugfs_read(struct file *filp, char __user *buf,
 	if (ctrl->flags & V4L2_CTRL_FLAG_WRITE_ONLY)
 		return -EACCES;
 
+	if (ctrl->handler == &vdev->v4l2_hdl)
+		debugfs_v4l2_read(filp, vdev, ctrl);
+
 	v4l2_ctrl_lock(ctrl);
 	/* g_volatile_ctrl will update the current control values */
 	if (ctrl->flags & V4L2_CTRL_FLAG_VOLATILE) {
@@ -2602,6 +2683,8 @@ static ssize_t debugfs_write(struct file *filp, const char __user *buf,
 	}
 	ret = try_or_set_cluster(NULL, master, true);
 	v4l2_ctrl_unlock(ctrl);
+	if (!ret && ctrl->handler == &vdev->v4l2_hdl)
+		ret = debugfs_v4l2_write(filp, vdev, ctrl);
 	return ret ? ret : sz;
 }
 
@@ -2611,7 +2694,8 @@ static int debugfs_open(struct inode *inode, struct file *filp)
 	struct v4l2_ctrl_ref *ref = filp->f_path.dentry->d_inode->i_private;
 	struct video_device *vdev = video_devdata(filp);
 
-	if (!ret && test_bit(V4L2_FL_USES_V4L2_FH, &vdev->flags)) {
+	if (!ret && ref->ctrl->handler != &vdev->v4l2_hdl &&
+			test_bit(V4L2_FL_USES_V4L2_FH, &vdev->flags)) {
 		struct v4l2_fh *fh = filp->private_data;
 		struct v4l2_event_subscription sub = {
 			.type = V4L2_EVENT_CTRL,
@@ -2632,7 +2716,7 @@ static unsigned int debugfs_poll(struct file *file, struct poll_table_struct *wa
 	struct video_device *vdev = video_devdata(file);
 	unsigned int mask = 0;
 
-	if (test_bit(V4L2_FL_USES_V4L2_FH, &vdev->flags)) {
+	if (ctrl->handler != &vdev->v4l2_hdl && test_bit(V4L2_FL_USES_V4L2_FH, &vdev->flags)) {
 		mask = v4l2_ctrl_poll(file, wait);
 
 		if (mask & POLLPRI) {
@@ -2659,6 +2743,44 @@ const struct file_operations debugfs_fops = {
 	.llseek = no_llseek,
 };
 
+
+static int v4l2_ctrl_debugfs_add_control(struct video_device *vdev,
+		struct v4l2_ctrl_ref *ref)
+{
+	struct v4l2_ctrl *ctrl = ref->ctrl;
+	const char *prefix = get_class_prefix(ctrl->id);
+	char name[strlen(ctrl->name) + strlen(prefix) + 1];
+	mode_t mode = S_IRUGO | S_IWUGO;
+	struct dentry *debugfs_file;
+
+	if (ctrl->type == V4L2_CTRL_TYPE_CTRL_CLASS)
+		return 0;
+	if (ctrl->flags & V4L2_CTRL_FLAG_DISABLED)
+		return 0;
+
+	strcpy(name, prefix);
+	strcat(name, ctrl->name);
+	make_debugfs_name(name);
+
+	if (ref->minor < 0)
+		ref->minor = vdev->minor;
+
+	if (ctrl->flags & V4L2_CTRL_FLAG_WRITE_ONLY)
+		mode = S_IWUGO;
+	else if (ctrl->flags & V4L2_CTRL_FLAG_READ_ONLY)
+		mode = S_IRUGO;
+
+	debugfs_file = debugfs_create_file(name,
+			mode, vdev->debugfs_controls, ref,
+			&debugfs_fops);
+	if (IS_ERR_OR_NULL(debugfs_file)) {
+		printk(KERN_ERR "%s: debugfs: could not create %s\n",
+				__func__, name);
+		return -ENODEV;
+	}
+	return 0;
+}
+
 /* Update the debugfs controls if something changed. */
 int v4l2_ctrl_debugfs_update_controls(struct video_device *vdev)
 {
@@ -2681,38 +2803,14 @@ int v4l2_ctrl_debugfs_update_controls(struct video_device *vdev)
 	}
 
 	list_for_each_entry(ref, &hdl->ctrl_refs, node) {
-		struct v4l2_ctrl *ctrl = ref->ctrl;
-		const char *prefix = get_class_prefix(ctrl->id);
-		char name[strlen(ctrl->name) + strlen(prefix) + 1];
-		mode_t mode = S_IRUGO | S_IWUGO;
-		struct dentry *debugfs_file;
-
-		if (ctrl->type == V4L2_CTRL_TYPE_CTRL_CLASS)
-			continue;
-		if (ctrl->flags & V4L2_CTRL_FLAG_DISABLED)
-			continue;
-
-		strcpy(name, prefix);
-		strcat(name, ctrl->name);
-		make_debugfs_name(name);
-
-		if (ref->minor < 0)
-			ref->minor = vdev->minor;
-
-		if (ctrl->flags & V4L2_CTRL_FLAG_WRITE_ONLY)
-			mode = S_IWUGO;
-		else if (ctrl->flags & V4L2_CTRL_FLAG_READ_ONLY)
-			mode = S_IRUGO;
-
-		debugfs_file = debugfs_create_file(name,
-				mode, vdev->debugfs_controls, ref,
-				&debugfs_fops);
-		if (IS_ERR_OR_NULL(debugfs_file)) {
-			printk(KERN_ERR "%s: debugfs: could not create %s\n",
-					__func__, name);
-			err = -ENODEV;
+		err = v4l2_ctrl_debugfs_add_control(vdev, ref);
+		if (err)
+			break;
+	}
+	list_for_each_entry(ref, &vdev->v4l2_hdl.ctrl_refs, node) {
+		err = v4l2_ctrl_debugfs_add_control(vdev, ref);
+		if (err)
 			break;
-		}
 	}
 	if (err) {
 		debugfs_remove_recursive(vdev->debugfs_controls);
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 0a59a75..3050d2e 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -701,6 +701,9 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
 					__func__, video_device_node_name(vdev));
 			goto cleanup;
 		}
+		ret = v4l2_ctrl_debugfs_v4l2_controls(vdev);
+		if (ret)
+			goto cleanup;
 		ret = v4l2_ctrl_debugfs_update_controls(vdev);
 		if (ret)
 			goto cleanup;
@@ -753,6 +756,7 @@ cleanup:
 		debugfs_remove_recursive(vdev->debugfs_devnode);
 		vdev->debugfs_devnode = NULL;
 		vdev->debugfs_controls = NULL;
+		v4l2_ctrl_handler_free(&vdev->v4l2_hdl);
 	}
 	mutex_lock(&videodev_lock);
 	if (vdev->cdev)
@@ -781,6 +785,7 @@ void video_unregister_device(struct video_device *vdev)
 	debugfs_remove_recursive(vdev->debugfs_devnode);
 	vdev->debugfs_devnode = NULL;
 	vdev->debugfs_controls = NULL;
+	v4l2_ctrl_handler_free(&vdev->v4l2_hdl);
 	mutex_lock(&videodev_lock);
 	/* This must be in a critical section to prevent a race with v4l2_open.
 	 * Once this bit has been cleared video_get may never be called again.
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 54c0fcf..9634ce8 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -524,6 +524,8 @@ int v4l2_s_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl,
    Returns 0 if vdev->ctrl_handler == NULL. */
 int v4l2_ctrl_debugfs_update_controls(struct video_device *vdev);
 
+int v4l2_ctrl_debugfs_v4l2_controls(struct video_device *vdev);
+
 
 /* Helpers for subdevices. If the associated ctrl_handler == NULL then they
    will all return -EINVAL. */
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 6f21386..df1872d 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -17,6 +17,7 @@
 #include <linux/videodev2.h>
 
 #include <media/media-entity.h>
+#include <media/v4l2-ctrls.h>
 
 #define VIDEO_MAJOR	81
 
@@ -94,6 +95,9 @@ struct video_device
 	/* Control handler associated with this device node. May be NULL. */
 	struct v4l2_ctrl_handler *ctrl_handler;
 
+	/* Control handler containing controls that map to V4L2 ioctls. */
+	struct v4l2_ctrl_handler v4l2_hdl;
+
 	/* debugfs directories */
 	struct dentry *debugfs_devnode;
 	struct dentry *debugfs_controls;
-- 
1.7.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux