On Thu, 21 Jul 2022 at 12:32, Alexander Stein
<alexander.stein@xxxxxxxxxxxxxxx> wrote:
>
> Hi Laurent,
>
> Am Donnerstag, 21. Juli 2022, 13:17:21 CEST schrieb Laurent Pinchart:
> > Hi Alexander,
> >
> > On Thu, Jul 21, 2022 at 12:05:46PM +0200, Alexander Stein wrote:
> > > Am Donnerstag, 21. Juli 2022, 10:35:35 CEST schrieb Laurent Pinchart:
> > > > Add support for the V4L2_CID_HBLANK and V4L2_CID_VBLANK controls to the
> > > > imx290 driver. Make the controls read-only to start with, to report the
> > > > values to userspace for timing calculation.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > > > ---
> > > >
> > > > drivers/media/i2c/imx290.c | 39 +++++++++++++++++++++++++++++++++++++-
> > > > 1 file changed, 38 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> > > > index 4408dd3e191f..7190399f4111 100644
> > > > --- a/drivers/media/i2c/imx290.c
> > > > +++ b/drivers/media/i2c/imx290.c
> > > > @@ -146,6 +146,8 @@ struct imx290 {
> > > >
> > > > struct v4l2_ctrl_handler ctrls;
> > > > struct v4l2_ctrl *link_freq;
> > > > struct v4l2_ctrl *pixel_rate;
> > > >
> > > > + struct v4l2_ctrl *hblank;
> > > > + struct v4l2_ctrl *vblank;
> > > >
> > > > struct mutex lock;
> > > >
> > > > };
> > > >
> > > > @@ -642,6 +644,20 @@ static int imx290_set_fmt(struct v4l2_subdev *sd,
> > > >
> > > > if (imx290->pixel_rate)
> > > >
> > > > __v4l2_ctrl_s_ctrl_int64(imx290->pixel_rate,
> > > >
> > > >
> imx290_calc_pixel_rate(imx290));
> > > >
> > > > +
> > > > + if (imx290->hblank) {
> > > > + unsigned int hblank = mode->hmax - mode-
> >width;
> > > > +
> > > > + __v4l2_ctrl_modify_range(imx290->hblank,
> hblank, hblank,
> > > > + 1, hblank);
> > > > + }
> > > > +
> > > > + if (imx290->vblank) {
> > > > + unsigned int vblank = IMX290_VMAX_DEFAULT -
> mode->height;
> > > > +
> > > > + __v4l2_ctrl_modify_range(imx290->vblank,
> vblank, vblank,
> > > > + 1, vblank);
> > > > + }
> > > >
> > > > }
> > > >
> > > > *format = fmt->format;
> > > >
> > > > @@ -880,9 +896,10 @@ static const struct media_entity_operations
> > > > imx290_subdev_entity_ops = {> >
> > > > static int imx290_ctrl_init(struct imx290 *imx290)
> > > > {
> > > >
> > > > + unsigned int blank;
> > > >
> > > > int ret;
> > > >
> > > > - v4l2_ctrl_handler_init(&imx290->ctrls, 5);
> > > > + v4l2_ctrl_handler_init(&imx290->ctrls, 7);
> > > >
> > > > imx290->ctrls.lock = &imx290->lock;
> > > >
> > > > v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> > > >
> > > > @@ -910,6 +927,26 @@ static int imx290_ctrl_init(struct imx290 *imx290)
> > > >
> > > >
> ARRAY_SIZE(imx290_test_pattern_menu) - 1,
> > > > 0, 0,
> imx290_test_pattern_menu);
> > > >
> > > > + /*
> > > > + * Horizontal blanking is controlled through the HMAX register,
> which
> > > > + * contains a line length in INCK clock units. The INCK frequency
> is
> > > > + * fixed to 74.25 MHz. The HMAX value is currently fixed to 1100,
> > > > + * convert it to a number of pixels based on the nominal pixel
> rate.
> > > > + */
> > >
> > > Currently the driver only supports 37.125 MHz, please refer to
> > > imx290_probe.
> > Indeed. Re-reading the comment, I suspect something is wrong, as hmax is
> > not converted to pixels here (and is also not fixed to 1100). The only
> > datasheet I found that is publicly accessed doesn't explain very clearly
> > how the HMAX value should be computed. Based on your experience with IMX
> > sensors, would you be able to shed some light on this ?
It is pretty much a standard HTS setting based on a pixel rate that is
fixed at 148.5MPix/s. Likewise VMAX is equivalent to a traditional
VTS.
I've been through the same path in
https://github.com/raspberrypi/linux/commits/rpi-5.15.y/drivers/media/i2c/imx290.c
> Can you share the link to this datasheet you found?
> Sorry, my experience is more like try and error. I don't fully understand this
> as well, but apparently this depends on frame rate select (FRSEL).
FRSEL is the one difference between IMX327 and IMX290 (and presumably
IMX462 too). IMX290 adds "0" as a valid value for 120/100fps mode.
However there is no need to change FRSEL for standard frame rate
control - you can set it at 0x01 and get a full range of frame rates
through VMAX and HMAX. If you wished to extend that range for slower
rates, you could conditionally set it to 0x2 to double the frame time.
Dave
> Best regards,
> Alexander
>
> > > > + blank = imx290->current_mode->hmax - imx290->current_mode->width;
> > > > + imx290->hblank = v4l2_ctrl_new_std(&imx290->ctrls,
> &imx290_ctrl_ops,
> > > > + V4L2_CID_HBLANK,
> blank, blank, 1,
> > > > + blank);
> > > > + if (imx290->hblank)
> > > > + imx290->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > > > +
> > > > + blank = IMX290_VMAX_DEFAULT - imx290->current_mode->height;
> > > > + imx290->vblank = v4l2_ctrl_new_std(&imx290->ctrls,
> &imx290_ctrl_ops,
> > > > + V4L2_CID_VBLANK,
> blank, blank, 1,
> > > > + blank);
> > > > + if (imx290->vblank)
> > > > + imx290->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > > > +
> > > >
> > > > imx290->sd.ctrl_handler = &imx290->ctrls;
> > > >
> > > > if (imx290->ctrls.error) {
>
>
>
>
