[v2,00/28] media: ov2680: Bugfixes + ACPI + selection(crop-tgt) API support

  Hi All,

Here is v2 of my ov2680 sensor driver patch series.

Changes in v2
- Drop "media: Add MIPI CCI register access helper functions"
  (being reviewed in its own thread / patch-submission)
- Drop "media: ov2680: Add g_skip_frames op support"
- Add "media: ov2680: Fix regulators being left enabled on
  ov2680_power_on() errors"
- Add "media: ov2680: Add link-freq and pixel-rate controls"
  with this the driver now works on IPU3 with ipu3-capture.sh
  (libcamera support requires adding a couple more controls)
- Limit line length to 80 chars everywhere
- Address various small remarks from Andy

During all the work done on the atomisp driver I have mostly been testing
on devices with an ov2680 sensor. As such I have also done a lot of work
on the atomisp-ov2680.c atomisp specific sensor driver.

With the latest atomisp code from:
https://git.kernel.org/pub/scm/linux/kernel/git/hansg/linux.git/tag/?h=media-atomisp-6.5-1

The atomisp code can now work with standard v4l2 sensor drivers using
the selections (crop-tgt) api and v4l2-async sensor driver registration.

This patch series modifies the main drivers/media/i2c/ov2680.c driver
to add bugfixes, ACPI enumeration, selection API support and further
improvments. After this the driver can be used with the atomisp driver
and atomisp-ov2680.c can be dropped.

This also gets the driver much closer to having everything needed for
use with IPU3 / libcamera. I have a Lenovo Miix 510 now with an IPU3 +
ov2680 sensor and with this series raw-capture using the ipu3-capture.sh
script works. I plan to work on libcamera support for this in the near
future.

This series consist of 3 parts:

1. Patches 1-8 are bugfixes these are put first for backporting

2. Patch 9 converts the ov2680 driver to the new CCI helpers,
the same has been done in the other series with the atomisp-ov2680
driver and this makes it much easier to sync things up.

Note this depends on the new CCI register helpers, these are being
reviewed here:

https://lore.kernel.org/linux-media/20230614192343.57280-1-hdegoede@redhat.com/

3. Patches 9 - 28 implement the ACPI enumeration,
selection API support and further improvments.

Regards,

Hans


Hans de Goede (28):
  media: ov2680: Remove auto-gain and auto-exposure controls
  media: ov2680: Fix ov2680_bayer_order()
  media: ov2680: Fix vflip / hflip set functions
  media: ov2680: Use select VIDEO_V4L2_SUBDEV_API
  media: ov2680: Don't take the lock for try_fmt calls
  media: ov2680: Add ov2680_fill_format() helper function
  media: ov2680: Fix ov2680_set_fmt() which == V4L2_SUBDEV_FORMAT_TRY
    not working
  media: ov2680: Fix regulators being left enabled on ov2680_power_on()
    errors
  media: ov2680: Convert to new CCI register access helpers
  media: ov2680: Store dev instead of i2c_client in ov2680_dev
  media: ov2680: Check for "powerdown" GPIO con-id before checking for
    "reset" GPIO con-id
  media: ov2680: Add runtime-pm support
  media: ov2680: Drop is_enabled flag
  media: ov2680: Add support for more clk setups
  media: ov2680: Add support for 19.2 MHz clock
  media: ov2680: Add endpoint matching support
  media: ov2680: Add support for ACPI enumeration
  media: ov2680: Fix ov2680_enum_frame_interval()
  media: ov2680: Annotate the per mode register setting lists
  media: ov2680: Add ov2680_mode struct
  media: ov2680: Make setting the mode algorithm based
  media: ov2680: Add an __ov2680_get_pad_format() helper function
  media: ov2680: Implement selection support
  media: ov2680: Fix exposure and gain ctrls range and default value
  media: ov2680: Add a bunch of register tweaks
  media: ov2680: Drop unnecessary pad checks
  media: ov2680: Read and log sensor revision during probe
  media: ov2680: Add link-freq and pixel-rate controls

 drivers/media/i2c/Kconfig  |    2 +
 drivers/media/i2c/ov2680.c | 1316 +++++++++++++++++++-----------------
 2 files changed, 689 insertions(+), 629 deletions(-)
  

Hi Hans,
Hans de Goede <hdegoede@redhat.com> writes:

> Hi All,
>
> Here is v2 of my ov2680 sensor driver patch series.
>
> Changes in v2
> - Drop "media: Add MIPI CCI register access helper functions"
>   (being reviewed in its own thread / patch-submission)
> - Drop "media: ov2680: Add g_skip_frames op support"
> - Add "media: ov2680: Fix regulators being left enabled on
>   ov2680_power_on() errors"
> - Add "media: ov2680: Add link-freq and pixel-rate controls"
>   with this the driver now works on IPU3 with ipu3-capture.sh
>   (libcamera support requires adding a couple more controls)
> - Limit line length to 80 chars everywhere
> - Address various small remarks from Andy
>
> During all the work done on the atomisp driver I have mostly been testing
> on devices with an ov2680 sensor. As such I have also done a lot of work
> on the atomisp-ov2680.c atomisp specific sensor driver.
>
> With the latest atomisp code from:
> https://git.kernel.org/pub/scm/linux/kernel/git/hansg/linux.git/tag/?h=media-atomisp-6.5-1
>
> The atomisp code can now work with standard v4l2 sensor drivers using
> the selections (crop-tgt) api and v4l2-async sensor driver registration.
>
> This patch series modifies the main drivers/media/i2c/ov2680.c driver
> to add bugfixes, ACPI enumeration, selection API support and further
> improvments. After this the driver can be used with the atomisp driver
> and atomisp-ov2680.c can be dropped.
>
> This also gets the driver much closer to having everything needed for
> use with IPU3 / libcamera. I have a Lenovo Miix 510 now with an IPU3 +
> ov2680 sensor and with this series raw-capture using the ipu3-capture.sh
> script works. I plan to work on libcamera support for this in the near
> future.
>
> This series consist of 3 parts:
>
> 1. Patches 1-8 are bugfixes these are put first for backporting
>
> 2. Patch 9 converts the ov2680 driver to the new CCI helpers,
> the same has been done in the other series with the atomisp-ov2680
> driver and this makes it much easier to sync things up.
>
> Note this depends on the new CCI register helpers, these are being
> reviewed here:
>
> https://lore.kernel.org/linux-media/20230614192343.57280-1-hdegoede@redhat.com/
>
> 3. Patches 9 - 28 implement the ACPI enumeration,
> selection API support and further improvments.

Wonder why you did not cc me, since the Maintainers entry is up to date
with my email on this driver.

Thanks a lot for all this work, I had at the time a very limited iot device
without IPU and could only do processing offline, and got out of working
on this device very quick.

So, looks like you have a much robust setup and test scenarios. Again
thanks for the fixes and updates using new api and extend functionality
on this one. I went over it and all looks pretty good, so for the all
series:

Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>

And please, if you agree of course, if you send a new version of this
series or as a follow up patch, I think would make sense to add you as
maintainer also.

Cheers,
   Rui

>
> Regards,
>
> Hans
>
>
> Hans de Goede (28):
>   media: ov2680: Remove auto-gain and auto-exposure controls
>   media: ov2680: Fix ov2680_bayer_order()
>   media: ov2680: Fix vflip / hflip set functions
>   media: ov2680: Use select VIDEO_V4L2_SUBDEV_API
>   media: ov2680: Don't take the lock for try_fmt calls
>   media: ov2680: Add ov2680_fill_format() helper function
>   media: ov2680: Fix ov2680_set_fmt() which == V4L2_SUBDEV_FORMAT_TRY
>     not working
>   media: ov2680: Fix regulators being left enabled on ov2680_power_on()
>     errors
>   media: ov2680: Convert to new CCI register access helpers
>   media: ov2680: Store dev instead of i2c_client in ov2680_dev
>   media: ov2680: Check for "powerdown" GPIO con-id before checking for
>     "reset" GPIO con-id
>   media: ov2680: Add runtime-pm support
>   media: ov2680: Drop is_enabled flag
>   media: ov2680: Add support for more clk setups
>   media: ov2680: Add support for 19.2 MHz clock
>   media: ov2680: Add endpoint matching support
>   media: ov2680: Add support for ACPI enumeration
>   media: ov2680: Fix ov2680_enum_frame_interval()
>   media: ov2680: Annotate the per mode register setting lists
>   media: ov2680: Add ov2680_mode struct
>   media: ov2680: Make setting the mode algorithm based
>   media: ov2680: Add an __ov2680_get_pad_format() helper function
>   media: ov2680: Implement selection support
>   media: ov2680: Fix exposure and gain ctrls range and default value
>   media: ov2680: Add a bunch of register tweaks
>   media: ov2680: Drop unnecessary pad checks
>   media: ov2680: Read and log sensor revision during probe
>   media: ov2680: Add link-freq and pixel-rate controls
>
>  drivers/media/i2c/Kconfig  |    2 +
>  drivers/media/i2c/ov2680.c | 1316 +++++++++++++++++++-----------------
>  2 files changed, 689 insertions(+), 629 deletions(-)
>
> -- 
> 2.40.1
  

Hi Rui,

On 6/15/23 19:32, Rui Miguel Silva wrote:
> Hi Hans,
> Hans de Goede <hdegoede@redhat.com> writes:
> 
>> Hi All,
>>
>> Here is v2 of my ov2680 sensor driver patch series.
>>
>> Changes in v2
>> - Drop "media: Add MIPI CCI register access helper functions"
>>   (being reviewed in its own thread / patch-submission)
>> - Drop "media: ov2680: Add g_skip_frames op support"
>> - Add "media: ov2680: Fix regulators being left enabled on
>>   ov2680_power_on() errors"
>> - Add "media: ov2680: Add link-freq and pixel-rate controls"
>>   with this the driver now works on IPU3 with ipu3-capture.sh
>>   (libcamera support requires adding a couple more controls)
>> - Limit line length to 80 chars everywhere
>> - Address various small remarks from Andy
>>
>> During all the work done on the atomisp driver I have mostly been testing
>> on devices with an ov2680 sensor. As such I have also done a lot of work
>> on the atomisp-ov2680.c atomisp specific sensor driver.
>>
>> With the latest atomisp code from:
>> https://git.kernel.org/pub/scm/linux/kernel/git/hansg/linux.git/tag/?h=media-atomisp-6.5-1
>>
>> The atomisp code can now work with standard v4l2 sensor drivers using
>> the selections (crop-tgt) api and v4l2-async sensor driver registration.
>>
>> This patch series modifies the main drivers/media/i2c/ov2680.c driver
>> to add bugfixes, ACPI enumeration, selection API support and further
>> improvments. After this the driver can be used with the atomisp driver
>> and atomisp-ov2680.c can be dropped.
>>
>> This also gets the driver much closer to having everything needed for
>> use with IPU3 / libcamera. I have a Lenovo Miix 510 now with an IPU3 +
>> ov2680 sensor and with this series raw-capture using the ipu3-capture.sh
>> script works. I plan to work on libcamera support for this in the near
>> future.
>>
>> This series consist of 3 parts:
>>
>> 1. Patches 1-8 are bugfixes these are put first for backporting
>>
>> 2. Patch 9 converts the ov2680 driver to the new CCI helpers,
>> the same has been done in the other series with the atomisp-ov2680
>> driver and this makes it much easier to sync things up.
>>
>> Note this depends on the new CCI register helpers, these are being
>> reviewed here:
>>
>> https://lore.kernel.org/linux-media/20230614192343.57280-1-hdegoede@redhat.com/
>>
>> 3. Patches 9 - 28 implement the ACPI enumeration,
>> selection API support and further improvments.
> 
> Wonder why you did not cc me, since the Maintainers entry is up to date
> with my email on this driver.

No particular reason. People already pointed this out during
the v1 posting of this series and I still managed to forget
to add you for v2. My bad, sorry.

> Thanks a lot for all this work, I had at the time a very limited iot device
> without IPU and could only do processing offline, and got out of working
> on this device very quick.
> 
> So, looks like you have a much robust setup and test scenarios. Again
> thanks for the fixes and updates using new api and extend functionality
> on this one. I went over it and all looks pretty good, so for the all
> series:
> 
> Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>

Thank you.

> And please, if you agree of course, if you send a new version of this
> series or as a follow up patch, I think would make sense to add you as
> maintainer also.

Ack I'll add a patch to do that for v3 (I'll add it to my local tree
right away).

Regards,

Hans


>> Hans de Goede (28):
>>   media: ov2680: Remove auto-gain and auto-exposure controls
>>   media: ov2680: Fix ov2680_bayer_order()
>>   media: ov2680: Fix vflip / hflip set functions
>>   media: ov2680: Use select VIDEO_V4L2_SUBDEV_API
>>   media: ov2680: Don't take the lock for try_fmt calls
>>   media: ov2680: Add ov2680_fill_format() helper function
>>   media: ov2680: Fix ov2680_set_fmt() which == V4L2_SUBDEV_FORMAT_TRY
>>     not working
>>   media: ov2680: Fix regulators being left enabled on ov2680_power_on()
>>     errors
>>   media: ov2680: Convert to new CCI register access helpers
>>   media: ov2680: Store dev instead of i2c_client in ov2680_dev
>>   media: ov2680: Check for "powerdown" GPIO con-id before checking for
>>     "reset" GPIO con-id
>>   media: ov2680: Add runtime-pm support
>>   media: ov2680: Drop is_enabled flag
>>   media: ov2680: Add support for more clk setups
>>   media: ov2680: Add support for 19.2 MHz clock
>>   media: ov2680: Add endpoint matching support
>>   media: ov2680: Add support for ACPI enumeration
>>   media: ov2680: Fix ov2680_enum_frame_interval()
>>   media: ov2680: Annotate the per mode register setting lists
>>   media: ov2680: Add ov2680_mode struct
>>   media: ov2680: Make setting the mode algorithm based
>>   media: ov2680: Add an __ov2680_get_pad_format() helper function
>>   media: ov2680: Implement selection support
>>   media: ov2680: Fix exposure and gain ctrls range and default value
>>   media: ov2680: Add a bunch of register tweaks
>>   media: ov2680: Drop unnecessary pad checks
>>   media: ov2680: Read and log sensor revision during probe
>>   media: ov2680: Add link-freq and pixel-rate controls
>>
>>  drivers/media/i2c/Kconfig  |    2 +
>>  drivers/media/i2c/ov2680.c | 1316 +++++++++++++++++++-----------------
>>  2 files changed, 689 insertions(+), 629 deletions(-)
>>
>> -- 
>> 2.40.1
>