Hi Dave
On Mon, Feb 20, 2023 at 06:18:14PM +0000, Dave Stevenson wrote:
> Hi Jacopo
>
> On Mon, 20 Feb 2023 at 12:40, Jacopo Mondi
> <jacopo.mondi@xxxxxxxxxxxxxxxx> wrote:
> >
> > From: Valentine Barshak <valentine.barshak@xxxxxxxxxxxxxxxxxx>
> >
> > This adds V4L2_CID_TEST_PATTERN control support.
> >
> > Signed-off-by: Valentine Barshak <valentine.barshak@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
> > ---
> > drivers/media/i2c/ov5647.c | 28 +++++++++++++++++++++++++++-
> > 1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> > index 847a7bbb69c5..0b88ac6dee41 100644
> > --- a/drivers/media/i2c/ov5647.c
> > +++ b/drivers/media/i2c/ov5647.c
> > @@ -58,6 +58,7 @@
> > #define OV5647_REG_MIPI_CTRL00 0x4800
> > #define OV5647_REG_MIPI_CTRL14 0x4814
> > #define OV5647_REG_AWB 0x5001
> > +#define OV5647_REG_ISPCTRL3D 0x503d
> >
> > #define REG_TERM 0xfffe
> > #define VAL_TERM 0xfe
> > @@ -116,6 +117,22 @@ static inline struct ov5647 *to_sensor(struct v4l2_subdev *sd)
> > return container_of(sd, struct ov5647, sd);
> > }
> >
> > +static const char * const ov5647_test_pattern_menu[] = {
> > + "Disabled",
> > + "Color Bars",
> > + "Color Squares",
> > + "Random Data",
> > + "Input Data"
>
> "Input Data" appears to give me just a black image. Have I missed
> something? What's it meant to be the input to?
I noticed as well, but not knowing what "input data" meant I just
"meh" and moved on
Should it be dropped in your opinion ?
> Is it worth adding 0x92 for a black and white checkboard as well?
I could
>
> Whichever way:
>
> Reviewed-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
>
> Just as a note, the test patterns appear to be valid only if 0x3820
> bit 1 = 0 and 0x3821 bit 1 = 1 (V & H flips respectively).
Unrelated: why do I see the 2592x1944 mode with {0x3821, 0x06} ?
I only tested whatever qcam gave me, I should better re-check
> The sensor appears to be assuming one particular colour pattern (BGGR)
> when producing a test pattern, so flips altering the format give some
> very weird effects. I do have patches that add the V4L2 flip controls,
Ah, that surprises me, I recall we discussed in the past the fact that
flip shouldn't alter test patterns...
> so those expose some interesting effects :-/
>
> Dave
>
> > +};
> > +
> > +static u8 ov5647_test_pattern_val[] = {
> > + 0x00, /* Disabled */
> > + 0x80, /* Color Bars */
> > + 0x82, /* Color Squares */
> > + 0x81, /* Random Data */
> > + 0x83, /* Input Data */
> > +};
> > +
> > static const struct regval_list sensor_oe_disable_regs[] = {
> > {0x3000, 0x00},
> > {0x3001, 0x00},
> > @@ -1242,6 +1259,10 @@ static int ov5647_s_ctrl(struct v4l2_ctrl *ctrl)
> > ret = ov5647_write16(sd, OV5647_REG_VTS_HI,
> > sensor->mode->format.height + ctrl->val);
> > break;
> > + case V4L2_CID_TEST_PATTERN:
> > + ret = ov5647_write(sd, OV5647_REG_ISPCTRL3D,
> > + ov5647_test_pattern_val[ctrl->val]);
> > + break;
> >
> > /* Read-only, but we adjust it based on mode. */
> > case V4L2_CID_PIXEL_RATE:
> > @@ -1270,7 +1291,7 @@ static int ov5647_init_controls(struct ov5647 *sensor)
> > struct i2c_client *client = v4l2_get_subdevdata(&sensor->sd);
> > int hblank, exposure_max, exposure_def;
> >
> > - v4l2_ctrl_handler_init(&sensor->ctrls, 8);
> > + v4l2_ctrl_handler_init(&sensor->ctrls, 9);
> >
> > v4l2_ctrl_new_std(&sensor->ctrls, &ov5647_ctrl_ops,
> > V4L2_CID_AUTOGAIN, 0, 1, 1, 0);
> > @@ -1314,6 +1335,11 @@ static int ov5647_init_controls(struct ov5647 *sensor)
> > sensor->mode->vts -
> > sensor->mode->format.height);
> >
> > + v4l2_ctrl_new_std_menu_items(&sensor->ctrls, &ov5647_ctrl_ops,
> > + V4L2_CID_TEST_PATTERN,
> > + ARRAY_SIZE(ov5647_test_pattern_menu) - 1,
> > + 0, 0, ov5647_test_pattern_menu);
> > +
> > if (sensor->ctrls.error)
> > goto handler_free;
> >
> > --
> > 2.39.0
> >
