[libcamera-devel,v2,1/2] libcamera: camera_sensor: Print warning when orientation is unknown

11352 diff mbox series

Print a warning when the orientation of a sensor is unknown. The
location property is still defaulted to external.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

---
Changes in v2:
- expand the warning message
---
 src/libcamera/camera_sensor.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

Hi Paul,

Thank you for the patch.

On Mon, Feb 22, 2021 at 05:01:11PM +0900, Paul Elder wrote:
> Print a warning when the orientation of a sensor is unknown. The
> location property is still defaulted to external.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
> Changes in v2:
> - expand the warning message
> ---
>  src/libcamera/camera_sensor.cpp | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index c9e8d49b..d7785f63 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -446,6 +446,12 @@ int CameraSensor::initProperties()
>  			break;
>  		}
>  	} else {
> +		LOG(CameraSensor, Warning)
> +			<< "Failed to retrieve the camera location, setting to External";
> +		LOG(CameraSensor, Warning)
> +			<< "The sensor kernel driver needs to be fixed";
> +		LOG(CameraSensor, Warning)
> +			<< "See Documentation/sensor_driver_requirements.rst in the libcamera sources for more information";
>  		propertyValue = properties::CameraLocationExternal;
>  	}
>  	properties_.set(properties::Location, propertyValue);

As an alternative approach, to tap into the existing infrastructure, how
about this ?

diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
index 0ea54a088a40..1987e84bfb9a 100644
--- a/src/libcamera/camera_sensor.cpp
+++ b/src/libcamera/camera_sensor.cpp
@@ -276,12 +276,13 @@ int CameraSensor::init()

 int CameraSensor::validateSensorDriver()
 {
+	int err = 0;
+
 	/*
 	 * Optional controls are used to register optional sensor properties. If
 	 * not present, some values will be defaulted.
 	 */
 	static constexpr uint32_t optionalControls[] = {
-		V4L2_CID_CAMERA_ORIENTATION,
 		V4L2_CID_CAMERA_SENSOR_ROTATION,
 	};

@@ -293,6 +294,24 @@ int CameraSensor::validateSensorDriver()
 				<< " not supported";
 	}

+	/*
+	 * Recommended controls are similar to optional controls, but will
+	 * become mandatory in the near future. Be loud if they're missing.
+	 */
+	static constexpr uint32_t recommendedControls[] = {
+		V4L2_CID_CAMERA_ORIENTATION,
+	};
+
+	const ControlIdMap &controls = subdev_->controls().idmap();
+	for (uint32_t ctrl : recommendedControls) {
+		if (!controls.count(ctrl)) {
+			LOG(CameraSensor, Warning)
+				<< "Recommended V4L2 control " << utils::hex(ctrl)
+				<< " not supported";
+			err = -EINVAL;
+		}
+	}
+
 	/*
 	 * Make sure the required selection targets are supported.
 	 *
@@ -303,7 +322,6 @@ int CameraSensor::validateSensorDriver()
 	 * \todo Make support for selection targets mandatory as soon as all
 	 * test platforms have been updated.
 	 */
-	int err = 0;
 	Rectangle rect;
 	int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect);
 	if (ret) {