Hi Kaaira, On 14/06/2020 21:02, Kaaira Gupta wrote: > Add a control in VIMC to show the correct order of the colors for a > given test pattern. > The control can be accessed by using show_colors_order in v4l2-ctl > > Signed-off-by: Kaaira Gupta <[email protected]> > --- > drivers/media/test-drivers/vimc/Kconfig | 2 ++ > drivers/media/test-drivers/vimc/vimc-common.h | 1 + > drivers/media/test-drivers/vimc/vimc-sensor.c | 34 +++++++++++++++++++ > 3 files changed, 37 insertions(+) > > diff --git a/drivers/media/test-drivers/vimc/Kconfig > b/drivers/media/test-drivers/vimc/Kconfig > index 4068a67585f9..da4b2ad6e40c 100644 > --- a/drivers/media/test-drivers/vimc/Kconfig > +++ b/drivers/media/test-drivers/vimc/Kconfig > @@ -2,6 +2,8 @@ > config VIDEO_VIMC > tristate "Virtual Media Controller Driver (VIMC)" > depends on VIDEO_DEV && VIDEO_V4L2 > + select FONT_SUPPORT > + select FONT_8x16 > select MEDIA_CONTROLLER > select VIDEO_V4L2_SUBDEV_API > select VIDEOBUF2_VMALLOC > diff --git a/drivers/media/test-drivers/vimc/vimc-common.h > b/drivers/media/test-drivers/vimc/vimc-common.h > index ae163dec2459..52376ba6146b 100644 > --- a/drivers/media/test-drivers/vimc/vimc-common.h > +++ b/drivers/media/test-drivers/vimc/vimc-common.h > @@ -20,6 +20,7 @@ > #define VIMC_CID_VIMC_CLASS (0x00f00000 | 1) > #define VIMC_CID_TEST_PATTERN (VIMC_CID_VIMC_BASE + 0) > #define VIMC_CID_MEAN_WIN_SIZE (VIMC_CID_VIMC_BASE + 1) > +#define VIMC_TEST_PATTERN_ORDER (VIMC_CID_VIMC_BASE + 2)
This should have the prefix CID like the others. I believe that stands for "Control ID" As below, I think it might warrant being a more generic name for the OSD overlay, as it could display more than just the colour sequence. > > #define VIMC_FRAME_MAX_WIDTH 4096 > #define VIMC_FRAME_MAX_HEIGHT 2160 > diff --git a/drivers/media/test-drivers/vimc/vimc-sensor.c > b/drivers/media/test-drivers/vimc/vimc-sensor.c > index a2f09ac9a360..da625f6accce 100644 > --- a/drivers/media/test-drivers/vimc/vimc-sensor.c > +++ b/drivers/media/test-drivers/vimc/vimc-sensor.c > @@ -5,6 +5,7 @@ > * Copyright (C) 2015-2017 Helen Koike <[email protected]> > */ > > +#include <linux/font.h> > #include <linux/v4l2-mediabus.h> > #include <linux/vmalloc.h> > #include <media/v4l2-ctrls.h> > @@ -19,6 +20,7 @@ struct vimc_sen_device { > struct v4l2_subdev sd; > struct tpg_data tpg; > u8 *frame; > + bool show_colors_order; I would say it's the 'sequence' 'show_color_sequence' but it's still a bit long ... but maybe that doesn't really matter. > /* The active format */ > struct v4l2_mbus_framefmt mbus_format; > struct v4l2_ctrl_handler hdl; > @@ -185,10 +187,18 @@ static const struct v4l2_subdev_pad_ops > vimc_sen_pad_ops = { > static void *vimc_sen_process_frame(struct vimc_ent_device *ved, > const void *sink_frame) > { > + u8 *basep[TPG_MAX_PLANES][2]; > + char *str; > struct vimc_sen_device *vsen = container_of(ved, struct vimc_sen_device, > ved); > > tpg_fill_plane_buffer(&vsen->tpg, 0, 0, vsen->frame); > + if (vsen->show_colors_order) { > + tpg_calc_text_basep(&vsen->tpg, basep, 0, vsen->frame); > + str = tpg_g_color_order(&vsen->tpg); > + tpg_gen_text(&vsen->tpg, basep, 1, 1, str); > + } > + > return vsen->frame; > } > > @@ -200,6 +210,14 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int > enable) > if (enable) { > const struct vimc_pix_map *vpix; > unsigned int frame_size; > + const struct font_desc *font = find_font("VGA8x16"); > + > + if (font == NULL) { > + pr_err("vimc: could not find font\n"); > + return -ENODEV; I wonder if this should instead just disable text rendering instead? It might be reasonable to compile the kernel without the font-library ? > + } > + > + tpg_set_font(font->data); > > /* Calculate the frame size */ > vpix = vimc_pix_map_by_code(vsen->mbus_format.code); > @@ -269,6 +287,9 @@ static int vimc_sen_s_ctrl(struct v4l2_ctrl *ctrl) > case V4L2_CID_SATURATION: > tpg_s_saturation(&vsen->tpg, ctrl->val); > break; > + case VIMC_TEST_PATTERN_ORDER: > + vsen->show_colors_order = ctrl->val; > + break; > default: > return -EINVAL; > } > @@ -307,6 +328,17 @@ static const struct v4l2_ctrl_config > vimc_sen_ctrl_test_pattern = { > .qmenu = tpg_pattern_strings, > }; > > +static const struct v4l2_ctrl_config vimc_sen_ctrl_order = { > + .ops = &vimc_sen_ctrl_ops, > + .id = VIMC_TEST_PATTERN_ORDER, Now that you've brought in support for rendering text on the frame, I wonder if more information should be displayed just like VIVID does. In that case, it would probably be better to have a more generic control that enables all of the text OSD with a more generic name. > + .name = "Show Colors Order", > + .type = V4L2_CTRL_TYPE_BOOLEAN, > + .min = 0, > + .max = 1, > + .step = 1, > + .def = 1, > +}; > + > static struct vimc_ent_device *vimc_sen_add(struct vimc_device *vimc, > const char *vcfg_name) > { > @@ -323,6 +355,7 @@ static struct vimc_ent_device *vimc_sen_add(struct > vimc_device *vimc, > > v4l2_ctrl_new_custom(&vsen->hdl, &vimc_sen_ctrl_class, NULL); > v4l2_ctrl_new_custom(&vsen->hdl, &vimc_sen_ctrl_test_pattern, NULL); > + v4l2_ctrl_new_custom(&vsen->hdl, &vimc_sen_ctrl_order, NULL); > v4l2_ctrl_new_std(&vsen->hdl, &vimc_sen_ctrl_ops, > V4L2_CID_VFLIP, 0, 1, 1, 0); > v4l2_ctrl_new_std(&vsen->hdl, &vimc_sen_ctrl_ops, > @@ -362,6 +395,7 @@ static struct vimc_ent_device *vimc_sen_add(struct > vimc_device *vimc, > > /* Initialize the frame format */ > vsen->mbus_format = fmt_default; > + vsen->show_colors_order = vimc_sen_ctrl_order.def; > > return &vsen->ved; > > -- Regards -- Kieran
