Re: [PATCH v3 3/5] media: uapi: Add ST VGXY61 header file

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Benjamin,

On Thu, May 12, 2022 at 09:40:35AM +0200, Benjamin Mugnier wrote:
> Define an HDR control in it, and adds its documentation in st-vgxy61.rst
> file.
> 
> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
> ---
>  .../userspace-api/media/drivers/st-vgxy61.rst | 23 +++++++++++++++++++
>  include/uapi/linux/st-vgxy61.h                | 15 ++++++++++++
>  2 files changed, 38 insertions(+)
>  create mode 100644 Documentation/userspace-api/media/drivers/st-vgxy61.rst
>  create mode 100644 include/uapi/linux/st-vgxy61.h
> 
> diff --git a/Documentation/userspace-api/media/drivers/st-vgxy61.rst b/Documentation/userspace-api/media/drivers/st-vgxy61.rst
> new file mode 100644
> index 000000000000..7a11adbb558f
> --- /dev/null
> +++ b/Documentation/userspace-api/media/drivers/st-vgxy61.rst
> @@ -0,0 +1,23 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +ST VGXY61 camera sensor driver
> +==============================
> +
> +The ST VGXY61 driver implements the following driver-specific controls:
> +
> +``V4L2_CID_STVGXY61_HDR``
> +-------------------------------
> +    Change the sensor HDR mode. A HDR picture is obtained by merging two captures of the same scene
> +    using two different exposure periods.
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       1 4
> +
> +    * - HDR linearize
> +      - The merger outputs a long exposure capture as long as it is not saturated.
> +    * - HDR substraction
> +      - This involves subtracting the short exposure frame from the long exposure frame.
> +    * - "no HDR"
> +      - This mode is used for standard dynamic range (SDR) exposures.

I wonder how many different kinds of HDR implementations there are that
could be meaningfully controlled using a single control. We have controls
such as scene mode that are much more generic than this.

Could this be standardised, even if the menu items wouldn't be? Say, call
it e.g. V4L2_CID_HDR_MODE? Raw sensors have different configuration with
more parameters though.

I wonder what others think.

> diff --git a/include/uapi/linux/st-vgxy61.h b/include/uapi/linux/st-vgxy61.h
> new file mode 100644
> index 000000000000..fbabe2cb64ac
> --- /dev/null
> +++ b/include/uapi/linux/st-vgxy61.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2022 STMicroelectronics SA
> + *
> + */
> +
> +#ifndef __UAPI_STVGXY61_H_
> +#define __UAPI_STVGXY61_H_
> +
> +#include <linux/v4l2-controls.h>
> +
> +/* Control HDR mode */
> +#define V4L2_CID_STVGXY61_HDR	(V4L2_CID_USER_STVGXY61_BASE + 0)
> +
> +#endif /* __UAPI_STVGXY61_H_ */

-- 
Kind regards,

Sakari Ailus