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

[Mirela Rabulea <mirela.rabulea@nxp.com>]

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
>