[PATCH 0/5] media: better type handling

[PATCH 0/5] media: better type handling

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This patch series takes Laurent's patch 1/2 and by adding the
device_caps field to video_device it makes it possible to create
a real is_media_entity_v4l2_io() function.

Regards,

	Hans

Hans Verkuil (4):
  v4l2: add device_caps to struct video_device
  v4l2-pci-skeleton.c: fill in device_caps in video_device
  vivid: set device_caps in video_device.
  media-entity.h: rename _io to _video_device and add real _io

Laurent Pinchart (1):
  media: Add type field to struct media_entity

 Documentation/video4linux/v4l2-pci-skeleton.c |  6 +--
 drivers/media/platform/vivid/vivid-core.c     | 22 +++-----
 drivers/media/v4l2-core/v4l2-dev.c            |  1 +
 drivers/media/v4l2-core/v4l2-ioctl.c          |  3 ++
 drivers/media/v4l2-core/v4l2-subdev.c         |  1 +
 include/media/media-entity.h                  | 77 ++++++++++++++-------------
 include/media/v4l2-common.h                   | 28 ++++++++++
 include/media/v4l2-dev.h                      |  3 ++
 8 files changed, 85 insertions(+), 56 deletions(-)

-- 
2.7.0

[PATCH 1/5] v4l2: add device_caps to struct video_device

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Instead of letting drivers fill in device_caps at querycap time,
let them fill it in when the video device is registered.

This has the advantage that in the future the v4l2 core can access
the video device's capabilities and take decisions based on that.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 3 +++
 include/media/v4l2-dev.h             | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 86c4c19..706bb42 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1020,9 +1020,12 @@ static int v4l_querycap(const struct v4l2_ioctl_ops *ops,
 				struct file *file, void *fh, void *arg)
 {
 	struct v4l2_capability *cap = (struct v4l2_capability *)arg;
+	struct video_device *vfd = video_devdata(file);
 	int ret;
 
 	cap->version = LINUX_VERSION_CODE;
+	cap->device_caps = vfd->device_caps;
+	cap->capabilities = vfd->device_caps | V4L2_CAP_DEVICE_CAPS;
 
 	ret = ops->vidioc_querycap(file, fh, cap);
 
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 76056ab..25a3190 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -92,6 +92,9 @@ struct video_device
 	/* device ops */
 	const struct v4l2_file_operations *fops;
 
+	/* device capabilities as used in v4l2_capabilities */
+	u32 device_caps;
+
 	/* sysfs */
 	struct device dev;		/* v4l device */
 	struct cdev *cdev;		/* character device */
-- 
2.7.0

Re: [PATCH 1/5] v4l2: add device_caps to struct video_device

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Monday 29 February 2016 12:45:41 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Instead of letting drivers fill in device_caps at querycap time,
> let them fill it in when the video device is registered.
> 
> This has the advantage that in the future the v4l2 core can access
> the video device's capabilities and take decisions based on that.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 3 +++
>  include/media/v4l2-dev.h             | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c index 86c4c19..706bb42 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1020,9 +1020,12 @@ static int v4l_querycap(const struct v4l2_ioctl_ops
> *ops, struct file *file, void *fh, void *arg)
>  {
>  	struct v4l2_capability *cap = (struct v4l2_capability *)arg;
> +	struct video_device *vfd = video_devdata(file);
>  	int ret;
> 
>  	cap->version = LINUX_VERSION_CODE;
> +	cap->device_caps = vfd->device_caps;
> +	cap->capabilities = vfd->device_caps | V4L2_CAP_DEVICE_CAPS;
> 
>  	ret = ops->vidioc_querycap(file, fh, cap);
> 
> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
> index 76056ab..25a3190 100644
> --- a/include/media/v4l2-dev.h
> +++ b/include/media/v4l2-dev.h
> @@ -92,6 +92,9 @@ struct video_device
>  	/* device ops */
>  	const struct v4l2_file_operations *fops;
> 
> +	/* device capabilities as used in v4l2_capabilities */
> +	u32 device_caps;

How about adding capabilities too (on 64-bit systems we won't lose any space 
as the previous and following fields are 64-bit aligned anyway) ? This would 
allow most, if not all, drivers to drop their vidioc_querycap implementation.

>  	/* sysfs */
>  	struct device dev;		/* v4l device */
>  	struct cdev *cdev;		/* character device */

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/5] v4l2: add device_caps to struct video_device

[Laurent Pinchart]

On Monday 29 February 2016 13:56:10 Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Monday 29 February 2016 12:45:41 Hans Verkuil wrote:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > Instead of letting drivers fill in device_caps at querycap time,
> > let them fill it in when the video device is registered.
> > 
> > This has the advantage that in the future the v4l2 core can access
> > the video device's capabilities and take decisions based on that.
> > 
> > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > ---
> > 
> >  drivers/media/v4l2-core/v4l2-ioctl.c | 3 +++
> >  include/media/v4l2-dev.h             | 3 +++
> >  2 files changed, 6 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> > b/drivers/media/v4l2-core/v4l2-ioctl.c index 86c4c19..706bb42 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1020,9 +1020,12 @@ static int v4l_querycap(const struct v4l2_ioctl_ops
> > *ops, struct file *file, void *fh, void *arg)
> > 
> >  {
> >  
> >  	struct v4l2_capability *cap = (struct v4l2_capability *)arg;
> > 
> > +	struct video_device *vfd = video_devdata(file);
> > 
> >  	int ret;
> >  	
> >  	cap->version = LINUX_VERSION_CODE;
> > 
> > +	cap->device_caps = vfd->device_caps;
> > +	cap->capabilities = vfd->device_caps | V4L2_CAP_DEVICE_CAPS;
> > 
> >  	ret = ops->vidioc_querycap(file, fh, cap);
> > 
> > diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
> > index 76056ab..25a3190 100644
> > --- a/include/media/v4l2-dev.h
> > +++ b/include/media/v4l2-dev.h
> > @@ -92,6 +92,9 @@ struct video_device
> > 
> >  	/* device ops */
> >  	const struct v4l2_file_operations *fops;
> > 
> > +	/* device capabilities as used in v4l2_capabilities */
> > +	u32 device_caps;
> 
> How about adding capabilities too (on 64-bit systems we won't lose any space
> as the previous and following fields are 64-bit aligned anyway) ? This
> would allow most, if not all, drivers to drop their vidioc_querycap
> implementation.

It obviously wouldn't as there are other fields to fill. I should wake up 
before sending e-mails.

> >  	/* sysfs */
> >  	struct device dev;		/* v4l device */
> >  	struct cdev *cdev;		/* character device */

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/5] v4l2: add device_caps to struct video_device

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Monday 29 February 2016 12:45:41 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Instead of letting drivers fill in device_caps at querycap time,
> let them fill it in when the video device is registered.
> 
> This has the advantage that in the future the v4l2 core can access
> the video device's capabilities and take decisions based on that.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I've acked the first three patches, could you get them merged through your 
tree ? I'll incorporate patch 4 in my media entity type series and post patch 
5 rebased on top of it.

> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 3 +++
>  include/media/v4l2-dev.h             | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c index 86c4c19..706bb42 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1020,9 +1020,12 @@ static int v4l_querycap(const struct v4l2_ioctl_ops
> *ops, struct file *file, void *fh, void *arg)
>  {
>  	struct v4l2_capability *cap = (struct v4l2_capability *)arg;
> +	struct video_device *vfd = video_devdata(file);
>  	int ret;
> 
>  	cap->version = LINUX_VERSION_CODE;
> +	cap->device_caps = vfd->device_caps;
> +	cap->capabilities = vfd->device_caps | V4L2_CAP_DEVICE_CAPS;
> 
>  	ret = ops->vidioc_querycap(file, fh, cap);
> 
> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
> index 76056ab..25a3190 100644
> --- a/include/media/v4l2-dev.h
> +++ b/include/media/v4l2-dev.h
> @@ -92,6 +92,9 @@ struct video_device
>  	/* device ops */
>  	const struct v4l2_file_operations *fops;
> 
> +	/* device capabilities as used in v4l2_capabilities */
> +	u32 device_caps;
> +
>  	/* sysfs */
>  	struct device dev;		/* v4l device */
>  	struct cdev *cdev;		/* character device */

-- 
Regards,

Laurent Pinchart

[PATCH 2/5] v4l2-pci-skeleton.c: fill in device_caps in video_device

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

With the new core support for the caps the driver no longer needs
to set device_caps and capabilities in the querycap call.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/video4linux/v4l2-pci-skeleton.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/video4linux/v4l2-pci-skeleton.c b/Documentation/video4linux/v4l2-pci-skeleton.c
index 79af0c0..65fae1a 100644
--- a/Documentation/video4linux/v4l2-pci-skeleton.c
+++ b/Documentation/video4linux/v4l2-pci-skeleton.c
@@ -308,9 +308,6 @@ static int skeleton_querycap(struct file *file, void *priv,
 	strlcpy(cap->card, "V4L2 PCI Skeleton", sizeof(cap->card));
 	snprintf(cap->bus_info, sizeof(cap->bus_info), "PCI:%s",
 		 pci_name(skel->pdev));
-	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
-			   V4L2_CAP_STREAMING;
-	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
 
@@ -872,6 +869,9 @@ static int skeleton_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	vdev->release = video_device_release_empty;
 	vdev->fops = &skel_fops,
 	vdev->ioctl_ops = &skel_ioctl_ops,
+	/* Fill in the device caps */
+	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
+			    V4L2_CAP_STREAMING;
 	/*
 	 * The main serialization lock. All ioctls are serialized by this
 	 * lock. Exception: if q->lock is set, then the streaming ioctls
-- 
2.7.0

Re: [PATCH 2/5] v4l2-pci-skeleton.c: fill in device_caps in video_device

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Monday 29 February 2016 12:45:42 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> With the new core support for the caps the driver no longer needs
> to set device_caps and capabilities in the querycap call.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  Documentation/video4linux/v4l2-pci-skeleton.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/video4linux/v4l2-pci-skeleton.c
> b/Documentation/video4linux/v4l2-pci-skeleton.c index 79af0c0..65fae1a
> 100644
> --- a/Documentation/video4linux/v4l2-pci-skeleton.c
> +++ b/Documentation/video4linux/v4l2-pci-skeleton.c
> @@ -308,9 +308,6 @@ static int skeleton_querycap(struct file *file, void
> *priv, strlcpy(cap->card, "V4L2 PCI Skeleton", sizeof(cap->card));
>  	snprintf(cap->bus_info, sizeof(cap->bus_info), "PCI:%s",
>  		 pci_name(skel->pdev));
> -	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
> -			   V4L2_CAP_STREAMING;
> -	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
>  	return 0;
>  }
> 
> @@ -872,6 +869,9 @@ static int skeleton_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent) vdev->release = video_device_release_empty;
>  	vdev->fops = &skel_fops,
>  	vdev->ioctl_ops = &skel_ioctl_ops,
> +	/* Fill in the device caps */

The comment seems a bit overkill to me, the next line is pretty clear. Apart 
from that,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
> +			    V4L2_CAP_STREAMING;
>  	/*
>  	 * The main serialization lock. All ioctls are serialized by this
>  	 * lock. Exception: if q->lock is set, then the streaming ioctls

-- 
Regards,

Laurent Pinchart

[PATCH 3/5] vivid: set device_caps in video_device.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This simplifies the querycap function.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
#
#WARNING: line over 80 characters
##48: FILE: include/media/v4l2-common.h:194:
#+ * is_media_entity_v4l2_io() - Check if the entity is a video_device and can do I/O
#
#total: 0 errors, 1 warnings, 64 lines checked
#
#Your patch has style problems, please review.
#
#NOTE: If any of the errors are false positives, please report
#      them to the maintainer, see CHECKPATCH in MAINTAINERS.
---
 drivers/media/platform/vivid/vivid-core.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c
index ec125bec..c14da84 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -200,27 +200,12 @@ static int vidioc_querycap(struct file *file, void  *priv,
 					struct v4l2_capability *cap)
 {
 	struct vivid_dev *dev = video_drvdata(file);
-	struct video_device *vdev = video_devdata(file);
 
 	strcpy(cap->driver, "vivid");
 	strcpy(cap->card, "vivid");
 	snprintf(cap->bus_info, sizeof(cap->bus_info),
 			"platform:%s", dev->v4l2_dev.name);
 
-	if (vdev->vfl_type == VFL_TYPE_GRABBER && vdev->vfl_dir == VFL_DIR_RX)
-		cap->device_caps = dev->vid_cap_caps;
-	if (vdev->vfl_type == VFL_TYPE_GRABBER && vdev->vfl_dir == VFL_DIR_TX)
-		cap->device_caps = dev->vid_out_caps;
-	else if (vdev->vfl_type == VFL_TYPE_VBI && vdev->vfl_dir == VFL_DIR_RX)
-		cap->device_caps = dev->vbi_cap_caps;
-	else if (vdev->vfl_type == VFL_TYPE_VBI && vdev->vfl_dir == VFL_DIR_TX)
-		cap->device_caps = dev->vbi_out_caps;
-	else if (vdev->vfl_type == VFL_TYPE_SDR)
-		cap->device_caps = dev->sdr_cap_caps;
-	else if (vdev->vfl_type == VFL_TYPE_RADIO && vdev->vfl_dir == VFL_DIR_RX)
-		cap->device_caps = dev->radio_rx_caps;
-	else if (vdev->vfl_type == VFL_TYPE_RADIO && vdev->vfl_dir == VFL_DIR_TX)
-		cap->device_caps = dev->radio_tx_caps;
 	cap->capabilities = dev->vid_cap_caps | dev->vid_out_caps |
 		dev->vbi_cap_caps | dev->vbi_out_caps |
 		dev->radio_rx_caps | dev->radio_tx_caps |
@@ -1135,6 +1120,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		strlcpy(vfd->name, "vivid-vid-cap", sizeof(vfd->name));
 		vfd->fops = &vivid_fops;
 		vfd->ioctl_ops = &vivid_ioctl_ops;
+		vfd->device_caps = dev->vid_cap_caps;
 		vfd->release = video_device_release_empty;
 		vfd->v4l2_dev = &dev->v4l2_dev;
 		vfd->queue = &dev->vb_vid_cap_q;
@@ -1160,6 +1146,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		vfd->vfl_dir = VFL_DIR_TX;
 		vfd->fops = &vivid_fops;
 		vfd->ioctl_ops = &vivid_ioctl_ops;
+		vfd->device_caps = dev->vid_out_caps;
 		vfd->release = video_device_release_empty;
 		vfd->v4l2_dev = &dev->v4l2_dev;
 		vfd->queue = &dev->vb_vid_out_q;
@@ -1184,6 +1171,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		strlcpy(vfd->name, "vivid-vbi-cap", sizeof(vfd->name));
 		vfd->fops = &vivid_fops;
 		vfd->ioctl_ops = &vivid_ioctl_ops;
+		vfd->device_caps = dev->vbi_cap_caps;
 		vfd->release = video_device_release_empty;
 		vfd->v4l2_dev = &dev->v4l2_dev;
 		vfd->queue = &dev->vb_vbi_cap_q;
@@ -1207,6 +1195,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		vfd->vfl_dir = VFL_DIR_TX;
 		vfd->fops = &vivid_fops;
 		vfd->ioctl_ops = &vivid_ioctl_ops;
+		vfd->device_caps = dev->vbi_out_caps;
 		vfd->release = video_device_release_empty;
 		vfd->v4l2_dev = &dev->v4l2_dev;
 		vfd->queue = &dev->vb_vbi_out_q;
@@ -1229,6 +1218,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		strlcpy(vfd->name, "vivid-sdr-cap", sizeof(vfd->name));
 		vfd->fops = &vivid_fops;
 		vfd->ioctl_ops = &vivid_ioctl_ops;
+		vfd->device_caps = dev->sdr_cap_caps;
 		vfd->release = video_device_release_empty;
 		vfd->v4l2_dev = &dev->v4l2_dev;
 		vfd->queue = &dev->vb_sdr_cap_q;
@@ -1247,6 +1237,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		strlcpy(vfd->name, "vivid-rad-rx", sizeof(vfd->name));
 		vfd->fops = &vivid_radio_fops;
 		vfd->ioctl_ops = &vivid_ioctl_ops;
+		vfd->device_caps = dev->radio_rx_caps;
 		vfd->release = video_device_release_empty;
 		vfd->v4l2_dev = &dev->v4l2_dev;
 		vfd->lock = &dev->mutex;
@@ -1265,6 +1256,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		vfd->vfl_dir = VFL_DIR_TX;
 		vfd->fops = &vivid_radio_fops;
 		vfd->ioctl_ops = &vivid_ioctl_ops;
+		vfd->device_caps = dev->radio_tx_caps;
 		vfd->release = video_device_release_empty;
 		vfd->v4l2_dev = &dev->v4l2_dev;
 		vfd->lock = &dev->mutex;
-- 
2.7.0

Re: [PATCH 3/5] vivid: set device_caps in video_device.

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Monday 29 February 2016 12:45:43 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This simplifies the querycap function.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> #
> #WARNING: line over 80 characters
> ##48: FILE: include/media/v4l2-common.h:194:
> #+ * is_media_entity_v4l2_io() - Check if the entity is a video_device and
> can do I/O #
> #total: 0 errors, 1 warnings, 64 lines checked
> #
> #Your patch has style problems, please review.
> #
> #NOTE: If any of the errors are false positives, please report
> #      them to the maintainer, see CHECKPATCH in MAINTAINERS.
> ---
>  drivers/media/platform/vivid/vivid-core.c | 22 +++++++---------------
>  1 file changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/platform/vivid/vivid-core.c
> b/drivers/media/platform/vivid/vivid-core.c index ec125bec..c14da84 100644
> --- a/drivers/media/platform/vivid/vivid-core.c
> +++ b/drivers/media/platform/vivid/vivid-core.c
> @@ -200,27 +200,12 @@ static int vidioc_querycap(struct file *file, void 
> *priv, struct v4l2_capability *cap)
>  {
>  	struct vivid_dev *dev = video_drvdata(file);
> -	struct video_device *vdev = video_devdata(file);
> 
>  	strcpy(cap->driver, "vivid");
>  	strcpy(cap->card, "vivid");
>  	snprintf(cap->bus_info, sizeof(cap->bus_info),
>  			"platform:%s", dev->v4l2_dev.name);
> 
> -	if (vdev->vfl_type == VFL_TYPE_GRABBER && vdev->vfl_dir == VFL_DIR_RX)
> -		cap->device_caps = dev->vid_cap_caps;
> -	if (vdev->vfl_type == VFL_TYPE_GRABBER && vdev->vfl_dir == VFL_DIR_TX)
> -		cap->device_caps = dev->vid_out_caps;
> -	else if (vdev->vfl_type == VFL_TYPE_VBI && vdev->vfl_dir == VFL_DIR_RX)
> -		cap->device_caps = dev->vbi_cap_caps;
> -	else if (vdev->vfl_type == VFL_TYPE_VBI && vdev->vfl_dir == VFL_DIR_TX)
> -		cap->device_caps = dev->vbi_out_caps;
> -	else if (vdev->vfl_type == VFL_TYPE_SDR)
> -		cap->device_caps = dev->sdr_cap_caps;
> -	else if (vdev->vfl_type == VFL_TYPE_RADIO && vdev->vfl_dir == VFL_DIR_RX)
> -		cap->device_caps = dev->radio_rx_caps;
> -	else if (vdev->vfl_type == VFL_TYPE_RADIO && vdev->vfl_dir == VFL_DIR_TX)
> -		cap->device_caps = dev->radio_tx_caps;
>  	cap->capabilities = dev->vid_cap_caps | dev->vid_out_caps |
>  		dev->vbi_cap_caps | dev->vbi_out_caps |
>  		dev->radio_rx_caps | dev->radio_tx_caps |
> @@ -1135,6 +1120,7 @@ static int vivid_create_instance(struct
> platform_device *pdev, int inst) strlcpy(vfd->name, "vivid-vid-cap",
> sizeof(vfd->name));
>  		vfd->fops = &vivid_fops;
>  		vfd->ioctl_ops = &vivid_ioctl_ops;
> +		vfd->device_caps = dev->vid_cap_caps;
>  		vfd->release = video_device_release_empty;
>  		vfd->v4l2_dev = &dev->v4l2_dev;
>  		vfd->queue = &dev->vb_vid_cap_q;
> @@ -1160,6 +1146,7 @@ static int vivid_create_instance(struct
> platform_device *pdev, int inst) vfd->vfl_dir = VFL_DIR_TX;
>  		vfd->fops = &vivid_fops;
>  		vfd->ioctl_ops = &vivid_ioctl_ops;
> +		vfd->device_caps = dev->vid_out_caps;
>  		vfd->release = video_device_release_empty;
>  		vfd->v4l2_dev = &dev->v4l2_dev;
>  		vfd->queue = &dev->vb_vid_out_q;
> @@ -1184,6 +1171,7 @@ static int vivid_create_instance(struct
> platform_device *pdev, int inst) strlcpy(vfd->name, "vivid-vbi-cap",
> sizeof(vfd->name));
>  		vfd->fops = &vivid_fops;
>  		vfd->ioctl_ops = &vivid_ioctl_ops;
> +		vfd->device_caps = dev->vbi_cap_caps;
>  		vfd->release = video_device_release_empty;
>  		vfd->v4l2_dev = &dev->v4l2_dev;
>  		vfd->queue = &dev->vb_vbi_cap_q;
> @@ -1207,6 +1195,7 @@ static int vivid_create_instance(struct
> platform_device *pdev, int inst) vfd->vfl_dir = VFL_DIR_TX;
>  		vfd->fops = &vivid_fops;
>  		vfd->ioctl_ops = &vivid_ioctl_ops;
> +		vfd->device_caps = dev->vbi_out_caps;
>  		vfd->release = video_device_release_empty;
>  		vfd->v4l2_dev = &dev->v4l2_dev;
>  		vfd->queue = &dev->vb_vbi_out_q;
> @@ -1229,6 +1218,7 @@ static int vivid_create_instance(struct
> platform_device *pdev, int inst) strlcpy(vfd->name, "vivid-sdr-cap",
> sizeof(vfd->name));
>  		vfd->fops = &vivid_fops;
>  		vfd->ioctl_ops = &vivid_ioctl_ops;
> +		vfd->device_caps = dev->sdr_cap_caps;
>  		vfd->release = video_device_release_empty;
>  		vfd->v4l2_dev = &dev->v4l2_dev;
>  		vfd->queue = &dev->vb_sdr_cap_q;
> @@ -1247,6 +1237,7 @@ static int vivid_create_instance(struct
> platform_device *pdev, int inst) strlcpy(vfd->name, "vivid-rad-rx",
> sizeof(vfd->name));
>  		vfd->fops = &vivid_radio_fops;
>  		vfd->ioctl_ops = &vivid_ioctl_ops;
> +		vfd->device_caps = dev->radio_rx_caps;
>  		vfd->release = video_device_release_empty;
>  		vfd->v4l2_dev = &dev->v4l2_dev;
>  		vfd->lock = &dev->mutex;
> @@ -1265,6 +1256,7 @@ static int vivid_create_instance(struct
> platform_device *pdev, int inst) vfd->vfl_dir = VFL_DIR_TX;
>  		vfd->fops = &vivid_radio_fops;
>  		vfd->ioctl_ops = &vivid_ioctl_ops;
> +		vfd->device_caps = dev->radio_tx_caps;
>  		vfd->release = video_device_release_empty;
>  		vfd->v4l2_dev = &dev->v4l2_dev;
>  		vfd->lock = &dev->mutex;

-- 
Regards,

Laurent Pinchart

[PATCH 4/5] media: Add type field to struct media_entity

[Hans Verkuil]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Code that processes media entities can require knowledge of the
structure type that embeds a particular media entity instance in order
to cast the entity to the proper object type. This needs is shown by the
presence of the is_media_entity_v4l2_io and is_media_entity_v4l2_subdev
functions.

The implementation of those two functions relies on the entity function
field, which is both a wrong and an inefficient design, without even
mentioning the maintenance issue involved in updating the functions
every time a new entity function is added. Fix this by adding add a type
field to the media entity structure to carry the information.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-dev.c    |  1 +
 drivers/media/v4l2-core/v4l2-subdev.c |  1 +
 include/media/media-entity.h          | 75 ++++++++++++++++++-----------------
 3 files changed, 40 insertions(+), 37 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index d8e5994..7e766a9 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -735,6 +735,7 @@ static int video_register_media_controller(struct video_device *vdev, int type)
 	if (!vdev->v4l2_dev->mdev)
 		return 0;
 
+	vdev->entity.type = MEDIA_ENTITY_TYPE_VIDEO_DEVICE;
 	vdev->entity.function = MEDIA_ENT_F_UNKNOWN;
 
 	switch (type) {
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index d630838..bb6e79f 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -584,6 +584,7 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
 	sd->host_priv = NULL;
 #if defined(CONFIG_MEDIA_CONTROLLER)
 	sd->entity.name = sd->name;
+	sd->entity.type = MEDIA_ENTITY_TYPE_V4L2_SUBDEV;
 	sd->entity.function = MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN;
 #endif
 }
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index d58e29d..cbd3753 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -187,10 +187,37 @@ struct media_entity_operations {
 };
 
 /**
+ * enum media_entity_type - Media entity type
+ *
+ * @MEDIA_ENTITY_TYPE_MEDIA_ENTITY:
+ *	The entity is a struct media_entity instance.
+ * @MEDIA_ENTITY_TYPE_VIDEO_DEVICE:
+ *	The entity is a struct video_device instance.
+ * @MEDIA_ENTITY_TYPE_V4L2_SUBDEV:
+ *	The entity is a struct v4l2_subdev instance.
+ *
+ * The entity type identifies the type of the object instance that implements
+ * the struct media_entity instance. This allows runtime type identification of
+ * media entities and safe casting to the project object type. For instance, a
+ * media entity structure instance embedded in a v4l2_subdev structure instance
+ * will have the type MEDIA_ENTITY_TYPE_V4L2_SUBDEV and can safely be cast to a
+ * v4l2_subdev structure using the container_of() macro.
+ *
+ * Media entities can be instantiated without creating any derived object type,
+ * in which case their type will be MEDIA_ENTITY_TYPE_MEDIA_ENTITY.
+ */
+enum media_entity_type {
+	MEDIA_ENTITY_TYPE_MEDIA_ENTITY,
+	MEDIA_ENTITY_TYPE_VIDEO_DEVICE,
+	MEDIA_ENTITY_TYPE_V4L2_SUBDEV,
+};
+
+/**
  * struct media_entity - A media entity graph object.
  *
  * @graph_obj:	Embedded structure containing the media object common data.
  * @name:	Entity name.
+ * @type:	Type of the object that implements the media_entity.
  * @function:	Entity main function, as defined in uapi/media.h
  *		(MEDIA_ENT_F_*)
  * @flags:	Entity flags, as defined in uapi/media.h (MEDIA_ENT_FL_*)
@@ -219,6 +246,7 @@ struct media_entity_operations {
 struct media_entity {
 	struct media_gobj graph_obj;	/* must be first field in struct */
 	const char *name;
+	enum media_entity_type type;
 	u32 function;
 	unsigned long flags;
 
@@ -328,56 +356,29 @@ static inline u32 media_gobj_gen_id(enum media_gobj_type type, u64 local_id)
 }
 
 /**
- * is_media_entity_v4l2_io() - identify if the entity main function
- *			       is a V4L2 I/O
- *
+ * is_media_entity_v4l2_io() - Check if the entity is a video_device
  * @entity:	pointer to entity
  *
- * Return: true if the entity main function is one of the V4L2 I/O types
- *	(video, VBI or SDR radio); false otherwise.
+ * Return: true if the entity is an instance of a video_device object and can
+ * safely be cast to a struct video_device using the container_of() macro, or
+ * false otherwise.
  */
 static inline bool is_media_entity_v4l2_io(struct media_entity *entity)
 {
-	if (!entity)
-		return false;
-
-	switch (entity->function) {
-	case MEDIA_ENT_F_IO_V4L:
-	case MEDIA_ENT_F_IO_VBI:
-	case MEDIA_ENT_F_IO_SWRADIO:
-		return true;
-	default:
-		return false;
-	}
+	return entity && entity->type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE;
 }
 
 /**
- * is_media_entity_v4l2_subdev - return true if the entity main function is
- *				 associated with the V4L2 API subdev usage
- *
+ * is_media_entity_v4l2_subdev() - Check if the entity is a v4l2_subdev
  * @entity:	pointer to entity
  *
- * This is an ancillary function used by subdev-based V4L2 drivers.
- * It checks if the entity function is one of functions used by a V4L2 subdev,
- * e. g. camera-relatef functions, analog TV decoder, TV tuner, V4L2 DSPs.
+ * Return: true if the entity is an instance of a v4l2_subdev object and can
+ * safely be cast to a struct v4l2_subdev using the container_of() macro, or
+ * false otherwise.
  */
 static inline bool is_media_entity_v4l2_subdev(struct media_entity *entity)
 {
-	if (!entity)
-		return false;
-
-	switch (entity->function) {
-	case MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN:
-	case MEDIA_ENT_F_CAM_SENSOR:
-	case MEDIA_ENT_F_FLASH:
-	case MEDIA_ENT_F_LENS:
-	case MEDIA_ENT_F_ATV_DECODER:
-	case MEDIA_ENT_F_TUNER:
-		return true;
-
-	default:
-		return false;
-	}
+	return entity && entity->type == MEDIA_ENTITY_TYPE_V4L2_SUBDEV;
 }
 
 /**
-- 
2.7.0

[PATCH 5/5] media-entity.h: rename _io to _video_device and add real _io

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The is_media_entity_v4l2_io() function should be renamed to
is_media_entity_v4l2_video_device.

Add a is_media_entity_v4l2_io to v4l2-common.h (since this is V4L2 specific)
that checks if the entity is a video_device AND if it does I/O.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 include/media/media-entity.h |  4 ++--
 include/media/v4l2-common.h  | 28 ++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index cbd3753..e5108f0 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -356,14 +356,14 @@ static inline u32 media_gobj_gen_id(enum media_gobj_type type, u64 local_id)
 }
 
 /**
- * is_media_entity_v4l2_io() - Check if the entity is a video_device
+ * is_media_entity_v4l2_video_device() - Check if the entity is a video_device
  * @entity:	pointer to entity
  *
  * Return: true if the entity is an instance of a video_device object and can
  * safely be cast to a struct video_device using the container_of() macro, or
  * false otherwise.
  */
-static inline bool is_media_entity_v4l2_io(struct media_entity *entity)
+static inline bool is_media_entity_v4l2_video_device(struct media_entity *entity)
 {
 	return entity && entity->type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE;
 }
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 1cc0c5b..858b165 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -189,4 +189,32 @@ const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(
 
 void v4l2_get_timestamp(struct timeval *tv);
 
+#ifdef CONFIG_MEDIA_CONTROLLER
+/**
+ * is_media_entity_v4l2_io() - Check if the entity is a video_device and can do I/O
+ * @entity:	pointer to entity
+ *
+ * Return: true if the entity is an instance of a video_device object and can
+ * safely be cast to a struct video_device using the container_of() macro and
+ * can do I/O, or false otherwise.
+ */
+static inline bool is_media_entity_v4l2_io(struct media_entity *entity)
+{
+	struct video_device *vdev;
+
+	if (!is_media_entity_v4l2_video_device(entity))
+		return false;
+	vdev = container_of(entity, struct video_device, entity);
+	/*
+	 * For now assume that is device_caps == 0, then I/O is available
+	 * unless it is a radio device.
+	 * Eventually all drivers should set vdev->device_caps and then
+	 * this assumption should be removed.
+	 */
+	if (vdev->device_caps == 0)
+		return vdev->vfl_type != VFL_TYPE_RADIO;
+	return vdev->device_caps & (V4L2_CAP_READWRITE | V4L2_CAP_STREAMING);
+}
+#endif
+
 #endif /* V4L2_COMMON_H_ */
-- 
2.7.0

Re: [PATCH 5/5] media-entity.h: rename _io to _video_device and add real _io

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Monday 29 February 2016 12:45:45 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> The is_media_entity_v4l2_io() function should be renamed to
> is_media_entity_v4l2_video_device.
> 
> Add a is_media_entity_v4l2_io to v4l2-common.h (since this is V4L2 specific)
> that checks if the entity is a video_device AND if it does I/O.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  include/media/media-entity.h |  4 ++--
>  include/media/v4l2-common.h  | 28 ++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index cbd3753..e5108f0 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -356,14 +356,14 @@ static inline u32 media_gobj_gen_id(enum
> media_gobj_type type, u64 local_id) }
> 
>  /**
> - * is_media_entity_v4l2_io() - Check if the entity is a video_device
> + * is_media_entity_v4l2_video_device() - Check if the entity is a
> video_device * @entity:	pointer to entity
>   *
>   * Return: true if the entity is an instance of a video_device object and
> can * safely be cast to a struct video_device using the container_of()
> macro, or * false otherwise.
>   */
> -static inline bool is_media_entity_v4l2_io(struct media_entity *entity)
> +static inline bool is_media_entity_v4l2_video_device(struct media_entity
> *entity) {

While at it, do you think this function should be moved to include/media/v4l2-
common.h ?

>  	return entity && entity->type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE;
>  }
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 1cc0c5b..858b165 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -189,4 +189,32 @@ const struct v4l2_frmsize_discrete
> *v4l2_find_nearest_format(
> 
>  void v4l2_get_timestamp(struct timeval *tv);
> 
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +/**
> + * is_media_entity_v4l2_io() - Check if the entity is a video_device and
> can do I/O
> + * @entity:	pointer to entity
> + *
> + * Return: true if the entity is an instance of a video_device object and
> can
> + * safely be cast to a struct video_device using the container_of() macro
> and
> + * can do I/O, or false otherwise.
> + */
> +static inline bool is_media_entity_v4l2_io(struct media_entity *entity)

Does this really qualify for an inline function ?

> +{
> +	struct video_device *vdev;
> +
> +	if (!is_media_entity_v4l2_video_device(entity))
> +		return false;
> +	vdev = container_of(entity, struct video_device, entity);
> +	/*
> +	 * For now assume that is device_caps == 0, then I/O is available
> +	 * unless it is a radio device.
> +	 * Eventually all drivers should set vdev->device_caps and then
> +	 * this assumption should be removed.
> +	 */
> +	if (vdev->device_caps == 0)
> +		return vdev->vfl_type != VFL_TYPE_RADIO;
> +	return vdev->device_caps & (V4L2_CAP_READWRITE | V4L2_CAP_STREAMING);
> +}
> +#endif
> +
>  #endif /* V4L2_COMMON_H_ */

-- 
Regards,

Laurent Pinchart

Re: [PATCH 5/5] media-entity.h: rename _io to _video_device and add real _io

[Hans Verkuil]

On 02/29/2016 01:23 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Monday 29 February 2016 12:45:45 Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> The is_media_entity_v4l2_io() function should be renamed to
>> is_media_entity_v4l2_video_device.
>>
>> Add a is_media_entity_v4l2_io to v4l2-common.h (since this is V4L2 specific)
>> that checks if the entity is a video_device AND if it does I/O.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  include/media/media-entity.h |  4 ++--
>>  include/media/v4l2-common.h  | 28 ++++++++++++++++++++++++++++
>>  2 files changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
>> index cbd3753..e5108f0 100644
>> --- a/include/media/media-entity.h
>> +++ b/include/media/media-entity.h
>> @@ -356,14 +356,14 @@ static inline u32 media_gobj_gen_id(enum
>> media_gobj_type type, u64 local_id) }
>>
>>  /**
>> - * is_media_entity_v4l2_io() - Check if the entity is a video_device
>> + * is_media_entity_v4l2_video_device() - Check if the entity is a
>> video_device * @entity:	pointer to entity
>>   *
>>   * Return: true if the entity is an instance of a video_device object and
>> can * safely be cast to a struct video_device using the container_of()
>> macro, or * false otherwise.
>>   */
>> -static inline bool is_media_entity_v4l2_io(struct media_entity *entity)
>> +static inline bool is_media_entity_v4l2_video_device(struct media_entity
>> *entity) {
> 
> While at it, do you think this function should be moved to include/media/v4l2-
> common.h ?

I don't think so. It might be useful for other subsystems to ignore v4l2
entities, and they can use this function for that.

> 
>>  	return entity && entity->type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE;
>>  }
>> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
>> index 1cc0c5b..858b165 100644
>> --- a/include/media/v4l2-common.h
>> +++ b/include/media/v4l2-common.h
>> @@ -189,4 +189,32 @@ const struct v4l2_frmsize_discrete
>> *v4l2_find_nearest_format(
>>
>>  void v4l2_get_timestamp(struct timeval *tv);
>>
>> +#ifdef CONFIG_MEDIA_CONTROLLER
>> +/**
>> + * is_media_entity_v4l2_io() - Check if the entity is a video_device and
>> can do I/O
>> + * @entity:	pointer to entity
>> + *
>> + * Return: true if the entity is an instance of a video_device object and
>> can
>> + * safely be cast to a struct video_device using the container_of() macro
>> and
>> + * can do I/O, or false otherwise.
>> + */
>> +static inline bool is_media_entity_v4l2_io(struct media_entity *entity)
> 
> Does this really qualify for an inline function ?

No, I was too lazy to move it to v4l2-common.c :-)

Regards,

	Hans

> 
>> +{
>> +	struct video_device *vdev;
>> +
>> +	if (!is_media_entity_v4l2_video_device(entity))
>> +		return false;
>> +	vdev = container_of(entity, struct video_device, entity);
>> +	/*
>> +	 * For now assume that is device_caps == 0, then I/O is available
>> +	 * unless it is a radio device.
>> +	 * Eventually all drivers should set vdev->device_caps and then
>> +	 * this assumption should be removed.
>> +	 */
>> +	if (vdev->device_caps == 0)
>> +		return vdev->vfl_type != VFL_TYPE_RADIO;
>> +	return vdev->device_caps & (V4L2_CAP_READWRITE | V4L2_CAP_STREAMING);
>> +}
>> +#endif
>> +
>>  #endif /* V4L2_COMMON_H_ */
>