[v4,00/20] Omnivision OV4689 refactoring and improvements

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

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/20240313072516.241106-1-sakari.ailus@linux.intel.com/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.

Hi Sakari,

On 2024-04-15 at 06:08 GMT, Sakari Ailus <sakari.ailus@linux.intel.com> 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/20240313072516.241106-1-sakari.ailus@linux.intel.com/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