[Linux-kernel-mentees] [PATCH v3] v4l2-core: Add metadata type to vfl_devnode_type

Hans Verkuil hverkuil at xs4all.nl
Tue Sep 17 09:56:58 UTC 2019


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 at 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 */
>>  };
>>  
>>
> 



More information about the Linux-kernel-mentees mailing list