mbox series

[00/19] Omnivision OV4689 refactoring and improvements

Message ID 20231211175023.1680247-1-mike.rudenko@gmail.com
Headers
Series Omnivision OV4689 refactoring and improvements |

Message

Mikhail Rudenko Dec. 11, 2023, 5:50 p.m. UTC 
  Hi,

This series contains refactoring and new features implementation for
the Omnivision OV4689 sensor driver. Specifically, patches 1, 2, 4, 5,
6, 9, 14, 15, 17, and 18 are refactorings, and are not supposed to
introduce any functional change. Patches 3 and 7 perform migration to
CCI helpers and subdevice active state respectively, and should not
introduce any hardware- and/or user-visible change either.

Patches 10-13 expose more sensor controls to the userspace, such as
(read-write) HBLANK, VFLIP/HFLIP, digital gain, and color
balance. Patch 16 implements configurable analogue crop rectangle via
.set_selection callback. And finally, patch 19 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.

Any comments and reviews are welcome!

Mikhail Rudenko (19):
  media: i2c: ov4689: Clean up and annotate the register table
  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: Set gain in one 16 bit write
  media: i2c: ov4689: Use sub-device active state
  media: i2c: ov4689: Enable runtime PM before registering sub-device
  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 | 951 ++++++++++++++++++++++---------------
 2 files changed, 579 insertions(+), 373 deletions(-)

--
2.43.0

Comments

Laurent Pinchart Dec. 11, 2023, 6:11 p.m. UTC | #1 
On Mon, Dec 11, 2023 at 08:50:09PM +0300, Mikhail Rudenko wrote:
> According to the datasheet, bits 0-7 of the AEC LONG GAIN
> register (0x3508) map to bits 8-15 of the gain value and no masking is
> required. Thus set analogue gain in a single 16-bit write instead of
> two 8-bit writes.
> 
> Signed-off-by: Mikhail Rudenko <mike.rudenko@gmail.com>

You could squash it with patch 03/19.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/i2c/ov4689.c | 15 ++-------------
>  1 file changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
> index 42700ecfbe0e..5392f650960c 100644
> --- a/drivers/media/i2c/ov4689.c
> +++ b/drivers/media/i2c/ov4689.c
> @@ -32,11 +32,7 @@
>  #define OV4689_EXPOSURE_STEP		1
>  #define OV4689_VTS_MAX			0x7fff
>  
> -#define OV4689_REG_GAIN_H		CCI_REG8(0x3508)
> -#define OV4689_REG_GAIN_L		CCI_REG8(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
>  
> @@ -613,14 +609,7 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl)
>  		break;
>  	case V4L2_CID_ANALOGUE_GAIN:
>  		ret = ov4689_map_gain(ov4689, val, &sensor_gain);
> -
> -		cci_write(regmap, OV4689_REG_GAIN_H,
> -			  (sensor_gain >> OV4689_GAIN_H_SHIFT) &
> -			  OV4689_GAIN_H_MASK, &ret);
> -
> -		cci_write(regmap, OV4689_REG_GAIN_L,
> -			  sensor_gain & OV4689_GAIN_L_MASK,
> -			  &ret);
> +		cci_write(regmap, OV4689_REG_GAIN, sensor_gain, &ret);
>  		break;
>  	case V4L2_CID_VBLANK:
>  		cci_write(regmap, OV4689_REG_VTS,