On Fri, Nov 25, 2022 at 10:12:24AM +0100, Jacopo Mondi wrote: > Hi Laurent > > On Wed, Nov 23, 2022 at 11:54:07AM +0200, Laurent Pinchart wrote: > > From: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx> > > > > The ov5640 driver incorrectly uses V4L2_CID_GAIN for the analogue gain. > > V4L2 has a specific control for analogue gain, V4L2_CID_ANALOGUE_GAIN. > > Use it. > > > > Signed-off-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > It's a bit of a shame we cannot report here your analysis of the gain > control handling on ov5640, so I'll link it here for reference I think we can :-) I'll post a v2 with an expanded commit message. > https://lists.libcamera.org/pipermail/libcamera-devel/2022-November/035655.html > > I agree with the above conclusions, until proven differently we can > consider 0x350a/b to control the analogue gain, as there's a separate > digital gain register, hence the below change makes sense to me. > > I've expanded the cc-list to who has been recently involved in ov5640 > developments. If required by any user, we should try to map > CID_GAIN on CID_ANALOGUE_GAIN for compatibility. > > From my side it's good the way it is: > Reviewed-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx> > > > --- > > drivers/media/i2c/ov5640.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c > > index 2d740397a5d4..c65c391bc1eb 100644 > > --- a/drivers/media/i2c/ov5640.c > > +++ b/drivers/media/i2c/ov5640.c > > @@ -3458,7 +3458,7 @@ static int ov5640_init_controls(struct ov5640_dev *sensor) > > /* Auto/manual gain */ > > ctrls->auto_gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_AUTOGAIN, > > 0, 1, 1, 1); > > - ctrls->gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_GAIN, > > + ctrls->gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_ANALOGUE_GAIN, > > 0, 1023, 1, 0); > > > > ctrls->saturation = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_SATURATION, -- Regards, Laurent Pinchart
