[v3,00/10] media: ar0521: Add analog gain, rework clock tree

  Hello,
  this series adds a few features to the ar0521 driver to enable its usage
with libcamera.

In particular:
- enum_frame_sizes
- global analog gain control
- LINK_FREQ
- Rework blanking handligs

v3 (Dave)
- Check __v4l2_ctrl_modify_range() return value
- Make LINK_FREQ readonly to avoid additional churn in s_ctrl
- Fix trivial early return in s_ctrl
- Use exposure's default value when modifying the controls' limits
- Change the exposure default to 0x70 to match the register default value

v2:
- I have dropped the most controverse part that allows to change the link
  frequency to obtain 60FPS. It can be eventually be applied on top.
- Use register 0x3028 to control analog gain not to overwrite the global digital
  gain.
- Fix the HBLANK/VBLANK max by using the values read from on-chip registers.
- Fix handling of LINK_FREQ in s_cltr (but do not make the control read only).
- Fix errors reported by 0-days:
  - use do_div() for 64-bit division
  - declare variables in function scope and not in case scope in s_ctrl

Jacopo Mondi (10):
  media: ar0521: Implement enum_frame_sizes
  media: ar0521: Add V4L2_CID_ANALOG_GAIN
  media: ar0521: Set maximum resolution to 2592x1944
  media: ar0521: Rework PLL computation
  media: ar0521: Refuse unsupported controls
  media: ar0521: Add LINK_FREQ control
  media: ar0521: Adjust exposure and blankings limits
  media: ar0521: Setup controls at s_stream time
  media: ar0521: Rework startup sequence
  media: ar0521: Tab-align definitions

 drivers/media/i2c/ar0521.c | 352 ++++++++++++++++++++++++++-----------
 1 file changed, 250 insertions(+), 102 deletions(-)

--
2.38.1
  

I just noticed patch 10/10 was missing.

Now sent in reply to this thread.

While at it, gentle ping to see if this can make it for 6.2 ?

Thanks
  j

On Fri, Nov 04, 2022 at 03:24:43PM +0100, Jacopo Mondi wrote:
> Hello,
>   this series adds a few features to the ar0521 driver to enable its usage
> with libcamera.
>
> In particular:
> - enum_frame_sizes
> - global analog gain control
> - LINK_FREQ
> - Rework blanking handligs
>
> v3 (Dave)
> - Check __v4l2_ctrl_modify_range() return value
> - Make LINK_FREQ readonly to avoid additional churn in s_ctrl
> - Fix trivial early return in s_ctrl
> - Use exposure's default value when modifying the controls' limits
> - Change the exposure default to 0x70 to match the register default value
>
> v2:
> - I have dropped the most controverse part that allows to change the link
>   frequency to obtain 60FPS. It can be eventually be applied on top.
> - Use register 0x3028 to control analog gain not to overwrite the global digital
>   gain.
> - Fix the HBLANK/VBLANK max by using the values read from on-chip registers.
> - Fix handling of LINK_FREQ in s_cltr (but do not make the control read only).
> - Fix errors reported by 0-days:
>   - use do_div() for 64-bit division
>   - declare variables in function scope and not in case scope in s_ctrl
>
> Jacopo Mondi (10):
>   media: ar0521: Implement enum_frame_sizes
>   media: ar0521: Add V4L2_CID_ANALOG_GAIN
>   media: ar0521: Set maximum resolution to 2592x1944
>   media: ar0521: Rework PLL computation
>   media: ar0521: Refuse unsupported controls
>   media: ar0521: Add LINK_FREQ control
>   media: ar0521: Adjust exposure and blankings limits
>   media: ar0521: Setup controls at s_stream time
>   media: ar0521: Rework startup sequence
>   media: ar0521: Tab-align definitions
>
>  drivers/media/i2c/ar0521.c | 352 ++++++++++++++++++++++++++-----------
>  1 file changed, 250 insertions(+), 102 deletions(-)
>
> --
> 2.38.1
>
  

Hi Jacopo,

On Mon, Nov 21, 2022 at 06:49:33PM +0100, Jacopo Mondi wrote:
> I just noticed patch 10/10 was missing.
> 
> Now sent in reply to this thread.
> 
> While at it, gentle ping to see if this can make it for 6.2 ?

I'm drowning in reviews, so you'll need another volunteer.

> On Fri, Nov 04, 2022 at 03:24:43PM +0100, Jacopo Mondi wrote:
> > Hello,
> >   this series adds a few features to the ar0521 driver to enable its usage
> > with libcamera.
> >
> > In particular:
> > - enum_frame_sizes
> > - global analog gain control
> > - LINK_FREQ
> > - Rework blanking handligs
> >
> > v3 (Dave)
> > - Check __v4l2_ctrl_modify_range() return value
> > - Make LINK_FREQ readonly to avoid additional churn in s_ctrl
> > - Fix trivial early return in s_ctrl
> > - Use exposure's default value when modifying the controls' limits
> > - Change the exposure default to 0x70 to match the register default value
> >
> > v2:
> > - I have dropped the most controverse part that allows to change the link
> >   frequency to obtain 60FPS. It can be eventually be applied on top.
> > - Use register 0x3028 to control analog gain not to overwrite the global digital
> >   gain.
> > - Fix the HBLANK/VBLANK max by using the values read from on-chip registers.
> > - Fix handling of LINK_FREQ in s_cltr (but do not make the control read only).
> > - Fix errors reported by 0-days:
> >   - use do_div() for 64-bit division
> >   - declare variables in function scope and not in case scope in s_ctrl
> >
> > Jacopo Mondi (10):
> >   media: ar0521: Implement enum_frame_sizes
> >   media: ar0521: Add V4L2_CID_ANALOG_GAIN
> >   media: ar0521: Set maximum resolution to 2592x1944
> >   media: ar0521: Rework PLL computation
> >   media: ar0521: Refuse unsupported controls
> >   media: ar0521: Add LINK_FREQ control
> >   media: ar0521: Adjust exposure and blankings limits
> >   media: ar0521: Setup controls at s_stream time
> >   media: ar0521: Rework startup sequence
> >   media: ar0521: Tab-align definitions
> >
> >  drivers/media/i2c/ar0521.c | 352 ++++++++++++++++++++++++++-----------
> >  1 file changed, 250 insertions(+), 102 deletions(-)
> >
  

Hi Laurent, Jacopo,

On Tue, Nov 22, 2022 at 01:38:41AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> On Mon, Nov 21, 2022 at 06:49:33PM +0100, Jacopo Mondi wrote:
> > I just noticed patch 10/10 was missing.
> > 
> > Now sent in reply to this thread.
> > 
> > While at it, gentle ping to see if this can make it for 6.2 ?
> 
> I'm drowning in reviews, so you'll need another volunteer.

I rather thought the set is good as there are no comments for almost three
weeks. It's in my tree now, I expect to send a PR soon...
  

Hi Jacopo and Co.,

Jacopo Mondi <jacopo@jmondi.org> writes:

> I just noticed patch 10/10 was missing.
>
> Now sent in reply to this thread.
>
> While at it, gentle ping to see if this can make it for 6.2 ?

I understand this has been tested on actual hw. I'd like to check how
does it work in my setup as well, but it is currently out of question.
This will eventually happen, though. For now I guess I can
Acked-by: Krzysztof Hałasa <khalasa@piap.pl>