[libcamera-devel,RFC,v2,2/5] libcamera: V4L2Device: Support v4l2 menu control
diff mbox series

Message ID 20210413075013.3069213-3-hiroh@chromium.org
State Superseded
Headers show
Series
  • Report available test pattern modes
Related show

Commit Message

Hirokazu Honda April 13, 2021, 7:50 a.m. UTC 
This adds a support of v4l2 menu.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
---
 include/libcamera/internal/v4l2_controls.h |   1 +
 include/libcamera/internal/v4l2_device.h   |   3 +
 src/libcamera/v4l2_controls.cpp            |  10 +-
 src/libcamera/v4l2_device.cpp              | 104 ++++++++++++++++++++-
 4 files changed, 112 insertions(+), 6 deletions(-)

Comments

Kieran Bingham April 14, 2021, 8:20 a.m. UTC | #1 
Hi Hiro,

On 13/04/2021 08:50, Hirokazu Honda wrote:
> This adds a support of v4l2 menu.
> 
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> ---
>  include/libcamera/internal/v4l2_controls.h |   1 +
>  include/libcamera/internal/v4l2_device.h   |   3 +
>  src/libcamera/v4l2_controls.cpp            |  10 +-
>  src/libcamera/v4l2_device.cpp              | 104 ++++++++++++++++++++-
>  4 files changed, 112 insertions(+), 6 deletions(-)
> 
> diff --git a/include/libcamera/internal/v4l2_controls.h b/include/libcamera/internal/v4l2_controls.h
> index 0851b8dd..c42f2529 100644
> --- a/include/libcamera/internal/v4l2_controls.h
> +++ b/include/libcamera/internal/v4l2_controls.h
> @@ -24,6 +24,7 @@ class V4L2ControlInfo : public ControlInfo
>  {
>  public:
>  	V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);
> +	V4L2ControlInfo(const ControlInfo &ctrl);
>  };
>  
>  } /* namespace libcamera */
> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
> index 4cce3840..2436a83c 100644
> --- a/include/libcamera/internal/v4l2_device.h
> +++ b/include/libcamera/internal/v4l2_device.h
> @@ -53,6 +53,9 @@ protected:
>  	int fd() const { return fd_; }
>  
>  private:
> +	bool createV4L2ControlInfoForMenu(const v4l2_query_ext_ctrl &ctrl,
> +					  V4L2ControlInfo &v4l2CtrlInfo);
> +
>  	void listControls();
>  	void updateControls(ControlList *ctrls,
>  			    const std::vector<v4l2_ext_control> &v4l2Ctrls);
> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> index 3f8ec6ca..2c965cfa 100644
> --- a/src/libcamera/v4l2_controls.cpp
> +++ b/src/libcamera/v4l2_controls.cpp
> @@ -73,9 +73,12 @@ ControlType v4l2_ctrl_type(const struct v4l2_query_ext_ctrl &ctrl)
>  		return ControlTypeInteger64;
>  
>  	case V4L2_CTRL_TYPE_MENU:
> +	case V4L2_CTRL_TYPE_INTEGER_MENU:
> +		return ControlTypeMenu;
> +
>  	case V4L2_CTRL_TYPE_BUTTON:
>  	case V4L2_CTRL_TYPE_BITMASK:
> -	case V4L2_CTRL_TYPE_INTEGER_MENU:
> +
>  		/*
>  		 * More precise types may be needed, for now use a 32-bit
>  		 * integer type.
> @@ -148,4 +151,9 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
>  	}
>  }
>  
> +V4L2ControlInfo::V4L2ControlInfo(const ControlInfo &ctrl)
> +	: ControlInfo(ctrl)
> +{
> +}
> +
>  } /* namespace libcamera */
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 8f29cd7f..7fbca8b1 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -10,6 +10,7 @@
>  #include <fcntl.h>
>  #include <iomanip>
>  #include <limits.h>
> +#include <linux/intel-ipu3.h>


The V4L2 device should not be IPU3 specific in any way.


>  #include <stdlib.h>
>  #include <string.h>
>  #include <sys/ioctl.h>
> @@ -459,6 +460,60 @@ int V4L2Device::ioctl(unsigned long request, void *argp)
>   * \return The device node path
>   */
>  
> +bool V4L2Device::createV4L2ControlInfoForMenu(const v4l2_query_ext_ctrl &ctrl,
> +					      V4L2ControlInfo &v4l2CtrlInfo)
> +{
> +	ASSERT(ctrl.type == V4L2_CTRL_TYPE_MENU ||
> +	       ctrl.type == V4L2_CTRL_TYPE_INTEGER_MENU);
> +	bool isName = false;
> +	switch (ctrl.id) {
> +	case V4L2_CID_TEST_PATTERN:
> +	case V4L2_CID_INTEL_IPU3_MODE:
> +		isName = true;
> +		break;

Which means processing like this can't be here.

Is there a way to do this generically from the information you have in
the v4l2_query_ext_ctrl ?



> +	case V4L2_CID_LINK_FREQ:
> +		isName = false;
> +		break;
> +	default:
> +		LOG(V4L2, Error) << "Unsupported menu: " << ctrl.name
> +				 << " (" << utils::hex(ctrl.id) << ")";
> +		return false;
> +	}
> +
> +	std::vector<ControlValue> values;
> +	v4l2_querymenu menu;
> +	memset(&menu, 0, sizeof(menu));
> +	menu.id = ctrl.id;
> +	for (menu.index = ctrl.minimum;
> +	     static_cast<int>(menu.index) <= ctrl.maximum; menu.index++) {
> +		if (ioctl(VIDIOC_QUERYMENU, &menu) != 0)
> +			continue;
> +		ControlMenu ctrlMenu;
> +		ctrlMenu.index = static_cast<int32_t>(menu.index);
> +		ctrlMenu.isName = isName;
> +		if (isName) {
> +			strcpy(ctrlMenu.name,
> +			       reinterpret_cast<const char *>(menu.name));
> +		} else {
> +			ctrlMenu.value = menu.value;
> +		}
> +
> +		values.push_back(ControlValue(ctrlMenu));
> +	}
> +
> +	if (values.empty()) {
> +		LOG(V4L2, Error)
> +			<< "No applicable value: " << utils::hex(ctrl.id);
> +		return false;
> +	}
> +
> +	v4l2CtrlInfo = V4L2ControlInfo(
> +		ControlInfo(libcamera::Span<const ControlValue>(
> +			values.data(), values.size())));
> +
> +	return true;
> +}
> +
>  /**
>   * \fn V4L2Device::fd()
>   * \brief Retrieve the V4L2 device file descriptor
> @@ -474,7 +529,6 @@ void V4L2Device::listControls()
>  	ControlInfoMap::Map ctrls;
>  	struct v4l2_query_ext_ctrl ctrl = {};
>  
> -	/* \todo Add support for menu controls. */
>  	while (1) {
>  		ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL |
>  			   V4L2_CTRL_FLAG_NEXT_COMPOUND;
> @@ -485,15 +539,20 @@ void V4L2Device::listControls()
>  		    ctrl.flags & V4L2_CTRL_FLAG_DISABLED)
>  			continue;
>  
> +		V4L2ControlInfo v4l2CtrlInfo(ControlInfo(0));
>  		switch (ctrl.type) {
>  		case V4L2_CTRL_TYPE_INTEGER:
>  		case V4L2_CTRL_TYPE_BOOLEAN:
> -		case V4L2_CTRL_TYPE_MENU:
>  		case V4L2_CTRL_TYPE_BUTTON:
>  		case V4L2_CTRL_TYPE_INTEGER64:
>  		case V4L2_CTRL_TYPE_BITMASK:
> -		case V4L2_CTRL_TYPE_INTEGER_MENU:
>  		case V4L2_CTRL_TYPE_U8:
> +			v4l2CtrlInfo = V4L2ControlInfo(ctrl);
> +			break;
> +		case V4L2_CTRL_TYPE_MENU:
> +		case V4L2_CTRL_TYPE_INTEGER_MENU:
> +			if (!createV4L2ControlInfoForMenu(ctrl, v4l2CtrlInfo))
> +				continue;
>  			break;
>  		/* \todo Support other control types. */
>  		default:
> @@ -503,10 +562,13 @@ void V4L2Device::listControls()
>  			continue;
>  		}
>  
> +		LOG(V4L2, Debug) << "Control: " << ctrl.name
> +				 << " (" << utils::hex(ctrl.id) << ")";
> +
>  		controlIds_.emplace_back(std::make_unique<V4L2ControlId>(ctrl));
>  		controlInfo_.emplace(ctrl.id, ctrl);
>  
> -		ctrls.emplace(controlIds_.back().get(), V4L2ControlInfo(ctrl));
> +		ctrls.emplace(controlIds_.back().get(), std::move(v4l2CtrlInfo));
>  	}
>  
>  	controls_ = std::move(ctrls);
> @@ -539,7 +601,39 @@ void V4L2Device::updateControls(ControlList *ctrls,
>  		case ControlTypeInteger64:
>  			newValue.set<int64_t>(v4l2Ctrl.value64);
>  			break;
> -
> +		case ControlTypeMenu:
> +			switch (v4l2Ctrl.id) {
> +			case V4L2_CID_TEST_PATTERN: {
> +				ControlMenu menu;
> +				menu.index = v4l2Ctrl.value;
> +				menu.isName = true;
> +
> +				bool found = false;
> +				for (const ControlValue &validValue : it->second.values()) {
> +					const auto &ctrlMenu = validValue.get<ControlMenu>();
> +					if (!ctrlMenu.isName)
> +						continue;
> +					if (menu.index == ctrlMenu.index) {
> +						strcpy(menu.name, ctrlMenu.name);
> +						found = true;
> +						break;
> +					}
> +				}
> +
> +				if (!found) {
> +					LOG(V4L2, Error)
> +						<< "Matched value is not found"
> +						<< ", index=" << menu.index;
> +					continue;
> +				}
> +
> +				newValue.set<ControlMenu>(menu);
> +				break;
> +			}
> +			default:
> +				LOG(V4L2, Error) << "Unknown id: " << v4l2Ctrl.id;
> +				break;
> +			}
>  		case ControlTypeByte:
>  			/*
>  			 * No action required, the VIDIOC_[GS]_EXT_CTRLS ioctl
>
Hirokazu Honda April 15, 2021, 10:48 a.m. UTC | #2 
HI Kieran, Thanks for the comment.

On Wed, Apr 14, 2021 at 5:20 PM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Hi Hiro,
>
> On 13/04/2021 08:50, Hirokazu Honda wrote:
> > This adds a support of v4l2 menu.
> >
> > Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> > ---
> >  include/libcamera/internal/v4l2_controls.h |   1 +
> >  include/libcamera/internal/v4l2_device.h   |   3 +
> >  src/libcamera/v4l2_controls.cpp            |  10 +-
> >  src/libcamera/v4l2_device.cpp              | 104 ++++++++++++++++++++-
> >  4 files changed, 112 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/libcamera/internal/v4l2_controls.h b/include/libcamera/internal/v4l2_controls.h
> > index 0851b8dd..c42f2529 100644
> > --- a/include/libcamera/internal/v4l2_controls.h
> > +++ b/include/libcamera/internal/v4l2_controls.h
> > @@ -24,6 +24,7 @@ class V4L2ControlInfo : public ControlInfo
> >  {
> >  public:
> >       V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);
> > +     V4L2ControlInfo(const ControlInfo &ctrl);
> >  };
> >
> >  } /* namespace libcamera */
> > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
> > index 4cce3840..2436a83c 100644
> > --- a/include/libcamera/internal/v4l2_device.h
> > +++ b/include/libcamera/internal/v4l2_device.h
> > @@ -53,6 +53,9 @@ protected:
> >       int fd() const { return fd_; }
> >
> >  private:
> > +     bool createV4L2ControlInfoForMenu(const v4l2_query_ext_ctrl &ctrl,
> > +                                       V4L2ControlInfo &v4l2CtrlInfo);
> > +
> >       void listControls();
> >       void updateControls(ControlList *ctrls,
> >                           const std::vector<v4l2_ext_control> &v4l2Ctrls);
> > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> > index 3f8ec6ca..2c965cfa 100644
> > --- a/src/libcamera/v4l2_controls.cpp
> > +++ b/src/libcamera/v4l2_controls.cpp
> > @@ -73,9 +73,12 @@ ControlType v4l2_ctrl_type(const struct v4l2_query_ext_ctrl &ctrl)
> >               return ControlTypeInteger64;
> >
> >       case V4L2_CTRL_TYPE_MENU:
> > +     case V4L2_CTRL_TYPE_INTEGER_MENU:
> > +             return ControlTypeMenu;
> > +
> >       case V4L2_CTRL_TYPE_BUTTON:
> >       case V4L2_CTRL_TYPE_BITMASK:
> > -     case V4L2_CTRL_TYPE_INTEGER_MENU:
> > +
> >               /*
> >                * More precise types may be needed, for now use a 32-bit
> >                * integer type.
> > @@ -148,4 +151,9 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
> >       }
> >  }
> >
> > +V4L2ControlInfo::V4L2ControlInfo(const ControlInfo &ctrl)
> > +     : ControlInfo(ctrl)
> > +{
> > +}
> > +
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > index 8f29cd7f..7fbca8b1 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -10,6 +10,7 @@
> >  #include <fcntl.h>
> >  #include <iomanip>
> >  #include <limits.h>
> > +#include <linux/intel-ipu3.h>
>
>
> The V4L2 device should not be IPU3 specific in any way.
>
>
> >  #include <stdlib.h>
> >  #include <string.h>
> >  #include <sys/ioctl.h>
> > @@ -459,6 +460,60 @@ int V4L2Device::ioctl(unsigned long request, void *argp)
> >   * \return The device node path
> >   */
> >
> > +bool V4L2Device::createV4L2ControlInfoForMenu(const v4l2_query_ext_ctrl &ctrl,
> > +                                           V4L2ControlInfo &v4l2CtrlInfo)
> > +{
> > +     ASSERT(ctrl.type == V4L2_CTRL_TYPE_MENU ||
> > +            ctrl.type == V4L2_CTRL_TYPE_INTEGER_MENU);
> > +     bool isName = false;
> > +     switch (ctrl.id) {
> > +     case V4L2_CID_TEST_PATTERN:
> > +     case V4L2_CID_INTEL_IPU3_MODE:
> > +             isName = true;
> > +             break;
>
> Which means processing like this can't be here.
>
> Is there a way to do this generically from the information you have in
> the v4l2_query_ext_ctrl ?
>
>

Okay, I think this should be
isName = ctrl.type == V4L2_CTRL_TYPE_MENU;
Then we removed this.
I would do it in the next patch series.

Thanks,
-Hiro

>
> > +     case V4L2_CID_LINK_FREQ:
> > +             isName = false;
> > +             break;
> > +     default:
> > +             LOG(V4L2, Error) << "Unsupported menu: " << ctrl.name
> > +                              << " (" << utils::hex(ctrl.id) << ")";
> > +             return false;
> > +     }
> > +
> > +     std::vector<ControlValue> values;
> > +     v4l2_querymenu menu;
> > +     memset(&menu, 0, sizeof(menu));
> > +     menu.id = ctrl.id;
> > +     for (menu.index = ctrl.minimum;
> > +          static_cast<int>(menu.index) <= ctrl.maximum; menu.index++) {
> > +             if (ioctl(VIDIOC_QUERYMENU, &menu) != 0)
> > +                     continue;
> > +             ControlMenu ctrlMenu;
> > +             ctrlMenu.index = static_cast<int32_t>(menu.index);
> > +             ctrlMenu.isName = isName;
> > +             if (isName) {
> > +                     strcpy(ctrlMenu.name,
> > +                            reinterpret_cast<const char *>(menu.name));
> > +             } else {
> > +                     ctrlMenu.value = menu.value;
> > +             }
> > +
> > +             values.push_back(ControlValue(ctrlMenu));
> > +     }
> > +
> > +     if (values.empty()) {
> > +             LOG(V4L2, Error)
> > +                     << "No applicable value: " << utils::hex(ctrl.id);
> > +             return false;
> > +     }
> > +
> > +     v4l2CtrlInfo = V4L2ControlInfo(
> > +             ControlInfo(libcamera::Span<const ControlValue>(
> > +                     values.data(), values.size())));
> > +
> > +     return true;
> > +}
> > +
> >  /**
> >   * \fn V4L2Device::fd()
> >   * \brief Retrieve the V4L2 device file descriptor
> > @@ -474,7 +529,6 @@ void V4L2Device::listControls()
> >       ControlInfoMap::Map ctrls;
> >       struct v4l2_query_ext_ctrl ctrl = {};
> >
> > -     /* \todo Add support for menu controls. */
> >       while (1) {
> >               ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL |
> >                          V4L2_CTRL_FLAG_NEXT_COMPOUND;
> > @@ -485,15 +539,20 @@ void V4L2Device::listControls()
> >                   ctrl.flags & V4L2_CTRL_FLAG_DISABLED)
> >                       continue;
> >
> > +             V4L2ControlInfo v4l2CtrlInfo(ControlInfo(0));
> >               switch (ctrl.type) {
> >               case V4L2_CTRL_TYPE_INTEGER:
> >               case V4L2_CTRL_TYPE_BOOLEAN:
> > -             case V4L2_CTRL_TYPE_MENU:
> >               case V4L2_CTRL_TYPE_BUTTON:
> >               case V4L2_CTRL_TYPE_INTEGER64:
> >               case V4L2_CTRL_TYPE_BITMASK:
> > -             case V4L2_CTRL_TYPE_INTEGER_MENU:
> >               case V4L2_CTRL_TYPE_U8:
> > +                     v4l2CtrlInfo = V4L2ControlInfo(ctrl);
> > +                     break;
> > +             case V4L2_CTRL_TYPE_MENU:
> > +             case V4L2_CTRL_TYPE_INTEGER_MENU:
> > +                     if (!createV4L2ControlInfoForMenu(ctrl, v4l2CtrlInfo))
> > +                             continue;
> >                       break;
> >               /* \todo Support other control types. */
> >               default:
> > @@ -503,10 +562,13 @@ void V4L2Device::listControls()
> >                       continue;
> >               }
> >
> > +             LOG(V4L2, Debug) << "Control: " << ctrl.name
> > +                              << " (" << utils::hex(ctrl.id) << ")";
> > +
> >               controlIds_.emplace_back(std::make_unique<V4L2ControlId>(ctrl));
> >               controlInfo_.emplace(ctrl.id, ctrl);
> >
> > -             ctrls.emplace(controlIds_.back().get(), V4L2ControlInfo(ctrl));
> > +             ctrls.emplace(controlIds_.back().get(), std::move(v4l2CtrlInfo));
> >       }
> >
> >       controls_ = std::move(ctrls);
> > @@ -539,7 +601,39 @@ void V4L2Device::updateControls(ControlList *ctrls,
> >               case ControlTypeInteger64:
> >                       newValue.set<int64_t>(v4l2Ctrl.value64);
> >                       break;
> > -
> > +             case ControlTypeMenu:
> > +                     switch (v4l2Ctrl.id) {
> > +                     case V4L2_CID_TEST_PATTERN: {
> > +                             ControlMenu menu;
> > +                             menu.index = v4l2Ctrl.value;
> > +                             menu.isName = true;
> > +
> > +                             bool found = false;
> > +                             for (const ControlValue &validValue : it->second.values()) {
> > +                                     const auto &ctrlMenu = validValue.get<ControlMenu>();
> > +                                     if (!ctrlMenu.isName)
> > +                                             continue;
> > +                                     if (menu.index == ctrlMenu.index) {
> > +                                             strcpy(menu.name, ctrlMenu.name);
> > +                                             found = true;
> > +                                             break;
> > +                                     }
> > +                             }
> > +
> > +                             if (!found) {
> > +                                     LOG(V4L2, Error)
> > +                                             << "Matched value is not found"
> > +                                             << ", index=" << menu.index;
> > +                                     continue;
> > +                             }
> > +
> > +                             newValue.set<ControlMenu>(menu);
> > +                             break;
> > +                     }
> > +                     default:
> > +                             LOG(V4L2, Error) << "Unknown id: " << v4l2Ctrl.id;
> > +                             break;
> > +                     }
> >               case ControlTypeByte:
> >                       /*
> >                        * No action required, the VIDIOC_[GS]_EXT_CTRLS ioctl
> >
>
> --
> Regards
> --
> Kieran

Patch
diff mbox series 

diff --git a/include/libcamera/internal/v4l2_controls.h b/include/libcamera/internal/v4l2_controls.h
index 0851b8dd..c42f2529 100644
--- a/include/libcamera/internal/v4l2_controls.h
+++ b/include/libcamera/internal/v4l2_controls.h
@@ -24,6 +24,7 @@  class V4L2ControlInfo : public ControlInfo
 {
 public:
 	V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);
+	V4L2ControlInfo(const ControlInfo &ctrl);
 };
 
 } /* namespace libcamera */
diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
index 4cce3840..2436a83c 100644
--- a/include/libcamera/internal/v4l2_device.h
+++ b/include/libcamera/internal/v4l2_device.h
@@ -53,6 +53,9 @@  protected:
 	int fd() const { return fd_; }
 
 private:
+	bool createV4L2ControlInfoForMenu(const v4l2_query_ext_ctrl &ctrl,
+					  V4L2ControlInfo &v4l2CtrlInfo);
+
 	void listControls();
 	void updateControls(ControlList *ctrls,
 			    const std::vector<v4l2_ext_control> &v4l2Ctrls);
diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
index 3f8ec6ca..2c965cfa 100644
--- a/src/libcamera/v4l2_controls.cpp
+++ b/src/libcamera/v4l2_controls.cpp
@@ -73,9 +73,12 @@  ControlType v4l2_ctrl_type(const struct v4l2_query_ext_ctrl &ctrl)
 		return ControlTypeInteger64;
 
 	case V4L2_CTRL_TYPE_MENU:
+	case V4L2_CTRL_TYPE_INTEGER_MENU:
+		return ControlTypeMenu;
+
 	case V4L2_CTRL_TYPE_BUTTON:
 	case V4L2_CTRL_TYPE_BITMASK:
-	case V4L2_CTRL_TYPE_INTEGER_MENU:
+
 		/*
 		 * More precise types may be needed, for now use a 32-bit
 		 * integer type.
@@ -148,4 +151,9 @@  V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
 	}
 }
 
+V4L2ControlInfo::V4L2ControlInfo(const ControlInfo &ctrl)
+	: ControlInfo(ctrl)
+{
+}
+
 } /* namespace libcamera */
diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
index 8f29cd7f..7fbca8b1 100644
--- a/src/libcamera/v4l2_device.cpp
+++ b/src/libcamera/v4l2_device.cpp
@@ -10,6 +10,7 @@ 
 #include <fcntl.h>
 #include <iomanip>
 #include <limits.h>
+#include <linux/intel-ipu3.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/ioctl.h>
@@ -459,6 +460,60 @@  int V4L2Device::ioctl(unsigned long request, void *argp)
  * \return The device node path
  */
 
+bool V4L2Device::createV4L2ControlInfoForMenu(const v4l2_query_ext_ctrl &ctrl,
+					      V4L2ControlInfo &v4l2CtrlInfo)
+{
+	ASSERT(ctrl.type == V4L2_CTRL_TYPE_MENU ||
+	       ctrl.type == V4L2_CTRL_TYPE_INTEGER_MENU);
+	bool isName = false;
+	switch (ctrl.id) {
+	case V4L2_CID_TEST_PATTERN:
+	case V4L2_CID_INTEL_IPU3_MODE:
+		isName = true;
+		break;
+	case V4L2_CID_LINK_FREQ:
+		isName = false;
+		break;
+	default:
+		LOG(V4L2, Error) << "Unsupported menu: " << ctrl.name
+				 << " (" << utils::hex(ctrl.id) << ")";
+		return false;
+	}
+
+	std::vector<ControlValue> values;
+	v4l2_querymenu menu;
+	memset(&menu, 0, sizeof(menu));
+	menu.id = ctrl.id;
+	for (menu.index = ctrl.minimum;
+	     static_cast<int>(menu.index) <= ctrl.maximum; menu.index++) {
+		if (ioctl(VIDIOC_QUERYMENU, &menu) != 0)
+			continue;
+		ControlMenu ctrlMenu;
+		ctrlMenu.index = static_cast<int32_t>(menu.index);
+		ctrlMenu.isName = isName;
+		if (isName) {
+			strcpy(ctrlMenu.name,
+			       reinterpret_cast<const char *>(menu.name));
+		} else {
+			ctrlMenu.value = menu.value;
+		}
+
+		values.push_back(ControlValue(ctrlMenu));
+	}
+
+	if (values.empty()) {
+		LOG(V4L2, Error)
+			<< "No applicable value: " << utils::hex(ctrl.id);
+		return false;
+	}
+
+	v4l2CtrlInfo = V4L2ControlInfo(
+		ControlInfo(libcamera::Span<const ControlValue>(
+			values.data(), values.size())));
+
+	return true;
+}
+
 /**
  * \fn V4L2Device::fd()
  * \brief Retrieve the V4L2 device file descriptor
@@ -474,7 +529,6 @@  void V4L2Device::listControls()
 	ControlInfoMap::Map ctrls;
 	struct v4l2_query_ext_ctrl ctrl = {};
 
-	/* \todo Add support for menu controls. */
 	while (1) {
 		ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL |
 			   V4L2_CTRL_FLAG_NEXT_COMPOUND;
@@ -485,15 +539,20 @@  void V4L2Device::listControls()
 		    ctrl.flags & V4L2_CTRL_FLAG_DISABLED)
 			continue;
 
+		V4L2ControlInfo v4l2CtrlInfo(ControlInfo(0));
 		switch (ctrl.type) {
 		case V4L2_CTRL_TYPE_INTEGER:
 		case V4L2_CTRL_TYPE_BOOLEAN:
-		case V4L2_CTRL_TYPE_MENU:
 		case V4L2_CTRL_TYPE_BUTTON:
 		case V4L2_CTRL_TYPE_INTEGER64:
 		case V4L2_CTRL_TYPE_BITMASK:
-		case V4L2_CTRL_TYPE_INTEGER_MENU:
 		case V4L2_CTRL_TYPE_U8:
+			v4l2CtrlInfo = V4L2ControlInfo(ctrl);
+			break;
+		case V4L2_CTRL_TYPE_MENU:
+		case V4L2_CTRL_TYPE_INTEGER_MENU:
+			if (!createV4L2ControlInfoForMenu(ctrl, v4l2CtrlInfo))
+				continue;
 			break;
 		/* \todo Support other control types. */
 		default:
@@ -503,10 +562,13 @@  void V4L2Device::listControls()
 			continue;
 		}
 
+		LOG(V4L2, Debug) << "Control: " << ctrl.name
+				 << " (" << utils::hex(ctrl.id) << ")";
+
 		controlIds_.emplace_back(std::make_unique<V4L2ControlId>(ctrl));
 		controlInfo_.emplace(ctrl.id, ctrl);
 
-		ctrls.emplace(controlIds_.back().get(), V4L2ControlInfo(ctrl));
+		ctrls.emplace(controlIds_.back().get(), std::move(v4l2CtrlInfo));
 	}
 
 	controls_ = std::move(ctrls);
@@ -539,7 +601,39 @@  void V4L2Device::updateControls(ControlList *ctrls,
 		case ControlTypeInteger64:
 			newValue.set<int64_t>(v4l2Ctrl.value64);
 			break;
-
+		case ControlTypeMenu:
+			switch (v4l2Ctrl.id) {
+			case V4L2_CID_TEST_PATTERN: {
+				ControlMenu menu;
+				menu.index = v4l2Ctrl.value;
+				menu.isName = true;
+
+				bool found = false;
+				for (const ControlValue &validValue : it->second.values()) {
+					const auto &ctrlMenu = validValue.get<ControlMenu>();
+					if (!ctrlMenu.isName)
+						continue;
+					if (menu.index == ctrlMenu.index) {
+						strcpy(menu.name, ctrlMenu.name);
+						found = true;
+						break;
+					}
+				}
+
+				if (!found) {
+					LOG(V4L2, Error)
+						<< "Matched value is not found"
+						<< ", index=" << menu.index;
+					continue;
+				}
+
+				newValue.set<ControlMenu>(menu);
+				break;
+			}
+			default:
+				LOG(V4L2, Error) << "Unknown id: " << v4l2Ctrl.id;
+				break;
+			}
 		case ControlTypeByte:
 			/*
 			 * No action required, the VIDIOC_[GS]_EXT_CTRLS ioctl