From patchwork Fri Aug 13 14:32:41 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 13346 X-Patchwork-Delegate: kieran.bingham@ideasonboard.com Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 6DF33C3240 for ; Fri, 13 Aug 2021 14:32:47 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E0E976888F; Fri, 13 Aug 2021 16:32:46 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="NNjtX2vC"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E2F75687FA for ; Fri, 13 Aug 2021 16:32:44 +0200 (CEST) Received: from Monstersaurus.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6F7D64A1; Fri, 13 Aug 2021 16:32:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1628865164; bh=6Dq0VavvQGxq7TIQtKK9uUSZKFiI0nzRENIUnF2IWow=; h=From:To:Cc:Subject:Date:From; b=NNjtX2vCciyFozawm+8vaUVcGlBkr8z7Um1yTJGW8jBIW6NM5t0phvOftsoHn0sfO qFQdJWQ//wS1YlkZHGmukE1t/eiuEpBaPqs1aOklZ+xP0Z+eAuVSykmWGQZyyAOeON y69LTKVgYMQ1wJImq+Tr4dfiQvfw5RY6qhlUM0Nk= From: Kieran Bingham To: libcamera devel Date: Fri, 13 Aug 2021 15:32:41 +0100 Message-Id: <20210813143241.3528968-1-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH] controls: Report required control names X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The V4L2 controls which are optional, recommended, and mandatory report errors if they are not found by CameraSensor::validateSensorDriver(). These errors report hex values for the controls, which can easily lead to confusion and incorrect assumption about which control needs to be investigated. This can be seen occuring already in issues such as [0] and is generally an unfriendly result to report to users in a warning or error message. Adapt the tables of controls to store both the id and name of the controls to report a user friendly message that can ease diagnosis of any CameraSensor validation issues. [0] https://github.com/raspberrypi/linux/issues/4500 Signed-off-by: Kieran Bingham --- I'm aware of course of the checkstyle changes suggested on V4L2_CID, but I thought it was better to keep those macros 'scoped' here, and indented accordingly. --- src/libcamera/camera_sensor.cpp | 45 ++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index 7a3864150c89..1409da1b071c 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -168,19 +168,26 @@ int CameraSensor::validateSensorDriver() { int err = 0; + struct v4l2_cid { + const uint32_t id; + const char *name; + }; + + #define V4L2_CID(x) { x, #x } + /* * 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_SENSOR_ROTATION, + static constexpr v4l2_cid optionalControls[] = { + V4L2_CID(V4L2_CID_CAMERA_SENSOR_ROTATION), }; const ControlIdMap &controls = subdev_->controls().idmap(); - for (uint32_t ctrl : optionalControls) { - if (!controls.count(ctrl)) + for (auto &ctrl : optionalControls) { + if (!controls.count(ctrl.id)) LOG(CameraSensor, Debug) - << "Optional V4L2 control " << utils::hex(ctrl) + << "Optional V4L2 control " << ctrl.name << " not supported"; } @@ -188,14 +195,14 @@ int CameraSensor::validateSensorDriver() * 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, + static constexpr v4l2_cid recommendedControls[] = { + V4L2_CID(V4L2_CID_CAMERA_ORIENTATION), }; - for (uint32_t ctrl : recommendedControls) { - if (!controls.count(ctrl)) { + for (auto &ctrl : recommendedControls) { + if (!controls.count(ctrl.id)) { LOG(CameraSensor, Warning) - << "Recommended V4L2 control " << utils::hex(ctrl) + << "Recommended V4L2 control " << ctrl.name << " not supported"; err = -EINVAL; } @@ -259,18 +266,20 @@ int CameraSensor::validateSensorDriver() * For raw sensors, make sure the sensor driver supports the controls * required by the CameraSensor class. */ - static constexpr uint32_t mandatoryControls[] = { - V4L2_CID_EXPOSURE, - V4L2_CID_HBLANK, - V4L2_CID_PIXEL_RATE, - V4L2_CID_VBLANK, + static constexpr v4l2_cid mandatoryControls[] = { + V4L2_CID(V4L2_CID_EXPOSURE), + V4L2_CID(V4L2_CID_HBLANK), + V4L2_CID(V4L2_CID_PIXEL_RATE), + V4L2_CID(V4L2_CID_VBLANK), }; + #undef V4L2_CID + err = 0; - for (uint32_t ctrl : mandatoryControls) { - if (!controls.count(ctrl)) { + for (auto &ctrl : mandatoryControls) { + if (!controls.count(ctrl.id)) { LOG(CameraSensor, Error) - << "Mandatory V4L2 control " << utils::hex(ctrl) + << "Mandatory V4L2 control " << ctrl.name << " not available"; err = -EINVAL; }