* [PATCH] media: i2c: ov9282: Add test pattern control @ 2026-03-16 9:05 Xiaolei Wang 2026-03-16 16:38 ` Dave Stevenson 0 siblings, 1 reply; 3+ messages in thread From: Xiaolei Wang @ 2026-03-16 9:05 UTC (permalink / raw) To: sakari.ailus, laurent.pinchart, tarang.raval, jacopo, mchehab, prabhakar.mahadev-lad.rj, hverkuil+cisco, johannes.goede, hverkuil-cisco, jai.luthra, dave.stevenson, Xiaolei.Wang Cc: linux-media, linux-kernel This adds V4L2_CID_TEST_PATTERN control support. Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com> --- drivers/media/i2c/ov9282.c | 47 +++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c index 2167fb73ea41..f64b2084b8e7 100644 --- a/drivers/media/i2c/ov9282.c +++ b/drivers/media/i2c/ov9282.c @@ -104,6 +104,12 @@ #define OV9282_REG_STROBE_FRAME_SPAN CCI_REG32(0x3925) #define OV9282_STROBE_FRAME_SPAN_DEFAULT 0x0000001a +/* Test Pattern registers */ +#define OV9282_REG_TEST_PATTERN_BAR CCI_REG8(0x5e00) +#define OV9282_TEST_PATTERN_BAR_EN BIT(7) +#define OV9282_REG_TEST_PATTERN_SOLID CCI_REG8(0x4320) +#define OV9282_TEST_PATTERN_SOLID_EN BIT(1) + /* Input clock rate */ #define OV9282_INCLK_RATE 24000000 @@ -462,6 +468,18 @@ static const struct ov9282_mode supported_modes[] = { }, }; +enum { + OV9282_TEST_PATTERN_DISABLED, + OV9282_TEST_PATTERN_COLOR_BAR, + OV9282_TEST_PATTERN_SOLID_COLOR, +}; + +static const char * const ov9282_test_pattern_menu[] = { + "Disabled", + "Color Bar", + "Solid Color", +}; + /** * to_ov9282() - ov9282 V4L2 sub-device to ov9282 device. * @subdev: pointer to ov9282 V4L2 sub-device @@ -586,6 +604,23 @@ static u32 ov9282_flash_duration_to_us(struct ov9282 *ov9282, u32 value) return DIV_ROUND_UP(value * frame_width, OV9282_STROBE_SPAN_FACTOR); } +static int ov9282_set_ctrl_test_pattern(struct ov9282 *ov9282, int pattern) +{ + int ret; + + ret = cci_update_bits(ov9282->regmap, OV9282_REG_TEST_PATTERN_BAR, + OV9282_TEST_PATTERN_BAR_EN, + pattern == OV9282_TEST_PATTERN_COLOR_BAR ? + OV9282_TEST_PATTERN_BAR_EN : 0, NULL); + if (ret) + return ret; + + return cci_update_bits(ov9282->regmap, OV9282_REG_TEST_PATTERN_SOLID, + OV9282_TEST_PATTERN_SOLID_EN, + pattern == OV9282_TEST_PATTERN_SOLID_COLOR ? + OV9282_TEST_PATTERN_SOLID_EN : 0, NULL); +} + /** * ov9282_set_ctrl() - Set subdevice control * @ctrl: pointer to v4l2_ctrl structure @@ -662,6 +697,11 @@ static int ov9282_set_ctrl(struct v4l2_ctrl *ctrl) case V4L2_CID_FLASH_DURATION: ret = cci_write(ov9282->regmap, OV9282_REG_STROBE_FRAME_SPAN, ctrl->val, NULL); break; + + case V4L2_CID_TEST_PATTERN: + ret = ov9282_set_ctrl_test_pattern(ov9282, ctrl->val); + break; + default: dev_err(ov9282->dev, "Invalid control %d", ctrl->id); ret = -EINVAL; @@ -1242,7 +1282,7 @@ static int ov9282_init_controls(struct ov9282 *ov9282) u32 lpfr; int ret; - ret = v4l2_ctrl_handler_init(ctrl_hdlr, 12); + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 13); if (ret) return ret; @@ -1314,6 +1354,11 @@ static int ov9282_init_controls(struct ov9282 *ov9282) V4L2_CID_FLASH_DURATION, 0, exposure_us, 1, OV9282_STROBE_FRAME_SPAN_DEFAULT); + v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &ov9282_ctrl_ops, + V4L2_CID_TEST_PATTERN, + ARRAY_SIZE(ov9282_test_pattern_menu) - 1, + 0, 0, ov9282_test_pattern_menu); + ret = v4l2_fwnode_device_parse(ov9282->dev, &props); if (!ret) { /* Failure sets ctrl_hdlr->error, which we check afterwards anyway */ -- 2.43.0 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] media: i2c: ov9282: Add test pattern control 2026-03-16 9:05 [PATCH] media: i2c: ov9282: Add test pattern control Xiaolei Wang @ 2026-03-16 16:38 ` Dave Stevenson 2026-03-17 3:49 ` Xiaolei Wang 0 siblings, 1 reply; 3+ messages in thread From: Dave Stevenson @ 2026-03-16 16:38 UTC (permalink / raw) To: Xiaolei Wang Cc: sakari.ailus, laurent.pinchart, tarang.raval, jacopo, mchehab, prabhakar.mahadev-lad.rj, hverkuil+cisco, johannes.goede, hverkuil-cisco, jai.luthra, linux-media, linux-kernel Hi Xiaolei On Mon, 16 Mar 2026 at 09:06, Xiaolei Wang <xiaolei.wang@windriver.com> wrote: > > This adds V4L2_CID_TEST_PATTERN control support. > > Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com> > --- > drivers/media/i2c/ov9282.c | 47 +++++++++++++++++++++++++++++++++++++- > 1 file changed, 46 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c > index 2167fb73ea41..f64b2084b8e7 100644 > --- a/drivers/media/i2c/ov9282.c > +++ b/drivers/media/i2c/ov9282.c > @@ -104,6 +104,12 @@ > #define OV9282_REG_STROBE_FRAME_SPAN CCI_REG32(0x3925) > #define OV9282_STROBE_FRAME_SPAN_DEFAULT 0x0000001a > > +/* Test Pattern registers */ > +#define OV9282_REG_TEST_PATTERN_BAR CCI_REG8(0x5e00) > +#define OV9282_TEST_PATTERN_BAR_EN BIT(7) > +#define OV9282_REG_TEST_PATTERN_SOLID CCI_REG8(0x4320) > +#define OV9282_TEST_PATTERN_SOLID_EN BIT(1) > + > /* Input clock rate */ > #define OV9282_INCLK_RATE 24000000 > > @@ -462,6 +468,18 @@ static const struct ov9282_mode supported_modes[] = { > }, > }; > > +enum { > + OV9282_TEST_PATTERN_DISABLED, > + OV9282_TEST_PATTERN_COLOR_BAR, This feels like an odd name to choose seeing as it is only a monochrome sensor so there is no color. > + OV9282_TEST_PATTERN_SOLID_COLOR, > +}; > + > +static const char * const ov9282_test_pattern_menu[] = { > + "Disabled", > + "Color Bar", > + "Solid Color", > +}; > + > /** > * to_ov9282() - ov9282 V4L2 sub-device to ov9282 device. > * @subdev: pointer to ov9282 V4L2 sub-device > @@ -586,6 +604,23 @@ static u32 ov9282_flash_duration_to_us(struct ov9282 *ov9282, u32 value) > return DIV_ROUND_UP(value * frame_width, OV9282_STROBE_SPAN_FACTOR); > } > > +static int ov9282_set_ctrl_test_pattern(struct ov9282 *ov9282, int pattern) > +{ > + int ret; > + > + ret = cci_update_bits(ov9282->regmap, OV9282_REG_TEST_PATTERN_BAR, > + OV9282_TEST_PATTERN_BAR_EN, > + pattern == OV9282_TEST_PATTERN_COLOR_BAR ? > + OV9282_TEST_PATTERN_BAR_EN : 0, NULL); This register is never written from anywhere else, and all the bits are related to the test pattern, so is there any reason not to set them all? > + if (ret) > + return ret; > + > + return cci_update_bits(ov9282->regmap, OV9282_REG_TEST_PATTERN_SOLID, > + OV9282_TEST_PATTERN_SOLID_EN, > + pattern == OV9282_TEST_PATTERN_SOLID_COLOR ? > + OV9282_TEST_PATTERN_SOLID_EN : 0, NULL); Again no need to use cci_update_bits as all the bits relate to the test pattern. If you're adding black, then you could add white as well. Registers 0x4322-0x4329 set the 4 pixel values that would equate to V4L2_CID_TEST_PATTERN_RED, etc, so writing them all as CCI_REG16(0x4322, 0x3ff) and repeating for 0x4324, 0x4326, and 0x4328 would give you white. Then again the only mechanism for implementing that is to use V4L2_CID_TEST_PATTERN_RED etc, which is rather quirky on a monochrome sensor. I am thinking that white is more useful than black if you only implement one. Dave > +} > + > /** > * ov9282_set_ctrl() - Set subdevice control > * @ctrl: pointer to v4l2_ctrl structure > @@ -662,6 +697,11 @@ static int ov9282_set_ctrl(struct v4l2_ctrl *ctrl) > case V4L2_CID_FLASH_DURATION: > ret = cci_write(ov9282->regmap, OV9282_REG_STROBE_FRAME_SPAN, ctrl->val, NULL); > break; > + > + case V4L2_CID_TEST_PATTERN: > + ret = ov9282_set_ctrl_test_pattern(ov9282, ctrl->val); > + break; > + > default: > dev_err(ov9282->dev, "Invalid control %d", ctrl->id); > ret = -EINVAL; > @@ -1242,7 +1282,7 @@ static int ov9282_init_controls(struct ov9282 *ov9282) > u32 lpfr; > int ret; > > - ret = v4l2_ctrl_handler_init(ctrl_hdlr, 12); > + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 13); > if (ret) > return ret; > > @@ -1314,6 +1354,11 @@ static int ov9282_init_controls(struct ov9282 *ov9282) > V4L2_CID_FLASH_DURATION, 0, exposure_us, 1, > OV9282_STROBE_FRAME_SPAN_DEFAULT); > > + v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &ov9282_ctrl_ops, > + V4L2_CID_TEST_PATTERN, > + ARRAY_SIZE(ov9282_test_pattern_menu) - 1, > + 0, 0, ov9282_test_pattern_menu); > + > ret = v4l2_fwnode_device_parse(ov9282->dev, &props); > if (!ret) { > /* Failure sets ctrl_hdlr->error, which we check afterwards anyway */ > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] media: i2c: ov9282: Add test pattern control 2026-03-16 16:38 ` Dave Stevenson @ 2026-03-17 3:49 ` Xiaolei Wang 0 siblings, 0 replies; 3+ messages in thread From: Xiaolei Wang @ 2026-03-17 3:49 UTC (permalink / raw) To: Dave Stevenson Cc: sakari.ailus, laurent.pinchart, tarang.raval, jacopo, mchehab, prabhakar.mahadev-lad.rj, hverkuil+cisco, johannes.goede, hverkuil-cisco, jai.luthra, linux-media, linux-kernel Hi Dave, Thanks for the review. On 3/17/26 00:38, Dave Stevenson wrote: > CAUTION: This email comes from a non Wind River email account! > Do not click links or open attachments unless you recognize the sender and know the content is safe. > > Hi Xiaolei > > On Mon, 16 Mar 2026 at 09:06, Xiaolei Wang <xiaolei.wang@windriver.com> wrote: >> This adds V4L2_CID_TEST_PATTERN control support. >> >> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com> >> --- >> drivers/media/i2c/ov9282.c | 47 +++++++++++++++++++++++++++++++++++++- >> 1 file changed, 46 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c >> index 2167fb73ea41..f64b2084b8e7 100644 >> --- a/drivers/media/i2c/ov9282.c >> +++ b/drivers/media/i2c/ov9282.c >> @@ -104,6 +104,12 @@ >> #define OV9282_REG_STROBE_FRAME_SPAN CCI_REG32(0x3925) >> #define OV9282_STROBE_FRAME_SPAN_DEFAULT 0x0000001a >> >> +/* Test Pattern registers */ >> +#define OV9282_REG_TEST_PATTERN_BAR CCI_REG8(0x5e00) >> +#define OV9282_TEST_PATTERN_BAR_EN BIT(7) >> +#define OV9282_REG_TEST_PATTERN_SOLID CCI_REG8(0x4320) >> +#define OV9282_TEST_PATTERN_SOLID_EN BIT(1) >> + >> /* Input clock rate */ >> #define OV9282_INCLK_RATE 24000000 >> >> @@ -462,6 +468,18 @@ static const struct ov9282_mode supported_modes[] = { >> }, >> }; >> >> +enum { >> + OV9282_TEST_PATTERN_DISABLED, >> + OV9282_TEST_PATTERN_COLOR_BAR, > This feels like an odd name to choose seeing as it is only a > monochrome sensor so there is no color. My mistake, I will correct it to: enum { OV9282_TEST_PATTERN_DISABLED, OV9282_TEST_PATTERN_BAR, OV9282_TEST_PATTERN_SOLID_WHITE, }; > >> + OV9282_TEST_PATTERN_SOLID_COLOR, >> +}; >> + >> +static const char * const ov9282_test_pattern_menu[] = { >> + "Disabled", >> + "Color Bar", >> + "Solid Color", >> +}; >> + >> /** >> * to_ov9282() - ov9282 V4L2 sub-device to ov9282 device. >> * @subdev: pointer to ov9282 V4L2 sub-device >> @@ -586,6 +604,23 @@ static u32 ov9282_flash_duration_to_us(struct ov9282 *ov9282, u32 value) >> return DIV_ROUND_UP(value * frame_width, OV9282_STROBE_SPAN_FACTOR); >> } >> >> +static int ov9282_set_ctrl_test_pattern(struct ov9282 *ov9282, int pattern) >> +{ >> + int ret; >> + >> + ret = cci_update_bits(ov9282->regmap, OV9282_REG_TEST_PATTERN_BAR, >> + OV9282_TEST_PATTERN_BAR_EN, >> + pattern == OV9282_TEST_PATTERN_COLOR_BAR ? >> + OV9282_TEST_PATTERN_BAR_EN : 0, NULL); > This register is never written from anywhere else, and all the bits > are related to the test pattern, so is there any reason not to set > them all? Agreed > >> + if (ret) >> + return ret; >> + >> + return cci_update_bits(ov9282->regmap, OV9282_REG_TEST_PATTERN_SOLID, >> + OV9282_TEST_PATTERN_SOLID_EN, >> + pattern == OV9282_TEST_PATTERN_SOLID_COLOR ? >> + OV9282_TEST_PATTERN_SOLID_EN : 0, NULL); > Again no need to use cci_update_bits as all the bits relate to the test pattern. Agreed, will switch to cci_write for both registers. > > If you're adding black, then you could add white as well. > Registers 0x4322-0x4329 set the 4 pixel values that would equate to > V4L2_CID_TEST_PATTERN_RED, etc, so writing them all as > CCI_REG16(0x4322, 0x3ff) and repeating for 0x4324, 0x4326, and 0x4328 > would give you white. > Then again the only mechanism for implementing that is to use > V4L2_CID_TEST_PATTERN_RED etc, which is rather quirky on a monochrome > sensor. I am thinking that white is more useful than black if you only > implement one. That makes sense. I will implement white. If there are no other comments, I will define these three as you suggested: enum { OV9282_TEST_PATTERN_DISABLED, OV9282_TEST_PATTERN_BAR, OV9282_TEST_PATTERN_SOLID_WHITE, }; static const char * const ov9282_test_pattern_menu[] = { "Disabled", "Bar", "Solid White", }; thanks xiaolei > > Dave > >> +} >> + >> /** >> * ov9282_set_ctrl() - Set subdevice control >> * @ctrl: pointer to v4l2_ctrl structure >> @@ -662,6 +697,11 @@ static int ov9282_set_ctrl(struct v4l2_ctrl *ctrl) >> case V4L2_CID_FLASH_DURATION: >> ret = cci_write(ov9282->regmap, OV9282_REG_STROBE_FRAME_SPAN, ctrl->val, NULL); >> break; >> + >> + case V4L2_CID_TEST_PATTERN: >> + ret = ov9282_set_ctrl_test_pattern(ov9282, ctrl->val); >> + break; >> + >> default: >> dev_err(ov9282->dev, "Invalid control %d", ctrl->id); >> ret = -EINVAL; >> @@ -1242,7 +1282,7 @@ static int ov9282_init_controls(struct ov9282 *ov9282) >> u32 lpfr; >> int ret; >> >> - ret = v4l2_ctrl_handler_init(ctrl_hdlr, 12); >> + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 13); >> if (ret) >> return ret; >> >> @@ -1314,6 +1354,11 @@ static int ov9282_init_controls(struct ov9282 *ov9282) >> V4L2_CID_FLASH_DURATION, 0, exposure_us, 1, >> OV9282_STROBE_FRAME_SPAN_DEFAULT); >> >> + v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &ov9282_ctrl_ops, >> + V4L2_CID_TEST_PATTERN, >> + ARRAY_SIZE(ov9282_test_pattern_menu) - 1, >> + 0, 0, ov9282_test_pattern_menu); >> + >> ret = v4l2_fwnode_device_parse(ov9282->dev, &props); >> if (!ret) { >> /* Failure sets ctrl_hdlr->error, which we check afterwards anyway */ >> -- >> 2.43.0 >> ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-03-17 3:50 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-03-16 9:05 [PATCH] media: i2c: ov9282: Add test pattern control Xiaolei Wang 2026-03-16 16:38 ` Dave Stevenson 2026-03-17 3:49 ` Xiaolei Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®