Re: [PATCH/RFC v2 2/4] v4l: Add metadata video device type

2016-05-12 Thread Sakari Ailus 
Hi Laurent,

Thanks for the patchset!

On Thu, May 12, 2016 at 03:18:01AM +0300, Laurent Pinchart wrote:
> The metadata video device is used to transfer metadata between
> userspace and kernelspace. It supports the metadata buffer type only.

I remember we briefly discussed this before but I have to bring this up
again: do you think we need a different device node type? It depends purely
on the use case for which purpose a DMA engine is used for. The receiver
hardware doesn't even really know whether the data is image data or
metadata; the hardware may well simply write all of it to memory and that's
it.

Should we use a different device node type for metadata, every driver which
uses sensors connected over the CSI-2 bus --- that hardware is designed to
receive metadata using a separate data type which is part of the same stream
--- would be required to create one node for image data and another for
metadata, essentially doubling the number of data device nodes.

Doubling doesn't necessarily sound bad, but modern devices tend to have tens
of DMA engines already.

For what it's worth, multi-plane buffers use video devices as well so this
is not the first time a type of a device node would make use of multiple
buffer types.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH/RFC v2 2/4] v4l: Add metadata video device type

2016-05-11 Thread Laurent Pinchart 
The metadata video device is used to transfer metadata between
userspace and kernelspace. It supports the metadata buffer type only.

Signed-off-by: Laurent Pinchart 
---
 Documentation/DocBook/media/v4l/dev-meta.xml | 10 +++---
 drivers/media/v4l2-core/v4l2-dev.c   | 21 -
 drivers/media/v4l2-core/v4l2-ioctl.c | 15 ++-
 include/media/v4l2-dev.h |  3 ++-
 include/uapi/linux/media.h   |  2 ++
 5 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/dev-meta.xml 
b/Documentation/DocBook/media/v4l/dev-meta.xml
index 9b5b1fba2007..75bd22521af7 100644
--- a/Documentation/DocBook/media/v4l/dev-meta.xml
+++ b/Documentation/DocBook/media/v4l/dev-meta.xml
@@ -14,9 +14,13 @@ intended for transfer of metadata to userspace and control 
of that operation.
   
 
   
-The metadata interface is implemented on video capture devices. The device can
-be dedicated to metadata or can implement both video and metadata capture as
-specified in its reported capabilities.
+The metadata interface can be implemented on video capture devices, metadata
+devices or both, at the discretion of drivers. Metadata devices are accessed
+through character device special files named
+/dev/v4l-meta[0-9]+ with major number 81 and dynamically
+allocated minor numbers. Video devices that support metadata capture can be
+dedicated to metadata or can implement both metadata capture and video capture
+and/or output, as specified in the device's reported capabilities.
   
 
   
diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
b/drivers/media/v4l2-core/v4l2-dev.c
index 74b79e60ac38..5a8d7b03ab97 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -527,6 +527,7 @@ static void determine_valid_ioctls(struct video_device 
*vdev)
bool is_vbi = vdev->vfl_type == VFL_TYPE_VBI;
bool is_radio = vdev->vfl_type == VFL_TYPE_RADIO;
bool is_sdr = vdev->vfl_type == VFL_TYPE_SDR;
+   bool is_meta = vdev->vfl_type == VFL_TYPE_META;
bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
 
@@ -664,9 +665,18 @@ static void determine_valid_ioctls(struct video_device 
*vdev)
set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
if (ops->vidioc_try_fmt_sdr_out)
set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
+   } else if (is_meta && is_rx) {
+   if (ops->vidioc_enum_fmt_meta_cap)
+   set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
+   if (ops->vidioc_g_fmt_meta_cap)
+   set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
+   if (ops->vidioc_s_fmt_meta_cap)
+   set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
+   if (ops->vidioc_try_fmt_meta_cap)
+   set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
}
 
-   if (is_vid || is_vbi || is_sdr) {
+   if (is_vid || is_vbi || is_sdr || is_meta) {
/* ioctls valid for video, metadata, vbi or sdr */
SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
@@ -767,6 +777,10 @@ static int video_register_media_controller(struct 
video_device *vdev, int type)
intf_type = MEDIA_INTF_T_V4L_SUBDEV;
/* Entity will be created via v4l2_device_register_subdev() */
break;
+   case VFL_TYPE_META:
+   intf_type = MEDIA_INTF_T_V4L_META;
+   vdev->entity.function = MEDIA_ENT_F_IO_META;
+   break;
default:
return 0;
}
@@ -849,6 +863,8 @@ static int video_register_media_controller(struct 
video_device *vdev, int type)
  * %VFL_TYPE_SUBDEV - A subdevice
  *
  * %VFL_TYPE_SDR - Software Defined Radio
+ *
+ * %VFL_TYPE_META - Metadata
  */
 int __video_register_device(struct video_device *vdev, int type, int nr,
int warn_if_nr_in_use, struct module *owner)
@@ -892,6 +908,9 @@ int __video_register_device(struct video_device *vdev, int 
type, int nr,
/* Use device name 'swradio' because 'sdr' was already taken. */
name_base = "swradio";
break;
+   case VFL_TYPE_META:
+   name_base = "v4l-meta";
+   break;
default:
printk(KERN_ERR "%s called with unknown type: %d\n",
   __func__, type);
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
b/drivers/media/v4l2-core/v4l2-ioctl.c
index 5d003152ff68..256938fff9e0 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -935,6 +935,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type 
type)
bool is_vid = vfd->vfl_type ==