Re: [RFC v5 10/15] media: v4l: uapi: Add a control for colour pattern flipping effect

Mirela Rabulea
2025-02-09T17:23:16Z

Hi Sakari,

On 03.02.2025 10:58, Sakari Ailus wrote:
> Add a bitmask control (V4L2_CID_COLOUR_PATTERN_FLIP) to tell whether
> flipping results in a change in the sensor's colour pattern, separately
> horizontally and vertically. The information is essential for raw formats
> when using generic raw mbus codes.

Is this control really necessary? Are there cases when V4L2_CID_HFLIP
or/and V4L2_CID_VFLIP is enabled and the CFA pattern does not change?

Could a raw sensor driver be capable of reporting the
V4L2_CID_COLOUR_PATTERN based on current values of the HFLIP/VFLIP? I'm
not sure I understood all the aspects of patch #7 (V4L2_CID_COLOUR_PATTERN).

Regards,

Mirela

>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> .../media/v4l/ext-ctrls-image-source.rst | 8 ++++++++
> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 2 ++
> include/uapi/linux/v4l2-controls.h | 15 +++++++++++++++
> 3 files changed, 25 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
> index fca729512b6f..ecfa38c118e3 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
> @@ -102,3 +102,11 @@ Image Source Control IDs
> This control may only be used on a V4L2 sub-device.
>
> This is a read-only control.
> +
> +``V4L2_CID_COLOUR_PATTERN_FLIP (bitmask)``
> + Whether the horizontal or vertical flipping controls (V4L2_CID_HFLIP and
> + V4L2_CID_VFLIP) have an effect on the pixel order of the output colour
> + pattern. Macros ``V4L2_COLOUR_PATTERN_FLIP_HORIZONTAL`` and
> + ``V4L2_COLOUR_PATTERN_FLIP_VERTICAL`` define bitmasks for both bits. If
> + either horizontal or vertical bit is set, the readout pattern order is that
> + of the reversed readout.
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> index 5b6a4a94f18f..3f0704a982b8 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -1156,6 +1156,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_TEST_PATTERN_GREENB: return "Green (Blue) Pixel Value";
> case V4L2_CID_NOTIFY_GAINS: return "Notify Gains";
> case V4L2_CID_COLOUR_PATTERN: return "Colour Pattern";
> + case V4L2_CID_COLOUR_PATTERN_FLIP: return "Colour Pattern Flip";
>
> /* Image processing controls */
> /* Keep the order of the 'case's the same as in v4l2-controls.h! */
> @@ -1474,6 +1475,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> *max = 0xffff;
> break;
> case V4L2_CID_FLASH_FAULT:
> + case V4L2_CID_COLOUR_PATTERN_FLIP:
> case V4L2_CID_JPEG_ACTIVE_MARKER:
> case V4L2_CID_3A_LOCK:
> case V4L2_CID_AUTO_FOCUS_STATUS:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 8e761c38b995..fd6465e9a743 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1215,11 +1215,26 @@ enum v4l2_jpeg_chroma_subsampling {
> #define V4L2_CID_NOTIFY_GAINS (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 9)
>
> #define V4L2_CID_COLOUR_PATTERN (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 10)
> +/*
> + * CFA pattern start shall be aligned with the number of patterns for colour
> + * components. Patterns shall be calculable based on flipping when it affects
> + * the pattern the following way:
> + *
> + * flipped_pattern = native_pattern ^
> + * ((hflip ? V4L2_COLOUR_PATTERN_FLIP_HORIZONTAL : 0) |
> + * (vflip ? V4L2_COLOUR_PATTERN_FLIP_VERTICAL : 0))
> + *
> + * where hflip and vflip are the values of V4L2_CID_HFLIP and V4L2_CID_VFLIP
> + * controls, respectively.
> + */
> #define V4L2_COLOUR_PATTERN_GRBG 0
> #define V4L2_COLOUR_PATTERN_RGGB 1
> #define V4L2_COLOUR_PATTERN_BGGR 2
> #define V4L2_COLOUR_PATTERN_GBRG 3
>
> +#define V4L2_CID_COLOUR_PATTERN_FLIP (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 11)
> +#define V4L2_COLOUR_PATTERN_FLIP_HORIZONTAL (1U << 0)
> +#define V4L2_COLOUR_PATTERN_FLIP_VERTICAL (1U << 1)
>
> /* Image processing controls */
>
> --
> 2.39.5
>