On 07/28/2017 04:28 PM, Laurent Pinchart wrote: > Hi Hans, > > On Friday 28 Jul 2017 16:04:48 Hans Verkuil wrote: >> On 07/28/2017 03:25 PM, Laurent Pinchart wrote: >>> On Friday 28 Jul 2017 13:05:27 Hans Verkuil wrote: >>>> From: Hans Verkuil <hans.verkuil@xxxxxxxxx> >>>> >>>> I tried to get this in back in 2015, but that effort stalled. >>>> >>>> Trying again, since I really need this in order to add proper v4l-subdev >>>> support to v4l2-ctl and v4l2-compliance. There currently is no way of >>>> unique identifying that the device really is a v4l-subdev device other >>>> than the device name (which can be changed by udev). >>>> >>>> So this patch series adds a VIDIOC_SUBDEV_QUERYCAP ioctl that is in >>>> the core so it's guaranteed to be there. >>>> >>>> If the subdev is part of an MC then it also gives the corresponding >>>> entity ID of the subdev and the major/minor numbers of the MC device >>>> so v4l2-compliance can relate the subdev device directly to the right >>>> MC device. The reserved array has room enough for more strings should >>>> we need them later, although I think what we have here is sufficient. >>> >>> I still think this is not correct for two reasons. >>> >>> First of all, the new querycap ioctl uses the same ioctl number as >>> VIDIOC_QUERYCAP. The full 32-bit number is different as the structures >>> used for the two ioctls currently have different sizes, but that's not >>> guaranteed going forward when we'll extend the structures used by the two >>> ioctls with new fields. >> >> I think it is extraordinarily unlikely that these two will ever become >> identical. And anyway, we control that ourselves. >> >>> To solve this, if you really want to identify the type of device node at >>> runtime, we should have a single ioctl supported by the two device nodes. >>> Given that we"re running out of capabilities bits for VIDIOC_QUERYCAP, >>> this could be a good occasion to introduce a new ioctl to query >>> capabilities. >> >> This makes more sense :-) >> >>> The second reason is that I don't think we should report the media device >>> node associated with the subdev. Userspace really needs to become >>> MC-centric, starting with the MC device, and going to the video nodes and >>> subdev nodes. The other way around just doesn't make sense to me. >> >> It's not for 'regular' applications. It's to easily find out to which media >> device a /dev/v4l-subdevX belongs. Primarily for applications like >> v4l2-compliance. >> >> We have the information, and right now there is no way to take a v4l-subdevX >> device and determine of which media device it is part other than scanning >> the topologies of all media devices. That's nuts. This is cheap and makes >> life for a certain class of applications much easier. Creating good >> compliance tests is critical and this is a small but important contribution >> to that. > > I fully agree with the need for compliance tools, but I believe they should go > from MC to subdev, not the other way around. Let's discuss this below. > >>> For MC-enabled devices, specifying subdev or video nodes by /dev node name >>> is painful. When you have multiple video devices on the system, or even >>> when you're modifying initialization order in a driver, the devnode names >>> will not be stable across boots. I used to type them out manually in the >>> very beginning and very quickly switched to retrieving them from the >>> subdev entity name in my test scripts. >>> >>> What I'd like the compliance tools to do is to test all video nodes and >>> subdev nodes for a given MC device, with an option to restrict the tests >>> to a subset of the video devices and subdevs specified by media entity >>> name. We obviously need to keep support for addressing video nodes >>> manually as not all devices are MC-enabled, but for subdev we don't have >>> to care about such a backward compatibility issue as there's currently no >>> compliance tool. >> >> I want two things: >> >> 1) v4l2-compliance to be able to test a v4l-subdevX, just in isolation. And >> to be able to find the corresponding media device and make sure that what >> the v4l-subdev does is compatible with the entity/link information from the >> MC. > > Why do you want to test /dev/v4l-subdev$(random) instead of /dev/mediaX + > "subdev entity name" ? > > Furthermore, if you want to test a subdev in complete isolation, why do you > need to know which media device it belongs to ? > > By the way, I'd like Sakari to join the discussion, but he won't be back > before the end of next week. > >> 2) make a media-compliance to look at the media topology as a whole. >> >> Having the major/minor numbers are specifically for 1. >> >> Actually, I really want to have the major/minor numbers of the media device >> for /dev/videoX as well, but entity ID + major + minor number would use up >> the available space in struct v4l2_capability, so your suggestion of making >> a new VIDIOC_EXT_QUERYCAP has merit. >> >>> On a side note, I believe subdev nodes should depend on MC. It has been a >>> historical mistake not to do so, and as far as I can see only three >>> drivers register subdev nodes without registering a media device. They >>> should be fixed if they want to benefit from the compliance tool. >> >> Which ones? > > atmel-isc, cobalt and rcar_drif. Huh, I thought I added media device support to cobalt. I probably started that but never finished it. I can certainly take care of that one, it should not be hard. > >> I'm not opposed to that. It would simplify matters quite a bit. >> >> But I very, very strongly believe that a VIDIOC_EXT_QUERYCAP should return >> the corresponding entity ID and /dev/mediaX major and minor numbers. It's >> very useful information for a certain class of applications. > > Do you have any application in mind other than the compliance tools ? No. v4l2-ctl, v4l2-compliance and perhaps a future media-compliance. Those are the primary use-cases. But even more important it is against all my 'good API sense' that I can't find the corresponding media device from a v4l-subdev/video device. If I can get to them from the media device through major/minor numbers then consistent API design requires that I can also go through the other direction. Especially since I see no downside to this. It's nuts that I can't just pass /dev/v4l-subdevX to v4l2-ctl/compliance and have it find out everything about it automatically. It's all about self-discovery in an API. Sorry, I feel very strongly about this. Regards, Hans > >> Heck, I want to do 'v4l2-ctl -d /dev/video0 -D' or 'v4l2-ctl -d >> /dev/v4l-subdev0' and see not only the device capabilities, but also the >> corresponding entity information. Without having to scan through all >> /dev/media devices or requiring the user to pass the /dev/mediaX device >> separately. >> >> This information is very cheap and I see no reason whatsoever not to >> implement this. It also feels much more symmetrical if I have what is >> effectively a backlink to the media device to which the subdev belongs. >> >> And yes, normally applications will never need this since they use the media >> device and never reference a /dev/v4l-subdevX by name. But for v4l2-ctl and >> v4l2-compliance it is very useful indeed. >