From: Dave Stevenson <dave.stevenson@raspberrypi.com>
To: Jacopo Mondi <jacopo@jmondi.org>
Cc: paul.j.murphy@intel.com, daniele.alessandrelli@intel.com,
linux-media@vger.kernel.org
Subject: Re: [PATCH 10/16] media: i2c: ov9282: Action CID_VBLANK when set.
Date: Thu, 6 Oct 2022 14:21:57 +0100 [thread overview]
Message-ID: <CAPY8ntBmB++um4GTwqg2CUXdSxuRxdJibi0nX3E0ZRpZ2imCZQ@mail.gmail.com> (raw)
In-Reply-To: <20221006092948.aosyb6weieoevlbq@uno.localdomain>
Hi Jacopo
On Thu, 6 Oct 2022 at 10:29, Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi Dave
>
> On Wed, Oct 05, 2022 at 04:28:03PM +0100, Dave Stevenson wrote:
> > Programming the sensor with TIMING_VTS (aka LPFR) was done
> > when triggered by a change in exposure or gain, but not
> > when V4L2_CID_VBLANK was changed. Dynamic frame rate
> > changes could therefore not be achieved.
> >
> > Separate out programming TIMING_VTS so that it is triggered
> > by set_ctrl(V4L2_CID_VBLANK)
> >
> > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > ---
> > drivers/media/i2c/ov9282.c | 29 ++++++++++++++++-------------
> > 1 file changed, 16 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > index 183283d191b1..5ddef6e2b3ac 100644
> > --- a/drivers/media/i2c/ov9282.c
> > +++ b/drivers/media/i2c/ov9282.c
> > @@ -419,22 +419,15 @@ static int ov9282_update_controls(struct ov9282 *ov9282,
> > */
> > static int ov9282_update_exp_gain(struct ov9282 *ov9282, u32 exposure, u32 gain)
> > {
> > - u32 lpfr;
> > int ret;
> >
> > - lpfr = ov9282->vblank + ov9282->cur_mode->height;
> > -
> > - dev_dbg(ov9282->dev, "Set exp %u, analog gain %u, lpfr %u",
> > - exposure, gain, lpfr);
> > + dev_dbg(ov9282->dev, "Set exp %u, analog gain %u",
> > + exposure, gain);
> >
> > ret = ov9282_write_reg(ov9282, OV9282_REG_HOLD, 1, 1);
> > if (ret)
> > return ret;
> >
> > - ret = ov9282_write_reg(ov9282, OV9282_REG_LPFR, 2, lpfr);
> > - if (ret)
> > - goto error_release_group_hold;
> > -
> > ret = ov9282_write_reg(ov9282, OV9282_REG_EXPOSURE, 3, exposure << 4);
> > if (ret)
> > goto error_release_group_hold;
> > @@ -465,6 +458,7 @@ static int ov9282_set_ctrl(struct v4l2_ctrl *ctrl)
> > container_of(ctrl->handler, struct ov9282, ctrl_handler);
> > u32 analog_gain;
> > u32 exposure;
> > + u32 lpfr;
>
> Only a nit about the fact lpfr is a u32 while you're writing 2 bytes.
> I guess it's safe as we likely don't risk any overflow
I was moving u32 lpfr from ov9282_update_exp_gain to here. All the
handling of lpfr (aka TIMING_VTS) is done as u32 in this driver.
The max range of V4L2_CID_VBLANK is set from the ov9282_mode
definition (not strictly necessary as it should be a fixed max value
handled by the register), so as long as the mode is defined correctly
then there should be no overflow.
Dave
> > int ret;
> >
> > switch (ctrl->id) {
> > @@ -482,10 +476,14 @@ static int ov9282_set_ctrl(struct v4l2_ctrl *ctrl)
> > OV9282_EXPOSURE_OFFSET,
> > 1, OV9282_EXPOSURE_DEFAULT);
> > break;
> > + }
> > +
> > + /* Set controls only if sensor is in power on state */
> > + if (!pm_runtime_get_if_in_use(ov9282->dev))
> > + return 0;
> > +
> > + switch (ctrl->id) {
> > case V4L2_CID_EXPOSURE:
> > - /* Set controls only if sensor is in power on state */
> > - if (!pm_runtime_get_if_in_use(ov9282->dev))
> > - return 0;
> >
> > exposure = ctrl->val;
> > analog_gain = ov9282->again_ctrl->val;
> > @@ -495,14 +493,19 @@ static int ov9282_set_ctrl(struct v4l2_ctrl *ctrl)
> >
> > ret = ov9282_update_exp_gain(ov9282, exposure, analog_gain);
> >
> > - pm_runtime_put(ov9282->dev);
> >
> Double empty line
> With this fixed:
>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Thanks
> j
> > + break;
> > + case V4L2_CID_VBLANK:
> > + lpfr = ov9282->vblank + ov9282->cur_mode->height;
> > + ret = ov9282_write_reg(ov9282, OV9282_REG_LPFR, 2, lpfr);
> > break;
> > default:
> > dev_err(ov9282->dev, "Invalid control %d", ctrl->id);
> > ret = -EINVAL;
> > }
> >
> > + pm_runtime_put(ov9282->dev);
> > +
> > return ret;
> > }
> >
> > --
> > 2.34.1
> >
next prev parent reply other threads:[~2022-10-06 13:22 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-05 15:27 [PATCH 00/16] Updates to ov9282 sensor driver Dave Stevenson
2022-10-05 15:27 ` [PATCH 01/16] media: i2c: ov9282: Remove duplication of registers Dave Stevenson
2022-10-06 9:14 ` Jacopo Mondi
2022-10-05 15:27 ` [PATCH 02/16] media: i2c: ov9282: Split registers into common and mode specific Dave Stevenson
2022-10-06 9:15 ` Jacopo Mondi
2022-10-05 15:27 ` [PATCH 03/16] media: i2c: ov9282: Remove format code from the mode Dave Stevenson
2022-10-06 9:15 ` Jacopo Mondi
2022-10-05 15:27 ` [PATCH 04/16] media: i2c: ov9282: Remove pixel rate from mode definition Dave Stevenson
2022-10-06 9:17 ` Jacopo Mondi
2022-10-06 11:51 ` Dave Stevenson
2022-10-05 15:27 ` [PATCH 05/16] media: i2c: ov9281: Support more than 1 mode Dave Stevenson
2022-10-06 9:18 ` Jacopo Mondi
2022-10-26 7:22 ` Sakari Ailus
2022-10-05 15:27 ` [PATCH 06/16] media: i2c: ov9282: Correct HTS register for configured pixel rate Dave Stevenson
2022-10-06 9:23 ` Jacopo Mondi
2022-10-06 13:01 ` Dave Stevenson
2022-10-05 15:28 ` [PATCH 07/16] media: i2c: ov9282: Reduce vblank_min values based on testing Dave Stevenson
2022-10-06 11:56 ` Jacopo Mondi
2022-10-06 13:02 ` Dave Stevenson
2022-10-05 15:28 ` [PATCH 08/16] media: i2c: ov9282: Add selection for CSI2 clock mode Dave Stevenson
2022-10-06 9:24 ` Jacopo Mondi
2022-10-26 7:21 ` Sakari Ailus
2022-10-28 12:57 ` Dave Stevenson
2022-10-28 14:30 ` Sakari Ailus
2022-10-28 15:03 ` Dave Stevenson
2022-10-31 13:06 ` Sakari Ailus
2022-10-05 15:28 ` [PATCH 09/16] media: i2c: ov9282: Add the properties from fwnode Dave Stevenson
2022-10-06 11:57 ` Jacopo Mondi
2022-10-05 15:28 ` [PATCH 10/16] media: i2c: ov9282: Action CID_VBLANK when set Dave Stevenson
2022-10-06 9:29 ` Jacopo Mondi
2022-10-06 13:21 ` Dave Stevenson [this message]
2022-10-05 15:28 ` [PATCH 11/16] media: i2c: ov9282: Add HFLIP and VFLIP support Dave Stevenson
2022-10-06 9:38 ` Jacopo Mondi
2022-10-06 14:21 ` Dave Stevenson
2022-10-05 15:28 ` [PATCH 12/16] media: i2c: ov9282: Make V4L2_CID_HBLANK r/w Dave Stevenson
2022-10-06 9:41 ` Jacopo Mondi
2022-10-06 11:33 ` Dave Stevenson
2022-10-06 11:53 ` Jacopo Mondi
2022-10-05 15:28 ` [PATCH 13/16] media: i2c: ov9282: Add selection API calls for cropping info Dave Stevenson
2022-10-06 9:43 ` Jacopo Mondi
2022-10-06 11:39 ` Dave Stevenson
2022-10-06 11:54 ` Jacopo Mondi
2022-10-05 15:28 ` [PATCH 14/16] media: i2c: ov9282: Add support for 1280x800 and 640x400 modes Dave Stevenson
2022-10-06 9:48 ` Jacopo Mondi
2022-10-06 11:46 ` Dave Stevenson
2022-10-05 15:28 ` [PATCH 15/16] media: i2c: ov9282: Add support for 8bit readout Dave Stevenson
2022-10-06 9:57 ` Jacopo Mondi
2022-10-06 12:20 ` Dave Stevenson
2022-10-05 15:28 ` [PATCH 16/16] media: i2c: ov9282: Support event handlers Dave Stevenson
2022-10-06 9:59 ` Jacopo Mondi
2022-10-07 10:22 ` Dave Stevenson
2022-10-07 12:57 ` Jacopo Mondi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPY8ntBmB++um4GTwqg2CUXdSxuRxdJibi0nX3E0ZRpZ2imCZQ@mail.gmail.com \
--to=dave.stevenson@raspberrypi.com \
--cc=daniele.alessandrelli@intel.com \
--cc=jacopo@jmondi.org \
--cc=linux-media@vger.kernel.org \
--cc=paul.j.murphy@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).