Re: [REVIEW PATCH 09/14] OMAP: CAM: Add ISP Core

[Sakari Ailus <sakari.ailus@nokia.com>]

DongSoo(Nathaniel) Kim wrote:
> Hello,
> 
> reviewing ISP driver, I found that we've got no querymenu support in
> ISP and also omap3 camera interface driver.
> 
> +/**
> + * struct vcontrol - Video control structure.
> + * @qc: V4L2 Query control structure.
> + * @current_value: Current value of the control.
> + */
> +static struct vcontrol {
> +       struct v4l2_queryctrl qc;
> +       int current_value;
> +} video_control[] = {
> 
> <snip>
> 
> +       {
> +               {
> +                       .id = V4L2_CID_PRIVATE_ISP_COLOR_FX,
> +                       .type = V4L2_CTRL_TYPE_INTEGER,
> +                       .name = "Color Effects",
> +                       .minimum = PREV_DEFAULT_COLOR,
> +                       .maximum = PREV_BW_COLOR,
> +                       .step = 1,
> +                       .default_value = PREV_DEFAULT_COLOR,
> +               },
> +               .current_value = PREV_DEFAULT_COLOR,
> +       }
> +};

I don't see these lines in Sergio's patch you replied to. I have 
V4L2_CTRL_TYPE_MENU there instead of V4L2_CTRL_TYPE_INTEGER.

> I think we should make it menu type for this color FX control.
> If that kind of control has no menu information, user has no way to
> figure out what kind of FX supported by device.
> BTW if we make querymenu support in omap3 camera subsystem, we should
> make querymenu support for v4l2 int device also.
> I think I've seen before a patch which intent to use querymenu in v4l2
> int device, but no patch for omap3 ISP and camera interface.
> Can I make a patch and post on linux-omap, linux-media list? of course
> if you don't mind.
> Or...am I digging wrong way? I mean.. querymenu for omap3 camera subsystem.
> Please let me know :)

I'm always happy to get patches. :-) But my understanding is that 
querymenu is implemented in ISP and camera drivers --- though I haven't 
tried it myself.

Cheers,

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com