[RFC] Querycap for subdevs, finding MC from device nodes

[RFC] Querycap for subdevs, finding MC from device nodes

[Hans Verkuil]

I've been thinking about extending v4l2-compliance with v4l-subdev tests.

However, there are a few missing pieces that are needed before this can be done.

First of all is that there is no subdev equivalent to VIDIOC_QUERYCAP, i.e. an
ioctl that is guaranteed to always be available.

So I propose the following ioctl:

VIDIOC_SUBDEV_QUERYCAP(struct v4l2_subdev_capability);

struct v4l2_subdev_capability {
	char name[V4L2_SUBDEV_NAME_SIZE];
	__u32 version;			// same as KERNEL_VERSION like QUERYCAP does
	__u32 device_caps;
	__u32 pads;
	__u32 entity_id;
	__u32 reserved[40];
};

/* This v4l2_subdev is also a media entity and the entity_id field is valid */
#define V4L2_SUBDEV_CAP_ENTITY		(1 << 0)

This will allow v4l2-compliance to discover that this is a bona fide v4l2 subdevice.
All the information in the struct above can be filled in by v4l2-subdev.c, no need
to change drivers.

The reason I included 'pads' as well is that subdev drivers can have a devnode
without being an entity, but still support ioctls like VIDIOC_SUBDEV_ENUM_MBUS_CODE.
In that case v4l2-compliance needs to know the number of pads in order to properly
test. If it is an entity, then the entity information can be obtained from the MC,
see below how to find the MC.

Note: I think it makes sense to extend VIDIOC_QUERYCAP as well with a CAP_ENTITY
and an entity_id.

The next step is to be able to associate a media device with a v4l2-subdev.

Originally I though of adding the major and minor numbers of the media device
to the capability struct, but I'd like to do that for VIDIOC_QUERYCAP as well,
and there are only 3 reserved fields (2 after taking one for the entity_id).

Instead I think we can just implement the MEDIA_IOC_DEVICE_INFO ioctl: this
returns all info about the media_device, and it can easily be extended to
include the major and minor number of that device.

Again, supporting this is easily done in the core, both for regular video nodes
and for subdev nodes, so drivers do not need to be changed.

This way you can easily find whether a V4L2 device node is an entity and what
the associated media controller is. All the information is available, we just
need to expose it.

Comments?

Regards,

	Hans

Re: [RFC] Querycap for subdevs, finding MC from device nodes

[Laurent Pinchart]

Hi Hans,

On Saturday 21 March 2015 09:41:29 Hans Verkuil wrote:
> I've been thinking about extending v4l2-compliance with v4l-subdev tests.
> 
> However, there are a few missing pieces that are needed before this can be
> done.
> 
> First of all is that there is no subdev equivalent to VIDIOC_QUERYCAP, i.e.
> an ioctl that is guaranteed to always be available.
> 
> So I propose the following ioctl:
> 
> VIDIOC_SUBDEV_QUERYCAP(struct v4l2_subdev_capability);
> 
> struct v4l2_subdev_capability {
> 	char name[V4L2_SUBDEV_NAME_SIZE];
> 	__u32 version;			// same as KERNEL_VERSION like QUERYCAP does
> 	__u32 device_caps;
> 	__u32 pads;
> 	__u32 entity_id;
> 	__u32 reserved[40];
> };
> 
> /* This v4l2_subdev is also a media entity and the entity_id field is valid
> */ #define V4L2_SUBDEV_CAP_ENTITY		(1 << 0)
> 
> This will allow v4l2-compliance to discover that this is a bona fide v4l2
> subdevice. All the information in the struct above can be filled in by
> v4l2-subdev.c, no need to change drivers.

This looks quite good to me. There's a prerequisite though, we need to 
formally define the naming scheme for subdevs, otherwise we'll expose yet 
another ill-defined name that will cause issues later.

> The reason I included 'pads' as well is that subdev drivers can have a
> devnode without being an entity, but still support ioctls like
> VIDIOC_SUBDEV_ENUM_MBUS_CODE. In that case v4l2-compliance needs to know
> the number of pads in order to properly test. If it is an entity, then the
> entity information can be obtained from the MC, see below how to find the
> MC.

Shouldn't we make it mandatory for subdevs to be entities if they want to 
expose a subdev node ? Otherwise applications won't be able to find out how 
the subdev relates to other subdevs and v4l2 devices, making the API quite 
shaky in my opinion.

> Note: I think it makes sense to extend VIDIOC_QUERYCAP as well with a
> CAP_ENTITY and an entity_id.
> 
> The next step is to be able to associate a media device with a v4l2-subdev.
> 
> Originally I though of adding the major and minor numbers of the media
> device to the capability struct, but I'd like to do that for
> VIDIOC_QUERYCAP as well, and there are only 3 reserved fields (2 after
> taking one for the entity_id).
> 
> Instead I think we can just implement the MEDIA_IOC_DEVICE_INFO ioctl: this
> returns all info about the media_device, and it can easily be extended to
> include the major and minor number of that device.

Hmmm... I'm not too found of that approach. Can't userspace find the 
corresponding media device through sysfs ? I'd like to point that the common 
use case is to start from the media device and find the entities. Finding the 
media device from a subdev is useful mostly for test tools, but it's not the 
main use case.

> Again, supporting this is easily done in the core, both for regular video
> nodes and for subdev nodes, so drivers do not need to be changed.
> 
> This way you can easily find whether a V4L2 device node is an entity and
> what the associated media controller is. All the information is available,
> we just need to expose it.
> 
> Comments?

Please see above :-)

-- 
Regards,

Laurent Pinchart

Re: [RFC] Querycap for subdevs, finding MC from device nodes

[Hans Verkuil]

Hi Laurent,

On 04/17/2015 05:07 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Saturday 21 March 2015 09:41:29 Hans Verkuil wrote:
>> I've been thinking about extending v4l2-compliance with v4l-subdev tests.
>>
>> However, there are a few missing pieces that are needed before this can be
>> done.
>>
>> First of all is that there is no subdev equivalent to VIDIOC_QUERYCAP, i.e.
>> an ioctl that is guaranteed to always be available.
>>
>> So I propose the following ioctl:
>>
>> VIDIOC_SUBDEV_QUERYCAP(struct v4l2_subdev_capability);
>>
>> struct v4l2_subdev_capability {
>> 	char name[V4L2_SUBDEV_NAME_SIZE];
>> 	__u32 version;			// same as KERNEL_VERSION like QUERYCAP does
>> 	__u32 device_caps;
>> 	__u32 pads;
>> 	__u32 entity_id;
>> 	__u32 reserved[40];
>> };
>>
>> /* This v4l2_subdev is also a media entity and the entity_id field is valid
>> */ #define V4L2_SUBDEV_CAP_ENTITY		(1 << 0)
>>
>> This will allow v4l2-compliance to discover that this is a bona fide v4l2
>> subdevice. All the information in the struct above can be filled in by
>> v4l2-subdev.c, no need to change drivers.
> 
> This looks quite good to me. There's a prerequisite though, we need to 
> formally define the naming scheme for subdevs, otherwise we'll expose yet 
> another ill-defined name that will cause issues later.

If memory serves I proposed in San Jose when we discussed this to leave the
'name' field out for this initial version. From the point of view of
v4l2-compliance all it really needs is 'pads' and a single guaranteed-to-
exist ioctl.

>> The reason I included 'pads' as well is that subdev drivers can have a
>> devnode without being an entity, but still support ioctls like
>> VIDIOC_SUBDEV_ENUM_MBUS_CODE. In that case v4l2-compliance needs to know
>> the number of pads in order to properly test. If it is an entity, then the
>> entity information can be obtained from the MC, see below how to find the
>> MC.
> 
> Shouldn't we make it mandatory for subdevs to be entities if they want to 
> expose a subdev node ? Otherwise applications won't be able to find out how 
> the subdev relates to other subdevs and v4l2 devices, making the API quite 
> shaky in my opinion.

I am inclined to agree with you. I just need time (hah!) to sit down and look
carefully through the API and if this would cause any problems. My gut says it
doesn't and that it would actually simplify things.

>> Note: I think it makes sense to extend VIDIOC_QUERYCAP as well with a
>> CAP_ENTITY and an entity_id.
>>
>> The next step is to be able to associate a media device with a v4l2-subdev.
>>
>> Originally I though of adding the major and minor numbers of the media
>> device to the capability struct, but I'd like to do that for
>> VIDIOC_QUERYCAP as well, and there are only 3 reserved fields (2 after
>> taking one for the entity_id).
>>
>> Instead I think we can just implement the MEDIA_IOC_DEVICE_INFO ioctl: this
>> returns all info about the media_device, and it can easily be extended to
>> include the major and minor number of that device.
> 
> Hmmm... I'm not too found of that approach. Can't userspace find the 
> corresponding media device through sysfs ? I'd like to point that the common 
> use case is to start from the media device and find the entities. Finding the 
> media device from a subdev is useful mostly for test tools, but it's not the 
> main use case.

I've never been happy with the idea of applications having to hunt through
sysfs trying to find a device node. And in general I think it is good to be
able to be able to go in both directions. However, this isn't the most urgent
task for now, so I'll drop it for the first version. Right now I just want to
start working on tests for v4l-subdev nodes.

Regards,

	Hans

>> Again, supporting this is easily done in the core, both for regular video
>> nodes and for subdev nodes, so drivers do not need to be changed.
>>
>> This way you can easily find whether a V4L2 device node is an entity and
>> what the associated media controller is. All the information is available,
>> we just need to expose it.
>>
>> Comments?
> 
> Please see above :-)
> 

Re: [RFC] Querycap for subdevs, finding MC from device nodes

[Laurent Pinchart]

Hi Hans,

On Monday 20 April 2015 10:37:59 Hans Verkuil wrote:
> On 04/17/2015 05:07 PM, Laurent Pinchart wrote:
> > On Saturday 21 March 2015 09:41:29 Hans Verkuil wrote:
> >> I've been thinking about extending v4l2-compliance with v4l-subdev tests.
> >> 
> >> However, there are a few missing pieces that are needed before this can
> >> be done.
> >> 
> >> First of all is that there is no subdev equivalent to VIDIOC_QUERYCAP,
> >> i.e. an ioctl that is guaranteed to always be available.
> >> 
> >> So I propose the following ioctl:
> >> 
> >> VIDIOC_SUBDEV_QUERYCAP(struct v4l2_subdev_capability);
> >> 
> >> struct v4l2_subdev_capability {
> >> 	char name[V4L2_SUBDEV_NAME_SIZE];
> >> 	__u32 version;			// same as KERNEL_VERSION like QUERYCAP 
does
> >> 	__u32 device_caps;
> >> 	__u32 pads;
> >> 	__u32 entity_id;
> >> 	__u32 reserved[40];
> >> 
> >> };
> >> 
> >> /* This v4l2_subdev is also a media entity and the entity_id field is
> >> valid
> >> */ #define V4L2_SUBDEV_CAP_ENTITY		(1 << 0)
> >> 
> >> This will allow v4l2-compliance to discover that this is a bona fide v4l2
> >> subdevice. All the information in the struct above can be filled in by
> >> v4l2-subdev.c, no need to change drivers.
> > 
> > This looks quite good to me. There's a prerequisite though, we need to
> > formally define the naming scheme for subdevs, otherwise we'll expose yet
> > another ill-defined name that will cause issues later.
> 
> If memory serves I proposed in San Jose when we discussed this to leave the
> 'name' field out for this initial version.

You're right. Thanks for refreshing my memory :-)

> From the point of view of v4l2-compliance all it really needs is 'pads' and
> a single guaranteed-to-exist ioctl.
> 
> >> The reason I included 'pads' as well is that subdev drivers can have a
> >> devnode without being an entity, but still support ioctls like
> >> VIDIOC_SUBDEV_ENUM_MBUS_CODE. In that case v4l2-compliance needs to know
> >> the number of pads in order to properly test. If it is an entity, then
> >> the entity information can be obtained from the MC, see below how to find
> >> the MC.
> > 
> > Shouldn't we make it mandatory for subdevs to be entities if they want to
> > expose a subdev node ? Otherwise applications won't be able to find out
> > how the subdev relates to other subdevs and v4l2 devices, making the API
> > quite shaky in my opinion.
> 
> I am inclined to agree with you. I just need time (hah!) to sit down and
> look carefully through the API and if this would cause any problems. My gut
> says it doesn't and that it would actually simplify things.
> 
> >> Note: I think it makes sense to extend VIDIOC_QUERYCAP as well with a
> >> CAP_ENTITY and an entity_id.
> >> 
> >> The next step is to be able to associate a media device with a
> >> v4l2-subdev.
> >> 
> >> Originally I though of adding the major and minor numbers of the media
> >> device to the capability struct, but I'd like to do that for
> >> VIDIOC_QUERYCAP as well, and there are only 3 reserved fields (2 after
> >> taking one for the entity_id).
> >> 
> >> Instead I think we can just implement the MEDIA_IOC_DEVICE_INFO ioctl:
> >> this returns all info about the media_device, and it can easily be
> >> extended to include the major and minor number of that device.
> > 
> > Hmmm... I'm not too found of that approach. Can't userspace find the
> > corresponding media device through sysfs ? I'd like to point that the
> > common use case is to start from the media device and find the entities.
> > Finding the media device from a subdev is useful mostly for test tools,
> > but it's not the main use case.
> 
> I've never been happy with the idea of applications having to hunt through
> sysfs trying to find a device node. And in general I think it is good to be
> able to be able to go in both directions. However, this isn't the most
> urgent task for now, so I'll drop it for the first version. Right now I
> just want to start working on tests for v4l-subdev nodes.

I'm not a big fan of open-coded sysfs support in applications either, but the 
need might be limited to test tools here.

> >> Again, supporting this is easily done in the core, both for regular video
> >> nodes and for subdev nodes, so drivers do not need to be changed.
> >> 
> >> This way you can easily find whether a V4L2 device node is an entity and
> >> what the associated media controller is. All the information is
> >> available, we just need to expose it.
> >> 
> >> Comments?
> > 
> > Please see above :-)

-- 
Regards,

Laurent Pinchart