[RFCv2 PATCH 00/10] Matrix and Motion Detection support

[RFCv2 PATCH 00/10] Matrix and Motion Detection support

[Hans Verkuil]

This patch series adds support for matrices and motion detection and
converts the solo6x10 and go7007 driver to use these new APIs.

See this RFCv2 for details on the motion detection API:

http://www.mail-archive.com/linux-media@vger.kernel.org/msg62085.html

And this RFC for details on the matrix API (which superseeds the v4l2_md_blocks
in the RFC above):

http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/65195

I have tested this with the solo and go7007 boards, both global motion detection
and regional motion detection, and it works well. Although note the commit
message for patch 10 regarding some uncertainties regarding regional MD in
the go7007 driver.

Changes since the first RFC patch series:

- document the new APIs
- implemented motion detection in the go7007 driver

I have adapted v4l2-ctl to support the new APIs:

http://git.linuxtv.org/hverkuil/v4l-utils.git/shortlog/refs/heads/matrix

If there are no more comments regarding this patch series, then I'll make
a pull request for this.

Once this is in, I can move the solo and go7007 drivers into the mainline
kernel, since the missing motion detection API is the only bit keeping
them in staging.

Regards,

        Hans

[RFCv2 PATCH 01/10] v4l2-controls: add motion detection controls.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Add support for two motion detection controls and a 'detect control class'.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 33 +++++++++++++++++++++++++++------
 include/uapi/linux/v4l2-controls.h   | 14 ++++++++++++++
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index fccd08b..89e7cfb 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -456,6 +456,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		"RGB full range (0-255)",
 		NULL,
 	};
+	static const char * const detect_motion_mode[] = {
+		"Disabled",
+		"Global",
+		"Regional",
+		NULL,
+	};
 
 
 	switch (id) {
@@ -545,6 +551,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 	case V4L2_CID_DV_TX_RGB_RANGE:
 	case V4L2_CID_DV_RX_RGB_RANGE:
 		return dv_rgb_range;
+	case V4L2_CID_DETECT_MOTION_MODE:
+		return detect_motion_mode;
 
 	default:
 		return NULL;
@@ -557,7 +565,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 {
 	switch (id) {
 	/* USER controls */
-	/* Keep the order of the 'case's the same as in videodev2.h! */
+	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
 	case V4L2_CID_USER_CLASS:		return "User Controls";
 	case V4L2_CID_BRIGHTNESS:		return "Brightness";
 	case V4L2_CID_CONTRAST:			return "Contrast";
@@ -601,7 +609,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_COLORFX_CBCR:		return "Color Effects, CbCr";
 
 	/* MPEG controls */
-	/* Keep the order of the 'case's the same as in videodev2.h! */
+	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
 	case V4L2_CID_MPEG_CLASS:		return "MPEG Encoder Controls";
 	case V4L2_CID_MPEG_STREAM_TYPE:		return "Stream Type";
 	case V4L2_CID_MPEG_STREAM_PID_PMT:	return "Stream PMT Program ID";
@@ -701,7 +709,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER:		return "Repeat Sequence Header";
 
 	/* CAMERA controls */
-	/* Keep the order of the 'case's the same as in videodev2.h! */
+	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
 	case V4L2_CID_CAMERA_CLASS:		return "Camera Controls";
 	case V4L2_CID_EXPOSURE_AUTO:		return "Auto Exposure";
 	case V4L2_CID_EXPOSURE_ABSOLUTE:	return "Exposure Time, Absolute";
@@ -735,8 +743,8 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_AUTO_FOCUS_STATUS:	return "Auto Focus, Status";
 	case V4L2_CID_AUTO_FOCUS_RANGE:		return "Auto Focus, Range";
 
-	/* FM Radio Modulator control */
-	/* Keep the order of the 'case's the same as in videodev2.h! */
+	/* FM Radio Modulator controls */
+	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
 	case V4L2_CID_FM_TX_CLASS:		return "FM Radio Modulator Controls";
 	case V4L2_CID_RDS_TX_DEVIATION:		return "RDS Signal Deviation";
 	case V4L2_CID_RDS_TX_PI:		return "RDS Program ID";
@@ -759,6 +767,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_TUNE_ANTENNA_CAPACITOR:	return "Tune Antenna Capacitor";
 
 	/* Flash controls */
+	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
 	case V4L2_CID_FLASH_CLASS:		return "Flash Controls";
 	case V4L2_CID_FLASH_LED_MODE:		return "LED Mode";
 	case V4L2_CID_FLASH_STROBE_SOURCE:	return "Strobe Source";
@@ -774,7 +783,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_FLASH_READY:		return "Ready to Strobe";
 
 	/* JPEG encoder controls */
-	/* Keep the order of the 'case's the same as in videodev2.h! */
+	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
 	case V4L2_CID_JPEG_CLASS:		return "JPEG Compression Controls";
 	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:	return "Chroma Subsampling";
 	case V4L2_CID_JPEG_RESTART_INTERVAL:	return "Restart Interval";
@@ -782,18 +791,21 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_JPEG_ACTIVE_MARKER:	return "Active Markers";
 
 	/* Image source controls */
+	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
 	case V4L2_CID_IMAGE_SOURCE_CLASS:	return "Image Source Controls";
 	case V4L2_CID_VBLANK:			return "Vertical Blanking";
 	case V4L2_CID_HBLANK:			return "Horizontal Blanking";
 	case V4L2_CID_ANALOGUE_GAIN:		return "Analogue Gain";
 
 	/* Image processing controls */
+	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
 	case V4L2_CID_IMAGE_PROC_CLASS:		return "Image Processing Controls";
 	case V4L2_CID_LINK_FREQ:		return "Link Frequency";
 	case V4L2_CID_PIXEL_RATE:		return "Pixel Rate";
 	case V4L2_CID_TEST_PATTERN:		return "Test Pattern";
 
 	/* DV controls */
+	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
 	case V4L2_CID_DV_CLASS:			return "Digital Video Controls";
 	case V4L2_CID_DV_TX_HOTPLUG:		return "Hotplug Present";
 	case V4L2_CID_DV_TX_RXSENSE:		return "RxSense Present";
@@ -806,6 +818,12 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_FM_RX_CLASS:		return "FM Radio Receiver Controls";
 	case V4L2_CID_TUNE_DEEMPHASIS:		return "De-Emphasis";
 	case V4L2_CID_RDS_RECEPTION:		return "RDS Reception";
+
+	/* FM Radio Receiver controls */
+	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
+	case V4L2_CID_DETECT_CLASS:		return "Detection Controls";
+	case V4L2_CID_DETECT_MOTION_MODE:	return "Motion Detection Mode";
+	case V4L2_CID_DETECT_MOTION_THRESHOLD:	return "Motion Detection Threshold";
 	default:
 		return NULL;
 	}
@@ -914,6 +932,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_DV_RX_RGB_RANGE:
 	case V4L2_CID_TEST_PATTERN:
 	case V4L2_CID_TUNE_DEEMPHASIS:
+	case V4L2_CID_DETECT_MOTION_MODE:
 		*type = V4L2_CTRL_TYPE_MENU;
 		break;
 	case V4L2_CID_LINK_FREQ:
@@ -937,6 +956,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_IMAGE_PROC_CLASS:
 	case V4L2_CID_DV_CLASS:
 	case V4L2_CID_FM_RX_CLASS:
+	case V4L2_CID_DETECT_CLASS:
 		*type = V4L2_CTRL_TYPE_CTRL_CLASS;
 		/* You can neither read not write these */
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
@@ -1009,6 +1029,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_PILOT_TONE_FREQUENCY:
 	case V4L2_CID_TUNE_POWER_LEVEL:
 	case V4L2_CID_TUNE_ANTENNA_CAPACITOR:
+	case V4L2_CID_DETECT_MOTION_THRESHOLD:
 		*flags |= V4L2_CTRL_FLAG_SLIDER;
 		break;
 	case V4L2_CID_PAN_RELATIVE:
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index e90a88a..d88eebd 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -60,6 +60,7 @@
 #define V4L2_CTRL_CLASS_IMAGE_PROC	0x009f0000	/* Image processing controls */
 #define V4L2_CTRL_CLASS_DV		0x00a00000	/* Digital Video controls */
 #define V4L2_CTRL_CLASS_FM_RX		0x00a10000	/* FM Receiver controls */
+#define V4L2_CTRL_CLASS_DETECT		0x00a20000	/* Detection controls */
 
 /* User-class control IDs */
 
@@ -853,4 +854,17 @@ enum v4l2_deemphasis {
 
 #define V4L2_CID_RDS_RECEPTION			(V4L2_CID_FM_RX_CLASS_BASE + 2)
 
+
+/*  Detection-class control IDs defined by V4L2 */
+#define V4L2_CID_DETECT_CLASS_BASE		(V4L2_CTRL_CLASS_DETECT | 0x900)
+#define V4L2_CID_DETECT_CLASS			(V4L2_CTRL_CLASS_DETECT | 1)
+
+#define	V4L2_CID_DETECT_MOTION_MODE		(V4L2_CID_DETECT_CLASS_BASE + 1)
+enum v4l2_detect_motion_mode {
+	V4L2_DETECT_MOTION_DISABLED	= 0,
+	V4L2_DETECT_MOTION_GLOBAL	= 1,
+	V4L2_DETECT_MOTION_REGIONAL	= 2,
+};
+#define	V4L2_CID_DETECT_MOTION_THRESHOLD	(V4L2_CID_DETECT_CLASS_BASE + 2)
+
 #endif
-- 
1.8.3.2

Re: [RFCv2 PATCH 01/10] v4l2-controls: add motion detection controls.

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Monday 12 August 2013 12:58:24 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Add support for two motion detection controls and a 'detect control class'.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 33 +++++++++++++++++++++++++++------
> include/uapi/linux/v4l2-controls.h   | 14 ++++++++++++++
>  2 files changed, 41 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> b/drivers/media/v4l2-core/v4l2-ctrls.c index fccd08b..89e7cfb 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -456,6 +456,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  		"RGB full range (0-255)",
>  		NULL,
>  	};
> +	static const char * const detect_motion_mode[] = {
> +		"Disabled",
> +		"Global",
> +		"Regional",
> +		NULL,
> +	};
> 
> 
>  	switch (id) {
> @@ -545,6 +551,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  	case V4L2_CID_DV_TX_RGB_RANGE:
>  	case V4L2_CID_DV_RX_RGB_RANGE:
>  		return dv_rgb_range;
> +	case V4L2_CID_DETECT_MOTION_MODE:
> +		return detect_motion_mode;
> 
>  	default:
>  		return NULL;
> @@ -557,7 +565,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  {
>  	switch (id) {
>  	/* USER controls */
> -	/* Keep the order of the 'case's the same as in videodev2.h! */
> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */

Maybe we could replace all the individual occurences of that comment with a 
single one at the beginning of the switch ?

>  	case V4L2_CID_USER_CLASS:		return "User Controls";
>  	case V4L2_CID_BRIGHTNESS:		return "Brightness";
>  	case V4L2_CID_CONTRAST:			return "Contrast";
> @@ -601,7 +609,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_COLORFX_CBCR:		return "Color Effects, CbCr";
> 
>  	/* MPEG controls */
> -	/* Keep the order of the 'case's the same as in videodev2.h! */
> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>  	case V4L2_CID_MPEG_CLASS:		return "MPEG Encoder Controls";
>  	case V4L2_CID_MPEG_STREAM_TYPE:		return "Stream Type";
>  	case V4L2_CID_MPEG_STREAM_PID_PMT:	return "Stream PMT Program ID";
> @@ -701,7 +709,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER:		return "Repeat Sequence
> Header";
> 
>  	/* CAMERA controls */
> -	/* Keep the order of the 'case's the same as in videodev2.h! */
> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>  	case V4L2_CID_CAMERA_CLASS:		return "Camera Controls";
>  	case V4L2_CID_EXPOSURE_AUTO:		return "Auto Exposure";
>  	case V4L2_CID_EXPOSURE_ABSOLUTE:	return "Exposure Time, Absolute";
> @@ -735,8 +743,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_AUTO_FOCUS_STATUS:	return "Auto Focus, Status";
>  	case V4L2_CID_AUTO_FOCUS_RANGE:		return "Auto Focus, Range";
> 
> -	/* FM Radio Modulator control */
> -	/* Keep the order of the 'case's the same as in videodev2.h! */
> +	/* FM Radio Modulator controls */
> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>  	case V4L2_CID_FM_TX_CLASS:		return "FM Radio Modulator Controls";
>  	case V4L2_CID_RDS_TX_DEVIATION:		return "RDS Signal Deviation";
>  	case V4L2_CID_RDS_TX_PI:		return "RDS Program ID";
> @@ -759,6 +767,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_TUNE_ANTENNA_CAPACITOR:	return "Tune Antenna Capacitor";
> 
>  	/* Flash controls */
> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>  	case V4L2_CID_FLASH_CLASS:		return "Flash Controls";
>  	case V4L2_CID_FLASH_LED_MODE:		return "LED Mode";
>  	case V4L2_CID_FLASH_STROBE_SOURCE:	return "Strobe Source";
> @@ -774,7 +783,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_FLASH_READY:		return "Ready to Strobe";
> 
>  	/* JPEG encoder controls */
> -	/* Keep the order of the 'case's the same as in videodev2.h! */
> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>  	case V4L2_CID_JPEG_CLASS:		return "JPEG Compression Controls";
>  	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:	return "Chroma Subsampling";
>  	case V4L2_CID_JPEG_RESTART_INTERVAL:	return "Restart Interval";
> @@ -782,18 +791,21 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_JPEG_ACTIVE_MARKER:	return "Active Markers";
> 
>  	/* Image source controls */
> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>  	case V4L2_CID_IMAGE_SOURCE_CLASS:	return "Image Source Controls";
>  	case V4L2_CID_VBLANK:			return "Vertical Blanking";
>  	case V4L2_CID_HBLANK:			return "Horizontal Blanking";
>  	case V4L2_CID_ANALOGUE_GAIN:		return "Analogue Gain";
> 
>  	/* Image processing controls */
> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>  	case V4L2_CID_IMAGE_PROC_CLASS:		return "Image Processing Controls";
>  	case V4L2_CID_LINK_FREQ:		return "Link Frequency";
>  	case V4L2_CID_PIXEL_RATE:		return "Pixel Rate";
>  	case V4L2_CID_TEST_PATTERN:		return "Test Pattern";
> 
>  	/* DV controls */
> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>  	case V4L2_CID_DV_CLASS:			return "Digital Video Controls";
>  	case V4L2_CID_DV_TX_HOTPLUG:		return "Hotplug Present";
>  	case V4L2_CID_DV_TX_RXSENSE:		return "RxSense Present";
> @@ -806,6 +818,12 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_FM_RX_CLASS:		return "FM Radio Receiver Controls";
>  	case V4L2_CID_TUNE_DEEMPHASIS:		return "De-Emphasis";
>  	case V4L2_CID_RDS_RECEPTION:		return "RDS Reception";
> +
> +	/* FM Radio Receiver controls */
> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
> +	case V4L2_CID_DETECT_CLASS:		return "Detection Controls";
> +	case V4L2_CID_DETECT_MOTION_MODE:	return "Motion Detection Mode";
> +	case V4L2_CID_DETECT_MOTION_THRESHOLD:	return "Motion Detection
> Threshold"; default:
>  		return NULL;
>  	}
> @@ -914,6 +932,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> v4l2_ctrl_type *type, case V4L2_CID_DV_RX_RGB_RANGE:
>  	case V4L2_CID_TEST_PATTERN:
>  	case V4L2_CID_TUNE_DEEMPHASIS:
> +	case V4L2_CID_DETECT_MOTION_MODE:
>  		*type = V4L2_CTRL_TYPE_MENU;
>  		break;
>  	case V4L2_CID_LINK_FREQ:
> @@ -937,6 +956,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> v4l2_ctrl_type *type, case V4L2_CID_IMAGE_PROC_CLASS:
>  	case V4L2_CID_DV_CLASS:
>  	case V4L2_CID_FM_RX_CLASS:
> +	case V4L2_CID_DETECT_CLASS:
>  		*type = V4L2_CTRL_TYPE_CTRL_CLASS;
>  		/* You can neither read not write these */
>  		*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
> @@ -1009,6 +1029,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> v4l2_ctrl_type *type, case V4L2_CID_PILOT_TONE_FREQUENCY:
>  	case V4L2_CID_TUNE_POWER_LEVEL:
>  	case V4L2_CID_TUNE_ANTENNA_CAPACITOR:
> +	case V4L2_CID_DETECT_MOTION_THRESHOLD:
>  		*flags |= V4L2_CTRL_FLAG_SLIDER;
>  		break;
>  	case V4L2_CID_PAN_RELATIVE:
> diff --git a/include/uapi/linux/v4l2-controls.h
> b/include/uapi/linux/v4l2-controls.h index e90a88a..d88eebd 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -60,6 +60,7 @@
>  #define V4L2_CTRL_CLASS_IMAGE_PROC	0x009f0000	/* Image processing 
controls
> */ #define V4L2_CTRL_CLASS_DV		0x00a00000	/* Digital Video controls */
> #define V4L2_CTRL_CLASS_FM_RX		0x00a10000	/* FM Receiver controls */
> +#define V4L2_CTRL_CLASS_DETECT		0x00a20000	/* Detection controls */
> 
>  /* User-class control IDs */
> 
> @@ -853,4 +854,17 @@ enum v4l2_deemphasis {
> 
>  #define V4L2_CID_RDS_RECEPTION			(V4L2_CID_FM_RX_CLASS_BASE + 2)
> 
> +
> +/*  Detection-class control IDs defined by V4L2 */
> +#define V4L2_CID_DETECT_CLASS_BASE		(V4L2_CTRL_CLASS_DETECT | 0x900)
> +#define V4L2_CID_DETECT_CLASS			(V4L2_CTRL_CLASS_DETECT | 1)
> +
> +#define	V4L2_CID_DETECT_MOTION_MODE		(V4L2_CID_DETECT_CLASS_BASE + 1)
> +enum v4l2_detect_motion_mode {
> +	V4L2_DETECT_MOTION_DISABLED	= 0,
> +	V4L2_DETECT_MOTION_GLOBAL	= 1,
> +	V4L2_DETECT_MOTION_REGIONAL	= 2,
> +};
> +#define	V4L2_CID_DETECT_MOTION_THRESHOLD	(V4L2_CID_DETECT_CLASS_BASE 
+ 2)
> +

How many more controls do you expect in this class ? Maybe we should make it a 
bit generic, by creating an EVENT class that would contain controls pertaining 
to event generation ?

>  #endif

-- 
Regards,

Laurent Pinchart

Re: [RFCv2 PATCH 01/10] v4l2-controls: add motion detection controls.

[Hans Verkuil]

On 08/21/2013 11:36 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Monday 12 August 2013 12:58:24 Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Add support for two motion detection controls and a 'detect control class'.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  drivers/media/v4l2-core/v4l2-ctrls.c | 33 +++++++++++++++++++++++++++------
>> include/uapi/linux/v4l2-controls.h   | 14 ++++++++++++++
>>  2 files changed, 41 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
>> b/drivers/media/v4l2-core/v4l2-ctrls.c index fccd08b..89e7cfb 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -456,6 +456,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>  		"RGB full range (0-255)",
>>  		NULL,
>>  	};
>> +	static const char * const detect_motion_mode[] = {
>> +		"Disabled",
>> +		"Global",
>> +		"Regional",
>> +		NULL,
>> +	};
>>
>>
>>  	switch (id) {
>> @@ -545,6 +551,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>  	case V4L2_CID_DV_TX_RGB_RANGE:
>>  	case V4L2_CID_DV_RX_RGB_RANGE:
>>  		return dv_rgb_range;
>> +	case V4L2_CID_DETECT_MOTION_MODE:
>> +		return detect_motion_mode;
>>
>>  	default:
>>  		return NULL;
>> @@ -557,7 +565,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>  {
>>  	switch (id) {
>>  	/* USER controls */
>> -	/* Keep the order of the 'case's the same as in videodev2.h! */
>> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
> 
> Maybe we could replace all the individual occurences of that comment with a 
> single one at the beginning of the switch ?

It's a pretty long switch, so I think it is good that this comment is repeated
every so often.

> 
>>  	case V4L2_CID_USER_CLASS:		return "User Controls";
>>  	case V4L2_CID_BRIGHTNESS:		return "Brightness";
>>  	case V4L2_CID_CONTRAST:			return "Contrast";
>> @@ -601,7 +609,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>  	case V4L2_CID_COLORFX_CBCR:		return "Color Effects, CbCr";
>>
>>  	/* MPEG controls */
>> -	/* Keep the order of the 'case's the same as in videodev2.h! */
>> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>>  	case V4L2_CID_MPEG_CLASS:		return "MPEG Encoder Controls";
>>  	case V4L2_CID_MPEG_STREAM_TYPE:		return "Stream Type";
>>  	case V4L2_CID_MPEG_STREAM_PID_PMT:	return "Stream PMT Program ID";
>> @@ -701,7 +709,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>  	case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER:		return "Repeat Sequence
>> Header";
>>
>>  	/* CAMERA controls */
>> -	/* Keep the order of the 'case's the same as in videodev2.h! */
>> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>>  	case V4L2_CID_CAMERA_CLASS:		return "Camera Controls";
>>  	case V4L2_CID_EXPOSURE_AUTO:		return "Auto Exposure";
>>  	case V4L2_CID_EXPOSURE_ABSOLUTE:	return "Exposure Time, Absolute";
>> @@ -735,8 +743,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>>  	case V4L2_CID_AUTO_FOCUS_STATUS:	return "Auto Focus, Status";
>>  	case V4L2_CID_AUTO_FOCUS_RANGE:		return "Auto Focus, Range";
>>
>> -	/* FM Radio Modulator control */
>> -	/* Keep the order of the 'case's the same as in videodev2.h! */
>> +	/* FM Radio Modulator controls */
>> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>>  	case V4L2_CID_FM_TX_CLASS:		return "FM Radio Modulator Controls";
>>  	case V4L2_CID_RDS_TX_DEVIATION:		return "RDS Signal Deviation";
>>  	case V4L2_CID_RDS_TX_PI:		return "RDS Program ID";
>> @@ -759,6 +767,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>  	case V4L2_CID_TUNE_ANTENNA_CAPACITOR:	return "Tune Antenna Capacitor";
>>
>>  	/* Flash controls */
>> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>>  	case V4L2_CID_FLASH_CLASS:		return "Flash Controls";
>>  	case V4L2_CID_FLASH_LED_MODE:		return "LED Mode";
>>  	case V4L2_CID_FLASH_STROBE_SOURCE:	return "Strobe Source";
>> @@ -774,7 +783,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>  	case V4L2_CID_FLASH_READY:		return "Ready to Strobe";
>>
>>  	/* JPEG encoder controls */
>> -	/* Keep the order of the 'case's the same as in videodev2.h! */
>> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>>  	case V4L2_CID_JPEG_CLASS:		return "JPEG Compression Controls";
>>  	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:	return "Chroma Subsampling";
>>  	case V4L2_CID_JPEG_RESTART_INTERVAL:	return "Restart Interval";
>> @@ -782,18 +791,21 @@ const char *v4l2_ctrl_get_name(u32 id)
>>  	case V4L2_CID_JPEG_ACTIVE_MARKER:	return "Active Markers";
>>
>>  	/* Image source controls */
>> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>>  	case V4L2_CID_IMAGE_SOURCE_CLASS:	return "Image Source Controls";
>>  	case V4L2_CID_VBLANK:			return "Vertical Blanking";
>>  	case V4L2_CID_HBLANK:			return "Horizontal Blanking";
>>  	case V4L2_CID_ANALOGUE_GAIN:		return "Analogue Gain";
>>
>>  	/* Image processing controls */
>> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>>  	case V4L2_CID_IMAGE_PROC_CLASS:		return "Image Processing Controls";
>>  	case V4L2_CID_LINK_FREQ:		return "Link Frequency";
>>  	case V4L2_CID_PIXEL_RATE:		return "Pixel Rate";
>>  	case V4L2_CID_TEST_PATTERN:		return "Test Pattern";
>>
>>  	/* DV controls */
>> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>>  	case V4L2_CID_DV_CLASS:			return "Digital Video Controls";
>>  	case V4L2_CID_DV_TX_HOTPLUG:		return "Hotplug Present";
>>  	case V4L2_CID_DV_TX_RXSENSE:		return "RxSense Present";
>> @@ -806,6 +818,12 @@ const char *v4l2_ctrl_get_name(u32 id)
>>  	case V4L2_CID_FM_RX_CLASS:		return "FM Radio Receiver Controls";
>>  	case V4L2_CID_TUNE_DEEMPHASIS:		return "De-Emphasis";
>>  	case V4L2_CID_RDS_RECEPTION:		return "RDS Reception";
>> +
>> +	/* FM Radio Receiver controls */
>> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>> +	case V4L2_CID_DETECT_CLASS:		return "Detection Controls";
>> +	case V4L2_CID_DETECT_MOTION_MODE:	return "Motion Detection Mode";
>> +	case V4L2_CID_DETECT_MOTION_THRESHOLD:	return "Motion Detection
>> Threshold"; default:
>>  		return NULL;
>>  	}
>> @@ -914,6 +932,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
>> v4l2_ctrl_type *type, case V4L2_CID_DV_RX_RGB_RANGE:
>>  	case V4L2_CID_TEST_PATTERN:
>>  	case V4L2_CID_TUNE_DEEMPHASIS:
>> +	case V4L2_CID_DETECT_MOTION_MODE:
>>  		*type = V4L2_CTRL_TYPE_MENU;
>>  		break;
>>  	case V4L2_CID_LINK_FREQ:
>> @@ -937,6 +956,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
>> v4l2_ctrl_type *type, case V4L2_CID_IMAGE_PROC_CLASS:
>>  	case V4L2_CID_DV_CLASS:
>>  	case V4L2_CID_FM_RX_CLASS:
>> +	case V4L2_CID_DETECT_CLASS:
>>  		*type = V4L2_CTRL_TYPE_CTRL_CLASS;
>>  		/* You can neither read not write these */
>>  		*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
>> @@ -1009,6 +1029,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
>> v4l2_ctrl_type *type, case V4L2_CID_PILOT_TONE_FREQUENCY:
>>  	case V4L2_CID_TUNE_POWER_LEVEL:
>>  	case V4L2_CID_TUNE_ANTENNA_CAPACITOR:
>> +	case V4L2_CID_DETECT_MOTION_THRESHOLD:
>>  		*flags |= V4L2_CTRL_FLAG_SLIDER;
>>  		break;
>>  	case V4L2_CID_PAN_RELATIVE:
>> diff --git a/include/uapi/linux/v4l2-controls.h
>> b/include/uapi/linux/v4l2-controls.h index e90a88a..d88eebd 100644
>> --- a/include/uapi/linux/v4l2-controls.h
>> +++ b/include/uapi/linux/v4l2-controls.h
>> @@ -60,6 +60,7 @@
>>  #define V4L2_CTRL_CLASS_IMAGE_PROC	0x009f0000	/* Image processing 
> controls
>> */ #define V4L2_CTRL_CLASS_DV		0x00a00000	/* Digital Video controls */
>> #define V4L2_CTRL_CLASS_FM_RX		0x00a10000	/* FM Receiver controls */
>> +#define V4L2_CTRL_CLASS_DETECT		0x00a20000	/* Detection controls */
>>
>>  /* User-class control IDs */
>>
>> @@ -853,4 +854,17 @@ enum v4l2_deemphasis {
>>
>>  #define V4L2_CID_RDS_RECEPTION			(V4L2_CID_FM_RX_CLASS_BASE + 2)
>>
>> +
>> +/*  Detection-class control IDs defined by V4L2 */
>> +#define V4L2_CID_DETECT_CLASS_BASE		(V4L2_CTRL_CLASS_DETECT | 0x900)
>> +#define V4L2_CID_DETECT_CLASS			(V4L2_CTRL_CLASS_DETECT | 1)
>> +
>> +#define	V4L2_CID_DETECT_MOTION_MODE		(V4L2_CID_DETECT_CLASS_BASE + 1)
>> +enum v4l2_detect_motion_mode {
>> +	V4L2_DETECT_MOTION_DISABLED	= 0,
>> +	V4L2_DETECT_MOTION_GLOBAL	= 1,
>> +	V4L2_DETECT_MOTION_REGIONAL	= 2,
>> +};
>> +#define	V4L2_CID_DETECT_MOTION_THRESHOLD	(V4L2_CID_DETECT_CLASS_BASE 
> + 2)
>> +
> 
> How many more controls do you expect in this class ? Maybe we should make it a 
> bit generic, by creating an EVENT class that would contain controls pertaining 
> to event generation ?

There could be quite a few controls here for all sorts of <something> detections
(face, smile, object, etc.). An event class seems awfully vague to me. If I am
looking for motion detection controls I wouldn't expect to find them in an EVENT
class.

Regards,

	Hans

[RFCv2 PATCH 02/10] v4l2: add matrix support.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This patch adds core support for matrices: querying, getting and setting.

Two initial matrix types are defined for motion detection (defining regions
and thresholds).

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/v4l2-dev.c   |  3 ++
 drivers/media/v4l2-core/v4l2-ioctl.c | 23 ++++++++++++-
 include/media/v4l2-ioctl.h           |  8 +++++
 include/uapi/linux/videodev2.h       | 64 ++++++++++++++++++++++++++++++++++++
 4 files changed, 97 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index c8859d6..5e58df6 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -598,6 +598,9 @@ static void determine_valid_ioctls(struct video_device *vdev)
 	SET_VALID_IOCTL(ops, VIDIOC_UNSUBSCRIBE_EVENT, vidioc_unsubscribe_event);
 	if (ops->vidioc_enum_freq_bands || ops->vidioc_g_tuner || ops->vidioc_g_modulator)
 		set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
+	SET_VALID_IOCTL(ops, VIDIOC_QUERY_MATRIX, vidioc_query_matrix);
+	SET_VALID_IOCTL(ops, VIDIOC_G_MATRIX, vidioc_g_matrix);
+	SET_VALID_IOCTL(ops, VIDIOC_S_MATRIX, vidioc_s_matrix);
 
 	if (is_vid) {
 		/* video specific ioctls */
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 68e6b5e..47debfc 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -549,7 +549,7 @@ static void v4l_print_cropcap(const void *arg, bool write_only)
 	const struct v4l2_cropcap *p = arg;
 
 	pr_cont("type=%s, bounds wxh=%dx%d, x,y=%d,%d, "
-		"defrect wxh=%dx%d, x,y=%d,%d\n, "
+		"defrect wxh=%dx%d, x,y=%d,%d, "
 		"pixelaspect %d/%d\n",
 		prt_names(p->type, v4l2_type_names),
 		p->bounds.width, p->bounds.height,
@@ -831,6 +831,24 @@ static void v4l_print_freq_band(const void *arg, bool write_only)
 			p->rangehigh, p->modulation);
 }
 
+static void v4l_print_query_matrix(const void *arg, bool write_only)
+{
+	const struct v4l2_query_matrix *p = arg;
+
+	pr_cont("type=0x%x, columns=%u, rows=%u, elem_min=%lld, elem_max=%lld, elem_size=%u\n",
+			p->type, p->columns, p->rows,
+			p->elem_min.val, p->elem_max.val, p->elem_size);
+}
+
+static void v4l_print_matrix(const void *arg, bool write_only)
+{
+	const struct v4l2_matrix *p = arg;
+
+	pr_cont("type=0x%x, wxh=%dx%d, x,y=%d,%d, matrix=%p\n",
+			p->type, p->rect.width, p->rect.height,
+			p->rect.top, p->rect.left, p->matrix);
+}
+
 static void v4l_print_u32(const void *arg, bool write_only)
 {
 	pr_cont("value=%u\n", *(const u32 *)arg);
@@ -2055,6 +2073,9 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
 	IOCTL_INFO_STD(VIDIOC_DV_TIMINGS_CAP, vidioc_dv_timings_cap, v4l_print_dv_timings_cap, INFO_FL_CLEAR(v4l2_dv_timings_cap, type)),
 	IOCTL_INFO_FNC(VIDIOC_ENUM_FREQ_BANDS, v4l_enum_freq_bands, v4l_print_freq_band, 0),
 	IOCTL_INFO_FNC(VIDIOC_DBG_G_CHIP_INFO, v4l_dbg_g_chip_info, v4l_print_dbg_chip_info, INFO_FL_CLEAR(v4l2_dbg_chip_info, match)),
+	IOCTL_INFO_STD(VIDIOC_QUERY_MATRIX, vidioc_query_matrix, v4l_print_query_matrix, INFO_FL_CLEAR(v4l2_query_matrix, ref)),
+	IOCTL_INFO_STD(VIDIOC_G_MATRIX, vidioc_g_matrix, v4l_print_matrix, INFO_FL_CLEAR(v4l2_matrix, matrix)),
+	IOCTL_INFO_STD(VIDIOC_S_MATRIX, vidioc_s_matrix, v4l_print_matrix, INFO_FL_PRIO | INFO_FL_CLEAR(v4l2_matrix, matrix)),
 };
 #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)
 
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index e0b74a4..7e4538e 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -271,6 +271,14 @@ struct v4l2_ioctl_ops {
 	int (*vidioc_unsubscribe_event)(struct v4l2_fh *fh,
 					const struct v4l2_event_subscription *sub);
 
+	/* Matrix ioctls */
+	int (*vidioc_query_matrix) (struct file *file, void *fh,
+				    struct v4l2_query_matrix *qmatrix);
+	int (*vidioc_g_matrix) (struct file *file, void *fh,
+				    struct v4l2_matrix *matrix);
+	int (*vidioc_s_matrix) (struct file *file, void *fh,
+				    struct v4l2_matrix *matrix);
+
 	/* For other private ioctls */
 	long (*vidioc_default)	       (struct file *file, void *fh,
 					bool valid_prio, unsigned int cmd, void *arg);
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 95ef455..605d295 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1838,6 +1838,64 @@ struct v4l2_create_buffers {
 	__u32			reserved[8];
 };
 
+/* Define to which motion detection region each element belongs.
+ * Each element is a __u8. */
+#define V4L2_MATRIX_T_MD_REGION     (1)
+/* Define the motion detection threshold for each element.
+ * Each element is a __u16. */
+#define V4L2_MATRIX_T_MD_THRESHOLD  (2)
+
+/**
+ * struct v4l2_query_matrix - VIDIOC_QUERY_MATRIX argument
+ * @type:	matrix type
+ * @ref:	reference to some object (if any) owning the matrix
+ * @columns:	number of columns in the matrix
+ * @rows:	number of rows in the matrix
+ * @elem_min:	minimum matrix element value
+ * @elem_max:	maximum matrix element value
+ * @elem_size:	size in bytes each matrix element
+ * @reserved:	future extensions, applications and drivers must zero this.
+ */
+struct v4l2_query_matrix {
+	__u32 type;
+	union {
+		__u32 raw[4];
+	} ref;
+	__u32 columns;
+	__u32 rows;
+	union {
+		__s64 val;
+		__u64 uval;
+		__u32 raw[4];
+	} elem_min;
+	union {
+		__s64 val;
+		__u64 uval;
+		__u32 raw[4];
+	} elem_max;
+	__u32 elem_size;
+	__u32 reserved[12];
+} __attribute__ ((packed));
+
+/**
+ * struct v4l2_matrix - VIDIOC_G/S_MATRIX argument
+ * @type:	matrix type
+ * @ref:	reference to some object (if any) owning the matrix
+ * @rect:	which part of the matrix to get/set
+ * @matrix:	pointer to the matrix of size (in bytes):
+ *		elem_size * rect.width * rect.height
+ * @reserved:	future extensions, applications and drivers must zero this.
+ */
+struct v4l2_matrix {
+	__u32 type;
+	union {
+		__u32 raw[4];
+	} ref;
+	struct v4l2_rect rect;
+	void __user *matrix;
+	__u32 reserved[12];
+} __attribute__ ((packed));
+
 /*
  *	I O C T L   C O D E S   F O R   V I D E O   D E V I C E S
  *
@@ -1946,6 +2004,12 @@ struct v4l2_create_buffers {
    Never use these in applications! */
 #define VIDIOC_DBG_G_CHIP_INFO  _IOWR('V', 102, struct v4l2_dbg_chip_info)
 
+/* Experimental, these three ioctls may change over the next couple of kernel
+   versions. */
+#define VIDIOC_QUERY_MATRIX	_IOWR('V', 103, struct v4l2_query_matrix)
+#define VIDIOC_G_MATRIX		_IOWR('V', 104, struct v4l2_matrix)
+#define VIDIOC_S_MATRIX		_IOWR('V', 105, struct v4l2_matrix)
+
 /* Reminder: when adding new ioctls please add support for them to
    drivers/media/video/v4l2-compat-ioctl32.c as well! */
 
-- 
1.8.3.2

Re: [RFCv2 PATCH 02/10] v4l2: add matrix support.

[Sakari Ailus]

Hi Hans,

Thanks for the set!

On Mon, Aug 12, 2013 at 12:58:25PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This patch adds core support for matrices: querying, getting and setting.
> 
> Two initial matrix types are defined for motion detection (defining regions
> and thresholds).

I requested in the past that no new IOCTLs would be added for an essential
extension of V4L2 control-like functionality. I understand developing a more
generic framework does not answer to the problems at hand right now, so I
think it's certainly fine to continue with matrix IOCTLs, too. But we still
should think a little about extensibility a little bit.

How about using the same ID space as the controls do for matrices, for
instance, so we won't get one more? The selections and controls have no ID
collisions at the moment.

> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/v4l2-core/v4l2-dev.c   |  3 ++
>  drivers/media/v4l2-core/v4l2-ioctl.c | 23 ++++++++++++-
>  include/media/v4l2-ioctl.h           |  8 +++++
>  include/uapi/linux/videodev2.h       | 64 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 97 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index c8859d6..5e58df6 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -598,6 +598,9 @@ static void determine_valid_ioctls(struct video_device *vdev)
>  	SET_VALID_IOCTL(ops, VIDIOC_UNSUBSCRIBE_EVENT, vidioc_unsubscribe_event);
>  	if (ops->vidioc_enum_freq_bands || ops->vidioc_g_tuner || ops->vidioc_g_modulator)
>  		set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
> +	SET_VALID_IOCTL(ops, VIDIOC_QUERY_MATRIX, vidioc_query_matrix);
> +	SET_VALID_IOCTL(ops, VIDIOC_G_MATRIX, vidioc_g_matrix);
> +	SET_VALID_IOCTL(ops, VIDIOC_S_MATRIX, vidioc_s_matrix);
>  
>  	if (is_vid) {
>  		/* video specific ioctls */
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 68e6b5e..47debfc 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -549,7 +549,7 @@ static void v4l_print_cropcap(const void *arg, bool write_only)
>  	const struct v4l2_cropcap *p = arg;
>  
>  	pr_cont("type=%s, bounds wxh=%dx%d, x,y=%d,%d, "
> -		"defrect wxh=%dx%d, x,y=%d,%d\n, "
> +		"defrect wxh=%dx%d, x,y=%d,%d, "
>  		"pixelaspect %d/%d\n",
>  		prt_names(p->type, v4l2_type_names),
>  		p->bounds.width, p->bounds.height,
> @@ -831,6 +831,24 @@ static void v4l_print_freq_band(const void *arg, bool write_only)
>  			p->rangehigh, p->modulation);
>  }
>  
> +static void v4l_print_query_matrix(const void *arg, bool write_only)
> +{
> +	const struct v4l2_query_matrix *p = arg;
> +
> +	pr_cont("type=0x%x, columns=%u, rows=%u, elem_min=%lld, elem_max=%lld, elem_size=%u\n",
> +			p->type, p->columns, p->rows,
> +			p->elem_min.val, p->elem_max.val, p->elem_size);
> +}
> +
> +static void v4l_print_matrix(const void *arg, bool write_only)
> +{
> +	const struct v4l2_matrix *p = arg;
> +
> +	pr_cont("type=0x%x, wxh=%dx%d, x,y=%d,%d, matrix=%p\n",
> +			p->type, p->rect.width, p->rect.height,
> +			p->rect.top, p->rect.left, p->matrix);
> +}
> +
>  static void v4l_print_u32(const void *arg, bool write_only)
>  {
>  	pr_cont("value=%u\n", *(const u32 *)arg);
> @@ -2055,6 +2073,9 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
>  	IOCTL_INFO_STD(VIDIOC_DV_TIMINGS_CAP, vidioc_dv_timings_cap, v4l_print_dv_timings_cap, INFO_FL_CLEAR(v4l2_dv_timings_cap, type)),
>  	IOCTL_INFO_FNC(VIDIOC_ENUM_FREQ_BANDS, v4l_enum_freq_bands, v4l_print_freq_band, 0),
>  	IOCTL_INFO_FNC(VIDIOC_DBG_G_CHIP_INFO, v4l_dbg_g_chip_info, v4l_print_dbg_chip_info, INFO_FL_CLEAR(v4l2_dbg_chip_info, match)),
> +	IOCTL_INFO_STD(VIDIOC_QUERY_MATRIX, vidioc_query_matrix, v4l_print_query_matrix, INFO_FL_CLEAR(v4l2_query_matrix, ref)),
> +	IOCTL_INFO_STD(VIDIOC_G_MATRIX, vidioc_g_matrix, v4l_print_matrix, INFO_FL_CLEAR(v4l2_matrix, matrix)),
> +	IOCTL_INFO_STD(VIDIOC_S_MATRIX, vidioc_s_matrix, v4l_print_matrix, INFO_FL_PRIO | INFO_FL_CLEAR(v4l2_matrix, matrix)),
>  };
>  #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)
>  
> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
> index e0b74a4..7e4538e 100644
> --- a/include/media/v4l2-ioctl.h
> +++ b/include/media/v4l2-ioctl.h
> @@ -271,6 +271,14 @@ struct v4l2_ioctl_ops {
>  	int (*vidioc_unsubscribe_event)(struct v4l2_fh *fh,
>  					const struct v4l2_event_subscription *sub);
>  
> +	/* Matrix ioctls */
> +	int (*vidioc_query_matrix) (struct file *file, void *fh,
> +				    struct v4l2_query_matrix *qmatrix);
> +	int (*vidioc_g_matrix) (struct file *file, void *fh,
> +				    struct v4l2_matrix *matrix);
> +	int (*vidioc_s_matrix) (struct file *file, void *fh,
> +				    struct v4l2_matrix *matrix);
> +
>  	/* For other private ioctls */
>  	long (*vidioc_default)	       (struct file *file, void *fh,
>  					bool valid_prio, unsigned int cmd, void *arg);
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 95ef455..605d295 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1838,6 +1838,64 @@ struct v4l2_create_buffers {
>  	__u32			reserved[8];
>  };
>  
> +/* Define to which motion detection region each element belongs.
> + * Each element is a __u8. */
> +#define V4L2_MATRIX_T_MD_REGION     (1)
> +/* Define the motion detection threshold for each element.
> + * Each element is a __u16. */
> +#define V4L2_MATRIX_T_MD_THRESHOLD  (2)
> +
> +/**
> + * struct v4l2_query_matrix - VIDIOC_QUERY_MATRIX argument
> + * @type:	matrix type
> + * @ref:	reference to some object (if any) owning the matrix

Is this for future extensibility only? Four __u32s don't say much to me. If
so, how about combining this with the reserved field below?

> + * @columns:	number of columns in the matrix
> + * @rows:	number of rows in the matrix

Two dimensions only? How about one or three? I could imagine use for one, at
the very least.

> + * @elem_min:	minimum matrix element value
> + * @elem_max:	maximum matrix element value
> + * @elem_size:	size in bytes each matrix element
> + * @reserved:	future extensions, applications and drivers must zero this.
> + */
> +struct v4l2_query_matrix {
> +	__u32 type;
> +	union {
> +		__u32 raw[4];
> +	} ref;
> +	__u32 columns;
> +	__u32 rows;
> +	union {
> +		__s64 val;
> +		__u64 uval;
> +		__u32 raw[4];
> +	} elem_min;
> +	union {
> +		__s64 val;
> +		__u64 uval;
> +		__u32 raw[4];
> +	} elem_max;

How about step; do you think it'd make sense to specify that? I have to
admit the step in controls hasn't been extemely useful to me: much of the
time the value of the control should have just been divided by the step,
with the exception of controls that have a standardised unit, but even then
step won't do good on them since there's typically no 1:1 mapping between
possible values and the actual values which leads the driver writer choosing
step of one.

> +	__u32 elem_size;
> +	__u32 reserved[12];
> +} __attribute__ ((packed));
> +
> +/**
> + * struct v4l2_matrix - VIDIOC_G/S_MATRIX argument
> + * @type:	matrix type
> + * @ref:	reference to some object (if any) owning the matrix
> + * @rect:	which part of the matrix to get/set

In some cases it might be possible to choose the size of the matrix. If this
isn't supported now, do you have ideas how to add it? Perhaps using rect
woulnd't be possible. A new IOCTL could be one possibility as well; that'd
make it quite clear and drivers not supporting it wouldn't implement it. I
think it might quite well make it together with S_MATRIX, though, e.g. a
flags field with a flag telling that the dimension fields are valid.

> + * @matrix:	pointer to the matrix of size (in bytes):
> + *		elem_size * rect.width * rect.height
> + * @reserved:	future extensions, applications and drivers must zero this.
> + */
> +struct v4l2_matrix {
> +	__u32 type;
> +	union {
> +		__u32 raw[4];
> +	} ref;
> +	struct v4l2_rect rect;
> +	void __user *matrix;
> +	__u32 reserved[12];
> +} __attribute__ ((packed));
> +
>  /*
>   *	I O C T L   C O D E S   F O R   V I D E O   D E V I C E S
>   *
> @@ -1946,6 +2004,12 @@ struct v4l2_create_buffers {
>     Never use these in applications! */
>  #define VIDIOC_DBG_G_CHIP_INFO  _IOWR('V', 102, struct v4l2_dbg_chip_info)
>  
> +/* Experimental, these three ioctls may change over the next couple of kernel
> +   versions. */
> +#define VIDIOC_QUERY_MATRIX	_IOWR('V', 103, struct v4l2_query_matrix)
> +#define VIDIOC_G_MATRIX		_IOWR('V', 104, struct v4l2_matrix)
> +#define VIDIOC_S_MATRIX		_IOWR('V', 105, struct v4l2_matrix)
> +
>  /* Reminder: when adding new ioctls please add support for them to
>     drivers/media/video/v4l2-compat-ioctl32.c as well! */
>  

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [RFCv2 PATCH 02/10] v4l2: add matrix support.

[Hans Verkuil]

On 08/14/2013 04:33 PM, Sakari Ailus wrote:
> Hi Hans,
> 
> Thanks for the set!
> 
> On Mon, Aug 12, 2013 at 12:58:25PM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> This patch adds core support for matrices: querying, getting and setting.
>>
>> Two initial matrix types are defined for motion detection (defining regions
>> and thresholds).
> 
> I requested in the past that no new IOCTLs would be added for an essential
> extension of V4L2 control-like functionality. I understand developing a more
> generic framework does not answer to the problems at hand right now, so I
> think it's certainly fine to continue with matrix IOCTLs, too. But we still
> should think a little about extensibility a little bit.
> 
> How about using the same ID space as the controls do for matrices, for
> instance, so we won't get one more? The selections and controls have no ID
> collisions at the moment.

Fair enough. That certainly doesn't hurt.

> 
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  drivers/media/v4l2-core/v4l2-dev.c   |  3 ++
>>  drivers/media/v4l2-core/v4l2-ioctl.c | 23 ++++++++++++-
>>  include/media/v4l2-ioctl.h           |  8 +++++
>>  include/uapi/linux/videodev2.h       | 64 ++++++++++++++++++++++++++++++++++++
>>  4 files changed, 97 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
>> index c8859d6..5e58df6 100644
>> --- a/drivers/media/v4l2-core/v4l2-dev.c
>> +++ b/drivers/media/v4l2-core/v4l2-dev.c
>> @@ -598,6 +598,9 @@ static void determine_valid_ioctls(struct video_device *vdev)
>>  	SET_VALID_IOCTL(ops, VIDIOC_UNSUBSCRIBE_EVENT, vidioc_unsubscribe_event);
>>  	if (ops->vidioc_enum_freq_bands || ops->vidioc_g_tuner || ops->vidioc_g_modulator)
>>  		set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
>> +	SET_VALID_IOCTL(ops, VIDIOC_QUERY_MATRIX, vidioc_query_matrix);
>> +	SET_VALID_IOCTL(ops, VIDIOC_G_MATRIX, vidioc_g_matrix);
>> +	SET_VALID_IOCTL(ops, VIDIOC_S_MATRIX, vidioc_s_matrix);
>>  
>>  	if (is_vid) {
>>  		/* video specific ioctls */
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 68e6b5e..47debfc 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -549,7 +549,7 @@ static void v4l_print_cropcap(const void *arg, bool write_only)
>>  	const struct v4l2_cropcap *p = arg;
>>  
>>  	pr_cont("type=%s, bounds wxh=%dx%d, x,y=%d,%d, "
>> -		"defrect wxh=%dx%d, x,y=%d,%d\n, "
>> +		"defrect wxh=%dx%d, x,y=%d,%d, "
>>  		"pixelaspect %d/%d\n",
>>  		prt_names(p->type, v4l2_type_names),
>>  		p->bounds.width, p->bounds.height,
>> @@ -831,6 +831,24 @@ static void v4l_print_freq_band(const void *arg, bool write_only)
>>  			p->rangehigh, p->modulation);
>>  }
>>  
>> +static void v4l_print_query_matrix(const void *arg, bool write_only)
>> +{
>> +	const struct v4l2_query_matrix *p = arg;
>> +
>> +	pr_cont("type=0x%x, columns=%u, rows=%u, elem_min=%lld, elem_max=%lld, elem_size=%u\n",
>> +			p->type, p->columns, p->rows,
>> +			p->elem_min.val, p->elem_max.val, p->elem_size);
>> +}
>> +
>> +static void v4l_print_matrix(const void *arg, bool write_only)
>> +{
>> +	const struct v4l2_matrix *p = arg;
>> +
>> +	pr_cont("type=0x%x, wxh=%dx%d, x,y=%d,%d, matrix=%p\n",
>> +			p->type, p->rect.width, p->rect.height,
>> +			p->rect.top, p->rect.left, p->matrix);
>> +}
>> +
>>  static void v4l_print_u32(const void *arg, bool write_only)
>>  {
>>  	pr_cont("value=%u\n", *(const u32 *)arg);
>> @@ -2055,6 +2073,9 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
>>  	IOCTL_INFO_STD(VIDIOC_DV_TIMINGS_CAP, vidioc_dv_timings_cap, v4l_print_dv_timings_cap, INFO_FL_CLEAR(v4l2_dv_timings_cap, type)),
>>  	IOCTL_INFO_FNC(VIDIOC_ENUM_FREQ_BANDS, v4l_enum_freq_bands, v4l_print_freq_band, 0),
>>  	IOCTL_INFO_FNC(VIDIOC_DBG_G_CHIP_INFO, v4l_dbg_g_chip_info, v4l_print_dbg_chip_info, INFO_FL_CLEAR(v4l2_dbg_chip_info, match)),
>> +	IOCTL_INFO_STD(VIDIOC_QUERY_MATRIX, vidioc_query_matrix, v4l_print_query_matrix, INFO_FL_CLEAR(v4l2_query_matrix, ref)),
>> +	IOCTL_INFO_STD(VIDIOC_G_MATRIX, vidioc_g_matrix, v4l_print_matrix, INFO_FL_CLEAR(v4l2_matrix, matrix)),
>> +	IOCTL_INFO_STD(VIDIOC_S_MATRIX, vidioc_s_matrix, v4l_print_matrix, INFO_FL_PRIO | INFO_FL_CLEAR(v4l2_matrix, matrix)),
>>  };
>>  #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)
>>  
>> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
>> index e0b74a4..7e4538e 100644
>> --- a/include/media/v4l2-ioctl.h
>> +++ b/include/media/v4l2-ioctl.h
>> @@ -271,6 +271,14 @@ struct v4l2_ioctl_ops {
>>  	int (*vidioc_unsubscribe_event)(struct v4l2_fh *fh,
>>  					const struct v4l2_event_subscription *sub);
>>  
>> +	/* Matrix ioctls */
>> +	int (*vidioc_query_matrix) (struct file *file, void *fh,
>> +				    struct v4l2_query_matrix *qmatrix);
>> +	int (*vidioc_g_matrix) (struct file *file, void *fh,
>> +				    struct v4l2_matrix *matrix);
>> +	int (*vidioc_s_matrix) (struct file *file, void *fh,
>> +				    struct v4l2_matrix *matrix);
>> +
>>  	/* For other private ioctls */
>>  	long (*vidioc_default)	       (struct file *file, void *fh,
>>  					bool valid_prio, unsigned int cmd, void *arg);
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index 95ef455..605d295 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -1838,6 +1838,64 @@ struct v4l2_create_buffers {
>>  	__u32			reserved[8];
>>  };
>>  
>> +/* Define to which motion detection region each element belongs.
>> + * Each element is a __u8. */
>> +#define V4L2_MATRIX_T_MD_REGION     (1)
>> +/* Define the motion detection threshold for each element.
>> + * Each element is a __u16. */
>> +#define V4L2_MATRIX_T_MD_THRESHOLD  (2)
>> +
>> +/**
>> + * struct v4l2_query_matrix - VIDIOC_QUERY_MATRIX argument
>> + * @type:	matrix type
>> + * @ref:	reference to some object (if any) owning the matrix
> 
> Is this for future extensibility only? Four __u32s don't say much to me. If
> so, how about combining this with the reserved field below?

Yes, it's for future extensibility. It shows how a feature like that could be
implemented, but I think you are right and that it should be dropped and
reserved should be increased by 4 elements.

>> + * @columns:	number of columns in the matrix
>> + * @rows:	number of rows in the matrix
> 
> Two dimensions only? How about one or three? I could imagine use for one, at
> the very least.

For one you just set rows to 1. A vector is after all a matrix of one row.
Should we need a third dimension, then there are enough reserved fields to make
that possible. I can't think of a single use-case that would require a three
dimensional matrix.
 
>> + * @elem_min:	minimum matrix element value
>> + * @elem_max:	maximum matrix element value
>> + * @elem_size:	size in bytes each matrix element
>> + * @reserved:	future extensions, applications and drivers must zero this.
>> + */
>> +struct v4l2_query_matrix {
>> +	__u32 type;
>> +	union {
>> +		__u32 raw[4];
>> +	} ref;
>> +	__u32 columns;
>> +	__u32 rows;
>> +	union {
>> +		__s64 val;
>> +		__u64 uval;
>> +		__u32 raw[4];
>> +	} elem_min;
>> +	union {
>> +		__s64 val;
>> +		__u64 uval;
>> +		__u32 raw[4];
>> +	} elem_max;
> 
> How about step; do you think it'd make sense to specify that? I have to
> admit the step in controls hasn't been extemely useful to me: much of the
> time the value of the control should have just been divided by the step,
> with the exception of controls that have a standardised unit, but even then
> step won't do good on them since there's typically no 1:1 mapping between
> possible values and the actual values which leads the driver writer choosing
> step of one.

You just explained why I decided against adding a step :-)

I also can't really see a use-case for a step in a matrix.

>> +	__u32 elem_size;
>> +	__u32 reserved[12];
>> +} __attribute__ ((packed));
>> +
>> +/**
>> + * struct v4l2_matrix - VIDIOC_G/S_MATRIX argument
>> + * @type:	matrix type
>> + * @ref:	reference to some object (if any) owning the matrix
>> + * @rect:	which part of the matrix to get/set
> 
> In some cases it might be possible to choose the size of the matrix. If this
> isn't supported now, do you have ideas how to add it? Perhaps using rect
> woulnd't be possible. A new IOCTL could be one possibility as well; that'd
> make it quite clear and drivers not supporting it wouldn't implement it. I
> think it might quite well make it together with S_MATRIX, though, e.g. a
> flags field with a flag telling that the dimension fields are valid.

Would it be an idea to add a flags field to both v4l2_matrix and v4l2_query_matrix?
We don't have flags yet, but that makes it easy to add. For a feature such as you
describe it would be easy enough to implement that by setting an e.g.
V4L2_MATRIX_FL_NEW_SIZE flag. In query_matrix you would than have a
V4L2_QMATRIX_FL_HAS_NEW_SIZE (or perhaps in query_matrix it should be called
'capabilities' instead).

I can also just leave it out and use one of the reserved fields when such a feature
is needed.

>> + * @matrix:	pointer to the matrix of size (in bytes):
>> + *		elem_size * rect.width * rect.height
>> + * @reserved:	future extensions, applications and drivers must zero this.
>> + */
>> +struct v4l2_matrix {
>> +	__u32 type;
>> +	union {
>> +		__u32 raw[4];
>> +	} ref;
>> +	struct v4l2_rect rect;
>> +	void __user *matrix;
>> +	__u32 reserved[12];
>> +} __attribute__ ((packed));
>> +
>>  /*
>>   *	I O C T L   C O D E S   F O R   V I D E O   D E V I C E S
>>   *
>> @@ -1946,6 +2004,12 @@ struct v4l2_create_buffers {
>>     Never use these in applications! */
>>  #define VIDIOC_DBG_G_CHIP_INFO  _IOWR('V', 102, struct v4l2_dbg_chip_info)
>>  
>> +/* Experimental, these three ioctls may change over the next couple of kernel
>> +   versions. */
>> +#define VIDIOC_QUERY_MATRIX	_IOWR('V', 103, struct v4l2_query_matrix)
>> +#define VIDIOC_G_MATRIX		_IOWR('V', 104, struct v4l2_matrix)
>> +#define VIDIOC_S_MATRIX		_IOWR('V', 105, struct v4l2_matrix)
>> +
>>  /* Reminder: when adding new ioctls please add support for them to
>>     drivers/media/video/v4l2-compat-ioctl32.c as well! */
>>  
> 

Thanks for the review!

I'll prepare a new version this weekend dropping the ref fields and integrating the ID
space into that of the controls.

Regards,

	Hans

Re: [RFCv2 PATCH 02/10] v4l2: add matrix support.

[Sakari Ailus]

Hi Hans,

On Thu, Aug 15, 2013 at 08:35:01AM +0200, Hans Verkuil wrote:
> On 08/14/2013 04:33 PM, Sakari Ailus wrote:
...
> >> + * @columns:	number of columns in the matrix
> >> + * @rows:	number of rows in the matrix
> > 
> > Two dimensions only? How about one or three? I could imagine use for one, at
> > the very least.
> 
> For one you just set rows to 1. A vector is after all a matrix of one row.
> Should we need a third dimension, then there are enough reserved fields to make
> that possible. I can't think of a single use-case that would require a three
> dimensional matrix.

Fine for me.

> >> + * @elem_min:	minimum matrix element value
> >> + * @elem_max:	maximum matrix element value
> >> + * @elem_size:	size in bytes each matrix element
> >> + * @reserved:	future extensions, applications and drivers must zero this.
> >> + */
> >> +struct v4l2_query_matrix {
> >> +	__u32 type;
> >> +	union {
> >> +		__u32 raw[4];
> >> +	} ref;
> >> +	__u32 columns;
> >> +	__u32 rows;
> >> +	union {
> >> +		__s64 val;
> >> +		__u64 uval;
> >> +		__u32 raw[4];
> >> +	} elem_min;
> >> +	union {
> >> +		__s64 val;
> >> +		__u64 uval;
> >> +		__u32 raw[4];
> >> +	} elem_max;
> > 
> > How about step; do you think it'd make sense to specify that? I have to
> > admit the step in controls hasn't been extemely useful to me: much of the
> > time the value of the control should have just been divided by the step,
> > with the exception of controls that have a standardised unit, but even then
> > step won't do good on them since there's typically no 1:1 mapping between
> > possible values and the actual values which leads the driver writer choosing
> > step of one.
> 
> You just explained why I decided against adding a step :-)
> 
> I also can't really see a use-case for a step in a matrix.

I agree --- I brought it up mostly since controls already do have step.

> >> +	__u32 elem_size;
> >> +	__u32 reserved[12];
> >> +} __attribute__ ((packed));
> >> +
> >> +/**
> >> + * struct v4l2_matrix - VIDIOC_G/S_MATRIX argument
> >> + * @type:	matrix type
> >> + * @ref:	reference to some object (if any) owning the matrix
> >> + * @rect:	which part of the matrix to get/set
> > 
> > In some cases it might be possible to choose the size of the matrix. If this
> > isn't supported now, do you have ideas how to add it? Perhaps using rect
> > woulnd't be possible. A new IOCTL could be one possibility as well; that'd
> > make it quite clear and drivers not supporting it wouldn't implement it. I
> > think it might quite well make it together with S_MATRIX, though, e.g. a
> > flags field with a flag telling that the dimension fields are valid.
> 
> Would it be an idea to add a flags field to both v4l2_matrix and v4l2_query_matrix?
> We don't have flags yet, but that makes it easy to add. For a feature such as you
> describe it would be easy enough to implement that by setting an e.g.
> V4L2_MATRIX_FL_NEW_SIZE flag. In query_matrix you would than have a
> V4L2_QMATRIX_FL_HAS_NEW_SIZE (or perhaps in query_matrix it should be called
> 'capabilities' instead).
> 
> I can also just leave it out and use one of the reserved fields when such a feature
> is needed.

I propose adding the flags fields once they're actually needed.

> >> + * @matrix:	pointer to the matrix of size (in bytes):
> >> + *		elem_size * rect.width * rect.height
> >> + * @reserved:	future extensions, applications and drivers must zero this.
> >> + */
> >> +struct v4l2_matrix {
> >> +	__u32 type;
> >> +	union {
> >> +		__u32 raw[4];
> >> +	} ref;
> >> +	struct v4l2_rect rect;
> >> +	void __user *matrix;
> >> +	__u32 reserved[12];
> >> +} __attribute__ ((packed));
> >> +
> >>  /*
> >>   *	I O C T L   C O D E S   F O R   V I D E O   D E V I C E S
> >>   *
> >> @@ -1946,6 +2004,12 @@ struct v4l2_create_buffers {
> >>     Never use these in applications! */
> >>  #define VIDIOC_DBG_G_CHIP_INFO  _IOWR('V', 102, struct v4l2_dbg_chip_info)
> >>  
> >> +/* Experimental, these three ioctls may change over the next couple of kernel
> >> +   versions. */
> >> +#define VIDIOC_QUERY_MATRIX	_IOWR('V', 103, struct v4l2_query_matrix)
> >> +#define VIDIOC_G_MATRIX		_IOWR('V', 104, struct v4l2_matrix)
> >> +#define VIDIOC_S_MATRIX		_IOWR('V', 105, struct v4l2_matrix)
> >> +
> >>  /* Reminder: when adding new ioctls please add support for them to
> >>     drivers/media/video/v4l2-compat-ioctl32.c as well! */
> >>  
> > 
> 
> Thanks for the review!
> 
> I'll prepare a new version this weekend dropping the ref fields and integrating the ID
> space into that of the controls.

Thanks! :-)

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[RFCv2 PATCH 03/10] v4l2-compat-ioctl32: add g/s_matrix support.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 54 +++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 8f7a6a4..1d238da 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -777,6 +777,43 @@ static int put_v4l2_subdev_edid32(struct v4l2_subdev_edid *kp, struct v4l2_subde
 	return 0;
 }
 
+struct v4l2_matrix32 {
+	__u32 type;
+	union {
+		__u32 raw[4];
+	} ref;
+	struct v4l2_rect rect;
+	compat_caddr_t matrix;
+	__u32 reserved[12];
+} __attribute__ ((packed));
+
+static int get_v4l2_matrix32(struct v4l2_matrix *kp, struct v4l2_matrix32 __user *up)
+{
+	u32 tmp;
+
+	if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_matrix32)) ||
+			get_user(kp->type, &up->type) ||
+			copy_from_user(&kp->ref, &up->ref, sizeof(up->ref)) ||
+			copy_from_user(&kp->rect, &up->rect, sizeof(up->rect)) ||
+			get_user(tmp, &up->matrix) ||
+			copy_from_user(kp->reserved, up->reserved, sizeof(kp->reserved)))
+		return -EFAULT;
+	kp->matrix = compat_ptr(tmp);
+	return 0;
+}
+
+static int put_v4l2_matrix32(struct v4l2_matrix *kp, struct v4l2_matrix32 __user *up)
+{
+	u32 tmp = (u32)((unsigned long)kp->matrix);
+
+	if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_matrix32)) ||
+			put_user(kp->type, &up->type) ||
+			copy_to_user(&kp->ref, &up->ref, sizeof(kp->ref)) ||
+			copy_to_user(&kp->rect, &up->rect, sizeof(kp->rect)) ||
+			copy_to_user(kp->reserved, up->reserved, sizeof(kp->reserved)))
+		return -EFAULT;
+	return 0;
+}
 
 #define VIDIOC_G_FMT32		_IOWR('V',  4, struct v4l2_format32)
 #define VIDIOC_S_FMT32		_IOWR('V',  5, struct v4l2_format32)
@@ -796,6 +833,8 @@ static int put_v4l2_subdev_edid32(struct v4l2_subdev_edid *kp, struct v4l2_subde
 #define	VIDIOC_DQEVENT32	_IOR ('V', 89, struct v4l2_event32)
 #define VIDIOC_CREATE_BUFS32	_IOWR('V', 92, struct v4l2_create_buffers32)
 #define VIDIOC_PREPARE_BUF32	_IOWR('V', 93, struct v4l2_buffer32)
+#define VIDIOC_G_MATRIX32	_IOWR('V', 104, struct v4l2_matrix32)
+#define VIDIOC_S_MATRIX32	_IOWR('V', 105, struct v4l2_matrix32)
 
 #define VIDIOC_OVERLAY32	_IOW ('V', 14, s32)
 #define VIDIOC_STREAMON32	_IOW ('V', 18, s32)
@@ -817,6 +856,7 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar
 		struct v4l2_event v2ev;
 		struct v4l2_create_buffers v2crt;
 		struct v4l2_subdev_edid v2edid;
+		struct v4l2_matrix v2matrix;
 		unsigned long vx;
 		int vi;
 	} karg;
@@ -851,6 +891,8 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar
 	case VIDIOC_PREPARE_BUF32: cmd = VIDIOC_PREPARE_BUF; break;
 	case VIDIOC_SUBDEV_G_EDID32: cmd = VIDIOC_SUBDEV_G_EDID; break;
 	case VIDIOC_SUBDEV_S_EDID32: cmd = VIDIOC_SUBDEV_S_EDID; break;
+	case VIDIOC_G_MATRIX32: cmd = VIDIOC_G_MATRIX; break;
+	case VIDIOC_S_MATRIX32: cmd = VIDIOC_S_MATRIX; break;
 	}
 
 	switch (cmd) {
@@ -922,6 +964,12 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar
 	case VIDIOC_DQEVENT:
 		compatible_arg = 0;
 		break;
+
+	case VIDIOC_G_MATRIX:
+	case VIDIOC_S_MATRIX:
+		err = get_v4l2_matrix32(&karg.v2matrix, up);
+		compatible_arg = 0;
+		break;
 	}
 	if (err)
 		return err;
@@ -994,6 +1042,11 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar
 	case VIDIOC_ENUMINPUT:
 		err = put_v4l2_input32(&karg.v2i, up);
 		break;
+
+	case VIDIOC_G_MATRIX:
+	case VIDIOC_S_MATRIX:
+		err = put_v4l2_matrix32(&karg.v2matrix, up);
+		break;
 	}
 	return err;
 }
@@ -1089,6 +1142,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
 	case VIDIOC_ENUM_FREQ_BANDS:
 	case VIDIOC_SUBDEV_G_EDID32:
 	case VIDIOC_SUBDEV_S_EDID32:
+	case VIDIOC_QUERY_MATRIX:
 		ret = do_video_ioctl(file, cmd, arg);
 		break;
 
-- 
1.8.3.2

Re: [RFCv2 PATCH 03/10] v4l2-compat-ioctl32: add g/s_matrix support.

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Monday 12 August 2013 12:58:26 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 54 ++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index 8f7a6a4..1d238da
> 100644
> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> @@ -777,6 +777,43 @@ static int put_v4l2_subdev_edid32(struct
> v4l2_subdev_edid *kp, struct v4l2_subde return 0;
>  }
> 
> +struct v4l2_matrix32 {
> +	__u32 type;
> +	union {
> +		__u32 raw[4];
> +	} ref;
> +	struct v4l2_rect rect;
> +	compat_caddr_t matrix;
> +	__u32 reserved[12];
> +} __attribute__ ((packed));
> +
> +static int get_v4l2_matrix32(struct v4l2_matrix *kp, struct v4l2_matrix32
> __user *up) +{
> +	u32 tmp;
> +
> +	if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_matrix32)) ||
> +			get_user(kp->type, &up->type) ||
> +			copy_from_user(&kp->ref, &up->ref, sizeof(up->ref)) ||
> +			copy_from_user(&kp->rect, &up->rect, sizeof(up->rect)) ||
> +			get_user(tmp, &up->matrix) ||
> +			copy_from_user(kp->reserved, up->reserved, sizeof(kp->reserved)))

Shouldn't you align all lines to the ! in the first line ?

> +		return -EFAULT;
> +	kp->matrix = compat_ptr(tmp);
> +	return 0;
> +}
> +
> +static int put_v4l2_matrix32(struct v4l2_matrix *kp, struct v4l2_matrix32
> __user *up) +{
> +	u32 tmp = (u32)((unsigned long)kp->matrix);
> +
> +	if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_matrix32)) ||
> +			put_user(kp->type, &up->type) ||
> +			copy_to_user(&kp->ref, &up->ref, sizeof(kp->ref)) ||

Are driver allowed to change the type and ref fields ? If not those two lines 
could be removed.

> +			copy_to_user(&kp->rect, &up->rect, sizeof(kp->rect)) ||
> +			copy_to_user(kp->reserved, up->reserved, sizeof(kp->reserved)))

Same question regarding the alignment here.

> +		return -EFAULT;
> +	return 0;
> +}
> 
>  #define VIDIOC_G_FMT32		_IOWR('V',  4, struct v4l2_format32)
>  #define VIDIOC_S_FMT32		_IOWR('V',  5, struct v4l2_format32)
> @@ -796,6 +833,8 @@ static int put_v4l2_subdev_edid32(struct
> v4l2_subdev_edid *kp, struct v4l2_subde #define	VIDIOC_DQEVENT32	_IOR ('V',
> 89, struct v4l2_event32)
>  #define VIDIOC_CREATE_BUFS32	_IOWR('V', 92, struct v4l2_create_buffers32)
>  #define VIDIOC_PREPARE_BUF32	_IOWR('V', 93, struct v4l2_buffer32)
> +#define VIDIOC_G_MATRIX32	_IOWR('V', 104, struct v4l2_matrix32)
> +#define VIDIOC_S_MATRIX32	_IOWR('V', 105, struct v4l2_matrix32)
> 
>  #define VIDIOC_OVERLAY32	_IOW ('V', 14, s32)
>  #define VIDIOC_STREAMON32	_IOW ('V', 18, s32)
> @@ -817,6 +856,7 @@ static long do_video_ioctl(struct file *file, unsigned
> int cmd, unsigned long ar struct v4l2_event v2ev;
>  		struct v4l2_create_buffers v2crt;
>  		struct v4l2_subdev_edid v2edid;
> +		struct v4l2_matrix v2matrix;
>  		unsigned long vx;
>  		int vi;
>  	} karg;
> @@ -851,6 +891,8 @@ static long do_video_ioctl(struct file *file, unsigned
> int cmd, unsigned long ar case VIDIOC_PREPARE_BUF32: cmd =
> VIDIOC_PREPARE_BUF; break;
>  	case VIDIOC_SUBDEV_G_EDID32: cmd = VIDIOC_SUBDEV_G_EDID; break;
>  	case VIDIOC_SUBDEV_S_EDID32: cmd = VIDIOC_SUBDEV_S_EDID; break;
> +	case VIDIOC_G_MATRIX32: cmd = VIDIOC_G_MATRIX; break;
> +	case VIDIOC_S_MATRIX32: cmd = VIDIOC_S_MATRIX; break;
>  	}
> 
>  	switch (cmd) {
> @@ -922,6 +964,12 @@ static long do_video_ioctl(struct file *file, unsigned
> int cmd, unsigned long ar case VIDIOC_DQEVENT:
>  		compatible_arg = 0;
>  		break;
> +
> +	case VIDIOC_G_MATRIX:
> +	case VIDIOC_S_MATRIX:
> +		err = get_v4l2_matrix32(&karg.v2matrix, up);
> +		compatible_arg = 0;
> +		break;
>  	}
>  	if (err)
>  		return err;
> @@ -994,6 +1042,11 @@ static long do_video_ioctl(struct file *file, unsigned
> int cmd, unsigned long ar case VIDIOC_ENUMINPUT:
>  		err = put_v4l2_input32(&karg.v2i, up);
>  		break;
> +
> +	case VIDIOC_G_MATRIX:
> +	case VIDIOC_S_MATRIX:
> +		err = put_v4l2_matrix32(&karg.v2matrix, up);
> +		break;
>  	}
>  	return err;
>  }
> @@ -1089,6 +1142,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned
> int cmd, unsigned long arg) case VIDIOC_ENUM_FREQ_BANDS:
>  	case VIDIOC_SUBDEV_G_EDID32:
>  	case VIDIOC_SUBDEV_S_EDID32:
> +	case VIDIOC_QUERY_MATRIX:
>  		ret = do_video_ioctl(file, cmd, arg);
>  		break;
-- 
Regards,

Laurent Pinchart

Re: [RFCv2 PATCH 03/10] v4l2-compat-ioctl32: add g/s_matrix support.

[Hans Verkuil]

On 08/22/2013 12:02 AM, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Monday 12 August 2013 12:58:26 Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 54 ++++++++++++++++++++++++
>>  1 file changed, 54 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>> b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index 8f7a6a4..1d238da
>> 100644
>> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>> @@ -777,6 +777,43 @@ static int put_v4l2_subdev_edid32(struct
>> v4l2_subdev_edid *kp, struct v4l2_subde return 0;
>>  }
>>
>> +struct v4l2_matrix32 {
>> +	__u32 type;
>> +	union {
>> +		__u32 raw[4];
>> +	} ref;
>> +	struct v4l2_rect rect;
>> +	compat_caddr_t matrix;
>> +	__u32 reserved[12];
>> +} __attribute__ ((packed));
>> +
>> +static int get_v4l2_matrix32(struct v4l2_matrix *kp, struct v4l2_matrix32
>> __user *up) +{
>> +	u32 tmp;
>> +
>> +	if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_matrix32)) ||
>> +			get_user(kp->type, &up->type) ||
>> +			copy_from_user(&kp->ref, &up->ref, sizeof(up->ref)) ||
>> +			copy_from_user(&kp->rect, &up->rect, sizeof(up->rect)) ||
>> +			get_user(tmp, &up->matrix) ||
>> +			copy_from_user(kp->reserved, up->reserved, sizeof(kp->reserved)))
> 
> Shouldn't you align all lines to the ! in the first line ?

Will change.

> 
>> +		return -EFAULT;
>> +	kp->matrix = compat_ptr(tmp);
>> +	return 0;
>> +}
>> +
>> +static int put_v4l2_matrix32(struct v4l2_matrix *kp, struct v4l2_matrix32
>> __user *up) +{
>> +	u32 tmp = (u32)((unsigned long)kp->matrix);
>> +
>> +	if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_matrix32)) ||
>> +			put_user(kp->type, &up->type) ||
>> +			copy_to_user(&kp->ref, &up->ref, sizeof(kp->ref)) ||
> 
> Are driver allowed to change the type and ref fields ? If not those two lines 
> could be removed.

Good point, I'll drop that. 'ref' goes away anyway after Sakari's comments.

> 
>> +			copy_to_user(&kp->rect, &up->rect, sizeof(kp->rect)) ||
>> +			copy_to_user(kp->reserved, up->reserved, sizeof(kp->reserved)))
> 
> Same question regarding the alignment here.

Will change.

> 
>> +		return -EFAULT;
>> +	return 0;
>> +}
>>
>>  #define VIDIOC_G_FMT32		_IOWR('V',  4, struct v4l2_format32)
>>  #define VIDIOC_S_FMT32		_IOWR('V',  5, struct v4l2_format32)
>> @@ -796,6 +833,8 @@ static int put_v4l2_subdev_edid32(struct
>> v4l2_subdev_edid *kp, struct v4l2_subde #define	VIDIOC_DQEVENT32	_IOR ('V',
>> 89, struct v4l2_event32)
>>  #define VIDIOC_CREATE_BUFS32	_IOWR('V', 92, struct v4l2_create_buffers32)
>>  #define VIDIOC_PREPARE_BUF32	_IOWR('V', 93, struct v4l2_buffer32)
>> +#define VIDIOC_G_MATRIX32	_IOWR('V', 104, struct v4l2_matrix32)
>> +#define VIDIOC_S_MATRIX32	_IOWR('V', 105, struct v4l2_matrix32)
>>
>>  #define VIDIOC_OVERLAY32	_IOW ('V', 14, s32)
>>  #define VIDIOC_STREAMON32	_IOW ('V', 18, s32)
>> @@ -817,6 +856,7 @@ static long do_video_ioctl(struct file *file, unsigned
>> int cmd, unsigned long ar struct v4l2_event v2ev;
>>  		struct v4l2_create_buffers v2crt;
>>  		struct v4l2_subdev_edid v2edid;
>> +		struct v4l2_matrix v2matrix;
>>  		unsigned long vx;
>>  		int vi;
>>  	} karg;
>> @@ -851,6 +891,8 @@ static long do_video_ioctl(struct file *file, unsigned
>> int cmd, unsigned long ar case VIDIOC_PREPARE_BUF32: cmd =
>> VIDIOC_PREPARE_BUF; break;
>>  	case VIDIOC_SUBDEV_G_EDID32: cmd = VIDIOC_SUBDEV_G_EDID; break;
>>  	case VIDIOC_SUBDEV_S_EDID32: cmd = VIDIOC_SUBDEV_S_EDID; break;
>> +	case VIDIOC_G_MATRIX32: cmd = VIDIOC_G_MATRIX; break;
>> +	case VIDIOC_S_MATRIX32: cmd = VIDIOC_S_MATRIX; break;
>>  	}
>>
>>  	switch (cmd) {
>> @@ -922,6 +964,12 @@ static long do_video_ioctl(struct file *file, unsigned
>> int cmd, unsigned long ar case VIDIOC_DQEVENT:
>>  		compatible_arg = 0;
>>  		break;
>> +
>> +	case VIDIOC_G_MATRIX:
>> +	case VIDIOC_S_MATRIX:
>> +		err = get_v4l2_matrix32(&karg.v2matrix, up);
>> +		compatible_arg = 0;
>> +		break;
>>  	}
>>  	if (err)
>>  		return err;
>> @@ -994,6 +1042,11 @@ static long do_video_ioctl(struct file *file, unsigned
>> int cmd, unsigned long ar case VIDIOC_ENUMINPUT:
>>  		err = put_v4l2_input32(&karg.v2i, up);
>>  		break;
>> +
>> +	case VIDIOC_G_MATRIX:
>> +	case VIDIOC_S_MATRIX:
>> +		err = put_v4l2_matrix32(&karg.v2matrix, up);
>> +		break;
>>  	}
>>  	return err;
>>  }
>> @@ -1089,6 +1142,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned
>> int cmd, unsigned long arg) case VIDIOC_ENUM_FREQ_BANDS:
>>  	case VIDIOC_SUBDEV_G_EDID32:
>>  	case VIDIOC_SUBDEV_S_EDID32:
>> +	case VIDIOC_QUERY_MATRIX:
>>  		ret = do_video_ioctl(file, cmd, arg);
>>  		break;

Thanks for the comments!

	Hans

[RFCv2 PATCH 04/10] solo: implement the new matrix ioctls instead of the custom ones.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c | 102 ++++++++++++++++++---
 drivers/staging/media/solo6x10/solo6x10.h          |  10 +-
 2 files changed, 89 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c b/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c
index a4c5896..6858993 100644
--- a/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c
+++ b/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c
@@ -1033,29 +1033,98 @@ static int solo_s_parm(struct file *file, void *priv,
 	return solo_g_parm(file, priv, sp);
 }
 
-static long solo_enc_default(struct file *file, void *fh,
-			bool valid_prio, unsigned int cmd, void *arg)
+static int solo_query_matrix(struct file *file, void *fh,
+			struct v4l2_query_matrix *qm)
+{
+	qm->columns = 45;
+	qm->rows = 36;
+	switch (qm->type) {
+	case V4L2_MATRIX_T_MD_REGION:
+		qm->elem_size = 1;
+		break;
+	case V4L2_MATRIX_T_MD_THRESHOLD:
+		qm->elem_max.val = 65535;
+		qm->elem_size = 2;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int solo_g_matrix(struct file *file, void *fh,
+			struct v4l2_matrix *m)
+{
+	struct solo_enc_dev *solo_enc = video_drvdata(file);
+	int w = m->rect.width;
+	int h = m->rect.height;
+	u16 *mt;
+	int y;
+
+	if (m->rect.top < 0 || m->rect.top + h > 35 || h < 0 || w < 0 ||
+	    m->rect.left < 0 || m->rect.left + w >= SOLO_MOTION_SZ)
+		return -EINVAL;
+	if (h == 0 || w == 0)
+		return 0;
+
+	switch (m->type) {
+	case V4L2_MATRIX_T_MD_REGION:
+		return clear_user(m->matrix, w * h);
+	case V4L2_MATRIX_T_MD_THRESHOLD:
+		mt = &solo_enc->motion_thresholds.thresholds[m->rect.top][m->rect.left];
+		for (y = 0; y < h; y++, mt += SOLO_MOTION_SZ)
+			if (copy_to_user(m->matrix + y * w * 2, mt, w * 2))
+				return -EFAULT;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int solo_s_matrix(struct file *file, void *fh,
+			struct v4l2_matrix *m)
 {
 	struct solo_enc_dev *solo_enc = video_drvdata(file);
 	struct solo_dev *solo_dev = solo_enc->solo_dev;
-	struct solo_motion_thresholds *thresholds = arg;
+	int w = m->rect.width;
+	int h = m->rect.height;
+	u16 *mt;
+	int y;
 
-	switch (cmd) {
-	case SOLO_IOC_G_MOTION_THRESHOLDS:
-		*thresholds = solo_enc->motion_thresholds;
+	if (m->rect.top < 0 || m->rect.top + h > 35 || h < 0 || w < 0 ||
+	    m->rect.left < 0 || m->rect.left + w >= SOLO_MOTION_SZ)
+		return -EINVAL;
+	if (h == 0 || w == 0)
 		return 0;
 
-	case SOLO_IOC_S_MOTION_THRESHOLDS:
-		if (!valid_prio)
-			return -EBUSY;
-		solo_enc->motion_thresholds = *thresholds;
-		if (solo_enc->motion_enabled && !solo_enc->motion_global)
-			return solo_set_motion_block(solo_dev, solo_enc->ch,
-						&solo_enc->motion_thresholds);
+	switch (m->type) {
+	case V4L2_MATRIX_T_MD_REGION:
+		/* Check that the region matrix is all zeroes */
+		for (y = 0; y < h; y++) {
+			u8 region[SOLO_MOTION_SZ];
+			static const u8 zeroes[SOLO_MOTION_SZ];
+
+			if (copy_from_user(region, m->matrix + y * w, w))
+				return -EFAULT;
+			if (memcmp(region, zeroes, w))
+				return -EINVAL;
+		}
 		return 0;
+	case V4L2_MATRIX_T_MD_THRESHOLD:
+		mt = &solo_enc->motion_thresholds.thresholds[m->rect.top][m->rect.left];
+		for (y = 0; y < h; y++, mt += SOLO_MOTION_SZ)
+			if (copy_from_user(mt, m->matrix + y * w * 2, w * 2))
+				return -EFAULT;
+		break;
 	default:
-		return -ENOTTY;
+		return -EINVAL;
 	}
+
+	if (solo_enc->motion_enabled && !solo_enc->motion_global)
+		return solo_set_motion_block(solo_dev, solo_enc->ch,
+				&solo_enc->motion_thresholds);
+	return 0;
 }
 
 static int solo_s_ctrl(struct v4l2_ctrl *ctrl)
@@ -1141,11 +1210,14 @@ static const struct v4l2_ioctl_ops solo_enc_ioctl_ops = {
 	/* Video capture parameters */
 	.vidioc_s_parm			= solo_s_parm,
 	.vidioc_g_parm			= solo_g_parm,
+	/* Motion Detection matrices */
+	.vidioc_query_matrix		= solo_query_matrix,
+	.vidioc_g_matrix		= solo_g_matrix,
+	.vidioc_s_matrix		= solo_s_matrix,
 	/* Logging and events */
 	.vidioc_log_status		= v4l2_ctrl_log_status,
 	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,
 	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
-	.vidioc_default			= solo_enc_default,
 };
 
 static const struct video_device solo_enc_template = {
diff --git a/drivers/staging/media/solo6x10/solo6x10.h b/drivers/staging/media/solo6x10/solo6x10.h
index 6f91d2e..01c8655 100644
--- a/drivers/staging/media/solo6x10/solo6x10.h
+++ b/drivers/staging/media/solo6x10/solo6x10.h
@@ -114,20 +114,14 @@
  * effect, 44x30 samples are used for NTSC, and 44x36 for PAL.
  * The 5th sample on the 10th row is (10*64)+5 = 645.
  *
- * Using a 64x64 array will result in a problem on some architectures like
- * the powerpc where the size of the argument is limited to 13 bits.
- * Since both PAL and NTSC do not use the full table anyway I've chosen
- * to limit the array to 45x45 (45*16 = 720, which is the maximum PAL/NTSC
- * width).
+ * Internally it is stored as a 45x45 array (45*16 = 720, which is the
+ * maximum PAL/NTSC width).
  */
 #define SOLO_MOTION_SZ (45)
 struct solo_motion_thresholds {
 	__u16	thresholds[SOLO_MOTION_SZ][SOLO_MOTION_SZ];
 };
 
-#define SOLO_IOC_G_MOTION_THRESHOLDS	_IOR('V', BASE_VIDIOC_PRIVATE+0, struct solo_motion_thresholds)
-#define SOLO_IOC_S_MOTION_THRESHOLDS	_IOW('V', BASE_VIDIOC_PRIVATE+1, struct solo_motion_thresholds)
-
 enum SOLO_I2C_STATE {
 	IIC_STATE_IDLE,
 	IIC_STATE_START,
-- 
1.8.3.2

[RFCv2 PATCH 05/10] v4l2: add a motion detection event.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 include/uapi/linux/videodev2.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 605d295..918f397 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1721,6 +1721,7 @@ struct v4l2_streamparm {
 #define V4L2_EVENT_EOS				2
 #define V4L2_EVENT_CTRL				3
 #define V4L2_EVENT_FRAME_SYNC			4
+#define V4L2_EVENT_MOTION_DET			5
 #define V4L2_EVENT_PRIVATE_START		0x08000000
 
 /* Payload for V4L2_EVENT_VSYNC */
@@ -1752,12 +1753,28 @@ struct v4l2_event_frame_sync {
 	__u32 frame_sequence;
 };
 
+#define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ	(1 << 0)
+
+/**
+ * struct v4l2_event_motion_det - motion detection event
+ * @flags:             if V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ is set, then the
+ *                     frame_sequence field is valid.
+ * @frame_sequence:    the frame sequence number associated with this event.
+ * @region_mask:       which regions detected motion.
+ */
+struct v4l2_event_motion_det {
+	__u32 flags;
+	__u32 frame_sequence;
+	__u32 region_mask;
+};
+
 struct v4l2_event {
 	__u32				type;
 	union {
 		struct v4l2_event_vsync		vsync;
 		struct v4l2_event_ctrl		ctrl;
 		struct v4l2_event_frame_sync	frame_sync;
+		struct v4l2_event_motion_det	motion_det;
 		__u8				data[64];
 	} u;
 	__u32				pending;
-- 
1.8.3.2

[RFCv2 PATCH 06/10] solo6x10: implement motion detection events and controls.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c | 117 +++++++++++++--------
 drivers/staging/media/solo6x10/solo6x10.h          |   9 +-
 2 files changed, 74 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c b/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c
index 6858993..db5ce20 100644
--- a/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c
+++ b/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c
@@ -270,6 +270,8 @@ static int solo_enc_on(struct solo_enc_dev *solo_enc)
 	if (solo_enc->bw_weight > solo_dev->enc_bw_remain)
 		return -EBUSY;
 	solo_enc->sequence = 0;
+	solo_enc->motion_last_state = false;
+	solo_enc->frames_since_last_motion = 0;
 	solo_dev->enc_bw_remain -= solo_enc->bw_weight;
 
 	if (solo_enc->type == SOLO_ENC_TYPE_EXT)
@@ -510,15 +512,6 @@ static int solo_enc_fillbuf(struct solo_enc_dev *solo_enc,
 	struct vop_header *vh = enc_buf->vh;
 	int ret;
 
-	/* Check for motion flags */
-	vb->v4l2_buf.flags &= ~(V4L2_BUF_FLAG_MOTION_ON |
-				V4L2_BUF_FLAG_MOTION_DETECTED);
-	if (solo_is_motion_on(solo_enc)) {
-		vb->v4l2_buf.flags |= V4L2_BUF_FLAG_MOTION_ON;
-		if (enc_buf->motion)
-			vb->v4l2_buf.flags |= V4L2_BUF_FLAG_MOTION_DETECTED;
-	}
-
 	switch (solo_enc->fmt) {
 	case V4L2_PIX_FMT_MPEG4:
 	case V4L2_PIX_FMT_H264:
@@ -530,9 +523,49 @@ static int solo_enc_fillbuf(struct solo_enc_dev *solo_enc,
 	}
 
 	if (!ret) {
+		bool send_event = false;
+
 		vb->v4l2_buf.sequence = solo_enc->sequence++;
 		vb->v4l2_buf.timestamp.tv_sec = vh->sec;
 		vb->v4l2_buf.timestamp.tv_usec = vh->usec;
+
+		/* Check for motion flags */
+		if (solo_is_motion_on(solo_enc)) {
+			/* It takes a few frames for the hardware to detect
+			 * motion. Once it does it clears the motion detection
+			 * register and it takes again a few frames before
+			 * motion is seen. This means in practice that when the
+			 * motion field is 1, it will go back to 0 for the next
+			 * frame. This leads to motion detection event being
+			 * sent all the time, which is not what we want.
+			 * Instead wait a few frames before deciding that the
+			 * motion has halted. After some experimentation it
+			 * turns out that waiting for 5 frames works well.
+			 */
+			if (enc_buf->motion == 0 &&
+			    solo_enc->motion_last_state &&
+			    solo_enc->frames_since_last_motion++ > 5)
+				send_event = true;
+			else if (enc_buf->motion) {
+				solo_enc->frames_since_last_motion = 0;
+				send_event = !solo_enc->motion_last_state;
+			}
+		}
+
+		if (send_event) {
+			struct v4l2_event ev = {
+				.type = V4L2_EVENT_MOTION_DET,
+				.u.motion_det = {
+					.flags = V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ,
+					.frame_sequence = vb->v4l2_buf.sequence,
+					.region_mask = enc_buf->motion ? 1 : 0,
+				},
+			};
+
+			solo_enc->motion_last_state = enc_buf->motion;
+			solo_enc->frames_since_last_motion = 0;
+			v4l2_event_queue(solo_enc->vfd, &ev);
+		}
 	}
 
 	vb2_buffer_done(vb, ret ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
@@ -1145,14 +1178,15 @@ static int solo_s_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
 		solo_enc->gop = ctrl->val;
 		return 0;
-	case V4L2_CID_MOTION_THRESHOLD:
-		solo_enc->motion_thresh = ctrl->val;
+	case V4L2_CID_DETECT_MOTION_THRESHOLD:
+		solo_enc->motion_thresh = ctrl->val << 8;
 		if (!solo_enc->motion_global || !solo_enc->motion_enabled)
 			return 0;
-		return solo_set_motion_threshold(solo_dev, solo_enc->ch, ctrl->val);
-	case V4L2_CID_MOTION_MODE:
-		solo_enc->motion_global = ctrl->val == 1;
-		solo_enc->motion_enabled = ctrl->val > 0;
+		return solo_set_motion_threshold(solo_dev, solo_enc->ch,
+				solo_enc->motion_thresh);
+	case V4L2_CID_DETECT_MOTION_MODE:
+		solo_enc->motion_global = ctrl->val == V4L2_DETECT_MOTION_GLOBAL;
+		solo_enc->motion_enabled = ctrl->val > V4L2_DETECT_MOTION_DISABLED;
 		if (ctrl->val) {
 			if (solo_enc->motion_global)
 				solo_set_motion_threshold(solo_dev, solo_enc->ch,
@@ -1174,6 +1208,21 @@ static int solo_s_ctrl(struct v4l2_ctrl *ctrl)
 	return 0;
 }
 
+static int solo_subscribe_event(struct v4l2_fh *fh,
+				const struct v4l2_event_subscription *sub)
+{
+
+	switch (sub->type) {
+	case V4L2_EVENT_CTRL:
+		return v4l2_ctrl_subscribe_event(fh, sub);
+	case V4L2_EVENT_MOTION_DET:
+		/* Allow for up to 30 events (1 second for NTSC) to be
+		 * stored. */
+		return v4l2_event_subscribe(fh, sub, 30, NULL);
+	}
+	return -EINVAL;
+}
+
 static const struct v4l2_file_operations solo_enc_fops = {
 	.owner			= THIS_MODULE,
 	.open			= v4l2_fh_open,
@@ -1216,7 +1265,7 @@ static const struct v4l2_ioctl_ops solo_enc_ioctl_ops = {
 	.vidioc_s_matrix		= solo_s_matrix,
 	/* Logging and events */
 	.vidioc_log_status		= v4l2_ctrl_log_status,
-	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,
+	.vidioc_subscribe_event		= solo_subscribe_event,
 	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
 };
 
@@ -1233,33 +1282,6 @@ static const struct v4l2_ctrl_ops solo_ctrl_ops = {
 	.s_ctrl = solo_s_ctrl,
 };
 
-static const struct v4l2_ctrl_config solo_motion_threshold_ctrl = {
-	.ops = &solo_ctrl_ops,
-	.id = V4L2_CID_MOTION_THRESHOLD,
-	.name = "Motion Detection Threshold",
-	.type = V4L2_CTRL_TYPE_INTEGER,
-	.max = 0xffff,
-	.def = SOLO_DEF_MOT_THRESH,
-	.step = 1,
-	.flags = V4L2_CTRL_FLAG_SLIDER,
-};
-
-static const char * const solo_motion_mode_menu[] = {
-	"Disabled",
-	"Global Threshold",
-	"Regional Threshold",
-	NULL
-};
-
-static const struct v4l2_ctrl_config solo_motion_enable_ctrl = {
-	.ops = &solo_ctrl_ops,
-	.id = V4L2_CID_MOTION_MODE,
-	.name = "Motion Detection Mode",
-	.type = V4L2_CTRL_TYPE_MENU,
-	.qmenu = solo_motion_mode_menu,
-	.max = 2,
-};
-
 static const struct v4l2_ctrl_config solo_osd_text_ctrl = {
 	.ops = &solo_ctrl_ops,
 	.id = V4L2_CID_OSD_TEXT,
@@ -1296,8 +1318,13 @@ static struct solo_enc_dev *solo_enc_alloc(struct solo_dev *solo_dev,
 			V4L2_CID_SHARPNESS, 0, 15, 1, 0);
 	v4l2_ctrl_new_std(hdl, &solo_ctrl_ops,
 			V4L2_CID_MPEG_VIDEO_GOP_SIZE, 1, 255, 1, solo_dev->fps);
-	v4l2_ctrl_new_custom(hdl, &solo_motion_threshold_ctrl, NULL);
-	v4l2_ctrl_new_custom(hdl, &solo_motion_enable_ctrl, NULL);
+	v4l2_ctrl_new_std_menu(hdl, &solo_ctrl_ops,
+			V4L2_CID_DETECT_MOTION_MODE,
+			V4L2_DETECT_MOTION_REGIONAL, 0,
+			V4L2_DETECT_MOTION_DISABLED);
+	v4l2_ctrl_new_std(hdl, &solo_ctrl_ops,
+			V4L2_CID_DETECT_MOTION_THRESHOLD, 0, 0xff, 1,
+			SOLO_DEF_MOT_THRESH >> 8);
 	v4l2_ctrl_new_custom(hdl, &solo_osd_text_ctrl, NULL);
 	if (hdl->error) {
 		ret = hdl->error;
diff --git a/drivers/staging/media/solo6x10/solo6x10.h b/drivers/staging/media/solo6x10/solo6x10.h
index 01c8655..df34a31 100644
--- a/drivers/staging/media/solo6x10/solo6x10.h
+++ b/drivers/staging/media/solo6x10/solo6x10.h
@@ -97,14 +97,7 @@
 #define SOLO_DEFAULT_GOP		30
 #define SOLO_DEFAULT_QP			3
 
-#ifndef V4L2_BUF_FLAG_MOTION_ON
-#define V4L2_BUF_FLAG_MOTION_ON		0x10000
-#define V4L2_BUF_FLAG_MOTION_DETECTED	0x20000
-#endif
-
 #define SOLO_CID_CUSTOM_BASE		(V4L2_CID_USER_BASE | 0xf000)
-#define V4L2_CID_MOTION_MODE		(SOLO_CID_CUSTOM_BASE+0)
-#define V4L2_CID_MOTION_THRESHOLD	(SOLO_CID_CUSTOM_BASE+1)
 #define V4L2_CID_MOTION_TRACE		(SOLO_CID_CUSTOM_BASE+2)
 #define V4L2_CID_OSD_TEXT		(SOLO_CID_CUSTOM_BASE+3)
 
@@ -174,6 +167,8 @@ struct solo_enc_dev {
 	struct solo_motion_thresholds motion_thresholds;
 	bool			motion_global;
 	bool			motion_enabled;
+	bool			motion_last_state;
+	u8			frames_since_last_motion;
 	u16			width;
 	u16			height;
 
-- 
1.8.3.2

[RFCv2 PATCH 07/10] DocBook: add the new v4l detection class controls.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/DocBook/media/v4l/controls.xml | 69 ++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
index c2fc9ec..dabc707 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -4772,4 +4772,73 @@ defines possible values for de-emphasis. Here they are:</entry>
       </table>
 
       </section>
+
+    <section id="detect-controls">
+      <title>Detect Control Reference</title>
+
+      <para>The Detect class includes controls for common features of
+      various motion or object detection capable devices.</para>
+
+      <table pgwide="1" frame="none" id="detect-control-id">
+      <title>Detect Control IDs</title>
+
+      <tgroup cols="4">
+        <colspec colname="c1" colwidth="1*" />
+        <colspec colname="c2" colwidth="6*" />
+        <colspec colname="c3" colwidth="2*" />
+        <colspec colname="c4" colwidth="6*" />
+        <spanspec namest="c1" nameend="c2" spanname="id" />
+        <spanspec namest="c2" nameend="c4" spanname="descr" />
+        <thead>
+          <row>
+            <entry spanname="id" align="left">ID</entry>
+            <entry align="left">Type</entry>
+          </row><row rowsep="1"><entry spanname="descr" align="left">Description</entry>
+          </row>
+        </thead>
+        <tbody valign="top">
+          <row><entry></entry></row>
+          <row>
+            <entry spanname="id"><constant>V4L2_CID_DETECT_CLASS</constant>&nbsp;</entry>
+            <entry>class</entry>
+          </row><row><entry spanname="descr">The Detect class
+descriptor. Calling &VIDIOC-QUERYCTRL; for this control will return a
+description of this control class.</entry>
+          </row>
+          <row>
+            <entry spanname="id"><constant>V4L2_CID_DETECT_MOTION_MODE</constant>&nbsp;</entry>
+            <entry>menu</entry>
+          </row><row><entry spanname="descr">Sets the motion detection mode.</entry>
+          </row>
+	  <row>
+	    <entrytbl spanname="descr" cols="2">
+	      <tbody valign="top">
+		<row>
+		  <entry><constant>V4L2_DETECT_MOTION_DISABLED</constant>
+		  </entry><entry>Disable motion detection.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_DETECT_MOTION_GLOBAL</constant>
+		  </entry><entry>Use a single motion detection threshold.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_DETECT_MOTION_REGIONAL</constant>
+		  </entry><entry>The image is divided into regions, each with their own
+		  motion detection threshold.</entry>
+		</row>
+	      </tbody>
+	    </entrytbl>
+	  </row>
+          <row>
+	    <entry spanname="id"><constant>V4L2_CID_DETECT_MOTION_THRESHOLD</constant>&nbsp;</entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row><entry spanname="descr">Sets the global motion detection threshold to be
+	  used with the <constant>V4L2_DETECT_MOTION_GLOBAL</constant> motion detection mode.</entry>
+          </row>
+        </tbody>
+      </tgroup>
+      </table>
+
+      </section>
 </section>
-- 
1.8.3.2

[RFCv2 PATCH 08/10] DocBook: document new v4l motion detection event.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/DocBook/media/v4l/vidioc-dqevent.xml | 40 ++++++++++++++++++++++
 .../DocBook/media/v4l/vidioc-subscribe-event.xml   |  9 +++++
 2 files changed, 49 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
index 89891ad..23ee1e3 100644
--- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
@@ -94,6 +94,12 @@
 	  </row>
 	  <row>
 	    <entry></entry>
+	    <entry>&v4l2-event-motion-det;</entry>
+            <entry><structfield>motion_det</structfield></entry>
+	    <entry>Event data for event V4L2_EVENT_MOTION_DET.</entry>
+	  </row>
+	  <row>
+	    <entry></entry>
 	    <entry>__u8</entry>
             <entry><structfield>data</structfield>[64]</entry>
 	    <entry>Event data. Defined by the event type. The union
@@ -242,6 +248,40 @@
       </tgroup>
     </table>
 
+    <table frame="none" pgwide="1" id="v4l2-event-motion-det">
+      <title>struct <structname>v4l2_event_motion_det</structname></title>
+      <tgroup cols="3">
+	&cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>flags</structfield></entry>
+	    <entry>
+	      Currently only one flag is available: if <constant>V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ</constant>
+	      is set, then the <structfield>frame_sequence</structfield> field is valid,
+	      otherwise that field should be ignored.
+	    </entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>frame_sequence</structfield></entry>
+	    <entry>
+	      The sequence number of the frame being received. Only valid if the
+	      <constant>V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ</constant> flag was set.
+	    </entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>region_mask</structfield></entry>
+	    <entry>
+	      The bitmask of the regions that reported motion. There is at least one
+	      region. If this field is 0, then no motion was detected at all.
+	    </entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
     <table pgwide="1" frame="none" id="changes-flags">
       <title>Changes</title>
       <tgroup cols="3">
diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
index 5c70b61..d9c3e66 100644
--- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
@@ -155,6 +155,15 @@
 	    </entry>
 	  </row>
 	  <row>
+	    <entry><constant>V4L2_EVENT_MOTION_DET</constant></entry>
+	    <entry>5</entry>
+	    <entry>
+	      <para>Triggered whenever the motion detection state changes, i.e.
+	      whether motion is detected or not. This event has a
+	      &v4l2-event-motion-det; associated with it.</para>
+	    </entry>
+	  </row>
+	  <row>
 	    <entry><constant>V4L2_EVENT_PRIVATE_START</constant></entry>
 	    <entry>0x08000000</entry>
 	    <entry>Base event number for driver-private events.</entry>
-- 
1.8.3.2

Re: [RFCv2 PATCH 08/10] DocBook: document new v4l motion detection event.

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Monday 12 August 2013 12:58:31 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  Documentation/DocBook/media/v4l/vidioc-dqevent.xml | 40 +++++++++++++++++++
>  .../DocBook/media/v4l/vidioc-subscribe-event.xml   |  9 +++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml index 89891ad..23ee1e3
> 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> @@ -94,6 +94,12 @@
>  	  </row>
>  	  <row>
>  	    <entry></entry>
> +	    <entry>&v4l2-event-motion-det;</entry>
> +            <entry><structfield>motion_det</structfield></entry>
> +	    <entry>Event data for event V4L2_EVENT_MOTION_DET.</entry>
> +	  </row>
> +	  <row>
> +	    <entry></entry>
>  	    <entry>__u8</entry>
>              <entry><structfield>data</structfield>[64]</entry>
>  	    <entry>Event data. Defined by the event type. The union
> @@ -242,6 +248,40 @@
>        </tgroup>
>      </table>
> 
> +    <table frame="none" pgwide="1" id="v4l2-event-motion-det">
> +      <title>struct <structname>v4l2_event_motion_det</structname></title>
> +      <tgroup cols="3">
> +	&cs-str;
> +	<tbody valign="top">
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>flags</structfield></entry>
> +	    <entry>
> +	      Currently only one flag is available: if
> <constant>V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ</constant> +	      is set, then
> the <structfield>frame_sequence</structfield> field is valid, +	     
> otherwise that field should be ignored.
> +	    </entry>
> +	  </row>
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>frame_sequence</structfield></entry>
> +	    <entry>
> +	      The sequence number of the frame being received. Only valid if the
> +	      <constant>V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ</constant> flag was set.
> +	    </entry>
> +	  </row>
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>region_mask</structfield></entry>
> +	    <entry>
> +	      The bitmask of the regions that reported motion. There is at least
> one
> +	      region. If this field is 0, then no motion was detected at all.
> +	    </entry>
> +	  </row>
> +	</tbody>
> +      </tgroup>
> +    </table>
> +
>      <table pgwide="1" frame="none" id="changes-flags">
>        <title>Changes</title>
>        <tgroup cols="3">
> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml index
> 5c70b61..d9c3e66 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> @@ -155,6 +155,15 @@
>  	    </entry>
>  	  </row>
>  	  <row>
> +	    <entry><constant>V4L2_EVENT_MOTION_DET</constant></entry>
> +	    <entry>5</entry>
> +	    <entry>
> +	      <para>Triggered whenever the motion detection state changes, i.e.
> +	      whether motion is detected or not.

Isn't the event also triggered when region_mask changes from a non-zero value 
to a different non-zero value ? The second part of the sentence seems to imply 
that the even is only triggered when motion starts being detected or stops 
being detected.

> This event has a
> +	      &v4l2-event-motion-det; associated with it.</para>
> +	    </entry>
> +	  </row>
> +	  <row>
>  	    <entry><constant>V4L2_EVENT_PRIVATE_START</constant></entry>
>  	    <entry>0x08000000</entry>
>  	    <entry>Base event number for driver-private events.</entry>
-- 
Regards,

Laurent Pinchart

Re: [RFCv2 PATCH 08/10] DocBook: document new v4l motion detection event.

[Hans Verkuil]

On 08/21/2013 11:41 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Monday 12 August 2013 12:58:31 Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  Documentation/DocBook/media/v4l/vidioc-dqevent.xml | 40 +++++++++++++++++++
>>  .../DocBook/media/v4l/vidioc-subscribe-event.xml   |  9 +++++
>>  2 files changed, 49 insertions(+)
>>
>> diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
>> b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml index 89891ad..23ee1e3
>> 100644
>> --- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
>> +++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
>> @@ -94,6 +94,12 @@
>>  	  </row>
>>  	  <row>
>>  	    <entry></entry>
>> +	    <entry>&v4l2-event-motion-det;</entry>
>> +            <entry><structfield>motion_det</structfield></entry>
>> +	    <entry>Event data for event V4L2_EVENT_MOTION_DET.</entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry></entry>
>>  	    <entry>__u8</entry>
>>              <entry><structfield>data</structfield>[64]</entry>
>>  	    <entry>Event data. Defined by the event type. The union
>> @@ -242,6 +248,40 @@
>>        </tgroup>
>>      </table>
>>
>> +    <table frame="none" pgwide="1" id="v4l2-event-motion-det">
>> +      <title>struct <structname>v4l2_event_motion_det</structname></title>
>> +      <tgroup cols="3">
>> +	&cs-str;
>> +	<tbody valign="top">
>> +	  <row>
>> +	    <entry>__u32</entry>
>> +	    <entry><structfield>flags</structfield></entry>
>> +	    <entry>
>> +	      Currently only one flag is available: if
>> <constant>V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ</constant> +	      is set, then
>> the <structfield>frame_sequence</structfield> field is valid, +	     
>> otherwise that field should be ignored.
>> +	    </entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry>__u32</entry>
>> +	    <entry><structfield>frame_sequence</structfield></entry>
>> +	    <entry>
>> +	      The sequence number of the frame being received. Only valid if the
>> +	      <constant>V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ</constant> flag was set.
>> +	    </entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry>__u32</entry>
>> +	    <entry><structfield>region_mask</structfield></entry>
>> +	    <entry>
>> +	      The bitmask of the regions that reported motion. There is at least
>> one
>> +	      region. If this field is 0, then no motion was detected at all.
>> +	    </entry>
>> +	  </row>
>> +	</tbody>
>> +      </tgroup>
>> +    </table>
>> +
>>      <table pgwide="1" frame="none" id="changes-flags">
>>        <title>Changes</title>
>>        <tgroup cols="3">
>> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml index
>> 5c70b61..d9c3e66 100644
>> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
>> @@ -155,6 +155,15 @@
>>  	    </entry>
>>  	  </row>
>>  	  <row>
>> +	    <entry><constant>V4L2_EVENT_MOTION_DET</constant></entry>
>> +	    <entry>5</entry>
>> +	    <entry>
>> +	      <para>Triggered whenever the motion detection state changes, i.e.
>> +	      whether motion is detected or not.
> 
> Isn't the event also triggered when region_mask changes from a non-zero value 
> to a different non-zero value ? The second part of the sentence seems to imply 
> that the even is only triggered when motion starts being detected or stops 
> being detected.

Good point. How about this:

"Triggered whenever the motion detection state for one or more of the regions
changes."

Regards,

	Hans

Re: [RFCv2 PATCH 08/10] DocBook: document new v4l motion detection event.

[Laurent Pinchart]

Hi Hans,

On Thursday 22 August 2013 08:38:59 Hans Verkuil wrote:
> On 08/21/2013 11:41 PM, Laurent Pinchart wrote:
> > On Monday 12 August 2013 12:58:31 Hans Verkuil wrote:
> >> From: Hans Verkuil <hans.verkuil@cisco.com>
> >> 
> >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >> ---
> >> 
> >>  Documentation/DocBook/media/v4l/vidioc-dqevent.xml | 40 ++++++++++++++++
> >>  .../DocBook/media/v4l/vidioc-subscribe-event.xml   |  9 +++++
> >>  2 files changed, 49 insertions(+)

[snip]

> >> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> >> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml index
> >> 5c70b61..d9c3e66 100644
> >> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> >> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> >> @@ -155,6 +155,15 @@
> >> 
> >>  	    </entry>
> >>  	  
> >>  	  </row>
> >>  	  <row>
> >> 
> >> +	    <entry><constant>V4L2_EVENT_MOTION_DET</constant></entry>
> >> +	    <entry>5</entry>
> >> +	    <entry>
> >> +	      <para>Triggered whenever the motion detection state changes, 
i.e.
> >> +	      whether motion is detected or not.
> > 
> > Isn't the event also triggered when region_mask changes from a non-zero
> > value to a different non-zero value ? The second part of the sentence
> > seems to imply that the even is only triggered when motion starts being
> > detected or stops being detected.
> 
> Good point. How about this:
> 
> "Triggered whenever the motion detection state for one or more of the
> regions changes."

That sounds good to me.

-- 
Regards,

Laurent Pinchart

[RFCv2 PATCH 09/10] DocBook: document the new v4l2 matrix ioctls.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/DocBook/media/v4l/v4l2.xml           |   2 +
 .../DocBook/media/v4l/vidioc-g-matrix.xml          | 115 +++++++++++++
 .../DocBook/media/v4l/vidioc-query-matrix.xml      | 178 +++++++++++++++++++++
 3 files changed, 295 insertions(+)
 create mode 100644 Documentation/DocBook/media/v4l/vidioc-g-matrix.xml
 create mode 100644 Documentation/DocBook/media/v4l/vidioc-query-matrix.xml

diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index 8469fe1..11687d5 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -584,6 +584,7 @@ and discussions on the V4L mailing list.</revremark>
     &sub-g-frequency;
     &sub-g-input;
     &sub-g-jpegcomp;
+    &sub-g-matrix;
     &sub-g-modulator;
     &sub-g-output;
     &sub-g-parm;
@@ -600,6 +601,7 @@ and discussions on the V4L mailing list.</revremark>
     &sub-querycap;
     &sub-queryctrl;
     &sub-query-dv-timings;
+    &sub-query-matrix;
     &sub-querystd;
     &sub-reqbufs;
     &sub-s-hw-freq-seek;
diff --git a/Documentation/DocBook/media/v4l/vidioc-g-matrix.xml b/Documentation/DocBook/media/v4l/vidioc-g-matrix.xml
new file mode 100644
index 0000000..95a3f4e
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/vidioc-g-matrix.xml
@@ -0,0 +1,115 @@
+<refentry id="vidioc-g-matrix">
+  <refmeta>
+    <refentrytitle>ioctl VIDIOC_G_MATRIX, VIDIOC_S_MATRIX</refentrytitle>
+    &manvol;
+  </refmeta>
+
+  <refnamediv>
+    <refname>VIDIOC_G_MATRIX</refname>
+    <refname>VIDIOC_S_MATRIX</refname>
+    <refpurpose>Get or set a matrix</refpurpose>
+  </refnamediv>
+
+  <refsynopsisdiv>
+    <funcsynopsis>
+      <funcprototype>
+	<funcdef>int <function>ioctl</function></funcdef>
+	<paramdef>int <parameter>fd</parameter></paramdef>
+	<paramdef>int <parameter>request</parameter></paramdef>
+	<paramdef>struct v4l2_matrix
+*<parameter>argp</parameter></paramdef>
+      </funcprototype>
+    </funcsynopsis>
+  </refsynopsisdiv>
+
+  <refsect1>
+    <title>Arguments</title>
+
+    <variablelist>
+      <varlistentry>
+	<term><parameter>fd</parameter></term>
+	<listitem>
+	  <para>&fd;</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>request</parameter></term>
+	<listitem>
+	  <para>VIDIOC_G_MATRIX, VIDIOC_S_MATRIX</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>argp</parameter></term>
+	<listitem>
+	  <para></para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+
+  <refsect1>
+    <title>Description</title>
+
+    <para>Get or set the elements of a matrix. To get a matrix the application fills in the
+    <structfield>type</structfield> and optionally the <structfield>ref</structfield>
+    fields of &v4l2-matrix;. All other fields will be returned by the driver.
+    To set a matrix the application fills all the fields of the structure.
+    </para>
+
+    <table frame="none" pgwide="1" id="v4l2-matrix">
+      <title>struct <structname>v4l2_matrix</structname></title>
+      <tgroup cols="4">
+	&cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>type</structfield></entry>
+            <entry></entry>
+	    <entry>Type of the matrix, see <xref linkend="v4l2-matrix-type" />.</entry>
+	  </row>
+	  <row>
+	    <entry>union</entry>
+	    <entry><structfield>ref</structfield></entry>
+            <entry></entry>
+	    <entry>This union makes it possible to identify the object owning the
+	    matrix. Currently the only defined matrix types are identified through
+	    the filehandle used to call the ioctl, so this union isn't used (yet).</entry>
+	  </row>
+	  <row>
+	    <entry>&v4l2-rect;</entry>
+	    <entry><structfield>rect</structfield></entry>
+            <entry></entry>
+	    <entry>The subset of the matrix that you want to get or set. The rectangle
+	    must fit within the total matrix dimensions, the top left element of the total
+	    matrix is always (0, 0). To get/set the full matrix <structfield>rect</structfield>
+	    should be set to (0, 0, <structfield>columns</structfield>, <structfield>rows</structfield>),
+	    where <structfield>columns</structfield> and <structfield>rows</structfield> are
+	    obtained from &VIDIOC-QUERY-MATRIX;.</entry>
+	  </row>
+	  <row>
+	    <entry>void *</entry>
+	    <entry><structfield>matrix</structfield></entry>
+            <entry></entry>
+	    <entry>A pointer to the matrix. This matrix has size <structfield>rect.width</structfield> *
+	    <structfield>rect.height</structfield> * <structfield>elem_size</structfield>.
+	    The <structfield>elem_size</structfield> can be obtained via &VIDIOC-QUERY-MATRIX;.
+	    The elements are stored row-by-row in the matrix. The first element is element
+	    (<structfield>rect.top</structfield>, <structfield>rect.left</structfield>) of the
+	    full matrix.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[12]</entry>
+            <entry></entry>
+	    <entry>Reserved for future extensions. Drivers and applications must set
+	    the array to zero.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+  </refsect1>
+  <refsect1>
+    &return-value;
+  </refsect1>
+</refentry>
diff --git a/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml b/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml
new file mode 100644
index 0000000..c2845c7
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml
@@ -0,0 +1,178 @@
+<refentry id="vidioc-query-matrix">
+  <refmeta>
+    <refentrytitle>ioctl VIDIOC_QUERY_MATRIX</refentrytitle>
+    &manvol;
+  </refmeta>
+
+  <refnamediv>
+    <refname>VIDIOC_QUERY_MATRIX</refname>
+    <refpurpose>Query the attributes of a matrix</refpurpose>
+  </refnamediv>
+
+  <refsynopsisdiv>
+    <funcsynopsis>
+      <funcprototype>
+	<funcdef>int <function>ioctl</function></funcdef>
+	<paramdef>int <parameter>fd</parameter></paramdef>
+	<paramdef>int <parameter>request</parameter></paramdef>
+	<paramdef>struct v4l2_query_matrix
+*<parameter>argp</parameter></paramdef>
+      </funcprototype>
+    </funcsynopsis>
+  </refsynopsisdiv>
+
+  <refsect1>
+    <title>Arguments</title>
+
+    <variablelist>
+      <varlistentry>
+	<term><parameter>fd</parameter></term>
+	<listitem>
+	  <para>&fd;</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>request</parameter></term>
+	<listitem>
+	  <para>VIDIOC_QUERY_MATRIX</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>argp</parameter></term>
+	<listitem>
+	  <para></para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+
+  <refsect1>
+    <title>Description</title>
+
+    <para>Query the attributes of a matrix. The application fills in the
+    <structfield>type</structfield> and optionally the <structfield>ref</structfield>
+    fields of &v4l2-query-matrix;. All other fields will be returned by the driver.
+    </para>
+
+    <table frame="none" pgwide="1" id="v4l2-query-matrix">
+      <title>struct <structname>v4l2_query_matrix</structname></title>
+      <tgroup cols="4">
+	&cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>type</structfield></entry>
+            <entry></entry>
+	    <entry>Type of the matrix, see <xref linkend="v4l2-matrix-type" />.</entry>
+	  </row>
+	  <row>
+	    <entry>union</entry>
+	    <entry><structfield>ref</structfield></entry>
+            <entry></entry>
+	    <entry>This union makes it possible to identify the object owning the
+	    matrix. Currently the only defined matrix types are identified through
+	    the filehandle used to call the ioctl, so this union isn't used (yet).</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>columns</structfield></entry>
+            <entry></entry>
+	    <entry>Number of columns in the matrix.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>rows</structfield></entry>
+            <entry></entry>
+	    <entry>Number of rows in the matrix.</entry>
+	  </row>
+	  <row>
+	    <entry>union</entry>
+	    <entry><structfield>elem_min</structfield></entry>
+            <entry></entry>
+            <entry></entry>
+	  </row>
+	  <row>
+	    <entry></entry>
+            <entry>__s64</entry>
+	    <entry><structfield>val</structfield></entry>
+            <entry>The minimal signed value of each matrix element.</entry>
+	  </row>
+	  <row>
+	    <entry></entry>
+            <entry>__u64</entry>
+	    <entry><structfield>uval</structfield></entry>
+            <entry>The minimal unsigned value of each matrix element.</entry>
+	  </row>
+	  <row>
+	    <entry>union</entry>
+	    <entry><structfield>elem_max</structfield></entry>
+            <entry></entry>
+            <entry></entry>
+	  </row>
+	  <row>
+	    <entry></entry>
+            <entry>__s64</entry>
+	    <entry><structfield>val</structfield></entry>
+            <entry>The maximal signed value of each matrix element.</entry>
+	  </row>
+	  <row>
+	    <entry></entry>
+            <entry>__u64</entry>
+	    <entry><structfield>uval</structfield></entry>
+            <entry>The maximal unsigned value of each matrix element.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>elem_size</structfield></entry>
+            <entry></entry>
+	    <entry>The size in bytes of a single matrix element.
+	    The full matrix size will be <structfield>columns</structfield> *
+	    <structfield>rows</structfield> * <structfield>elem_size</structfield>.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[12]</entry>
+            <entry></entry>
+	    <entry>Reserved for future extensions. Drivers must set
+	    the array to zero.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+    <table pgwide="1" frame="none" id="v4l2-matrix-type">
+      <title>Matrix Types</title>
+      <tgroup cols="2" align="left">
+	<colspec colwidth="30*" />
+	<colspec colwidth="55*" />
+	<thead>
+	  <row>
+	    <entry>Type</entry>
+	    <entry>Description</entry>
+	  </row>
+	</thead>
+	<tbody valign="top">
+	  <row>
+	    <entry><constant>V4L2_MATRIX_T_MD_REGION</constant></entry>
+	    <entry>Hardware motion detection often divides the image into several
+	    regions, and each region can have its own motion detection thresholds.
+	    This matrix assigns a region number to each element. Each element is a __u8.
+	    Generally each element refers to a block of pixels in the image.
+	    </entry>
+	  </row>
+	  <row>
+	    <entry><constant>V4L2_MATRIX_T_MD_THRESHOLD</constant></entry>
+	    <entry>Hardware motion detection can assign motion detection threshold
+	    values to each element of an image. Each element is a __u16.
+            Generally each element refers to a block of pixels in the image.
+	    </entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+  </refsect1>
+  <refsect1>
+    &return-value;
+  </refsect1>
+</refentry>
-- 
1.8.3.2

Re: [RFCv2 PATCH 09/10] DocBook: document the new v4l2 matrix ioctls.

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Monday 12 August 2013 12:58:32 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  Documentation/DocBook/media/v4l/v4l2.xml           |   2 +
>  .../DocBook/media/v4l/vidioc-g-matrix.xml          | 115 +++++++++++++
>  .../DocBook/media/v4l/vidioc-query-matrix.xml      | 178 ++++++++++++++++++
>  3 files changed, 295 insertions(+)
>  create mode 100644 Documentation/DocBook/media/v4l/vidioc-g-matrix.xml
>  create mode 100644 Documentation/DocBook/media/v4l/vidioc-query-matrix.xml

[snip]

> diff --git a/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml
> b/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml new file mode
> 100644
> index 0000000..c2845c7
> --- /dev/null
> +++ b/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml
> @@ -0,0 +1,178 @@
> +<refentry id="vidioc-query-matrix">
> +  <refmeta>
> +    <refentrytitle>ioctl VIDIOC_QUERY_MATRIX</refentrytitle>
> +    &manvol;
> +  </refmeta>
> +
> +  <refnamediv>
> +    <refname>VIDIOC_QUERY_MATRIX</refname>
> +    <refpurpose>Query the attributes of a matrix</refpurpose>
> +  </refnamediv>
> +
> +  <refsynopsisdiv>
> +    <funcsynopsis>
> +      <funcprototype>
> +	<funcdef>int <function>ioctl</function></funcdef>
> +	<paramdef>int <parameter>fd</parameter></paramdef>
> +	<paramdef>int <parameter>request</parameter></paramdef>
> +	<paramdef>struct v4l2_query_matrix
> +*<parameter>argp</parameter></paramdef>
> +      </funcprototype>
> +    </funcsynopsis>
> +  </refsynopsisdiv>
> +
> +  <refsect1>
> +    <title>Arguments</title>
> +
> +    <variablelist>
> +      <varlistentry>
> +	<term><parameter>fd</parameter></term>
> +	<listitem>
> +	  <para>&fd;</para>
> +	</listitem>
> +      </varlistentry>
> +      <varlistentry>
> +	<term><parameter>request</parameter></term>
> +	<listitem>
> +	  <para>VIDIOC_QUERY_MATRIX</para>
> +	</listitem>
> +      </varlistentry>
> +      <varlistentry>
> +	<term><parameter>argp</parameter></term>
> +	<listitem>
> +	  <para></para>
> +	</listitem>
> +      </varlistentry>
> +    </variablelist>
> +  </refsect1>
> +
> +  <refsect1>
> +    <title>Description</title>
> +
> +    <para>Query the attributes of a matrix. The application fills in the
> +    <structfield>type</structfield> and optionally the
> <structfield>ref</structfield>
> +    fields of &v4l2-query-matrix;. All other fields will be returned by the
> driver.
> +    </para>
> +
> +    <table frame="none" pgwide="1" id="v4l2-query-matrix">
> +      <title>struct <structname>v4l2_query_matrix</structname></title>
> +      <tgroup cols="4">
> +	&cs-str;
> +	<tbody valign="top">
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>type</structfield></entry>
> +            <entry></entry>
> +	    <entry>Type of the matrix, see <xref linkend="v4l2-matrix-type"
> />.</entry> +	  </row>
> +	  <row>
> +	    <entry>union</entry>
> +	    <entry><structfield>ref</structfield></entry>
> +            <entry></entry>
> +	    <entry>This union makes it possible to identify the object owning the
> +	    matrix. Currently the only defined matrix types are identified
> through
> +	    the filehandle used to call the ioctl, so this union isn't used
> (yet).</entry>
> +	  </row>
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>columns</structfield></entry>
> +            <entry></entry>
> +	    <entry>Number of columns in the matrix.</entry>
> +	  </row>
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>rows</structfield></entry>
> +            <entry></entry>
> +	    <entry>Number of rows in the matrix.</entry>
> +	  </row>
> +	  <row>
> +	    <entry>union</entry>
> +	    <entry><structfield>elem_min</structfield></entry>
> +            <entry></entry>
> +            <entry></entry>
> +	  </row>
> +	  <row>
> +	    <entry></entry>
> +            <entry>__s64</entry>
> +	    <entry><structfield>val</structfield></entry>
> +            <entry>The minimal signed value of each matrix element.</entry>
> +	  </row>
> +	  <row>
> +	    <entry></entry>
> +            <entry>__u64</entry>
> +	    <entry><structfield>uval</structfield></entry>
> +            <entry>The minimal unsigned value of each matrix
> element.</entry>
> +	  </row>
> +	  <row>
> +	    <entry>union</entry>
> +	    <entry><structfield>elem_max</structfield></entry>
> +            <entry></entry>
> +            <entry></entry>
> +	  </row>
> +	  <row>
> +	    <entry></entry>
> +            <entry>__s64</entry>
> +	    <entry><structfield>val</structfield></entry>
> +            <entry>The maximal signed value of each matrix element.</entry>
> +	  </row>
> +	  <row>
> +	    <entry></entry>
> +            <entry>__u64</entry>
> +	    <entry><structfield>uval</structfield></entry>
> +            <entry>The maximal unsigned value of each matrix
> element.</entry>
> +	  </row>
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>elem_size</structfield></entry>
> +            <entry></entry>
> +	    <entry>The size in bytes of a single matrix element.
> +	    The full matrix size will be <structfield>columns</structfield> *
> +	    <structfield>rows</structfield> *
> <structfield>elem_size</structfield>.</entry>
> +	  </row>
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>reserved</structfield>[12]</entry>
> +            <entry></entry>
> +	    <entry>Reserved for future extensions. Drivers must set
> +	    the array to zero.</entry>
> +	  </row>
> +	</tbody>
> +      </tgroup>
> +    </table>
> +
> +    <table pgwide="1" frame="none" id="v4l2-matrix-type">
> +      <title>Matrix Types</title>
> +      <tgroup cols="2" align="left">
> +	<colspec colwidth="30*" />
> +	<colspec colwidth="55*" />
> +	<thead>
> +	  <row>
> +	    <entry>Type</entry>
> +	    <entry>Description</entry>
> +	  </row>
> +	</thead>
> +	<tbody valign="top">
> +	  <row>
> +	    <entry><constant>V4L2_MATRIX_T_MD_REGION</constant></entry>
> +	    <entry>Hardware motion detection often divides the image into several
> +	    regions, and each region can have its own motion detection
> thresholds.
> +	    This matrix assigns a region number to each element. Each element is
> a __u8.
> +	    Generally each element refers to a block of pixels in the image.

>From the description I have trouble understanding what the matrix type is for. 
Do you think we could make the explanation more detailed ?

> +	    </entry>
> +	  </row>
> +	  <row>
> +	    <entry><constant>V4L2_MATRIX_T_MD_THRESHOLD</constant></entry>
> +	    <entry>Hardware motion detection can assign motion detection 
threshold
> +	    values to each element of an image. Each element is a __u16. +       
>     Generally each element refers to a block of pixels in the image. +	   
> </entry>
> +	  </row>
> +	</tbody>
> +      </tgroup>
> +    </table>
> +
> +  </refsect1>
> +  <refsect1>
> +    &return-value;
> +  </refsect1>
> +</refentry>

-- 
Regards,

Laurent Pinchart

Re: [RFCv2 PATCH 09/10] DocBook: document the new v4l2 matrix ioctls.

[Hans Verkuil]

On 08/21/2013 11:58 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Monday 12 August 2013 12:58:32 Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  Documentation/DocBook/media/v4l/v4l2.xml           |   2 +
>>  .../DocBook/media/v4l/vidioc-g-matrix.xml          | 115 +++++++++++++
>>  .../DocBook/media/v4l/vidioc-query-matrix.xml      | 178 ++++++++++++++++++
>>  3 files changed, 295 insertions(+)
>>  create mode 100644 Documentation/DocBook/media/v4l/vidioc-g-matrix.xml
>>  create mode 100644 Documentation/DocBook/media/v4l/vidioc-query-matrix.xml
> 
> [snip]
> 
>> diff --git a/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml
>> b/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml new file mode
>> 100644
>> index 0000000..c2845c7
>> --- /dev/null
>> +++ b/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml
>> @@ -0,0 +1,178 @@
>> +<refentry id="vidioc-query-matrix">
>> +  <refmeta>
>> +    <refentrytitle>ioctl VIDIOC_QUERY_MATRIX</refentrytitle>
>> +    &manvol;
>> +  </refmeta>
>> +
>> +  <refnamediv>
>> +    <refname>VIDIOC_QUERY_MATRIX</refname>
>> +    <refpurpose>Query the attributes of a matrix</refpurpose>
>> +  </refnamediv>
>> +
>> +  <refsynopsisdiv>
>> +    <funcsynopsis>
>> +      <funcprototype>
>> +	<funcdef>int <function>ioctl</function></funcdef>
>> +	<paramdef>int <parameter>fd</parameter></paramdef>
>> +	<paramdef>int <parameter>request</parameter></paramdef>
>> +	<paramdef>struct v4l2_query_matrix
>> +*<parameter>argp</parameter></paramdef>
>> +      </funcprototype>
>> +    </funcsynopsis>
>> +  </refsynopsisdiv>
>> +
>> +  <refsect1>
>> +    <title>Arguments</title>
>> +
>> +    <variablelist>
>> +      <varlistentry>
>> +	<term><parameter>fd</parameter></term>
>> +	<listitem>
>> +	  <para>&fd;</para>
>> +	</listitem>
>> +      </varlistentry>
>> +      <varlistentry>
>> +	<term><parameter>request</parameter></term>
>> +	<listitem>
>> +	  <para>VIDIOC_QUERY_MATRIX</para>
>> +	</listitem>
>> +      </varlistentry>
>> +      <varlistentry>
>> +	<term><parameter>argp</parameter></term>
>> +	<listitem>
>> +	  <para></para>
>> +	</listitem>
>> +      </varlistentry>
>> +    </variablelist>
>> +  </refsect1>
>> +
>> +  <refsect1>
>> +    <title>Description</title>
>> +
>> +    <para>Query the attributes of a matrix. The application fills in the
>> +    <structfield>type</structfield> and optionally the
>> <structfield>ref</structfield>
>> +    fields of &v4l2-query-matrix;. All other fields will be returned by the
>> driver.
>> +    </para>
>> +
>> +    <table frame="none" pgwide="1" id="v4l2-query-matrix">
>> +      <title>struct <structname>v4l2_query_matrix</structname></title>
>> +      <tgroup cols="4">
>> +	&cs-str;
>> +	<tbody valign="top">
>> +	  <row>
>> +	    <entry>__u32</entry>
>> +	    <entry><structfield>type</structfield></entry>
>> +            <entry></entry>
>> +	    <entry>Type of the matrix, see <xref linkend="v4l2-matrix-type"
>> />.</entry> +	  </row>
>> +	  <row>
>> +	    <entry>union</entry>
>> +	    <entry><structfield>ref</structfield></entry>
>> +            <entry></entry>
>> +	    <entry>This union makes it possible to identify the object owning the
>> +	    matrix. Currently the only defined matrix types are identified
>> through
>> +	    the filehandle used to call the ioctl, so this union isn't used
>> (yet).</entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry>__u32</entry>
>> +	    <entry><structfield>columns</structfield></entry>
>> +            <entry></entry>
>> +	    <entry>Number of columns in the matrix.</entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry>__u32</entry>
>> +	    <entry><structfield>rows</structfield></entry>
>> +            <entry></entry>
>> +	    <entry>Number of rows in the matrix.</entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry>union</entry>
>> +	    <entry><structfield>elem_min</structfield></entry>
>> +            <entry></entry>
>> +            <entry></entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry></entry>
>> +            <entry>__s64</entry>
>> +	    <entry><structfield>val</structfield></entry>
>> +            <entry>The minimal signed value of each matrix element.</entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry></entry>
>> +            <entry>__u64</entry>
>> +	    <entry><structfield>uval</structfield></entry>
>> +            <entry>The minimal unsigned value of each matrix
>> element.</entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry>union</entry>
>> +	    <entry><structfield>elem_max</structfield></entry>
>> +            <entry></entry>
>> +            <entry></entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry></entry>
>> +            <entry>__s64</entry>
>> +	    <entry><structfield>val</structfield></entry>
>> +            <entry>The maximal signed value of each matrix element.</entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry></entry>
>> +            <entry>__u64</entry>
>> +	    <entry><structfield>uval</structfield></entry>
>> +            <entry>The maximal unsigned value of each matrix
>> element.</entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry>__u32</entry>
>> +	    <entry><structfield>elem_size</structfield></entry>
>> +            <entry></entry>
>> +	    <entry>The size in bytes of a single matrix element.
>> +	    The full matrix size will be <structfield>columns</structfield> *
>> +	    <structfield>rows</structfield> *
>> <structfield>elem_size</structfield>.</entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry>__u32</entry>
>> +	    <entry><structfield>reserved</structfield>[12]</entry>
>> +            <entry></entry>
>> +	    <entry>Reserved for future extensions. Drivers must set
>> +	    the array to zero.</entry>
>> +	  </row>
>> +	</tbody>
>> +      </tgroup>
>> +    </table>
>> +
>> +    <table pgwide="1" frame="none" id="v4l2-matrix-type">
>> +      <title>Matrix Types</title>
>> +      <tgroup cols="2" align="left">
>> +	<colspec colwidth="30*" />
>> +	<colspec colwidth="55*" />
>> +	<thead>
>> +	  <row>
>> +	    <entry>Type</entry>
>> +	    <entry>Description</entry>
>> +	  </row>
>> +	</thead>
>> +	<tbody valign="top">
>> +	  <row>
>> +	    <entry><constant>V4L2_MATRIX_T_MD_REGION</constant></entry>
>> +	    <entry>Hardware motion detection often divides the image into several
>> +	    regions, and each region can have its own motion detection
>> thresholds.
>> +	    This matrix assigns a region number to each element. Each element is
>> a __u8.
>> +	    Generally each element refers to a block of pixels in the image.
> 
> From the description I have trouble understanding what the matrix type is for. 
> Do you think we could make the explanation more detailed ?

How about this:

Hardware motion detection divides the image up into cells. If the image resolution
is WxH and the matrix size is COLSxROWS, then each cell is a rectangle of (W/COLS)x(H/ROWS)
pixels (approximately as there may be some rounding involved). Depending on the
hardware each cell can have its own properties. This matrix type sets the 'region'
property which is a __u8. Each region will typically have its own set of motion detection
parameters such as a threshold that determines the motion detection sensitivity. By
assigning each cell a region you can create regions with lower and regions with higher
motion sensitivity. 

> > +	    </entry>
> > +	  </row>
> > +	  <row>
> > +	    <entry><constant>V4L2_MATRIX_T_MD_THRESHOLD</constant></entry>
> > +	    <entry>Hardware motion detection can assign motion detection threshold
> > +	    values to each element of an image. Each element is a __u16.
> > +       Generally each element refers to a block of pixels in the image.

This would be improved as well along the same lines:

Hardware motion detection divides the image up into cells. If the image resolution
is WxH and the matrix size is COLSxROWS, then each cell is a rectangle of (W/COLS)x(H/ROWS)
pixels (approximately as there may be some rounding involved). Depending on the
hardware each cell can have its own motion detection sensitivity threshold. This matrix
type sets the motion detection threshold property which is a __u16.


> > +	    </entry>
> > +	  </row>
> > +	</tbody>
> > +      </tgroup>
> > +    </table>
> > +
> > +  </refsect1>
> > +  <refsect1>
> > +    &return-value;
> > +  </refsect1>
> > +</refentry>

Regards,

	Hans

Re: [RFCv2 PATCH 09/10] DocBook: document the new v4l2 matrix ioctls.

[Laurent Pinchart]

Hi Hans,

On Thursday 22 August 2013 08:56:00 Hans Verkuil wrote:
> On 08/21/2013 11:58 PM, Laurent Pinchart wrote:
> > On Monday 12 August 2013 12:58:32 Hans Verkuil wrote:
> >> From: Hans Verkuil <hans.verkuil@cisco.com>
> >> 
> >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >> ---
> >> 
> >>  Documentation/DocBook/media/v4l/v4l2.xml           |   2 +
> >>  .../DocBook/media/v4l/vidioc-g-matrix.xml          | 115 +++++++++++++
> >>  .../DocBook/media/v4l/vidioc-query-matrix.xml      | 178 +++++++++++++++
> >>  3 files changed, 295 insertions(+)
> >>  create mode 100644 Documentation/DocBook/media/v4l/vidioc-g-matrix.xml
> >>  create mode 100644
> >>  Documentation/DocBook/media/v4l/vidioc-query-matrix.xml
> > 
> > [snip]
> > 
> >> diff --git a/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml
> >> b/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml new file mode
> >> 100644
> >> index 0000000..c2845c7
> >> --- /dev/null
> >> +++ b/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml

[snip]

> >> +    <table pgwide="1" frame="none" id="v4l2-matrix-type">
> >> +      <title>Matrix Types</title>
> >> +      <tgroup cols="2" align="left">
> >> +	<colspec colwidth="30*" />
> >> +	<colspec colwidth="55*" />
> >> +	<thead>
> >> +	  <row>
> >> +	    <entry>Type</entry>
> >> +	    <entry>Description</entry>
> >> +	  </row>
> >> +	</thead>
> >> +	<tbody valign="top">
> >> +	  <row>
> >> +	    <entry><constant>V4L2_MATRIX_T_MD_REGION</constant></entry>
> >> +	    <entry>Hardware motion detection often divides the image into
> >> several
> >> +	    regions, and each region can have its own motion detection
> >> thresholds.
> >> +	    This matrix assigns a region number to each element. Each element
> >> is
> >> a __u8.
> >> +	    Generally each element refers to a block of pixels in the image.
> > 
> > From the description I have trouble understanding what the matrix type is
> > for. Do you think we could make the explanation more detailed ?
> 
> How about this:
> 
> Hardware motion detection divides the image up into cells. If the image
> resolution is WxH and the matrix size is COLSxROWS, then each cell is a
> rectangle of (W/COLS)x(H/ROWS) pixels (approximately as there may be some
> rounding involved). Depending on the hardware each cell can have its own
> properties. This matrix type sets the 'region' property which is a __u8.
> Each region will typically have its own set of motion detection parameters
> such as a threshold that determines the motion detection sensitivity. By
> assigning each cell a region you can create regions with lower and regions
> with higher motion sensitivity.

That sounds good to me. One more question, however: if the hardware divides 
the sub-sampled image into regions, how do you configure per-region thresholds 
? The V4L2_MATRIX_T_MD_THRESHOLD matrix only configures per-cell thresholds.

> > > +	    </entry>
> > > +	  </row>
> > > +	  <row>
> > > +	    <entry><constant>V4L2_MATRIX_T_MD_THRESHOLD</constant></entry>
> > > +	    <entry>Hardware motion detection can assign motion detection
> > > threshold +	    values to each element of an image. Each element is a
> > > __u16. +       Generally each element refers to a block of pixels in
> > > the image.
> This would be improved as well along the same lines:
> 
> Hardware motion detection divides the image up into cells. If the image
> resolution is WxH and the matrix size is COLSxROWS, then each cell is a
> rectangle of (W/COLS)x(H/ROWS) pixels (approximately as there may be some
> rounding involved). Depending on the hardware each cell can have its own
> motion detection sensitivity threshold. This matrix type sets the motion
> detection threshold property which is a __u16.
> > > +	    </entry>
> > > +	  </row>
> > > +	</tbody>
> > > +      </tgroup>
> > > +    </table>
> > > +
> > > +  </refsect1>
> > > +  <refsect1>
> > > +    &return-value;
> > > +  </refsect1>
> > > +</refentry>

-- 
Regards,

Laurent Pinchart

Re: [RFCv2 PATCH 09/10] DocBook: document the new v4l2 matrix ioctls.

[Hans Verkuil]

On Thu 22 August 2013 12:34:56 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Thursday 22 August 2013 08:56:00 Hans Verkuil wrote:
> > On 08/21/2013 11:58 PM, Laurent Pinchart wrote:
> > > On Monday 12 August 2013 12:58:32 Hans Verkuil wrote:
> > >> From: Hans Verkuil <hans.verkuil@cisco.com>
> > >> 
> > >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > >> ---
> > >> 
> > >>  Documentation/DocBook/media/v4l/v4l2.xml           |   2 +
> > >>  .../DocBook/media/v4l/vidioc-g-matrix.xml          | 115 +++++++++++++
> > >>  .../DocBook/media/v4l/vidioc-query-matrix.xml      | 178 +++++++++++++++
> > >>  3 files changed, 295 insertions(+)
> > >>  create mode 100644 Documentation/DocBook/media/v4l/vidioc-g-matrix.xml
> > >>  create mode 100644
> > >>  Documentation/DocBook/media/v4l/vidioc-query-matrix.xml
> > > 
> > > [snip]
> > > 
> > >> diff --git a/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml
> > >> b/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml new file mode
> > >> 100644
> > >> index 0000000..c2845c7
> > >> --- /dev/null
> > >> +++ b/Documentation/DocBook/media/v4l/vidioc-query-matrix.xml
> 
> [snip]
> 
> > >> +    <table pgwide="1" frame="none" id="v4l2-matrix-type">
> > >> +      <title>Matrix Types</title>
> > >> +      <tgroup cols="2" align="left">
> > >> +	<colspec colwidth="30*" />
> > >> +	<colspec colwidth="55*" />
> > >> +	<thead>
> > >> +	  <row>
> > >> +	    <entry>Type</entry>
> > >> +	    <entry>Description</entry>
> > >> +	  </row>
> > >> +	</thead>
> > >> +	<tbody valign="top">
> > >> +	  <row>
> > >> +	    <entry><constant>V4L2_MATRIX_T_MD_REGION</constant></entry>
> > >> +	    <entry>Hardware motion detection often divides the image into
> > >> several
> > >> +	    regions, and each region can have its own motion detection
> > >> thresholds.
> > >> +	    This matrix assigns a region number to each element. Each element
> > >> is
> > >> a __u8.
> > >> +	    Generally each element refers to a block of pixels in the image.
> > > 
> > > From the description I have trouble understanding what the matrix type is
> > > for. Do you think we could make the explanation more detailed ?
> > 
> > How about this:
> > 
> > Hardware motion detection divides the image up into cells. If the image
> > resolution is WxH and the matrix size is COLSxROWS, then each cell is a
> > rectangle of (W/COLS)x(H/ROWS) pixels (approximately as there may be some
> > rounding involved). Depending on the hardware each cell can have its own
> > properties. This matrix type sets the 'region' property which is a __u8.
> > Each region will typically have its own set of motion detection parameters
> > such as a threshold that determines the motion detection sensitivity. By
> > assigning each cell a region you can create regions with lower and regions
> > with higher motion sensitivity.
> 
> That sounds good to me. One more question, however: if the hardware divides 
> the sub-sampled image into regions, how do you configure per-region thresholds 
> ? The V4L2_MATRIX_T_MD_THRESHOLD matrix only configures per-cell thresholds.

That's hardware dependent. The go7007 has four different threshold parameters
per region, so that's a total of 16 controls for all four regions.

If we get more drivers doing motion detection in the future, then some of those
parameters might become standardized, but at the moment I have only one driver
and I don't want to standardize that as long as I don't know if it can be
standardized in the first place.

Regards,

	Hans

> 
> > > > +	    </entry>
> > > > +	  </row>
> > > > +	  <row>
> > > > +	    <entry><constant>V4L2_MATRIX_T_MD_THRESHOLD</constant></entry>
> > > > +	    <entry>Hardware motion detection can assign motion detection
> > > > threshold +	    values to each element of an image. Each element is a
> > > > __u16. +       Generally each element refers to a block of pixels in
> > > > the image.
> > This would be improved as well along the same lines:
> > 
> > Hardware motion detection divides the image up into cells. If the image
> > resolution is WxH and the matrix size is COLSxROWS, then each cell is a
> > rectangle of (W/COLS)x(H/ROWS) pixels (approximately as there may be some
> > rounding involved). Depending on the hardware each cell can have its own
> > motion detection sensitivity threshold. This matrix type sets the motion
> > detection threshold property which is a __u16.
> > > > +	    </entry>
> > > > +	  </row>
> > > > +	</tbody>
> > > > +      </tgroup>
> > > > +    </table>
> > > > +
> > > > +  </refsect1>
> > > > +  <refsect1>
> > > > +    &return-value;
> > > > +  </refsect1>
> > > > +</refentry>
> 
> 

[RFCv2 PATCH 10/10] go7007: add motion detection support.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This patch adds motion detection support to the go7007 driver using the new
motion detection controls, events and matrices.

The global motion detection works fine, but the regional motion detection
support probably needs more work. There seems to be some interaction between
regions that makes setting correct thresholds difficult. The exact meaning of
the thresholds isn't entirely clear either.

I do not have any documentation, the only information I have is the custom code
in the driver and a modet.c application.

My suspicion is that the internal motion detection bitmap is only updated for
a region if motion is detected for that region. This means that additional work
has to be done to check if the motion bits for a region have changed, and if not,
then that region should be discarded from the region_mask.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/staging/media/go7007/go7007-driver.c  | 119 +++++---
 drivers/staging/media/go7007/go7007-fw.c      |  28 +-
 drivers/staging/media/go7007/go7007-priv.h    |  16 ++
 drivers/staging/media/go7007/go7007-v4l2.c    | 382 +++++++++++++++++++-------
 drivers/staging/media/go7007/go7007.h         |  40 ---
 drivers/staging/media/go7007/saa7134-go7007.c |   1 -
 6 files changed, 403 insertions(+), 183 deletions(-)
 delete mode 100644 drivers/staging/media/go7007/go7007.h

diff --git a/drivers/staging/media/go7007/go7007-driver.c b/drivers/staging/media/go7007/go7007-driver.c
index 3640df0..8e1a04f 100644
--- a/drivers/staging/media/go7007/go7007-driver.c
+++ b/drivers/staging/media/go7007/go7007-driver.c
@@ -33,6 +33,7 @@
 #include <linux/videodev2.h>
 #include <media/tuner.h>
 #include <media/v4l2-common.h>
+#include <media/v4l2-event.h>
 
 #include "go7007-priv.h"
 
@@ -333,20 +334,33 @@ EXPORT_SYMBOL(go7007_register_encoder);
 int go7007_start_encoder(struct go7007 *go)
 {
 	u8 *fw;
-	int fw_len, rv = 0, i;
+	int fw_len, rv = 0, i, x, y;
 	u16 intr_val, intr_data;
 
 	go->modet_enable = 0;
-	if (!go->dvd_mode)
-		for (i = 0; i < 4; ++i) {
-			if (go->modet[i].enable) {
-				go->modet_enable = 1;
-				continue;
+	for (i = 0; i < 4; i++)
+		go->modet[i].enable = 0;
+
+	switch (v4l2_ctrl_g_ctrl(go->modet_mode)) {
+	case V4L2_DETECT_MOTION_GLOBAL:
+		memset(go->modet_map, 0, sizeof(go->modet_map));
+		go->modet[0].enable = 1;
+		go->modet_enable = 1;
+		break;
+	case V4L2_DETECT_MOTION_REGIONAL:
+		for (y = 0; y < go->height / 16; y++) {
+			for (x = 0; x < go->width / 16; x++) {
+				int idx = y * go->width / 16 + x;
+
+				go->modet[go->modet_map[idx]].enable = 1;
 			}
-			go->modet[i].pixel_threshold = 32767;
-			go->modet[i].motion_threshold = 32767;
-			go->modet[i].mb_threshold = 32767;
 		}
+		go->modet_enable = 1;
+		break;
+	}
+
+	if (go->dvd_mode)
+		go->modet_enable = 0;
 
 	if (go7007_construct_fw_image(go, &fw, &fw_len) < 0)
 		return -1;
@@ -385,43 +399,80 @@ static inline void store_byte(struct go7007_buffer *vb, u8 byte)
 }
 
 /*
- * Deliver the last video buffer and get a new one to start writing to.
+ * Determine regions with motion and send a motion detection event
+ * in case of changes.
  */
-static struct go7007_buffer *frame_boundary(struct go7007 *go, struct go7007_buffer *vb)
+static void go7007_motion_regions(struct go7007 *go, struct go7007_buffer *vb)
 {
-	struct go7007_buffer *vb_tmp = NULL;
 	u32 *bytesused = &vb->vb.v4l2_planes[0].bytesused;
+	unsigned motion[4] = { 0, 0, 0, 0 };
+	u32 motion_regions = 0;
+	unsigned stride = (go->width + 7) >> 3;
+	unsigned x, y;
 	int i;
 
-	if (vb) {
-		if (vb->modet_active) {
-			if (*bytesused + 216 < GO7007_BUF_SIZE) {
-				for (i = 0; i < 216; ++i)
-					store_byte(vb, go->active_map[i]);
-				*bytesused -= 216;
-			} else
-				vb->modet_active = 0;
+	for (i = 0; i < 216; ++i)
+		store_byte(vb, go->active_map[i]);
+	for (y = 0; y < go->height / 16; y++) {
+		for (x = 0; x < go->width / 16; x++) {
+			if (!(go->active_map[y * stride + (x >> 3)] & (1 << (x & 7))))
+				continue;
+			motion[go->modet_map[y * (go->width / 16) + x]]++;
 		}
-		vb->vb.v4l2_buf.sequence = go->next_seq++;
-		v4l2_get_timestamp(&vb->vb.v4l2_buf.timestamp);
-		vb_tmp = vb;
+	}
+	motion_regions = ((motion[0] > 0) << 0) |
+			 ((motion[1] > 0) << 1) |
+			 ((motion[2] > 0) << 2) |
+			 ((motion[3] > 0) << 3);
+	*bytesused -= 216;
+	if (motion_regions != go->modet_event_status) {
+		struct v4l2_event ev = {
+			.type = V4L2_EVENT_MOTION_DET,
+			.u.motion_det = {
+				.flags = V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ,
+				.frame_sequence = vb->vb.v4l2_buf.sequence,
+				.region_mask = motion_regions,
+			},
+		};
+
+		v4l2_event_queue(&go->vdev, &ev);
+		go->modet_event_status = motion_regions;
+	}
+}
+
+/*
+ * Deliver the last video buffer and get a new one to start writing to.
+ */
+static struct go7007_buffer *frame_boundary(struct go7007 *go, struct go7007_buffer *vb)
+{
+	u32 *bytesused = &vb->vb.v4l2_planes[0].bytesused;
+	struct go7007_buffer *vb_tmp = NULL;
+
+	if (vb == NULL) {
 		spin_lock(&go->spinlock);
-		list_del(&vb->list);
-		if (list_empty(&go->vidq_active))
-			vb = NULL;
-		else
-			vb = list_first_entry(&go->vidq_active, struct go7007_buffer, list);
-		go->active_buf = vb;
+		if (!list_empty(&go->vidq_active))
+			vb = go->active_buf =
+				list_first_entry(&go->vidq_active, struct go7007_buffer, list);
 		spin_unlock(&go->spinlock);
-		vb2_buffer_done(&vb_tmp->vb, VB2_BUF_STATE_DONE);
+		go->next_seq++;
 		return vb;
 	}
+
+	vb->vb.v4l2_buf.sequence = go->next_seq++;
+	if (vb->modet_active && *bytesused + 216 < GO7007_BUF_SIZE)
+		go7007_motion_regions(go, vb);
+
+	v4l2_get_timestamp(&vb->vb.v4l2_buf.timestamp);
+	vb_tmp = vb;
 	spin_lock(&go->spinlock);
-	if (!list_empty(&go->vidq_active))
-		vb = go->active_buf =
-			list_first_entry(&go->vidq_active, struct go7007_buffer, list);
+	list_del(&vb->list);
+	if (list_empty(&go->vidq_active))
+		vb = NULL;
+	else
+		vb = list_first_entry(&go->vidq_active, struct go7007_buffer, list);
+	go->active_buf = vb;
 	spin_unlock(&go->spinlock);
-	go->next_seq++;
+	vb2_buffer_done(&vb_tmp->vb, VB2_BUF_STATE_DONE);
 	return vb;
 }
 
diff --git a/drivers/staging/media/go7007/go7007-fw.c b/drivers/staging/media/go7007/go7007-fw.c
index c2d0e58af..43cb634 100644
--- a/drivers/staging/media/go7007/go7007-fw.c
+++ b/drivers/staging/media/go7007/go7007-fw.c
@@ -1432,22 +1432,26 @@ static int audio_to_package(struct go7007 *go, __le16 *code, int space)
 
 static int modet_to_package(struct go7007 *go, __le16 *code, int space)
 {
+	bool has_modet0 = go->modet[0].enable;
+	bool has_modet1 = go->modet[1].enable;
+	bool has_modet2 = go->modet[2].enable;
+	bool has_modet3 = go->modet[3].enable;
 	int ret, mb, i, addr, cnt = 0;
 	u16 pack[32];
 	u16 thresholds[] = {
 		0x200e,		0,
-		0xbf82,		go->modet[0].pixel_threshold,
-		0xbf83,		go->modet[1].pixel_threshold,
-		0xbf84,		go->modet[2].pixel_threshold,
-		0xbf85,		go->modet[3].pixel_threshold,
-		0xbf86,		go->modet[0].motion_threshold,
-		0xbf87,		go->modet[1].motion_threshold,
-		0xbf88,		go->modet[2].motion_threshold,
-		0xbf89,		go->modet[3].motion_threshold,
-		0xbf8a,		go->modet[0].mb_threshold,
-		0xbf8b,		go->modet[1].mb_threshold,
-		0xbf8c,		go->modet[2].mb_threshold,
-		0xbf8d,		go->modet[3].mb_threshold,
+		0xbf82,		has_modet0 ? go->modet[0].pixel_threshold : 32767,
+		0xbf83,		has_modet1 ? go->modet[1].pixel_threshold : 32767,
+		0xbf84,		has_modet2 ? go->modet[2].pixel_threshold : 32767,
+		0xbf85,		has_modet3 ? go->modet[3].pixel_threshold : 32767,
+		0xbf86,		has_modet0 ? go->modet[0].motion_threshold : 32767,
+		0xbf87,		has_modet1 ? go->modet[1].motion_threshold : 32767,
+		0xbf88,		has_modet2 ? go->modet[2].motion_threshold : 32767,
+		0xbf89,		has_modet3 ? go->modet[3].motion_threshold : 32767,
+		0xbf8a,		has_modet0 ? go->modet[0].mb_threshold : 32767,
+		0xbf8b,		has_modet1 ? go->modet[1].mb_threshold : 32767,
+		0xbf8c,		has_modet2 ? go->modet[2].mb_threshold : 32767,
+		0xbf8d,		has_modet3 ? go->modet[3].mb_threshold : 32767,
 		0xbf8e,		0,
 		0xbf8f,		0,
 		0,		0,
diff --git a/drivers/staging/media/go7007/go7007-priv.h b/drivers/staging/media/go7007/go7007-priv.h
index 6e16af7..a8aefed 100644
--- a/drivers/staging/media/go7007/go7007-priv.h
+++ b/drivers/staging/media/go7007/go7007-priv.h
@@ -75,6 +75,20 @@ struct go7007;
 #define GO7007_AUDIO_I2S_MASTER		(1<<16)
 #define GO7007_AUDIO_OKI_MODE		(1<<17)
 
+#define GO7007_CID_CUSTOM_BASE		(V4L2_CID_DETECT_CLASS_BASE + 0x1000)
+#define V4L2_CID_PIXEL_THRESHOLD0	(GO7007_CID_CUSTOM_BASE+1)
+#define V4L2_CID_MOTION_THRESHOLD0	(GO7007_CID_CUSTOM_BASE+2)
+#define V4L2_CID_MB_THRESHOLD0		(GO7007_CID_CUSTOM_BASE+3)
+#define V4L2_CID_PIXEL_THRESHOLD1	(GO7007_CID_CUSTOM_BASE+4)
+#define V4L2_CID_MOTION_THRESHOLD1	(GO7007_CID_CUSTOM_BASE+5)
+#define V4L2_CID_MB_THRESHOLD1		(GO7007_CID_CUSTOM_BASE+6)
+#define V4L2_CID_PIXEL_THRESHOLD2	(GO7007_CID_CUSTOM_BASE+7)
+#define V4L2_CID_MOTION_THRESHOLD2	(GO7007_CID_CUSTOM_BASE+8)
+#define V4L2_CID_MB_THRESHOLD2		(GO7007_CID_CUSTOM_BASE+9)
+#define V4L2_CID_PIXEL_THRESHOLD3	(GO7007_CID_CUSTOM_BASE+10)
+#define V4L2_CID_MOTION_THRESHOLD3	(GO7007_CID_CUSTOM_BASE+11)
+#define V4L2_CID_MB_THRESHOLD3		(GO7007_CID_CUSTOM_BASE+12)
+
 struct go7007_board_info {
 	unsigned int flags;
 	int hpi_buffer_cap;
@@ -168,6 +182,7 @@ struct go7007 {
 	struct v4l2_ctrl *mpeg_video_aspect_ratio;
 	struct v4l2_ctrl *mpeg_video_b_frames;
 	struct v4l2_ctrl *mpeg_video_rep_seqheader;
+	struct v4l2_ctrl *modet_mode;
 	enum { STATUS_INIT, STATUS_ONLINE, STATUS_SHUTDOWN } status;
 	spinlock_t spinlock;
 	struct mutex hw_lock;
@@ -216,6 +231,7 @@ struct go7007 {
 	} modet[4];
 	unsigned char modet_map[1624];
 	unsigned char active_map[216];
+	u32 modet_event_status;
 
 	/* Video streaming */
 	struct mutex queue_lock;
diff --git a/drivers/staging/media/go7007/go7007-v4l2.c b/drivers/staging/media/go7007/go7007-v4l2.c
index 50eb69a..ebf8f73 100644
--- a/drivers/staging/media/go7007/go7007-v4l2.c
+++ b/drivers/staging/media/go7007/go7007-v4l2.c
@@ -37,7 +37,6 @@
 #include <media/videobuf2-vmalloc.h>
 #include <media/saa7115.h>
 
-#include "go7007.h"
 #include "go7007-priv.h"
 
 #define call_all(dev, o, f, args...) \
@@ -190,7 +189,7 @@ static void set_formatting(struct go7007 *go)
 static int set_capture_size(struct go7007 *go, struct v4l2_format *fmt, int try)
 {
 	int sensor_height = 0, sensor_width = 0;
-	int width, height, i;
+	int width, height;
 
 	if (fmt != NULL && !valid_pixelformat(fmt->fmt.pix.pixelformat))
 		return -EINVAL;
@@ -254,10 +253,6 @@ static int set_capture_size(struct go7007 *go, struct v4l2_format *fmt, int try)
 	go->height = height;
 	go->encoder_h_offset = go->board_info->sensor_h_offset;
 	go->encoder_v_offset = go->board_info->sensor_v_offset;
-	for (i = 0; i < 4; ++i)
-		go->modet[i].enable = 0;
-	for (i = 0; i < 1624; ++i)
-		go->modet_map[i] = 0;
 
 	if (go->board_info->sensor_flags & GO7007_SENSOR_SCALING) {
 		struct v4l2_mbus_framefmt mbus_fmt;
@@ -287,64 +282,6 @@ static int set_capture_size(struct go7007 *go, struct v4l2_format *fmt, int try)
 	return 0;
 }
 
-#if 0
-static int clip_to_modet_map(struct go7007 *go, int region,
-		struct v4l2_clip *clip_list)
-{
-	struct v4l2_clip clip, *clip_ptr;
-	int x, y, mbnum;
-
-	/* Check if coordinates are OK and if any macroblocks are already
-	 * used by other regions (besides 0) */
-	clip_ptr = clip_list;
-	while (clip_ptr) {
-		if (copy_from_user(&clip, clip_ptr, sizeof(clip)))
-			return -EFAULT;
-		if (clip.c.left < 0 || (clip.c.left & 0xF) ||
-				clip.c.width <= 0 || (clip.c.width & 0xF))
-			return -EINVAL;
-		if (clip.c.left + clip.c.width > go->width)
-			return -EINVAL;
-		if (clip.c.top < 0 || (clip.c.top & 0xF) ||
-				clip.c.height <= 0 || (clip.c.height & 0xF))
-			return -EINVAL;
-		if (clip.c.top + clip.c.height > go->height)
-			return -EINVAL;
-		for (y = 0; y < clip.c.height; y += 16)
-			for (x = 0; x < clip.c.width; x += 16) {
-				mbnum = (go->width >> 4) *
-						((clip.c.top + y) >> 4) +
-					((clip.c.left + x) >> 4);
-				if (go->modet_map[mbnum] != 0 &&
-						go->modet_map[mbnum] != region)
-					return -EBUSY;
-			}
-		clip_ptr = clip.next;
-	}
-
-	/* Clear old region macroblocks */
-	for (mbnum = 0; mbnum < 1624; ++mbnum)
-		if (go->modet_map[mbnum] == region)
-			go->modet_map[mbnum] = 0;
-
-	/* Claim macroblocks in this list */
-	clip_ptr = clip_list;
-	while (clip_ptr) {
-		if (copy_from_user(&clip, clip_ptr, sizeof(clip)))
-			return -EFAULT;
-		for (y = 0; y < clip.c.height; y += 16)
-			for (x = 0; x < clip.c.width; x += 16) {
-				mbnum = (go->width >> 4) *
-						((clip.c.top + y) >> 4) +
-					((clip.c.left + x) >> 4);
-				go->modet_map[mbnum] = region;
-			}
-		clip_ptr = clip.next;
-	}
-	return 0;
-}
-#endif
-
 static int vidioc_querycap(struct file *file, void  *priv,
 					struct v4l2_capability *cap)
 {
@@ -496,6 +433,7 @@ static int go7007_start_streaming(struct vb2_queue *q, unsigned int count)
 	mutex_lock(&go->hw_lock);
 	go->next_seq = 0;
 	go->active_buf = NULL;
+	go->modet_event_status = 0;
 	q->streaming = 1;
 	if (go7007_start_encoder(go) < 0)
 		ret = -EIO;
@@ -849,41 +787,149 @@ static int vidioc_log_status(struct file *file, void *priv)
 	return call_all(&go->v4l2_dev, core, log_status);
 }
 
-/* FIXME:
-	Those ioctls are private, and not needed, since several standard
-	extended controls already provide streaming control.
-	So, those ioctls should be converted into vidioc_g_ext_ctrls()
-	and vidioc_s_ext_ctrls()
- */
+static int vidioc_subscribe_event(struct v4l2_fh *fh,
+				const struct v4l2_event_subscription *sub)
+{
 
-#if 0
-	case GO7007IOC_S_MD_PARAMS:
-	{
-		struct go7007_md_params *mdp = arg;
+	switch (sub->type) {
+	case V4L2_EVENT_CTRL:
+		return v4l2_ctrl_subscribe_event(fh, sub);
+	case V4L2_EVENT_MOTION_DET:
+		/* Allow for up to 30 events (1 second for NTSC) to be
+		 * stored. */
+		return v4l2_event_subscribe(fh, sub, 30, NULL);
+	}
+	return -EINVAL;
+}
 
-		if (mdp->region > 3)
-			return -EINVAL;
-		if (mdp->trigger > 0) {
-			go->modet[mdp->region].pixel_threshold =
-					mdp->pixel_threshold >> 1;
-			go->modet[mdp->region].motion_threshold =
-					mdp->motion_threshold >> 1;
-			go->modet[mdp->region].mb_threshold =
-					mdp->trigger >> 1;
-			go->modet[mdp->region].enable = 1;
-		} else
-			go->modet[mdp->region].enable = 0;
-		/* fall-through */
+static int vidioc_query_matrix(struct file *file, void *fh,
+			struct v4l2_query_matrix *qm)
+{
+	struct go7007 *go = video_drvdata(file);
+
+	qm->columns = go->width / 16;
+	qm->rows = go->height / 16;
+	switch (qm->type) {
+	case V4L2_MATRIX_T_MD_REGION:
+		qm->elem_size = 1;
+		qm->elem_max.uval = 3;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int vidioc_g_matrix(struct file *file, void *fh,
+			struct v4l2_matrix *m)
+{
+	struct go7007 *go = video_drvdata(file);
+	int w = m->rect.width;
+	int h = m->rect.height;
+	u8 *mt;
+	int y;
+
+	if (m->rect.top < 0 || m->rect.top + h > go->height / 16 || h < 0 || w < 0 ||
+	    m->rect.left < 0 || m->rect.left + w > go->width / 16) {
+		return -EINVAL;
 	}
-	case GO7007IOC_S_MD_REGION:
-	{
-		struct go7007_md_region *region = arg;
+	if (h == 0 || w == 0)
+		return 0;
 
-		if (region->region < 1 || region->region > 3)
-			return -EINVAL;
-		return clip_to_modet_map(go, region->region, region->clips);
+	switch (m->type) {
+	case V4L2_MATRIX_T_MD_REGION:
+		mt = go->modet_map + m->rect.top * (go->width / 16) + m->rect.left;
+		for (y = 0; y < h; y++, mt += go->width / 16)
+			if (copy_to_user(m->matrix + y * w, mt, w))
+				return -EFAULT;
+		break;
+	default:
+		return -EINVAL;
 	}
-#endif
+	return 0;
+}
+
+static int vidioc_s_matrix(struct file *file, void *fh,
+			struct v4l2_matrix *m)
+{
+	struct go7007 *go = video_drvdata(file);
+	int w = m->rect.width;
+	int h = m->rect.height;
+	u8 *mt;
+	u8 *new_matrix = m->matrix;
+	int y;
+	int i;
+
+	if (m->rect.top < 0 || m->rect.top + h > go->height / 16 || h < 0 || w < 0 ||
+	    m->rect.left < 0 || m->rect.left + w > go->width / 16)
+		return -EINVAL;
+	if (h == 0 || w == 0)
+		return 0;
+
+	switch (m->type) {
+	case V4L2_MATRIX_T_MD_REGION:
+		mt = go->modet_map + m->rect.top * (go->width / 16) + m->rect.left;
+		for (i = 0; i < h * w; i++)
+			if (new_matrix[i] > 3)
+				return -EINVAL;
+		for (y = 0; y < h; y++, mt += go->width / 16)
+			if (copy_from_user(mt, new_matrix + y * w, w))
+				return -EFAULT;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+
+static int go7007_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct go7007 *go =
+		container_of(ctrl->handler, struct go7007, hdl);
+
+	switch (ctrl->id) {
+	case V4L2_CID_PIXEL_THRESHOLD0:
+		go->modet[0].pixel_threshold = ctrl->val;
+		break;
+	case V4L2_CID_MOTION_THRESHOLD0:
+		go->modet[0].motion_threshold = ctrl->val;
+		break;
+	case V4L2_CID_MB_THRESHOLD0:
+		go->modet[0].mb_threshold = ctrl->val;
+		break;
+	case V4L2_CID_PIXEL_THRESHOLD1:
+		go->modet[1].pixel_threshold = ctrl->val;
+		break;
+	case V4L2_CID_MOTION_THRESHOLD1:
+		go->modet[1].motion_threshold = ctrl->val;
+		break;
+	case V4L2_CID_MB_THRESHOLD1:
+		go->modet[1].mb_threshold = ctrl->val;
+		break;
+	case V4L2_CID_PIXEL_THRESHOLD2:
+		go->modet[2].pixel_threshold = ctrl->val;
+		break;
+	case V4L2_CID_MOTION_THRESHOLD2:
+		go->modet[2].motion_threshold = ctrl->val;
+		break;
+	case V4L2_CID_MB_THRESHOLD2:
+		go->modet[2].mb_threshold = ctrl->val;
+		break;
+	case V4L2_CID_PIXEL_THRESHOLD3:
+		go->modet[3].pixel_threshold = ctrl->val;
+		break;
+	case V4L2_CID_MOTION_THRESHOLD3:
+		go->modet[3].motion_threshold = ctrl->val;
+		break;
+	case V4L2_CID_MB_THRESHOLD3:
+		go->modet[3].mb_threshold = ctrl->val;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
 
 static struct v4l2_file_operations go7007_fops = {
 	.owner		= THIS_MODULE,
@@ -925,7 +971,11 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_enum_framesizes   = vidioc_enum_framesizes,
 	.vidioc_enum_frameintervals = vidioc_enum_frameintervals,
 	.vidioc_log_status        = vidioc_log_status,
-	.vidioc_subscribe_event   = v4l2_ctrl_subscribe_event,
+	/* Motion Detection matrices */
+	.vidioc_query_matrix	  = vidioc_query_matrix,
+	.vidioc_g_matrix	  = vidioc_g_matrix,
+	.vidioc_s_matrix	  = vidioc_s_matrix,
+	.vidioc_subscribe_event   = vidioc_subscribe_event,
 	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
 };
 
@@ -937,12 +987,136 @@ static struct video_device go7007_template = {
 	.tvnorms	= V4L2_STD_ALL,
 };
 
+static const struct v4l2_ctrl_ops go7007_ctrl_ops = {
+	.s_ctrl = go7007_s_ctrl,
+};
+
+static const struct v4l2_ctrl_config go7007_pixel_threshold0_ctrl = {
+	.ops = &go7007_ctrl_ops,
+	.id = V4L2_CID_PIXEL_THRESHOLD0,
+	.name = "Pixel Threshold Region 0",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.def = 50,
+	.max = 32767,
+	.step = 1,
+};
+
+static const struct v4l2_ctrl_config go7007_motion_threshold0_ctrl = {
+	.ops = &go7007_ctrl_ops,
+	.id = V4L2_CID_MOTION_THRESHOLD0,
+	.name = "Motion Threshold Region 0",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.def = 4000,
+	.max = 32767,
+	.step = 1,
+};
+
+static const struct v4l2_ctrl_config go7007_mb_threshold0_ctrl = {
+	.ops = &go7007_ctrl_ops,
+	.id = V4L2_CID_MB_THRESHOLD0,
+	.name = "MB Threshold Region 0",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.def = 10,
+	.max = 32767,
+	.step = 1,
+};
+
+static const struct v4l2_ctrl_config go7007_pixel_threshold1_ctrl = {
+	.ops = &go7007_ctrl_ops,
+	.id = V4L2_CID_PIXEL_THRESHOLD1,
+	.name = "Pixel Threshold Region 1",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.def = 50,
+	.max = 32767,
+	.step = 1,
+};
+
+static const struct v4l2_ctrl_config go7007_motion_threshold1_ctrl = {
+	.ops = &go7007_ctrl_ops,
+	.id = V4L2_CID_MOTION_THRESHOLD1,
+	.name = "Motion Threshold Region 1",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.def = 4000,
+	.max = 32767,
+	.step = 1,
+};
+
+static const struct v4l2_ctrl_config go7007_mb_threshold1_ctrl = {
+	.ops = &go7007_ctrl_ops,
+	.id = V4L2_CID_MB_THRESHOLD1,
+	.name = "MB Threshold Region 1",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.def = 10,
+	.max = 32767,
+	.step = 1,
+};
+
+static const struct v4l2_ctrl_config go7007_pixel_threshold2_ctrl = {
+	.ops = &go7007_ctrl_ops,
+	.id = V4L2_CID_PIXEL_THRESHOLD2,
+	.name = "Pixel Threshold Region 2",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.def = 50,
+	.max = 32767,
+	.step = 1,
+};
+
+static const struct v4l2_ctrl_config go7007_motion_threshold2_ctrl = {
+	.ops = &go7007_ctrl_ops,
+	.id = V4L2_CID_MOTION_THRESHOLD2,
+	.name = "Motion Threshold Region 2",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.def = 4000,
+	.max = 32767,
+	.step = 1,
+};
+
+static const struct v4l2_ctrl_config go7007_mb_threshold2_ctrl = {
+	.ops = &go7007_ctrl_ops,
+	.id = V4L2_CID_MB_THRESHOLD2,
+	.name = "MB Threshold Region 2",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.def = 10,
+	.max = 32767,
+	.step = 1,
+};
+
+static const struct v4l2_ctrl_config go7007_pixel_threshold3_ctrl = {
+	.ops = &go7007_ctrl_ops,
+	.id = V4L2_CID_PIXEL_THRESHOLD3,
+	.name = "Pixel Threshold Region 3",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.def = 50,
+	.max = 32767,
+	.step = 1,
+};
+
+static const struct v4l2_ctrl_config go7007_motion_threshold3_ctrl = {
+	.ops = &go7007_ctrl_ops,
+	.id = V4L2_CID_MOTION_THRESHOLD3,
+	.name = "Motion Threshold Region 3",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.def = 4000,
+	.max = 32767,
+	.step = 1,
+};
+
+static const struct v4l2_ctrl_config go7007_mb_threshold3_ctrl = {
+	.ops = &go7007_ctrl_ops,
+	.id = V4L2_CID_MB_THRESHOLD3,
+	.name = "MB Threshold Region 3",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.def = 10,
+	.max = 32767,
+	.step = 1,
+};
+
 int go7007_v4l2_ctrl_init(struct go7007 *go)
 {
 	struct v4l2_ctrl_handler *hdl = &go->hdl;
 	struct v4l2_ctrl *ctrl;
 
-	v4l2_ctrl_handler_init(hdl, 13);
+	v4l2_ctrl_handler_init(hdl, 22);
 	go->mpeg_video_gop_size = v4l2_ctrl_new_std(hdl, NULL,
 			V4L2_CID_MPEG_VIDEO_GOP_SIZE, 0, 34, 1, 15);
 	go->mpeg_video_gop_closure = v4l2_ctrl_new_std(hdl, NULL,
@@ -965,6 +1139,22 @@ int go7007_v4l2_ctrl_init(struct go7007 *go)
 			V4L2_JPEG_ACTIVE_MARKER_DQT | V4L2_JPEG_ACTIVE_MARKER_DHT);
 	if (ctrl)
 		ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+	v4l2_ctrl_new_custom(hdl, &go7007_pixel_threshold0_ctrl, NULL);
+	v4l2_ctrl_new_custom(hdl, &go7007_motion_threshold0_ctrl, NULL);
+	v4l2_ctrl_new_custom(hdl, &go7007_mb_threshold0_ctrl, NULL);
+	v4l2_ctrl_new_custom(hdl, &go7007_pixel_threshold1_ctrl, NULL);
+	v4l2_ctrl_new_custom(hdl, &go7007_motion_threshold1_ctrl, NULL);
+	v4l2_ctrl_new_custom(hdl, &go7007_mb_threshold1_ctrl, NULL);
+	v4l2_ctrl_new_custom(hdl, &go7007_pixel_threshold2_ctrl, NULL);
+	v4l2_ctrl_new_custom(hdl, &go7007_motion_threshold2_ctrl, NULL);
+	v4l2_ctrl_new_custom(hdl, &go7007_mb_threshold2_ctrl, NULL);
+	v4l2_ctrl_new_custom(hdl, &go7007_pixel_threshold3_ctrl, NULL);
+	v4l2_ctrl_new_custom(hdl, &go7007_motion_threshold3_ctrl, NULL);
+	v4l2_ctrl_new_custom(hdl, &go7007_mb_threshold3_ctrl, NULL);
+	go->modet_mode = v4l2_ctrl_new_std_menu(hdl, NULL,
+			V4L2_CID_DETECT_MOTION_MODE,
+			V4L2_DETECT_MOTION_REGIONAL, 0,
+			V4L2_DETECT_MOTION_DISABLED);
 	if (hdl->error) {
 		int rv = hdl->error;
 
diff --git a/drivers/staging/media/go7007/go7007.h b/drivers/staging/media/go7007/go7007.h
deleted file mode 100644
index 54b9897..0000000
--- a/drivers/staging/media/go7007/go7007.h
+++ /dev/null
@@ -1,40 +0,0 @@
-/*
- * Copyright (C) 2005-2006 Micronas USA Inc.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and the associated README documentation file (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sublicense, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
- * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
- * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
- * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
- * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
- * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- */
-
-struct go7007_md_params {
-	__u16 region;
-	__u16 trigger;
-	__u16 pixel_threshold;
-	__u16 motion_threshold;
-	__u32 reserved[8];
-};
-
-struct go7007_md_region {
-	__u16 region;
-	__u16 flags;
-	struct v4l2_clip *clips;
-	__u32 reserved[8];
-};
-
-#define	GO7007IOC_S_MD_PARAMS	_IOWR('V', BASE_VIDIOC_PRIVATE + 6, \
-					struct go7007_md_params)
-#define	GO7007IOC_G_MD_PARAMS	_IOR('V', BASE_VIDIOC_PRIVATE + 7, \
-					struct go7007_md_params)
-#define	GO7007IOC_S_MD_REGION	_IOW('V', BASE_VIDIOC_PRIVATE + 8, \
-					struct go7007_md_region)
diff --git a/drivers/staging/media/go7007/saa7134-go7007.c b/drivers/staging/media/go7007/saa7134-go7007.c
index d80b235..e0eab32 100644
--- a/drivers/staging/media/go7007/saa7134-go7007.c
+++ b/drivers/staging/media/go7007/saa7134-go7007.c
@@ -33,7 +33,6 @@
 
 #include "saa7134.h"
 #include "saa7134-reg.h"
-#include "go7007.h"
 #include "go7007-priv.h"
 
 /*#define GO7007_HPI_DEBUG*/
-- 
1.8.3.2