Hi,
this is the fourth revision of the series containing refactoring and new
features implementation for the Omnivision OV4689 sensor
driver. Specifically, patches 1, 2, 3, 5, 6, 10, 15, 16, 18, and 19
are refactorings, and are not supposed to introduce any functional
change. Patches 4 and 7 perform migration to CCI helpers and subdevice
active state respectively, and should not introduce any hardware-
and/or user-visible change either. Patch 8 fixes a possible race
condition due to v4l2_async_register_subdev_sensor being called too
early in ov4689_probe, and patch 9 migrates power management to PM
autosuspend.
Patches 11-14 expose more sensor controls to the userspace, such as
(read-write) HBLANK, VFLIP/HFLIP, digital gain, and color
balance. Patch 17 implements configurable analogue crop rectangle via
set_selection callback. And finally, patch 20 enables 2x2 binning
option. It should be noted that publicly available sensor
documentation is lacking description of many registers and their value
ranges, so a lot of values had to be found by experimentation.
Changes in v4:
- rebase on top of media_stage
- collect Reviewed-by's from v3
- add comments "Horizontal" and "Vertical" to TIMING_FORMAT registers
- fix a typo in the commit message in patch 17/20
- remove "__" prefix from stream on/off functions
- rename the label in ov4689_stream_off and remove extra space
Changes in v3:
- rebase on top of media_stage
- collect Reviewed-by and Acked-by from v2
- update copyright year
- zero-initialize ret in ov4689_set_ctrl
- get back blank line before return in ov4689_set_ctrl
- move `sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE` before
`ov4689_initialize_controls()` in ov4689_probe
- remove blank line after `v4l2_subdev_init_finalize(sd)`
- use pm_runtime_put instead of pm_runtime_put_sync
- add comment for dummy columns/rows defines
- fix OV4689_PIXEL_ARRAY_WIDTH use instead of OV4689_PIXEL_ARRAY_HEIGHT
in ov4689_get_selection
- split s_stream into two functions for start and stop
Changes in v2:
- collect Laurent's r-b's
- squash together "CCI conversion" and "Set gain in one 16 bit write"
- use ctrl->val in ov4689_set_ctrl
- rename try_fmt to fmt in ov4689_init_cfg and drop corresponding comment
- rebase on top of media-stage and rename init_cfg->init_state
- sort register definitions by address throughout the whole series
- fix number of controls hint in v4l2_ctrl_handler_init
- make all hexadecimal constants lowercase
- disable runtime pm in probe error path
- implement pm autosuspend
Mikhail Rudenko (20):
media: i2c: ov4689: Clean up and annotate the register table
media: i2c: ov4689: Sort register definitions by address
media: i2c: ov4689: Fix typo in a comment
media: i2c: ov4689: CCI conversion
media: i2c: ov4689: Remove i2c_client from ov4689 struct
media: i2c: ov4689: Refactor ov4689_set_ctrl
media: i2c: ov4689: Use sub-device active state
media: i2c: ov4689: Enable runtime PM before registering sub-device
media: i2c: ov4689: Use runtime PM autosuspend
media: i2c: ov4689: Remove max_fps field from struct ov4689_mode
media: i2c: ov4689: Make horizontal blanking configurable
media: i2c: ov4689: Implement vflip/hflip controls
media: i2c: ov4689: Implement digital gain control
media: i2c: ov4689: Implement manual color balance controls
media: i2c: ov4689: Move pixel array size out of struct ov4689_mode
media: i2c: ov4689: Set timing registers programmatically
media: i2c: ov4689: Configurable analogue crop
media: i2c: ov4689: Eliminate struct ov4689_mode
media: i2c: ov4689: Refactor ov4689_s_stream
media: i2c: ov4689: Implement 2x2 binning
drivers/media/i2c/Kconfig | 1 +
drivers/media/i2c/ov4689.c | 1003 ++++++++++++++++++++++--------------
2 files changed, 618 insertions(+), 386 deletions(-)
--
2.44.0
Put register defininitions in the order of increasing register
address.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/ov4689.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index 5a509e908e3c..a6c101044eb3 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -19,15 +19,15 @@
#include <media/v4l2-subdev.h>
#include <media/v4l2-fwnode.h>
-#define CHIP_ID 0x004688
-#define OV4689_REG_CHIP_ID 0x300a
-
#define OV4689_XVCLK_FREQ 24000000
#define OV4689_REG_CTRL_MODE 0x0100
#define OV4689_MODE_SW_STANDBY 0x0
#define OV4689_MODE_STREAMING BIT(0)
+#define OV4689_REG_CHIP_ID 0x300a
+#define CHIP_ID 0x004688
+
#define OV4689_REG_EXPOSURE 0x3500
#define OV4689_EXPOSURE_MIN 4
#define OV4689_EXPOSURE_STEP 1
@@ -41,12 +41,12 @@
#define OV4689_GAIN_STEP 1
#define OV4689_GAIN_DEFAULT 0x80
+#define OV4689_REG_VTS 0x380e
+
#define OV4689_REG_TEST_PATTERN 0x5040
#define OV4689_TEST_PATTERN_ENABLE 0x80
#define OV4689_TEST_PATTERN_DISABLE 0x0
-#define OV4689_REG_VTS 0x380e
-
#define REG_NULL 0xFFFF
#define OV4689_REG_VALUE_08BIT 1
--
2.44.0
Fix a spelling error in a comment.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/ov4689.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index a6c101044eb3..f2614a0b1340 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -692,7 +692,7 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
switch (ctrl->id) {
case V4L2_CID_EXPOSURE:
- /* 4 least significant bits of expsoure are fractional part */
+ /* 4 least significant bits of exposure are fractional part */
ret = ov4689_write_reg(ov4689->client, OV4689_REG_EXPOSURE,
OV4689_REG_VALUE_24BIT, ctrl->val << 4);
break;
--
2.44.0
Convert the i2c register accesses to utilize the CCI helper library
rather than relying on driver-specific functions. Also, set analogue
gain in a single 16-bit write instead of two 8-bit writes.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/Kconfig | 1 +
drivers/media/i2c/ov4689.c | 358 ++++++++++++++-----------------------
2 files changed, 133 insertions(+), 226 deletions(-)
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 56f276b920ab..cef6cab14fd4 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -405,6 +405,7 @@ config VIDEO_OV2740
config VIDEO_OV4689
tristate "OmniVision OV4689 sensor support"
depends on GPIOLIB
+ select V4L2_CCI_I2C
help
This is a Video4Linux2 sensor-level driver for the OmniVision
OV4689 camera.
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index f2614a0b1340..d31659139a6e 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -15,45 +15,35 @@
#include <linux/regulator/consumer.h>
#include <media/media-entity.h>
#include <media/v4l2-async.h>
+#include <media/v4l2-cci.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-fwnode.h>
-#define OV4689_XVCLK_FREQ 24000000
-
-#define OV4689_REG_CTRL_MODE 0x0100
+#define OV4689_REG_CTRL_MODE CCI_REG8(0x0100)
#define OV4689_MODE_SW_STANDBY 0x0
#define OV4689_MODE_STREAMING BIT(0)
-#define OV4689_REG_CHIP_ID 0x300a
+#define OV4689_REG_CHIP_ID CCI_REG16(0x300a)
#define CHIP_ID 0x004688
-#define OV4689_REG_EXPOSURE 0x3500
+#define OV4689_REG_EXPOSURE CCI_REG24(0x3500)
#define OV4689_EXPOSURE_MIN 4
#define OV4689_EXPOSURE_STEP 1
#define OV4689_VTS_MAX 0x7fff
-#define OV4689_REG_GAIN_H 0x3508
-#define OV4689_REG_GAIN_L 0x3509
-#define OV4689_GAIN_H_MASK 0x07
-#define OV4689_GAIN_H_SHIFT 8
-#define OV4689_GAIN_L_MASK 0xff
+#define OV4689_REG_GAIN CCI_REG16(0x3508)
#define OV4689_GAIN_STEP 1
#define OV4689_GAIN_DEFAULT 0x80
-#define OV4689_REG_VTS 0x380e
+#define OV4689_REG_VTS CCI_REG16(0x380e)
-#define OV4689_REG_TEST_PATTERN 0x5040
+#define OV4689_REG_TEST_PATTERN CCI_REG8(0x5040)
#define OV4689_TEST_PATTERN_ENABLE 0x80
#define OV4689_TEST_PATTERN_DISABLE 0x0
-#define REG_NULL 0xFFFF
-
-#define OV4689_REG_VALUE_08BIT 1
-#define OV4689_REG_VALUE_16BIT 2
-#define OV4689_REG_VALUE_24BIT 3
-
#define OV4689_LANES 4
+#define OV4689_XVCLK_FREQ 24000000
static const char *const ov4689_supply_names[] = {
"avdd", /* Analog power */
@@ -61,11 +51,6 @@ static const char *const ov4689_supply_names[] = {
"dvdd", /* Digital core power */
};
-struct regval {
- u16 addr;
- u8 val;
-};
-
enum ov4689_mode_id {
OV4689_MODE_2688_1520 = 0,
OV4689_NUM_MODES,
@@ -84,11 +69,13 @@ struct ov4689_mode {
u32 sensor_height;
u32 crop_top;
u32 crop_left;
- const struct regval *reg_list;
+ const struct cci_reg_sequence *reg_list;
+ unsigned int num_regs;
};
struct ov4689 {
struct i2c_client *client;
+ struct regmap *regmap;
struct clk *xvclk;
struct gpio_desc *reset_gpio;
struct gpio_desc *pwdn_gpio;
@@ -122,125 +109,124 @@ struct ov4689_gain_range {
* max_framerate 30fps
* mipi_datarate per lane 1008Mbps
*/
-static const struct regval ov4689_2688x1520_regs[] = {
+static const struct cci_reg_sequence ov4689_2688x1520_regs[] = {
/* System control*/
- {0x0103, 0x01}, /* SC_CTRL0103 software_reset = 1 */
- {0x3000, 0x20}, /* SC_CMMN_PAD_OEN0 FSIN_output_enable = 1 */
- {0x3021, 0x03}, /*
- * SC_CMMN_MISC_CTRL fst_stby_ctr = 0,
- * sleep_no_latch_enable = 0
- */
+ {CCI_REG8(0x0103), 0x01}, /* SC_CTRL0103 software_reset = 1 */
+ {CCI_REG8(0x3000), 0x20}, /* SC_CMMN_PAD_OEN0 FSIN_output_enable = 1 */
+ {CCI_REG8(0x3021), 0x03}, /*
+ * SC_CMMN_MISC_CTRL fst_stby_ctr = 0,
+ * sleep_no_latch_enable = 0
+ */
/* AEC PK */
- {0x3503, 0x04}, /* AEC_MANUAL gain_input_as_sensor_gain_format = 1 */
- {0x352a, 0x08}, /* DIG_GAIN_FRAC_LONG dig_gain_long[14:8] = 0x08 (2x) */
+ {CCI_REG8(0x3503), 0x04}, /* AEC_MANUAL gain_input_as_sensor_gain_format = 1 */
+ {CCI_REG8(0x352a), 0x08}, /* DIG_GAIN_FRAC_LONG dig_gain_long[14:8] = 0x08 (2x) */
/* ADC and analog control*/
- {0x3603, 0x40},
- {0x3604, 0x02},
- {0x3609, 0x12},
- {0x360c, 0x08},
- {0x360f, 0xe5},
- {0x3608, 0x8f},
- {0x3611, 0x00},
- {0x3613, 0xf7},
- {0x3616, 0x58},
- {0x3619, 0x99},
- {0x361b, 0x60},
- {0x361e, 0x79},
- {0x3634, 0x10},
- {0x3635, 0x10},
- {0x3636, 0x15},
- {0x3646, 0x86},
- {0x364a, 0x0b},
+ {CCI_REG8(0x3603), 0x40},
+ {CCI_REG8(0x3604), 0x02},
+ {CCI_REG8(0x3609), 0x12},
+ {CCI_REG8(0x360c), 0x08},
+ {CCI_REG8(0x360f), 0xe5},
+ {CCI_REG8(0x3608), 0x8f},
+ {CCI_REG8(0x3611), 0x00},
+ {CCI_REG8(0x3613), 0xf7},
+ {CCI_REG8(0x3616), 0x58},
+ {CCI_REG8(0x3619), 0x99},
+ {CCI_REG8(0x361b), 0x60},
+ {CCI_REG8(0x361e), 0x79},
+ {CCI_REG8(0x3634), 0x10},
+ {CCI_REG8(0x3635), 0x10},
+ {CCI_REG8(0x3636), 0x15},
+ {CCI_REG8(0x3646), 0x86},
+ {CCI_REG8(0x364a), 0x0b},
/* Sensor control */
- {0x3700, 0x17},
- {0x3701, 0x22},
- {0x3703, 0x10},
- {0x370a, 0x37},
- {0x3706, 0x63},
- {0x3709, 0x3c},
- {0x370c, 0x30},
- {0x3710, 0x24},
- {0x3720, 0x28},
- {0x3729, 0x7b},
- {0x372b, 0xbd},
- {0x372c, 0xbc},
- {0x372e, 0x52},
- {0x373c, 0x0e},
- {0x373e, 0x33},
- {0x3743, 0x10},
- {0x3744, 0x88},
- {0x3745, 0xc0},
- {0x374c, 0x00},
- {0x374e, 0x23},
- {0x3751, 0x7b},
- {0x3753, 0xbd},
- {0x3754, 0xbc},
- {0x3756, 0x52},
- {0x376b, 0x20},
- {0x3774, 0x51},
- {0x3776, 0xbd},
- {0x3777, 0xbd},
- {0x3781, 0x18},
- {0x3783, 0x25},
- {0x3798, 0x1b},
+ {CCI_REG8(0x3700), 0x17},
+ {CCI_REG8(0x3701), 0x22},
+ {CCI_REG8(0x3703), 0x10},
+ {CCI_REG8(0x370a), 0x37},
+ {CCI_REG8(0x3706), 0x63},
+ {CCI_REG8(0x3709), 0x3c},
+ {CCI_REG8(0x370c), 0x30},
+ {CCI_REG8(0x3710), 0x24},
+ {CCI_REG8(0x3720), 0x28},
+ {CCI_REG8(0x3729), 0x7b},
+ {CCI_REG8(0x372b), 0xbd},
+ {CCI_REG8(0x372c), 0xbc},
+ {CCI_REG8(0x372e), 0x52},
+ {CCI_REG8(0x373c), 0x0e},
+ {CCI_REG8(0x373e), 0x33},
+ {CCI_REG8(0x3743), 0x10},
+ {CCI_REG8(0x3744), 0x88},
+ {CCI_REG8(0x3745), 0xc0},
+ {CCI_REG8(0x374c), 0x00},
+ {CCI_REG8(0x374e), 0x23},
+ {CCI_REG8(0x3751), 0x7b},
+ {CCI_REG8(0x3753), 0xbd},
+ {CCI_REG8(0x3754), 0xbc},
+ {CCI_REG8(0x3756), 0x52},
+ {CCI_REG8(0x376b), 0x20},
+ {CCI_REG8(0x3774), 0x51},
+ {CCI_REG8(0x3776), 0xbd},
+ {CCI_REG8(0x3777), 0xbd},
+ {CCI_REG8(0x3781), 0x18},
+ {CCI_REG8(0x3783), 0x25},
+ {CCI_REG8(0x3798), 0x1b},
/* Timing control */
- {0x3801, 0x08}, /* H_CROP_START_L h_crop_start[7:0] = 0x08 */
- {0x3805, 0x97}, /* H_CROP_END_L h_crop_end[7:0] = 0x97 */
- {0x380c, 0x0a}, /* TIMING_HTS_H hts[14:8] = 0x0a */
- {0x380d, 0x0e}, /* TIMING_HTS_L hts[7:0] = 0x0e */
- {0x3811, 0x08}, /* H_WIN_OFF_L h_win_off[7:0] = 0x08*/
- {0x3813, 0x04}, /* V_WIN_OFF_L v_win_off[7:0] = 0x04 */
- {0x3819, 0x01}, /* VSYNC_END_L vsync_end_point[7:0] = 0x01 */
- {0x3821, 0x06}, /* TIMING_FORMAT2 array_h_mirror = 1, digital_h_mirror = 1 */
+ {CCI_REG8(0x3801), 0x08}, /* H_CROP_START_L h_crop_start[7:0] = 0x08 */
+ {CCI_REG8(0x3805), 0x97}, /* H_CROP_END_L h_crop_end[7:0] = 0x97 */
+ {CCI_REG8(0x380c), 0x0a}, /* TIMING_HTS_H hts[14:8] = 0x0a */
+ {CCI_REG8(0x380d), 0x0e}, /* TIMING_HTS_L hts[7:0] = 0x0e */
+ {CCI_REG8(0x3811), 0x08}, /* H_WIN_OFF_L h_win_off[7:0] = 0x08*/
+ {CCI_REG8(0x3813), 0x04}, /* V_WIN_OFF_L v_win_off[7:0] = 0x04 */
+ {CCI_REG8(0x3819), 0x01}, /* VSYNC_END_L vsync_end_point[7:0] = 0x01 */
+ {CCI_REG8(0x3821), 0x06}, /* TIMING_FORMAT2 array_h_mirror = 1, digital_h_mirror = 1 */
/* OTP control */
- {0x3d85, 0x36}, /* OTP_REG85 OTP_power_up_load_setting_enable = 1,
- * OTP_power_up_load_data_enable = 1,
- * OTP_bist_select = 1 (compare with zero)
- */
- {0x3d8c, 0x71}, /* OTP_SETTING_STT_ADDRESS_H */
- {0x3d8d, 0xcb}, /* OTP_SETTING_STT_ADDRESS_L */
+ {CCI_REG8(0x3d85), 0x36}, /* OTP_REG85 OTP_power_up_load_setting_enable = 1,
+ * OTP_power_up_load_data_enable = 1,
+ * OTP_bist_select = 1 (compare with zero)
+ */
+ {CCI_REG8(0x3d8c), 0x71}, /* OTP_SETTING_STT_ADDRESS_H */
+ {CCI_REG8(0x3d8d), 0xcb}, /* OTP_SETTING_STT_ADDRESS_L */
/* BLC registers*/
- {0x4001, 0x40}, /* DEBUG_MODE */
- {0x401b, 0x00}, /* DEBUG_MODE */
- {0x401d, 0x00}, /* DEBUG_MODE */
- {0x401f, 0x00}, /* DEBUG_MODE */
- {0x4020, 0x00}, /* ANCHOR_LEFT_START_H anchor_left_start[11:8] = 0 */
- {0x4021, 0x10}, /* ANCHOR_LEFT_START_L anchor_left_start[7:0] = 0x10 */
- {0x4022, 0x07}, /* ANCHOR_LEFT_END_H anchor_left_end[11:8] = 0x07 */
- {0x4023, 0xcf}, /* ANCHOR_LEFT_END_L anchor_left_end[7:0] = 0xcf */
- {0x4024, 0x09}, /* ANCHOR_RIGHT_START_H anchor_right_start[11:8] = 0x09 */
- {0x4025, 0x60}, /* ANCHOR_RIGHT_START_L anchor_right_start[7:0] = 0x60 */
- {0x4026, 0x09}, /* ANCHOR_RIGHT_END_H anchor_right_end[11:8] = 0x09 */
- {0x4027, 0x6f}, /* ANCHOR_RIGHT_END_L anchor_right_end[7:0] = 0x6f */
+ {CCI_REG8(0x4001), 0x40}, /* DEBUG_MODE */
+ {CCI_REG8(0x401b), 0x00}, /* DEBUG_MODE */
+ {CCI_REG8(0x401d), 0x00}, /* DEBUG_MODE */
+ {CCI_REG8(0x401f), 0x00}, /* DEBUG_MODE */
+ {CCI_REG8(0x4020), 0x00}, /* ANCHOR_LEFT_START_H anchor_left_start[11:8] = 0 */
+ {CCI_REG8(0x4021), 0x10}, /* ANCHOR_LEFT_START_L anchor_left_start[7:0] = 0x10 */
+ {CCI_REG8(0x4022), 0x07}, /* ANCHOR_LEFT_END_H anchor_left_end[11:8] = 0x07 */
+ {CCI_REG8(0x4023), 0xcf}, /* ANCHOR_LEFT_END_L anchor_left_end[7:0] = 0xcf */
+ {CCI_REG8(0x4024), 0x09}, /* ANCHOR_RIGHT_START_H anchor_right_start[11:8] = 0x09 */
+ {CCI_REG8(0x4025), 0x60}, /* ANCHOR_RIGHT_START_L anchor_right_start[7:0] = 0x60 */
+ {CCI_REG8(0x4026), 0x09}, /* ANCHOR_RIGHT_END_H anchor_right_end[11:8] = 0x09 */
+ {CCI_REG8(0x4027), 0x6f}, /* ANCHOR_RIGHT_END_L anchor_right_end[7:0] = 0x6f */
/* ADC sync control */
- {0x4500, 0x6c}, /* ADC_SYNC_CTRL */
- {0x4503, 0x01}, /* ADC_SYNC_CTRL */
+ {CCI_REG8(0x4500), 0x6c}, /* ADC_SYNC_CTRL */
+ {CCI_REG8(0x4503), 0x01}, /* ADC_SYNC_CTRL */
/* VFIFO */
- {0x4601, 0xa7}, /* VFIFO_CTRL_01 r_vfifo_read_start[7:0] = 0xa7 */
+ {CCI_REG8(0x4601), 0xa7}, /* VFIFO_CTRL_01 r_vfifo_read_start[7:0] = 0xa7 */
/* Temperature monitor */
- {0x4d00, 0x04}, /* TPM_CTRL_00 tmp_slope[15:8] = 0x04 */
- {0x4d01, 0x42}, /* TPM_CTRL_01 tmp_slope[7:0] = 0x42 */
- {0x4d02, 0xd1}, /* TPM_CTRL_02 tpm_offset[31:24] = 0xd1 */
- {0x4d03, 0x93}, /* TPM_CTRL_03 tpm_offset[23:16] = 0x93 */
- {0x4d04, 0xf5}, /* TPM_CTRL_04 tpm_offset[15:8] = 0xf5 */
- {0x4d05, 0xc1}, /* TPM_CTRL_05 tpm_offset[7:0] = 0xc1 */
+ {CCI_REG8(0x4d00), 0x04}, /* TPM_CTRL_00 tmp_slope[15:8] = 0x04 */
+ {CCI_REG8(0x4d01), 0x42}, /* TPM_CTRL_01 tmp_slope[7:0] = 0x42 */
+ {CCI_REG8(0x4d02), 0xd1}, /* TPM_CTRL_02 tpm_offset[31:24] = 0xd1 */
+ {CCI_REG8(0x4d03), 0x93}, /* TPM_CTRL_03 tpm_offset[23:16] = 0x93 */
+ {CCI_REG8(0x4d04), 0xf5}, /* TPM_CTRL_04 tpm_offset[15:8] = 0xf5 */
+ {CCI_REG8(0x4d05), 0xc1}, /* TPM_CTRL_05 tpm_offset[7:0] = 0xc1 */
/* pre-ISP control */
- {0x5050, 0x0c}, /* DEBUG_MODE */
+ {CCI_REG8(0x5050), 0x0c}, /* DEBUG_MODE */
/* OTP-DPC control */
- {0x5501, 0x10}, /* OTP_DPC_START_L otp_start_address[7:0] = 0x10 */
- {0x5503, 0x0f}, /* OTP_DPC_END_L otp_end_address[7:0] = 0x0f */
- {REG_NULL, 0x00},
+ {CCI_REG8(0x5501), 0x10}, /* OTP_DPC_START_L otp_start_address[7:0] = 0x10 */
+ {CCI_REG8(0x5503), 0x0f}, /* OTP_DPC_END_L otp_end_address[7:0] = 0x0f */
};
static const struct ov4689_mode supported_modes[] = {
@@ -258,6 +244,7 @@ static const struct ov4689_mode supported_modes[] = {
.vts_def = 1554,
.pixel_rate = 480000000,
.reg_list = ov4689_2688x1520_regs,
+ .num_regs = ARRAY_SIZE(ov4689_2688x1520_regs),
},
};
@@ -310,83 +297,6 @@ static const struct ov4689_gain_range ov4689_gain_ranges[] = {
},
};
-/* Write registers up to 4 at a time */
-static int ov4689_write_reg(struct i2c_client *client, u16 reg, u32 len,
- u32 val)
-{
- u32 buf_i, val_i;
- __be32 val_be;
- u8 *val_p;
- u8 buf[6];
-
- if (len > 4)
- return -EINVAL;
-
- buf[0] = reg >> 8;
- buf[1] = reg & 0xff;
-
- val_be = cpu_to_be32(val);
- val_p = (u8 *)&val_be;
- buf_i = 2;
- val_i = 4 - len;
-
- while (val_i < 4)
- buf[buf_i++] = val_p[val_i++];
-
- if (i2c_master_send(client, buf, len + 2) != len + 2)
- return -EIO;
-
- return 0;
-}
-
-static int ov4689_write_array(struct i2c_client *client,
- const struct regval *regs)
-{
- int ret = 0;
- u32 i;
-
- for (i = 0; ret == 0 && regs[i].addr != REG_NULL; i++)
- ret = ov4689_write_reg(client, regs[i].addr,
- OV4689_REG_VALUE_08BIT, regs[i].val);
-
- return ret;
-}
-
-/* Read registers up to 4 at a time */
-static int ov4689_read_reg(struct i2c_client *client, u16 reg, unsigned int len,
- u32 *val)
-{
- __be16 reg_addr_be = cpu_to_be16(reg);
- struct i2c_msg msgs[2];
- __be32 data_be = 0;
- u8 *data_be_p;
- int ret;
-
- if (len > 4 || !len)
- return -EINVAL;
-
- data_be_p = (u8 *)&data_be;
- /* Write register address */
- msgs[0].addr = client->addr;
- msgs[0].flags = 0;
- msgs[0].len = 2;
- msgs[0].buf = (u8 *)®_addr_be;
-
- /* Read data from register */
- msgs[1].addr = client->addr;
- msgs[1].flags = I2C_M_RD;
- msgs[1].len = len;
- msgs[1].buf = &data_be_p[4 - len];
-
- ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
- if (ret != ARRAY_SIZE(msgs))
- return -EIO;
-
- *val = be32_to_cpu(data_be);
-
- return 0;
-}
-
static void ov4689_fill_fmt(const struct ov4689_mode *mode,
struct v4l2_mbus_framefmt *fmt)
{
@@ -460,8 +370,8 @@ static int ov4689_enable_test_pattern(struct ov4689 *ov4689, u32 pattern)
else
val = OV4689_TEST_PATTERN_DISABLE;
- return ov4689_write_reg(ov4689->client, OV4689_REG_TEST_PATTERN,
- OV4689_REG_VALUE_08BIT, val);
+ return cci_write(ov4689->regmap, OV4689_REG_TEST_PATTERN,
+ val, NULL);
}
static int ov4689_get_selection(struct v4l2_subdev *sd,
@@ -505,8 +415,10 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
if (ret < 0)
goto unlock_and_return;
- ret = ov4689_write_array(ov4689->client,
- ov4689->cur_mode->reg_list);
+ ret = cci_multi_reg_write(ov4689->regmap,
+ ov4689->cur_mode->reg_list,
+ ov4689->cur_mode->num_regs,
+ NULL);
if (ret) {
pm_runtime_put(&client->dev);
goto unlock_and_return;
@@ -518,17 +430,15 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
goto unlock_and_return;
}
- ret = ov4689_write_reg(ov4689->client, OV4689_REG_CTRL_MODE,
- OV4689_REG_VALUE_08BIT,
- OV4689_MODE_STREAMING);
+ ret = cci_write(ov4689->regmap, OV4689_REG_CTRL_MODE,
+ OV4689_MODE_STREAMING, NULL);
if (ret) {
pm_runtime_put(&client->dev);
goto unlock_and_return;
}
} else {
- ov4689_write_reg(ov4689->client, OV4689_REG_CTRL_MODE,
- OV4689_REG_VALUE_08BIT,
- OV4689_MODE_SW_STANDBY);
+ cci_write(ov4689->regmap, OV4689_REG_CTRL_MODE,
+ OV4689_MODE_SW_STANDBY, NULL);
pm_runtime_put(&client->dev);
}
@@ -693,26 +603,16 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
switch (ctrl->id) {
case V4L2_CID_EXPOSURE:
/* 4 least significant bits of exposure are fractional part */
- ret = ov4689_write_reg(ov4689->client, OV4689_REG_EXPOSURE,
- OV4689_REG_VALUE_24BIT, ctrl->val << 4);
+ ret = cci_write(ov4689->regmap, OV4689_REG_EXPOSURE,
+ ctrl->val << 4, NULL);
break;
case V4L2_CID_ANALOGUE_GAIN:
ret = ov4689_map_gain(ov4689, ctrl->val, &sensor_gain);
-
- ret = ret ?:
- ov4689_write_reg(ov4689->client, OV4689_REG_GAIN_H,
- OV4689_REG_VALUE_08BIT,
- (sensor_gain >> OV4689_GAIN_H_SHIFT) &
- OV4689_GAIN_H_MASK);
- ret = ret ?:
- ov4689_write_reg(ov4689->client, OV4689_REG_GAIN_L,
- OV4689_REG_VALUE_08BIT,
- sensor_gain & OV4689_GAIN_L_MASK);
+ cci_write(ov4689->regmap, OV4689_REG_GAIN, sensor_gain, &ret);
break;
case V4L2_CID_VBLANK:
- ret = ov4689_write_reg(ov4689->client, OV4689_REG_VTS,
- OV4689_REG_VALUE_16BIT,
- ctrl->val + ov4689->cur_mode->height);
+ ret = cci_write(ov4689->regmap, OV4689_REG_VTS,
+ ctrl->val + ov4689->cur_mode->height, NULL);
break;
case V4L2_CID_TEST_PATTERN:
ret = ov4689_enable_test_pattern(ov4689, ctrl->val);
@@ -817,18 +717,17 @@ static int ov4689_check_sensor_id(struct ov4689 *ov4689,
struct i2c_client *client)
{
struct device *dev = &ov4689->client->dev;
- u32 id = 0;
+ u64 id = 0;
int ret;
- ret = ov4689_read_reg(client, OV4689_REG_CHIP_ID,
- OV4689_REG_VALUE_16BIT, &id);
+ ret = cci_read(ov4689->regmap, OV4689_REG_CHIP_ID, &id, NULL);
if (ret) {
dev_err(dev, "Cannot read sensor ID\n");
return ret;
}
if (id != CHIP_ID) {
- dev_err(dev, "Unexpected sensor ID %06x, expected %06x\n",
+ dev_err(dev, "Unexpected sensor ID %06llx, expected %06x\n",
id, CHIP_ID);
return -ENODEV;
}
@@ -938,6 +837,13 @@ static int ov4689_probe(struct i2c_client *client)
return -EINVAL;
}
+ ov4689->regmap = devm_cci_regmap_init_i2c(client, 16);
+ if (IS_ERR(ov4689->regmap)) {
+ ret = PTR_ERR(ov4689->regmap);
+ dev_err(dev, "failed to initialize CCI: %d\n", ret);
+ return ret;
+ }
+
ov4689->reset_gpio = devm_gpiod_get_optional(dev, "reset",
GPIOD_OUT_LOW);
if (IS_ERR(ov4689->reset_gpio)) {
--
2.44.0
The 'client' field within the 'ov4689' structure is solely used to
access its 'dev' member. This commit removes the 'client' field and
directly stores a pointer to the 'struct device'.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/ov4689.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index d31659139a6e..56bf1d964e43 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -74,7 +74,7 @@ struct ov4689_mode {
};
struct ov4689 {
- struct i2c_client *client;
+ struct device *dev;
struct regmap *regmap;
struct clk *xvclk;
struct gpio_desc *reset_gpio;
@@ -405,13 +405,13 @@ static int ov4689_get_selection(struct v4l2_subdev *sd,
static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
{
struct ov4689 *ov4689 = to_ov4689(sd);
- struct i2c_client *client = ov4689->client;
+ struct device *dev = ov4689->dev;
int ret = 0;
mutex_lock(&ov4689->mutex);
if (on) {
- ret = pm_runtime_resume_and_get(&client->dev);
+ ret = pm_runtime_resume_and_get(dev);
if (ret < 0)
goto unlock_and_return;
@@ -420,26 +420,26 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
ov4689->cur_mode->num_regs,
NULL);
if (ret) {
- pm_runtime_put(&client->dev);
+ pm_runtime_put(dev);
goto unlock_and_return;
}
ret = __v4l2_ctrl_handler_setup(&ov4689->ctrl_handler);
if (ret) {
- pm_runtime_put(&client->dev);
+ pm_runtime_put(dev);
goto unlock_and_return;
}
ret = cci_write(ov4689->regmap, OV4689_REG_CTRL_MODE,
OV4689_MODE_STREAMING, NULL);
if (ret) {
- pm_runtime_put(&client->dev);
+ pm_runtime_put(dev);
goto unlock_and_return;
}
} else {
cci_write(ov4689->regmap, OV4689_REG_CTRL_MODE,
OV4689_MODE_SW_STANDBY, NULL);
- pm_runtime_put(&client->dev);
+ pm_runtime_put(dev);
}
unlock_and_return:
@@ -553,7 +553,6 @@ static const struct v4l2_subdev_ops ov4689_subdev_ops = {
*/
static int ov4689_map_gain(struct ov4689 *ov4689, int logical_gain, int *result)
{
- const struct device *dev = &ov4689->client->dev;
const struct ov4689_gain_range *range;
unsigned int n;
@@ -564,7 +563,8 @@ static int ov4689_map_gain(struct ov4689 *ov4689, int logical_gain, int *result)
}
if (n == ARRAY_SIZE(ov4689_gain_ranges)) {
- dev_warn_ratelimited(dev, "no mapping found for gain %d\n",
+ dev_warn_ratelimited(ov4689->dev,
+ "no mapping found for gain %d\n",
logical_gain);
return -EINVAL;
}
@@ -580,7 +580,7 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
{
struct ov4689 *ov4689 =
container_of(ctrl->handler, struct ov4689, ctrl_handler);
- struct i2c_client *client = ov4689->client;
+ struct device *dev = ov4689->dev;
int sensor_gain;
s64 max_expo;
int ret;
@@ -597,7 +597,7 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
break;
}
- if (!pm_runtime_get_if_in_use(&client->dev))
+ if (!pm_runtime_get_if_in_use(dev))
return 0;
switch (ctrl->id) {
@@ -618,13 +618,13 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
ret = ov4689_enable_test_pattern(ov4689, ctrl->val);
break;
default:
- dev_warn(&client->dev, "%s Unhandled id:0x%x, val:0x%x\n",
+ dev_warn(dev, "%s Unhandled id:0x%x, val:0x%x\n",
__func__, ctrl->id, ctrl->val);
ret = -EINVAL;
break;
}
- pm_runtime_put(&client->dev);
+ pm_runtime_put(dev);
return ret;
}
@@ -689,8 +689,7 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
if (handler->error) {
ret = handler->error;
- dev_err(&ov4689->client->dev, "Failed to init controls(%d)\n",
- ret);
+ dev_err(ov4689->dev, "Failed to init controls(%d)\n", ret);
goto err_free_handler;
}
@@ -716,7 +715,7 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
static int ov4689_check_sensor_id(struct ov4689 *ov4689,
struct i2c_client *client)
{
- struct device *dev = &ov4689->client->dev;
+ struct device *dev = ov4689->dev;
u64 id = 0;
int ret;
@@ -744,7 +743,7 @@ static int ov4689_configure_regulators(struct ov4689 *ov4689)
for (i = 0; i < ARRAY_SIZE(ov4689_supply_names); i++)
ov4689->supplies[i].supply = ov4689_supply_names[i];
- return devm_regulator_bulk_get(&ov4689->client->dev,
+ return devm_regulator_bulk_get(ov4689->dev,
ARRAY_SIZE(ov4689_supply_names),
ov4689->supplies);
}
@@ -813,7 +812,8 @@ static int ov4689_probe(struct i2c_client *client)
if (!ov4689)
return -ENOMEM;
- ov4689->client = client;
+ ov4689->dev = dev;
+
ov4689->cur_mode = &supported_modes[OV4689_MODE_2688_1520];
ov4689->xvclk = devm_clk_get_optional(dev, NULL);
--
2.44.0
Introduce local variable for regmap within the ov4689_set_ctrl
function. This adjustment eliminates repetition within the function.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Kieran Bingham <[email protected]>
---
drivers/media/i2c/ov4689.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index 56bf1d964e43..1bc00404f3fb 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -580,10 +580,11 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
{
struct ov4689 *ov4689 =
container_of(ctrl->handler, struct ov4689, ctrl_handler);
+ struct regmap *regmap = ov4689->regmap;
struct device *dev = ov4689->dev;
int sensor_gain;
s64 max_expo;
- int ret;
+ int ret = 0;
/* Propagate change of current control to all related controls */
switch (ctrl->id) {
@@ -603,16 +604,15 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
switch (ctrl->id) {
case V4L2_CID_EXPOSURE:
/* 4 least significant bits of exposure are fractional part */
- ret = cci_write(ov4689->regmap, OV4689_REG_EXPOSURE,
- ctrl->val << 4, NULL);
+ cci_write(regmap, OV4689_REG_EXPOSURE, ctrl->val << 4, &ret);
break;
case V4L2_CID_ANALOGUE_GAIN:
ret = ov4689_map_gain(ov4689, ctrl->val, &sensor_gain);
- cci_write(ov4689->regmap, OV4689_REG_GAIN, sensor_gain, &ret);
+ cci_write(regmap, OV4689_REG_GAIN, sensor_gain, &ret);
break;
case V4L2_CID_VBLANK:
- ret = cci_write(ov4689->regmap, OV4689_REG_VTS,
- ctrl->val + ov4689->cur_mode->height, NULL);
+ cci_write(regmap, OV4689_REG_VTS,
+ ctrl->val + ov4689->cur_mode->height, &ret);
break;
case V4L2_CID_TEST_PATTERN:
ret = ov4689_enable_test_pattern(ov4689, ctrl->val);
--
2.44.0
Use sub-device active state. Employ control handler lock to
synchronize access to the active state and s_stream.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/ov4689.c | 75 ++++++++++++++++----------------------
1 file changed, 31 insertions(+), 44 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index 1bc00404f3fb..47feb6b5ede8 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -86,7 +86,6 @@ struct ov4689 {
u32 clock_rate;
- struct mutex mutex; /* lock to protect ctrls and cur_mode */
struct v4l2_ctrl_handler ctrl_handler;
struct v4l2_ctrl *exposure;
@@ -319,19 +318,6 @@ static int ov4689_set_fmt(struct v4l2_subdev *sd,
return 0;
}
-static int ov4689_get_fmt(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *fmt)
-{
- struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
- struct ov4689 *ov4689 = to_ov4689(sd);
-
- /* only one mode supported for now */
- ov4689_fill_fmt(ov4689->cur_mode, mbus_fmt);
-
- return 0;
-}
-
static int ov4689_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_mbus_code_enum *code)
@@ -405,10 +391,11 @@ static int ov4689_get_selection(struct v4l2_subdev *sd,
static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
{
struct ov4689 *ov4689 = to_ov4689(sd);
+ struct v4l2_subdev_state *sd_state;
struct device *dev = ov4689->dev;
int ret = 0;
- mutex_lock(&ov4689->mutex);
+ sd_state = v4l2_subdev_lock_and_get_active_state(&ov4689->subdev);
if (on) {
ret = pm_runtime_resume_and_get(dev);
@@ -443,7 +430,7 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
}
unlock_and_return:
- mutex_unlock(&ov4689->mutex);
+ v4l2_subdev_unlock_state(sd_state);
return ret;
}
@@ -506,18 +493,13 @@ static int __maybe_unused ov4689_power_off(struct device *dev)
return 0;
}
-static int ov4689_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+static int ov4689_init_state(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state)
{
- struct ov4689 *ov4689 = to_ov4689(sd);
- struct v4l2_mbus_framefmt *try_fmt;
-
- mutex_lock(&ov4689->mutex);
-
- try_fmt = v4l2_subdev_state_get_format(fh->state, 0);
- /* Initialize try_fmt */
- ov4689_fill_fmt(&supported_modes[OV4689_MODE_2688_1520], try_fmt);
+ struct v4l2_mbus_framefmt *fmt =
+ v4l2_subdev_state_get_format(sd_state, 0);
- mutex_unlock(&ov4689->mutex);
+ ov4689_fill_fmt(&supported_modes[OV4689_MODE_2688_1520], fmt);
return 0;
}
@@ -526,10 +508,6 @@ static const struct dev_pm_ops ov4689_pm_ops = {
SET_RUNTIME_PM_OPS(ov4689_power_off, ov4689_power_on, NULL)
};
-static const struct v4l2_subdev_internal_ops ov4689_internal_ops = {
- .open = ov4689_open,
-};
-
static const struct v4l2_subdev_video_ops ov4689_video_ops = {
.s_stream = ov4689_s_stream,
};
@@ -537,11 +515,15 @@ static const struct v4l2_subdev_video_ops ov4689_video_ops = {
static const struct v4l2_subdev_pad_ops ov4689_pad_ops = {
.enum_mbus_code = ov4689_enum_mbus_code,
.enum_frame_size = ov4689_enum_frame_sizes,
- .get_fmt = ov4689_get_fmt,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = ov4689_set_fmt,
.get_selection = ov4689_get_selection,
};
+static const struct v4l2_subdev_internal_ops ov4689_internal_ops = {
+ .init_state = ov4689_init_state,
+};
+
static const struct v4l2_subdev_ops ov4689_subdev_ops = {
.video = &ov4689_video_ops,
.pad = &ov4689_pad_ops,
@@ -649,7 +631,6 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
ret = v4l2_ctrl_handler_init(handler, 10);
if (ret)
return ret;
- handler->lock = &ov4689->mutex;
ctrl = v4l2_ctrl_new_int_menu(handler, NULL, V4L2_CID_LINK_FREQ, 0, 0,
link_freq_menu_items);
@@ -862,13 +843,15 @@ static int ov4689_probe(struct i2c_client *client)
return dev_err_probe(dev, ret,
"Failed to get power regulators\n");
- mutex_init(&ov4689->mutex);
-
sd = &ov4689->subdev;
v4l2_i2c_subdev_init(sd, client, &ov4689_subdev_ops);
+ sd->internal_ops = &ov4689_internal_ops;
+ sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
ret = ov4689_initialize_controls(ov4689);
- if (ret)
- goto err_destroy_mutex;
+ if (ret) {
+ dev_err(dev, "Failed to initialize controls\n");
+ return ret;
+ }
ret = ov4689_power_on(dev);
if (ret)
@@ -878,19 +861,24 @@ static int ov4689_probe(struct i2c_client *client)
if (ret)
goto err_power_off;
- sd->internal_ops = &ov4689_internal_ops;
- sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
- ov4689->pad.flags = MEDIA_PAD_FL_SOURCE;
sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
+ ov4689->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(&sd->entity, 1, &ov4689->pad);
if (ret < 0)
goto err_power_off;
+ sd->state_lock = ov4689->ctrl_handler.lock;
+ ret = v4l2_subdev_init_finalize(sd);
+ if (ret) {
+ dev_err(dev, "Could not register v4l2 device\n");
+ goto err_clean_entity;
+ }
+
ret = v4l2_async_register_subdev_sensor(sd);
if (ret) {
dev_err(dev, "v4l2 async register subdev failed\n");
- goto err_clean_entity;
+ goto err_clean_subdev;
}
pm_runtime_set_active(dev);
@@ -899,14 +887,14 @@ static int ov4689_probe(struct i2c_client *client)
return 0;
+err_clean_subdev:
+ v4l2_subdev_cleanup(sd);
err_clean_entity:
media_entity_cleanup(&sd->entity);
err_power_off:
ov4689_power_off(dev);
err_free_handler:
v4l2_ctrl_handler_free(&ov4689->ctrl_handler);
-err_destroy_mutex:
- mutex_destroy(&ov4689->mutex);
return ret;
}
@@ -918,9 +906,8 @@ static void ov4689_remove(struct i2c_client *client)
v4l2_async_unregister_subdev(sd);
media_entity_cleanup(&sd->entity);
-
+ v4l2_subdev_cleanup(sd);
v4l2_ctrl_handler_free(&ov4689->ctrl_handler);
- mutex_destroy(&ov4689->mutex);
pm_runtime_disable(&client->dev);
if (!pm_runtime_status_suspended(&client->dev))
--
2.44.0
As the sensor may be accessible right after its async sub-device is
registered, enable runtime PM before doing so.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/ov4689.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index 47feb6b5ede8..0e0e694510aa 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -875,19 +875,21 @@ static int ov4689_probe(struct i2c_client *client)
goto err_clean_entity;
}
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+ pm_runtime_idle(dev);
+
ret = v4l2_async_register_subdev_sensor(sd);
if (ret) {
dev_err(dev, "v4l2 async register subdev failed\n");
- goto err_clean_subdev;
+ goto err_clean_subdev_pm;
}
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
- pm_runtime_idle(dev);
-
return 0;
-err_clean_subdev:
+err_clean_subdev_pm:
+ pm_runtime_disable(dev);
+ pm_runtime_set_suspended(dev);
v4l2_subdev_cleanup(sd);
err_clean_entity:
media_entity_cleanup(&sd->entity);
--
2.44.0
Use runtime PM autosuspend to avoid powering off the sensor during
fast stop-reconfigure-restart cycles.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/ov4689.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index 0e0e694510aa..2354397ab34c 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -426,7 +426,8 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
} else {
cci_write(ov4689->regmap, OV4689_REG_CTRL_MODE,
OV4689_MODE_SW_STANDBY, NULL);
- pm_runtime_put(dev);
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
}
unlock_and_return:
@@ -606,7 +607,8 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
break;
}
- pm_runtime_put(dev);
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
return ret;
}
@@ -876,8 +878,10 @@ static int ov4689_probe(struct i2c_client *client)
}
pm_runtime_set_active(dev);
+ pm_runtime_get_noresume(dev);
pm_runtime_enable(dev);
- pm_runtime_idle(dev);
+ pm_runtime_set_autosuspend_delay(dev, 1000);
+ pm_runtime_use_autosuspend(dev);
ret = v4l2_async_register_subdev_sensor(sd);
if (ret) {
@@ -885,11 +889,14 @@ static int ov4689_probe(struct i2c_client *client)
goto err_clean_subdev_pm;
}
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+
return 0;
err_clean_subdev_pm:
pm_runtime_disable(dev);
- pm_runtime_set_suspended(dev);
+ pm_runtime_put_noidle(dev);
v4l2_subdev_cleanup(sd);
err_clean_entity:
media_entity_cleanup(&sd->entity);
--
2.44.0
The OV4689 sensor supports horizontal and vertical flipping. Add
appropriate controls to the driver. Toggling both array flip and
digital flip bits allows to achieve flipping while maintaining output
Bayer order. Note that the default value of hflip control corresponds
to both bits set, as it was before this patch.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Kieran Bingham <[email protected]>
---
drivers/media/i2c/ov4689.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index 8283a9ac86c9..60c60bbfe3bb 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -42,6 +42,14 @@
#define OV4689_REG_VTS CCI_REG16(0x380e)
#define OV4689_VTS_MAX 0x7fff
+#define OV4689_REG_TIMING_FORMAT1 CCI_REG8(0x3820) /* Vertical */
+#define OV4689_REG_TIMING_FORMAT2 CCI_REG8(0x3821) /* Horizontal */
+#define OV4689_TIMING_FLIP_MASK GENMASK(2, 1)
+#define OV4689_TIMING_FLIP_ARRAY BIT(1)
+#define OV4689_TIMING_FLIP_DIGITAL BIT(2)
+#define OV4689_TIMING_FLIP_BOTH (OV4689_TIMING_FLIP_ARRAY |\
+ OV4689_TIMING_FLIP_DIGITAL)
+
#define OV4689_REG_TEST_PATTERN CCI_REG8(0x5040)
#define OV4689_TEST_PATTERN_ENABLE 0x80
#define OV4689_TEST_PATTERN_DISABLE 0x0
@@ -183,7 +191,6 @@ static const struct cci_reg_sequence ov4689_2688x1520_regs[] = {
{CCI_REG8(0x3811), 0x08}, /* H_WIN_OFF_L h_win_off[7:0] = 0x08*/
{CCI_REG8(0x3813), 0x04}, /* V_WIN_OFF_L v_win_off[7:0] = 0x04 */
{CCI_REG8(0x3819), 0x01}, /* VSYNC_END_L vsync_end_point[7:0] = 0x01 */
- {CCI_REG8(0x3821), 0x06}, /* TIMING_FORMAT2 array_h_mirror = 1, digital_h_mirror = 1 */
/* OTP control */
{CCI_REG8(0x3d85), 0x36}, /* OTP_REG85 OTP_power_up_load_setting_enable = 1,
@@ -607,6 +614,16 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
(ctrl->val + ov4689->cur_mode->width) /
OV4689_HTS_DIVIDER, &ret);
break;
+ case V4L2_CID_VFLIP:
+ cci_update_bits(regmap, OV4689_REG_TIMING_FORMAT1,
+ OV4689_TIMING_FLIP_MASK,
+ ctrl->val ? OV4689_TIMING_FLIP_BOTH : 0, &ret);
+ break;
+ case V4L2_CID_HFLIP:
+ cci_update_bits(regmap, OV4689_REG_TIMING_FORMAT2,
+ OV4689_TIMING_FLIP_MASK,
+ ctrl->val ? 0 : OV4689_TIMING_FLIP_BOTH, &ret);
+ break;
default:
dev_warn(dev, "%s Unhandled id:0x%x, val:0x%x\n",
__func__, ctrl->id, ctrl->val);
@@ -637,7 +654,7 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
handler = &ov4689->ctrl_handler;
mode = ov4689->cur_mode;
- ret = v4l2_ctrl_handler_init(handler, 10);
+ ret = v4l2_ctrl_handler_init(handler, 12);
if (ret)
return ret;
@@ -677,6 +694,9 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
ARRAY_SIZE(ov4689_test_pattern_menu) - 1,
0, 0, ov4689_test_pattern_menu);
+ v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_HFLIP, 0, 1, 1, 0);
+
if (handler->error) {
ret = handler->error;
dev_err(ov4689->dev, "Failed to init controls(%d)\n", ret);
--
2.44.0
The OV4689 sensor supports digital gain up to 16x. Implement
corresponding control in the driver. Default digital gain value is not
modified by this patch.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/ov4689.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index 60c60bbfe3bb..d6be0856afe1 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -35,6 +35,12 @@
#define OV4689_GAIN_STEP 1
#define OV4689_GAIN_DEFAULT 0x80
+#define OV4689_REG_DIG_GAIN CCI_REG16(0x352a)
+#define OV4689_DIG_GAIN_MIN 1
+#define OV4689_DIG_GAIN_MAX 0x7fff
+#define OV4689_DIG_GAIN_STEP 1
+#define OV4689_DIG_GAIN_DEFAULT 0x800
+
#define OV4689_REG_HTS CCI_REG16(0x380c)
#define OV4689_HTS_DIVIDER 4
#define OV4689_HTS_MAX 0x7fff
@@ -131,7 +137,6 @@ static const struct cci_reg_sequence ov4689_2688x1520_regs[] = {
/* AEC PK */
{CCI_REG8(0x3503), 0x04}, /* AEC_MANUAL gain_input_as_sensor_gain_format = 1 */
- {CCI_REG8(0x352a), 0x08}, /* DIG_GAIN_FRAC_LONG dig_gain_long[14:8] = 0x08 (2x) */
/* ADC and analog control*/
{CCI_REG8(0x3603), 0x40},
@@ -624,6 +629,9 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
OV4689_TIMING_FLIP_MASK,
ctrl->val ? 0 : OV4689_TIMING_FLIP_BOTH, &ret);
break;
+ case V4L2_CID_DIGITAL_GAIN:
+ cci_write(regmap, OV4689_REG_DIG_GAIN, ctrl->val, &ret);
+ break;
default:
dev_warn(dev, "%s Unhandled id:0x%x, val:0x%x\n",
__func__, ctrl->id, ctrl->val);
@@ -654,7 +662,7 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
handler = &ov4689->ctrl_handler;
mode = ov4689->cur_mode;
- ret = v4l2_ctrl_handler_init(handler, 12);
+ ret = v4l2_ctrl_handler_init(handler, 13);
if (ret)
return ret;
@@ -697,6 +705,10 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0);
v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_HFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_DIGITAL_GAIN,
+ OV4689_DIG_GAIN_MIN, OV4689_DIG_GAIN_MAX,
+ OV4689_DIG_GAIN_STEP, OV4689_DIG_GAIN_DEFAULT);
+
if (handler->error) {
ret = handler->error;
dev_err(ov4689->dev, "Failed to init controls(%d)\n", ret);
--
2.44.0
The OV4689 sensor has separate red and blue gain settings (up to 4x).
Implement appropriate controls in the driver. Default gain values
are not modified.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/ov4689.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index d6be0856afe1..28470627ec45 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -56,6 +56,13 @@
#define OV4689_TIMING_FLIP_BOTH (OV4689_TIMING_FLIP_ARRAY |\
OV4689_TIMING_FLIP_DIGITAL)
+#define OV4689_REG_WB_GAIN_RED CCI_REG16(0x500c)
+#define OV4689_REG_WB_GAIN_BLUE CCI_REG16(0x5010)
+#define OV4689_WB_GAIN_MIN 1
+#define OV4689_WB_GAIN_MAX 0xfff
+#define OV4689_WB_GAIN_STEP 1
+#define OV4689_WB_GAIN_DEFAULT 0x400
+
#define OV4689_REG_TEST_PATTERN CCI_REG8(0x5040)
#define OV4689_TEST_PATTERN_ENABLE 0x80
#define OV4689_TEST_PATTERN_DISABLE 0x0
@@ -632,6 +639,12 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_DIGITAL_GAIN:
cci_write(regmap, OV4689_REG_DIG_GAIN, ctrl->val, &ret);
break;
+ case V4L2_CID_RED_BALANCE:
+ cci_write(regmap, OV4689_REG_WB_GAIN_RED, ctrl->val, &ret);
+ break;
+ case V4L2_CID_BLUE_BALANCE:
+ cci_write(regmap, OV4689_REG_WB_GAIN_BLUE, ctrl->val, &ret);
+ break;
default:
dev_warn(dev, "%s Unhandled id:0x%x, val:0x%x\n",
__func__, ctrl->id, ctrl->val);
@@ -662,7 +675,7 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
handler = &ov4689->ctrl_handler;
mode = ov4689->cur_mode;
- ret = v4l2_ctrl_handler_init(handler, 13);
+ ret = v4l2_ctrl_handler_init(handler, 15);
if (ret)
return ret;
@@ -709,6 +722,14 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
OV4689_DIG_GAIN_MIN, OV4689_DIG_GAIN_MAX,
OV4689_DIG_GAIN_STEP, OV4689_DIG_GAIN_DEFAULT);
+ v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_RED_BALANCE,
+ OV4689_WB_GAIN_MIN, OV4689_WB_GAIN_MAX,
+ OV4689_WB_GAIN_STEP, OV4689_WB_GAIN_DEFAULT);
+
+ v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_BLUE_BALANCE,
+ OV4689_WB_GAIN_MIN, OV4689_WB_GAIN_MAX,
+ OV4689_WB_GAIN_STEP, OV4689_WB_GAIN_DEFAULT);
+
if (handler->error) {
ret = handler->error;
dev_err(ov4689->dev, "Failed to init controls(%d)\n", ret);
--
2.44.0
Pixel array dimensions and default crop size do not belong to the
ov4689_mode structure, since they are mode independent. Make them
defines instead.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Kieran Bingham <[email protected]>
---
drivers/media/i2c/ov4689.c | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index 28470627ec45..fe7d095dc51c 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -70,6 +70,11 @@
#define OV4689_LANES 4
#define OV4689_XVCLK_FREQ 24000000
+#define OV4689_PIXEL_ARRAY_WIDTH 2720
+#define OV4689_PIXEL_ARRAY_HEIGHT 1536
+#define OV4689_DUMMY_ROWS 8 /* 8 dummy rows on each side */
+#define OV4689_DUMMY_COLUMNS 16 /* 16 dummy columns on each side */
+
static const char *const ov4689_supply_names[] = {
"avdd", /* Analog power */
"dovdd", /* Digital I/O power */
@@ -90,10 +95,6 @@ struct ov4689_mode {
u32 vts_def;
u32 exp_def;
u32 pixel_rate;
- u32 sensor_width;
- u32 sensor_height;
- u32 crop_top;
- u32 crop_left;
const struct cci_reg_sequence *reg_list;
unsigned int num_regs;
};
@@ -254,10 +255,6 @@ static const struct ov4689_mode supported_modes[] = {
.id = OV4689_MODE_2688_1520,
.width = 2688,
.height = 1520,
- .sensor_width = 2720,
- .sensor_height = 1536,
- .crop_top = 8,
- .crop_left = 16,
.exp_def = 1536,
.hts_def = 10296,
.hts_min = 3432,
@@ -385,8 +382,6 @@ static int ov4689_get_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
{
- const struct ov4689_mode *mode = to_ov4689(sd)->cur_mode;
-
if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
return -EINVAL;
@@ -394,15 +389,17 @@ static int ov4689_get_selection(struct v4l2_subdev *sd,
case V4L2_SEL_TGT_CROP_BOUNDS:
sel->r.top = 0;
sel->r.left = 0;
- sel->r.width = mode->sensor_width;
- sel->r.height = mode->sensor_height;
+ sel->r.width = OV4689_PIXEL_ARRAY_WIDTH;
+ sel->r.height = OV4689_PIXEL_ARRAY_HEIGHT;
return 0;
case V4L2_SEL_TGT_CROP:
case V4L2_SEL_TGT_CROP_DEFAULT:
- sel->r.top = mode->crop_top;
- sel->r.left = mode->crop_left;
- sel->r.width = mode->width;
- sel->r.height = mode->height;
+ sel->r.top = OV4689_DUMMY_ROWS;
+ sel->r.left = OV4689_DUMMY_COLUMNS;
+ sel->r.width =
+ OV4689_PIXEL_ARRAY_WIDTH - 2 * OV4689_DUMMY_COLUMNS;
+ sel->r.height =
+ OV4689_PIXEL_ARRAY_HEIGHT - 2 * OV4689_DUMMY_ROWS;
return 0;
}
--
2.44.0
Set timing-related and BLC anchor registers via cci calls instead of
hardcoding them in the register table. This prepares the driver for
implementation of configurable analogue crop and binning. No
functional change intended.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/ov4689.c | 83 +++++++++++++++++++++++++++++++-------
1 file changed, 68 insertions(+), 15 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index fe7d095dc51c..4b099213dfea 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -41,6 +41,13 @@
#define OV4689_DIG_GAIN_STEP 1
#define OV4689_DIG_GAIN_DEFAULT 0x800
+#define OV4689_REG_H_CROP_START CCI_REG16(0x3800)
+#define OV4689_REG_V_CROP_START CCI_REG16(0x3802)
+#define OV4689_REG_H_CROP_END CCI_REG16(0x3804)
+#define OV4689_REG_V_CROP_END CCI_REG16(0x3806)
+#define OV4689_REG_H_OUTPUT_SIZE CCI_REG16(0x3808)
+#define OV4689_REG_V_OUTPUT_SIZE CCI_REG16(0x380a)
+
#define OV4689_REG_HTS CCI_REG16(0x380c)
#define OV4689_HTS_DIVIDER 4
#define OV4689_HTS_MAX 0x7fff
@@ -48,6 +55,9 @@
#define OV4689_REG_VTS CCI_REG16(0x380e)
#define OV4689_VTS_MAX 0x7fff
+#define OV4689_REG_H_WIN_OFF CCI_REG16(0x3810)
+#define OV4689_REG_V_WIN_OFF CCI_REG16(0x3812)
+
#define OV4689_REG_TIMING_FORMAT1 CCI_REG8(0x3820) /* Vertical */
#define OV4689_REG_TIMING_FORMAT2 CCI_REG8(0x3821) /* Horizontal */
#define OV4689_TIMING_FLIP_MASK GENMASK(2, 1)
@@ -56,6 +66,17 @@
#define OV4689_TIMING_FLIP_BOTH (OV4689_TIMING_FLIP_ARRAY |\
OV4689_TIMING_FLIP_DIGITAL)
+#define OV4689_REG_ANCHOR_LEFT_START CCI_REG16(0x4020)
+#define OV4689_ANCHOR_LEFT_START_DEF 576
+#define OV4689_REG_ANCHOR_LEFT_END CCI_REG16(0x4022)
+#define OV4689_ANCHOR_LEFT_END_DEF 831
+#define OV4689_REG_ANCHOR_RIGHT_START CCI_REG16(0x4024)
+#define OV4689_ANCHOR_RIGHT_START_DEF 1984
+#define OV4689_REG_ANCHOR_RIGHT_END CCI_REG16(0x4026)
+#define OV4689_ANCHOR_RIGHT_END_DEF 2239
+
+#define OV4689_REG_VFIFO_CTRL_01 CCI_REG8(0x4601)
+
#define OV4689_REG_WB_GAIN_RED CCI_REG16(0x500c)
#define OV4689_REG_WB_GAIN_BLUE CCI_REG16(0x5010)
#define OV4689_WB_GAIN_MIN 1
@@ -199,10 +220,6 @@ static const struct cci_reg_sequence ov4689_2688x1520_regs[] = {
{CCI_REG8(0x3798), 0x1b},
/* Timing control */
- {CCI_REG8(0x3801), 0x08}, /* H_CROP_START_L h_crop_start[7:0] = 0x08 */
- {CCI_REG8(0x3805), 0x97}, /* H_CROP_END_L h_crop_end[7:0] = 0x97 */
- {CCI_REG8(0x3811), 0x08}, /* H_WIN_OFF_L h_win_off[7:0] = 0x08*/
- {CCI_REG8(0x3813), 0x04}, /* V_WIN_OFF_L v_win_off[7:0] = 0x04 */
{CCI_REG8(0x3819), 0x01}, /* VSYNC_END_L vsync_end_point[7:0] = 0x01 */
/* OTP control */
@@ -218,22 +235,11 @@ static const struct cci_reg_sequence ov4689_2688x1520_regs[] = {
{CCI_REG8(0x401b), 0x00}, /* DEBUG_MODE */
{CCI_REG8(0x401d), 0x00}, /* DEBUG_MODE */
{CCI_REG8(0x401f), 0x00}, /* DEBUG_MODE */
- {CCI_REG8(0x4020), 0x00}, /* ANCHOR_LEFT_START_H anchor_left_start[11:8] = 0 */
- {CCI_REG8(0x4021), 0x10}, /* ANCHOR_LEFT_START_L anchor_left_start[7:0] = 0x10 */
- {CCI_REG8(0x4022), 0x07}, /* ANCHOR_LEFT_END_H anchor_left_end[11:8] = 0x07 */
- {CCI_REG8(0x4023), 0xcf}, /* ANCHOR_LEFT_END_L anchor_left_end[7:0] = 0xcf */
- {CCI_REG8(0x4024), 0x09}, /* ANCHOR_RIGHT_START_H anchor_right_start[11:8] = 0x09 */
- {CCI_REG8(0x4025), 0x60}, /* ANCHOR_RIGHT_START_L anchor_right_start[7:0] = 0x60 */
- {CCI_REG8(0x4026), 0x09}, /* ANCHOR_RIGHT_END_H anchor_right_end[11:8] = 0x09 */
- {CCI_REG8(0x4027), 0x6f}, /* ANCHOR_RIGHT_END_L anchor_right_end[7:0] = 0x6f */
/* ADC sync control */
{CCI_REG8(0x4500), 0x6c}, /* ADC_SYNC_CTRL */
{CCI_REG8(0x4503), 0x01}, /* ADC_SYNC_CTRL */
- /* VFIFO */
- {CCI_REG8(0x4601), 0xa7}, /* VFIFO_CTRL_01 r_vfifo_read_start[7:0] = 0xa7 */
-
/* Temperature monitor */
{CCI_REG8(0x4d00), 0x04}, /* TPM_CTRL_00 tmp_slope[15:8] = 0x04 */
{CCI_REG8(0x4d01), 0x42}, /* TPM_CTRL_01 tmp_slope[7:0] = 0x42 */
@@ -406,6 +412,41 @@ static int ov4689_get_selection(struct v4l2_subdev *sd,
return -EINVAL;
}
+static int ov4689_setup_timings(struct ov4689 *ov4689)
+{
+ const struct ov4689_mode *mode = ov4689->cur_mode;
+ struct regmap *rm = ov4689->regmap;
+ int ret = 0;
+
+ cci_write(rm, OV4689_REG_H_CROP_START, 8, &ret);
+ cci_write(rm, OV4689_REG_V_CROP_START, 8, &ret);
+ cci_write(rm, OV4689_REG_H_CROP_END, 2711, &ret);
+ cci_write(rm, OV4689_REG_V_CROP_END, 1531, &ret);
+
+ cci_write(rm, OV4689_REG_H_OUTPUT_SIZE, mode->width, &ret);
+ cci_write(rm, OV4689_REG_V_OUTPUT_SIZE, mode->height, &ret);
+
+ cci_write(rm, OV4689_REG_H_WIN_OFF, 8, &ret);
+ cci_write(rm, OV4689_REG_V_WIN_OFF, 4, &ret);
+
+ cci_write(rm, OV4689_REG_VFIFO_CTRL_01, 167, &ret);
+
+ return ret;
+}
+
+static int ov4689_setup_blc_anchors(struct ov4689 *ov4689)
+{
+ struct regmap *rm = ov4689->regmap;
+ int ret = 0;
+
+ cci_write(rm, OV4689_REG_ANCHOR_LEFT_START, 16, &ret);
+ cci_write(rm, OV4689_REG_ANCHOR_LEFT_END, 1999, &ret);
+ cci_write(rm, OV4689_REG_ANCHOR_RIGHT_START, 2400, &ret);
+ cci_write(rm, OV4689_REG_ANCHOR_RIGHT_END, 2415, &ret);
+
+ return ret;
+}
+
static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
{
struct ov4689 *ov4689 = to_ov4689(sd);
@@ -429,6 +470,18 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
goto unlock_and_return;
}
+ ret = ov4689_setup_timings(ov4689);
+ if (ret) {
+ pm_runtime_put(dev);
+ goto unlock_and_return;
+ }
+
+ ret = ov4689_setup_blc_anchors(ov4689);
+ if (ret) {
+ pm_runtime_put(dev);
+ goto unlock_and_return;
+ }
+
ret = __v4l2_ctrl_handler_setup(&ov4689->ctrl_handler);
if (ret) {
pm_runtime_put(dev);
--
2.44.0
Implement configurable analogue crop via .set_selection call.
ov4689_init_cfg is modified to initialize default subdev selection.
Offsets are aligned to 2 to preserve Bayer order, selection width is
aligned to 4 and height to 2 to meet hardware requirements.
Experimentally discovered values of the cropping-related registers and
vfifo_read_start for various output sizes are used. Default BLC anchor
positions are used for the default analogue crop, scaling down
proportionally for the smaller crop sizes.
When analogue crop is adjusted, several consequential actions take
place: the output format is reset, exposure/vblank/hblank control
ranges and default values are adjusted accordingly. Additionally,
ov4689_set_ctrl utilizes pad crop instead of cur_mode width and
height for HTS and VTS calculation. Also, ov4689_enum_frame_sizes is
modified to report crop size as available frame size.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Kieran Bingham <[email protected]>
---
drivers/media/i2c/ov4689.c | 276 ++++++++++++++++++++++++++++---------
1 file changed, 212 insertions(+), 64 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index 4b099213dfea..f25bc260dde2 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -45,8 +45,12 @@
#define OV4689_REG_V_CROP_START CCI_REG16(0x3802)
#define OV4689_REG_H_CROP_END CCI_REG16(0x3804)
#define OV4689_REG_V_CROP_END CCI_REG16(0x3806)
+
#define OV4689_REG_H_OUTPUT_SIZE CCI_REG16(0x3808)
+#define OV4689_H_OUTPUT_SIZE_DEFAULT 2688
+
#define OV4689_REG_V_OUTPUT_SIZE CCI_REG16(0x380a)
+#define OV4689_V_OUTPUT_SIZE_DEFAULT 1520
#define OV4689_REG_HTS CCI_REG16(0x380c)
#define OV4689_HTS_DIVIDER 4
@@ -96,6 +100,19 @@
#define OV4689_DUMMY_ROWS 8 /* 8 dummy rows on each side */
#define OV4689_DUMMY_COLUMNS 16 /* 16 dummy columns on each side */
+/*
+ * These values are not hardware limits, but rather the minimums that
+ * the driver has been tested to.
+ */
+#define OV4689_H_CROP_MIN 128
+#define OV4689_V_CROP_MIN 128
+
+/*
+ * Minimum working vertical blanking value. Found experimentally at
+ * minimum HTS values.
+ */
+#define OV4689_VBLANK_MIN 31
+
static const char *const ov4689_supply_names[] = {
"avdd", /* Analog power */
"dovdd", /* Digital I/O power */
@@ -134,7 +151,7 @@ struct ov4689 {
u32 clock_rate;
struct v4l2_ctrl_handler ctrl_handler;
- struct v4l2_ctrl *exposure;
+ struct v4l2_ctrl *exposure, *hblank, *vblank;
const struct ov4689_mode *cur_mode;
};
@@ -320,24 +337,27 @@ static const struct ov4689_gain_range ov4689_gain_ranges[] = {
},
};
-static void ov4689_fill_fmt(const struct ov4689_mode *mode,
- struct v4l2_mbus_framefmt *fmt)
-{
- fmt->code = MEDIA_BUS_FMT_SBGGR10_1X10;
- fmt->width = mode->width;
- fmt->height = mode->height;
- fmt->field = V4L2_FIELD_NONE;
-}
-
static int ov4689_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_format *fmt)
{
- struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
- struct ov4689 *ov4689 = to_ov4689(sd);
+ struct v4l2_mbus_framefmt *format;
+ struct v4l2_rect *crop;
+
+ crop = v4l2_subdev_state_get_crop(sd_state, fmt->pad);
+ format = v4l2_subdev_state_get_format(sd_state, fmt->pad);
+
+ format->width = crop->width;
+ format->height = crop->height;
- /* only one mode supported for now */
- ov4689_fill_fmt(ov4689->cur_mode, mbus_fmt);
+ format->code = MEDIA_BUS_FMT_SBGGR10_1X10;
+ format->field = V4L2_FIELD_NONE;
+ format->colorspace = V4L2_COLORSPACE_RAW;
+ format->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+ format->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+ format->xfer_func = V4L2_XFER_FUNC_NONE;
+
+ fmt->format = *format;
return 0;
}
@@ -357,16 +377,20 @@ static int ov4689_enum_frame_sizes(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_frame_size_enum *fse)
{
- if (fse->index >= ARRAY_SIZE(supported_modes))
+ const struct v4l2_rect *crop;
+
+ if (fse->index >= 1)
return -EINVAL;
if (fse->code != MEDIA_BUS_FMT_SBGGR10_1X10)
return -EINVAL;
- fse->min_width = supported_modes[fse->index].width;
- fse->max_width = supported_modes[fse->index].width;
- fse->max_height = supported_modes[fse->index].height;
- fse->min_height = supported_modes[fse->index].height;
+ crop = v4l2_subdev_state_get_crop(sd_state, 0);
+
+ fse->min_width = crop->width;
+ fse->max_width = crop->width;
+ fse->max_height = crop->height;
+ fse->min_height = crop->height;
return 0;
}
@@ -388,20 +412,14 @@ static int ov4689_get_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
{
- if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
- return -EINVAL;
-
switch (sel->target) {
- case V4L2_SEL_TGT_CROP_BOUNDS:
- sel->r.top = 0;
- sel->r.left = 0;
- sel->r.width = OV4689_PIXEL_ARRAY_WIDTH;
- sel->r.height = OV4689_PIXEL_ARRAY_HEIGHT;
- return 0;
case V4L2_SEL_TGT_CROP:
+ sel->r = *v4l2_subdev_state_get_crop(state, sel->pad);
+ return 0;
+ case V4L2_SEL_TGT_CROP_BOUNDS:
case V4L2_SEL_TGT_CROP_DEFAULT:
- sel->r.top = OV4689_DUMMY_ROWS;
sel->r.left = OV4689_DUMMY_COLUMNS;
+ sel->r.top = OV4689_DUMMY_ROWS;
sel->r.width =
OV4689_PIXEL_ARRAY_WIDTH - 2 * OV4689_DUMMY_COLUMNS;
sel->r.height =
@@ -412,37 +430,141 @@ static int ov4689_get_selection(struct v4l2_subdev *sd,
return -EINVAL;
}
-static int ov4689_setup_timings(struct ov4689 *ov4689)
+/*
+ * Minimum working HTS value for given output width (found
+ * experimentally).
+ */
+static unsigned int ov4689_hts_min(unsigned int width)
+{
+ return max_t(unsigned int, 3156, 224 + width * 19 / 16);
+}
+
+static void ov4689_update_ctrl_ranges(struct ov4689 *ov4689,
+ struct v4l2_rect *crop)
+{
+ struct v4l2_ctrl *exposure = ov4689->exposure;
+ struct v4l2_ctrl *vblank = ov4689->vblank;
+ struct v4l2_ctrl *hblank = ov4689->hblank;
+ s64 def_val, min_val, max_val;
+
+ min_val = ov4689_hts_min(crop->width) - crop->width;
+ max_val = OV4689_HTS_MAX - crop->width;
+ def_val = clamp_t(s64, hblank->default_value, min_val, max_val);
+ __v4l2_ctrl_modify_range(hblank, min_val, max_val, hblank->step,
+ def_val);
+
+ min_val = OV4689_VBLANK_MIN;
+ max_val = OV4689_HTS_MAX - crop->width;
+ def_val = clamp_t(s64, vblank->default_value, min_val, max_val);
+ __v4l2_ctrl_modify_range(vblank, min_val, max_val, vblank->step,
+ def_val);
+
+ min_val = exposure->minimum;
+ max_val = crop->height + vblank->val - 4;
+ def_val = clamp_t(s64, exposure->default_value, min_val, max_val);
+ __v4l2_ctrl_modify_range(exposure, min_val, max_val, exposure->step,
+ def_val);
+}
+
+static int ov4689_set_selection(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ struct v4l2_subdev_selection *sel)
{
- const struct ov4689_mode *mode = ov4689->cur_mode;
+ struct ov4689 *ov4689 = to_ov4689(sd);
+ struct v4l2_mbus_framefmt *format;
+ struct v4l2_rect *crop;
+ struct v4l2_rect rect;
+
+ if (sel->target != V4L2_SEL_TGT_CROP)
+ return -EINVAL;
+
+ rect.left = clamp(ALIGN(sel->r.left, 2), OV4689_DUMMY_COLUMNS,
+ OV4689_PIXEL_ARRAY_WIDTH);
+ rect.top = clamp(ALIGN(sel->r.top, 2), OV4689_DUMMY_ROWS,
+ OV4689_PIXEL_ARRAY_HEIGHT);
+
+ rect.width = clamp_t(unsigned int, ALIGN(sel->r.width, 4),
+ OV4689_H_CROP_MIN, OV4689_PIXEL_ARRAY_WIDTH -
+ 2 * OV4689_DUMMY_COLUMNS);
+ rect.height = clamp_t(unsigned int, ALIGN(sel->r.height, 2),
+ OV4689_V_CROP_MIN, OV4689_PIXEL_ARRAY_HEIGHT -
+ 2 * OV4689_DUMMY_ROWS);
+
+ crop = v4l2_subdev_state_get_crop(state, sel->pad);
+
+ if (rect.width != crop->width || rect.height != crop->height) {
+ /*
+ * Reset the output image size if the crop rectangle size has
+ * been modified.
+ */
+ format = v4l2_subdev_state_get_format(state, sel->pad);
+ format->width = rect.width;
+ format->height = rect.height;
+
+ if (sel->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+ ov4689_update_ctrl_ranges(ov4689, &rect);
+ }
+
+ *crop = rect;
+ sel->r = rect;
+
+ return 0;
+}
+
+static int ov4689_setup_timings(struct ov4689 *ov4689,
+ struct v4l2_subdev_state *state)
+{
+ const struct v4l2_mbus_framefmt *format;
struct regmap *rm = ov4689->regmap;
+ const struct v4l2_rect *crop;
int ret = 0;
- cci_write(rm, OV4689_REG_H_CROP_START, 8, &ret);
- cci_write(rm, OV4689_REG_V_CROP_START, 8, &ret);
- cci_write(rm, OV4689_REG_H_CROP_END, 2711, &ret);
- cci_write(rm, OV4689_REG_V_CROP_END, 1531, &ret);
+ format = v4l2_subdev_state_get_format(state, 0);
+ crop = v4l2_subdev_state_get_crop(state, 0);
+
+ cci_write(rm, OV4689_REG_H_CROP_START, crop->left, &ret);
+ cci_write(rm, OV4689_REG_V_CROP_START, crop->top, &ret);
+ cci_write(rm, OV4689_REG_H_CROP_END, crop->left + crop->width + 1, &ret);
+ cci_write(rm, OV4689_REG_V_CROP_END, crop->top + crop->height + 1, &ret);
- cci_write(rm, OV4689_REG_H_OUTPUT_SIZE, mode->width, &ret);
- cci_write(rm, OV4689_REG_V_OUTPUT_SIZE, mode->height, &ret);
+ cci_write(rm, OV4689_REG_H_OUTPUT_SIZE, format->width, &ret);
+ cci_write(rm, OV4689_REG_V_OUTPUT_SIZE, format->height, &ret);
- cci_write(rm, OV4689_REG_H_WIN_OFF, 8, &ret);
- cci_write(rm, OV4689_REG_V_WIN_OFF, 4, &ret);
+ cci_write(rm, OV4689_REG_H_WIN_OFF, 0, &ret);
+ cci_write(rm, OV4689_REG_V_WIN_OFF, 0, &ret);
- cci_write(rm, OV4689_REG_VFIFO_CTRL_01, 167, &ret);
+ /*
+ * Maximum working value of vfifo_read_start for given output
+ * width (found experimentally).
+ */
+ cci_write(rm, OV4689_REG_VFIFO_CTRL_01, format->width / 16 - 1, &ret);
return ret;
}
-static int ov4689_setup_blc_anchors(struct ov4689 *ov4689)
+/*
+ * Setup black level compensation anchors. For the default frame width
+ * default anchors positions are used. For smaller crop sizes they are
+ * scaled accordingly.
+ */
+static int ov4689_setup_blc_anchors(struct ov4689 *ov4689,
+ struct v4l2_subdev_state *state)
{
+ unsigned int width_def = OV4689_H_OUTPUT_SIZE_DEFAULT;
struct regmap *rm = ov4689->regmap;
+ const struct v4l2_rect *crop;
int ret = 0;
- cci_write(rm, OV4689_REG_ANCHOR_LEFT_START, 16, &ret);
- cci_write(rm, OV4689_REG_ANCHOR_LEFT_END, 1999, &ret);
- cci_write(rm, OV4689_REG_ANCHOR_RIGHT_START, 2400, &ret);
- cci_write(rm, OV4689_REG_ANCHOR_RIGHT_END, 2415, &ret);
+ crop = v4l2_subdev_state_get_crop(state, 0);
+
+ cci_write(rm, OV4689_REG_ANCHOR_LEFT_START,
+ OV4689_ANCHOR_LEFT_START_DEF * crop->width / width_def, &ret);
+ cci_write(rm, OV4689_REG_ANCHOR_LEFT_END,
+ OV4689_ANCHOR_LEFT_END_DEF * crop->width / width_def, &ret);
+ cci_write(rm, OV4689_REG_ANCHOR_RIGHT_START,
+ OV4689_ANCHOR_RIGHT_START_DEF * crop->width / width_def, &ret);
+ cci_write(rm, OV4689_REG_ANCHOR_RIGHT_END,
+ OV4689_ANCHOR_RIGHT_END_DEF * crop->width / width_def, &ret);
return ret;
}
@@ -470,13 +592,13 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
goto unlock_and_return;
}
- ret = ov4689_setup_timings(ov4689);
+ ret = ov4689_setup_timings(ov4689, sd_state);
if (ret) {
pm_runtime_put(dev);
goto unlock_and_return;
}
- ret = ov4689_setup_blc_anchors(ov4689);
+ ret = ov4689_setup_blc_anchors(ov4689, sd_state);
if (ret) {
pm_runtime_put(dev);
goto unlock_and_return;
@@ -568,10 +690,25 @@ static int __maybe_unused ov4689_power_off(struct device *dev)
static int ov4689_init_state(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state)
{
- struct v4l2_mbus_framefmt *fmt =
- v4l2_subdev_state_get_format(sd_state, 0);
+ u32 width_def = OV4689_H_OUTPUT_SIZE_DEFAULT;
+ u32 height_def = OV4689_V_OUTPUT_SIZE_DEFAULT;
+
+ struct v4l2_subdev_selection sel = {
+ .target = V4L2_SEL_TGT_CROP,
+ .r.left = OV4689_DUMMY_COLUMNS,
+ .r.top = OV4689_DUMMY_ROWS,
+ .r.width = width_def,
+ .r.height = height_def,
+ };
+ struct v4l2_subdev_format format = {
+ .format = {
+ .width = width_def,
+ .height = height_def,
+ },
+ };
- ov4689_fill_fmt(&supported_modes[OV4689_MODE_2688_1520], fmt);
+ ov4689_set_selection(sd, sd_state, &sel);
+ ov4689_set_fmt(sd, sd_state, &format);
return 0;
}
@@ -590,6 +727,7 @@ static const struct v4l2_subdev_pad_ops ov4689_pad_ops = {
.get_fmt = v4l2_subdev_get_fmt,
.set_fmt = ov4689_set_fmt,
.get_selection = ov4689_get_selection,
+ .set_selection = ov4689_set_selection,
};
static const struct v4l2_subdev_internal_ops ov4689_internal_ops = {
@@ -635,20 +773,28 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
struct ov4689 *ov4689 =
container_of(ctrl->handler, struct ov4689, ctrl_handler);
struct regmap *regmap = ov4689->regmap;
+ struct v4l2_subdev_state *sd_state;
struct device *dev = ov4689->dev;
+ struct v4l2_rect *crop;
+ s64 max_expo, def_expo;
int sensor_gain;
- s64 max_expo;
int ret = 0;
+ sd_state = v4l2_subdev_get_locked_active_state(&ov4689->subdev);
+ crop = v4l2_subdev_state_get_crop(sd_state, 0);
+
/* Propagate change of current control to all related controls */
switch (ctrl->id) {
case V4L2_CID_VBLANK:
/* Update max exposure while meeting expected vblanking */
- max_expo = ov4689->cur_mode->height + ctrl->val - 4;
- __v4l2_ctrl_modify_range(ov4689->exposure,
- ov4689->exposure->minimum, max_expo,
- ov4689->exposure->step,
- ov4689->exposure->default_value);
+ max_expo = crop->height + ctrl->val - 4;
+ def_expo = clamp_t(s64, ov4689->exposure->default_value,
+ ov4689->exposure->minimum, max_expo);
+
+ ret = __v4l2_ctrl_modify_range(ov4689->exposure,
+ ov4689->exposure->minimum,
+ max_expo, ov4689->exposure->step,
+ def_expo);
break;
}
@@ -666,14 +812,14 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
break;
case V4L2_CID_VBLANK:
cci_write(regmap, OV4689_REG_VTS,
- ctrl->val + ov4689->cur_mode->height, &ret);
+ ctrl->val + crop->height, &ret);
break;
case V4L2_CID_TEST_PATTERN:
ret = ov4689_enable_test_pattern(ov4689, ctrl->val);
break;
case V4L2_CID_HBLANK:
cci_write(regmap, OV4689_REG_HTS,
- (ctrl->val + ov4689->cur_mode->width) /
+ (ctrl->val + crop->width) /
OV4689_HTS_DIVIDER, &ret);
break;
case V4L2_CID_VFLIP:
@@ -739,14 +885,16 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
hblank_def = mode->hts_def - mode->width;
hblank_min = mode->hts_min - mode->width;
- v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_HBLANK,
- hblank_min, OV4689_HTS_MAX - mode->width,
- OV4689_HTS_DIVIDER, hblank_def);
+ ov4689->hblank = v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops,
+ V4L2_CID_HBLANK, hblank_min,
+ OV4689_HTS_MAX - mode->width,
+ OV4689_HTS_DIVIDER, hblank_def);
vblank_def = mode->vts_def - mode->height;
- v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_VBLANK,
- vblank_def, OV4689_VTS_MAX - mode->height, 1,
- vblank_def);
+ ov4689->vblank = v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops,
+ V4L2_CID_VBLANK, OV4689_VBLANK_MIN,
+ OV4689_VTS_MAX - mode->height, 1,
+ vblank_def);
exposure_max = mode->vts_def - 4;
ov4689->exposure =
--
2.44.0
With the output frame size now controlled by selection rather than
cur_mode, this commit relocates pixel rate and default VTS to
defines. Consequently, it removes struct ov4689_mode and the cur_mode
field from struct ov4689.
Signed-off-by: Mikhail Rudenko <[email protected]>
---
drivers/media/i2c/ov4689.c | 70 +++++++++-----------------------------
1 file changed, 17 insertions(+), 53 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index f25bc260dde2..f43be255234b 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -57,6 +57,8 @@
#define OV4689_HTS_MAX 0x7fff
#define OV4689_REG_VTS CCI_REG16(0x380e)
+/* Default VTS corresponds to 30 fps at default crop and minimal HTS */
+#define OV4689_VTS_DEF 4683
#define OV4689_VTS_MAX 0x7fff
#define OV4689_REG_H_WIN_OFF CCI_REG16(0x3810)
@@ -94,6 +96,7 @@
#define OV4689_LANES 4
#define OV4689_XVCLK_FREQ 24000000
+#define OV4689_PIXEL_RATE 480000000
#define OV4689_PIXEL_ARRAY_WIDTH 2720
#define OV4689_PIXEL_ARRAY_HEIGHT 1536
@@ -119,24 +122,6 @@ static const char *const ov4689_supply_names[] = {
"dvdd", /* Digital core power */
};
-enum ov4689_mode_id {
- OV4689_MODE_2688_1520 = 0,
- OV4689_NUM_MODES,
-};
-
-struct ov4689_mode {
- enum ov4689_mode_id id;
- u32 width;
- u32 height;
- u32 hts_def;
- u32 hts_min;
- u32 vts_def;
- u32 exp_def;
- u32 pixel_rate;
- const struct cci_reg_sequence *reg_list;
- unsigned int num_regs;
-};
-
struct ov4689 {
struct device *dev;
struct regmap *regmap;
@@ -152,8 +137,6 @@ struct ov4689 {
struct v4l2_ctrl_handler ctrl_handler;
struct v4l2_ctrl *exposure, *hblank, *vblank;
-
- const struct ov4689_mode *cur_mode;
};
#define to_ov4689(sd) container_of(sd, struct ov4689, subdev)
@@ -172,7 +155,7 @@ struct ov4689_gain_range {
* max_framerate 90fps
* mipi_datarate per lane 1008Mbps
*/
-static const struct cci_reg_sequence ov4689_2688x1520_regs[] = {
+static const struct cci_reg_sequence ov4689_common_regs[] = {
/* System control*/
{CCI_REG8(0x0103), 0x01}, /* SC_CTRL0103 software_reset = 1 */
{CCI_REG8(0x3000), 0x20}, /* SC_CMMN_PAD_OEN0 FSIN_output_enable = 1 */
@@ -273,21 +256,6 @@ static const struct cci_reg_sequence ov4689_2688x1520_regs[] = {
{CCI_REG8(0x5503), 0x0f}, /* OTP_DPC_END_L otp_end_address[7:0] = 0x0f */
};
-static const struct ov4689_mode supported_modes[] = {
- {
- .id = OV4689_MODE_2688_1520,
- .width = 2688,
- .height = 1520,
- .exp_def = 1536,
- .hts_def = 10296,
- .hts_min = 3432,
- .vts_def = 1554,
- .pixel_rate = 480000000,
- .reg_list = ov4689_2688x1520_regs,
- .num_regs = ARRAY_SIZE(ov4689_2688x1520_regs),
- },
-};
-
static const u64 link_freq_menu_items[] = { 504000000 };
static const char *const ov4689_test_pattern_menu[] = {
@@ -584,8 +552,8 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
goto unlock_and_return;
ret = cci_multi_reg_write(ov4689->regmap,
- ov4689->cur_mode->reg_list,
- ov4689->cur_mode->num_regs,
+ ov4689_common_regs,
+ ARRAY_SIZE(ov4689_common_regs),
NULL);
if (ret) {
pm_runtime_put(dev);
@@ -863,14 +831,12 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
struct i2c_client *client = v4l2_get_subdevdata(&ov4689->subdev);
struct v4l2_fwnode_device_properties props;
struct v4l2_ctrl_handler *handler;
- const struct ov4689_mode *mode;
s64 exposure_max, vblank_def;
- s64 hblank_def, hblank_min;
struct v4l2_ctrl *ctrl;
+ s64 hblank_def;
int ret;
handler = &ov4689->ctrl_handler;
- mode = ov4689->cur_mode;
ret = v4l2_ctrl_handler_init(handler, 15);
if (ret)
return ret;
@@ -881,26 +847,26 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
v4l2_ctrl_new_std(handler, NULL, V4L2_CID_PIXEL_RATE, 0,
- mode->pixel_rate, 1, mode->pixel_rate);
+ OV4689_PIXEL_RATE, 1, OV4689_PIXEL_RATE);
- hblank_def = mode->hts_def - mode->width;
- hblank_min = mode->hts_min - mode->width;
+ hblank_def = ov4689_hts_min(OV4689_H_OUTPUT_SIZE_DEFAULT) -
+ OV4689_H_OUTPUT_SIZE_DEFAULT;
ov4689->hblank = v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops,
- V4L2_CID_HBLANK, hblank_min,
- OV4689_HTS_MAX - mode->width,
+ V4L2_CID_HBLANK, hblank_def,
+ OV4689_HTS_MAX - OV4689_H_OUTPUT_SIZE_DEFAULT,
OV4689_HTS_DIVIDER, hblank_def);
- vblank_def = mode->vts_def - mode->height;
+ vblank_def = OV4689_VTS_DEF - OV4689_V_OUTPUT_SIZE_DEFAULT;
ov4689->vblank = v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops,
V4L2_CID_VBLANK, OV4689_VBLANK_MIN,
- OV4689_VTS_MAX - mode->height, 1,
- vblank_def);
+ OV4689_VTS_MAX - OV4689_V_OUTPUT_SIZE_DEFAULT,
+ 1, vblank_def);
- exposure_max = mode->vts_def - 4;
+ exposure_max = OV4689_VTS_DEF - 4;
ov4689->exposure =
v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_EXPOSURE,
OV4689_EXPOSURE_MIN, exposure_max,
- OV4689_EXPOSURE_STEP, mode->exp_def);
+ OV4689_EXPOSURE_STEP, exposure_max);
v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_ANALOGUE_GAIN,
ov4689_gain_ranges[0].logical_min,
@@ -1055,8 +1021,6 @@ static int ov4689_probe(struct i2c_client *client)
ov4689->dev = dev;
- ov4689->cur_mode = &supported_modes[OV4689_MODE_2688_1520];
-
ov4689->xvclk = devm_clk_get_optional(dev, NULL);
if (IS_ERR(ov4689->xvclk))
return dev_err_probe(dev, PTR_ERR(ov4689->xvclk),
--
2.44.0
Many values in the register table are actually power-on
defaults. Remove those and also unused HDR exposures and gains.
Annotate the remaining values using the publicly available datasheet
to facilitate further development. No functional change intended.
Signed-off-by: Mikhail Rudenko <[email protected]>
Acked-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/ov4689.c | 203 +++++++++++++++++++++----------------
1 file changed, 118 insertions(+), 85 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index 403091651885..5a509e908e3c 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -3,7 +3,7 @@
* ov4689 driver
*
* Copyright (C) 2017 Fuzhou Rockchip Electronics Co., Ltd.
- * Copyright (C) 2022 Mikhail Rudenko
+ * Copyright (C) 2022, 2024 Mikhail Rudenko
*/
#include <linux/clk.h>
@@ -123,90 +123,123 @@ struct ov4689_gain_range {
* mipi_datarate per lane 1008Mbps
*/
static const struct regval ov4689_2688x1520_regs[] = {
- {0x0103, 0x01}, {0x3638, 0x00}, {0x0300, 0x00},
- {0x0302, 0x2a}, {0x0303, 0x00}, {0x0304, 0x03},
- {0x030b, 0x00}, {0x030d, 0x1e}, {0x030e, 0x04},
- {0x030f, 0x01}, {0x0312, 0x01}, {0x031e, 0x00},
- {0x3000, 0x20}, {0x3002, 0x00}, {0x3018, 0x72},
- {0x3020, 0x93}, {0x3021, 0x03}, {0x3022, 0x01},
- {0x3031, 0x0a}, {0x303f, 0x0c}, {0x3305, 0xf1},
- {0x3307, 0x04}, {0x3309, 0x29}, {0x3500, 0x00},
- {0x3501, 0x60}, {0x3502, 0x00}, {0x3503, 0x04},
- {0x3504, 0x00}, {0x3505, 0x00}, {0x3506, 0x00},
- {0x3507, 0x00}, {0x3508, 0x00}, {0x3509, 0x80},
- {0x350a, 0x00}, {0x350b, 0x00}, {0x350c, 0x00},
- {0x350d, 0x00}, {0x350e, 0x00}, {0x350f, 0x80},
- {0x3510, 0x00}, {0x3511, 0x00}, {0x3512, 0x00},
- {0x3513, 0x00}, {0x3514, 0x00}, {0x3515, 0x80},
- {0x3516, 0x00}, {0x3517, 0x00}, {0x3518, 0x00},
- {0x3519, 0x00}, {0x351a, 0x00}, {0x351b, 0x80},
- {0x351c, 0x00}, {0x351d, 0x00}, {0x351e, 0x00},
- {0x351f, 0x00}, {0x3520, 0x00}, {0x3521, 0x80},
- {0x3522, 0x08}, {0x3524, 0x08}, {0x3526, 0x08},
- {0x3528, 0x08}, {0x352a, 0x08}, {0x3602, 0x00},
- {0x3603, 0x40}, {0x3604, 0x02}, {0x3605, 0x00},
- {0x3606, 0x00}, {0x3607, 0x00}, {0x3609, 0x12},
- {0x360a, 0x40}, {0x360c, 0x08}, {0x360f, 0xe5},
- {0x3608, 0x8f}, {0x3611, 0x00}, {0x3613, 0xf7},
- {0x3616, 0x58}, {0x3619, 0x99}, {0x361b, 0x60},
- {0x361c, 0x7a}, {0x361e, 0x79}, {0x361f, 0x02},
- {0x3632, 0x00}, {0x3633, 0x10}, {0x3634, 0x10},
- {0x3635, 0x10}, {0x3636, 0x15}, {0x3646, 0x86},
- {0x364a, 0x0b}, {0x3700, 0x17}, {0x3701, 0x22},
- {0x3703, 0x10}, {0x370a, 0x37}, {0x3705, 0x00},
- {0x3706, 0x63}, {0x3709, 0x3c}, {0x370b, 0x01},
- {0x370c, 0x30}, {0x3710, 0x24}, {0x3711, 0x0c},
- {0x3716, 0x00}, {0x3720, 0x28}, {0x3729, 0x7b},
- {0x372a, 0x84}, {0x372b, 0xbd}, {0x372c, 0xbc},
- {0x372e, 0x52}, {0x373c, 0x0e}, {0x373e, 0x33},
- {0x3743, 0x10}, {0x3744, 0x88}, {0x3745, 0xc0},
- {0x374a, 0x43}, {0x374c, 0x00}, {0x374e, 0x23},
- {0x3751, 0x7b}, {0x3752, 0x84}, {0x3753, 0xbd},
- {0x3754, 0xbc}, {0x3756, 0x52}, {0x375c, 0x00},
- {0x3760, 0x00}, {0x3761, 0x00}, {0x3762, 0x00},
- {0x3763, 0x00}, {0x3764, 0x00}, {0x3767, 0x04},
- {0x3768, 0x04}, {0x3769, 0x08}, {0x376a, 0x08},
- {0x376b, 0x20}, {0x376c, 0x00}, {0x376d, 0x00},
- {0x376e, 0x00}, {0x3773, 0x00}, {0x3774, 0x51},
- {0x3776, 0xbd}, {0x3777, 0xbd}, {0x3781, 0x18},
- {0x3783, 0x25}, {0x3798, 0x1b}, {0x3800, 0x00},
- {0x3801, 0x08}, {0x3802, 0x00}, {0x3803, 0x04},
- {0x3804, 0x0a}, {0x3805, 0x97}, {0x3806, 0x05},
- {0x3807, 0xfb}, {0x3808, 0x0a}, {0x3809, 0x80},
- {0x380a, 0x05}, {0x380b, 0xf0}, {0x380c, 0x0a},
- {0x380d, 0x0e}, {0x380e, 0x06}, {0x380f, 0x12},
- {0x3810, 0x00}, {0x3811, 0x08}, {0x3812, 0x00},
- {0x3813, 0x04}, {0x3814, 0x01}, {0x3815, 0x01},
- {0x3819, 0x01}, {0x3820, 0x00}, {0x3821, 0x06},
- {0x3829, 0x00}, {0x382a, 0x01}, {0x382b, 0x01},
- {0x382d, 0x7f}, {0x3830, 0x04}, {0x3836, 0x01},
- {0x3837, 0x00}, {0x3841, 0x02}, {0x3846, 0x08},
- {0x3847, 0x07}, {0x3d85, 0x36}, {0x3d8c, 0x71},
- {0x3d8d, 0xcb}, {0x3f0a, 0x00}, {0x4000, 0xf1},
- {0x4001, 0x40}, {0x4002, 0x04}, {0x4003, 0x14},
- {0x400e, 0x00}, {0x4011, 0x00}, {0x401a, 0x00},
- {0x401b, 0x00}, {0x401c, 0x00}, {0x401d, 0x00},
- {0x401f, 0x00}, {0x4020, 0x00}, {0x4021, 0x10},
- {0x4022, 0x07}, {0x4023, 0xcf}, {0x4024, 0x09},
- {0x4025, 0x60}, {0x4026, 0x09}, {0x4027, 0x6f},
- {0x4028, 0x00}, {0x4029, 0x02}, {0x402a, 0x06},
- {0x402b, 0x04}, {0x402c, 0x02}, {0x402d, 0x02},
- {0x402e, 0x0e}, {0x402f, 0x04}, {0x4302, 0xff},
- {0x4303, 0xff}, {0x4304, 0x00}, {0x4305, 0x00},
- {0x4306, 0x00}, {0x4308, 0x02}, {0x4500, 0x6c},
- {0x4501, 0xc4}, {0x4502, 0x40}, {0x4503, 0x01},
- {0x4601, 0xa7}, {0x4800, 0x04}, {0x4813, 0x08},
- {0x481f, 0x40}, {0x4829, 0x78}, {0x4837, 0x10},
- {0x4b00, 0x2a}, {0x4b0d, 0x00}, {0x4d00, 0x04},
- {0x4d01, 0x42}, {0x4d02, 0xd1}, {0x4d03, 0x93},
- {0x4d04, 0xf5}, {0x4d05, 0xc1}, {0x5000, 0xf3},
- {0x5001, 0x11}, {0x5004, 0x00}, {0x500a, 0x00},
- {0x500b, 0x00}, {0x5032, 0x00}, {0x5040, 0x00},
- {0x5050, 0x0c}, {0x5500, 0x00}, {0x5501, 0x10},
- {0x5502, 0x01}, {0x5503, 0x0f}, {0x8000, 0x00},
- {0x8001, 0x00}, {0x8002, 0x00}, {0x8003, 0x00},
- {0x8004, 0x00}, {0x8005, 0x00}, {0x8006, 0x00},
- {0x8007, 0x00}, {0x8008, 0x00}, {0x3638, 0x00},
+ /* System control*/
+ {0x0103, 0x01}, /* SC_CTRL0103 software_reset = 1 */
+ {0x3000, 0x20}, /* SC_CMMN_PAD_OEN0 FSIN_output_enable = 1 */
+ {0x3021, 0x03}, /*
+ * SC_CMMN_MISC_CTRL fst_stby_ctr = 0,
+ * sleep_no_latch_enable = 0
+ */
+
+ /* AEC PK */
+ {0x3503, 0x04}, /* AEC_MANUAL gain_input_as_sensor_gain_format = 1 */
+ {0x352a, 0x08}, /* DIG_GAIN_FRAC_LONG dig_gain_long[14:8] = 0x08 (2x) */
+
+ /* ADC and analog control*/
+ {0x3603, 0x40},
+ {0x3604, 0x02},
+ {0x3609, 0x12},
+ {0x360c, 0x08},
+ {0x360f, 0xe5},
+ {0x3608, 0x8f},
+ {0x3611, 0x00},
+ {0x3613, 0xf7},
+ {0x3616, 0x58},
+ {0x3619, 0x99},
+ {0x361b, 0x60},
+ {0x361e, 0x79},
+ {0x3634, 0x10},
+ {0x3635, 0x10},
+ {0x3636, 0x15},
+ {0x3646, 0x86},
+ {0x364a, 0x0b},
+
+ /* Sensor control */
+ {0x3700, 0x17},
+ {0x3701, 0x22},
+ {0x3703, 0x10},
+ {0x370a, 0x37},
+ {0x3706, 0x63},
+ {0x3709, 0x3c},
+ {0x370c, 0x30},
+ {0x3710, 0x24},
+ {0x3720, 0x28},
+ {0x3729, 0x7b},
+ {0x372b, 0xbd},
+ {0x372c, 0xbc},
+ {0x372e, 0x52},
+ {0x373c, 0x0e},
+ {0x373e, 0x33},
+ {0x3743, 0x10},
+ {0x3744, 0x88},
+ {0x3745, 0xc0},
+ {0x374c, 0x00},
+ {0x374e, 0x23},
+ {0x3751, 0x7b},
+ {0x3753, 0xbd},
+ {0x3754, 0xbc},
+ {0x3756, 0x52},
+ {0x376b, 0x20},
+ {0x3774, 0x51},
+ {0x3776, 0xbd},
+ {0x3777, 0xbd},
+ {0x3781, 0x18},
+ {0x3783, 0x25},
+ {0x3798, 0x1b},
+
+ /* Timing control */
+ {0x3801, 0x08}, /* H_CROP_START_L h_crop_start[7:0] = 0x08 */
+ {0x3805, 0x97}, /* H_CROP_END_L h_crop_end[7:0] = 0x97 */
+ {0x380c, 0x0a}, /* TIMING_HTS_H hts[14:8] = 0x0a */
+ {0x380d, 0x0e}, /* TIMING_HTS_L hts[7:0] = 0x0e */
+ {0x3811, 0x08}, /* H_WIN_OFF_L h_win_off[7:0] = 0x08*/
+ {0x3813, 0x04}, /* V_WIN_OFF_L v_win_off[7:0] = 0x04 */
+ {0x3819, 0x01}, /* VSYNC_END_L vsync_end_point[7:0] = 0x01 */
+ {0x3821, 0x06}, /* TIMING_FORMAT2 array_h_mirror = 1, digital_h_mirror = 1 */
+
+ /* OTP control */
+ {0x3d85, 0x36}, /* OTP_REG85 OTP_power_up_load_setting_enable = 1,
+ * OTP_power_up_load_data_enable = 1,
+ * OTP_bist_select = 1 (compare with zero)
+ */
+ {0x3d8c, 0x71}, /* OTP_SETTING_STT_ADDRESS_H */
+ {0x3d8d, 0xcb}, /* OTP_SETTING_STT_ADDRESS_L */
+
+ /* BLC registers*/
+ {0x4001, 0x40}, /* DEBUG_MODE */
+ {0x401b, 0x00}, /* DEBUG_MODE */
+ {0x401d, 0x00}, /* DEBUG_MODE */
+ {0x401f, 0x00}, /* DEBUG_MODE */
+ {0x4020, 0x00}, /* ANCHOR_LEFT_START_H anchor_left_start[11:8] = 0 */
+ {0x4021, 0x10}, /* ANCHOR_LEFT_START_L anchor_left_start[7:0] = 0x10 */
+ {0x4022, 0x07}, /* ANCHOR_LEFT_END_H anchor_left_end[11:8] = 0x07 */
+ {0x4023, 0xcf}, /* ANCHOR_LEFT_END_L anchor_left_end[7:0] = 0xcf */
+ {0x4024, 0x09}, /* ANCHOR_RIGHT_START_H anchor_right_start[11:8] = 0x09 */
+ {0x4025, 0x60}, /* ANCHOR_RIGHT_START_L anchor_right_start[7:0] = 0x60 */
+ {0x4026, 0x09}, /* ANCHOR_RIGHT_END_H anchor_right_end[11:8] = 0x09 */
+ {0x4027, 0x6f}, /* ANCHOR_RIGHT_END_L anchor_right_end[7:0] = 0x6f */
+
+ /* ADC sync control */
+ {0x4500, 0x6c}, /* ADC_SYNC_CTRL */
+ {0x4503, 0x01}, /* ADC_SYNC_CTRL */
+
+ /* VFIFO */
+ {0x4601, 0xa7}, /* VFIFO_CTRL_01 r_vfifo_read_start[7:0] = 0xa7 */
+
+ /* Temperature monitor */
+ {0x4d00, 0x04}, /* TPM_CTRL_00 tmp_slope[15:8] = 0x04 */
+ {0x4d01, 0x42}, /* TPM_CTRL_01 tmp_slope[7:0] = 0x42 */
+ {0x4d02, 0xd1}, /* TPM_CTRL_02 tpm_offset[31:24] = 0xd1 */
+ {0x4d03, 0x93}, /* TPM_CTRL_03 tpm_offset[23:16] = 0x93 */
+ {0x4d04, 0xf5}, /* TPM_CTRL_04 tpm_offset[15:8] = 0xf5 */
+ {0x4d05, 0xc1}, /* TPM_CTRL_05 tpm_offset[7:0] = 0xc1 */
+
+ /* pre-ISP control */
+ {0x5050, 0x0c}, /* DEBUG_MODE */
+
+ /* OTP-DPC control */
+ {0x5501, 0x10}, /* OTP_DPC_START_L otp_start_address[7:0] = 0x10 */
+ {0x5503, 0x0f}, /* OTP_DPC_END_L otp_end_address[7:0] = 0x0f */
{REG_NULL, 0x00},
};
--
2.44.0
Make horizontal blanking configurable. To do so, set HTS register
according to the requested horizontal blanking in ov4689_set_ctrl
instead of the register table. Default HTS value is not changed by
this patch. Minimal HTS value is found experimentally and corresponds
to 90 fps framerate at minimum vertical blanking. Real HTS value is
the register value multiplied by 4.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/ov4689.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index 45f055c57436..8283a9ac86c9 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -30,13 +30,17 @@
#define OV4689_REG_EXPOSURE CCI_REG24(0x3500)
#define OV4689_EXPOSURE_MIN 4
#define OV4689_EXPOSURE_STEP 1
-#define OV4689_VTS_MAX 0x7fff
#define OV4689_REG_GAIN CCI_REG16(0x3508)
#define OV4689_GAIN_STEP 1
#define OV4689_GAIN_DEFAULT 0x80
+#define OV4689_REG_HTS CCI_REG16(0x380c)
+#define OV4689_HTS_DIVIDER 4
+#define OV4689_HTS_MAX 0x7fff
+
#define OV4689_REG_VTS CCI_REG16(0x380e)
+#define OV4689_VTS_MAX 0x7fff
#define OV4689_REG_TEST_PATTERN CCI_REG8(0x5040)
#define OV4689_TEST_PATTERN_ENABLE 0x80
@@ -61,6 +65,7 @@ struct ov4689_mode {
u32 width;
u32 height;
u32 hts_def;
+ u32 hts_min;
u32 vts_def;
u32 exp_def;
u32 pixel_rate;
@@ -104,7 +109,7 @@ struct ov4689_gain_range {
/*
* Xclk 24Mhz
- * max_framerate 30fps
+ * max_framerate 90fps
* mipi_datarate per lane 1008Mbps
*/
static const struct cci_reg_sequence ov4689_2688x1520_regs[] = {
@@ -175,8 +180,6 @@ static const struct cci_reg_sequence ov4689_2688x1520_regs[] = {
/* Timing control */
{CCI_REG8(0x3801), 0x08}, /* H_CROP_START_L h_crop_start[7:0] = 0x08 */
{CCI_REG8(0x3805), 0x97}, /* H_CROP_END_L h_crop_end[7:0] = 0x97 */
- {CCI_REG8(0x380c), 0x0a}, /* TIMING_HTS_H hts[14:8] = 0x0a */
- {CCI_REG8(0x380d), 0x0e}, /* TIMING_HTS_L hts[7:0] = 0x0e */
{CCI_REG8(0x3811), 0x08}, /* H_WIN_OFF_L h_win_off[7:0] = 0x08*/
{CCI_REG8(0x3813), 0x04}, /* V_WIN_OFF_L v_win_off[7:0] = 0x04 */
{CCI_REG8(0x3819), 0x01}, /* VSYNC_END_L vsync_end_point[7:0] = 0x01 */
@@ -237,7 +240,8 @@ static const struct ov4689_mode supported_modes[] = {
.crop_top = 8,
.crop_left = 16,
.exp_def = 1536,
- .hts_def = 4 * 2574,
+ .hts_def = 10296,
+ .hts_min = 3432,
.vts_def = 1554,
.pixel_rate = 480000000,
.reg_list = ov4689_2688x1520_regs,
@@ -598,6 +602,11 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_TEST_PATTERN:
ret = ov4689_enable_test_pattern(ov4689, ctrl->val);
break;
+ case V4L2_CID_HBLANK:
+ cci_write(regmap, OV4689_REG_HTS,
+ (ctrl->val + ov4689->cur_mode->width) /
+ OV4689_HTS_DIVIDER, &ret);
+ break;
default:
dev_warn(dev, "%s Unhandled id:0x%x, val:0x%x\n",
__func__, ctrl->id, ctrl->val);
@@ -622,8 +631,8 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
struct v4l2_ctrl_handler *handler;
const struct ov4689_mode *mode;
s64 exposure_max, vblank_def;
+ s64 hblank_def, hblank_min;
struct v4l2_ctrl *ctrl;
- s64 h_blank_def;
int ret;
handler = &ov4689->ctrl_handler;
@@ -640,11 +649,11 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
v4l2_ctrl_new_std(handler, NULL, V4L2_CID_PIXEL_RATE, 0,
mode->pixel_rate, 1, mode->pixel_rate);
- h_blank_def = mode->hts_def - mode->width;
- ctrl = v4l2_ctrl_new_std(handler, NULL, V4L2_CID_HBLANK, h_blank_def,
- h_blank_def, 1, h_blank_def);
- if (ctrl)
- ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+ hblank_def = mode->hts_def - mode->width;
+ hblank_min = mode->hts_min - mode->width;
+ v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_HBLANK,
+ hblank_min, OV4689_HTS_MAX - mode->width,
+ OV4689_HTS_DIVIDER, hblank_def);
vblank_def = mode->vts_def - mode->height;
v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_VBLANK,
--
2.44.0
max_fps field of struct ov4689_mode is unused in this driver, so
remove it.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/ov4689.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index 2354397ab34c..45f055c57436 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -60,7 +60,6 @@ struct ov4689_mode {
enum ov4689_mode_id id;
u32 width;
u32 height;
- u32 max_fps;
u32 hts_def;
u32 vts_def;
u32 exp_def;
@@ -237,7 +236,6 @@ static const struct ov4689_mode supported_modes[] = {
.sensor_height = 1536,
.crop_top = 8,
.crop_left = 16,
- .max_fps = 30,
.exp_def = 1536,
.hts_def = 4 * 2574,
.vts_def = 1554,
--
2.44.0
Split ov4689_s_stream into ov4689_stream_on and ov4689_stream_off
functions. Also remove repetitive pm_runtime_put calls and call
pm_runtime_put once at the end of the ov4689_stream_on function if any
error occurred.
Signed-off-by: Mikhail Rudenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/media/i2c/ov4689.c | 100 ++++++++++++++++++++-----------------
1 file changed, 53 insertions(+), 47 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index f43be255234b..e652d626f32f 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -537,61 +537,67 @@ static int ov4689_setup_blc_anchors(struct ov4689 *ov4689,
return ret;
}
+static int ov4689_stream_on(struct ov4689 *ov4689,
+ struct v4l2_subdev_state *sd_state)
+{
+ int ret;
+
+ ret = pm_runtime_resume_and_get(ov4689->dev);
+ if (ret < 0)
+ return ret;
+
+ ret = cci_multi_reg_write(ov4689->regmap, ov4689_common_regs,
+ ARRAY_SIZE(ov4689_common_regs), NULL);
+ if (ret)
+ goto error;
+
+ ret = ov4689_setup_timings(ov4689, sd_state);
+ if (ret)
+ goto error;
+
+ ret = ov4689_setup_blc_anchors(ov4689, sd_state);
+ if (ret)
+ goto error;
+
+ ret = __v4l2_ctrl_handler_setup(&ov4689->ctrl_handler);
+ if (ret)
+ goto error;
+
+ ret = cci_write(ov4689->regmap, OV4689_REG_CTRL_MODE,
+ OV4689_MODE_STREAMING, NULL);
+ if (ret)
+ goto error;
+
+ return 0;
+
+error:
+ pm_runtime_put(ov4689->dev);
+ return ret;
+}
+
+static int ov4689_stream_off(struct ov4689 *ov4689)
+{
+ cci_write(ov4689->regmap, OV4689_REG_CTRL_MODE, OV4689_MODE_SW_STANDBY,
+ NULL);
+ pm_runtime_mark_last_busy(ov4689->dev);
+ pm_runtime_put_autosuspend(ov4689->dev);
+
+ return 0;
+}
+
static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
{
struct ov4689 *ov4689 = to_ov4689(sd);
struct v4l2_subdev_state *sd_state;
- struct device *dev = ov4689->dev;
- int ret = 0;
+ int ret;
sd_state = v4l2_subdev_lock_and_get_active_state(&ov4689->subdev);
- if (on) {
- ret = pm_runtime_resume_and_get(dev);
- if (ret < 0)
- goto unlock_and_return;
-
- ret = cci_multi_reg_write(ov4689->regmap,
- ov4689_common_regs,
- ARRAY_SIZE(ov4689_common_regs),
- NULL);
- if (ret) {
- pm_runtime_put(dev);
- goto unlock_and_return;
- }
-
- ret = ov4689_setup_timings(ov4689, sd_state);
- if (ret) {
- pm_runtime_put(dev);
- goto unlock_and_return;
- }
-
- ret = ov4689_setup_blc_anchors(ov4689, sd_state);
- if (ret) {
- pm_runtime_put(dev);
- goto unlock_and_return;
- }
-
- ret = __v4l2_ctrl_handler_setup(&ov4689->ctrl_handler);
- if (ret) {
- pm_runtime_put(dev);
- goto unlock_and_return;
- }
-
- ret = cci_write(ov4689->regmap, OV4689_REG_CTRL_MODE,
- OV4689_MODE_STREAMING, NULL);
- if (ret) {
- pm_runtime_put(dev);
- goto unlock_and_return;
- }
- } else {
- cci_write(ov4689->regmap, OV4689_REG_CTRL_MODE,
- OV4689_MODE_SW_STANDBY, NULL);
- pm_runtime_mark_last_busy(dev);
- pm_runtime_put_autosuspend(dev);
- }
+ if (on)
+ ret = ov4689_stream_on(ov4689, sd_state);
+ else
+ ret = ov4689_stream_off(ov4689);
-unlock_and_return:
v4l2_subdev_unlock_state(sd_state);
return ret;
--
2.44.0
Implement 2x2 binning support. Compute best binning mode (none or 2x2)
from pad crop and pad format in ov4689_set_fmt. Use output frame size
instead of analogue crop to compute control ranges and BLC anchors.
Also move ov4689_hts_min and ov4689_update_ctrl_ranges, since they are
now also called from ov4689_set_fmt. Update frame timings to
accommodate the requirements of binning mode and avoid visual
artefacts. Additionally, report 2x2 binned mode in addition to
non-binned one in ov4689_enum_frame_sizes.
Signed-off-by: Mikhail Rudenko <[email protected]>
---
drivers/media/i2c/ov4689.c | 192 +++++++++++++++++++++++++------------
1 file changed, 130 insertions(+), 62 deletions(-)
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index e652d626f32f..83c7d0bae7d1 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -114,7 +114,7 @@
* Minimum working vertical blanking value. Found experimentally at
* minimum HTS values.
*/
-#define OV4689_VBLANK_MIN 31
+#define OV4689_VBLANK_MIN 35
static const char *const ov4689_supply_names[] = {
"avdd", /* Analog power */
@@ -256,6 +256,18 @@ static const struct cci_reg_sequence ov4689_common_regs[] = {
{CCI_REG8(0x5503), 0x0f}, /* OTP_DPC_END_L otp_end_address[7:0] = 0x0f */
};
+static const struct cci_reg_sequence ov4689_2x2_binning_regs[] = {
+ {CCI_REG8(0x3632), 0x05}, /* ADC */
+ {CCI_REG8(0x376b), 0x40}, /* Sensor control */
+ {CCI_REG8(0x3814), 0x03}, /* H_INC_ODD */
+ {CCI_REG8(0x3821), 0x07}, /* TIMING_FORMAT_2 hor_binning = 1*/
+ {CCI_REG8(0x382a), 0x03}, /* V_INC_ODD */
+ {CCI_REG8(0x3830), 0x08}, /* BLC_NUM_OPTION blc_use_num_2 = 1 */
+ {CCI_REG8(0x3836), 0x02}, /* TIMING_REG_36 r_zline_use_num_2 = 1 */
+ {CCI_REG8(0x4001), 0x50}, /* BLC DEBUG MODE */
+ {CCI_REG8(0x4502), 0x44}, /* ADC synch control*/
+};
+
static const u64 link_freq_menu_items[] = { 504000000 };
static const char *const ov4689_test_pattern_menu[] = {
@@ -305,18 +317,96 @@ static const struct ov4689_gain_range ov4689_gain_ranges[] = {
},
};
+/*
+ * For now, only 2x2 binning implemented in this driver.
+ */
+static int ov4689_best_binning(struct ov4689 *ov4689,
+ const struct v4l2_mbus_framefmt *format,
+ const struct v4l2_rect *crop,
+ unsigned int *binning)
+{
+ const struct v4l2_area candidates[] = {
+ { crop->width, crop->height },
+ { crop->width / 2, crop->height / 2 },
+ };
+
+ const struct v4l2_area *best;
+ int index;
+
+ best = v4l2_find_nearest_size(candidates, ARRAY_SIZE(candidates), width,
+ height, format->width, format->height);
+ if (!best) {
+ dev_err(ov4689->dev,
+ "failed to find best binning for requested mode\n");
+ return -EINVAL;
+ }
+
+ index = best - candidates;
+ *binning = index + 1;
+
+ dev_dbg(ov4689->dev,
+ "best_binning: crop=%dx%d format=%dx%d binning=%d\n",
+ crop->width, crop->height, format->width, format->height,
+ *binning);
+
+ return 0;
+}
+
+/*
+ * Minimum working HTS value for given output width (found
+ * experimentally).
+ */
+static unsigned int ov4689_hts_min(unsigned int width)
+{
+ return max_t(unsigned int, 3156, 224 + width * 19 / 16);
+}
+
+static void ov4689_update_ctrl_ranges(struct ov4689 *ov4689, unsigned int width,
+ unsigned int height)
+{
+ struct v4l2_ctrl *exposure = ov4689->exposure;
+ struct v4l2_ctrl *vblank = ov4689->vblank;
+ struct v4l2_ctrl *hblank = ov4689->hblank;
+ s64 def_val, min_val, max_val;
+
+ min_val = ov4689_hts_min(width) - width;
+ max_val = OV4689_HTS_MAX - width;
+ def_val = clamp_t(s64, hblank->default_value, min_val, max_val);
+ __v4l2_ctrl_modify_range(hblank, min_val, max_val, hblank->step,
+ def_val);
+
+ min_val = OV4689_VBLANK_MIN;
+ max_val = OV4689_HTS_MAX - width;
+ def_val = clamp_t(s64, vblank->default_value, min_val, max_val);
+ __v4l2_ctrl_modify_range(vblank, min_val, max_val, vblank->step,
+ def_val);
+
+ min_val = exposure->minimum;
+ max_val = height + vblank->val - 4;
+ def_val = clamp_t(s64, exposure->default_value, min_val, max_val);
+ __v4l2_ctrl_modify_range(exposure, min_val, max_val, exposure->step,
+ def_val);
+}
+
static int ov4689_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_format *fmt)
{
+ struct ov4689 *ov4689 = to_ov4689(sd);
struct v4l2_mbus_framefmt *format;
struct v4l2_rect *crop;
+ unsigned int binning;
+ int ret;
crop = v4l2_subdev_state_get_crop(sd_state, fmt->pad);
format = v4l2_subdev_state_get_format(sd_state, fmt->pad);
- format->width = crop->width;
- format->height = crop->height;
+ ret = ov4689_best_binning(ov4689, &fmt->format, crop, &binning);
+ if (ret)
+ return ret;
+
+ format->width = crop->width / binning;
+ format->height = crop->height / binning;
format->code = MEDIA_BUS_FMT_SBGGR10_1X10;
format->field = V4L2_FIELD_NONE;
@@ -327,6 +417,9 @@ static int ov4689_set_fmt(struct v4l2_subdev *sd,
fmt->format = *format;
+ if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+ ov4689_update_ctrl_ranges(ov4689, format->width, format->height);
+
return 0;
}
@@ -346,8 +439,9 @@ static int ov4689_enum_frame_sizes(struct v4l2_subdev *sd,
struct v4l2_subdev_frame_size_enum *fse)
{
const struct v4l2_rect *crop;
+ int binning;
- if (fse->index >= 1)
+ if (fse->index >= 2)
return -EINVAL;
if (fse->code != MEDIA_BUS_FMT_SBGGR10_1X10)
@@ -355,10 +449,11 @@ static int ov4689_enum_frame_sizes(struct v4l2_subdev *sd,
crop = v4l2_subdev_state_get_crop(sd_state, 0);
- fse->min_width = crop->width;
- fse->max_width = crop->width;
- fse->max_height = crop->height;
- fse->min_height = crop->height;
+ binning = fse->index + 1;
+ fse->min_width = crop->width / binning;
+ fse->max_width = crop->width / binning;
+ fse->max_height = crop->height / binning;
+ fse->min_height = crop->height / binning;
return 0;
}
@@ -398,42 +493,6 @@ static int ov4689_get_selection(struct v4l2_subdev *sd,
return -EINVAL;
}
-/*
- * Minimum working HTS value for given output width (found
- * experimentally).
- */
-static unsigned int ov4689_hts_min(unsigned int width)
-{
- return max_t(unsigned int, 3156, 224 + width * 19 / 16);
-}
-
-static void ov4689_update_ctrl_ranges(struct ov4689 *ov4689,
- struct v4l2_rect *crop)
-{
- struct v4l2_ctrl *exposure = ov4689->exposure;
- struct v4l2_ctrl *vblank = ov4689->vblank;
- struct v4l2_ctrl *hblank = ov4689->hblank;
- s64 def_val, min_val, max_val;
-
- min_val = ov4689_hts_min(crop->width) - crop->width;
- max_val = OV4689_HTS_MAX - crop->width;
- def_val = clamp_t(s64, hblank->default_value, min_val, max_val);
- __v4l2_ctrl_modify_range(hblank, min_val, max_val, hblank->step,
- def_val);
-
- min_val = OV4689_VBLANK_MIN;
- max_val = OV4689_HTS_MAX - crop->width;
- def_val = clamp_t(s64, vblank->default_value, min_val, max_val);
- __v4l2_ctrl_modify_range(vblank, min_val, max_val, vblank->step,
- def_val);
-
- min_val = exposure->minimum;
- max_val = crop->height + vblank->val - 4;
- def_val = clamp_t(s64, exposure->default_value, min_val, max_val);
- __v4l2_ctrl_modify_range(exposure, min_val, max_val, exposure->step,
- def_val);
-}
-
static int ov4689_set_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
@@ -470,7 +529,8 @@ static int ov4689_set_selection(struct v4l2_subdev *sd,
format->height = rect.height;
if (sel->which == V4L2_SUBDEV_FORMAT_ACTIVE)
- ov4689_update_ctrl_ranges(ov4689, &rect);
+ ov4689_update_ctrl_ranges(ov4689, rect.width,
+ rect.height);
}
*crop = rect;
@@ -485,21 +545,27 @@ static int ov4689_setup_timings(struct ov4689 *ov4689,
const struct v4l2_mbus_framefmt *format;
struct regmap *rm = ov4689->regmap;
const struct v4l2_rect *crop;
+ const int v_offset = 2;
+ unsigned int binning;
int ret = 0;
format = v4l2_subdev_state_get_format(state, 0);
crop = v4l2_subdev_state_get_crop(state, 0);
+ ret = ov4689_best_binning(ov4689, format, crop, &binning);
+ if (ret)
+ return ret;
+
cci_write(rm, OV4689_REG_H_CROP_START, crop->left, &ret);
- cci_write(rm, OV4689_REG_V_CROP_START, crop->top, &ret);
- cci_write(rm, OV4689_REG_H_CROP_END, crop->left + crop->width + 1, &ret);
- cci_write(rm, OV4689_REG_V_CROP_END, crop->top + crop->height + 1, &ret);
+ cci_write(rm, OV4689_REG_V_CROP_START, crop->top - v_offset, &ret);
+ cci_write(rm, OV4689_REG_H_CROP_END, crop->left + crop->width + 3, &ret);
+ cci_write(rm, OV4689_REG_V_CROP_END, crop->top + crop->height + 7, &ret);
cci_write(rm, OV4689_REG_H_OUTPUT_SIZE, format->width, &ret);
cci_write(rm, OV4689_REG_V_OUTPUT_SIZE, format->height, &ret);
cci_write(rm, OV4689_REG_H_WIN_OFF, 0, &ret);
- cci_write(rm, OV4689_REG_V_WIN_OFF, 0, &ret);
+ cci_write(rm, OV4689_REG_V_WIN_OFF, v_offset, &ret);
/*
* Maximum working value of vfifo_read_start for given output
@@ -507,6 +573,10 @@ static int ov4689_setup_timings(struct ov4689 *ov4689,
*/
cci_write(rm, OV4689_REG_VFIFO_CTRL_01, format->width / 16 - 1, &ret);
+ if (binning == 2)
+ cci_multi_reg_write(ov4689->regmap, ov4689_2x2_binning_regs,
+ ARRAY_SIZE(ov4689_2x2_binning_regs),
+ &ret);
return ret;
}
@@ -519,20 +589,20 @@ static int ov4689_setup_blc_anchors(struct ov4689 *ov4689,
struct v4l2_subdev_state *state)
{
unsigned int width_def = OV4689_H_OUTPUT_SIZE_DEFAULT;
+ const struct v4l2_mbus_framefmt *format;
struct regmap *rm = ov4689->regmap;
- const struct v4l2_rect *crop;
int ret = 0;
- crop = v4l2_subdev_state_get_crop(state, 0);
+ format = v4l2_subdev_state_get_format(state, 0);
cci_write(rm, OV4689_REG_ANCHOR_LEFT_START,
- OV4689_ANCHOR_LEFT_START_DEF * crop->width / width_def, &ret);
+ OV4689_ANCHOR_LEFT_START_DEF * format->width / width_def, &ret);
cci_write(rm, OV4689_REG_ANCHOR_LEFT_END,
- OV4689_ANCHOR_LEFT_END_DEF * crop->width / width_def, &ret);
+ OV4689_ANCHOR_LEFT_END_DEF * format->width / width_def, &ret);
cci_write(rm, OV4689_REG_ANCHOR_RIGHT_START,
- OV4689_ANCHOR_RIGHT_START_DEF * crop->width / width_def, &ret);
+ OV4689_ANCHOR_RIGHT_START_DEF * format->width / width_def, &ret);
cci_write(rm, OV4689_REG_ANCHOR_RIGHT_END,
- OV4689_ANCHOR_RIGHT_END_DEF * crop->width / width_def, &ret);
+ OV4689_ANCHOR_RIGHT_END_DEF * format->width / width_def, &ret);
return ret;
}
@@ -749,19 +819,19 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
struct regmap *regmap = ov4689->regmap;
struct v4l2_subdev_state *sd_state;
struct device *dev = ov4689->dev;
- struct v4l2_rect *crop;
+ struct v4l2_mbus_framefmt *fmt;
s64 max_expo, def_expo;
int sensor_gain;
int ret = 0;
sd_state = v4l2_subdev_get_locked_active_state(&ov4689->subdev);
- crop = v4l2_subdev_state_get_crop(sd_state, 0);
+ fmt = v4l2_subdev_state_get_format(sd_state, 0);
/* Propagate change of current control to all related controls */
switch (ctrl->id) {
case V4L2_CID_VBLANK:
/* Update max exposure while meeting expected vblanking */
- max_expo = crop->height + ctrl->val - 4;
+ max_expo = fmt->height + ctrl->val - 4;
def_expo = clamp_t(s64, ov4689->exposure->default_value,
ov4689->exposure->minimum, max_expo);
@@ -785,16 +855,14 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
cci_write(regmap, OV4689_REG_GAIN, sensor_gain, &ret);
break;
case V4L2_CID_VBLANK:
- cci_write(regmap, OV4689_REG_VTS,
- ctrl->val + crop->height, &ret);
+ cci_write(regmap, OV4689_REG_VTS, ctrl->val + fmt->height, &ret);
break;
case V4L2_CID_TEST_PATTERN:
ret = ov4689_enable_test_pattern(ov4689, ctrl->val);
break;
case V4L2_CID_HBLANK:
cci_write(regmap, OV4689_REG_HTS,
- (ctrl->val + crop->width) /
- OV4689_HTS_DIVIDER, &ret);
+ (ctrl->val + fmt->width) / OV4689_HTS_DIVIDER, &ret);
break;
case V4L2_CID_VFLIP:
cci_update_bits(regmap, OV4689_REG_TIMING_FORMAT1,
--
2.44.0
Hi Nikhail,
On Tue, Apr 02, 2024 at 07:45:51PM +0300, Mikhail Rudenko wrote:
> Implement 2x2 binning support. Compute best binning mode (none or 2x2)
> from pad crop and pad format in ov4689_set_fmt. Use output frame size
> instead of analogue crop to compute control ranges and BLC anchors.
>
> Also move ov4689_hts_min and ov4689_update_ctrl_ranges, since they are
> now also called from ov4689_set_fmt. Update frame timings to
> accommodate the requirements of binning mode and avoid visual
> artefacts. Additionally, report 2x2 binned mode in addition to
> non-binned one in ov4689_enum_frame_sizes.
>
> Signed-off-by: Mikhail Rudenko <[email protected]>
> ---
> drivers/media/i2c/ov4689.c | 192 +++++++++++++++++++++++++------------
> 1 file changed, 130 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
> index e652d626f32f..83c7d0bae7d1 100644
> --- a/drivers/media/i2c/ov4689.c
> +++ b/drivers/media/i2c/ov4689.c
> @@ -114,7 +114,7 @@
> * Minimum working vertical blanking value. Found experimentally at
> * minimum HTS values.
> */
> -#define OV4689_VBLANK_MIN 31
> +#define OV4689_VBLANK_MIN 35
>
> static const char *const ov4689_supply_names[] = {
> "avdd", /* Analog power */
> @@ -256,6 +256,18 @@ static const struct cci_reg_sequence ov4689_common_regs[] = {
> {CCI_REG8(0x5503), 0x0f}, /* OTP_DPC_END_L otp_end_address[7:0] = 0x0f */
> };
>
> +static const struct cci_reg_sequence ov4689_2x2_binning_regs[] = {
> + {CCI_REG8(0x3632), 0x05}, /* ADC */
> + {CCI_REG8(0x376b), 0x40}, /* Sensor control */
> + {CCI_REG8(0x3814), 0x03}, /* H_INC_ODD */
> + {CCI_REG8(0x3821), 0x07}, /* TIMING_FORMAT_2 hor_binning = 1*/
> + {CCI_REG8(0x382a), 0x03}, /* V_INC_ODD */
> + {CCI_REG8(0x3830), 0x08}, /* BLC_NUM_OPTION blc_use_num_2 = 1 */
> + {CCI_REG8(0x3836), 0x02}, /* TIMING_REG_36 r_zline_use_num_2 = 1 */
> + {CCI_REG8(0x4001), 0x50}, /* BLC DEBUG MODE */
> + {CCI_REG8(0x4502), 0x44}, /* ADC synch control*/
Spaces inside { }'s, please.
> +};
> +
> static const u64 link_freq_menu_items[] = { 504000000 };
>
> static const char *const ov4689_test_pattern_menu[] = {
> @@ -305,18 +317,96 @@ static const struct ov4689_gain_range ov4689_gain_ranges[] = {
> },
> };
>
> +/*
> + * For now, only 2x2 binning implemented in this driver.
> + */
> +static int ov4689_best_binning(struct ov4689 *ov4689,
> + const struct v4l2_mbus_framefmt *format,
> + const struct v4l2_rect *crop,
> + unsigned int *binning)
> +{
> + const struct v4l2_area candidates[] = {
> + { crop->width, crop->height },
> + { crop->width / 2, crop->height / 2 },
> + };
> +
> + const struct v4l2_area *best;
> + int index;
> +
> + best = v4l2_find_nearest_size(candidates, ARRAY_SIZE(candidates), width,
> + height, format->width, format->height);
You can assume v4l2_find_nearest_size() returns a non-NULL value (see the
other patch I cc'd you).
> + if (!best) {
> + dev_err(ov4689->dev,
> + "failed to find best binning for requested mode\n");
> + return -EINVAL;
> + }
> +
> + index = best - candidates;
> + *binning = index + 1;
> +
> + dev_dbg(ov4689->dev,
> + "best_binning: crop=%dx%d format=%dx%d binning=%d\n",
> + crop->width, crop->height, format->width, format->height,
> + *binning);
> +
> + return 0;
> +}
> +
> +/*
> + * Minimum working HTS value for given output width (found
> + * experimentally).
> + */
> +static unsigned int ov4689_hts_min(unsigned int width)
> +{
> + return max_t(unsigned int, 3156, 224 + width * 19 / 16);
> +}
> +
> +static void ov4689_update_ctrl_ranges(struct ov4689 *ov4689, unsigned int width,
> + unsigned int height)
> +{
> + struct v4l2_ctrl *exposure = ov4689->exposure;
> + struct v4l2_ctrl *vblank = ov4689->vblank;
> + struct v4l2_ctrl *hblank = ov4689->hblank;
> + s64 def_val, min_val, max_val;
> +
> + min_val = ov4689_hts_min(width) - width;
> + max_val = OV4689_HTS_MAX - width;
> + def_val = clamp_t(s64, hblank->default_value, min_val, max_val);
> + __v4l2_ctrl_modify_range(hblank, min_val, max_val, hblank->step,
> + def_val);
Note that __v4l2_ctrl_modify_range() may fail. The problem isn't introduced
by this patch but it'd be nice to fix it (but maybe in a separate patch).
> +
> + min_val = OV4689_VBLANK_MIN;
> + max_val = OV4689_HTS_MAX - width;
> + def_val = clamp_t(s64, vblank->default_value, min_val, max_val);
> + __v4l2_ctrl_modify_range(vblank, min_val, max_val, vblank->step,
> + def_val);
> +
> + min_val = exposure->minimum;
> + max_val = height + vblank->val - 4;
> + def_val = clamp_t(s64, exposure->default_value, min_val, max_val);
> + __v4l2_ctrl_modify_range(exposure, min_val, max_val, exposure->step,
> + def_val);
> +}
> +
> static int ov4689_set_fmt(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_format *fmt)
> {
> + struct ov4689 *ov4689 = to_ov4689(sd);
> struct v4l2_mbus_framefmt *format;
> struct v4l2_rect *crop;
> + unsigned int binning;
> + int ret;
>
> crop = v4l2_subdev_state_get_crop(sd_state, fmt->pad);
> format = v4l2_subdev_state_get_format(sd_state, fmt->pad);
>
> - format->width = crop->width;
> - format->height = crop->height;
> + ret = ov4689_best_binning(ov4689, &fmt->format, crop, &binning);
> + if (ret)
> + return ret;
> +
> + format->width = crop->width / binning;
> + format->height = crop->height / binning;
>
> format->code = MEDIA_BUS_FMT_SBGGR10_1X10;
> format->field = V4L2_FIELD_NONE;
> @@ -327,6 +417,9 @@ static int ov4689_set_fmt(struct v4l2_subdev *sd,
>
> fmt->format = *format;
>
> + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> + ov4689_update_ctrl_ranges(ov4689, format->width, format->height);
> +
> return 0;
> }
>
> @@ -346,8 +439,9 @@ static int ov4689_enum_frame_sizes(struct v4l2_subdev *sd,
> struct v4l2_subdev_frame_size_enum *fse)
> {
> const struct v4l2_rect *crop;
> + int binning;
>
> - if (fse->index >= 1)
> + if (fse->index >= 2)
> return -EINVAL;
>
> if (fse->code != MEDIA_BUS_FMT_SBGGR10_1X10)
> @@ -355,10 +449,11 @@ static int ov4689_enum_frame_sizes(struct v4l2_subdev *sd,
>
> crop = v4l2_subdev_state_get_crop(sd_state, 0);
>
> - fse->min_width = crop->width;
> - fse->max_width = crop->width;
> - fse->max_height = crop->height;
> - fse->min_height = crop->height;
> + binning = fse->index + 1;
> + fse->min_width = crop->width / binning;
> + fse->max_width = crop->width / binning;
> + fse->max_height = crop->height / binning;
> + fse->min_height = crop->height / binning;
>
> return 0;
> }
> @@ -398,42 +493,6 @@ static int ov4689_get_selection(struct v4l2_subdev *sd,
> return -EINVAL;
> }
>
> -/*
> - * Minimum working HTS value for given output width (found
> - * experimentally).
> - */
> -static unsigned int ov4689_hts_min(unsigned int width)
> -{
> - return max_t(unsigned int, 3156, 224 + width * 19 / 16);
> -}
> -
> -static void ov4689_update_ctrl_ranges(struct ov4689 *ov4689,
> - struct v4l2_rect *crop)
> -{
> - struct v4l2_ctrl *exposure = ov4689->exposure;
> - struct v4l2_ctrl *vblank = ov4689->vblank;
> - struct v4l2_ctrl *hblank = ov4689->hblank;
> - s64 def_val, min_val, max_val;
> -
> - min_val = ov4689_hts_min(crop->width) - crop->width;
> - max_val = OV4689_HTS_MAX - crop->width;
> - def_val = clamp_t(s64, hblank->default_value, min_val, max_val);
> - __v4l2_ctrl_modify_range(hblank, min_val, max_val, hblank->step,
> - def_val);
> -
> - min_val = OV4689_VBLANK_MIN;
> - max_val = OV4689_HTS_MAX - crop->width;
> - def_val = clamp_t(s64, vblank->default_value, min_val, max_val);
> - __v4l2_ctrl_modify_range(vblank, min_val, max_val, vblank->step,
> - def_val);
> -
> - min_val = exposure->minimum;
> - max_val = crop->height + vblank->val - 4;
> - def_val = clamp_t(s64, exposure->default_value, min_val, max_val);
> - __v4l2_ctrl_modify_range(exposure, min_val, max_val, exposure->step,
> - def_val);
> -}
> -
> static int ov4689_set_selection(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state,
> struct v4l2_subdev_selection *sel)
> @@ -470,7 +529,8 @@ static int ov4689_set_selection(struct v4l2_subdev *sd,
> format->height = rect.height;
>
> if (sel->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> - ov4689_update_ctrl_ranges(ov4689, &rect);
> + ov4689_update_ctrl_ranges(ov4689, rect.width,
> + rect.height);
> }
>
> *crop = rect;
> @@ -485,21 +545,27 @@ static int ov4689_setup_timings(struct ov4689 *ov4689,
> const struct v4l2_mbus_framefmt *format;
> struct regmap *rm = ov4689->regmap;
> const struct v4l2_rect *crop;
> + const int v_offset = 2;
> + unsigned int binning;
> int ret = 0;
>
> format = v4l2_subdev_state_get_format(state, 0);
> crop = v4l2_subdev_state_get_crop(state, 0);
>
> + ret = ov4689_best_binning(ov4689, format, crop, &binning);
> + if (ret)
> + return ret;
> +
> cci_write(rm, OV4689_REG_H_CROP_START, crop->left, &ret);
> - cci_write(rm, OV4689_REG_V_CROP_START, crop->top, &ret);
> - cci_write(rm, OV4689_REG_H_CROP_END, crop->left + crop->width + 1, &ret);
> - cci_write(rm, OV4689_REG_V_CROP_END, crop->top + crop->height + 1, &ret);
> + cci_write(rm, OV4689_REG_V_CROP_START, crop->top - v_offset, &ret);
> + cci_write(rm, OV4689_REG_H_CROP_END, crop->left + crop->width + 3, &ret);
> + cci_write(rm, OV4689_REG_V_CROP_END, crop->top + crop->height + 7, &ret);
>
> cci_write(rm, OV4689_REG_H_OUTPUT_SIZE, format->width, &ret);
> cci_write(rm, OV4689_REG_V_OUTPUT_SIZE, format->height, &ret);
>
> cci_write(rm, OV4689_REG_H_WIN_OFF, 0, &ret);
> - cci_write(rm, OV4689_REG_V_WIN_OFF, 0, &ret);
> + cci_write(rm, OV4689_REG_V_WIN_OFF, v_offset, &ret);
>
> /*
> * Maximum working value of vfifo_read_start for given output
> @@ -507,6 +573,10 @@ static int ov4689_setup_timings(struct ov4689 *ov4689,
> */
> cci_write(rm, OV4689_REG_VFIFO_CTRL_01, format->width / 16 - 1, &ret);
>
> + if (binning == 2)
> + cci_multi_reg_write(ov4689->regmap, ov4689_2x2_binning_regs,
> + ARRAY_SIZE(ov4689_2x2_binning_regs),
> + &ret);
> return ret;
> }
>
> @@ -519,20 +589,20 @@ static int ov4689_setup_blc_anchors(struct ov4689 *ov4689,
> struct v4l2_subdev_state *state)
> {
> unsigned int width_def = OV4689_H_OUTPUT_SIZE_DEFAULT;
> + const struct v4l2_mbus_framefmt *format;
> struct regmap *rm = ov4689->regmap;
> - const struct v4l2_rect *crop;
> int ret = 0;
>
> - crop = v4l2_subdev_state_get_crop(state, 0);
> + format = v4l2_subdev_state_get_format(state, 0);
>
> cci_write(rm, OV4689_REG_ANCHOR_LEFT_START,
> - OV4689_ANCHOR_LEFT_START_DEF * crop->width / width_def, &ret);
> + OV4689_ANCHOR_LEFT_START_DEF * format->width / width_def, &ret);
> cci_write(rm, OV4689_REG_ANCHOR_LEFT_END,
> - OV4689_ANCHOR_LEFT_END_DEF * crop->width / width_def, &ret);
> + OV4689_ANCHOR_LEFT_END_DEF * format->width / width_def, &ret);
> cci_write(rm, OV4689_REG_ANCHOR_RIGHT_START,
> - OV4689_ANCHOR_RIGHT_START_DEF * crop->width / width_def, &ret);
> + OV4689_ANCHOR_RIGHT_START_DEF * format->width / width_def, &ret);
> cci_write(rm, OV4689_REG_ANCHOR_RIGHT_END,
> - OV4689_ANCHOR_RIGHT_END_DEF * crop->width / width_def, &ret);
> + OV4689_ANCHOR_RIGHT_END_DEF * format->width / width_def, &ret);
>
> return ret;
> }
> @@ -749,19 +819,19 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
> struct regmap *regmap = ov4689->regmap;
> struct v4l2_subdev_state *sd_state;
> struct device *dev = ov4689->dev;
> - struct v4l2_rect *crop;
> + struct v4l2_mbus_framefmt *fmt;
> s64 max_expo, def_expo;
> int sensor_gain;
> int ret = 0;
>
> sd_state = v4l2_subdev_get_locked_active_state(&ov4689->subdev);
> - crop = v4l2_subdev_state_get_crop(sd_state, 0);
> + fmt = v4l2_subdev_state_get_format(sd_state, 0);
>
> /* Propagate change of current control to all related controls */
> switch (ctrl->id) {
> case V4L2_CID_VBLANK:
> /* Update max exposure while meeting expected vblanking */
> - max_expo = crop->height + ctrl->val - 4;
> + max_expo = fmt->height + ctrl->val - 4;
> def_expo = clamp_t(s64, ov4689->exposure->default_value,
> ov4689->exposure->minimum, max_expo);
>
> @@ -785,16 +855,14 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
> cci_write(regmap, OV4689_REG_GAIN, sensor_gain, &ret);
> break;
> case V4L2_CID_VBLANK:
> - cci_write(regmap, OV4689_REG_VTS,
> - ctrl->val + crop->height, &ret);
> + cci_write(regmap, OV4689_REG_VTS, ctrl->val + fmt->height, &ret);
> break;
> case V4L2_CID_TEST_PATTERN:
> ret = ov4689_enable_test_pattern(ov4689, ctrl->val);
> break;
> case V4L2_CID_HBLANK:
> cci_write(regmap, OV4689_REG_HTS,
> - (ctrl->val + crop->width) /
> - OV4689_HTS_DIVIDER, &ret);
> + (ctrl->val + fmt->width) / OV4689_HTS_DIVIDER, &ret);
> break;
> case V4L2_CID_VFLIP:
> cci_update_bits(regmap, OV4689_REG_TIMING_FORMAT1,
--
Kind regards,
Sakari Ailus
Hi Mikhail,
On Tue, Apr 02, 2024 at 07:45:48PM +0300, Mikhail Rudenko wrote:
> Implement configurable analogue crop via .set_selection call.
> ov4689_init_cfg is modified to initialize default subdev selection.
> Offsets are aligned to 2 to preserve Bayer order, selection width is
> aligned to 4 and height to 2 to meet hardware requirements.
>
> Experimentally discovered values of the cropping-related registers and
> vfifo_read_start for various output sizes are used. Default BLC anchor
> positions are used for the default analogue crop, scaling down
> proportionally for the smaller crop sizes.
>
> When analogue crop is adjusted, several consequential actions take
> place: the output format is reset, exposure/vblank/hblank control
> ranges and default values are adjusted accordingly. Additionally,
> ov4689_set_ctrl utilizes pad crop instead of cur_mode width and
> height for HTS and VTS calculation. Also, ov4689_enum_frame_sizes is
> modified to report crop size as available frame size.
We're amidst of a change to the APIs touching sensors with the the
introduction of the internal pads.
<URL:https://lore.kernel.org/linux-media/[email protected]/T/#t>.
I'd therefore postpone this bit so it would align with the new practices
(also subject to change in the metadata set).
The rest of the patches would seem more or less ready for merging to me.
--
Kind regards,
Sakari Ailus
Hi Sakari,
and thanks for your review!
On 2024-04-15 at 06:07 GMT, Sakari Ailus <[email protected]> wrote:
> Hi Nikhail,
>
> On Tue, Apr 02, 2024 at 07:45:51PM +0300, Mikhail Rudenko wrote:
>> Implement 2x2 binning support. Compute best binning mode (none or 2x2)
>> from pad crop and pad format in ov4689_set_fmt. Use output frame size
>> instead of analogue crop to compute control ranges and BLC anchors.
>>
>> Also move ov4689_hts_min and ov4689_update_ctrl_ranges, since they are
>> now also called from ov4689_set_fmt. Update frame timings to
>> accommodate the requirements of binning mode and avoid visual
>> artefacts. Additionally, report 2x2 binned mode in addition to
>> non-binned one in ov4689_enum_frame_sizes.
>>
>> Signed-off-by: Mikhail Rudenko <[email protected]>
>> ---
>> drivers/media/i2c/ov4689.c | 192 +++++++++++++++++++++++++------------
>> 1 file changed, 130 insertions(+), 62 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
>> index e652d626f32f..83c7d0bae7d1 100644
>> --- a/drivers/media/i2c/ov4689.c
>> +++ b/drivers/media/i2c/ov4689.c
>> @@ -114,7 +114,7 @@
>> * Minimum working vertical blanking value. Found experimentally at
>> * minimum HTS values.
>> */
>> -#define OV4689_VBLANK_MIN 31
>> +#define OV4689_VBLANK_MIN 35
>>
>> static const char *const ov4689_supply_names[] = {
>> "avdd", /* Analog power */
>> @@ -256,6 +256,18 @@ static const struct cci_reg_sequence ov4689_common_regs[] = {
>> {CCI_REG8(0x5503), 0x0f}, /* OTP_DPC_END_L otp_end_address[7:0] = 0x0f */
>> };
>>
>> +static const struct cci_reg_sequence ov4689_2x2_binning_regs[] = {
>> + {CCI_REG8(0x3632), 0x05}, /* ADC */
>> + {CCI_REG8(0x376b), 0x40}, /* Sensor control */
>> + {CCI_REG8(0x3814), 0x03}, /* H_INC_ODD */
>> + {CCI_REG8(0x3821), 0x07}, /* TIMING_FORMAT_2 hor_binning = 1*/
>> + {CCI_REG8(0x382a), 0x03}, /* V_INC_ODD */
>> + {CCI_REG8(0x3830), 0x08}, /* BLC_NUM_OPTION blc_use_num_2 = 1 */
>> + {CCI_REG8(0x3836), 0x02}, /* TIMING_REG_36 r_zline_use_num_2 = 1 */
>> + {CCI_REG8(0x4001), 0x50}, /* BLC DEBUG MODE */
>> + {CCI_REG8(0x4502), 0x44}, /* ADC synch control*/
>
> Spaces inside { }'s, please.
Do you mean this array only, or ov4689_common_regs too?
>> +};
>> +
>> static const u64 link_freq_menu_items[] = { 504000000 };
>>
>> static const char *const ov4689_test_pattern_menu[] = {
>> @@ -305,18 +317,96 @@ static const struct ov4689_gain_range ov4689_gain_ranges[] = {
>> },
>> };
>>
>> +/*
>> + * For now, only 2x2 binning implemented in this driver.
>> + */
>> +static int ov4689_best_binning(struct ov4689 *ov4689,
>> + const struct v4l2_mbus_framefmt *format,
>> + const struct v4l2_rect *crop,
>> + unsigned int *binning)
>> +{
>> + const struct v4l2_area candidates[] = {
>> + { crop->width, crop->height },
>> + { crop->width / 2, crop->height / 2 },
>> + };
>> +
>> + const struct v4l2_area *best;
>> + int index;
>> +
>> + best = v4l2_find_nearest_size(candidates, ARRAY_SIZE(candidates), width,
>> + height, format->width, format->height);
>
> You can assume v4l2_find_nearest_size() returns a non-NULL value (see the
> other patch I cc'd you).
Ack, will remove the NULL check.
>> + if (!best) {
>> + dev_err(ov4689->dev,
>> + "failed to find best binning for requested mode\n");
>> + return -EINVAL;
>> + }
>> +
>> + index = best - candidates;
>> + *binning = index + 1;
>> +
>> + dev_dbg(ov4689->dev,
>> + "best_binning: crop=%dx%d format=%dx%d binning=%d\n",
>> + crop->width, crop->height, format->width, format->height,
>> + *binning);
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Minimum working HTS value for given output width (found
>> + * experimentally).
>> + */
>> +static unsigned int ov4689_hts_min(unsigned int width)
>> +{
>> + return max_t(unsigned int, 3156, 224 + width * 19 / 16);
>> +}
>> +
>> +static void ov4689_update_ctrl_ranges(struct ov4689 *ov4689, unsigned int width,
>> + unsigned int height)
>> +{
>> + struct v4l2_ctrl *exposure = ov4689->exposure;
>> + struct v4l2_ctrl *vblank = ov4689->vblank;
>> + struct v4l2_ctrl *hblank = ov4689->hblank;
>> + s64 def_val, min_val, max_val;
>> +
>> + min_val = ov4689_hts_min(width) - width;
>> + max_val = OV4689_HTS_MAX - width;
>> + def_val = clamp_t(s64, hblank->default_value, min_val, max_val);
>> + __v4l2_ctrl_modify_range(hblank, min_val, max_val, hblank->step,
>> + def_val);
>
> Note that __v4l2_ctrl_modify_range() may fail. The problem isn't introduced
> by this patch but it'd be nice to fix it (but maybe in a separate patch).
Okay, will do this in a follow-up patch.
>> +
>> + min_val = OV4689_VBLANK_MIN;
>> + max_val = OV4689_HTS_MAX - width;
>> + def_val = clamp_t(s64, vblank->default_value, min_val, max_val);
>> + __v4l2_ctrl_modify_range(vblank, min_val, max_val, vblank->step,
>> + def_val);
>> +
>> + min_val = exposure->minimum;
>> + max_val = height + vblank->val - 4;
>> + def_val = clamp_t(s64, exposure->default_value, min_val, max_val);
>> + __v4l2_ctrl_modify_range(exposure, min_val, max_val, exposure->step,
>> + def_val);
>> +}
>> +
>> static int ov4689_set_fmt(struct v4l2_subdev *sd,
>> struct v4l2_subdev_state *sd_state,
>> struct v4l2_subdev_format *fmt)
>> {
>> + struct ov4689 *ov4689 = to_ov4689(sd);
>> struct v4l2_mbus_framefmt *format;
>> struct v4l2_rect *crop;
>> + unsigned int binning;
>> + int ret;
>>
>> crop = v4l2_subdev_state_get_crop(sd_state, fmt->pad);
>> format = v4l2_subdev_state_get_format(sd_state, fmt->pad);
>>
>> - format->width = crop->width;
>> - format->height = crop->height;
>> + ret = ov4689_best_binning(ov4689, &fmt->format, crop, &binning);
>> + if (ret)
>> + return ret;
>> +
>> + format->width = crop->width / binning;
>> + format->height = crop->height / binning;
>>
>> format->code = MEDIA_BUS_FMT_SBGGR10_1X10;
>> format->field = V4L2_FIELD_NONE;
>> @@ -327,6 +417,9 @@ static int ov4689_set_fmt(struct v4l2_subdev *sd,
>>
>> fmt->format = *format;
>>
>> + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>> + ov4689_update_ctrl_ranges(ov4689, format->width, format->height);
>> +
>> return 0;
>> }
>>
>> @@ -346,8 +439,9 @@ static int ov4689_enum_frame_sizes(struct v4l2_subdev *sd,
>> struct v4l2_subdev_frame_size_enum *fse)
>> {
>> const struct v4l2_rect *crop;
>> + int binning;
>>
>> - if (fse->index >= 1)
>> + if (fse->index >= 2)
>> return -EINVAL;
>>
>> if (fse->code != MEDIA_BUS_FMT_SBGGR10_1X10)
>> @@ -355,10 +449,11 @@ static int ov4689_enum_frame_sizes(struct v4l2_subdev *sd,
>>
>> crop = v4l2_subdev_state_get_crop(sd_state, 0);
>>
>> - fse->min_width = crop->width;
>> - fse->max_width = crop->width;
>> - fse->max_height = crop->height;
>> - fse->min_height = crop->height;
>> + binning = fse->index + 1;
>> + fse->min_width = crop->width / binning;
>> + fse->max_width = crop->width / binning;
>> + fse->max_height = crop->height / binning;
>> + fse->min_height = crop->height / binning;
>>
>> return 0;
>> }
>> @@ -398,42 +493,6 @@ static int ov4689_get_selection(struct v4l2_subdev *sd,
>> return -EINVAL;
>> }
>>
>> -/*
>> - * Minimum working HTS value for given output width (found
>> - * experimentally).
>> - */
>> -static unsigned int ov4689_hts_min(unsigned int width)
>> -{
>> - return max_t(unsigned int, 3156, 224 + width * 19 / 16);
>> -}
>> -
>> -static void ov4689_update_ctrl_ranges(struct ov4689 *ov4689,
>> - struct v4l2_rect *crop)
>> -{
>> - struct v4l2_ctrl *exposure = ov4689->exposure;
>> - struct v4l2_ctrl *vblank = ov4689->vblank;
>> - struct v4l2_ctrl *hblank = ov4689->hblank;
>> - s64 def_val, min_val, max_val;
>> -
>> - min_val = ov4689_hts_min(crop->width) - crop->width;
>> - max_val = OV4689_HTS_MAX - crop->width;
>> - def_val = clamp_t(s64, hblank->default_value, min_val, max_val);
>> - __v4l2_ctrl_modify_range(hblank, min_val, max_val, hblank->step,
>> - def_val);
>> -
>> - min_val = OV4689_VBLANK_MIN;
>> - max_val = OV4689_HTS_MAX - crop->width;
>> - def_val = clamp_t(s64, vblank->default_value, min_val, max_val);
>> - __v4l2_ctrl_modify_range(vblank, min_val, max_val, vblank->step,
>> - def_val);
>> -
>> - min_val = exposure->minimum;
>> - max_val = crop->height + vblank->val - 4;
>> - def_val = clamp_t(s64, exposure->default_value, min_val, max_val);
>> - __v4l2_ctrl_modify_range(exposure, min_val, max_val, exposure->step,
>> - def_val);
>> -}
>> -
>> static int ov4689_set_selection(struct v4l2_subdev *sd,
>> struct v4l2_subdev_state *state,
>> struct v4l2_subdev_selection *sel)
>> @@ -470,7 +529,8 @@ static int ov4689_set_selection(struct v4l2_subdev *sd,
>> format->height = rect.height;
>>
>> if (sel->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>> - ov4689_update_ctrl_ranges(ov4689, &rect);
>> + ov4689_update_ctrl_ranges(ov4689, rect.width,
>> + rect.height);
>> }
>>
>> *crop = rect;
>> @@ -485,21 +545,27 @@ static int ov4689_setup_timings(struct ov4689 *ov4689,
>> const struct v4l2_mbus_framefmt *format;
>> struct regmap *rm = ov4689->regmap;
>> const struct v4l2_rect *crop;
>> + const int v_offset = 2;
>> + unsigned int binning;
>> int ret = 0;
>>
>> format = v4l2_subdev_state_get_format(state, 0);
>> crop = v4l2_subdev_state_get_crop(state, 0);
>>
>> + ret = ov4689_best_binning(ov4689, format, crop, &binning);
>> + if (ret)
>> + return ret;
>> +
>> cci_write(rm, OV4689_REG_H_CROP_START, crop->left, &ret);
>> - cci_write(rm, OV4689_REG_V_CROP_START, crop->top, &ret);
>> - cci_write(rm, OV4689_REG_H_CROP_END, crop->left + crop->width + 1, &ret);
>> - cci_write(rm, OV4689_REG_V_CROP_END, crop->top + crop->height + 1, &ret);
>> + cci_write(rm, OV4689_REG_V_CROP_START, crop->top - v_offset, &ret);
>> + cci_write(rm, OV4689_REG_H_CROP_END, crop->left + crop->width + 3, &ret);
>> + cci_write(rm, OV4689_REG_V_CROP_END, crop->top + crop->height + 7, &ret);
>>
>> cci_write(rm, OV4689_REG_H_OUTPUT_SIZE, format->width, &ret);
>> cci_write(rm, OV4689_REG_V_OUTPUT_SIZE, format->height, &ret);
>>
>> cci_write(rm, OV4689_REG_H_WIN_OFF, 0, &ret);
>> - cci_write(rm, OV4689_REG_V_WIN_OFF, 0, &ret);
>> + cci_write(rm, OV4689_REG_V_WIN_OFF, v_offset, &ret);
>>
>> /*
>> * Maximum working value of vfifo_read_start for given output
>> @@ -507,6 +573,10 @@ static int ov4689_setup_timings(struct ov4689 *ov4689,
>> */
>> cci_write(rm, OV4689_REG_VFIFO_CTRL_01, format->width / 16 - 1, &ret);
>>
>> + if (binning == 2)
>> + cci_multi_reg_write(ov4689->regmap, ov4689_2x2_binning_regs,
>> + ARRAY_SIZE(ov4689_2x2_binning_regs),
>> + &ret);
>> return ret;
>> }
>>
>> @@ -519,20 +589,20 @@ static int ov4689_setup_blc_anchors(struct ov4689 *ov4689,
>> struct v4l2_subdev_state *state)
>> {
>> unsigned int width_def = OV4689_H_OUTPUT_SIZE_DEFAULT;
>> + const struct v4l2_mbus_framefmt *format;
>> struct regmap *rm = ov4689->regmap;
>> - const struct v4l2_rect *crop;
>> int ret = 0;
>>
>> - crop = v4l2_subdev_state_get_crop(state, 0);
>> + format = v4l2_subdev_state_get_format(state, 0);
>>
>> cci_write(rm, OV4689_REG_ANCHOR_LEFT_START,
>> - OV4689_ANCHOR_LEFT_START_DEF * crop->width / width_def, &ret);
>> + OV4689_ANCHOR_LEFT_START_DEF * format->width / width_def, &ret);
>> cci_write(rm, OV4689_REG_ANCHOR_LEFT_END,
>> - OV4689_ANCHOR_LEFT_END_DEF * crop->width / width_def, &ret);
>> + OV4689_ANCHOR_LEFT_END_DEF * format->width / width_def, &ret);
>> cci_write(rm, OV4689_REG_ANCHOR_RIGHT_START,
>> - OV4689_ANCHOR_RIGHT_START_DEF * crop->width / width_def, &ret);
>> + OV4689_ANCHOR_RIGHT_START_DEF * format->width / width_def, &ret);
>> cci_write(rm, OV4689_REG_ANCHOR_RIGHT_END,
>> - OV4689_ANCHOR_RIGHT_END_DEF * crop->width / width_def, &ret);
>> + OV4689_ANCHOR_RIGHT_END_DEF * format->width / width_def, &ret);
>>
>> return ret;
>> }
>> @@ -749,19 +819,19 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
>> struct regmap *regmap = ov4689->regmap;
>> struct v4l2_subdev_state *sd_state;
>> struct device *dev = ov4689->dev;
>> - struct v4l2_rect *crop;
>> + struct v4l2_mbus_framefmt *fmt;
>> s64 max_expo, def_expo;
>> int sensor_gain;
>> int ret = 0;
>>
>> sd_state = v4l2_subdev_get_locked_active_state(&ov4689->subdev);
>> - crop = v4l2_subdev_state_get_crop(sd_state, 0);
>> + fmt = v4l2_subdev_state_get_format(sd_state, 0);
>>
>> /* Propagate change of current control to all related controls */
>> switch (ctrl->id) {
>> case V4L2_CID_VBLANK:
>> /* Update max exposure while meeting expected vblanking */
>> - max_expo = crop->height + ctrl->val - 4;
>> + max_expo = fmt->height + ctrl->val - 4;
>> def_expo = clamp_t(s64, ov4689->exposure->default_value,
>> ov4689->exposure->minimum, max_expo);
>>
>> @@ -785,16 +855,14 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
>> cci_write(regmap, OV4689_REG_GAIN, sensor_gain, &ret);
>> break;
>> case V4L2_CID_VBLANK:
>> - cci_write(regmap, OV4689_REG_VTS,
>> - ctrl->val + crop->height, &ret);
>> + cci_write(regmap, OV4689_REG_VTS, ctrl->val + fmt->height, &ret);
>> break;
>> case V4L2_CID_TEST_PATTERN:
>> ret = ov4689_enable_test_pattern(ov4689, ctrl->val);
>> break;
>> case V4L2_CID_HBLANK:
>> cci_write(regmap, OV4689_REG_HTS,
>> - (ctrl->val + crop->width) /
>> - OV4689_HTS_DIVIDER, &ret);
>> + (ctrl->val + fmt->width) / OV4689_HTS_DIVIDER, &ret);
>> break;
>> case V4L2_CID_VFLIP:
>> cci_update_bits(regmap, OV4689_REG_TIMING_FORMAT1,
--
Best regards,
Mikhail Rudenko
Hi Sakari,
On 2024-04-15 at 06:08 GMT, Sakari Ailus <[email protected]> wrote:
> Hi Mikhail,
>
> On Tue, Apr 02, 2024 at 07:45:48PM +0300, Mikhail Rudenko wrote:
>> Implement configurable analogue crop via .set_selection call.
>> ov4689_init_cfg is modified to initialize default subdev selection.
>> Offsets are aligned to 2 to preserve Bayer order, selection width is
>> aligned to 4 and height to 2 to meet hardware requirements.
>>
>> Experimentally discovered values of the cropping-related registers and
>> vfifo_read_start for various output sizes are used. Default BLC anchor
>> positions are used for the default analogue crop, scaling down
>> proportionally for the smaller crop sizes.
>>
>> When analogue crop is adjusted, several consequential actions take
>> place: the output format is reset, exposure/vblank/hblank control
>> ranges and default values are adjusted accordingly. Additionally,
>> ov4689_set_ctrl utilizes pad crop instead of cur_mode width and
>> height for HTS and VTS calculation. Also, ov4689_enum_frame_sizes is
>> modified to report crop size as available frame size.
>
> We're amidst of a change to the APIs touching sensors with the the
> introduction of the internal pads.
> <URL:https://lore.kernel.org/linux-media/[email protected]/T/#t>.
>
> I'd therefore postpone this bit so it would align with the new practices
> (also subject to change in the metadata set).
>
> The rest of the patches would seem more or less ready for merging to me.
Okay, so I'll post a v5 of patches 1-16 with whitespace fixes (as you
suggested in patch 20) soon, and the remaining patches affected by the
metadata-related API changes as a separate series as soon those changes
land in media_stage. Do I get you right?
--
Best regards,
Mikhail Rudenko
On Mon, Apr 15, 2024 at 11:22:09PM +0300, Mikhail Rudenko wrote:
>
> Hi Sakari,
>
> On 2024-04-15 at 06:08 GMT, Sakari Ailus <[email protected]> wrote:
>
> > Hi Mikhail,
> >
> > On Tue, Apr 02, 2024 at 07:45:48PM +0300, Mikhail Rudenko wrote:
> >> Implement configurable analogue crop via .set_selection call.
> >> ov4689_init_cfg is modified to initialize default subdev selection.
> >> Offsets are aligned to 2 to preserve Bayer order, selection width is
> >> aligned to 4 and height to 2 to meet hardware requirements.
> >>
> >> Experimentally discovered values of the cropping-related registers and
> >> vfifo_read_start for various output sizes are used. Default BLC anchor
> >> positions are used for the default analogue crop, scaling down
> >> proportionally for the smaller crop sizes.
> >>
> >> When analogue crop is adjusted, several consequential actions take
> >> place: the output format is reset, exposure/vblank/hblank control
> >> ranges and default values are adjusted accordingly. Additionally,
> >> ov4689_set_ctrl utilizes pad crop instead of cur_mode width and
> >> height for HTS and VTS calculation. Also, ov4689_enum_frame_sizes is
> >> modified to report crop size as available frame size.
> >
> > We're amidst of a change to the APIs touching sensors with the the
> > introduction of the internal pads.
> > <URL:https://lore.kernel.org/linux-media/[email protected]/T/#t>.
> >
> > I'd therefore postpone this bit so it would align with the new practices
> > (also subject to change in the metadata set).
> >
> > The rest of the patches would seem more or less ready for merging to me.
>
> Okay, so I'll post a v5 of patches 1-16 with whitespace fixes (as you
> suggested in patch 20) soon, and the remaining patches affected by the
> metadata-related API changes as a separate series as soon those changes
> land in media_stage. Do I get you right?
Yes, please.
--
Sakari Ailus
On 2024-04-16 at 06:47 GMT, Sakari Ailus <[email protected]> wrote:
> On Mon, Apr 15, 2024 at 11:22:09PM +0300, Mikhail Rudenko wrote:
>>
>> Hi Sakari,
>>
>> On 2024-04-15 at 06:08 GMT, Sakari Ailus <[email protected]> wrote:
>>
>> > Hi Mikhail,
>> >
>> > On Tue, Apr 02, 2024 at 07:45:48PM +0300, Mikhail Rudenko wrote:
>> >> Implement configurable analogue crop via .set_selection call.
>> >> ov4689_init_cfg is modified to initialize default subdev selection.
>> >> Offsets are aligned to 2 to preserve Bayer order, selection width is
>> >> aligned to 4 and height to 2 to meet hardware requirements.
>> >>
>> >> Experimentally discovered values of the cropping-related registers and
>> >> vfifo_read_start for various output sizes are used. Default BLC anchor
>> >> positions are used for the default analogue crop, scaling down
>> >> proportionally for the smaller crop sizes.
>> >>
>> >> When analogue crop is adjusted, several consequential actions take
>> >> place: the output format is reset, exposure/vblank/hblank control
>> >> ranges and default values are adjusted accordingly. Additionally,
>> >> ov4689_set_ctrl utilizes pad crop instead of cur_mode width and
>> >> height for HTS and VTS calculation. Also, ov4689_enum_frame_sizes is
>> >> modified to report crop size as available frame size.
>> >
>> > We're amidst of a change to the APIs touching sensors with the the
>> > introduction of the internal pads.
>> > <URL:https://lore.kernel.org/linux-media/[email protected]/T/#t>.
>> >
>> > I'd therefore postpone this bit so it would align with the new practices
>> > (also subject to change in the metadata set).
>> >
>> > The rest of the patches would seem more or less ready for merging to me.
>>
>> Okay, so I'll post a v5 of patches 1-16 with whitespace fixes (as you
>> suggested in patch 20) soon, and the remaining patches affected by the
>> metadata-related API changes as a separate series as soon those changes
>> land in media_stage. Do I get you right?
>
> Yes, please.
Done, see [1].
[1] https://lore.kernel.org/all/[email protected]/
--
Best regards,
Mikhail Rudenko