On 11/10/2024 09:55, Sakari Ailus wrote:
> Add the V4L2_CID_CONFIG_MODEL control for the configuration model.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> .../userspace-api/media/v4l/ext-ctrls-image-process.rst | 4 ++++
> .../userspace-api/media/v4l/subdev-config-model.rst | 2 ++
> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 5 +++++
> include/uapi/linux/v4l2-controls.h | 3 +++
> 4 files changed, 14 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-image-process.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-image-process.rst
> index 27803dca8d3e..928e8e3eed7f 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-image-process.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-image-process.rst
> @@ -55,3 +55,7 @@ Image Process Control IDs
> control value divided by e.g. 0x100, meaning that to get no
> digital gain the control value needs to be 0x100. The no-gain
> configuration is also typically the default.
> +
> +``V4L2_CID_CONFIG_MODEL (bitmask)``
> + Which configuration models the sub-device supports. Please see
> + :ref:`media_subdev_config_model`.
First of all the naming is confusing: since this is specific to sub-devices, it
should at least have 'SUBDEV' in the name. I first thought this reported the
model name or something like that, I'm not sure "configuration model" is a very
good name.
Secondly, is this supposed to be valid for all subdevices? Only for sensors?
Would an HDMI-to-CSI bridge qualify?
Thirdly, only V4L2_CID_CONFIG_MODEL_COMMON_RAW is defined right now. What other
models do you have in mind? What models can co-exist (since this is a bitmask)?
Finally, why choose a control for this? Should this perhaps be better done as
a field in media_entity_desc/media_v2_entity?
Regards,
Hans
> diff --git a/Documentation/userspace-api/media/v4l/subdev-config-model.rst b/Documentation/userspace-api/media/v4l/subdev-config-model.rst
> index 8ec801998f5f..d4ae921b69c8 100644
> --- a/Documentation/userspace-api/media/v4l/subdev-config-model.rst
> +++ b/Documentation/userspace-api/media/v4l/subdev-config-model.rst
> @@ -1,5 +1,7 @@
> .. SPDX-License-Identifier: GPL-2.0 OR GFDL-1.1-no-invariants-or-later
>
> +.. _media_subdev_config_model:
> +
> Sub-device configuration models
> ===============================
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> index 6b9188a4a220..378657a52cd5 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -1167,6 +1167,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_TEST_PATTERN: return "Test Pattern";
> case V4L2_CID_DEINTERLACING_MODE: return "Deinterlacing Mode";
> case V4L2_CID_DIGITAL_GAIN: return "Digital Gain";
> + case V4L2_CID_CONFIG_MODEL: return "Sub-device configuration model";
Start each word capitalized, just like all the other strings.
>
> /* DV controls */
> /* Keep the order of the 'case's the same as in v4l2-controls.h! */
> @@ -1489,6 +1490,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> case V4L2_CID_DV_RX_POWER_PRESENT:
> *type = V4L2_CTRL_TYPE_BITMASK;
> break;
> + case V4L2_CID_CONFIG_MODEL:
> + *flags |= V4L2_CTRL_FLAG_READ_ONLY;
> + *type = V4L2_CTRL_TYPE_BITMASK;
> + break;
> case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
> case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:
> *type = V4L2_CTRL_TYPE_INTEGER;
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 974fd254e573..0152240229ab 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1225,6 +1225,9 @@ enum v4l2_jpeg_chroma_subsampling {
> #define V4L2_CID_TEST_PATTERN (V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
> #define V4L2_CID_DEINTERLACING_MODE (V4L2_CID_IMAGE_PROC_CLASS_BASE + 4)
> #define V4L2_CID_DIGITAL_GAIN (V4L2_CID_IMAGE_PROC_CLASS_BASE + 5)
> +#define V4L2_CID_CONFIG_MODEL (V4L2_CID_IMAGE_PROC_CLASS_BASE + 6)
> +
> +#define V4L2_CID_CONFIG_MODEL_COMMON_RAW (1ULL << 0)
>
> /* DV-class control IDs defined by V4L2 */
> #define V4L2_CID_DV_CLASS_BASE (V4L2_CTRL_CLASS_DV | 0x900)
