Add V4L2_CID_FLASH_DURATION support using the "strobe_frame_span"
feature of the sensor. This is implemented by transforming the given µs
value by an interpolated formula to a "span step width" value and
writing it to register PWM_CTRL_25, PWM_CTRL_26, PWM_CTRL_27,
PWM_CTRL_28 (0x3925, 0x3926, 0x3927, 0x3928).
The maximum control value is set to the period of the current default
framerate.
All register values are based on the OV9281 datasheet v1.53 (jan 2019)
and tested using an ov9281 VisionComponents module.
Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
---
drivers/media/i2c/ov9282.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index 35edbe1e0efb61a0e03da0ed817c4c4ec0ae9c35..5ddbfc51586111fbd2e17b739fb3d28bfb0aee1e 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -97,6 +97,10 @@
#define OV9282_REG_MIPI_CTRL00 0x4800
#define OV9282_GATED_CLOCK BIT(5)
+/* Flash/Strobe control registers */
+#define OV9282_REG_FLASH_DURATION 0x3925
+#define OV9282_FLASH_DURATION_DEFAULT 0x0000001A
+
/* Input clock rate */
#define OV9282_INCLK_RATE 24000000
@@ -687,6 +691,24 @@ static int ov9282_set_ctrl_flash_led_mode(struct ov9282 *ov9282, int mode)
current_val);
}
+static int ov9282_set_ctrl_flash_duration(struct ov9282 *ov9282, int value)
+{
+ /* Calculate "strobe_frame_span" increments from a given value (µs).
+ * This is quite tricky as "The step width of shift and span is
+ * programmable under system clock domain.", but it's not documented
+ * how to program this step width (at least in the datasheet available
+ * to the author at time of writing).
+ * The formula below is interpolated from different modes/framerates
+ * and should work quite well for most settings.
+ */
+ u32 val = value * 192 / (ov9282->cur_mode->width + ov9282->hblank_ctrl->val);
+
+ ov9282_write_reg(ov9282, OV9282_REG_FLASH_DURATION, 1, (val >> 24) & 0xff);
+ ov9282_write_reg(ov9282, OV9282_REG_FLASH_DURATION + 1, 1, (val >> 16) & 0xff);
+ ov9282_write_reg(ov9282, OV9282_REG_FLASH_DURATION + 2, 1, (val >> 8) & 0xff);
+ return ov9282_write_reg(ov9282, OV9282_REG_FLASH_DURATION + 3, 1, val & 0xff);
+}
+
/**
* ov9282_set_ctrl() - Set subdevice control
* @ctrl: pointer to v4l2_ctrl structure
@@ -756,6 +778,9 @@ static int ov9282_set_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_FLASH_LED_MODE:
ret = ov9282_set_ctrl_flash_led_mode(ov9282, ctrl->val);
break;
+ case V4L2_CID_FLASH_DURATION:
+ ret = ov9282_set_ctrl_flash_duration(ov9282, ctrl->val);
+ break;
default:
dev_err(ov9282->dev, "Invalid control %d", ctrl->id);
ret = -EINVAL;
@@ -1418,6 +1443,9 @@ static int ov9282_init_controls(struct ov9282 *ov9282)
(1 << V4L2_FLASH_LED_MODE_TORCH),
V4L2_FLASH_LED_MODE_NONE);
+ v4l2_ctrl_new_std(ctrl_hdlr, &ov9282_ctrl_ops, V4L2_CID_FLASH_DURATION,
+ 0, 13900, 1, 8);
+
ret = v4l2_fwnode_device_parse(ov9282->dev, &props);
if (!ret) {
/* Failure sets ctrl_hdlr->error, which we check afterwards anyway */
--
2.47.2
Hi Richard,
On Tue, Apr 29, 2025 at 02:59:13PM +0200, Richard Leitner wrote:
> Add V4L2_CID_FLASH_DURATION support using the "strobe_frame_span"
> feature of the sensor. This is implemented by transforming the given µs
> value by an interpolated formula to a "span step width" value and
> writing it to register PWM_CTRL_25, PWM_CTRL_26, PWM_CTRL_27,
> PWM_CTRL_28 (0x3925, 0x3926, 0x3927, 0x3928).
>
> The maximum control value is set to the period of the current default
> framerate.
>
> All register values are based on the OV9281 datasheet v1.53 (jan 2019)
> and tested using an ov9281 VisionComponents module.
>
> Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> ---
> drivers/media/i2c/ov9282.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> index 35edbe1e0efb61a0e03da0ed817c4c4ec0ae9c35..5ddbfc51586111fbd2e17b739fb3d28bfb0aee1e 100644
> --- a/drivers/media/i2c/ov9282.c
> +++ b/drivers/media/i2c/ov9282.c
> @@ -97,6 +97,10 @@
> #define OV9282_REG_MIPI_CTRL00 0x4800
> #define OV9282_GATED_CLOCK BIT(5)
>
> +/* Flash/Strobe control registers */
> +#define OV9282_REG_FLASH_DURATION 0x3925
> +#define OV9282_FLASH_DURATION_DEFAULT 0x0000001A
> +
> /* Input clock rate */
> #define OV9282_INCLK_RATE 24000000
>
> @@ -687,6 +691,24 @@ static int ov9282_set_ctrl_flash_led_mode(struct ov9282 *ov9282, int mode)
> current_val);
> }
>
> +static int ov9282_set_ctrl_flash_duration(struct ov9282 *ov9282, int value)
> +{
> + /* Calculate "strobe_frame_span" increments from a given value (µs).
/*
* Multi-line
* comment.
*/
> + * This is quite tricky as "The step width of shift and span is
> + * programmable under system clock domain.", but it's not documented
> + * how to program this step width (at least in the datasheet available
> + * to the author at time of writing).
> + * The formula below is interpolated from different modes/framerates
> + * and should work quite well for most settings.
> + */
> + u32 val = value * 192 / (ov9282->cur_mode->width + ov9282->hblank_ctrl->val);
> +
> + ov9282_write_reg(ov9282, OV9282_REG_FLASH_DURATION, 1, (val >> 24) & 0xff);
> + ov9282_write_reg(ov9282, OV9282_REG_FLASH_DURATION + 1, 1, (val >> 16) & 0xff);
> + ov9282_write_reg(ov9282, OV9282_REG_FLASH_DURATION + 2, 1, (val >> 8) & 0xff);
> + return ov9282_write_reg(ov9282, OV9282_REG_FLASH_DURATION + 3, 1, val & 0xff);
The granularity of the hardware supported values is lower than that of the
control. Could you add try_ctrl op to provide the actual value back to the
user space?
> +}
> +
> /**
> * ov9282_set_ctrl() - Set subdevice control
> * @ctrl: pointer to v4l2_ctrl structure
> @@ -756,6 +778,9 @@ static int ov9282_set_ctrl(struct v4l2_ctrl *ctrl)
> case V4L2_CID_FLASH_LED_MODE:
> ret = ov9282_set_ctrl_flash_led_mode(ov9282, ctrl->val);
> break;
> + case V4L2_CID_FLASH_DURATION:
> + ret = ov9282_set_ctrl_flash_duration(ov9282, ctrl->val);
> + break;
> default:
> dev_err(ov9282->dev, "Invalid control %d", ctrl->id);
> ret = -EINVAL;
> @@ -1418,6 +1443,9 @@ static int ov9282_init_controls(struct ov9282 *ov9282)
> (1 << V4L2_FLASH_LED_MODE_TORCH),
> V4L2_FLASH_LED_MODE_NONE);
>
> + v4l2_ctrl_new_std(ctrl_hdlr, &ov9282_ctrl_ops, V4L2_CID_FLASH_DURATION,
Should the number of controls in the handler be updated?
> + 0, 13900, 1, 8);
> +
> ret = v4l2_fwnode_device_parse(ov9282->dev, &props);
> if (!ret) {
> /* Failure sets ctrl_hdlr->error, which we check afterwards anyway */
>
--
Regards,
Sakari Ailus
Hi Sakari,
thanks for the review!
On Wed, Apr 30, 2025 at 08:27:39AM +0000, Sakari Ailus wrote:
> Hi Richard,
>
> On Tue, Apr 29, 2025 at 02:59:13PM +0200, Richard Leitner wrote:
> > Add V4L2_CID_FLASH_DURATION support using the "strobe_frame_span"
> > feature of the sensor. This is implemented by transforming the given µs
> > value by an interpolated formula to a "span step width" value and
> > writing it to register PWM_CTRL_25, PWM_CTRL_26, PWM_CTRL_27,
> > PWM_CTRL_28 (0x3925, 0x3926, 0x3927, 0x3928).
> >
> > The maximum control value is set to the period of the current default
> > framerate.
> >
> > All register values are based on the OV9281 datasheet v1.53 (jan 2019)
> > and tested using an ov9281 VisionComponents module.
> >
> > Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> > ---
> > drivers/media/i2c/ov9282.c | 28 ++++++++++++++++++++++++++++
> > 1 file changed, 28 insertions(+)
> >
> > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > index 35edbe1e0efb61a0e03da0ed817c4c4ec0ae9c35..5ddbfc51586111fbd2e17b739fb3d28bfb0aee1e 100644
> > --- a/drivers/media/i2c/ov9282.c
> > +++ b/drivers/media/i2c/ov9282.c
> > @@ -97,6 +97,10 @@
> > #define OV9282_REG_MIPI_CTRL00 0x4800
> > #define OV9282_GATED_CLOCK BIT(5)
> >
> > +/* Flash/Strobe control registers */
> > +#define OV9282_REG_FLASH_DURATION 0x3925
> > +#define OV9282_FLASH_DURATION_DEFAULT 0x0000001A
> > +
> > /* Input clock rate */
> > #define OV9282_INCLK_RATE 24000000
> >
> > @@ -687,6 +691,24 @@ static int ov9282_set_ctrl_flash_led_mode(struct ov9282 *ov9282, int mode)
> > current_val);
> > }
> >
> > +static int ov9282_set_ctrl_flash_duration(struct ov9282 *ov9282, int value)
> > +{
> > + /* Calculate "strobe_frame_span" increments from a given value (µs).
>
> /*
> * Multi-line
> * comment.
> */
sure. will adapt that in v4.
>
> > + * This is quite tricky as "The step width of shift and span is
> > + * programmable under system clock domain.", but it's not documented
> > + * how to program this step width (at least in the datasheet available
> > + * to the author at time of writing).
> > + * The formula below is interpolated from different modes/framerates
> > + * and should work quite well for most settings.
> > + */
> > + u32 val = value * 192 / (ov9282->cur_mode->width + ov9282->hblank_ctrl->val);
> > +
> > + ov9282_write_reg(ov9282, OV9282_REG_FLASH_DURATION, 1, (val >> 24) & 0xff);
> > + ov9282_write_reg(ov9282, OV9282_REG_FLASH_DURATION + 1, 1, (val >> 16) & 0xff);
> > + ov9282_write_reg(ov9282, OV9282_REG_FLASH_DURATION + 2, 1, (val >> 8) & 0xff);
> > + return ov9282_write_reg(ov9282, OV9282_REG_FLASH_DURATION + 3, 1, val & 0xff);
>
> The granularity of the hardware supported values is lower than that of the
> control. Could you add try_ctrl op to provide the actual value back to the
> user space?
Tbh, I've never implemented a try_ctrl before, but sure. I will dig into it and
add it for the ov9282 FLASH_DURATION in v4.
>
> > +}
> > +
> > /**
> > * ov9282_set_ctrl() - Set subdevice control
> > * @ctrl: pointer to v4l2_ctrl structure
> > @@ -756,6 +778,9 @@ static int ov9282_set_ctrl(struct v4l2_ctrl *ctrl)
> > case V4L2_CID_FLASH_LED_MODE:
> > ret = ov9282_set_ctrl_flash_led_mode(ov9282, ctrl->val);
> > break;
> > + case V4L2_CID_FLASH_DURATION:
> > + ret = ov9282_set_ctrl_flash_duration(ov9282, ctrl->val);
> > + break;
> > default:
> > dev_err(ov9282->dev, "Invalid control %d", ctrl->id);
> > ret = -EINVAL;
> > @@ -1418,6 +1443,9 @@ static int ov9282_init_controls(struct ov9282 *ov9282)
> > (1 << V4L2_FLASH_LED_MODE_TORCH),
> > V4L2_FLASH_LED_MODE_NONE);
> >
> > + v4l2_ctrl_new_std(ctrl_hdlr, &ov9282_ctrl_ops, V4L2_CID_FLASH_DURATION,
>
> Should the number of controls in the handler be updated?
What do you mean by "number of controls in the handler" exactly? Which
handler?
> > + 0, 13900, 1, 8);
> > +
> > ret = v4l2_fwnode_device_parse(ov9282->dev, &props);
> > if (!ret) {
> > /* Failure sets ctrl_hdlr->error, which we check afterwards anyway */
> >
>
> --
> Regards,
>
> Sakari Ailus
regards;rl
Hi Richard, On Wed, Apr 30, 2025 at 11:09:30AM +0200, Richard Leitner wrote: > Hi Sakari, > > thanks for the review! You're welcome. > > > + v4l2_ctrl_new_std(ctrl_hdlr, &ov9282_ctrl_ops, V4L2_CID_FLASH_DURATION, > > > > Should the number of controls in the handler be updated? > > What do you mean by "number of controls in the handler" exactly? Which > handler? A number of buckets allocated in control handler's initialisation is based on the number of controls given to the init function. You should update that number when adding controls. -- Regards, Sakari Ailus
On Thu, May 01, 2025 at 10:22:38PM +0000, Sakari Ailus wrote: > Hi Richard, > > On Wed, Apr 30, 2025 at 11:09:30AM +0200, Richard Leitner wrote: > > Hi Sakari, > > > > thanks for the review! > > You're welcome. > > > > > + v4l2_ctrl_new_std(ctrl_hdlr, &ov9282_ctrl_ops, V4L2_CID_FLASH_DURATION, > > > > > > Should the number of controls in the handler be updated? > > > > What do you mean by "number of controls in the handler" exactly? Which > > handler? > > A number of buckets allocated in control handler's initialisation is based > on the number of controls given to the init function. You should update > that number when adding controls. Ah, now I got it. :-) Thanks for the clarification. Will fix that in v4 for all newly added controls! regards;rl > > -- > Regards, > > Sakari Ailus
© 2016 - 2026 Red Hat, Inc.