| Message ID | 20231218174042.794012-1-mike.rudenko@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Omnivision OV4689 refactoring and improvements | expand |
Hi, On 2023-12-18 at 20:40 +03, Mikhail Rudenko <mike.rudenko@gmail.com> wrote: > Hi, > > This series contains 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. Gentle ping on this series. Anything I can do to help getting it reviewed and merged? Maybe split patches 15-20 which implement cropping and binning and change the driver away from register list based model into a separate series? Anyone? > 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 | 964 +++++++++++++++++++++++-------------- > 2 files changed, 592 insertions(+), 373 deletions(-) -- Best regards, Mikhail Rudenko
Hi Mikhail, On Wed, Feb 21, 2024 at 06:02:15PM +0300, Mikhail Rudenko wrote: > > Hi, > > On 2023-12-18 at 20:40 +03, Mikhail Rudenko <mike.rudenko@gmail.com> wrote: > > > Hi, > > > > This series contains 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. > > Gentle ping on this series. Anything I can do to help getting it > reviewed and merged? Maybe split patches 15-20 which implement cropping > and binning and change the driver away from register list based model > into a separate series? Anyone? Oops, my bad. I'll review these shortly. I can already tell there's not much to do though.
On Fri, Feb 23, 2024 at 08:15:41AM +0000, Sakari Ailus wrote: > Hi Mikhail, > > On Wed, Feb 21, 2024 at 06:02:15PM +0300, Mikhail Rudenko wrote: > > > > Hi, > > > > On 2023-12-18 at 20:40 +03, Mikhail Rudenko <mike.rudenko@gmail.com> wrote: > > > > > Hi, > > > > > > This series contains 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. > > > > Gentle ping on this series. Anything I can do to help getting it > > reviewed and merged? Maybe split patches 15-20 which implement cropping > > and binning and change the driver away from register list based model > > into a separate series? Anyone? > > Oops, my bad. I'll review these shortly. I can already tell there's not > much to do though. Done.
Hi Sakari, On 2024-02-23 at 08:43 GMT, Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > On Fri, Feb 23, 2024 at 08:15:41AM +0000, Sakari Ailus wrote: >> Hi Mikhail, >> >> On Wed, Feb 21, 2024 at 06:02:15PM +0300, Mikhail Rudenko wrote: >> > >> > Hi, >> > >> > On 2023-12-18 at 20:40 +03, Mikhail Rudenko <mike.rudenko@gmail.com> wrote: >> > >> > > Hi, >> > > >> > > This series contains 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. >> > >> > Gentle ping on this series. Anything I can do to help getting it >> > reviewed and merged? Maybe split patches 15-20 which implement cropping >> > and binning and change the driver away from register list based model >> > into a separate series? Anyone? >> >> Oops, my bad. I'll review these shortly. I can already tell there's not >> much to do though. > > Done. Thanks for the prompt review! I'll wait a few days to collect more comments (especially on patches 17, 18, 20) and then post v3. -- Best regards, Mikhail Rudenko