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/[email protected]/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 <[email protected]>
---
 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

_______________________________________________
pvrusb2 mailing list
[email protected]
http://www.isely.net/cgi-bin/mailman/listinfo/pvrusb2

Reply via email to