* [PATCH] v4l2-dev: refine selection of valid v4l-touch ioctls
@ 2019-09-12 7:36 Hans Verkuil
2019-09-12 8:06 ` [PATCH v2] v4l2-core: Add metadata type to vfl_devnode_type Vandana BN
0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2019-09-12 7:36 UTC (permalink / raw)
To: Linux Media Mailing List; +Cc: Vandana B N
The selection of which ioctls are valid for touch devices was too
wide. Refine this to only the set of ioctls that make sense for such
devices.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
This patch sits on top of Vandana's metadata patch:
https://patchwork.linuxtv.org/patch/58693/
---
drivers/media/v4l2-core/v4l2-dev.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 8110127d0e3d..1fbc4af0e5ef 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -575,7 +575,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
set_bit(_IOC_NR(VIDIOC_TRY_EXT_CTRLS), valid_ioctls);
if (vdev->ctrl_handler || ops->vidioc_querymenu)
set_bit(_IOC_NR(VIDIOC_QUERYMENU), valid_ioctls);
- if (!is_meta) {
+ if (!is_tch && !is_meta) {
SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
}
@@ -592,7 +592,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
if (ops->vidioc_enum_freq_bands || ops->vidioc_g_tuner || ops->vidioc_g_modulator)
set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
- if (is_vid || is_tch) {
+ if (is_vid) {
/* video and metadata specific ioctls */
if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
ops->vidioc_enum_fmt_vid_overlay)) ||
@@ -677,6 +677,19 @@ 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_tch) {
+ /* touch specific ioctls */
+ set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
+ set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
+ set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
+ set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
+ SET_VALID_IOCTL(ops, VIDIOC_ENUMINPUT, vidioc_enum_input);
+ SET_VALID_IOCTL(ops, VIDIOC_G_INPUT, vidioc_g_input);
+ SET_VALID_IOCTL(ops, VIDIOC_S_INPUT, vidioc_s_input);
+ SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMESIZES, vidioc_enum_framesizes);
+ SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMEINTERVALS, vidioc_enum_frameintervals);
+ SET_VALID_IOCTL(ops, VIDIOC_G_PARM, vidioc_g_parm);
+ SET_VALID_IOCTL(ops, VIDIOC_S_PARM, vidioc_s_parm);
} else if (is_meta) {
/* metadata specific ioctls */
if ((is_rx && ops->vidioc_enum_fmt_meta_cap) ||
@@ -706,7 +719,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
SET_VALID_IOCTL(ops, VIDIOC_STREAMOFF, vidioc_streamoff);
}
- if (is_vid || is_vbi || is_tch) {
+ if (is_vid || is_vbi) {
/* ioctls valid for video or vbi */
if (ops->vidioc_s_std)
set_bit(_IOC_NR(VIDIOC_ENUMSTD), valid_ioctls);
@@ -746,7 +759,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
SET_VALID_IOCTL(ops, VIDIOC_G_MODULATOR, vidioc_g_modulator);
SET_VALID_IOCTL(ops, VIDIOC_S_MODULATOR, vidioc_s_modulator);
}
- if (is_rx && !is_meta) {
+ if (is_rx && !is_tch && !is_meta) {
/* receiver only ioctls */
SET_VALID_IOCTL(ops, VIDIOC_G_TUNER, vidioc_g_tuner);
SET_VALID_IOCTL(ops, VIDIOC_S_TUNER, vidioc_s_tuner);
--
2.23.0.rc1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] v4l2-core: Add metadata type to vfl_devnode_type.
2019-09-12 7:36 [PATCH] v4l2-dev: refine selection of valid v4l-touch ioctls Hans Verkuil
@ 2019-09-12 8:06 ` Vandana BN
2019-09-12 14:02 ` Hans Verkuil
0 siblings, 1 reply; 6+ messages in thread
From: Vandana BN @ 2019-09-12 8:06 UTC (permalink / raw)
To: linux-media, linux-kernel-mentees; +Cc: hverkuil, Vandana BN
Add VFL_TYPE_METADATA, to detect devices of type metadata and
to disable unneeded ioctls.
Signed-off-by: Vandana BN <bnvandana@gmail.com>
---
drivers/media/v4l2-core/v4l2-dev.c | 57 ++++++++++++++++++----------
drivers/media/v4l2-core/v4l2-ioctl.c | 5 ++-
include/media/v4l2-dev.h | 2 +
3 files changed, 41 insertions(+), 23 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 4037689a945a..5f2ead772c5f 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -112,6 +112,7 @@ static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
one single bitmap for the purposes of finding a free node number
since all those unassigned types use the same minor range. */
int idx = (vfl_type > VFL_TYPE_RADIO) ? VFL_TYPE_MAX - 1 : vfl_type;
+ idx = (vfl_type == VFL_TYPE_METADATA) ? VFL_TYPE_GRABBER : vfl_type;
return devnode_nums[idx];
}
@@ -119,7 +120,9 @@ static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
/* Return the bitmap corresponding to vfl_type. */
static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
{
- return devnode_nums[vfl_type];
+ int idx = (vfl_type == VFL_TYPE_METADATA) ? VFL_TYPE_GRABBER : vfl_type;
+
+ return devnode_nums[idx];
}
#endif
@@ -540,6 +543,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
bool is_radio = vdev->vfl_type == VFL_TYPE_RADIO;
bool is_sdr = vdev->vfl_type == VFL_TYPE_SDR;
bool is_tch = vdev->vfl_type == VFL_TYPE_TOUCH;
+ bool is_meta = vdev->vfl_type == VFL_TYPE_METADATA;
bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
@@ -571,8 +575,10 @@ static void determine_valid_ioctls(struct video_device *vdev)
set_bit(_IOC_NR(VIDIOC_TRY_EXT_CTRLS), valid_ioctls);
if (vdev->ctrl_handler || ops->vidioc_querymenu)
set_bit(_IOC_NR(VIDIOC_QUERYMENU), valid_ioctls);
- SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
- SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
+ if (!is_meta) {
+ SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
+ SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
+ }
SET_VALID_IOCTL(ops, VIDIOC_LOG_STATUS, vidioc_log_status);
#ifdef CONFIG_VIDEO_ADV_DEBUG
set_bit(_IOC_NR(VIDIOC_DBG_G_CHIP_INFO), valid_ioctls);
@@ -589,37 +595,29 @@ static void determine_valid_ioctls(struct video_device *vdev)
if (is_vid || is_tch) {
/* video and metadata specific ioctls */
if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
- ops->vidioc_enum_fmt_vid_overlay ||
- ops->vidioc_enum_fmt_meta_cap)) ||
- (is_tx && (ops->vidioc_enum_fmt_vid_out ||
- ops->vidioc_enum_fmt_meta_out)))
+ ops->vidioc_enum_fmt_vid_overlay)) ||
+ (is_tx && ops->vidioc_enum_fmt_vid_out))
set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
if ((is_rx && (ops->vidioc_g_fmt_vid_cap ||
ops->vidioc_g_fmt_vid_cap_mplane ||
- ops->vidioc_g_fmt_vid_overlay ||
- ops->vidioc_g_fmt_meta_cap)) ||
+ ops->vidioc_g_fmt_vid_overlay)) ||
(is_tx && (ops->vidioc_g_fmt_vid_out ||
ops->vidioc_g_fmt_vid_out_mplane ||
- ops->vidioc_g_fmt_vid_out_overlay ||
- ops->vidioc_g_fmt_meta_out)))
+ ops->vidioc_g_fmt_vid_out_overlay)))
set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
if ((is_rx && (ops->vidioc_s_fmt_vid_cap ||
ops->vidioc_s_fmt_vid_cap_mplane ||
- ops->vidioc_s_fmt_vid_overlay ||
- ops->vidioc_s_fmt_meta_cap)) ||
+ ops->vidioc_s_fmt_vid_overlay)) ||
(is_tx && (ops->vidioc_s_fmt_vid_out ||
ops->vidioc_s_fmt_vid_out_mplane ||
- ops->vidioc_s_fmt_vid_out_overlay ||
- ops->vidioc_s_fmt_meta_out)))
+ ops->vidioc_s_fmt_vid_out_overlay)))
set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
if ((is_rx && (ops->vidioc_try_fmt_vid_cap ||
ops->vidioc_try_fmt_vid_cap_mplane ||
- ops->vidioc_try_fmt_vid_overlay ||
- ops->vidioc_try_fmt_meta_cap)) ||
+ ops->vidioc_try_fmt_vid_overlay)) ||
(is_tx && (ops->vidioc_try_fmt_vid_out ||
ops->vidioc_try_fmt_vid_out_mplane ||
- ops->vidioc_try_fmt_vid_out_overlay ||
- ops->vidioc_try_fmt_meta_out)))
+ ops->vidioc_try_fmt_vid_out_overlay)))
set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
@@ -679,9 +677,23 @@ 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) {
+ /* metadata specific ioctls */
+ if ((is_rx && ops->vidioc_enum_fmt_meta_cap) ||
+ (is_tx && ops->vidioc_enum_fmt_meta_out))
+ set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
+ if ((is_rx && ops->vidioc_g_fmt_meta_cap) ||
+ (is_tx && ops->vidioc_g_fmt_meta_out))
+ set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
+ if ((is_rx && ops->vidioc_s_fmt_meta_cap) ||
+ (is_tx && ops->vidioc_s_fmt_meta_out))
+ set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
+ if ((is_rx && ops->vidioc_try_fmt_meta_cap) ||
+ (is_tx && ops->vidioc_try_fmt_meta_out))
+ set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
}
- if (is_vid || is_vbi || is_sdr || is_tch) {
+ if (is_vid || is_vbi || is_sdr || is_tch || 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);
@@ -734,7 +746,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
SET_VALID_IOCTL(ops, VIDIOC_G_MODULATOR, vidioc_g_modulator);
SET_VALID_IOCTL(ops, VIDIOC_S_MODULATOR, vidioc_s_modulator);
}
- if (is_rx) {
+ if (is_rx && !is_meta) {
/* receiver only ioctls */
SET_VALID_IOCTL(ops, VIDIOC_G_TUNER, vidioc_g_tuner);
SET_VALID_IOCTL(ops, VIDIOC_S_TUNER, vidioc_s_tuner);
@@ -762,6 +774,7 @@ static int video_register_media_controller(struct video_device *vdev)
switch (vdev->vfl_type) {
case VFL_TYPE_GRABBER:
+ case VFL_TYPE_METADATA:
intf_type = MEDIA_INTF_T_V4L_VIDEO;
vdev->entity.function = MEDIA_ENT_F_IO_V4L;
break;
@@ -870,6 +883,7 @@ int __video_register_device(struct video_device *vdev,
/* Part 1: check device type */
switch (type) {
case VFL_TYPE_GRABBER:
+ case VFL_TYPE_METADATA:
name_base = "video";
break;
case VFL_TYPE_VBI:
@@ -914,6 +928,7 @@ int __video_register_device(struct video_device *vdev,
* (new style). */
switch (type) {
case VFL_TYPE_GRABBER:
+ case VFL_TYPE_METADATA:
minor_offset = 0;
minor_cnt = 64;
break;
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 51b912743f0f..0d71c06c82cf 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -938,6 +938,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
bool is_vbi = vfd->vfl_type == VFL_TYPE_VBI;
bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
bool is_tch = vfd->vfl_type == VFL_TYPE_TOUCH;
+ bool is_meta = vfd->vfl_type == VFL_TYPE_METADATA;
bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
@@ -996,11 +997,11 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
return 0;
break;
case V4L2_BUF_TYPE_META_CAPTURE:
- if (is_vid && is_rx && ops->vidioc_g_fmt_meta_cap)
+ if (is_meta && is_rx && ops->vidioc_g_fmt_meta_cap)
return 0;
break;
case V4L2_BUF_TYPE_META_OUTPUT:
- if (is_vid && is_tx && ops->vidioc_g_fmt_meta_out)
+ if (is_meta && is_tx && ops->vidioc_g_fmt_meta_out)
return 0;
break;
default:
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 48531e57cc5a..2da91d454c10 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -30,6 +30,7 @@
* @VFL_TYPE_SUBDEV: for V4L2 subdevices
* @VFL_TYPE_SDR: for Software Defined Radio tuners
* @VFL_TYPE_TOUCH: for touch sensors
+ * @VFL_TYPE_METADATA: for Metadata device
* @VFL_TYPE_MAX: number of VFL types, must always be last in the enum
*/
enum vfl_devnode_type {
@@ -39,6 +40,7 @@ enum vfl_devnode_type {
VFL_TYPE_SUBDEV,
VFL_TYPE_SDR,
VFL_TYPE_TOUCH,
+ VFL_TYPE_METADATA,
VFL_TYPE_MAX /* Shall be the last one */
};
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] v4l2-core: Add metadata type to vfl_devnode_type.
2019-09-12 8:06 ` [PATCH v2] v4l2-core: Add metadata type to vfl_devnode_type Vandana BN
@ 2019-09-12 14:02 ` Hans Verkuil
2019-09-13 6:57 ` [PATCH v3] " Vandana BN
0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2019-09-12 14:02 UTC (permalink / raw)
To: Vandana BN, linux-media, linux-kernel-mentees
Hi Vandana,
On 9/12/19 10:06 AM, Vandana BN wrote:
> Add VFL_TYPE_METADATA, to detect devices of type metadata and
> to disable unneeded ioctls.
The commit text can be improved. It would be good to add the reason
for this change (in this case because without this check the format
ioctls for both video and meta data could be called for both device
nodes. And it's true for other ioctls as well.
It's always good to explain why a change is made.
Regards,
Hans
>
> Signed-off-by: Vandana BN <bnvandana@gmail.com>
> ---
> drivers/media/v4l2-core/v4l2-dev.c | 57 ++++++++++++++++++----------
> drivers/media/v4l2-core/v4l2-ioctl.c | 5 ++-
> include/media/v4l2-dev.h | 2 +
> 3 files changed, 41 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 4037689a945a..5f2ead772c5f 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -112,6 +112,7 @@ static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
> one single bitmap for the purposes of finding a free node number
> since all those unassigned types use the same minor range. */
> int idx = (vfl_type > VFL_TYPE_RADIO) ? VFL_TYPE_MAX - 1 : vfl_type;
> + idx = (vfl_type == VFL_TYPE_METADATA) ? VFL_TYPE_GRABBER : vfl_type;
>
> return devnode_nums[idx];
> }
> @@ -119,7 +120,9 @@ static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
> /* Return the bitmap corresponding to vfl_type. */
> static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
> {
> - return devnode_nums[vfl_type];
> + int idx = (vfl_type == VFL_TYPE_METADATA) ? VFL_TYPE_GRABBER : vfl_type;
> +
> + return devnode_nums[idx];
> }
> #endif
>
> @@ -540,6 +543,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
> bool is_radio = vdev->vfl_type == VFL_TYPE_RADIO;
> bool is_sdr = vdev->vfl_type == VFL_TYPE_SDR;
> bool is_tch = vdev->vfl_type == VFL_TYPE_TOUCH;
> + bool is_meta = vdev->vfl_type == VFL_TYPE_METADATA;
> bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
> bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
>
> @@ -571,8 +575,10 @@ static void determine_valid_ioctls(struct video_device *vdev)
> set_bit(_IOC_NR(VIDIOC_TRY_EXT_CTRLS), valid_ioctls);
> if (vdev->ctrl_handler || ops->vidioc_querymenu)
> set_bit(_IOC_NR(VIDIOC_QUERYMENU), valid_ioctls);
> - SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
> - SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
> + if (!is_meta) {
> + SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
> + SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
> + }
> SET_VALID_IOCTL(ops, VIDIOC_LOG_STATUS, vidioc_log_status);
> #ifdef CONFIG_VIDEO_ADV_DEBUG
> set_bit(_IOC_NR(VIDIOC_DBG_G_CHIP_INFO), valid_ioctls);
> @@ -589,37 +595,29 @@ static void determine_valid_ioctls(struct video_device *vdev)
> if (is_vid || is_tch) {
> /* video and metadata specific ioctls */
> if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
> - ops->vidioc_enum_fmt_vid_overlay ||
> - ops->vidioc_enum_fmt_meta_cap)) ||
> - (is_tx && (ops->vidioc_enum_fmt_vid_out ||
> - ops->vidioc_enum_fmt_meta_out)))
> + ops->vidioc_enum_fmt_vid_overlay)) ||
> + (is_tx && ops->vidioc_enum_fmt_vid_out))
> set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
> if ((is_rx && (ops->vidioc_g_fmt_vid_cap ||
> ops->vidioc_g_fmt_vid_cap_mplane ||
> - ops->vidioc_g_fmt_vid_overlay ||
> - ops->vidioc_g_fmt_meta_cap)) ||
> + ops->vidioc_g_fmt_vid_overlay)) ||
> (is_tx && (ops->vidioc_g_fmt_vid_out ||
> ops->vidioc_g_fmt_vid_out_mplane ||
> - ops->vidioc_g_fmt_vid_out_overlay ||
> - ops->vidioc_g_fmt_meta_out)))
> + ops->vidioc_g_fmt_vid_out_overlay)))
> set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
> if ((is_rx && (ops->vidioc_s_fmt_vid_cap ||
> ops->vidioc_s_fmt_vid_cap_mplane ||
> - ops->vidioc_s_fmt_vid_overlay ||
> - ops->vidioc_s_fmt_meta_cap)) ||
> + ops->vidioc_s_fmt_vid_overlay)) ||
> (is_tx && (ops->vidioc_s_fmt_vid_out ||
> ops->vidioc_s_fmt_vid_out_mplane ||
> - ops->vidioc_s_fmt_vid_out_overlay ||
> - ops->vidioc_s_fmt_meta_out)))
> + ops->vidioc_s_fmt_vid_out_overlay)))
> set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
> if ((is_rx && (ops->vidioc_try_fmt_vid_cap ||
> ops->vidioc_try_fmt_vid_cap_mplane ||
> - ops->vidioc_try_fmt_vid_overlay ||
> - ops->vidioc_try_fmt_meta_cap)) ||
> + ops->vidioc_try_fmt_vid_overlay)) ||
> (is_tx && (ops->vidioc_try_fmt_vid_out ||
> ops->vidioc_try_fmt_vid_out_mplane ||
> - ops->vidioc_try_fmt_vid_out_overlay ||
> - ops->vidioc_try_fmt_meta_out)))
> + ops->vidioc_try_fmt_vid_out_overlay)))
> set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
> SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
> SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
> @@ -679,9 +677,23 @@ 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) {
> + /* metadata specific ioctls */
> + if ((is_rx && ops->vidioc_enum_fmt_meta_cap) ||
> + (is_tx && ops->vidioc_enum_fmt_meta_out))
> + set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
> + if ((is_rx && ops->vidioc_g_fmt_meta_cap) ||
> + (is_tx && ops->vidioc_g_fmt_meta_out))
> + set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
> + if ((is_rx && ops->vidioc_s_fmt_meta_cap) ||
> + (is_tx && ops->vidioc_s_fmt_meta_out))
> + set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
> + if ((is_rx && ops->vidioc_try_fmt_meta_cap) ||
> + (is_tx && ops->vidioc_try_fmt_meta_out))
> + set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
> }
>
> - if (is_vid || is_vbi || is_sdr || is_tch) {
> + if (is_vid || is_vbi || is_sdr || is_tch || 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);
> @@ -734,7 +746,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
> SET_VALID_IOCTL(ops, VIDIOC_G_MODULATOR, vidioc_g_modulator);
> SET_VALID_IOCTL(ops, VIDIOC_S_MODULATOR, vidioc_s_modulator);
> }
> - if (is_rx) {
> + if (is_rx && !is_meta) {
> /* receiver only ioctls */
> SET_VALID_IOCTL(ops, VIDIOC_G_TUNER, vidioc_g_tuner);
> SET_VALID_IOCTL(ops, VIDIOC_S_TUNER, vidioc_s_tuner);
> @@ -762,6 +774,7 @@ static int video_register_media_controller(struct video_device *vdev)
>
> switch (vdev->vfl_type) {
> case VFL_TYPE_GRABBER:
> + case VFL_TYPE_METADATA:
> intf_type = MEDIA_INTF_T_V4L_VIDEO;
> vdev->entity.function = MEDIA_ENT_F_IO_V4L;
> break;
> @@ -870,6 +883,7 @@ int __video_register_device(struct video_device *vdev,
> /* Part 1: check device type */
> switch (type) {
> case VFL_TYPE_GRABBER:
> + case VFL_TYPE_METADATA:
> name_base = "video";
> break;
> case VFL_TYPE_VBI:
> @@ -914,6 +928,7 @@ int __video_register_device(struct video_device *vdev,
> * (new style). */
> switch (type) {
> case VFL_TYPE_GRABBER:
> + case VFL_TYPE_METADATA:
> minor_offset = 0;
> minor_cnt = 64;
> break;
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 51b912743f0f..0d71c06c82cf 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -938,6 +938,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
> bool is_vbi = vfd->vfl_type == VFL_TYPE_VBI;
> bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
> bool is_tch = vfd->vfl_type == VFL_TYPE_TOUCH;
> + bool is_meta = vfd->vfl_type == VFL_TYPE_METADATA;
> bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
> bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
>
> @@ -996,11 +997,11 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
> return 0;
> break;
> case V4L2_BUF_TYPE_META_CAPTURE:
> - if (is_vid && is_rx && ops->vidioc_g_fmt_meta_cap)
> + if (is_meta && is_rx && ops->vidioc_g_fmt_meta_cap)
> return 0;
> break;
> case V4L2_BUF_TYPE_META_OUTPUT:
> - if (is_vid && is_tx && ops->vidioc_g_fmt_meta_out)
> + if (is_meta && is_tx && ops->vidioc_g_fmt_meta_out)
> return 0;
> break;
> default:
> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
> index 48531e57cc5a..2da91d454c10 100644
> --- a/include/media/v4l2-dev.h
> +++ b/include/media/v4l2-dev.h
> @@ -30,6 +30,7 @@
> * @VFL_TYPE_SUBDEV: for V4L2 subdevices
> * @VFL_TYPE_SDR: for Software Defined Radio tuners
> * @VFL_TYPE_TOUCH: for touch sensors
> + * @VFL_TYPE_METADATA: for Metadata device
> * @VFL_TYPE_MAX: number of VFL types, must always be last in the enum
> */
> enum vfl_devnode_type {
> @@ -39,6 +40,7 @@ enum vfl_devnode_type {
> VFL_TYPE_SUBDEV,
> VFL_TYPE_SDR,
> VFL_TYPE_TOUCH,
> + VFL_TYPE_METADATA,
> VFL_TYPE_MAX /* Shall be the last one */
> };
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3] v4l2-core: Add metadata type to vfl_devnode_type
2019-09-12 14:02 ` Hans Verkuil
@ 2019-09-13 6:57 ` Vandana BN
2019-09-17 9:54 ` Hans Verkuil
0 siblings, 1 reply; 6+ messages in thread
From: Vandana BN @ 2019-09-13 6:57 UTC (permalink / raw)
To: linux-media, linux-kernel-mentees; +Cc: hverkuil, Vandana BN
Add VFL_TYPE_METADATA, to detect devices of type metadata and
to disable unneeded ioctls.
Without this change, format ioctls for both video and metadata devices
could be called on both device nodes.This is true for other ioctls as
well.
Signed-off-by: Vandana BN <bnvandana@gmail.com>
---
V3 Updated commit message
drivers/media/v4l2-core/v4l2-dev.c | 57 ++++++++++++++++++----------
drivers/media/v4l2-core/v4l2-ioctl.c | 5 ++-
include/media/v4l2-dev.h | 2 +
3 files changed, 41 insertions(+), 23 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 4037689a945a..5f2ead772c5f 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -112,6 +112,7 @@ static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
one single bitmap for the purposes of finding a free node number
since all those unassigned types use the same minor range. */
int idx = (vfl_type > VFL_TYPE_RADIO) ? VFL_TYPE_MAX - 1 : vfl_type;
+ idx = (vfl_type == VFL_TYPE_METADATA) ? VFL_TYPE_GRABBER : vfl_type;
return devnode_nums[idx];
}
@@ -119,7 +120,9 @@ static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
/* Return the bitmap corresponding to vfl_type. */
static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
{
- return devnode_nums[vfl_type];
+ int idx = (vfl_type == VFL_TYPE_METADATA) ? VFL_TYPE_GRABBER : vfl_type;
+
+ return devnode_nums[idx];
}
#endif
@@ -540,6 +543,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
bool is_radio = vdev->vfl_type == VFL_TYPE_RADIO;
bool is_sdr = vdev->vfl_type == VFL_TYPE_SDR;
bool is_tch = vdev->vfl_type == VFL_TYPE_TOUCH;
+ bool is_meta = vdev->vfl_type == VFL_TYPE_METADATA;
bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
@@ -571,8 +575,10 @@ static void determine_valid_ioctls(struct video_device *vdev)
set_bit(_IOC_NR(VIDIOC_TRY_EXT_CTRLS), valid_ioctls);
if (vdev->ctrl_handler || ops->vidioc_querymenu)
set_bit(_IOC_NR(VIDIOC_QUERYMENU), valid_ioctls);
- SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
- SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
+ if (!is_meta) {
+ SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
+ SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
+ }
SET_VALID_IOCTL(ops, VIDIOC_LOG_STATUS, vidioc_log_status);
#ifdef CONFIG_VIDEO_ADV_DEBUG
set_bit(_IOC_NR(VIDIOC_DBG_G_CHIP_INFO), valid_ioctls);
@@ -589,37 +595,29 @@ static void determine_valid_ioctls(struct video_device *vdev)
if (is_vid || is_tch) {
/* video and metadata specific ioctls */
if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
- ops->vidioc_enum_fmt_vid_overlay ||
- ops->vidioc_enum_fmt_meta_cap)) ||
- (is_tx && (ops->vidioc_enum_fmt_vid_out ||
- ops->vidioc_enum_fmt_meta_out)))
+ ops->vidioc_enum_fmt_vid_overlay)) ||
+ (is_tx && ops->vidioc_enum_fmt_vid_out))
set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
if ((is_rx && (ops->vidioc_g_fmt_vid_cap ||
ops->vidioc_g_fmt_vid_cap_mplane ||
- ops->vidioc_g_fmt_vid_overlay ||
- ops->vidioc_g_fmt_meta_cap)) ||
+ ops->vidioc_g_fmt_vid_overlay)) ||
(is_tx && (ops->vidioc_g_fmt_vid_out ||
ops->vidioc_g_fmt_vid_out_mplane ||
- ops->vidioc_g_fmt_vid_out_overlay ||
- ops->vidioc_g_fmt_meta_out)))
+ ops->vidioc_g_fmt_vid_out_overlay)))
set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
if ((is_rx && (ops->vidioc_s_fmt_vid_cap ||
ops->vidioc_s_fmt_vid_cap_mplane ||
- ops->vidioc_s_fmt_vid_overlay ||
- ops->vidioc_s_fmt_meta_cap)) ||
+ ops->vidioc_s_fmt_vid_overlay)) ||
(is_tx && (ops->vidioc_s_fmt_vid_out ||
ops->vidioc_s_fmt_vid_out_mplane ||
- ops->vidioc_s_fmt_vid_out_overlay ||
- ops->vidioc_s_fmt_meta_out)))
+ ops->vidioc_s_fmt_vid_out_overlay)))
set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
if ((is_rx && (ops->vidioc_try_fmt_vid_cap ||
ops->vidioc_try_fmt_vid_cap_mplane ||
- ops->vidioc_try_fmt_vid_overlay ||
- ops->vidioc_try_fmt_meta_cap)) ||
+ ops->vidioc_try_fmt_vid_overlay)) ||
(is_tx && (ops->vidioc_try_fmt_vid_out ||
ops->vidioc_try_fmt_vid_out_mplane ||
- ops->vidioc_try_fmt_vid_out_overlay ||
- ops->vidioc_try_fmt_meta_out)))
+ ops->vidioc_try_fmt_vid_out_overlay)))
set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
@@ -679,9 +677,23 @@ 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) {
+ /* metadata specific ioctls */
+ if ((is_rx && ops->vidioc_enum_fmt_meta_cap) ||
+ (is_tx && ops->vidioc_enum_fmt_meta_out))
+ set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
+ if ((is_rx && ops->vidioc_g_fmt_meta_cap) ||
+ (is_tx && ops->vidioc_g_fmt_meta_out))
+ set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
+ if ((is_rx && ops->vidioc_s_fmt_meta_cap) ||
+ (is_tx && ops->vidioc_s_fmt_meta_out))
+ set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
+ if ((is_rx && ops->vidioc_try_fmt_meta_cap) ||
+ (is_tx && ops->vidioc_try_fmt_meta_out))
+ set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
}
- if (is_vid || is_vbi || is_sdr || is_tch) {
+ if (is_vid || is_vbi || is_sdr || is_tch || 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);
@@ -734,7 +746,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
SET_VALID_IOCTL(ops, VIDIOC_G_MODULATOR, vidioc_g_modulator);
SET_VALID_IOCTL(ops, VIDIOC_S_MODULATOR, vidioc_s_modulator);
}
- if (is_rx) {
+ if (is_rx && !is_meta) {
/* receiver only ioctls */
SET_VALID_IOCTL(ops, VIDIOC_G_TUNER, vidioc_g_tuner);
SET_VALID_IOCTL(ops, VIDIOC_S_TUNER, vidioc_s_tuner);
@@ -762,6 +774,7 @@ static int video_register_media_controller(struct video_device *vdev)
switch (vdev->vfl_type) {
case VFL_TYPE_GRABBER:
+ case VFL_TYPE_METADATA:
intf_type = MEDIA_INTF_T_V4L_VIDEO;
vdev->entity.function = MEDIA_ENT_F_IO_V4L;
break;
@@ -870,6 +883,7 @@ int __video_register_device(struct video_device *vdev,
/* Part 1: check device type */
switch (type) {
case VFL_TYPE_GRABBER:
+ case VFL_TYPE_METADATA:
name_base = "video";
break;
case VFL_TYPE_VBI:
@@ -914,6 +928,7 @@ int __video_register_device(struct video_device *vdev,
* (new style). */
switch (type) {
case VFL_TYPE_GRABBER:
+ case VFL_TYPE_METADATA:
minor_offset = 0;
minor_cnt = 64;
break;
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 51b912743f0f..0d71c06c82cf 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -938,6 +938,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
bool is_vbi = vfd->vfl_type == VFL_TYPE_VBI;
bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
bool is_tch = vfd->vfl_type == VFL_TYPE_TOUCH;
+ bool is_meta = vfd->vfl_type == VFL_TYPE_METADATA;
bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
@@ -996,11 +997,11 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
return 0;
break;
case V4L2_BUF_TYPE_META_CAPTURE:
- if (is_vid && is_rx && ops->vidioc_g_fmt_meta_cap)
+ if (is_meta && is_rx && ops->vidioc_g_fmt_meta_cap)
return 0;
break;
case V4L2_BUF_TYPE_META_OUTPUT:
- if (is_vid && is_tx && ops->vidioc_g_fmt_meta_out)
+ if (is_meta && is_tx && ops->vidioc_g_fmt_meta_out)
return 0;
break;
default:
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 48531e57cc5a..2da91d454c10 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -30,6 +30,7 @@
* @VFL_TYPE_SUBDEV: for V4L2 subdevices
* @VFL_TYPE_SDR: for Software Defined Radio tuners
* @VFL_TYPE_TOUCH: for touch sensors
+ * @VFL_TYPE_METADATA: for Metadata device
* @VFL_TYPE_MAX: number of VFL types, must always be last in the enum
*/
enum vfl_devnode_type {
@@ -39,6 +40,7 @@ enum vfl_devnode_type {
VFL_TYPE_SUBDEV,
VFL_TYPE_SDR,
VFL_TYPE_TOUCH,
+ VFL_TYPE_METADATA,
VFL_TYPE_MAX /* Shall be the last one */
};
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] v4l2-core: Add metadata type to vfl_devnode_type
2019-09-13 6:57 ` [PATCH v3] " Vandana BN
@ 2019-09-17 9:54 ` Hans Verkuil
2019-09-17 9:56 ` Hans Verkuil
0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2019-09-17 9:54 UTC (permalink / raw)
To: Vandana BN, linux-media, linux-kernel-mentees
On 9/13/19 8:57 AM, Vandana BN wrote:
> Add VFL_TYPE_METADATA, to detect devices of type metadata and
> to disable unneeded ioctls.
> Without this change, format ioctls for both video and metadata devices
> could be called on both device nodes.This is true for other ioctls as
> well.
>
> Signed-off-by: Vandana BN <bnvandana@gmail.com>
> ---
> V3 Updated commit message
>
> drivers/media/v4l2-core/v4l2-dev.c | 57 ++++++++++++++++++----------
> drivers/media/v4l2-core/v4l2-ioctl.c | 5 ++-
> include/media/v4l2-dev.h | 2 +
> 3 files changed, 41 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 4037689a945a..5f2ead772c5f 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -112,6 +112,7 @@ static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
> one single bitmap for the purposes of finding a free node number
> since all those unassigned types use the same minor range. */
> int idx = (vfl_type > VFL_TYPE_RADIO) ? VFL_TYPE_MAX - 1 : vfl_type;
> + idx = (vfl_type == VFL_TYPE_METADATA) ? VFL_TYPE_GRABBER : vfl_type;
>
> return devnode_nums[idx];
> }
> @@ -119,7 +120,9 @@ static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
> /* Return the bitmap corresponding to vfl_type. */
> static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
> {
> - return devnode_nums[vfl_type];
> + int idx = (vfl_type == VFL_TYPE_METADATA) ? VFL_TYPE_GRABBER : vfl_type;
> +
> + return devnode_nums[idx];
> }
> #endif
>
> @@ -540,6 +543,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
> bool is_radio = vdev->vfl_type == VFL_TYPE_RADIO;
> bool is_sdr = vdev->vfl_type == VFL_TYPE_SDR;
> bool is_tch = vdev->vfl_type == VFL_TYPE_TOUCH;
> + bool is_meta = vdev->vfl_type == VFL_TYPE_METADATA;
> bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
> bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
>
> @@ -571,8 +575,10 @@ static void determine_valid_ioctls(struct video_device *vdev)
> set_bit(_IOC_NR(VIDIOC_TRY_EXT_CTRLS), valid_ioctls);
> if (vdev->ctrl_handler || ops->vidioc_querymenu)
> set_bit(_IOC_NR(VIDIOC_QUERYMENU), valid_ioctls);
> - SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
> - SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
> + if (!is_meta) {
> + SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
> + SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
> + }
> SET_VALID_IOCTL(ops, VIDIOC_LOG_STATUS, vidioc_log_status);
> #ifdef CONFIG_VIDEO_ADV_DEBUG
> set_bit(_IOC_NR(VIDIOC_DBG_G_CHIP_INFO), valid_ioctls);
> @@ -589,37 +595,29 @@ static void determine_valid_ioctls(struct video_device *vdev)
> if (is_vid || is_tch) {
> /* video and metadata specific ioctls */
> if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
> - ops->vidioc_enum_fmt_vid_overlay ||
> - ops->vidioc_enum_fmt_meta_cap)) ||
> - (is_tx && (ops->vidioc_enum_fmt_vid_out ||
> - ops->vidioc_enum_fmt_meta_out)))
> + ops->vidioc_enum_fmt_vid_overlay)) ||
> + (is_tx && ops->vidioc_enum_fmt_vid_out))
> set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
> if ((is_rx && (ops->vidioc_g_fmt_vid_cap ||
> ops->vidioc_g_fmt_vid_cap_mplane ||
> - ops->vidioc_g_fmt_vid_overlay ||
> - ops->vidioc_g_fmt_meta_cap)) ||
> + ops->vidioc_g_fmt_vid_overlay)) ||
> (is_tx && (ops->vidioc_g_fmt_vid_out ||
> ops->vidioc_g_fmt_vid_out_mplane ||
> - ops->vidioc_g_fmt_vid_out_overlay ||
> - ops->vidioc_g_fmt_meta_out)))
> + ops->vidioc_g_fmt_vid_out_overlay)))
> set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
> if ((is_rx && (ops->vidioc_s_fmt_vid_cap ||
> ops->vidioc_s_fmt_vid_cap_mplane ||
> - ops->vidioc_s_fmt_vid_overlay ||
> - ops->vidioc_s_fmt_meta_cap)) ||
> + ops->vidioc_s_fmt_vid_overlay)) ||
> (is_tx && (ops->vidioc_s_fmt_vid_out ||
> ops->vidioc_s_fmt_vid_out_mplane ||
> - ops->vidioc_s_fmt_vid_out_overlay ||
> - ops->vidioc_s_fmt_meta_out)))
> + ops->vidioc_s_fmt_vid_out_overlay)))
> set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
> if ((is_rx && (ops->vidioc_try_fmt_vid_cap ||
> ops->vidioc_try_fmt_vid_cap_mplane ||
> - ops->vidioc_try_fmt_vid_overlay ||
> - ops->vidioc_try_fmt_meta_cap)) ||
> + ops->vidioc_try_fmt_vid_overlay)) ||
> (is_tx && (ops->vidioc_try_fmt_vid_out ||
> ops->vidioc_try_fmt_vid_out_mplane ||
> - ops->vidioc_try_fmt_vid_out_overlay ||
> - ops->vidioc_try_fmt_meta_out)))
> + ops->vidioc_try_fmt_vid_out_overlay)))
> set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
> SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
> SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
> @@ -679,9 +677,23 @@ 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) {
> + /* metadata specific ioctls */
> + if ((is_rx && ops->vidioc_enum_fmt_meta_cap) ||
> + (is_tx && ops->vidioc_enum_fmt_meta_out))
> + set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
> + if ((is_rx && ops->vidioc_g_fmt_meta_cap) ||
> + (is_tx && ops->vidioc_g_fmt_meta_out))
> + set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
> + if ((is_rx && ops->vidioc_s_fmt_meta_cap) ||
> + (is_tx && ops->vidioc_s_fmt_meta_out))
> + set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
> + if ((is_rx && ops->vidioc_try_fmt_meta_cap) ||
> + (is_tx && ops->vidioc_try_fmt_meta_out))
> + set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
After some more testing I realized that metadata devices should still support
ENUM/G/S_INPUT for capture and ENUM/S/G_OUTPUT for output.
Can you post a v4 with support for that?
Thanks!
Hans
> }
>
> - if (is_vid || is_vbi || is_sdr || is_tch) {
> + if (is_vid || is_vbi || is_sdr || is_tch || 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);
> @@ -734,7 +746,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
> SET_VALID_IOCTL(ops, VIDIOC_G_MODULATOR, vidioc_g_modulator);
> SET_VALID_IOCTL(ops, VIDIOC_S_MODULATOR, vidioc_s_modulator);
> }
> - if (is_rx) {
> + if (is_rx && !is_meta) {
> /* receiver only ioctls */
> SET_VALID_IOCTL(ops, VIDIOC_G_TUNER, vidioc_g_tuner);
> SET_VALID_IOCTL(ops, VIDIOC_S_TUNER, vidioc_s_tuner);
> @@ -762,6 +774,7 @@ static int video_register_media_controller(struct video_device *vdev)
>
> switch (vdev->vfl_type) {
> case VFL_TYPE_GRABBER:
> + case VFL_TYPE_METADATA:
> intf_type = MEDIA_INTF_T_V4L_VIDEO;
> vdev->entity.function = MEDIA_ENT_F_IO_V4L;
> break;
> @@ -870,6 +883,7 @@ int __video_register_device(struct video_device *vdev,
> /* Part 1: check device type */
> switch (type) {
> case VFL_TYPE_GRABBER:
> + case VFL_TYPE_METADATA:
> name_base = "video";
> break;
> case VFL_TYPE_VBI:
> @@ -914,6 +928,7 @@ int __video_register_device(struct video_device *vdev,
> * (new style). */
> switch (type) {
> case VFL_TYPE_GRABBER:
> + case VFL_TYPE_METADATA:
> minor_offset = 0;
> minor_cnt = 64;
> break;
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 51b912743f0f..0d71c06c82cf 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -938,6 +938,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
> bool is_vbi = vfd->vfl_type == VFL_TYPE_VBI;
> bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
> bool is_tch = vfd->vfl_type == VFL_TYPE_TOUCH;
> + bool is_meta = vfd->vfl_type == VFL_TYPE_METADATA;
> bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
> bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
>
> @@ -996,11 +997,11 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
> return 0;
> break;
> case V4L2_BUF_TYPE_META_CAPTURE:
> - if (is_vid && is_rx && ops->vidioc_g_fmt_meta_cap)
> + if (is_meta && is_rx && ops->vidioc_g_fmt_meta_cap)
> return 0;
> break;
> case V4L2_BUF_TYPE_META_OUTPUT:
> - if (is_vid && is_tx && ops->vidioc_g_fmt_meta_out)
> + if (is_meta && is_tx && ops->vidioc_g_fmt_meta_out)
> return 0;
> break;
> default:
> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
> index 48531e57cc5a..2da91d454c10 100644
> --- a/include/media/v4l2-dev.h
> +++ b/include/media/v4l2-dev.h
> @@ -30,6 +30,7 @@
> * @VFL_TYPE_SUBDEV: for V4L2 subdevices
> * @VFL_TYPE_SDR: for Software Defined Radio tuners
> * @VFL_TYPE_TOUCH: for touch sensors
> + * @VFL_TYPE_METADATA: for Metadata device
> * @VFL_TYPE_MAX: number of VFL types, must always be last in the enum
> */
> enum vfl_devnode_type {
> @@ -39,6 +40,7 @@ enum vfl_devnode_type {
> VFL_TYPE_SUBDEV,
> VFL_TYPE_SDR,
> VFL_TYPE_TOUCH,
> + VFL_TYPE_METADATA,
> VFL_TYPE_MAX /* Shall be the last one */
> };
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] v4l2-core: Add metadata type to vfl_devnode_type
2019-09-17 9:54 ` Hans Verkuil
@ 2019-09-17 9:56 ` Hans Verkuil
0 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2019-09-17 9:56 UTC (permalink / raw)
To: Vandana BN, linux-media, linux-kernel-mentees
On 9/17/19 11:54 AM, Hans Verkuil wrote:
> On 9/13/19 8:57 AM, Vandana BN wrote:
>> Add VFL_TYPE_METADATA, to detect devices of type metadata and
>> to disable unneeded ioctls.
>> Without this change, format ioctls for both video and metadata devices
>> could be called on both device nodes.This is true for other ioctls as
>> well.
>>
>> Signed-off-by: Vandana BN <bnvandana@gmail.com>
>> ---
>> V3 Updated commit message
>>
>> drivers/media/v4l2-core/v4l2-dev.c | 57 ++++++++++++++++++----------
>> drivers/media/v4l2-core/v4l2-ioctl.c | 5 ++-
>> include/media/v4l2-dev.h | 2 +
>> 3 files changed, 41 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
>> index 4037689a945a..5f2ead772c5f 100644
>> --- a/drivers/media/v4l2-core/v4l2-dev.c
>> +++ b/drivers/media/v4l2-core/v4l2-dev.c
>> @@ -112,6 +112,7 @@ static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
>> one single bitmap for the purposes of finding a free node number
>> since all those unassigned types use the same minor range. */
>> int idx = (vfl_type > VFL_TYPE_RADIO) ? VFL_TYPE_MAX - 1 : vfl_type;
>> + idx = (vfl_type == VFL_TYPE_METADATA) ? VFL_TYPE_GRABBER : vfl_type;
>>
>> return devnode_nums[idx];
>> }
>> @@ -119,7 +120,9 @@ static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
>> /* Return the bitmap corresponding to vfl_type. */
>> static inline unsigned long *devnode_bits(enum vfl_devnode_type vfl_type)
>> {
>> - return devnode_nums[vfl_type];
>> + int idx = (vfl_type == VFL_TYPE_METADATA) ? VFL_TYPE_GRABBER : vfl_type;
>> +
>> + return devnode_nums[idx];
>> }
>> #endif
>>
>> @@ -540,6 +543,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
>> bool is_radio = vdev->vfl_type == VFL_TYPE_RADIO;
>> bool is_sdr = vdev->vfl_type == VFL_TYPE_SDR;
>> bool is_tch = vdev->vfl_type == VFL_TYPE_TOUCH;
>> + bool is_meta = vdev->vfl_type == VFL_TYPE_METADATA;
>> bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
>> bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
>>
>> @@ -571,8 +575,10 @@ static void determine_valid_ioctls(struct video_device *vdev)
>> set_bit(_IOC_NR(VIDIOC_TRY_EXT_CTRLS), valid_ioctls);
>> if (vdev->ctrl_handler || ops->vidioc_querymenu)
>> set_bit(_IOC_NR(VIDIOC_QUERYMENU), valid_ioctls);
>> - SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
>> - SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
>> + if (!is_meta) {
>> + SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
>> + SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
>> + }
>> SET_VALID_IOCTL(ops, VIDIOC_LOG_STATUS, vidioc_log_status);
>> #ifdef CONFIG_VIDEO_ADV_DEBUG
>> set_bit(_IOC_NR(VIDIOC_DBG_G_CHIP_INFO), valid_ioctls);
>> @@ -589,37 +595,29 @@ static void determine_valid_ioctls(struct video_device *vdev)
>> if (is_vid || is_tch) {
>> /* video and metadata specific ioctls */
>> if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
>> - ops->vidioc_enum_fmt_vid_overlay ||
>> - ops->vidioc_enum_fmt_meta_cap)) ||
>> - (is_tx && (ops->vidioc_enum_fmt_vid_out ||
>> - ops->vidioc_enum_fmt_meta_out)))
>> + ops->vidioc_enum_fmt_vid_overlay)) ||
>> + (is_tx && ops->vidioc_enum_fmt_vid_out))
>> set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
>> if ((is_rx && (ops->vidioc_g_fmt_vid_cap ||
>> ops->vidioc_g_fmt_vid_cap_mplane ||
>> - ops->vidioc_g_fmt_vid_overlay ||
>> - ops->vidioc_g_fmt_meta_cap)) ||
>> + ops->vidioc_g_fmt_vid_overlay)) ||
>> (is_tx && (ops->vidioc_g_fmt_vid_out ||
>> ops->vidioc_g_fmt_vid_out_mplane ||
>> - ops->vidioc_g_fmt_vid_out_overlay ||
>> - ops->vidioc_g_fmt_meta_out)))
>> + ops->vidioc_g_fmt_vid_out_overlay)))
>> set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
>> if ((is_rx && (ops->vidioc_s_fmt_vid_cap ||
>> ops->vidioc_s_fmt_vid_cap_mplane ||
>> - ops->vidioc_s_fmt_vid_overlay ||
>> - ops->vidioc_s_fmt_meta_cap)) ||
>> + ops->vidioc_s_fmt_vid_overlay)) ||
>> (is_tx && (ops->vidioc_s_fmt_vid_out ||
>> ops->vidioc_s_fmt_vid_out_mplane ||
>> - ops->vidioc_s_fmt_vid_out_overlay ||
>> - ops->vidioc_s_fmt_meta_out)))
>> + ops->vidioc_s_fmt_vid_out_overlay)))
>> set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
>> if ((is_rx && (ops->vidioc_try_fmt_vid_cap ||
>> ops->vidioc_try_fmt_vid_cap_mplane ||
>> - ops->vidioc_try_fmt_vid_overlay ||
>> - ops->vidioc_try_fmt_meta_cap)) ||
>> + ops->vidioc_try_fmt_vid_overlay)) ||
>> (is_tx && (ops->vidioc_try_fmt_vid_out ||
>> ops->vidioc_try_fmt_vid_out_mplane ||
>> - ops->vidioc_try_fmt_vid_out_overlay ||
>> - ops->vidioc_try_fmt_meta_out)))
>> + ops->vidioc_try_fmt_vid_out_overlay)))
>> set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
>> SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
>> SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
>> @@ -679,9 +677,23 @@ 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) {
>> + /* metadata specific ioctls */
>> + if ((is_rx && ops->vidioc_enum_fmt_meta_cap) ||
>> + (is_tx && ops->vidioc_enum_fmt_meta_out))
>> + set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
>> + if ((is_rx && ops->vidioc_g_fmt_meta_cap) ||
>> + (is_tx && ops->vidioc_g_fmt_meta_out))
>> + set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
>> + if ((is_rx && ops->vidioc_s_fmt_meta_cap) ||
>> + (is_tx && ops->vidioc_s_fmt_meta_out))
>> + set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
>> + if ((is_rx && ops->vidioc_try_fmt_meta_cap) ||
>> + (is_tx && ops->vidioc_try_fmt_meta_out))
>> + set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
>
> After some more testing I realized that metadata devices should still support
> ENUM/G/S_INPUT for capture and ENUM/S/G_OUTPUT for output.
>
> Can you post a v4 with support for that?
Hmm, now the compliance test fails if that's added. Let me dig a bit deeper
into this.
Regards,
Hans
>
> Thanks!
>
> Hans
>
>> }
>>
>> - if (is_vid || is_vbi || is_sdr || is_tch) {
>> + if (is_vid || is_vbi || is_sdr || is_tch || 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);
>> @@ -734,7 +746,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
>> SET_VALID_IOCTL(ops, VIDIOC_G_MODULATOR, vidioc_g_modulator);
>> SET_VALID_IOCTL(ops, VIDIOC_S_MODULATOR, vidioc_s_modulator);
>> }
>> - if (is_rx) {
>> + if (is_rx && !is_meta) {
>> /* receiver only ioctls */
>> SET_VALID_IOCTL(ops, VIDIOC_G_TUNER, vidioc_g_tuner);
>> SET_VALID_IOCTL(ops, VIDIOC_S_TUNER, vidioc_s_tuner);
>> @@ -762,6 +774,7 @@ static int video_register_media_controller(struct video_device *vdev)
>>
>> switch (vdev->vfl_type) {
>> case VFL_TYPE_GRABBER:
>> + case VFL_TYPE_METADATA:
>> intf_type = MEDIA_INTF_T_V4L_VIDEO;
>> vdev->entity.function = MEDIA_ENT_F_IO_V4L;
>> break;
>> @@ -870,6 +883,7 @@ int __video_register_device(struct video_device *vdev,
>> /* Part 1: check device type */
>> switch (type) {
>> case VFL_TYPE_GRABBER:
>> + case VFL_TYPE_METADATA:
>> name_base = "video";
>> break;
>> case VFL_TYPE_VBI:
>> @@ -914,6 +928,7 @@ int __video_register_device(struct video_device *vdev,
>> * (new style). */
>> switch (type) {
>> case VFL_TYPE_GRABBER:
>> + case VFL_TYPE_METADATA:
>> minor_offset = 0;
>> minor_cnt = 64;
>> break;
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 51b912743f0f..0d71c06c82cf 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -938,6 +938,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
>> bool is_vbi = vfd->vfl_type == VFL_TYPE_VBI;
>> bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
>> bool is_tch = vfd->vfl_type == VFL_TYPE_TOUCH;
>> + bool is_meta = vfd->vfl_type == VFL_TYPE_METADATA;
>> bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
>> bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
>>
>> @@ -996,11 +997,11 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
>> return 0;
>> break;
>> case V4L2_BUF_TYPE_META_CAPTURE:
>> - if (is_vid && is_rx && ops->vidioc_g_fmt_meta_cap)
>> + if (is_meta && is_rx && ops->vidioc_g_fmt_meta_cap)
>> return 0;
>> break;
>> case V4L2_BUF_TYPE_META_OUTPUT:
>> - if (is_vid && is_tx && ops->vidioc_g_fmt_meta_out)
>> + if (is_meta && is_tx && ops->vidioc_g_fmt_meta_out)
>> return 0;
>> break;
>> default:
>> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
>> index 48531e57cc5a..2da91d454c10 100644
>> --- a/include/media/v4l2-dev.h
>> +++ b/include/media/v4l2-dev.h
>> @@ -30,6 +30,7 @@
>> * @VFL_TYPE_SUBDEV: for V4L2 subdevices
>> * @VFL_TYPE_SDR: for Software Defined Radio tuners
>> * @VFL_TYPE_TOUCH: for touch sensors
>> + * @VFL_TYPE_METADATA: for Metadata device
>> * @VFL_TYPE_MAX: number of VFL types, must always be last in the enum
>> */
>> enum vfl_devnode_type {
>> @@ -39,6 +40,7 @@ enum vfl_devnode_type {
>> VFL_TYPE_SUBDEV,
>> VFL_TYPE_SDR,
>> VFL_TYPE_TOUCH,
>> + VFL_TYPE_METADATA,
>> VFL_TYPE_MAX /* Shall be the last one */
>> };
>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-17 9:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-12 7:36 [PATCH] v4l2-dev: refine selection of valid v4l-touch ioctls Hans Verkuil
2019-09-12 8:06 ` [PATCH v2] v4l2-core: Add metadata type to vfl_devnode_type Vandana BN
2019-09-12 14:02 ` Hans Verkuil
2019-09-13 6:57 ` [PATCH v3] " Vandana BN
2019-09-17 9:54 ` Hans Verkuil
2019-09-17 9:56 ` Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).