Add new V4L2 controls for quad Bayer sensors

[davidplowman]

@naushir @6by9

I've been looking at one of these "quad Bayer" (some manufacturers call them "tetracell") sensors. These typically have a binned mode of operation where you get a standard Bayer pattern at 1/2 the full width and height, but also a special mode where the non-standard Bayer pattern is converted to a standard one, at the full resolution. For this to work well, the sensor needs to know the colour gains that are going to be applied (by the ISP), but it does not apply them to the pixels itself.

I'll be needing this feature in libcamera which means I need to start the up-streaming process before they can adopt it. A brief discussion the other week suggested it was reasonable to put the change here first.

I don't think there's anything too controversial, but any suggestions or clarifications are of course always welcome. I wasn't entirely sure whether to include a control for both flavours of green (my sensor only wants an overall green gain), but it feels like one of those things where you would be annoyed if you didn't and then one day a sensor pops up that did want them both.

Thanks!

[6by9]

It does seem to vary as to whether docs and support are committed together or as independent commits. eg https://github.com/torvalds/linux/commit/4ad1b0d410c88c7c8e8fd1298c9d2293b651e35c cf https://github.com/torvalds/linux/commit/926645d43fd43622a2b056471a2cf41cc19cbf4c and https://github.com/torvalds/linux/commit/9926c2248740a632b0629fd8c07d0fc361dc15cc

It sort of feels more sensible to add them together, but I've only done a quick survey of commits.

You have missed out the definitions of the type and names from v4l2_ctrl_fill and v4l2_ctrl_get_name. Those do need to be in the commit alongside defining the V4L2_CID_xxx values.

[davidplowman]

Ah, thanks, I had no idea I needed to do that - good idea to start here, then!

v4l2_ctrl_get_name is obvious enough, however, v4l2_ctrl_fill seems not to list any of the other image source ones (like V4L2_CID_ANALOGUE_GAIN), and in fact I have no particular idea what I'd put there for the min/max/step etc. So I'm feeling like it might be OK to ignore that one...? (Actually I see a note in the kernel documentation that it's for backward compatibility now and is going to be un-exported.)

[6by9]

v4l2_ctrl_fill is used to set up default additional parameters, eg READ_ONLY, or a different control type (eg menu). I suspect that your controls are simple integers, so there is no need to add any extra flags in there.

[davidplowman]

Thanks for the feedback. I've pushed a revised version which adds v4l2_ctrl_get_name. As regards the other things, I've tried to follow the newest camera related control that I could find:

926645d43fd43622a2b056471a2cf41cc19cbf4c media: v4l2-ctrls: Add camera orientation and rotation
9926c2248740a632b0629fd8c07d0fc361dc15cc media: v4l2-ctrl: Document V4L2_CID_CAMERA_SENSOR_ROTATION
9397a83f40183eeafd5c787af2240ed0d6b26daa media: v4l2-ctrl: Document V4L2_CID_CAMERA_ORIENTATION

These seem to ignore v4l2_ctrl_fill and have committed the documentation separately, so hopefully I'm OK with that too...

[davidplowman]

Does anyone think there's anything else glaring that needs fixing here - or is it time to try sending this to linux media? Thanks!

[6by9]

LGTM. Ensure you're based off https://git.linuxtv.org/media_tree.git/tree/, ensure checkpatch.pl --strict is clean, and send it off to the list.

[hailfinger]

AFAICS this was merged in mainline Linux 5.16.

[6by9]

Merged in mainline as a single control with an array of values. Closing.