[PATCH v2 3/8] media: i2c: ov9282: fix flash duration to/from microseconds conversion

[Richard Leitner <richard.leitner@linux.dev>]

Currently the flash duration is converted to/from microseconds using a
fixed OV9282_STROBE_SPAN_FACTOR constant. This is inaccurate as it was
found that the "step width of shift and span" (which is not documented
further in the datasheet) scales with the line, so the span is counted
in lines. This was verified by comparing the stobe pulses measured on
the camera module against the calculated line times.

As this is the same conversion as for the exposure: rename those functions
to a more generic ov9282_us_to_lines/ov9282_lines_to_us and use them also
for the flash/strobe duration.

Fixes: b95d8058a2786 ("media: i2c: ov9282: implement try_ctrl for strobe_duration")
Cc: stable@vger.kernel.org
Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
---
 drivers/media/i2c/ov9282.c | 65 +++++++++++++++++-----------------------------
 1 file changed, 24 insertions(+), 41 deletions(-)

diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index 5907f57b34523..e2a1f875160b9 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -136,8 +136,6 @@
 #define OV9282_REG_MIN		0x00
 #define OV9282_REG_MAX		0xfffff
 
-#define OV9282_STROBE_SPAN_FACTOR	192
-
 static const char * const ov9282_supply_names[] = {
 	"avdd",		/* Analog power */
 	"dovdd",	/* Digital I/O power */
@@ -491,18 +489,31 @@ static u32 ov9282_line_time_ns(struct ov9282 *ov9282)
 }
 
 /**
- * ov9282_exposure_to_us() - Convert an exposure register value to microseconds
+ * ov9282_lines_to_us() - Convert a number of lines to microseconds
  * @ov9282: pointer to ov9282 device
- * @exposure: exposure register value to convert
+ * @lines: line count to convert
  *
- * Return: microsecond representation of the given exposure register value.
+ * Return: microsecond representation of the given line count.
  */
-static u32 ov9282_exposure_to_us(struct ov9282 *ov9282, u32 exposure)
+static u32 ov9282_lines_to_us(struct ov9282 *ov9282, u32 lines)
 {
-	return div_u64((u64)exposure * ov9282_line_time_ns(ov9282),
+	return div_u64((u64)lines * ov9282_line_time_ns(ov9282),
 		       NSEC_PER_USEC);
 }
 
+/**
+ * ov9282_us_to_lines() - Convert µs to line count
+ * @ov9282: pointer to ov9282 device
+ * @duration: microseconds value to convert
+ *
+ * Return: number of lines fitting into the given duration
+ */
+static u32 ov9282_us_to_lines(struct ov9282 *ov9282, u32 duration)
+{
+	return DIV_ROUND_UP((u64)duration * NSEC_PER_USEC,
+			    ov9282_line_time_ns(ov9282));
+}
+
 /**
  * ov9282_update_controls() - Update control ranges based on streaming mode
  * @ov9282: pointer to ov9282 device
@@ -551,7 +562,7 @@ static int ov9282_update_controls(struct ov9282 *ov9282,
  */
 static int ov9282_update_exp_gain(struct ov9282 *ov9282, u32 exposure, u32 gain)
 {
-	u32 exposure_us = ov9282_exposure_to_us(ov9282, exposure);
+	u32 exposure_us = ov9282_lines_to_us(ov9282, exposure);
 	int ret, ret_hold;
 
 	dev_dbg(ov9282->dev, "Set exp %u (~%u us), analog gain %u",
@@ -579,34 +590,6 @@ static int ov9282_update_exp_gain(struct ov9282 *ov9282, u32 exposure, u32 gain)
 	return ret ? ret : ret_hold;
 }
 
-static u32 ov9282_us_to_flash_duration(struct ov9282 *ov9282, u32 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 frame_width = ov9282->cur_mode->width + ov9282->hblank_ctrl->val;
-
-	return value * OV9282_STROBE_SPAN_FACTOR / frame_width;
-}
-
-static u32 ov9282_flash_duration_to_us(struct ov9282 *ov9282, u32 value)
-{
-	/*
-	 * Calculate back to microseconds from "strobe_frame_span" increments.
-	 * As the calculation in ov9282_us_to_flash_duration uses an integer
-	 * divison round up here.
-	 */
-	u32 frame_width = ov9282->cur_mode->width + ov9282->hblank_ctrl->val;
-
-	return DIV_ROUND_UP(value * frame_width, OV9282_STROBE_SPAN_FACTOR);
-}
-
 static int ov9282_set_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct ov9282 *ov9282 =
@@ -672,7 +655,7 @@ static int ov9282_set_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 	case V4L2_CID_FLASH_DURATION:
 		ret = cci_write(ov9282->regmap, OV9282_REG_STROBE_FRAME_SPAN,
-				ov9282_us_to_flash_duration(ov9282, ctrl->val), NULL);
+				ov9282_us_to_lines(ov9282, ctrl->val), NULL);
 		break;
 	default:
 		dev_err(ov9282->dev, "Invalid control %d", ctrl->id);
@@ -691,11 +674,11 @@ static int ov9282_try_ctrl(struct v4l2_ctrl *ctrl)
 
 	if (ctrl->id == V4L2_CID_FLASH_DURATION) {
 		u32 us = ctrl->val;
-		u32 fd = ov9282_us_to_flash_duration(ov9282, us);
+		u32 fd = ov9282_us_to_lines(ov9282, us);
 
 		/* get nearest strobe_duration value */
-		u32 us0 = ov9282_flash_duration_to_us(ov9282, fd);
-		u32 us1 = ov9282_flash_duration_to_us(ov9282, fd + 1);
+		u32 us0 = ov9282_lines_to_us(ov9282, fd);
+		u32 us1 = ov9282_lines_to_us(ov9282, fd + 1);
 
 		if (abs(us1 - us) < abs(us - us0))
 			ctrl->val = us1;
@@ -1277,7 +1260,7 @@ static int ov9282_init_controls(struct ov9282 *ov9282)
 	v4l2_ctrl_new_std(ctrl_hdlr, &ov9282_ctrl_ops,
 			  V4L2_CID_FLASH_STROBE_OE, 0, 1, 1, 0);
 
-	exposure_us = ov9282_exposure_to_us(ov9282, OV9282_EXPOSURE_DEFAULT);
+	exposure_us = ov9282_lines_to_us(ov9282, OV9282_EXPOSURE_DEFAULT);
 	ov9282->flash_duration =
 		v4l2_ctrl_new_std(ctrl_hdlr, &ov9282_ctrl_ops,
 				  V4L2_CID_FLASH_DURATION, 0, exposure_us, 1,

-- 
2.53.0