2023-06-22 07:17:47

by Benjamin Bara

[permalink] [raw]
Subject: [PATCH WIP RFC v2 0/2] media: uapi: Add V4L2_CID_VTOTAL control

Hi!

This series adds new controls for the vertical and horizontal total
size. Camera sensors usually have registers regarding the total size and
not the blanking size. Also user space prefers to calculate with total
frame sizes. Therefore, this would simplify implementations on both
sides and could be seen as a replacement or upgrade for V4L2_CID_VBLANK.
Additionally, its value is independent from format changes and therefore
simplifies calculations in user space.

For v2, I added a little bit more documentation and my personal
expectations to 1/2. As I am fairly new to the camera world, they might
be a little bit naive so please correct me if this is utopical. I added
the RFC tag for that reason. However, my intention is to define a
documented behaviour regarding the values and limits which could
basically depend on HTOTAL. Currently, HBLANK is always set to the
minimum in libcamera (I guess to simplify calculations). I think with
HTOTAL, we could always use WIDTH_MAX(all modes) + HBLANK_MIN(WIDTH_MAX)
as default to enable a constant frame size. If the current HTOTAL value
differs from the default, we could infer that the user wants to either
have a higher frame rate, or a higher exposure and could adapt to that.
E.g. use the minimums to get the highest frame rate possible if the
current values are outside of the possible range, or not limiting the
exposure control by the current vertical blanking as it is done now. I
guess this would help the driver to "understand" the needs of the user
space, and therefore allow it to react in a defined and expected manner.

2/2 is a WIP (same as in v1) and currently implements VTOTAL for the
imx290, basically extending the current V4L2_CID_VBLANK implementation.

Thanks Dave for the feedback, insights and the examples (ov5647,
ov9282). I might need some time to skim through the data sheets to learn
why they do stuff like it is done now.

---
Changes in v2:
- 1/2: add HTOTAL
- 1/2: add documentation about expectations
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Benjamin Bara (2):
media: uapi: Add V4L2_CID_{V,H}TOTAL controls
media: i2c: imx290: Add support for V4L2_CID_VTOTAL

Documentation/driver-api/media/camera-sensor.rst | 11 ++++-
.../media/v4l/ext-ctrls-image-source.rst | 16 ++++++++
drivers/media/i2c/imx290.c | 47 ++++++++++++++++------
drivers/media/v4l2-core/v4l2-ctrls-defs.c | 2 +
include/uapi/linux/v4l2-controls.h | 2 +
5 files changed, 64 insertions(+), 14 deletions(-)
---
base-commit: 9561de3a55bed6bdd44a12820ba81ec416e705a7
change-id: 20230609-v4l2-vtotal-eb15d37cafea

Best regards,
--
Benjamin Bara <[email protected]>



2023-06-22 07:34:41

by Benjamin Bara

[permalink] [raw]
Subject: [PATCH WIP RFC v2 2/2] media: i2c: imx290: Add support for V4L2_CID_VTOTAL

From: Benjamin Bara <[email protected]>

The new V4L2_CID_VTOTAL control represents the VMAX register.
Implementing it simplifies calculations in user space, as it is
independent of the current mode (format height), meaning its value does
not change with format changes.

Signed-off-by: Benjamin Bara <[email protected]>
---
drivers/media/i2c/imx290.c | 47 ++++++++++++++++++++++++++++++++++------------
1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 5ea25b7acc55..42938400efb0 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -255,6 +255,7 @@ struct imx290 {
struct v4l2_ctrl *link_freq;
struct v4l2_ctrl *hblank;
struct v4l2_ctrl *vblank;
+ struct v4l2_ctrl *vtotal;
struct v4l2_ctrl *exposure;
struct {
struct v4l2_ctrl *hflip;
@@ -782,8 +783,7 @@ static void imx290_exposure_update(struct imx290 *imx290,
{
unsigned int exposure_max;

- exposure_max = imx290->vblank->val + mode->height -
- IMX290_EXPOSURE_OFFSET;
+ exposure_max = imx290->vtotal->val - IMX290_EXPOSURE_OFFSET;
__v4l2_ctrl_modify_range(imx290->exposure, 1, exposure_max, 1,
exposure_max);
}
@@ -794,7 +794,7 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
struct imx290, ctrls);
const struct v4l2_mbus_framefmt *format;
struct v4l2_subdev_state *state;
- int ret = 0, vmax;
+ int ret = 0;

/*
* Return immediately for controls that don't need to be applied to the
@@ -803,10 +803,22 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
if (ctrl->flags & V4L2_CTRL_FLAG_READ_ONLY)
return 0;

- if (ctrl->id == V4L2_CID_VBLANK) {
- /* Changing vblank changes the allowed range for exposure. */
+ /* Changing vtotal changes the allowed range for exposure. */
+ if (ctrl->id == V4L2_CID_VTOTAL)
imx290_exposure_update(imx290, imx290->current_mode);
- }
+
+ /*
+ * vblank and vtotal depend on each other, therefore also update the
+ * other one.
+ */
+ if (ctrl->id == V4L2_CID_VBLANK &&
+ imx290->vtotal->val != ctrl->val + imx290->current_mode->height)
+ __v4l2_ctrl_s_ctrl(imx290->vtotal,
+ ctrl->val + imx290->current_mode->height);
+ if (ctrl->id == V4L2_CID_VTOTAL &&
+ imx290->vblank->val != ctrl->val - imx290->current_mode->height)
+ __v4l2_ctrl_s_ctrl(imx290->vblank,
+ ctrl->val - imx290->current_mode->height);

/* V4L2 controls values will be applied only when power is already up */
if (!pm_runtime_get_if_in_use(imx290->dev))
@@ -821,9 +833,14 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
break;

case V4L2_CID_VBLANK:
- ret = imx290_write(imx290, IMX290_VMAX,
- ctrl->val + imx290->current_mode->height,
- NULL);
+ /* vblank is updated by vtotal. */
+ break;
+
+ case V4L2_CID_VTOTAL:
+ ret = imx290_write(imx290, IMX290_VMAX, ctrl->val, NULL);
+ if (ret)
+ goto err;
+
/*
* Due to the way that exposure is programmed in this sensor in
* relation to VMAX, we have to reprogramme it whenever VMAX is
@@ -834,9 +851,8 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
ctrl = imx290->exposure;
fallthrough;
case V4L2_CID_EXPOSURE:
- vmax = imx290->vblank->val + imx290->current_mode->height;
ret = imx290_write(imx290, IMX290_SHS1,
- vmax - ctrl->val - 1, NULL);
+ imx290->vtotal->val - ctrl->val - 1, NULL);
break;

case V4L2_CID_TEST_PATTERN:
@@ -880,6 +896,7 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
break;
}

+err:
pm_runtime_mark_last_busy(imx290->dev);
pm_runtime_put_autosuspend(imx290->dev);

@@ -911,11 +928,14 @@ static void imx290_ctrl_update(struct imx290 *imx290,
unsigned int vblank_max = IMX290_VMAX_MAX - mode->height;

__v4l2_ctrl_s_ctrl(imx290->link_freq, mode->link_freq_index);
+ __v4l2_ctrl_s_ctrl(imx290->vblank, imx290->vtotal->val - mode->height);

__v4l2_ctrl_modify_range(imx290->hblank, hblank_min, hblank_max, 1,
hblank_min);
__v4l2_ctrl_modify_range(imx290->vblank, vblank_min, vblank_max, 1,
vblank_min);
+ __v4l2_ctrl_modify_range(imx290->vtotal, mode->vmax_min,
+ IMX290_VMAX_MAX, 1, mode->vmax_min);
}

static int imx290_ctrl_init(struct imx290 *imx290)
@@ -947,7 +967,7 @@ static int imx290_ctrl_init(struct imx290 *imx290)

/*
* Correct range will be determined through imx290_ctrl_update setting
- * V4L2_CID_VBLANK.
+ * V4L2_CID_VTOTAL.
*/
imx290->exposure = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
V4L2_CID_EXPOSURE, 1, 65535, 1,
@@ -983,6 +1003,9 @@ static int imx290_ctrl_init(struct imx290 *imx290)

imx290->vblank = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
V4L2_CID_VBLANK, 1, 1, 1, 1);
+ imx290->vtotal = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
+ V4L2_CID_VTOTAL, 1, IMX290_VMAX_MAX,
+ 1, 1);

imx290->hflip = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
V4L2_CID_HFLIP, 0, 1, 1, 0);

--
2.34.1


2023-06-22 07:36:18

by Benjamin Bara

[permalink] [raw]
Subject: [PATCH WIP RFC v2 1/2] media: uapi: Add V4L2_CID_{V,H}TOTAL controls

From: Benjamin Bara <[email protected]>

Currently, V4L2_CID_{V,H}BLANK can be used to control the frame duration
of a stream. However, camera sensors usually have a register for the
total size (image data + blanking), e.g. {V,H}MAX on the imx290. As the
user space also typically wants to set a frame size, both sides
currently have to calculate with values they actually don't care about.

The dependency between format height and vertical blanking also results
to a change of the vertical blanking value and limits whenever the
format of the frame is changed and therefore makes it harder for user
space to do calculations, e.g. the frame duration.
V4L2_CID_{V,H}TOTAL do not depend on the format and therefore simplify
calculations. Additionally, they represent the hardware "better" and
therefore also simplify calculations on the driver side.

Add additional documentation about V4L2_CID_HTOTAL to have a control
which enables the user space to express its intends to the driver. The
driver can then update its controls and limits accordingly, and can do
possible re-calculations during mode switches in a defined behaviour.
This increases compatibility between different camera sensors.

Signed-off-by: Benjamin Bara <[email protected]>
---
v2:
- add HTOTAL
- add documentation about expectations
---
Documentation/driver-api/media/camera-sensor.rst | 11 +++++++++--
.../userspace-api/media/v4l/ext-ctrls-image-source.rst | 16 ++++++++++++++++
drivers/media/v4l2-core/v4l2-ctrls-defs.c | 2 ++
include/uapi/linux/v4l2-controls.h | 2 ++
4 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
index c7d4891bd24e..3ddeb0533728 100644
--- a/Documentation/driver-api/media/camera-sensor.rst
+++ b/Documentation/driver-api/media/camera-sensor.rst
@@ -85,12 +85,14 @@ a result of the configuration of a number of camera sensor implementation
specific parameters. Luckily, these parameters tend to be the same for more or
less all modern raw camera sensors.

-The frame interval is calculated using the following equation::
+The frame interval is calculated using one of the following equations::

frame interval = (analogue crop width + horizontal blanking) *
(analogue crop height + vertical blanking) / pixel rate

-The formula is bus independent and is applicable for raw timing parameters on
+ frame interval = horizontal total size * vertical total size / pixel rate
+
+The formulas are bus independent and are applicable for raw timing parameters on
large variety of devices beyond camera sensors. Devices that have no analogue
crop, use the full source image size, i.e. pixel array size.

@@ -100,6 +102,11 @@ is pixels and the unit of the ``V4L2_CID_VBLANK`` is lines. The pixel rate in
the sensor's **pixel array** is specified by ``V4L2_CID_PIXEL_RATE`` in the same
sub-device. The unit of that control is pixels per second.

+Additionally, the horizontal and vertical total sizes are specified by
+``V4L2_CID_HTOTAL`` and ``V4L2_CID_VTOTAL``, respectively. The unit of the
+``V4L2_CID_HTOTAL`` control is pixels and the unit of the ``V4L2_CID_VTOTAL`` is
+lines, analogous to the blanking.
+
Register list based drivers need to implement read-only sub-device nodes for the
purpose. Devices that are not register list based need these to configure the
device's internal processing pipeline.
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 71f23f131f97..5451fa0223cd 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
@@ -59,6 +59,22 @@ Image Source Control IDs
non-sensitive.
This control is required for automatic calibration of sensors/cameras.

+``V4L2_CID_VTOTAL (integer)``
+ Number of total lines per frame, including data and idle lines (blanking).
+ The unit of the vertical total size is a line. Every line has length of the
+ image width plus horizontal blanking at the pixel rate defined by
+ ``V4L2_CID_PIXEL_RATE`` control in the same sub-device.
+
+``V4L2_CID_HTOTAL (integer)``
+ Number of total pixels per line, including data and idle pixels (blanking).
+ The unit of the horizontal total size is pixels. The default value of this
+ control is set to a value which allows a constant total size for every
+ supported mode of the sensor. The control can be used to indicate the driver
+ if a high frame rate (value < default) or a high exposure (value > default)
+ should be achieved. With value = default, a constant frame size across the
+ different modes is targeted. Note that the actual frame rate depends on the
+ ``V4L2_CID_PIXEL_RATE`` control, which might vary between mode switches.
+
.. c:type:: v4l2_area

.. flat-table:: struct v4l2_area
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
index 564fedee2c88..34e17e1faa7a 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
@@ -1112,6 +1112,8 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_TEST_PATTERN_BLUE: return "Blue Pixel Value";
case V4L2_CID_TEST_PATTERN_GREENB: return "Green (Blue) Pixel Value";
case V4L2_CID_NOTIFY_GAINS: return "Notify Gains";
+ case V4L2_CID_VTOTAL: return "Vertical Total Size";
+ case V4L2_CID_HTOTAL: return "Horizontal Total Size";

/* Image processing controls */
/* Keep the order of the 'case's the same as in v4l2-controls.h! */
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 5e80daa4ffe0..a4bbd91b8aef 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1117,6 +1117,8 @@ enum v4l2_jpeg_chroma_subsampling {
#define V4L2_CID_TEST_PATTERN_GREENB (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 7)
#define V4L2_CID_UNIT_CELL_SIZE (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 8)
#define V4L2_CID_NOTIFY_GAINS (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 9)
+#define V4L2_CID_VTOTAL (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 10)
+#define V4L2_CID_HTOTAL (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 11)


/* Image processing controls */

--
2.34.1


2023-06-22 15:32:44

by Nicolas Dufresne

[permalink] [raw]
Subject: Re: [PATCH WIP RFC v2 0/2] media: uapi: Add V4L2_CID_VTOTAL control

Hi,

Le jeudi 22 juin 2023 à 09:12 +0200, Benjamin Bara a écrit :
> Hi!
>
> This series adds new controls for the vertical and horizontal total
> size. Camera sensors usually have registers regarding the total size and
> not the blanking size. Also user space prefers to calculate with total
> frame sizes. Therefore, this would simplify implementations on both
> sides and could be seen as a replacement or upgrade for V4L2_CID_VBLANK.
> Additionally, its value is independent from format changes and therefore
> simplifies calculations in user space.

The naming is getting more and more generic. We need to find a way to give these
controls a name that associates them to their usage field (sensors). If we keep
growing the control list this way, in few years it will be terribly hard to
understand what is used for what.

What about, V4L2_CID_SENSOR_VTOTAL/HTOTAL ? I'm expecting the last bit of the
name to come from very well known specification, otherwise, its not a great name
either in my opinion.

>
> For v2, I added a little bit more documentation and my personal
> expectations to 1/2. As I am fairly new to the camera world, they might
> be a little bit naive so please correct me if this is utopical. I added
> the RFC tag for that reason. However, my intention is to define a
> documented behaviour regarding the values and limits which could
> basically depend on HTOTAL. Currently, HBLANK is always set to the
> minimum in libcamera (I guess to simplify calculations). I think with
> HTOTAL, we could always use WIDTH_MAX(all modes) + HBLANK_MIN(WIDTH_MAX)
> as default to enable a constant frame size. If the current HTOTAL value
> differs from the default, we could infer that the user wants to either
> have a higher frame rate, or a higher exposure and could adapt to that.
> E.g. use the minimums to get the highest frame rate possible if the
> current values are outside of the possible range, or not limiting the
> exposure control by the current vertical blanking as it is done now. I
> guess this would help the driver to "understand" the needs of the user
> space, and therefore allow it to react in a defined and expected manner.
>
> 2/2 is a WIP (same as in v1) and currently implements VTOTAL for the
> imx290, basically extending the current V4L2_CID_VBLANK implementation.
>
> Thanks Dave for the feedback, insights and the examples (ov5647,
> ov9282). I might need some time to skim through the data sheets to learn
> why they do stuff like it is done now.
>
> ---
> Changes in v2:
> - 1/2: add HTOTAL
> - 1/2: add documentation about expectations
> - Link to v1: https://lore.kernel.org/r/[email protected]
>

Do you have an link to an example user of this new API ?

Nicolas

> ---
> Benjamin Bara (2):
> media: uapi: Add V4L2_CID_{V,H}TOTAL controls
> media: i2c: imx290: Add support for V4L2_CID_VTOTAL
>
> Documentation/driver-api/media/camera-sensor.rst | 11 ++++-
> .../media/v4l/ext-ctrls-image-source.rst | 16 ++++++++
> drivers/media/i2c/imx290.c | 47 ++++++++++++++++------
> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 2 +
> include/uapi/linux/v4l2-controls.h | 2 +
> 5 files changed, 64 insertions(+), 14 deletions(-)
> ---
> base-commit: 9561de3a55bed6bdd44a12820ba81ec416e705a7
> change-id: 20230609-v4l2-vtotal-eb15d37cafea
>
> Best regards,


2023-09-25 14:51:56

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH WIP RFC v2 0/2] media: uapi: Add V4L2_CID_VTOTAL control

Hi Nicolas, Benjamin,

On Thu, Jun 22, 2023 at 11:14:41AM -0400, Nicolas Dufresne wrote:
> Hi,
>
> Le jeudi 22 juin 2023 ? 09:12 +0200, Benjamin Bara a ?crit?:
> > Hi!
> >
> > This series adds new controls for the vertical and horizontal total
> > size. Camera sensors usually have registers regarding the total size and
> > not the blanking size. Also user space prefers to calculate with total
> > frame sizes. Therefore, this would simplify implementations on both
> > sides and could be seen as a replacement or upgrade for V4L2_CID_VBLANK.
> > Additionally, its value is independent from format changes and therefore
> > simplifies calculations in user space.
>
> The naming is getting more and more generic. We need to find a way to give these
> controls a name that associates them to their usage field (sensors). If we keep
> growing the control list this way, in few years it will be terribly hard to
> understand what is used for what.
>
> What about, V4L2_CID_SENSOR_VTOTAL/HTOTAL ? I'm expecting the last bit of the
> name to come from very well known specification, otherwise, its not a great name
> either in my opinion.

These controls can, in theory at least, be used for any image data bus:
they're not related to sensors but to any source of image data on a bus.
Compare this to the LINK_FREQ control, for instance: you basically need
these to calculate the frame rate (or calculate these based on the desired
frame rate).

These configurations are often called line length (in) pixels and frame
length (in) lines, the controls could be thus V4L2_CID_LLP and
V4L2_CID_FLL, to keep them a little bit shorter.

--
Regards,

Sakari Ailus

2023-09-25 16:43:17

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH WIP RFC v2 1/2] media: uapi: Add V4L2_CID_{V,H}TOTAL controls

Hi Benjamin,

Thank you for the patchset and my apologies for reviewing this so late.

On Thu, Jun 22, 2023 at 09:12:23AM +0200, Benjamin Bara wrote:
> From: Benjamin Bara <[email protected]>
>
> Currently, V4L2_CID_{V,H}BLANK can be used to control the frame duration
> of a stream. However, camera sensors usually have a register for the
> total size (image data + blanking), e.g. {V,H}MAX on the imx290. As the
> user space also typically wants to set a frame size, both sides
> currently have to calculate with values they actually don't care about.
>
> The dependency between format height and vertical blanking also results
> to a change of the vertical blanking value and limits whenever the
> format of the frame is changed and therefore makes it harder for user
> space to do calculations, e.g. the frame duration.
> V4L2_CID_{V,H}TOTAL do not depend on the format and therefore simplify
> calculations. Additionally, they represent the hardware "better" and
> therefore also simplify calculations on the driver side.
>
> Add additional documentation about V4L2_CID_HTOTAL to have a control
> which enables the user space to express its intends to the driver. The
> driver can then update its controls and limits accordingly, and can do
> possible re-calculations during mode switches in a defined behaviour.
> This increases compatibility between different camera sensors.
>
> Signed-off-by: Benjamin Bara <[email protected]>
> ---
> v2:
> - add HTOTAL
> - add documentation about expectations
> ---
> Documentation/driver-api/media/camera-sensor.rst | 11 +++++++++--
> .../userspace-api/media/v4l/ext-ctrls-image-source.rst | 16 ++++++++++++++++
> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 2 ++
> include/uapi/linux/v4l2-controls.h | 2 ++
> 4 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
> index c7d4891bd24e..3ddeb0533728 100644
> --- a/Documentation/driver-api/media/camera-sensor.rst
> +++ b/Documentation/driver-api/media/camera-sensor.rst
> @@ -85,12 +85,14 @@ a result of the configuration of a number of camera sensor implementation
> specific parameters. Luckily, these parameters tend to be the same for more or
> less all modern raw camera sensors.
>
> -The frame interval is calculated using the following equation::
> +The frame interval is calculated using one of the following equations::
>
> frame interval = (analogue crop width + horizontal blanking) *
> (analogue crop height + vertical blanking) / pixel rate
>
> -The formula is bus independent and is applicable for raw timing parameters on
> + frame interval = horizontal total size * vertical total size / pixel rate
> +
> +The formulas are bus independent and are applicable for raw timing parameters on

I think we should continue to have a single formula, but that the values
could be obtained different ways, depending on the driver.

For the user space it should be easier to use the controls you're
proposing but existing drivers will still continue to use the existing
controls.

In particular, you should add something like:

``V4L2_CID_VTOTAL`` value equals to analogue crop height + value of the
``V4L2_CID_VBLANK``. Similarly, ``V4L2_CID_HTOTAL`` value equals to
analogue crop width + value of the ``V4L2_CID_HBLANK``.

New drivers shall and existing drivers should implement ``V4L2_CID_VTOTAL``
and ``V4L2_CID_HTOTAL`` whereas existing drivers shall continue to
implement ``V4L2_CID_VBLANK`` and ``V4L2_CID_HBLANK``.

Implementing both in drivers can be a little bit painful. It'd be good to
add driver support for this and use this as an example (e.g. CCS). I'll see
if I could provide a patch, this will take some time still though.

I wonder what Laurent thinks.

> large variety of devices beyond camera sensors. Devices that have no analogue
> crop, use the full source image size, i.e. pixel array size.
>
> @@ -100,6 +102,11 @@ is pixels and the unit of the ``V4L2_CID_VBLANK`` is lines. The pixel rate in
> the sensor's **pixel array** is specified by ``V4L2_CID_PIXEL_RATE`` in the same
> sub-device. The unit of that control is pixels per second.
>
> +Additionally, the horizontal and vertical total sizes are specified by
> +``V4L2_CID_HTOTAL`` and ``V4L2_CID_VTOTAL``, respectively. The unit of the
> +``V4L2_CID_HTOTAL`` control is pixels and the unit of the ``V4L2_CID_VTOTAL`` is
> +lines, analogous to the blanking.

Add "controls" before the period.

> +
> Register list based drivers need to implement read-only sub-device nodes for the
> purpose. Devices that are not register list based need these to configure the
> device's internal processing pipeline.
> 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 71f23f131f97..5451fa0223cd 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
> @@ -59,6 +59,22 @@ Image Source Control IDs
> non-sensitive.
> This control is required for automatic calibration of sensors/cameras.
>
> +``V4L2_CID_VTOTAL (integer)``
> + Number of total lines per frame, including data and idle lines (blanking).
> + The unit of the vertical total size is a line. Every line has length of the
> + image width plus horizontal blanking at the pixel rate defined by
> + ``V4L2_CID_PIXEL_RATE`` control in the same sub-device.
> +
> +``V4L2_CID_HTOTAL (integer)``
> + Number of total pixels per line, including data and idle pixels (blanking).
> + The unit of the horizontal total size is pixels. The default value of this
> + control is set to a value which allows a constant total size for every
> + supported mode of the sensor. The control can be used to indicate the driver
> + if a high frame rate (value < default) or a high exposure (value > default)
> + should be achieved. With value = default, a constant frame size across the
> + different modes is targeted. Note that the actual frame rate depends on the
> + ``V4L2_CID_PIXEL_RATE`` control, which might vary between mode switches.
> +
> .. c:type:: v4l2_area
>
> .. flat-table:: struct v4l2_area
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> index 564fedee2c88..34e17e1faa7a 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -1112,6 +1112,8 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_TEST_PATTERN_BLUE: return "Blue Pixel Value";
> case V4L2_CID_TEST_PATTERN_GREENB: return "Green (Blue) Pixel Value";
> case V4L2_CID_NOTIFY_GAINS: return "Notify Gains";
> + case V4L2_CID_VTOTAL: return "Vertical Total Size";
> + case V4L2_CID_HTOTAL: return "Horizontal Total Size";
>
> /* Image processing controls */
> /* Keep the order of the 'case's the same as in v4l2-controls.h! */
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 5e80daa4ffe0..a4bbd91b8aef 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1117,6 +1117,8 @@ enum v4l2_jpeg_chroma_subsampling {
> #define V4L2_CID_TEST_PATTERN_GREENB (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 7)
> #define V4L2_CID_UNIT_CELL_SIZE (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 8)
> #define V4L2_CID_NOTIFY_GAINS (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 9)
> +#define V4L2_CID_VTOTAL (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 10)
> +#define V4L2_CID_HTOTAL (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 11)
>
>
> /* Image processing controls */
>

--
Kind regards,

Sakari Ailus