* [PATCH v3 01/22] media: imx219: Rename "PIXEL_ARRAY" as "VISIBLE"
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
@ 2026-03-25 10:57 ` Sakari Ailus
2026-03-25 10:57 ` [PATCH v3 02/22] media: imx219: Fix maximum frame length in lines Sakari Ailus
` (20 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:57 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
The imx219 driver uses macros for denoting the size of the pixel array.
The values reflect the area of manufacturer-designated visible pixels,
reflect this in the naming by calling it "VISIBLE" instead of
"PIXEL_ARRAY".
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/i2c/imx219.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 7da02ce5da15..cbd151d4af5f 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -142,10 +142,10 @@
/* IMX219 native and active pixel array size. */
#define IMX219_NATIVE_WIDTH 3296U
#define IMX219_NATIVE_HEIGHT 2480U
-#define IMX219_PIXEL_ARRAY_LEFT 8U
-#define IMX219_PIXEL_ARRAY_TOP 8U
-#define IMX219_PIXEL_ARRAY_WIDTH 3280U
-#define IMX219_PIXEL_ARRAY_HEIGHT 2464U
+#define IMX219_VISIBLE_LEFT 8U
+#define IMX219_VISIBLE_TOP 8U
+#define IMX219_VISIBLE_WIDTH 3280U
+#define IMX219_VISIBLE_HEIGHT 2464U
/* Mode : resolution and related config&values */
struct imx219_mode {
@@ -675,13 +675,13 @@ static int imx219_set_framefmt(struct imx219 *imx219,
bpp = imx219_get_format_bpp(format);
cci_write(imx219->regmap, IMX219_REG_X_ADD_STA_A,
- crop->left - IMX219_PIXEL_ARRAY_LEFT, &ret);
+ crop->left - IMX219_VISIBLE_LEFT, &ret);
cci_write(imx219->regmap, IMX219_REG_X_ADD_END_A,
- crop->left - IMX219_PIXEL_ARRAY_LEFT + crop->width - 1, &ret);
+ crop->left - IMX219_VISIBLE_LEFT + crop->width - 1, &ret);
cci_write(imx219->regmap, IMX219_REG_Y_ADD_STA_A,
- crop->top - IMX219_PIXEL_ARRAY_TOP, &ret);
+ crop->top - IMX219_VISIBLE_TOP, &ret);
cci_write(imx219->regmap, IMX219_REG_Y_ADD_END_A,
- crop->top - IMX219_PIXEL_ARRAY_TOP + crop->height - 1, &ret);
+ crop->top - IMX219_VISIBLE_TOP + crop->height - 1, &ret);
imx219_get_binning(state, &bin_h, &bin_v);
cci_write(imx219->regmap, IMX219_REG_BINNING_MODE_H, bin_h, &ret);
@@ -867,8 +867,8 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
* Use binning to maximize the crop rectangle size, and centre it in the
* sensor.
*/
- bin_h = min(IMX219_PIXEL_ARRAY_WIDTH / format->width, 2U);
- bin_v = min(IMX219_PIXEL_ARRAY_HEIGHT / format->height, 2U);
+ bin_h = min(IMX219_VISIBLE_WIDTH / format->width, 2U);
+ bin_v = min(IMX219_VISIBLE_HEIGHT / format->height, 2U);
/* Ensure bin_h and bin_v are same to avoid 1:2 or 2:1 stretching */
binning = min(bin_h, bin_v);
@@ -967,10 +967,10 @@ static int imx219_get_selection(struct v4l2_subdev *sd,
case V4L2_SEL_TGT_CROP_DEFAULT:
case V4L2_SEL_TGT_CROP_BOUNDS:
- sel->r.top = IMX219_PIXEL_ARRAY_TOP;
- sel->r.left = IMX219_PIXEL_ARRAY_LEFT;
- sel->r.width = IMX219_PIXEL_ARRAY_WIDTH;
- sel->r.height = IMX219_PIXEL_ARRAY_HEIGHT;
+ sel->r.top = IMX219_VISIBLE_TOP;
+ sel->r.left = IMX219_VISIBLE_LEFT;
+ sel->r.width = IMX219_VISIBLE_WIDTH;
+ sel->r.height = IMX219_VISIBLE_HEIGHT;
return 0;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 02/22] media: imx219: Fix maximum frame length in lines
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
2026-03-25 10:57 ` [PATCH v3 01/22] media: imx219: Rename "PIXEL_ARRAY" as "VISIBLE" Sakari Ailus
@ 2026-03-25 10:57 ` Sakari Ailus
2026-03-25 11:36 ` Dave Stevenson
2026-03-25 10:57 ` [PATCH v3 03/22] media: imx219: Set horizontal blanking on mode change Sakari Ailus
` (19 subsequent siblings)
21 siblings, 1 reply; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:57 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
The driver used the maximum frame length in lines value of 0xffff, but the
maximum appears to be 0xfffe instead. Fix it.
Fixes: 1283b3b8f82b ("media: i2c: Add driver for Sony IMX219 sensor")
Cc: stable@vger.kernel.org
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/i2c/imx219.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index cbd151d4af5f..89061dc1842d 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -72,7 +72,7 @@
/* V_TIMING internal */
#define IMX219_REG_FRM_LENGTH_A CCI_REG16(0x0160)
-#define IMX219_FLL_MAX 0xffff
+#define IMX219_FLL_MAX 0xfffe
#define IMX219_VBLANK_MIN 32
#define IMX219_REG_LINE_LENGTH_A CCI_REG16(0x0162)
#define IMX219_LLP_MIN 0x0d78
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v3 02/22] media: imx219: Fix maximum frame length in lines
2026-03-25 10:57 ` [PATCH v3 02/22] media: imx219: Fix maximum frame length in lines Sakari Ailus
@ 2026-03-25 11:36 ` Dave Stevenson
0 siblings, 0 replies; 29+ messages in thread
From: Dave Stevenson @ 2026-03-25 11:36 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, hans, laurent.pinchart, Prabhakar, Kate Hsuan,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Hi Sakari
On Wed, 25 Mar 2026 at 10:58, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
>
> The driver used the maximum frame length in lines value of 0xffff, but the
> maximum appears to be 0xfffe instead. Fix it.
Agreed that the datasheet says the max_frame_length_lines register
0x1142/0x1143 defaults to 0xfffe (and is read-only).
I haven't checked whether using 0xffff actually works or not, but it
makes no real difference.
> Fixes: 1283b3b8f82b ("media: i2c: Add driver for Sony IMX219 sensor")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> ---
> drivers/media/i2c/imx219.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index cbd151d4af5f..89061dc1842d 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -72,7 +72,7 @@
>
> /* V_TIMING internal */
> #define IMX219_REG_FRM_LENGTH_A CCI_REG16(0x0160)
> -#define IMX219_FLL_MAX 0xffff
> +#define IMX219_FLL_MAX 0xfffe
> #define IMX219_VBLANK_MIN 32
> #define IMX219_REG_LINE_LENGTH_A CCI_REG16(0x0162)
> #define IMX219_LLP_MIN 0x0d78
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3 03/22] media: imx219: Set horizontal blanking on mode change
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
2026-03-25 10:57 ` [PATCH v3 01/22] media: imx219: Rename "PIXEL_ARRAY" as "VISIBLE" Sakari Ailus
2026-03-25 10:57 ` [PATCH v3 02/22] media: imx219: Fix maximum frame length in lines Sakari Ailus
@ 2026-03-25 10:57 ` Sakari Ailus
2026-03-25 11:48 ` Dave Stevenson
2026-03-25 10:58 ` [PATCH v3 04/22] media: imx219: Scale the vblank limits according to rate_factor Sakari Ailus
` (18 subsequent siblings)
21 siblings, 1 reply; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:57 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
The driver UAPI is mode-based, allowing the user to choose a mode from a
small list based on the output size. The vertical blanking is set based on
the mode, do the same for horizontal blanking so the frame rate obtained
is constant.
Additinally, it's best to use a known-good horizontal blanking value as
choosing the value freely may affect image quality. While the minimum
value may not be the best value for horizontal blanking, at least it is
constant rather than a minimum value of a different configuration.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/i2c/imx219.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 89061dc1842d..62a23541b1dc 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -837,11 +837,9 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
struct v4l2_mbus_framefmt *format;
struct v4l2_rect *crop;
u8 bin_h, bin_v, binning;
- u32 prev_line_len;
int ret;
format = v4l2_subdev_state_get_format(state, 0);
- prev_line_len = format->width + imx219->hblank->val;
/*
* Adjust the requested format to match the closest mode. The Bayer
@@ -882,7 +880,7 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
int exposure_max;
int exposure_def;
- int hblank, llp_min;
+ int llp_min;
int pixel_rate;
/* Update limits and set FPS to default */
@@ -924,15 +922,8 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
llp_min - mode->width);
if (ret)
return ret;
- /*
- * Retain PPL setting from previous mode so that the
- * line time does not change on a mode change.
- * Limits have to be recomputed as the controls define
- * the blanking only, so PPL values need to have the
- * mode width subtracted.
- */
- hblank = prev_line_len - mode->width;
- ret = __v4l2_ctrl_s_ctrl(imx219->hblank, hblank);
+
+ ret = __v4l2_ctrl_s_ctrl(imx219->hblank, llp_min - mode->width);
if (ret)
return ret;
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v3 03/22] media: imx219: Set horizontal blanking on mode change
2026-03-25 10:57 ` [PATCH v3 03/22] media: imx219: Set horizontal blanking on mode change Sakari Ailus
@ 2026-03-25 11:48 ` Dave Stevenson
2026-03-25 12:20 ` Sakari Ailus
0 siblings, 1 reply; 29+ messages in thread
From: Dave Stevenson @ 2026-03-25 11:48 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, hans, laurent.pinchart, Prabhakar, Kate Hsuan,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Hi Sakari
On Wed, 25 Mar 2026 at 10:58, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
>
> The driver UAPI is mode-based, allowing the user to choose a mode from a
> small list based on the output size. The vertical blanking is set based on
> the mode, do the same for horizontal blanking so the frame rate obtained
> is constant.
>
> Additinally, it's best to use a known-good horizontal blanking value as
s/Additinally/Additionally
> choosing the value freely may affect image quality. While the minimum
> value may not be the best value for horizontal blanking, at least it is
> constant rather than a minimum value of a different configuration.
I've never known what the preferred behaviour is here. Ranges
typically change on mode change, and v4l2_ctrl_modify_range will reset
to the default if the current value is out of range, but otherwise
leave things alone.
Seeing as you would be the one defining the preferred behaviour, I'll
take this desire as gospel.
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
With the typo corrected:
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> ---
> drivers/media/i2c/imx219.c | 15 +++------------
> 1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index 89061dc1842d..62a23541b1dc 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -837,11 +837,9 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
> struct v4l2_mbus_framefmt *format;
> struct v4l2_rect *crop;
> u8 bin_h, bin_v, binning;
> - u32 prev_line_len;
> int ret;
>
> format = v4l2_subdev_state_get_format(state, 0);
> - prev_line_len = format->width + imx219->hblank->val;
>
> /*
> * Adjust the requested format to match the closest mode. The Bayer
> @@ -882,7 +880,7 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
> if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> int exposure_max;
> int exposure_def;
> - int hblank, llp_min;
> + int llp_min;
> int pixel_rate;
>
> /* Update limits and set FPS to default */
> @@ -924,15 +922,8 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
> llp_min - mode->width);
> if (ret)
> return ret;
> - /*
> - * Retain PPL setting from previous mode so that the
> - * line time does not change on a mode change.
> - * Limits have to be recomputed as the controls define
> - * the blanking only, so PPL values need to have the
> - * mode width subtracted.
> - */
> - hblank = prev_line_len - mode->width;
> - ret = __v4l2_ctrl_s_ctrl(imx219->hblank, hblank);
> +
> + ret = __v4l2_ctrl_s_ctrl(imx219->hblank, llp_min - mode->width);
> if (ret)
> return ret;
>
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v3 03/22] media: imx219: Set horizontal blanking on mode change
2026-03-25 11:48 ` Dave Stevenson
@ 2026-03-25 12:20 ` Sakari Ailus
0 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 12:20 UTC (permalink / raw)
To: Dave Stevenson
Cc: linux-media, hans, laurent.pinchart, Prabhakar, Kate Hsuan,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Hi Dave,
Thanks for the review.
On Wed, Mar 25, 2026 at 11:48:26AM +0000, Dave Stevenson wrote:
> Hi Sakari
>
> On Wed, 25 Mar 2026 at 10:58, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> >
> > The driver UAPI is mode-based, allowing the user to choose a mode from a
> > small list based on the output size. The vertical blanking is set based on
> > the mode, do the same for horizontal blanking so the frame rate obtained
> > is constant.
> >
> > Additinally, it's best to use a known-good horizontal blanking value as
>
> s/Additinally/Additionally
Yes.
>
> > choosing the value freely may affect image quality. While the minimum
> > value may not be the best value for horizontal blanking, at least it is
> > constant rather than a minimum value of a different configuration.
>
> I've never known what the preferred behaviour is here. Ranges
> typically change on mode change, and v4l2_ctrl_modify_range will reset
> to the default if the current value is out of range, but otherwise
> leave things alone.
>
> Seeing as you would be the one defining the preferred behaviour, I'll
> take this desire as gospel.
I wouldn't do this for freely configurable drivers but this driver has a
list of modes that obviously have hand-picked cropping, binning and frame
rate configuration. The further patches (metadata branch in my linuxtv.org
tree) adding support to the Common Raw Sensor Model implement a different
behaviour actually. I guess in practice the difference will be minor as
libcamera presumably sets the control values in any case to its liking.
Maybe adding a few words of documentation on this could make sense?
>
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
> With the typo corrected:
> Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Thanks!
>
> > ---
> > drivers/media/i2c/imx219.c | 15 +++------------
> > 1 file changed, 3 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> > index 89061dc1842d..62a23541b1dc 100644
> > --- a/drivers/media/i2c/imx219.c
> > +++ b/drivers/media/i2c/imx219.c
> > @@ -837,11 +837,9 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
> > struct v4l2_mbus_framefmt *format;
> > struct v4l2_rect *crop;
> > u8 bin_h, bin_v, binning;
> > - u32 prev_line_len;
> > int ret;
> >
> > format = v4l2_subdev_state_get_format(state, 0);
> > - prev_line_len = format->width + imx219->hblank->val;
> >
> > /*
> > * Adjust the requested format to match the closest mode. The Bayer
> > @@ -882,7 +880,7 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
> > if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> > int exposure_max;
> > int exposure_def;
> > - int hblank, llp_min;
> > + int llp_min;
> > int pixel_rate;
> >
> > /* Update limits and set FPS to default */
> > @@ -924,15 +922,8 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
> > llp_min - mode->width);
> > if (ret)
> > return ret;
> > - /*
> > - * Retain PPL setting from previous mode so that the
> > - * line time does not change on a mode change.
> > - * Limits have to be recomputed as the controls define
> > - * the blanking only, so PPL values need to have the
> > - * mode width subtracted.
> > - */
> > - hblank = prev_line_len - mode->width;
> > - ret = __v4l2_ctrl_s_ctrl(imx219->hblank, hblank);
> > +
> > + ret = __v4l2_ctrl_s_ctrl(imx219->hblank, llp_min - mode->width);
> > if (ret)
> > return ret;
> >
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3 04/22] media: imx219: Scale the vblank limits according to rate_factor
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (2 preceding siblings ...)
2026-03-25 10:57 ` [PATCH v3 03/22] media: imx219: Set horizontal blanking on mode change Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 05/22] media: imx219: Fix vertical blanking and exposure for analogue binning Sakari Ailus
` (17 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
The limits for vertical blanking (and frame length in pixels) is related
to the properties of the hardware, it's not in half-line units the driver
uses. Multiply the vertical blanking limits by the rate_factor to satisty
hardware requirements.
Fixes: f513997119f4 ("media: i2c: imx219: Scale the pixel rate for analog binning")
Cc: stable@vger.kernel.org
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/i2c/imx219.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 62a23541b1dc..6819a2fa3262 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -878,14 +878,17 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
crop->top = (IMX219_NATIVE_HEIGHT - crop->height) / 2;
if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
+ unsigned int rate_factor = imx219_get_rate_factor(state);
int exposure_max;
int exposure_def;
int llp_min;
int pixel_rate;
/* Update limits and set FPS to default */
- ret = __v4l2_ctrl_modify_range(imx219->vblank, IMX219_VBLANK_MIN,
- IMX219_FLL_MAX - mode->height, 1,
+ ret = __v4l2_ctrl_modify_range(imx219->vblank,
+ IMX219_VBLANK_MIN * rate_factor,
+ (IMX219_FLL_MAX - mode->height) *
+ rate_factor, rate_factor,
mode->fll_def - mode->height);
if (ret)
return ret;
@@ -928,8 +931,7 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
return ret;
/* Scale the pixel rate based on the mode specific factor */
- pixel_rate = imx219_get_pixel_rate(imx219) *
- imx219_get_rate_factor(state);
+ pixel_rate = imx219_get_pixel_rate(imx219) * rate_factor;
ret = __v4l2_ctrl_modify_range(imx219->pixel_rate, pixel_rate,
pixel_rate, 1, pixel_rate);
if (ret)
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 05/22] media: imx219: Fix vertical blanking and exposure for analogue binning
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (3 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 04/22] media: imx219: Scale the vblank limits according to rate_factor Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 06/22] media: imx219: Don't update exposure limits while setting format Sakari Ailus
` (16 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
When vertical analogue binning is in use, the minimum frame length in
lines decreases to around half of the normal. In relation to the sensor's
output size this means vertical blanking can be negative but that's not an
issue as control values are signed. Remove the workaround for this
non-issue that doubled the pixel rate, frame length in lines and exposure
time.
The resulting change also fixes the minimum, the maximum and the step
values for the control.
Fixes: f513997119f4 ("media: i2c: imx219: Scale the pixel rate for analog binning")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/i2c/imx219.c | 29 +++++++++--------------------
1 file changed, 9 insertions(+), 20 deletions(-)
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 6819a2fa3262..a72630ad1561 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -420,15 +420,6 @@ static void imx219_get_binning(struct v4l2_subdev_state *state, u8 *bin_h,
}
-static inline u32 imx219_get_rate_factor(struct v4l2_subdev_state *state)
-{
- u8 bin_h, bin_v;
-
- imx219_get_binning(state, &bin_h, &bin_v);
-
- return (bin_h & bin_v) == IMX219_BINNING_X2_ANALOG ? 2 : 1;
-}
-
/* -----------------------------------------------------------------------------
* Controls
*/
@@ -440,12 +431,10 @@ static int imx219_set_ctrl(struct v4l2_ctrl *ctrl)
struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
const struct v4l2_mbus_framefmt *format;
struct v4l2_subdev_state *state;
- u32 rate_factor;
int ret = 0;
state = v4l2_subdev_get_locked_active_state(&imx219->sd);
format = v4l2_subdev_state_get_format(state, 0);
- rate_factor = imx219_get_rate_factor(state);
if (ctrl->id == V4L2_CID_VBLANK) {
int exposure_max, exposure_def;
@@ -478,7 +467,7 @@ static int imx219_set_ctrl(struct v4l2_ctrl *ctrl)
break;
case V4L2_CID_EXPOSURE:
cci_write(imx219->regmap, IMX219_REG_EXPOSURE,
- ctrl->val / rate_factor, &ret);
+ ctrl->val, &ret);
break;
case V4L2_CID_DIGITAL_GAIN:
cci_write(imx219->regmap, IMX219_REG_DIGITAL_GAIN,
@@ -495,7 +484,7 @@ static int imx219_set_ctrl(struct v4l2_ctrl *ctrl)
break;
case V4L2_CID_VBLANK:
cci_write(imx219->regmap, IMX219_REG_FRM_LENGTH_A,
- (format->height + ctrl->val) / rate_factor, &ret);
+ format->height + ctrl->val, &ret);
break;
case V4L2_CID_HBLANK:
cci_write(imx219->regmap, IMX219_REG_LINE_LENGTH_A,
@@ -878,7 +867,6 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
crop->top = (IMX219_NATIVE_HEIGHT - crop->height) / 2;
if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
- unsigned int rate_factor = imx219_get_rate_factor(state);
int exposure_max;
int exposure_def;
int llp_min;
@@ -886,15 +874,16 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
/* Update limits and set FPS to default */
ret = __v4l2_ctrl_modify_range(imx219->vblank,
- IMX219_VBLANK_MIN * rate_factor,
- (IMX219_FLL_MAX - mode->height) *
- rate_factor, rate_factor,
- mode->fll_def - mode->height);
+ (int)(mode->height / binning),
+ IMX219_FLL_MAX - mode->height, 1,
+ (int)(mode->fll_def / binning) -
+ (int)mode->height);
if (ret)
return ret;
ret = __v4l2_ctrl_s_ctrl(imx219->vblank,
- mode->fll_def - mode->height);
+ (int)(mode->fll_def / binning) -
+ (int)mode->height);
if (ret)
return ret;
@@ -931,7 +920,7 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
return ret;
/* Scale the pixel rate based on the mode specific factor */
- pixel_rate = imx219_get_pixel_rate(imx219) * rate_factor;
+ pixel_rate = imx219_get_pixel_rate(imx219);
ret = __v4l2_ctrl_modify_range(imx219->pixel_rate, pixel_rate,
pixel_rate, 1, pixel_rate);
if (ret)
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 06/22] media: imx219: Don't update exposure limits while setting format
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (4 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 05/22] media: imx219: Fix vertical blanking and exposure for analogue binning Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 12:37 ` Dave Stevenson
2026-03-25 10:58 ` [PATCH v3 07/22] media: imx219: Rename "binning" as "bin_hv" in imx219_set_pad_format Sakari Ailus
` (15 subsequent siblings)
21 siblings, 1 reply; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Don't update exposure limits explicitly while setting format. This is
already done through the s_ctrl() callback.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/i2c/imx219.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index a72630ad1561..ca6a5939773d 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -867,8 +867,6 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
crop->top = (IMX219_NATIVE_HEIGHT - crop->height) / 2;
if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
- int exposure_max;
- int exposure_def;
int llp_min;
int pixel_rate;
@@ -887,18 +885,6 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
if (ret)
return ret;
- /* Update max exposure while meeting expected vblanking */
- exposure_max = mode->fll_def - IMX219_EXPOSURE_OFFSET;
- exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ?
- exposure_max : IMX219_EXPOSURE_DEFAULT;
- ret = __v4l2_ctrl_modify_range(imx219->exposure,
- imx219->exposure->minimum,
- exposure_max,
- imx219->exposure->step,
- exposure_def);
- if (ret)
- return ret;
-
/*
* With analog binning the default minimum line length of 3448
* can cause artefacts with RAW10 formats, because the ADC
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v3 06/22] media: imx219: Don't update exposure limits while setting format
2026-03-25 10:58 ` [PATCH v3 06/22] media: imx219: Don't update exposure limits while setting format Sakari Ailus
@ 2026-03-25 12:37 ` Dave Stevenson
2026-03-25 12:47 ` Sakari Ailus
0 siblings, 1 reply; 29+ messages in thread
From: Dave Stevenson @ 2026-03-25 12:37 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, hans, laurent.pinchart, Prabhakar, Kate Hsuan,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Hi Sakari
On Wed, 25 Mar 2026 at 10:58, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
>
> Don't update exposure limits explicitly while setting format. This is
> already done through the s_ctrl() callback.
done through the s_ctrl() callback for V4L2_CID_VBLANK.
I believe this change leaves a potential for future failure if any
modes get added because the ctrl handler framework swallows any s_ctrl
that doesn't change the value.
vblank is an offset on top of height. exposure is absolute in the
number of lines.
As an example, take the current mode of 1920x1080 with fll_def 1763,
therefore a vblank of 1763-1080 = 683, and max exposure of 1080-4 =
1076. Add a new mode of 640x480 with fll_def of 1163, therefore vblank
is 1163-480 = 683, and max exposure is 480-4 = 476. set_pad_format for
either mode will set vblank to 683. As it's the same value,
imx219_set_ctrl will not be called to update vblank, leaving the wrong
max exposure value.
You could add V4L2_CTRL_FLAG_EXECUTE_ON_WRITE to V4L2_CID_VBLANK to
always call s_ctrl, but that would have a larger downside if userspace
repeatedly set it to the same value.
It doesn't affect any of the current modes, but it's going to be a
very ugly one for the unwary. imx219 is often pointed to as a
reference for those writing new drivers, so this would likely
propagate.
Dave
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> drivers/media/i2c/imx219.c | 14 --------------
> 1 file changed, 14 deletions(-)
>
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index a72630ad1561..ca6a5939773d 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -867,8 +867,6 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
> crop->top = (IMX219_NATIVE_HEIGHT - crop->height) / 2;
>
> if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> - int exposure_max;
> - int exposure_def;
> int llp_min;
> int pixel_rate;
>
> @@ -887,18 +885,6 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
> if (ret)
> return ret;
>
> - /* Update max exposure while meeting expected vblanking */
> - exposure_max = mode->fll_def - IMX219_EXPOSURE_OFFSET;
> - exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ?
> - exposure_max : IMX219_EXPOSURE_DEFAULT;
> - ret = __v4l2_ctrl_modify_range(imx219->exposure,
> - imx219->exposure->minimum,
> - exposure_max,
> - imx219->exposure->step,
> - exposure_def);
> - if (ret)
> - return ret;
> -
> /*
> * With analog binning the default minimum line length of 3448
> * can cause artefacts with RAW10 formats, because the ADC
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v3 06/22] media: imx219: Don't update exposure limits while setting format
2026-03-25 12:37 ` Dave Stevenson
@ 2026-03-25 12:47 ` Sakari Ailus
0 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 12:47 UTC (permalink / raw)
To: Dave Stevenson
Cc: linux-media, hans, laurent.pinchart, Prabhakar, Kate Hsuan,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Hi Dave,
On Wed, Mar 25, 2026 at 12:37:54PM +0000, Dave Stevenson wrote:
> Hi Sakari
>
> On Wed, 25 Mar 2026 at 10:58, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> >
> > Don't update exposure limits explicitly while setting format. This is
> > already done through the s_ctrl() callback.
>
> done through the s_ctrl() callback for V4L2_CID_VBLANK.
>
> I believe this change leaves a potential for future failure if any
> modes get added because the ctrl handler framework swallows any s_ctrl
> that doesn't change the value.
> vblank is an offset on top of height. exposure is absolute in the
> number of lines.
That's a fair point. I'll postpone doing this patch for now or squash it
with something else. With the Common Raw Sensor Model, the driver
internally uses frame length in lines and line length in pixels so the
problem goes away by itself.
>
> As an example, take the current mode of 1920x1080 with fll_def 1763,
> therefore a vblank of 1763-1080 = 683, and max exposure of 1080-4 =
> 1076. Add a new mode of 640x480 with fll_def of 1163, therefore vblank
> is 1163-480 = 683, and max exposure is 480-4 = 476. set_pad_format for
> either mode will set vblank to 683. As it's the same value,
> imx219_set_ctrl will not be called to update vblank, leaving the wrong
> max exposure value.
>
> You could add V4L2_CTRL_FLAG_EXECUTE_ON_WRITE to V4L2_CID_VBLANK to
> always call s_ctrl, but that would have a larger downside if userspace
> repeatedly set it to the same value.
I agree.
>
> It doesn't affect any of the current modes, but it's going to be a
> very ugly one for the unwary. imx219 is often pointed to as a
> reference for those writing new drivers, so this would likely
> propagate.
I hope no new modes will be added to this driver. The Common Raw Sensor
Model support makes it freely configurable. I hope to send an update on
that soon.
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3 07/22] media: imx219: Rename "binning" as "bin_hv" in imx219_set_pad_format
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (5 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 06/22] media: imx219: Don't update exposure limits while setting format Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 08/22] media: Documentation: Improve LINK_FREQ documentation Sakari Ailus
` (14 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Rename "binning" as "bin_hv" in anticipation of having a variable called
"binning" for another purpose.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/i2c/imx219.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index ca6a5939773d..5a85d76af65a 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -825,7 +825,7 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
const struct imx219_mode *mode;
struct v4l2_mbus_framefmt *format;
struct v4l2_rect *crop;
- u8 bin_h, bin_v, binning;
+ u8 bin_h, bin_v, bin_hv;
int ret;
format = v4l2_subdev_state_get_format(state, 0);
@@ -858,11 +858,11 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
bin_v = min(IMX219_VISIBLE_HEIGHT / format->height, 2U);
/* Ensure bin_h and bin_v are same to avoid 1:2 or 2:1 stretching */
- binning = min(bin_h, bin_v);
+ bin_hv = min(bin_h, bin_v);
crop = v4l2_subdev_state_get_crop(state, 0);
- crop->width = format->width * binning;
- crop->height = format->height * binning;
+ crop->width = format->width * bin_hv;
+ crop->height = format->height * bin_hv;
crop->left = (IMX219_NATIVE_WIDTH - crop->width) / 2;
crop->top = (IMX219_NATIVE_HEIGHT - crop->height) / 2;
@@ -872,15 +872,15 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
/* Update limits and set FPS to default */
ret = __v4l2_ctrl_modify_range(imx219->vblank,
- (int)(mode->height / binning),
+ (int)(mode->height / bin_hv),
IMX219_FLL_MAX - mode->height, 1,
- (int)(mode->fll_def / binning) -
+ (int)(mode->fll_def / bin_hv) -
(int)mode->height);
if (ret)
return ret;
ret = __v4l2_ctrl_s_ctrl(imx219->vblank,
- (int)(mode->fll_def / binning) -
+ (int)(mode->fll_def / bin_hv) -
(int)mode->height);
if (ret)
return ret;
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 08/22] media: Documentation: Improve LINK_FREQ documentation
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (6 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 07/22] media: imx219: Rename "binning" as "bin_hv" in imx219_set_pad_format Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 09/22] media: Documentation: Improve pixel rate calculation documentation Sakari Ailus
` (13 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Add a reference to the LINK_FREQ control and clarify the meaning of the
control as for C-PHY the matter is less obvious.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Mirela Rabulea <mirela.rabulea@nxp.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
Documentation/driver-api/media/tx-rx.rst | 3 ++-
.../userspace-api/media/v4l/ext-ctrls-image-process.rst | 4 +++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/Documentation/driver-api/media/tx-rx.rst b/Documentation/driver-api/media/tx-rx.rst
index 22e1b13ecde9..7df2407817b3 100644
--- a/Documentation/driver-api/media/tx-rx.rst
+++ b/Documentation/driver-api/media/tx-rx.rst
@@ -93,7 +93,8 @@ where
* - variable or constant
- description
* - link_freq
- - The value of the ``V4L2_CID_LINK_FREQ`` integer64 menu item.
+ - The value of the :ref:`V4L2_CID_LINK_FREQ <v4l2-cid-link-freq>` integer64
+ menu item.
* - nr_of_lanes
- Number of data lanes used on the CSI-2 link.
* - 2
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 6d516f041ca2..ee88933256dd 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-image-process.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-image-process.rst
@@ -24,7 +24,9 @@ Image Process Control IDs
.. _v4l2-cid-link-freq:
``V4L2_CID_LINK_FREQ (integer menu)``
- The frequency of the data bus (e.g. parallel or CSI-2).
+ The fundamental frequency of the operating symbol rate (serial interfaces
+ such as CSI-2) or the sampling rate (parallel interfaces such as DVP or
+ Bt.565) of the data interface.
.. _v4l2-cid-pixel-rate:
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 09/22] media: Documentation: Improve pixel rate calculation documentation
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (7 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 08/22] media: Documentation: Improve LINK_FREQ documentation Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 10/22] media: v4l2-subdev: Refactor returning routes Sakari Ailus
` (12 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Improve documentation on calculating the pixel rate, by adding references
to relevant functions and mentioning V4L2 fwnode endpoint instead of OF
endpoint.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Mirela Rabulea <mirela.rabulea@nxp.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
Documentation/driver-api/media/tx-rx.rst | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/driver-api/media/tx-rx.rst b/Documentation/driver-api/media/tx-rx.rst
index 7df2407817b3..9b231fa0216a 100644
--- a/Documentation/driver-api/media/tx-rx.rst
+++ b/Documentation/driver-api/media/tx-rx.rst
@@ -104,7 +104,11 @@ where
* - k
- 16 for D-PHY and 7 for C-PHY.
-Information on whether D-PHY or C-PHY is used, and the value of ``nr_of_lanes``, can be obtained from the OF endpoint configuration.
+Information on whether D-PHY or C-PHY is used as well as the value of
+``nr_of_lanes`` can be obtained from the V4L2 endpoint configuration; see
+:c:func:`v4l2_fwnode_endpoint_alloc_parse()`,
+:c:func:`v4l2_fwnode_endpoint_parse()` and
+:c:func:`v4l2_get_active_data_lanes()`.
.. note::
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 10/22] media: v4l2-subdev: Refactor returning routes
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (8 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 09/22] media: Documentation: Improve pixel rate calculation documentation Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 11/22] media: v4l2-subdev: Allow accessing routes with STREAMS client capability Sakari Ailus
` (11 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Refactor returning the routes by adding a new function that essentially
does a memcopy and sets the number of the routes in the routing table.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/v4l2-core/v4l2-subdev.c | 34 +++++++++++++--------------
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 831c69c958b8..f8fde395a53a 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -629,6 +629,19 @@ subdev_ioctl_get_state(struct v4l2_subdev *sd, struct v4l2_subdev_fh *subdev_fh,
v4l2_subdev_get_unlocked_active_state(sd);
}
+static void copy_routes_state_to_routing(struct v4l2_subdev_routing *routing,
+ const struct v4l2_subdev_state *state)
+{
+ struct v4l2_subdev_route *routes =
+ (struct v4l2_subdev_route *)(uintptr_t)routing->routes;
+ u32 copy_routes = min(routing->len_routes, state->routing.num_routes);
+
+ for (u32 i = 0; i < copy_routes; i++)
+ routes[i] = state->routing.routes[i];
+
+ routing->num_routes = state->routing.num_routes;
+}
+
static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
struct v4l2_subdev_state *state)
{
@@ -1000,7 +1013,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
case VIDIOC_SUBDEV_G_ROUTING: {
struct v4l2_subdev_routing *routing = arg;
- struct v4l2_subdev_krouting *krouting;
if (!v4l2_subdev_enable_streams_api)
return -ENOIOCTLCMD;
@@ -1010,13 +1022,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
memset(routing->reserved, 0, sizeof(routing->reserved));
- krouting = &state->routing;
-
- memcpy((struct v4l2_subdev_route *)(uintptr_t)routing->routes,
- krouting->routes,
- min(krouting->num_routes, routing->len_routes) *
- sizeof(*krouting->routes));
- routing->num_routes = krouting->num_routes;
+ copy_routes_state_to_routing(routing, state);
return 0;
}
@@ -1084,11 +1090,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
* the routing table.
*/
if (!v4l2_subdev_has_op(sd, pad, set_routing)) {
- memcpy((struct v4l2_subdev_route *)(uintptr_t)routing->routes,
- state->routing.routes,
- min(state->routing.num_routes, routing->len_routes) *
- sizeof(*state->routing.routes));
- routing->num_routes = state->routing.num_routes;
+ copy_routes_state_to_routing(routing, state);
return 0;
}
@@ -1102,11 +1104,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
if (rval < 0)
return rval;
- memcpy((struct v4l2_subdev_route *)(uintptr_t)routing->routes,
- state->routing.routes,
- min(state->routing.num_routes, routing->len_routes) *
- sizeof(*state->routing.routes));
- routing->num_routes = state->routing.num_routes;
+ copy_routes_state_to_routing(routing, state);
return 0;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 11/22] media: v4l2-subdev: Allow accessing routes with STREAMS client capability
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (9 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 10/22] media: v4l2-subdev: Refactor returning routes Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 12/22] media: mc: Simplify link processing in __media_pipeline_start() Sakari Ailus
` (10 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Disable access to routes when the STREAMS client capability bit isn't set.
Routes aren't relevant otherwise anyway.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by <mirela.rabulea@nxp.com>
---
drivers/media/v4l2-core/v4l2-subdev.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index f8fde395a53a..647587c0499a 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1020,6 +1020,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
if (!(sd->flags & V4L2_SUBDEV_FL_STREAMS))
return -ENOIOCTLCMD;
+ if (!client_supports_streams)
+ return -EINVAL;
+
memset(routing->reserved, 0, sizeof(routing->reserved));
copy_routes_state_to_routing(routing, state);
@@ -1041,6 +1044,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
if (!(sd->flags & V4L2_SUBDEV_FL_STREAMS))
return -ENOIOCTLCMD;
+ if (!client_supports_streams)
+ return -EINVAL;
+
if (routing->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)
return -EPERM;
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 12/22] media: mc: Simplify link processing in __media_pipeline_start()
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (10 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 11/22] media: v4l2-subdev: Allow accessing routes with STREAMS client capability Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 13/22] media: mc: Separate single link validation into a new function Sakari Ailus
` (9 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
There are two conditions checking the ENABLED link flag in the loop
going through the links related to an entity. Drop the other one and
simplify the remaining code.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/mc/mc-entity.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 3fa0bc687851..6bf4730b89d2 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -838,17 +838,16 @@ __must_check int __media_pipeline_start(struct media_pad *origin,
if (link->sink != pad && link->source != pad)
continue;
- /* Record if the pad has links and enabled links. */
- if (link->flags & MEDIA_LNK_FL_ENABLED)
- has_enabled_link = true;
-
/*
- * Validate the link if it's enabled and has the
- * current pad as its sink.
+ * Ensure the link is enabled and if so, record
+ * it. Proceed to the next link if the current pad isn't
+ * the sink pad of the link.
*/
if (!(link->flags & MEDIA_LNK_FL_ENABLED))
continue;
+ has_enabled_link = true;
+
if (link->sink != pad)
continue;
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 13/22] media: mc: Separate single link validation into a new function
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (11 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 12/22] media: mc: Simplify link processing in __media_pipeline_start() Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 14/14] media: Improve enable_streams and disable_streams documentation Sakari Ailus
` (8 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Add a new function __media_pipeline_validate_one() to validate a single
link in a pipeline. This will soon be used for performing validation in
multiple phases.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/mc/mc-entity.c | 74 ++++++++++++++++++++----------------
1 file changed, 42 insertions(+), 32 deletions(-)
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 6bf4730b89d2..717569bd1a8c 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -768,6 +768,45 @@ static int media_pipeline_populate(struct media_pipeline *pipe,
return ret;
}
+static int
+__media_pipeline_validate_one(struct media_pad *origin,
+ struct media_pad *pad, struct media_link *link,
+ bool *has_enabled_link)
+{
+ struct media_device *mdev = origin->graph_obj.mdev;
+ struct media_entity *entity = pad->entity;
+ int ret;
+
+ /* Return here if the link is disabled. */
+ if (!(link->flags & MEDIA_LNK_FL_ENABLED))
+ return 0;
+
+ if (has_enabled_link)
+ *has_enabled_link = true;
+
+ /* Skip validation if the current pad isn't the sink pad of the link. */
+ if (link->sink != pad)
+ return 0;
+
+ if (!entity->ops || !entity->ops->link_validate)
+ return 0;
+
+ ret = entity->ops->link_validate(link);
+ if (ret) {
+ dev_dbg(mdev->dev,
+ "Link '%s':%u -> '%s':%u failed validation: %d\n",
+ link->source->entity->name, link->source->index,
+ link->sink->entity->name, link->sink->index, ret);
+ return ret;
+ }
+
+ dev_dbg(mdev->dev, "Link '%s':%u -> '%s':%u is valid\n",
+ link->source->entity->name, link->source->index,
+ link->sink->entity->name, link->sink->index);
+
+ return 0;
+}
+
__must_check int __media_pipeline_start(struct media_pad *origin,
struct media_pipeline *pipe)
{
@@ -838,39 +877,10 @@ __must_check int __media_pipeline_start(struct media_pad *origin,
if (link->sink != pad && link->source != pad)
continue;
- /*
- * Ensure the link is enabled and if so, record
- * it. Proceed to the next link if the current pad isn't
- * the sink pad of the link.
- */
- if (!(link->flags & MEDIA_LNK_FL_ENABLED))
- continue;
-
- has_enabled_link = true;
-
- if (link->sink != pad)
- continue;
-
- if (!entity->ops || !entity->ops->link_validate)
- continue;
-
- ret = entity->ops->link_validate(link);
- if (ret) {
- dev_dbg(mdev->dev,
- "Link '%s':%u -> '%s':%u failed validation: %d\n",
- link->source->entity->name,
- link->source->index,
- link->sink->entity->name,
- link->sink->index, ret);
+ ret = __media_pipeline_validate_one(origin, pad, link,
+ &has_enabled_link);
+ if (ret)
goto error;
- }
-
- dev_dbg(mdev->dev,
- "Link '%s':%u -> '%s':%u is valid\n",
- link->source->entity->name,
- link->source->index,
- link->sink->entity->name,
- link->sink->index);
}
/*
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 14/14] media: Improve enable_streams and disable_streams documentation
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (12 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 13/22] media: mc: Separate single link validation into a new function Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 12:34 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 14/22] media: uapi: Bump the STREAMS bit a little Sakari Ailus
` (7 subsequent siblings)
21 siblings, 1 reply; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Document that enable_streams may start additional streams and
disable_streams may not disable requested streams if other related streams
are still enabled.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
include/media/v4l2-subdev.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index d256b7ec8f84..4588992b4417 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -814,6 +814,10 @@ struct v4l2_subdev_state {
* V4L2_SUBDEV_CAP_STREAMS sub-device capability flag can ignore the mask
* argument.
*
+ * Starting the requested streams may require starting additional
+ * streams. Streams that are started together due to hardware are called a
+ * stream group.
+ *
* @disable_streams: Disable the streams defined in streams_mask on the given
* source pad. Subdevs that implement this operation must use the active
* state management provided by the subdev core (enabled through a call to
@@ -823,6 +827,9 @@ struct v4l2_subdev_state {
* Drivers that support only a single stream without setting the
* V4L2_SUBDEV_CAP_STREAMS sub-device capability flag can ignore the mask
* argument.
+ *
+ * A stream group is disabled when one or more streams in the stream
+ * group are disabled.
*/
struct v4l2_subdev_pad_ops {
int (*enum_mbus_code)(struct v4l2_subdev *sd,
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v3 14/14] media: Improve enable_streams and disable_streams documentation
2026-03-25 10:58 ` [PATCH v3 14/14] media: Improve enable_streams and disable_streams documentation Sakari Ailus
@ 2026-03-25 12:34 ` Sakari Ailus
0 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 12:34 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, hans, laurent.pinchart, Prabhakar, Kate Hsuan,
Dave Stevenson, Tommaso Merciai, Benjamin Mugnier,
Sylvain Petinot, Christophe JAILLET, Julien Massot,
Naushir Patuck, Yan, Dongcheng, Cao, Bingbu, Qiu, Tian Shu,
Stefan Klug, Mirela Rabulea, André Apitzsch,
Heimir Thor Sverrisson, Kieran Bingham, Mehdi Djait,
Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Folks,
Please ignore this patch -- it was somehow left to the directory where I
sent the set.
On Wed, Mar 25, 2026 at 12:58:10PM +0200, Sakari Ailus wrote:
> Document that enable_streams may start additional streams and
> disable_streams may not disable requested streams if other related streams
> are still enabled.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Reviewed-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> ---
> include/media/v4l2-subdev.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index d256b7ec8f84..4588992b4417 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -814,6 +814,10 @@ struct v4l2_subdev_state {
> * V4L2_SUBDEV_CAP_STREAMS sub-device capability flag can ignore the mask
> * argument.
> *
> + * Starting the requested streams may require starting additional
> + * streams. Streams that are started together due to hardware are called a
> + * stream group.
> + *
> * @disable_streams: Disable the streams defined in streams_mask on the given
> * source pad. Subdevs that implement this operation must use the active
> * state management provided by the subdev core (enabled through a call to
> @@ -823,6 +827,9 @@ struct v4l2_subdev_state {
> * Drivers that support only a single stream without setting the
> * V4L2_SUBDEV_CAP_STREAMS sub-device capability flag can ignore the mask
> * argument.
> + *
> + * A stream group is disabled when one or more streams in the stream
> + * group are disabled.
> */
> struct v4l2_subdev_pad_ops {
> int (*enum_mbus_code)(struct v4l2_subdev *sd,
> --
> 2.47.3
>
>
--
Sakari Ailus
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3 14/22] media: uapi: Bump the STREAMS bit a little
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (13 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 14/14] media: Improve enable_streams and disable_streams documentation Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 15/22] media: mc: Don't care about unsettable flags in MEDIA_IOC_LINK_SETUP Sakari Ailus
` (6 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Bump the V4L2_SUBDEV_CAP_STREAMS by one bit up, order to avoid confusing
libcamera with streams that has moved forward from the original libcamera
implementation. The bit can presumably be taken into use but only after
the other free bits.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
include/uapi/linux/v4l2-subdev.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
index 2347e266cf75..6160c3e21436 100644
--- a/include/uapi/linux/v4l2-subdev.h
+++ b/include/uapi/linux/v4l2-subdev.h
@@ -196,8 +196,11 @@ struct v4l2_subdev_capability {
/* The v4l2 sub-device video device node is registered in read-only mode. */
#define V4L2_SUBDEV_CAP_RO_SUBDEV 0x00000001
+/* Reserved, old STREAMS bit libcamera used before API stabilisation. */
+/* #define V4L2_SUBDEV_CAP_STREAMS_PRELIMINARY 0x00000002 */
+
/* The v4l2 sub-device supports routing and multiplexed streams. */
-#define V4L2_SUBDEV_CAP_STREAMS 0x00000002
+#define V4L2_SUBDEV_CAP_STREAMS 0x00000004
/*
* Is the route active? An active route will start when streaming is enabled
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 15/22] media: mc: Don't care about unsettable flags in MEDIA_IOC_LINK_SETUP
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (14 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 14/22] media: uapi: Bump the STREAMS bit a little Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 16/22] media: mc: Add MEDIA_LNK_FL_VALIDATE_LATE Sakari Ailus
` (5 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
The implementation of MEDIA_IOC_LINK_SETUP currently requires that all
flags that are set by the driver are correctly set as the driver expects.
This poses a problem for adding new flags as programs could not work with
links that have unknown flags even when the use of these flags wouldn't
affect the program.
Ignore the non-settable link flags.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/mc/mc-entity.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 717569bd1a8c..287eded356bb 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -1319,7 +1319,7 @@ static int __media_entity_setup_link_notify(struct media_link *link, u32 flags)
int __media_entity_setup_link(struct media_link *link, u32 flags)
{
- const u32 mask = MEDIA_LNK_FL_ENABLED;
+ const u32 settable_flags = MEDIA_LNK_FL_ENABLED;
struct media_device *mdev;
struct media_pad *source, *sink;
int ret = -EBUSY;
@@ -1327,9 +1327,9 @@ int __media_entity_setup_link(struct media_link *link, u32 flags)
if (link == NULL)
return -EINVAL;
- /* The non-modifiable link flags must not be modified. */
- if ((link->flags & ~mask) != (flags & ~mask))
- return -EINVAL;
+ /* Only allow changing user-settable flags. */
+ flags &= settable_flags;
+ flags |= link->flags & ~settable_flags;
if (link->flags & MEDIA_LNK_FL_IMMUTABLE)
return link->flags == flags ? 0 : -EINVAL;
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 16/22] media: mc: Add MEDIA_LNK_FL_VALIDATE_LATE
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (15 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 15/22] media: mc: Don't care about unsettable flags in MEDIA_IOC_LINK_SETUP Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 17/22] media: Improve enable_streams and disable_streams documentation Sakari Ailus
` (4 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Add MEDIA_LNK_FL_VALIDATE_LATE flag to support late validation of links.
This is serving the use case where video devices are configured and
started streaming indepenently of each other but this sequence may be run
in series, in such a way that a video device in a pipeline starts
streaming before another one is configured.
Before this flag, drivers have resorted to implementing the link
validation separately for the video nodes as part of streaming start
sequence.
media_pipeline_start() shall be called on each leaf entity connected to
the graph with a link where MEDIA_LNK_FL_VALIDATE_LATE is set before
uphardware operation.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
.../media/mediactl/media-ioc-setup-link.rst | 4 +
.../media/mediactl/media-types.rst | 5 ++
drivers/media/mc/mc-entity.c | 82 ++++++++++++++++++-
include/uapi/linux/media.h | 1 +
4 files changed, 88 insertions(+), 4 deletions(-)
diff --git a/Documentation/userspace-api/media/mediactl/media-ioc-setup-link.rst b/Documentation/userspace-api/media/mediactl/media-ioc-setup-link.rst
index 23208300cb61..7c2bced57e77 100644
--- a/Documentation/userspace-api/media/mediactl/media-ioc-setup-link.rst
+++ b/Documentation/userspace-api/media/mediactl/media-ioc-setup-link.rst
@@ -49,6 +49,10 @@ Only links marked with the ``DYNAMIC`` link flag can be enabled/disabled
while streaming media data. Attempting to enable or disable a streaming
non-dynamic link will return an ``EBUSY`` error code.
+Pipeline validation may be delayed for links marked with the ``VALIDATE_LATE``
+flag until actual hardware operation even if the rest of the pipeline would be
+validated at an earlier point of time.
+
If the specified link can't be found the driver returns with an ``EINVAL``
error code.
diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst
index 6332e8395263..d6a690655a01 100644
--- a/Documentation/userspace-api/media/mediactl/media-types.rst
+++ b/Documentation/userspace-api/media/mediactl/media-types.rst
@@ -391,6 +391,7 @@ must be set for every pad.
.. _MEDIA-LNK-FL-ENABLED:
.. _MEDIA-LNK-FL-IMMUTABLE:
.. _MEDIA-LNK-FL-DYNAMIC:
+.. _MEDIA-LNK-FL-VALIDATE-LATE:
.. _MEDIA-LNK-FL-LINK-TYPE:
.. flat-table:: Media link flags
@@ -410,6 +411,10 @@ must be set for every pad.
- The link enabled state can be modified during streaming. This flag
is set by drivers and is read-only for applications.
+ * - ``MEDIA_LNK_FL_VALIDATE_LATE``
+ - The validation of the link may be delayed up to until the start of
+ hardware operation.
+
* - ``MEDIA_LNK_FL_LINK_TYPE``
- This is a bitmask that defines the type of the link. The following
link types are currently supported:
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 287eded356bb..bb639ce4d86e 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -771,7 +771,7 @@ static int media_pipeline_populate(struct media_pipeline *pipe,
static int
__media_pipeline_validate_one(struct media_pad *origin,
struct media_pad *pad, struct media_link *link,
- bool *has_enabled_link)
+ bool *has_enabled_link, bool skip_validation)
{
struct media_device *mdev = origin->graph_obj.mdev;
struct media_entity *entity = pad->entity;
@@ -784,6 +784,9 @@ __media_pipeline_validate_one(struct media_pad *origin,
if (has_enabled_link)
*has_enabled_link = true;
+ if (skip_validation)
+ return 0;
+
/* Skip validation if the current pad isn't the sink pad of the link. */
if (link->sink != pad)
return 0;
@@ -825,11 +828,48 @@ __must_check int __media_pipeline_start(struct media_pad *origin,
return -EINVAL;
/*
- * If the pipeline has already been started, it is guaranteed to be
- * valid, so just increase the start count.
+ * Increase start count on pipelines that have been validated
+ * earlier. Also check links with the VALIDATE_LATE flag here.
*/
if (pipe->start_count) {
+ struct media_link *link;
+
+ link = __media_entity_next_link(origin->entity, NULL,
+ MEDIA_LNK_FL_DATA_LINK);
+ if (link && link->flags & MEDIA_LNK_FL_VALIDATE_LATE) {
+ struct media_link *link2 =
+ __media_entity_next_link(origin->entity, link,
+ MEDIA_LNK_FL_DATA_LINK);
+ bool has_enabled_link = false;
+
+ /*
+ * Only a single pad is allowed for VALIDATE_LATE
+ * links. That pad needs to have exactly one link.
+ */
+ if (origin->entity->num_pads != 1)
+ return -EINVAL;
+
+ if (!link || link2)
+ return -EINVAL;
+
+ dev_dbg(mdev->dev,
+ "Validating pad '%s':%u late\n",
+ origin->entity->name, origin->index);
+
+ ret = __media_pipeline_validate_one(link->sink,
+ link->sink, link,
+ &has_enabled_link,
+ false);
+ if (ret)
+ return ret;
+
+ if (origin->flags & MEDIA_PAD_FL_MUST_CONNECT &&
+ !has_enabled_link)
+ return -ENOLINK;
+ }
+
pipe->start_count++;
+
return 0;
}
@@ -873,12 +913,19 @@ __must_check int __media_pipeline_start(struct media_pad *origin,
* the connected sink pad to avoid duplicating checks.
*/
for_each_media_entity_data_link(entity, link) {
+ /* Skip late-validated links not connected to origin. */
+ bool skip_validation =
+ link->flags & MEDIA_LNK_FL_VALIDATE_LATE &&
+ (link->sink == origin ||
+ link->source == origin);
+
/* Skip links unrelated to the current pad. */
if (link->sink != pad && link->source != pad)
continue;
ret = __media_pipeline_validate_one(origin, pad, link,
- &has_enabled_link);
+ &has_enabled_link,
+ skip_validation);
if (ret)
goto error;
}
@@ -1158,6 +1205,33 @@ media_create_pad_link(struct media_entity *source, u16 source_pad,
if (WARN_ON(!(sink->pads[sink_pad].flags & MEDIA_PAD_FL_SINK)))
return -EINVAL;
+ /*
+ * With the late validate flag, either source or sink shall have exactly
+ * one pad and no links before this one. Similarly, no links may be
+ * added to entities with a single pad and an existing late-validated
+ * link.
+ */
+ if (flags & MEDIA_LNK_FL_VALIDATE_LATE) {
+ if (!(source->num_pads == 1 && !source->num_links) &&
+ !(sink->num_pads == 1 && !sink->num_links))
+ return -EINVAL;
+ } else {
+ struct media_entity *entities[] = { source, sink };
+
+ for (unsigned int i = 0; i < ARRAY_SIZE(entities); i++) {
+ if (entities[i]->num_pads != 1)
+ continue;
+
+ struct media_link *__link =
+ __media_entity_next_link(entities[i], NULL,
+ MEDIA_LNK_FL_DATA_LINK);
+
+ if (__link &&
+ __link->flags & MEDIA_LNK_FL_VALIDATE_LATE)
+ return -EINVAL;
+ }
+ }
+
link = media_add_link(&source->links);
if (link == NULL)
return -ENOMEM;
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 1c80b1d6bbaf..c96e2118ea99 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -219,6 +219,7 @@ struct media_pad_desc {
#define MEDIA_LNK_FL_ENABLED (1U << 0)
#define MEDIA_LNK_FL_IMMUTABLE (1U << 1)
#define MEDIA_LNK_FL_DYNAMIC (1U << 2)
+#define MEDIA_LNK_FL_VALIDATE_LATE (1U << 3)
#define MEDIA_LNK_FL_LINK_TYPE (0xf << 28)
# define MEDIA_LNK_FL_DATA_LINK (0U << 28)
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 17/22] media: Improve enable_streams and disable_streams documentation
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (16 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 16/22] media: mc: Add MEDIA_LNK_FL_VALIDATE_LATE Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 18/22] media: v4l2-subdev: Move subdev client capabilities into a new struct Sakari Ailus
` (3 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Document that enable_streams may start additional streams and
disable_streams may not disable requested streams if other related streams
are still enabled.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
include/media/v4l2-subdev.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index d256b7ec8f84..4588992b4417 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -814,6 +814,10 @@ struct v4l2_subdev_state {
* V4L2_SUBDEV_CAP_STREAMS sub-device capability flag can ignore the mask
* argument.
*
+ * Starting the requested streams may require starting additional
+ * streams. Streams that are started together due to hardware are called a
+ * stream group.
+ *
* @disable_streams: Disable the streams defined in streams_mask on the given
* source pad. Subdevs that implement this operation must use the active
* state management provided by the subdev core (enabled through a call to
@@ -823,6 +827,9 @@ struct v4l2_subdev_state {
* Drivers that support only a single stream without setting the
* V4L2_SUBDEV_CAP_STREAMS sub-device capability flag can ignore the mask
* argument.
+ *
+ * A stream group is disabled when one or more streams in the stream
+ * group are disabled.
*/
struct v4l2_subdev_pad_ops {
int (*enum_mbus_code)(struct v4l2_subdev *sd,
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 18/22] media: v4l2-subdev: Move subdev client capabilities into a new struct
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (17 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 17/22] media: Improve enable_streams and disable_streams documentation Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 19/22] media: v4l2-subdev: Add struct v4l2_subdev_client_info pointer to pad ops Sakari Ailus
` (2 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Add struct v4l2_subdev_client_info to hold sub-device client capability
bits that used to be stored in the client_caps field of struct
v4l2_subdev_fh. The intent is to enable passing this struct to sub-device
pad operation callbacks for capability information. The main reason why
this is a new struct instead of a u64 field is that modifying the callback
arguments requires touching almost every sub-device driver and that is
desirable to avoid in the future, should more than the client capability bits
need to be known to the callbacks.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
drivers/media/v4l2-core/v4l2-subdev.c | 8 ++++----
include/media/v4l2-subdev.h | 12 ++++++++++--
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 647587c0499a..f437bd0e528f 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -611,7 +611,7 @@ subdev_ioctl_get_state(struct v4l2_subdev *sd, struct v4l2_subdev_fh *subdev_fh,
case VIDIOC_SUBDEV_S_FRAME_INTERVAL: {
struct v4l2_subdev_frame_interval *fi = arg;
- if (!(subdev_fh->client_caps &
+ if (!(subdev_fh->ci.client_caps &
V4L2_SUBDEV_CLIENT_CAP_INTERVAL_USES_WHICH))
fi->which = V4L2_SUBDEV_FORMAT_ACTIVE;
@@ -651,7 +651,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
struct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh);
bool ro_subdev = test_bit(V4L2_FL_SUBDEV_RO_DEVNODE, &vdev->flags);
bool streams_subdev = sd->flags & V4L2_SUBDEV_FL_STREAMS;
- bool client_supports_streams = subdev_fh->client_caps &
+ bool client_supports_streams = subdev_fh->ci.client_caps &
V4L2_SUBDEV_CLIENT_CAP_STREAMS;
int rval;
@@ -1118,7 +1118,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
case VIDIOC_SUBDEV_G_CLIENT_CAP: {
struct v4l2_subdev_client_capability *client_cap = arg;
- client_cap->capabilities = subdev_fh->client_caps;
+ client_cap->capabilities = subdev_fh->ci.client_caps;
return 0;
}
@@ -1138,7 +1138,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
client_cap->capabilities &= (V4L2_SUBDEV_CLIENT_CAP_STREAMS |
V4L2_SUBDEV_CLIENT_CAP_INTERVAL_USES_WHICH);
- subdev_fh->client_caps = client_cap->capabilities;
+ subdev_fh->ci.client_caps = client_cap->capabilities;
return 0;
}
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 4588992b4417..6106e4409575 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -734,6 +734,14 @@ struct v4l2_subdev_state {
struct v4l2_subdev_stream_configs stream_configs;
};
+/**
+ * struct v4l2_subdev_client_info - Sub-device client information
+ * @client_caps: bitmask of ``V4L2_SUBDEV_CLIENT_CAP_*``
+ */
+struct v4l2_subdev_client_info {
+ u64 client_caps;
+};
+
/**
* struct v4l2_subdev_pad_ops - v4l2-subdev pad level operations
*
@@ -1129,14 +1137,14 @@ struct v4l2_subdev {
* @vfh: pointer to &struct v4l2_fh
* @state: pointer to &struct v4l2_subdev_state
* @owner: module pointer to the owner of this file handle
- * @client_caps: bitmask of ``V4L2_SUBDEV_CLIENT_CAP_*``
+ * @ci: sub-device client info related to this file handle
*/
struct v4l2_subdev_fh {
struct v4l2_fh vfh;
struct module *owner;
#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
struct v4l2_subdev_state *state;
- u64 client_caps;
+ struct v4l2_subdev_client_info ci;
#endif
};
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 19/22] media: v4l2-subdev: Add struct v4l2_subdev_client_info pointer to pad ops
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (18 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 18/22] media: v4l2-subdev: Move subdev client capabilities into a new struct Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 20/22] media: v4l2-subdev: Add v4l2_subdev_call_ci_active_state Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 21/22] media: v4l2-subdev: Perform client info changes to i2c drivers Sakari Ailus
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Add a pointer to const struct v4l2_subdev_client_info to the get_fmt,
set_fmt, get_selection and set_selection sub-device pad ops. The client
info struct will soon be used to differentiate UAPI based on client
capabilities.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/v4l2-core/v4l2-subdev.c | 72 +++++++++++++++++++--------
include/media/v4l2-subdev.h | 9 +++-
2 files changed, 60 insertions(+), 21 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index f437bd0e528f..04a5cb2ad3e3 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -245,19 +245,21 @@ static inline int check_format(struct v4l2_subdev *sd,
}
static int call_get_fmt(struct v4l2_subdev *sd,
+ const struct v4l2_subdev_client_info *ci,
struct v4l2_subdev_state *state,
struct v4l2_subdev_format *format)
{
return check_format(sd, state, format) ? :
- sd->ops->pad->get_fmt(sd, state, format);
+ sd->ops->pad->get_fmt(sd, ci, state, format);
}
static int call_set_fmt(struct v4l2_subdev *sd,
+ const struct v4l2_subdev_client_info *ci,
struct v4l2_subdev_state *state,
struct v4l2_subdev_format *format)
{
return check_format(sd, state, format) ? :
- sd->ops->pad->set_fmt(sd, state, format);
+ sd->ops->pad->set_fmt(sd, ci, state, format);
}
static int call_enum_mbus_code(struct v4l2_subdev *sd,
@@ -308,19 +310,21 @@ static inline int check_selection(struct v4l2_subdev *sd,
}
static int call_get_selection(struct v4l2_subdev *sd,
+ const struct v4l2_subdev_client_info *ci,
struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
{
return check_selection(sd, state, sel) ? :
- sd->ops->pad->get_selection(sd, state, sel);
+ sd->ops->pad->get_selection(sd, ci, state, sel);
}
static int call_set_selection(struct v4l2_subdev *sd,
+ const struct v4l2_subdev_client_info *ci,
struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
{
return check_selection(sd, state, sel) ? :
- sd->ops->pad->set_selection(sd, state, sel);
+ sd->ops->pad->set_selection(sd, ci, state, sel);
}
static inline int check_frame_interval(struct v4l2_subdev *sd,
@@ -523,6 +527,21 @@ static int call_s_stream(struct v4l2_subdev *sd, int enable)
v4l2_subdev_unlock_state(state); \
return ret; \
}
+#define DEFINE_CI_STATE_WRAPPER(f, arg_type) \
+ static int call_##f##_state(struct v4l2_subdev *sd, \
+ const struct v4l2_subdev_client_info *ci, \
+ struct v4l2_subdev_state *_state, \
+ arg_type *arg) \
+ { \
+ struct v4l2_subdev_state *state = _state; \
+ int ret; \
+ if (!_state) \
+ state = v4l2_subdev_lock_and_get_active_state(sd); \
+ ret = call_##f(sd, ci, state, arg); \
+ if (!_state && state) \
+ v4l2_subdev_unlock_state(state); \
+ return ret; \
+ }
#else /* CONFIG_MEDIA_CONTROLLER */
@@ -534,15 +553,24 @@ static int call_s_stream(struct v4l2_subdev *sd, int enable)
return call_##f(sd, state, arg); \
}
+#define DEFINE_CI_STATE_WRAPPER(f, arg_type) \
+ static int call_##f##_state(struct v4l2_subdev *sd, \
+ const struct v4l2_subdev_client_info *ci, \
+ struct v4l2_subdev_state *state, \
+ arg_type *arg) \
+ { \
+ return call_##f(sd, ci, state, arg); \
+ }
+
#endif /* CONFIG_MEDIA_CONTROLLER */
-DEFINE_STATE_WRAPPER(get_fmt, struct v4l2_subdev_format);
-DEFINE_STATE_WRAPPER(set_fmt, struct v4l2_subdev_format);
+DEFINE_CI_STATE_WRAPPER(get_fmt, struct v4l2_subdev_format);
+DEFINE_CI_STATE_WRAPPER(set_fmt, struct v4l2_subdev_format);
DEFINE_STATE_WRAPPER(enum_mbus_code, struct v4l2_subdev_mbus_code_enum);
DEFINE_STATE_WRAPPER(enum_frame_size, struct v4l2_subdev_frame_size_enum);
DEFINE_STATE_WRAPPER(enum_frame_interval, struct v4l2_subdev_frame_interval_enum);
-DEFINE_STATE_WRAPPER(get_selection, struct v4l2_subdev_selection);
-DEFINE_STATE_WRAPPER(set_selection, struct v4l2_subdev_selection);
+DEFINE_CI_STATE_WRAPPER(get_selection, struct v4l2_subdev_selection);
+DEFINE_CI_STATE_WRAPPER(set_selection, struct v4l2_subdev_selection);
static const struct v4l2_subdev_pad_ops v4l2_subdev_call_pad_wrappers = {
.get_fmt = call_get_fmt_state,
@@ -804,7 +832,8 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
memset(format->reserved, 0, sizeof(format->reserved));
memset(format->format.reserved, 0, sizeof(format->format.reserved));
- return v4l2_subdev_call(sd, pad, get_fmt, state, format);
+ return v4l2_subdev_call(sd, pad, get_fmt, &subdev_fh->ci, state,
+ format);
}
case VIDIOC_SUBDEV_S_FMT: {
@@ -818,7 +847,8 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
memset(format->reserved, 0, sizeof(format->reserved));
memset(format->format.reserved, 0, sizeof(format->format.reserved));
- return v4l2_subdev_call(sd, pad, set_fmt, state, format);
+ return v4l2_subdev_call(sd, pad, set_fmt, &subdev_fh->ci, state,
+ format);
}
case VIDIOC_SUBDEV_G_CROP: {
@@ -835,8 +865,8 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
sel.stream = crop->stream;
sel.target = V4L2_SEL_TGT_CROP;
- rval = v4l2_subdev_call(
- sd, pad, get_selection, state, &sel);
+ rval = v4l2_subdev_call(sd, pad, get_selection, &subdev_fh->ci,
+ state, &sel);
crop->rect = sel.r;
@@ -861,8 +891,8 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
sel.target = V4L2_SEL_TGT_CROP;
sel.r = crop->rect;
- rval = v4l2_subdev_call(
- sd, pad, set_selection, state, &sel);
+ rval = v4l2_subdev_call(sd, pad, set_selection, &subdev_fh->ci,
+ state, &sel);
crop->rect = sel.r;
@@ -932,8 +962,8 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
sel->stream = 0;
memset(sel->reserved, 0, sizeof(sel->reserved));
- return v4l2_subdev_call(
- sd, pad, get_selection, state, sel);
+ return v4l2_subdev_call(sd, pad, get_selection, &subdev_fh->ci,
+ state, sel);
}
case VIDIOC_SUBDEV_S_SELECTION: {
@@ -946,8 +976,8 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
sel->stream = 0;
memset(sel->reserved, 0, sizeof(sel->reserved));
- return v4l2_subdev_call(
- sd, pad, set_selection, state, sel);
+ return v4l2_subdev_call(sd, pad, set_selection, &subdev_fh->ci,
+ state, sel);
}
case VIDIOC_G_EDID: {
@@ -1342,7 +1372,7 @@ v4l2_subdev_link_validate_get_format(struct media_pad *pad, u32 stream,
else
state = v4l2_subdev_lock_and_get_active_state(sd);
- ret = v4l2_subdev_call(sd, pad, get_fmt, state, fmt);
+ ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, state, fmt);
if (!states_locked && state)
v4l2_subdev_unlock_state(state);
@@ -1923,7 +1953,9 @@ v4l2_subdev_init_stream_configs(struct v4l2_subdev_stream_configs *stream_config
return 0;
}
-int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
+int v4l2_subdev_get_fmt(struct v4l2_subdev *sd,
+ const struct v4l2_subdev_client_info *ci,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_format *format)
{
struct v4l2_mbus_framefmt *fmt;
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 6106e4409575..9706bc22d64d 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -850,15 +850,19 @@ struct v4l2_subdev_pad_ops {
struct v4l2_subdev_state *state,
struct v4l2_subdev_frame_interval_enum *fie);
int (*get_fmt)(struct v4l2_subdev *sd,
+ const struct v4l2_subdev_client_info *ci,
struct v4l2_subdev_state *state,
struct v4l2_subdev_format *format);
int (*set_fmt)(struct v4l2_subdev *sd,
+ const struct v4l2_subdev_client_info *ci,
struct v4l2_subdev_state *state,
struct v4l2_subdev_format *format);
int (*get_selection)(struct v4l2_subdev *sd,
+ const struct v4l2_subdev_client_info *ci,
struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel);
int (*set_selection)(struct v4l2_subdev *sd,
+ const struct v4l2_subdev_client_info *ci,
struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel);
int (*get_frame_interval)(struct v4l2_subdev *sd,
@@ -1461,6 +1465,7 @@ __v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state,
* @sd: subdevice
* @state: subdevice state
* @format: pointer to &struct v4l2_subdev_format
+ * @ci: pointer to sub-device client information, including client capabilities
*
* Fill @format->format field based on the information in the @format struct.
*
@@ -1470,7 +1475,9 @@ __v4l2_subdev_state_get_interval(struct v4l2_subdev_state *state,
*
* Returns 0 on success, error value otherwise.
*/
-int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
+int v4l2_subdev_get_fmt(struct v4l2_subdev *sd,
+ const struct v4l2_subdev_client_info *ci,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_format *format);
/**
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 20/22] media: v4l2-subdev: Add v4l2_subdev_call_ci_active_state
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (19 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 19/22] media: v4l2-subdev: Add struct v4l2_subdev_client_info pointer to pad ops Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
2026-03-25 10:58 ` [PATCH v3 21/22] media: v4l2-subdev: Perform client info changes to i2c drivers Sakari Ailus
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Add v4l2_subdev_call_ci_active_state(), to call sub-device pad ops that
take struct v4l2_subdev_client_info pointer as an argument.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
include/media/v4l2-subdev.h | 49 ++++++++++++++++++++++++++++---------
1 file changed, 38 insertions(+), 11 deletions(-)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 9706bc22d64d..97b487b1507a 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1968,6 +1968,22 @@ extern const struct v4l2_subdev_ops v4l2_subdev_call_wrappers;
__result; \
})
+#define v4l2_subdev_call_drop_fourth(first, second, third, fourth, rest...) \
+ v4l2_subdev_call(first, second, third, ##rest)
+
+#define __v4l2_subdev_call_state_active(call, sd, o, f, args...) \
+ ({ \
+ int __result; \
+ struct v4l2_subdev_state *state; \
+ state = v4l2_subdev_get_unlocked_active_state(sd); \
+ if (state) \
+ v4l2_subdev_lock_state(state); \
+ __result = call(sd, o, f, NULL, state, ##args); \
+ if (state) \
+ v4l2_subdev_unlock_state(state); \
+ __result; \
+ })
+
/**
* v4l2_subdev_call_state_active - call an operation of a v4l2_subdev which
* takes state as a parameter, passing the
@@ -1986,17 +2002,28 @@ extern const struct v4l2_subdev_ops v4l2_subdev_call_wrappers;
* active state, lock it before calling the op and unlock it after the call.
*/
#define v4l2_subdev_call_state_active(sd, o, f, args...) \
- ({ \
- int __result; \
- struct v4l2_subdev_state *state; \
- state = v4l2_subdev_get_unlocked_active_state(sd); \
- if (state) \
- v4l2_subdev_lock_state(state); \
- __result = v4l2_subdev_call(sd, o, f, state, ##args); \
- if (state) \
- v4l2_subdev_unlock_state(state); \
- __result; \
- })
+ __v4l2_subdev_call_state_active(v4l2_subdev_call_drop_fourth, \
+ sd, o, f, ##args)
+
+/**
+ * v4l2_subdev_call_ci_state_active - call an operation of a v4l2_subdev which
+ * takes state as a parameter, passing the
+ * subdev its active state.
+ *
+ * @sd: pointer to the &struct v4l2_subdev
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ * Each element there groups a set of callbacks functions.
+ * @f: callback function to be called.
+ * The callback functions are defined in groups, according to
+ * each element at &struct v4l2_subdev_ops.
+ * @args: arguments for @f.
+ *
+ * This macro is just as v4l2_subdev_call_state_active(), with the exception
+ * that it passes NULL as the client info to sub-device ops that need it
+ * (currently pad ops get_fmt, set_fmt, get_selection and set_selection).
+ */
+#define v4l2_subdev_call_ci_state_active(sd, o, f, args...) \
+ __v4l2_subdev_call_state_active(v4l2_subdev_call, sd, o, f, ##args)
/**
* v4l2_subdev_call_state_try - call an operation of a v4l2_subdev which
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 21/22] media: v4l2-subdev: Perform client info changes to i2c drivers
2026-03-25 10:57 [PATCH v3 00/22] Metadata series preparation Sakari Ailus
` (20 preceding siblings ...)
2026-03-25 10:58 ` [PATCH v3 20/22] media: v4l2-subdev: Add v4l2_subdev_call_ci_active_state Sakari Ailus
@ 2026-03-25 10:58 ` Sakari Ailus
21 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2026-03-25 10:58 UTC (permalink / raw)
To: linux-media
Cc: hans, laurent.pinchart, Prabhakar, Kate Hsuan, Dave Stevenson,
Tommaso Merciai, Benjamin Mugnier, Sylvain Petinot,
Christophe JAILLET, Julien Massot, Naushir Patuck, Yan, Dongcheng,
Cao, Bingbu, Qiu, Tian Shu, Stefan Klug, Mirela Rabulea,
André Apitzsch, Heimir Thor Sverrisson, Kieran Bingham,
Mehdi Djait, Ricardo Ribalda Delgado, Hans de Goede, Jacopo Mondi,
Tomi Valkeinen, David Plowman, Yu, Ong Hock, Ng, Khai Wen,
Jai Luthra
Perform client info argument related changes to two i2c drivers (s5k5baf
and tc358743). These changes are not done by Coccinelle scripts in the
following patch and will be squashed to the previous patch eventually.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/i2c/og01a1b.c | 2 +-
drivers/media/i2c/s5k5baf.c | 1 +
drivers/media/i2c/tc358743.c | 2 +-
drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h | 1 +
4 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/og01a1b.c b/drivers/media/i2c/og01a1b.c
index 1675f0460969..1d109ca75d76 100644
--- a/drivers/media/i2c/og01a1b.c
+++ b/drivers/media/i2c/og01a1b.c
@@ -763,7 +763,7 @@ static int og01a1b_init_state(struct v4l2_subdev *sd,
},
};
- og01a1b_set_format(sd, state, &fmt);
+ og01a1b_set_format(sd, NULL, state, &fmt);
return 0;
}
diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
index d1d00eca8708..a580b7e63302 100644
--- a/drivers/media/i2c/s5k5baf.c
+++ b/drivers/media/i2c/s5k5baf.c
@@ -1463,6 +1463,7 @@ static bool s5k5baf_cmp_rect(const struct v4l2_rect *r1,
}
static int s5k5baf_set_selection(struct v4l2_subdev *sd,
+ const struct v4l2_subdev_client_info *ci,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_selection *sel)
{
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index a0ca19359c43..59f509aa1939 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1822,7 +1822,7 @@ static int tc358743_set_fmt(struct v4l2_subdev *sd,
struct tc358743_state *state = to_state(sd);
u32 code = format->format.code; /* is overwritten by get_fmt */
- int ret = tc358743_get_fmt(sd, sd_state, format);
+ int ret = tc358743_get_fmt(sd, ci, sd_state, format);
if (code == MEDIA_BUS_FMT_RGB888_1X24 ||
code == MEDIA_BUS_FMT_UYVY8_1X16)
diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h
index 35069099c364..d4f76d513dc6 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h
@@ -31,6 +31,7 @@ bool ipu6_isys_is_bayer_format(u32 code);
u32 ipu6_isys_convert_bayer_order(u32 code, int x, int y);
int ipu6_isys_subdev_set_fmt(struct v4l2_subdev *sd,
+ const struct v4l2_subdev_client_info *ci,
struct v4l2_subdev_state *state,
struct v4l2_subdev_format *fmt);
int ipu6_isys_subdev_enum_mbus_code(struct v4l2_subdev *sd,
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread