| Message ID | 1547523465-27807-1-git-send-email-yong.zhi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] media: ipu3-imgu: Use MENU type for mode control | expand |
Hi Yong, On Tue, Jan 15, 2019 at 12:38 PM Yong Zhi <yong.zhi@intel.com> wrote: > > This addresses the below TODO item. > - Use V4L2_CTRL_TYPE_MENU for dual-pipe mode control. (Sakari) > > Signed-off-by: Yong Zhi <yong.zhi@intel.com> > --- > drivers/staging/media/ipu3/TODO | 2 -- > drivers/staging/media/ipu3/include/intel-ipu3.h | 6 ------ > drivers/staging/media/ipu3/ipu3-v4l2.c | 18 +++++++++++++----- > 3 files changed, 13 insertions(+), 13 deletions(-) > Thanks for the patch. Please see my comments inline. > diff --git a/drivers/staging/media/ipu3/TODO b/drivers/staging/media/ipu3/TODO > index 905bbb190217..0dc9a2e79978 100644 > --- a/drivers/staging/media/ipu3/TODO > +++ b/drivers/staging/media/ipu3/TODO > @@ -11,8 +11,6 @@ staging directory. > - Prefix imgu for all public APIs, i.e. change ipu3_v4l2_register() to > imgu_v4l2_register(). (Sakari) > > -- Use V4L2_CTRL_TYPE_MENU for dual-pipe mode control. (Sakari) > - > - IPU3 driver documentation (Laurent) > Add diagram in driver rst to describe output capability. > Comments on configuring v4l2 subdevs for CIO2 and ImgU. > diff --git a/drivers/staging/media/ipu3/include/intel-ipu3.h b/drivers/staging/media/ipu3/include/intel-ipu3.h > index ec0b74829351..eb6f52aca992 100644 > --- a/drivers/staging/media/ipu3/include/intel-ipu3.h > +++ b/drivers/staging/media/ipu3/include/intel-ipu3.h > @@ -16,12 +16,6 @@ > #define V4L2_CID_INTEL_IPU3_BASE (V4L2_CID_USER_BASE + 0x10c0) > #define V4L2_CID_INTEL_IPU3_MODE (V4L2_CID_INTEL_IPU3_BASE + 1) > > -/* custom ctrl to set pipe mode */ > -enum ipu3_running_mode { > - IPU3_RUNNING_MODE_VIDEO = 0, > - IPU3_RUNNING_MODE_STILL = 1, > -}; > - > /******************* ipu3_uapi_stats_3a *******************/ > > #define IPU3_UAPI_MAX_STRIPES 2 > diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c b/drivers/staging/media/ipu3/ipu3-v4l2.c > index c7936032beb9..d2a0b62d5688 100644 > --- a/drivers/staging/media/ipu3/ipu3-v4l2.c > +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c > @@ -12,6 +12,9 @@ > > /******************** v4l2_subdev_ops ********************/ > > +#define IPU3_RUNNING_MODE_VIDEO 0 > +#define IPU3_RUNNING_MODE_STILL 1 Just a single space after "#define" please. > + > static int ipu3_subdev_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) > { > struct imgu_v4l2_subdev *imgu_sd = container_of(sd, > @@ -1035,15 +1038,20 @@ static const struct v4l2_ctrl_ops ipu3_subdev_ctrl_ops = { > .s_ctrl = ipu3_sd_s_ctrl, > }; > > +static const char * const ipu3_ctrl_mode_strings[] = { > + "Video mode", > + "Still mode", > + NULL, Do you need this NULL entry? I don't see it in other drivers. > +}; > + > static const struct v4l2_ctrl_config ipu3_subdev_ctrl_mode = { > .ops = &ipu3_subdev_ctrl_ops, > .id = V4L2_CID_INTEL_IPU3_MODE, > .name = "IPU3 Pipe Mode", > - .type = V4L2_CTRL_TYPE_INTEGER, > - .min = IPU3_RUNNING_MODE_VIDEO, > - .max = IPU3_RUNNING_MODE_STILL, > - .step = 1, > - .def = IPU3_RUNNING_MODE_VIDEO, > + .type = V4L2_CTRL_TYPE_MENU, > + .max = ARRAY_SIZE(ipu3_ctrl_mode_strings) - 2, > + .def = 0, IPU3_RUNNING_MODE_VIDEO? > + .qmenu = ipu3_ctrl_mode_strings, > }; > > /******************** Framework registration ********************/ > -- > 2.7.4 > Best regards, Tomasz
Hi Yong, On Mon, Jan 14, 2019 at 09:37:44PM -0600, Yong Zhi wrote: > This addresses the below TODO item. > - Use V4L2_CTRL_TYPE_MENU for dual-pipe mode control. (Sakari) > > Signed-off-by: Yong Zhi <yong.zhi@intel.com> > --- > drivers/staging/media/ipu3/TODO | 2 -- > drivers/staging/media/ipu3/include/intel-ipu3.h | 6 ------ > drivers/staging/media/ipu3/ipu3-v4l2.c | 18 +++++++++++++----- > 3 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/drivers/staging/media/ipu3/TODO b/drivers/staging/media/ipu3/TODO > index 905bbb190217..0dc9a2e79978 100644 > --- a/drivers/staging/media/ipu3/TODO > +++ b/drivers/staging/media/ipu3/TODO > @@ -11,8 +11,6 @@ staging directory. > - Prefix imgu for all public APIs, i.e. change ipu3_v4l2_register() to > imgu_v4l2_register(). (Sakari) > > -- Use V4L2_CTRL_TYPE_MENU for dual-pipe mode control. (Sakari) > - It's good to see TODO entries being addressed. :-) With Tomasz's comments addressed, this is good to go in IMO.
diff --git a/drivers/staging/media/ipu3/TODO b/drivers/staging/media/ipu3/TODO index 905bbb190217..0dc9a2e79978 100644 --- a/drivers/staging/media/ipu3/TODO +++ b/drivers/staging/media/ipu3/TODO @@ -11,8 +11,6 @@ staging directory. - Prefix imgu for all public APIs, i.e. change ipu3_v4l2_register() to imgu_v4l2_register(). (Sakari) -- Use V4L2_CTRL_TYPE_MENU for dual-pipe mode control. (Sakari) - - IPU3 driver documentation (Laurent) Add diagram in driver rst to describe output capability. Comments on configuring v4l2 subdevs for CIO2 and ImgU. diff --git a/drivers/staging/media/ipu3/include/intel-ipu3.h b/drivers/staging/media/ipu3/include/intel-ipu3.h index ec0b74829351..eb6f52aca992 100644 --- a/drivers/staging/media/ipu3/include/intel-ipu3.h +++ b/drivers/staging/media/ipu3/include/intel-ipu3.h @@ -16,12 +16,6 @@ #define V4L2_CID_INTEL_IPU3_BASE (V4L2_CID_USER_BASE + 0x10c0) #define V4L2_CID_INTEL_IPU3_MODE (V4L2_CID_INTEL_IPU3_BASE + 1) -/* custom ctrl to set pipe mode */ -enum ipu3_running_mode { - IPU3_RUNNING_MODE_VIDEO = 0, - IPU3_RUNNING_MODE_STILL = 1, -}; - /******************* ipu3_uapi_stats_3a *******************/ #define IPU3_UAPI_MAX_STRIPES 2 diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c b/drivers/staging/media/ipu3/ipu3-v4l2.c index c7936032beb9..d2a0b62d5688 100644 --- a/drivers/staging/media/ipu3/ipu3-v4l2.c +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c @@ -12,6 +12,9 @@ /******************** v4l2_subdev_ops ********************/ +#define IPU3_RUNNING_MODE_VIDEO 0 +#define IPU3_RUNNING_MODE_STILL 1 + static int ipu3_subdev_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) { struct imgu_v4l2_subdev *imgu_sd = container_of(sd, @@ -1035,15 +1038,20 @@ static const struct v4l2_ctrl_ops ipu3_subdev_ctrl_ops = { .s_ctrl = ipu3_sd_s_ctrl, }; +static const char * const ipu3_ctrl_mode_strings[] = { + "Video mode", + "Still mode", + NULL, +}; + static const struct v4l2_ctrl_config ipu3_subdev_ctrl_mode = { .ops = &ipu3_subdev_ctrl_ops, .id = V4L2_CID_INTEL_IPU3_MODE, .name = "IPU3 Pipe Mode", - .type = V4L2_CTRL_TYPE_INTEGER, - .min = IPU3_RUNNING_MODE_VIDEO, - .max = IPU3_RUNNING_MODE_STILL, - .step = 1, - .def = IPU3_RUNNING_MODE_VIDEO, + .type = V4L2_CTRL_TYPE_MENU, + .max = ARRAY_SIZE(ipu3_ctrl_mode_strings) - 2, + .def = 0, + .qmenu = ipu3_ctrl_mode_strings, }; /******************** Framework registration ********************/
This addresses the below TODO item. - Use V4L2_CTRL_TYPE_MENU for dual-pipe mode control. (Sakari) Signed-off-by: Yong Zhi <yong.zhi@intel.com> --- drivers/staging/media/ipu3/TODO | 2 -- drivers/staging/media/ipu3/include/intel-ipu3.h | 6 ------ drivers/staging/media/ipu3/ipu3-v4l2.c | 18 +++++++++++++----- 3 files changed, 13 insertions(+), 13 deletions(-)