imx219: Recommended V4L2 control 0x009a0922 not supported (V4L2_CID_CAMERA_ORIENTATION)

[dwrobel]

On Raspberry Pi 4 with:

• dtoverlay=imx219 and camera connected,
• kernel 5.10.52: 7aa66c0,
• latest libcamera: https://github.com/libcamera-org/libcamera/tree/b40a8d4b454008aeab4c0eb1f63a07083d7d7c74,

lc-compliance is not able to detect support for recommended ioctls and returns with EINVAL effectively preventing any libcamera based software from working:

$ lc-compliance
[0:06:10.105502307] [1581]  INFO Camera camera_manager.cpp:294 libcamera v0.0.0
[0:06:10.117127528] [1585]  WARN CameraSensor camera_sensor.cpp:197 'imx219 10-0010': Recommended V4L2 control 0x009a0922 not supported
[0:06:10.117350062] [1585]  WARN CameraSensor camera_sensor.cpp:249 'imx219 10-0010': The sensor kernel driver needs to be fixed
[0:06:10.117488097] [1585]  WARN CameraSensor camera_sensor.cpp:251 'imx219 10-0010': See Documentation/sensor_driver_requirements.rst in the libcamera sources for more information
[0:06:10.118380397] [1585]  WARN CameraSensor camera_sensor.cpp:403 'imx219 10-0010': Failed to retrieve the camera location

when I temporarily patched libcamera sources as following:

diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
index cde431cc..fe1bd508 100644
--- a/src/libcamera/camera_sensor.cpp
+++ b/src/libcamera/camera_sensor.cpp
@@ -197,7 +197,6 @@ int CameraSensor::validateSensorDriver()
                        LOG(CameraSensor, Warning)
                                << "Recommended V4L2 control " << utils::hex(ctrl)
                                << " not supported";
-                       err = -EINVAL;
                }
        }

then it started to work:

$ lc-compliance -c /base/soc/i2c0mux/i2c@1/imx219@10
[0:33:12.444359909] [8055]  INFO Camera camera_manager.cpp:294 libcamera v0.0.0+2-71c56bc3
[0:33:12.456142707] [8058]  WARN CameraSensor camera_sensor.cpp:197 'imx219 10-0010': Recommended V4L2 control 0x009a0922 not supported
[0:33:12.456925511] [8058]  WARN CameraSensor camera_sensor.cpp:402 'imx219 10-0010': Failed to retrieve the camera location
Using camera /base/soc/i2c0mux/i2c@1/imx219@10
[==========] Running 120 tests from 1 test suite.

.
.
.

[----------] Global test environment tear-down
[==========] 120 tests from 1 test suite ran. (417423 ms total)
[  PASSED  ] 106 tests.
[  SKIPPED ] 14 tests, listed below:
[  SKIPPED ] CaptureTests/SingleStream.Capture/Raw_1
[  SKIPPED ] CaptureTests/SingleStream.Capture/VideoRecording_1
[  SKIPPED ] CaptureTests/SingleStream.Capture/VideoRecording_2
[  SKIPPED ] CaptureTests/SingleStream.Capture/VideoRecording_3
[  SKIPPED ] CaptureTests/SingleStream.Capture/Viewfinder_1
[  SKIPPED ] CaptureTests/SingleStream.Capture/Viewfinder_2
[  SKIPPED ] CaptureTests/SingleStream.Capture/Viewfinder_3
[  SKIPPED ] CaptureTests/SingleStream.CaptureStartStop/Raw_1
[  SKIPPED ] CaptureTests/SingleStream.CaptureStartStop/VideoRecording_1
[  SKIPPED ] CaptureTests/SingleStream.CaptureStartStop/VideoRecording_2
[  SKIPPED ] CaptureTests/SingleStream.CaptureStartStop/VideoRecording_3
[  SKIPPED ] CaptureTests/SingleStream.CaptureStartStop/Viewfinder_1
[  SKIPPED ] CaptureTests/SingleStream.CaptureStartStop/Viewfinder_2
[  SKIPPED ] CaptureTests/SingleStream.CaptureStartStop/Viewfinder_3

To avoid patching libcamera specifically for Raspberry Pi would it be possible to satisfy libcamera sensor driver requirements?

[6by9]

The driver does support it via the call to v4l2_ctrl_new_fwnode_properties reading the configuration out of device tree.

The default overlay then defines the rotation - https://github.com/raspberrypi/linux/blob/rpi-5.10.y/arch/arm/boot/dts/overlays/imx219-overlay.dts#L30

On my system:

pi@raspberrypi:~ $ v4l2-ctl --list-ctrls

User Controls

                       exposure 0x00980911 (int)    : min=4 max=3522 step=1 default=1600 value=1600
                horizontal_flip 0x00980914 (bool)   : default=0 value=0 flags=modify-layout
                  vertical_flip 0x00980915 (bool)   : default=0 value=0 flags=modify-layout

Camera Controls

         camera_sensor_rotation 0x009a0923 (int)    : min=180 max=180 step=1 default=180 value=180 flags=read-only

Image Source Controls

              vertical_blanking 0x009e0901 (int)    : min=4 max=63071 step=1 default=1062 value=1062
            horizontal_blanking 0x009e0902 (int)    : min=168 max=168 step=1 default=168 value=168 flags=read-only
                  analogue_gain 0x009e0903 (int)    : min=0 max=232 step=1 default=0 value=0
                red_pixel_value 0x009e0904 (int)    : min=0 max=1023 step=1 default=1023 value=1023
          green_red_pixel_value 0x009e0905 (int)    : min=0 max=1023 step=1 default=1023 value=1023
               blue_pixel_value 0x009e0906 (int)    : min=0 max=1023 step=1 default=1023 value=1023
         green_blue_pixel_value 0x009e0907 (int)    : min=0 max=1023 step=1 default=1023 value=1023

Image Processing Controls

                     pixel_rate 0x009f0902 (int64)  : min=182400000 max=182400000 step=1 default=182400000 value=182400000 flags=read-only
                   test_pattern 0x009f0903 (menu)   : min=0 max=4 default=0 value=0
                   digital_gain 0x009f0905 (int)    : min=256 max=4095 step=1 default=256 value=256

Odd that it's reported there as 0x009a0923 when you reported 0x009a0922.

It's defined in https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/v4l2-controls.h#L995 (mainline) as (V4L2_CID_CAMERA_CLASS_BASE+35) = (V4L2_CTRL_CLASS_CAMERA | 0x900) + 35 = 0x009a0000 | 0x900 + 35 = 0x009a0923.

It looks like your kernel headers are incorrect somehow.
libcamera uses an internal copy of the headers, and it is also correct in being V4L2_CID_CAMERA_CLASS_BASE+35 - https://git.linuxtv.org/libcamera.git/tree/include/linux/v4l2-controls.h#n987

[dwrobel]

Thank you for the prompt and comprehensive response.

On my system:

pi@raspberrypi:~ $ v4l2-ctl --list-ctrls

At least that looks consistent with my system, where 0x009a0923 is reported:

$ v4l2-ctl --list-ctrls

User Controls

                       exposure 0x00980911 (int)    : min=4 max=2464 step=1 default=1600 value=1058
                horizontal_flip 0x00980914 (bool)   : default=0 value=1 flags=modify-layout
                  vertical_flip 0x00980915 (bool)   : default=0 value=1 flags=modify-layout

Camera Controls

         camera_sensor_rotation 0x009a0923 (int)    : min=180 max=180 step=1 default=180 value=180 flags=read-only

Image Source Controls

              vertical_blanking 0x009e0901 (int)    : min=4 max=63071 step=1 default=1062 value=4
            horizontal_blanking 0x009e0902 (int)    : min=168 max=168 step=1 default=168 value=168 flags=read-only
                  analogue_gain 0x009e0903 (int)    : min=0 max=232 step=1 default=0 value=0
                red_pixel_value 0x009e0904 (int)    : min=0 max=1023 step=1 default=1023 value=1023
          green_red_pixel_value 0x009e0905 (int)    : min=0 max=1023 step=1 default=1023 value=1023
               blue_pixel_value 0x009e0906 (int)    : min=0 max=1023 step=1 default=1023 value=1023
         green_blue_pixel_value 0x009e0907 (int)    : min=0 max=1023 step=1 default=1023 value=1023

Image Processing Controls

                     pixel_rate 0x009f0902 (int64)  : min=182400000 max=182400000 step=1 default=182400000 value=182400000 flags=read-only
                   test_pattern 0x009f0903 (menu)   : min=0 max=4 default=0 value=0
                   digital_gain 0x009f0905 (int)    : min=256 max=4095 step=1 default=256 value=256

so I'll have to look into libcamera, I guess?

[6by9]

so I'll have to look into libcamera, I guess?

Yes, check intially that the include/linux/v4l2-controls.h file is sensible.

[dwrobel]

I was my fault to put in the title of the issue V4L2_CID_CAMERA_SENSOR_ROTATION instead of V4L2_CID_CAMERA_ORIENTATION. I'm sorry for the confusion.

Now everything seems to be consistent the driver is lacking support for:
#define V4L2_CID_CAMERA_ORIENTATION (V4L2_CID_CAMERA_CLASS_BASE+34)

The question is, could this be added to avoid patching libcamera like here?

[6by9]

Again it comes from device tree via a property named orientation.

@naushir @davidplowman Presumably this is working for you?

It looks to me like V4L2_CID_CAMERA_ORIENTATION is a recommended control, not mandatory. The reset of err at https://git.linuxtv.org/libcamera.git/tree/src/libcamera/camera_sensor.cpp#n269 should mean that validateSensorDriver doesn't fail.

It looks to be a dt overlay change being required to add (probably)

orientation = <2>;

to https://github.com/raspberrypi/linux/blob/rpi-5.10.y/arch/arm/boot/dts/overlays/imx219-overlay.dts#L30.
2 is external, 1 is back, 0 is front, so it probably ought to have an override too.

[naushir]

I do see the following when running on my device:

WARN CameraSensor camera_sensor.cpp:197 'imx219 10-0010': Recommended V4L2 control 0x009a0922 not supported

However, the consequence is the warning message, and apps seem to run fine. Not tried lc-compliance though!
Given that it is a recommended control (rather than mandatory), perhaps returning -EINVAL when it is missing is not the right thing to do here? I suppose that is up to the libcamera folks to decide that, but maybe we add the orientation flag to our overlay files now? This would stop the warning, and if (when?) this does get promoted to a mandatory control, we should not see any breakages.

[dwrobel]

perhaps returning -EINVAL when it is missing is not the right thing to do here?

As per https://github.com/libcamera-org/libcamera/blob/b40a8d4b454008aeab4c0eb1f63a07083d7d7c74/Documentation/sensor_driver_requirements.rst

Support for the selection API is scheduled to become a mandatory feature in the near future.

Also the source code is mentioning https://github.com/libcamera-org/libcamera/blob/bdf04cca086eb72a9e0528ba90a5c53d96e52a01/src/libcamera/camera_sensor.cpp#L188-L189

Recommended controls are similar to optional controls, but will
become mandatory in the near future. Be loud if they're missing.

[6by9]

but maybe we add the orientation flag to our overlay files now? This would stop the warning, and iff this does get promoted to a mandatory control, we should not see any breakages.

I was just thinking the same. PR incoming.

[6by9]

#4501 adds the relevant overlay parameters, and fixes up imx290/327, ov9281, and ov7251 to read the relevant properties from DT.