From: Tarang Raval <tarang.raval@siliconsignals.io>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: "mehdi.djait@linux.intel.com" <mehdi.djait@linux.intel.com>,
Himanshu Bhavani <himanshu.bhavani@siliconsignals.io>,
Elgin Perumbilly <elgin.perumbilly@siliconsignals.io>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 11/13] media: i2c: os05b10: Update active format before adjusting framing controls
Date: Mon, 9 Mar 2026 08:19:11 +0000 [thread overview]
Message-ID: <PN3P287MB18292213A1DE4E8BB57600F98B79A@PN3P287MB1829.INDP287.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <PN3P287MB1829F11B3BC3D07BE13C4C228B7BA@PN3P287MB1829.INDP287.PROD.OUTLOOK.COM>
Hi Sakari,
> > On Fri, Mar 06, 2026 at 06:03:01PM +0530, Tarang Raval wrote:
> > > os05b10_set_pad_format() calls os05b10_set_framing_limits() before updating
> > > the ACTIVE format. As a result, the VBLANK control handler uses the old
> > > height when recalculating exposure limits, causing -ERANGE when switching
> > > to a larger resolution.
> > >
> > > Update the ACTIVE format before adjusting framing controls so control
> > > callbacks use the correct dimensions.
> > >
> > > Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
> > > ---
> > > drivers/media/i2c/os05b10.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/os05b10.c b/drivers/media/i2c/os05b10.c
> > > index 4601e33b7e8f..476dbcb49351 100644
> > > --- a/drivers/media/i2c/os05b10.c
> > > +++ b/drivers/media/i2c/os05b10.c
> > > @@ -902,14 +902,14 @@ static int os05b10_set_pad_format(struct v4l2_subdev *sd,
> > >
> > > format = v4l2_subdev_state_get_format(sd_state, 0);
> > >
> > > + *format = fmt->format;
> > > +
> > > if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> > > ret = os05b10_set_framing_limits(os05b10, mode);
> >
> > Does it take a driver bug for this to happen? Presumably? I guess the
> > result would be somewhat inconsistent state in any case.
>
> In the current driver only a single mode is present, so the issue is not
> observed because no resolution change occurs.
>
> This issue became visible while adding a new mode. During a mode switch
> (from a smaller resolution to a larger one) the limits are calculated
> using the previous format, which results in -ERANGE and the new mode is
> not applied.
>
> Updating *format = fmt->format before adjusting the framing controls
> ensures the control handlers see the correct dimensions.
During testing I messed up.
Sorry for the confusion. With patch 10 the -ERANGE error is already resolved.
During a mode change, os05b10_set_framing_limits() receives the selected
mode as an argument, so it already uses the new resolution when calculating
the limits.
Also, the control handlers are only triggered when streaming starts, so
there is no practical difference whether *format = fmt->format is updated
before or after adjusting the framing controls.
Given this, the change proposed in this patch is not necessary.
I will drop this patch.
Best Regards,
Tarang
next prev parent reply other threads:[~2026-03-09 8:19 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-06 12:32 [PATCH 00/13] media: i2c: os05b10: Refactor driver and Add new features Tarang Raval
2026-03-06 12:32 ` [PATCH 01/13] media: i2c: os05b10: drop unused group-hold programming Tarang Raval
2026-03-06 12:32 ` [PATCH 02/13] media: i2c: os05b10: add register definitions and use them in init table Tarang Raval
2026-03-06 12:32 ` [PATCH 03/13] media: i2c: os05b10: split common and mode-specific init registers Tarang Raval
2026-03-06 12:32 ` [PATCH 04/13] media: i2c: os05b10: add V4L2 digital gain control Tarang Raval
2026-03-06 12:32 ` [PATCH 05/13] media: i2c: os05b10: Add H/V flip support Tarang Raval
2026-03-06 12:32 ` [PATCH 06/13] media: i2c: os05b10: Add test pattern options Tarang Raval
2026-03-06 12:32 ` [PATCH 07/13] media: i2c: os05b10: add 12-bit RAW mode support Tarang Raval
2026-03-06 12:32 ` [PATCH 08/13] media: i2c: os05b10: update pixel rate on 10/12-bit mode switch Tarang Raval
2026-03-06 12:32 ` [PATCH 09/13] media: i2c: os05b10: Add 1080p and 2x2 binning 720p modes Tarang Raval
2026-03-06 12:33 ` [PATCH 10/13] media: i2c: os05b10: keep vblank/exposure in sync on mode switch Tarang Raval
2026-03-06 13:35 ` Sakari Ailus
2026-03-07 5:28 ` Tarang Raval
2026-03-06 12:33 ` [PATCH 11/13] media: i2c: os05b10: Update active format before adjusting framing controls Tarang Raval
2026-03-06 13:36 ` Sakari Ailus
2026-03-07 5:22 ` Tarang Raval
2026-03-09 8:19 ` Tarang Raval [this message]
2026-03-24 16:31 ` Tarang Raval
2026-03-25 8:56 ` sakari.ailus
2026-03-25 9:05 ` Tarang Raval
2026-03-06 12:33 ` [PATCH 12/13] media: i2c: os05b10: Rename vmax variable in VBLANK control Tarang Raval
2026-03-06 12:33 ` [PATCH 13/13] media: i2c: os05b10: add 2-lane support Tarang Raval
2026-03-07 1:56 ` kernel test robot
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=PN3P287MB18292213A1DE4E8BB57600F98B79A@PN3P287MB1829.INDP287.PROD.OUTLOOK.COM \
--to=tarang.raval@siliconsignals.io \
--cc=elgin.perumbilly@siliconsignals.io \
--cc=himanshu.bhavani@siliconsignals.io \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=mehdi.djait@linux.intel.com \
--cc=sakari.ailus@linux.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