[PATCH v2 0/3] media: vimc: Add timing controls for fps config

[PATCH v2 0/3] media: vimc: Add timing controls for fps config

[Faizel K B]

Control frame timing of vimc sensor by adding support for V4L2 timing
controls (VBLANK, HBLANK, PIXEL_RATE). PIXEL_RATE and HBLANK are fixed as
readonly with default values. VBLANK is configurable.

First version patch attempted to implement direct frame rate control as
fraction. vimc sensor should have controls like real sensor to control the
framerate.

- Link to v1: https://lore.kernel.org/linux-media/cr4tjjlq53ezux7lzrusy4wb7ji26trwkipfkjl33kwv6z4ut7@r6hvuzki66bj/

Changes in v2:
- Changed the patch subject to indicate timing control.
- Removed 'v4l2_fract' element from sensor struct
- Removed frameinterval functions for vimc_sensor_pad_ops
- Added PIXEL_RATE,HBLANK,VBLANK V4L2 Controls
- Added fps delay calculation based on timing controls
- Default PIXEL_RATE as 160 MHz. HBLANK as 800
- Default fps 30 for resolution up to 1920 * 1080 (2,073,600 pixels)
  above which the fps is 10 (PIXEL_RATE must be changed to support
  higher resolutions).

Example usage:
  # Set vblank as 1000
  v4l2-ctl -d /dev/v4l-subdev0 --set-ctrl=vertical_blanking=1000

  # Capture and verify frame rate.
  # Real framerate depends on the CPU performance

  yavta /dev/video2 --capture=100

Faizel K B (3):
  media: vimc: sensor: Move vimc_sensor_device to common header
  media: vimc: sensor: Add pixel_rate,vblank and hblank configuration
  media: vimc: streamer: Apply sensor frame rate in streamer thread

 drivers/media/test-drivers/vimc/vimc-common.h |  41 +++++++
 drivers/media/test-drivers/vimc/vimc-sensor.c | 108 +++++++++++++-----
 .../media/test-drivers/vimc/vimc-streamer.c   |  33 +++++-
 3 files changed, 154 insertions(+), 28 deletions(-)


base-commit: c824345288d11e269ce41b36c105715bc2286050
-- 
2.43.0

[PATCH v2 1/3] media: vimc: sensor: Move vimc_sensor_device to common header

[Faizel K B]

Move the vimc_sensor_device structure from vimc-sensor.c to
vimc-common.h to make it accessible to the vimc-streamer component.

Signed-off-by: Faizel K B <faizel.kb@gmail.com>
---
 drivers/media/test-drivers/vimc/vimc-common.h | 28 +++++++++++++++++++
 drivers/media/test-drivers/vimc/vimc-sensor.c | 26 -----------------
 2 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/media/test-drivers/vimc/vimc-common.h b/drivers/media/test-drivers/vimc/vimc-common.h
index 7a45a2117748..35789add6b4a 100644
--- a/drivers/media/test-drivers/vimc/vimc-common.h
+++ b/drivers/media/test-drivers/vimc/vimc-common.h
@@ -12,6 +12,8 @@
 #include <linux/slab.h>
 #include <media/media-device.h>
 #include <media/v4l2-device.h>
+#include <media/tpg/v4l2-tpg.h>
+#include <media/v4l2-ctrls.h>
 
 #define VIMC_PDEV_NAME "vimc"
 
@@ -159,6 +161,32 @@ struct vimc_ent_config {
 	const struct vimc_ent_type *type;
 };
 
+enum vimc_sensor_osd_mode {
+	VIMC_SENSOR_OSD_SHOW_ALL = 0,
+	VIMC_SENSOR_OSD_SHOW_COUNTERS = 1,
+	VIMC_SENSOR_OSD_SHOW_NONE = 2
+};
+
+struct vimc_sensor_device {
+	struct vimc_ent_device ved;
+	struct v4l2_subdev sd;
+	struct tpg_data tpg;
+	struct v4l2_ctrl_handler hdl;
+	struct media_pad pad;
+
+	u8 *frame;
+
+	/*
+	 * Virtual "hardware" configuration, filled when the stream starts or
+	 * when controls are set.
+	 */
+	struct {
+		struct v4l2_area size;
+		enum vimc_sensor_osd_mode osd_value;
+		u64 start_stream_ts;
+	} hw;
+};
+
 /**
  * vimc_is_source - returns true if the entity has only source pads
  *
diff --git a/drivers/media/test-drivers/vimc/vimc-sensor.c b/drivers/media/test-drivers/vimc/vimc-sensor.c
index 027767777763..2b07dc1f1278 100644
--- a/drivers/media/test-drivers/vimc/vimc-sensor.c
+++ b/drivers/media/test-drivers/vimc/vimc-sensor.c
@@ -14,32 +14,6 @@
 
 #include "vimc-common.h"
 
-enum vimc_sensor_osd_mode {
-	VIMC_SENSOR_OSD_SHOW_ALL = 0,
-	VIMC_SENSOR_OSD_SHOW_COUNTERS = 1,
-	VIMC_SENSOR_OSD_SHOW_NONE = 2
-};
-
-struct vimc_sensor_device {
-	struct vimc_ent_device ved;
-	struct v4l2_subdev sd;
-	struct tpg_data tpg;
-	struct v4l2_ctrl_handler hdl;
-	struct media_pad pad;
-
-	u8 *frame;
-
-	/*
-	 * Virtual "hardware" configuration, filled when the stream starts or
-	 * when controls are set.
-	 */
-	struct {
-		struct v4l2_area size;
-		enum vimc_sensor_osd_mode osd_value;
-		u64 start_stream_ts;
-	} hw;
-};
-
 static const struct v4l2_mbus_framefmt fmt_default = {
 	.width = 640,
 	.height = 480,
-- 
2.43.0

[PATCH v2 2/3] media: vimc: sensor: Add pixel_rate,vblank and hblank configuration

[Faizel K B]

pixel_rate and hblank as read only parameter. vblank can be configured
to match the desired frame rate.

Default values are, pixel_rate - 160 MHz, hblank - 800.
vblank defaults to an equivalent value of 30 fps for resolutions less than
or equal to 1920x1080 and 10 fps for higher resolutions. For higher
resolutions, modify pixel_rate in the driver code.
fps = pixel_rate / ((width + hblank) * (height + vblank))
minimum vblank - 4, maximum vblank - 65535

The configured fps delay is pre-calculated into jiffies and
stored in the sensor's hw structure for efficient access by the
streamer thread.

Signed-off-by: Faizel K B <faizel.kb@gmail.com>
---
 drivers/media/test-drivers/vimc/vimc-common.h | 13 +++
 drivers/media/test-drivers/vimc/vimc-sensor.c | 82 +++++++++++++++++++
 2 files changed, 95 insertions(+)

diff --git a/drivers/media/test-drivers/vimc/vimc-common.h b/drivers/media/test-drivers/vimc/vimc-common.h
index 35789add6b4a..861b334ffc65 100644
--- a/drivers/media/test-drivers/vimc/vimc-common.h
+++ b/drivers/media/test-drivers/vimc/vimc-common.h
@@ -29,6 +29,15 @@
 #define VIMC_FRAME_MIN_WIDTH 16
 #define VIMC_FRAME_MIN_HEIGHT 16
 
+#define VIMC_PIXEL_RATE_FIXED		160000000	/* 160 MHz */
+#define VIMC_HBLANK_FIXED		800
+/* VBLANK - vertical blanking (primary FPS control) */
+#define VIMC_VBLANK_MIN			4
+#define VIMC_VBLANK_MAX			65535
+#define VIMC_VBLANK_STEP		1
+#define VIMC_VBLANK_DEFAULT	        3223           /* 30fps vga */
+#define VIMC_PIXELS_THRESHOLD_30FPS	(1920 * 1080) /* 2073600 pixels */
+
 #define VIMC_FRAME_INDEX(lin, col, width, bpp) ((lin * width + col) * bpp)
 
 /* Source and sink pad checks */
@@ -173,6 +182,9 @@ struct vimc_sensor_device {
 	struct tpg_data tpg;
 	struct v4l2_ctrl_handler hdl;
 	struct media_pad pad;
+	struct v4l2_ctrl *pixel_rate;
+	struct v4l2_ctrl *hblank;
+	struct v4l2_ctrl *vblank;
 
 	u8 *frame;
 
@@ -184,6 +196,7 @@ struct vimc_sensor_device {
 		struct v4l2_area size;
 		enum vimc_sensor_osd_mode osd_value;
 		u64 start_stream_ts;
+		unsigned long fps_jiffies;
 	} hw;
 };
 
diff --git a/drivers/media/test-drivers/vimc/vimc-sensor.c b/drivers/media/test-drivers/vimc/vimc-sensor.c
index 2b07dc1f1278..84cef55b68f8 100644
--- a/drivers/media/test-drivers/vimc/vimc-sensor.c
+++ b/drivers/media/test-drivers/vimc/vimc-sensor.c
@@ -25,10 +25,15 @@ static const struct v4l2_mbus_framefmt fmt_default = {
 static int vimc_sensor_init_state(struct v4l2_subdev *sd,
 				  struct v4l2_subdev_state *sd_state)
 {
+	struct vimc_sensor_device *vsensor =
+		container_of(sd, struct vimc_sensor_device, sd);
+
 	struct v4l2_mbus_framefmt *mf;
 
 	mf = v4l2_subdev_state_get_format(sd_state, 0);
 	*mf = fmt_default;
+	vsensor->hw.size.width = fmt_default.width;
+	vsensor->hw.size.height = fmt_default.height;
 
 	return 0;
 }
@@ -87,6 +92,26 @@ static void vimc_sensor_tpg_s_format(struct vimc_sensor_device *vsensor,
 	tpg_s_xfer_func(&vsensor->tpg, format->xfer_func);
 }
 
+static int vimc_sensor_update_frame_timing(struct v4l2_subdev *sd,
+					   u32 width, u32 height)
+{
+	struct vimc_sensor_device *vsensor =
+		container_of(sd, struct vimc_sensor_device, sd);
+	u64 pixel_rate = vsensor->pixel_rate->val;
+	u32 hts = width + vsensor->hblank->val;
+	u32 vts = height + vsensor->vblank->val;
+	u64 total_pixels = (u64)hts * vts;
+	u64 frame_interval_ns;
+
+	frame_interval_ns = total_pixels * NSEC_PER_SEC;
+	do_div(frame_interval_ns, pixel_rate);
+	vsensor->hw.fps_jiffies = nsecs_to_jiffies(frame_interval_ns);
+	if (vsensor->hw.fps_jiffies == 0)
+		vsensor->hw.fps_jiffies = 1;
+
+	return 0;
+}
+
 static void vimc_sensor_adjust_fmt(struct v4l2_mbus_framefmt *fmt)
 {
 	const struct vimc_pix_map *vpix;
@@ -108,6 +133,18 @@ static void vimc_sensor_adjust_fmt(struct v4l2_mbus_framefmt *fmt)
 	vimc_colorimetry_clamp(fmt);
 }
 
+static u32 vimc_calc_vblank(u32 width, u32 height,
+			    s64 pixel_rate, s32 hblank)
+{
+	u32 hts = width + hblank;
+	u32 target_fps;
+	u32 vblank_def;
+
+	target_fps = (width * height <= VIMC_PIXELS_THRESHOLD_30FPS) ? 30 : 10;
+	vblank_def = (pixel_rate / (target_fps * hts)) - height;
+	return clamp(vblank_def, VIMC_VBLANK_MIN, VIMC_VBLANK_MAX);
+}
+
 static int vimc_sensor_set_fmt(struct v4l2_subdev *sd,
 			       struct v4l2_subdev_state *sd_state,
 			       struct v4l2_subdev_format *fmt)
@@ -137,6 +174,20 @@ static int vimc_sensor_set_fmt(struct v4l2_subdev *sd,
 		fmt->format.xfer_func, fmt->format.ycbcr_enc);
 
 	*mf = fmt->format;
+	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
+		u32 vblank_def = vimc_calc_vblank(fmt->format.width,
+						  fmt->format.height,
+						  vsensor->pixel_rate->val,
+						  vsensor->hblank->val);
+		vsensor->hw.size.width = fmt->format.width;
+		vsensor->hw.size.height = fmt->format.height;
+		__v4l2_ctrl_modify_range(vsensor->vblank,
+					 VIMC_VBLANK_MIN,
+					 VIMC_VBLANK_MAX,
+					 VIMC_VBLANK_STEP,
+					 vblank_def);
+		__v4l2_ctrl_s_ctrl(vsensor->vblank, vblank_def);
+	}
 
 	return 0;
 }
@@ -222,6 +273,8 @@ static int vimc_sensor_s_stream(struct v4l2_subdev *sd, int enable)
 
 		vsensor->hw.size.width = format->width;
 		vsensor->hw.size.height = format->height;
+		vimc_sensor_update_frame_timing(sd, format->width,
+						format->height);
 
 		v4l2_subdev_unlock_state(state);
 
@@ -293,6 +346,15 @@ static int vimc_sensor_s_ctrl(struct v4l2_ctrl *ctrl)
 	case VIMC_CID_OSD_TEXT_MODE:
 		vsensor->hw.osd_value = ctrl->val;
 		break;
+	case V4L2_CID_PIXEL_RATE:
+		break;
+	case V4L2_CID_HBLANK:
+		break;
+	case V4L2_CID_VBLANK:
+		vimc_sensor_update_frame_timing(&vsensor->sd,
+						vsensor->hw.size.width,
+						vsensor->hw.size.height);
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -377,6 +439,26 @@ static struct vimc_ent_device *vimc_sensor_add(struct vimc_device *vimc,
 			  V4L2_CID_HUE, -128, 127, 1, 0);
 	v4l2_ctrl_new_std(&vsensor->hdl, &vimc_sensor_ctrl_ops,
 			  V4L2_CID_SATURATION, 0, 255, 1, 128);
+	/* Timing controls for frame interval configuration */
+	vsensor->pixel_rate = v4l2_ctrl_new_std(&vsensor->hdl, &vimc_sensor_ctrl_ops,
+						V4L2_CID_PIXEL_RATE,
+						VIMC_PIXEL_RATE_FIXED, VIMC_PIXEL_RATE_FIXED,
+						1, VIMC_PIXEL_RATE_FIXED);
+	if (vsensor->pixel_rate)
+		vsensor->pixel_rate->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+	vsensor->hblank = v4l2_ctrl_new_std(&vsensor->hdl, &vimc_sensor_ctrl_ops,
+					    V4L2_CID_HBLANK,
+					    VIMC_HBLANK_FIXED, VIMC_HBLANK_FIXED,
+					    1, VIMC_HBLANK_FIXED);
+	if (vsensor->hblank)
+		vsensor->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+	vsensor->vblank = v4l2_ctrl_new_std(&vsensor->hdl, &vimc_sensor_ctrl_ops,
+					    V4L2_CID_VBLANK,
+					    VIMC_VBLANK_MIN, VIMC_VBLANK_MAX,
+					    VIMC_VBLANK_STEP, VIMC_VBLANK_DEFAULT);
+
 	vsensor->sd.ctrl_handler = &vsensor->hdl;
 	if (vsensor->hdl.error) {
 		ret = vsensor->hdl.error;
-- 
2.43.0

[PATCH v2 3/3] media: vimc: streamer: Apply sensor frame rate in streamer thread

[Faizel K B]

Use the sensor's pre-calculated jiffies value to add appropriate
delay between frames according to the configured timing value.
The actual frame rate  will vary depending on processing delays in
other media pipeline components.

Tested using yavta frame rate display with QCIF resolution:
yavta <video-node> --capture=<no_of_frames>

Signed-off-by: Faizel K B <faizel.kb@gmail.com>
---
 .../media/test-drivers/vimc/vimc-streamer.c   | 33 +++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/media/test-drivers/vimc/vimc-streamer.c b/drivers/media/test-drivers/vimc/vimc-streamer.c
index 15d863f97cbf..3ebf5798fa3d 100644
--- a/drivers/media/test-drivers/vimc/vimc-streamer.c
+++ b/drivers/media/test-drivers/vimc/vimc-streamer.c
@@ -139,6 +139,29 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
 	return -EINVAL;
 }
 
+/**
+ * vimc_streamer_get_sensor() - Get sensor from pipeline
+ * @stream: the pipeline
+ *
+ * Helper function to find the sensor device in the pipeline.
+ * Returns pointer to sensor device or NULL if not found.
+ */
+static struct vimc_sensor_device *vimc_streamer_get_sensor(struct vimc_stream *stream)
+{
+	int i;
+
+	for (i = 0; i < stream->pipe_size; i++) {
+		struct vimc_ent_device *ved = stream->ved_pipeline[i];
+
+		if (ved && ved->ent &&
+		    ved->ent->function == MEDIA_ENT_F_CAM_SENSOR) {
+			return container_of(ved, struct vimc_sensor_device, ved);
+		}
+	}
+
+	return NULL;
+}
+
 /**
  * vimc_streamer_thread - Process frames through the pipeline
  *
@@ -154,25 +177,31 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
 static int vimc_streamer_thread(void *data)
 {
 	struct vimc_stream *stream = data;
+	struct vimc_sensor_device *vsensor;
 	u8 *frame = NULL;
 	int i;
+	unsigned long fps_jiffies;
+	const unsigned long default_jiffies = HZ / 30;
 
 	set_freezable();
+	vsensor = vimc_streamer_get_sensor(stream);
 
 	for (;;) {
 		try_to_freeze();
 		if (kthread_should_stop())
 			break;
 
+		/* Read from hardware configuration */
+		fps_jiffies = vsensor ? vsensor->hw.fps_jiffies : default_jiffies;
+
 		for (i = stream->pipe_size - 1; i >= 0; i--) {
 			frame = stream->ved_pipeline[i]->process_frame(
 					stream->ved_pipeline[i], frame);
 			if (!frame || IS_ERR(frame))
 				break;
 		}
-		//wait for 60hz
 		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(HZ / 60);
+		schedule_timeout(fps_jiffies);
 	}
 
 	return 0;
-- 
2.43.0

Re: [PATCH v2 2/3] media: vimc: sensor: Add pixel_rate,vblank and hblank configuration

[kernel test robot]

Hi Faizel,

kernel test robot noticed the following build errors:

[auto build test ERROR on c824345288d11e269ce41b36c105715bc2286050]

url:    https://github.com/intel-lab-lkp/linux/commits/Faizel-K-B/media-vimc-sensor-Move-vimc_sensor_device-to-common-header/20260203-035653
base:   c824345288d11e269ce41b36c105715bc2286050
patch link:    https://lore.kernel.org/r/20260202194645.1287757-3-faizel.kb%40gmail.com
patch subject: [PATCH v2 2/3] media: vimc: sensor: Add pixel_rate,vblank and hblank configuration
config: powerpc-randconfig-002-20260203 (https://download.01.org/0day-ci/archive/20260203/202602031319.6Nd13RwV-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 10.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260203/202602031319.6Nd13RwV-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602031319.6Nd13RwV-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "__divdi3" [drivers/media/test-drivers/vimc/vimc.ko] undefined!

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for HOTPLUG_CPU
   Depends on [n]: SMP [=y] && (PPC_PSERIES [=n] || PPC_PMAC [=n] || PPC_POWERNV [=n] || FSL_SOC_BOOKE [=n])
   Selected by [y]:
   - PM_SLEEP_SMP [=y] && SMP [=y] && (ARCH_SUSPEND_POSSIBLE [=n] || ARCH_HIBERNATION_POSSIBLE [=y]) && PM_SLEEP [=y]
   WARNING: unmet direct dependencies detected for NET_SELFTESTS
   Depends on [n]: NET [=y] && PHYLIB [=y] && INET [=n]
   Selected by [y]:
   - AMD_XGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_AMD [=y] && (OF_ADDRESS [=y] || ACPI || PCI [=n]) && HAS_IOMEM [=y] && (X86 || ARM64 || COMPILE_TEST [=y]) && PTP_1588_CLOCK_OPTIONAL [=y]

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 2/3] media: vimc: sensor: Add pixel_rate,vblank and hblank configuration

[kernel test robot]

Hi Faizel,

kernel test robot noticed the following build errors:

[auto build test ERROR on c824345288d11e269ce41b36c105715bc2286050]

url:    https://github.com/intel-lab-lkp/linux/commits/Faizel-K-B/media-vimc-sensor-Move-vimc_sensor_device-to-common-header/20260203-035653
base:   c824345288d11e269ce41b36c105715bc2286050
patch link:    https://lore.kernel.org/r/20260202194645.1287757-3-faizel.kb%40gmail.com
patch subject: [PATCH v2 2/3] media: vimc: sensor: Add pixel_rate,vblank and hblank configuration
config: m68k-randconfig-r054-20260203 (https://download.01.org/0day-ci/archive/20260203/202602031452.JzAaJ8BU-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260203/202602031452.JzAaJ8BU-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602031452.JzAaJ8BU-lkp@intel.com/

All errors (new ones prefixed by >>):

   m68k-linux-ld: drivers/net/phy/air_en8811h.o: in function `en8811h_resume':
   drivers/net/phy/air_en8811h.c:1175:(.text+0x722): undefined reference to `clk_restore_context'
   m68k-linux-ld: drivers/net/phy/air_en8811h.o: in function `en8811h_suspend':
   drivers/net/phy/air_en8811h.c:1182:(.text+0x73a): undefined reference to `clk_save_context'
   m68k-linux-ld: drivers/media/test-drivers/vimc/vimc-sensor.o: in function `vimc_calc_vblank':
>> drivers/media/test-drivers/vimc/vimc-sensor.c:144:(.text+0x376): undefined reference to `__divdi3'


vim +144 drivers/media/test-drivers/vimc/vimc-sensor.c

   135	
   136	static u32 vimc_calc_vblank(u32 width, u32 height,
   137				    s64 pixel_rate, s32 hblank)
   138	{
   139		u32 hts = width + hblank;
   140		u32 target_fps;
   141		u32 vblank_def;
   142	
   143		target_fps = (width * height <= VIMC_PIXELS_THRESHOLD_30FPS) ? 30 : 10;
 > 144		vblank_def = (pixel_rate / (target_fps * hts)) - height;
   145		return clamp(vblank_def, VIMC_VBLANK_MIN, VIMC_VBLANK_MAX);
   146	}
   147	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki