Hi Fabio, Thanks for the patch. On Tue, Apr 26, 2022 at 04:14:41PM -0300, Fabio Estevam wrote: > ADV7180 has a built-in mechanism to generate some video patterns, > which is very useful for debug/bring-up activities. > > Add support for it. > > The test_pattern parameter can be one of the following values: > > 0: "Single color" > 1: "Color bars" > 2: "Luma ramp" > 5: "Boundary box" > 6 "Disable" > > Tested on a imx6q board with an ADV7280. > > Signed-off-by: Fabio Estevam <festevam@xxxxxxxxx> > --- > drivers/media/i2c/adv7180.c | 42 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > > diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c > index 4f5db195e66d..09e01ef99694 100644 > --- a/drivers/media/i2c/adv7180.c > +++ b/drivers/media/i2c/adv7180.c > @@ -66,6 +66,9 @@ > #define ADV7180_HUE_DEF 0 > #define ADV7180_HUE_MAX 128 > > +#define ADV7180_REG_DEF_VALUE_Y 0x000c > +#define ADV7180_DEF_VAL_EN 0x1 > +#define ADV7180_DEF_VAL_AUTO_EN 0x2 > #define ADV7180_REG_CTRL 0x000e > #define ADV7180_CTRL_IRQ_SPACE 0x20 > > @@ -549,6 +552,37 @@ static int adv7180_s_power(struct v4l2_subdev *sd, int on) > return ret; > } > > +static const char * const test_pattern_menu[] = { > + "Single color", > + "Color bars", > + "Luma ramp", > + "reserved", > + "reserved", Why are there reserved options in the menu? These should probably be simply removed from the menu, or at masked out if you think you would later on add support for them. Please tell which one you intend to do. > + "Boundary box", > + "Disable", > +}; > + > +static int adv7180_test_pattern(struct adv7180_state *state, int value) > +{ > + unsigned int reg; > + > + v4l_info(state->client, "Testing pattern: %s\n", test_pattern_menu[value]); This could be on debug level but I would simply remove it. > + adv7180_write(state, ADV7180_REG_ANALOG_CLAMP_CTL, value); > + > + if (value == ARRAY_SIZE(test_pattern_menu) - 1) { > + reg = adv7180_read(state, ADV7180_REG_DEF_VALUE_Y); > + reg &= ~ADV7180_DEF_VAL_EN; > + adv7180_write(state, ADV7180_REG_DEF_VALUE_Y, reg); > + return 0; > + } > + > + reg = adv7180_read(state, ADV7180_REG_DEF_VALUE_Y); > + reg |= ADV7180_DEF_VAL_EN | ADV7180_DEF_VAL_AUTO_EN; > + adv7180_write(state, ADV7180_REG_DEF_VALUE_Y, reg); > + > + return 0; > +} > + > static int adv7180_s_ctrl(struct v4l2_ctrl *ctrl) > { > struct v4l2_subdev *sd = to_adv7180_sd(ctrl); > @@ -592,6 +626,9 @@ static int adv7180_s_ctrl(struct v4l2_ctrl *ctrl) > adv7180_write(state, ADV7180_REG_FLCONTROL, 0x00); > } > break; > + case V4L2_CID_TEST_PATTERN: > + ret = adv7180_test_pattern(state, val); > + break; > default: > ret = -EINVAL; > } > @@ -632,6 +669,11 @@ static int adv7180_init_controls(struct adv7180_state *state) > ADV7180_HUE_MAX, 1, ADV7180_HUE_DEF); > v4l2_ctrl_new_custom(&state->ctrl_hdl, &adv7180_ctrl_fast_switch, NULL); > > + v4l2_ctrl_new_std_menu_items(&state->ctrl_hdl, &adv7180_ctrl_ops, > + V4L2_CID_TEST_PATTERN, > + ARRAY_SIZE(test_pattern_menu) - 1, > + 0, 0, test_pattern_menu); > + > state->sd.ctrl_handler = &state->ctrl_hdl; > if (state->ctrl_hdl.error) { > int err = state->ctrl_hdl.error; -- Kind regards, Sakari Ailus