[PATCH v4 0/8] adv7180 subdev fixes, v4

[PATCH v4 0/8] adv7180 subdev fixes, v4

[Steve Longerbeam]

Steve Longerbeam (8):
  media: adv7180: fix field type
  media: adv7180: define more registers
  media: adv7180: add support for NEWAVMODE
  media: adv7180: add power pin control
  media: adv7180: implement g_parm
  media: adv7180: change mbus format to UYVY
  v4l: Add signal lock status to source change events
  media: adv7180: enable lock/unlock interrupts

 .../devicetree/bindings/media/i2c/adv7180.txt      |   8 +
 Documentation/media/uapi/v4l/vidioc-dqevent.rst    |   9 +
 Documentation/media/videodev2.h.rst.exceptions     |   1 +
 drivers/media/i2c/Kconfig                          |   2 +-
 drivers/media/i2c/adv7180.c                        | 200 +++++++++++++++++----
 include/uapi/linux/videodev2.h                     |   1 +
 6 files changed, 183 insertions(+), 38 deletions(-)

-- 
1.9.1

[PATCH v4 1/8] media: adv7180: fix field type

[Steve Longerbeam]

From: Steve Longerbeam <slongerbeam@gmail.com>

The ADV7180 and ADV7182 transmit whole fields, bottom field followed
by top (or vice-versa, depending on detected video standard). So
for chips that do not have support for explicitly setting the field
mode via I2P, set the field mode to V4L2_FIELD_ALTERNATE.

Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

---

v4:
- switch V4L2_FIELD_SEQ_TB/V4L2_FIELD_SEQ_BT to V4L2_FIELD_ALTERNATE.
  This is from Niklas Söderlund.
- remove checks for ADV7180_FLAG_I2P when setting field mode, since I2P
  support is planned to be removed.
- move init of state->curr_norm back to its original location, since
  state->field init is no longer dependent on state->curr_norm.

v3: no changes

v2:
- the init of state->curr_norm in probe needs to be moved up, ahead
  of the init of state->field.
---
 drivers/media/i2c/adv7180.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 95cbc85..192eeae 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -679,10 +679,10 @@ static int adv7180_set_pad_format(struct v4l2_subdev *sd,
 	switch (format->format.field) {
 	case V4L2_FIELD_NONE:
 		if (!(state->chip_info->flags & ADV7180_FLAG_I2P))
-			format->format.field = V4L2_FIELD_INTERLACED;
+			format->format.field = V4L2_FIELD_ALTERNATE;
 		break;
 	default:
-		format->format.field = V4L2_FIELD_INTERLACED;
+		format->format.field = V4L2_FIELD_ALTERNATE;
 		break;
 	}
 
@@ -1251,7 +1251,7 @@ static int adv7180_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	state->client = client;
-	state->field = V4L2_FIELD_INTERLACED;
+	state->field = V4L2_FIELD_ALTERNATE;
 	state->chip_info = (struct adv7180_chip_info *)id->driver_data;
 
 	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
-- 
1.9.1

Re: [PATCH v4 1/8] media: adv7180: fix field type

[Lars-Peter Clausen]

On 08/03/2016 08:03 PM, Steve Longerbeam wrote:
> From: Steve Longerbeam <slongerbeam@gmail.com>
> 
> The ADV7180 and ADV7182 transmit whole fields, bottom field followed
> by top (or vice-versa, depending on detected video standard). So
> for chips that do not have support for explicitly setting the field
> mode via I2P, set the field mode to V4L2_FIELD_ALTERNATE.
> 
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

Thanks.

Re: [PATCH v4 1/8] media: adv7180: fix field type

[Hans Verkuil]

On 08/03/2016 08:03 PM, Steve Longerbeam wrote:
> From: Steve Longerbeam <slongerbeam@gmail.com>
> 
> The ADV7180 and ADV7182 transmit whole fields, bottom field followed
> by top (or vice-versa, depending on detected video standard). So
> for chips that do not have support for explicitly setting the field
> mode via I2P, set the field mode to V4L2_FIELD_ALTERNATE.
> 
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> ---
> 
> v4:
> - switch V4L2_FIELD_SEQ_TB/V4L2_FIELD_SEQ_BT to V4L2_FIELD_ALTERNATE.
>   This is from Niklas Söderlund.
> - remove checks for ADV7180_FLAG_I2P when setting field mode, since I2P
>   support is planned to be removed.
> - move init of state->curr_norm back to its original location, since
>   state->field init is no longer dependent on state->curr_norm.
> 
> v3: no changes
> 
> v2:
> - the init of state->curr_norm in probe needs to be moved up, ahead
>   of the init of state->field.
> ---
>  drivers/media/i2c/adv7180.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index 95cbc85..192eeae 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -679,10 +679,10 @@ static int adv7180_set_pad_format(struct v4l2_subdev *sd,
>  	switch (format->format.field) {
>  	case V4L2_FIELD_NONE:
>  		if (!(state->chip_info->flags & ADV7180_FLAG_I2P))
> -			format->format.field = V4L2_FIELD_INTERLACED;
> +			format->format.field = V4L2_FIELD_ALTERNATE;
>  		break;

I'd change this to:

  		if (state->chip_info->flags & ADV7180_FLAG_I2P)
			break;
		/* fall through */

>  	default:
> -		format->format.field = V4L2_FIELD_INTERLACED;
> +		format->format.field = V4L2_FIELD_ALTERNATE;
>  		break;
>  	}
>  
> @@ -1251,7 +1251,7 @@ static int adv7180_probe(struct i2c_client *client,
>  		return -ENOMEM;
>  
>  	state->client = client;
> -	state->field = V4L2_FIELD_INTERLACED;
> +	state->field = V4L2_FIELD_ALTERNATE;
>  	state->chip_info = (struct adv7180_chip_info *)id->driver_data;
>  
>  	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
> 

Regards,

	Hans

[PATCH v4 2/8] media: adv7180: define more registers

[Steve Longerbeam]

Replace hard-coded addresses with new register macro defines. No
functional changes.

Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>

---
v4: no changes
v3: no changes
---
 drivers/media/i2c/adv7180.c | 73 ++++++++++++++++++++++++++++++---------------
 1 file changed, 49 insertions(+), 24 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 192eeae..6e093c22 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -56,10 +56,11 @@
 
 #define ADV7182_REG_INPUT_VIDSEL			0x0002
 
+#define ADV7180_REG_OUTPUT_CONTROL			0x0003
 #define ADV7180_REG_EXTENDED_OUTPUT_CONTROL		0x0004
 #define ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS		0xC5
 
-#define ADV7180_REG_AUTODETECT_ENABLE			0x07
+#define ADV7180_REG_AUTODETECT_ENABLE			0x0007
 #define ADV7180_AUTODETECT_DEFAULT			0x7f
 /* Contrast */
 #define ADV7180_REG_CON		0x0008	/*Unsigned */
@@ -100,6 +101,20 @@
 #define ADV7180_REG_IDENT 0x0011
 #define ADV7180_ID_7180 0x18
 
+#define ADV7180_REG_STATUS3		0x0013
+#define ADV7180_REG_ANALOG_CLAMP_CTL	0x0014
+#define ADV7180_REG_SHAP_FILTER_CTL_1	0x0017
+#define ADV7180_REG_CTRL_2		0x001d
+#define ADV7180_REG_VSYNC_FIELD_CTL_1	0x0031
+#define ADV7180_REG_MANUAL_WIN_CTL_1	0x003d
+#define ADV7180_REG_MANUAL_WIN_CTL_2	0x003e
+#define ADV7180_REG_MANUAL_WIN_CTL_3	0x003f
+#define ADV7180_REG_LOCK_CNT		0x0051
+#define ADV7180_REG_CVBS_TRIM		0x0052
+#define ADV7180_REG_CLAMP_ADJ		0x005a
+#define ADV7180_REG_RES_CIR		0x005f
+#define ADV7180_REG_DIFF_MODE		0x0060
+
 #define ADV7180_REG_ICONF1		0x2040
 #define ADV7180_ICONF1_ACTIVE_LOW	0x01
 #define ADV7180_ICONF1_PSYNC_ONLY	0x10
@@ -129,9 +144,15 @@
 #define ADV7180_REG_VPP_SLAVE_ADDR	0xFD
 #define ADV7180_REG_CSI_SLAVE_ADDR	0xFE
 
-#define ADV7180_REG_FLCONTROL 0x40e0
+#define ADV7180_REG_ACE_CTRL1		0x4080
+#define ADV7180_REG_ACE_CTRL5		0x4084
+#define ADV7180_REG_FLCONTROL		0x40e0
 #define ADV7180_FLCONTROL_FL_ENABLE 0x1
 
+#define ADV7180_REG_RST_CLAMP	0x809c
+#define ADV7180_REG_AGC_ADJ1	0x80b6
+#define ADV7180_REG_AGC_ADJ2	0x80c0
+
 #define ADV7180_CSI_REG_PWRDN	0x00
 #define ADV7180_CSI_PWRDN	0x80
 
@@ -886,16 +907,20 @@ static int adv7182_init(struct adv7180_state *state)
 
 	/* ADI required writes */
 	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
-		adv7180_write(state, 0x0003, 0x4e);
-		adv7180_write(state, 0x0004, 0x57);
-		adv7180_write(state, 0x001d, 0xc0);
+		adv7180_write(state, ADV7180_REG_OUTPUT_CONTROL, 0x4e);
+		adv7180_write(state, ADV7180_REG_EXTENDED_OUTPUT_CONTROL, 0x57);
+		adv7180_write(state, ADV7180_REG_CTRL_2, 0xc0);
 	} else {
 		if (state->chip_info->flags & ADV7180_FLAG_V2)
-			adv7180_write(state, 0x0004, 0x17);
+			adv7180_write(state,
+				      ADV7180_REG_EXTENDED_OUTPUT_CONTROL,
+				      0x17);
 		else
-			adv7180_write(state, 0x0004, 0x07);
-		adv7180_write(state, 0x0003, 0x0c);
-		adv7180_write(state, 0x001d, 0x40);
+			adv7180_write(state,
+				      ADV7180_REG_EXTENDED_OUTPUT_CONTROL,
+				      0x07);
+		adv7180_write(state, ADV7180_REG_OUTPUT_CONTROL, 0x0c);
+		adv7180_write(state, ADV7180_REG_CTRL_2, 0x40);
 	}
 
 	adv7180_write(state, 0x0013, 0x00);
@@ -972,8 +997,8 @@ static int adv7182_select_input(struct adv7180_state *state, unsigned int input)
 		return ret;
 
 	/* Reset clamp circuitry - ADI recommended writes */
-	adv7180_write(state, 0x809c, 0x00);
-	adv7180_write(state, 0x809c, 0xff);
+	adv7180_write(state, ADV7180_REG_RST_CLAMP, 0x00);
+	adv7180_write(state, ADV7180_REG_RST_CLAMP, 0xff);
 
 	input_type = adv7182_get_input_type(input);
 
@@ -981,10 +1006,10 @@ static int adv7182_select_input(struct adv7180_state *state, unsigned int input)
 	case ADV7182_INPUT_TYPE_CVBS:
 	case ADV7182_INPUT_TYPE_DIFF_CVBS:
 		/* ADI recommends to use the SH1 filter */
-		adv7180_write(state, 0x0017, 0x41);
+		adv7180_write(state, ADV7180_REG_SHAP_FILTER_CTL_1, 0x41);
 		break;
 	default:
-		adv7180_write(state, 0x0017, 0x01);
+		adv7180_write(state, ADV7180_REG_SHAP_FILTER_CTL_1, 0x01);
 		break;
 	}
 
@@ -994,21 +1019,21 @@ static int adv7182_select_input(struct adv7180_state *state, unsigned int input)
 		lbias = adv7182_lbias_settings[input_type];
 
 	for (i = 0; i < ARRAY_SIZE(adv7182_lbias_settings[0]); i++)
-		adv7180_write(state, 0x0052 + i, lbias[i]);
+		adv7180_write(state, ADV7180_REG_CVBS_TRIM + i, lbias[i]);
 
 	if (input_type == ADV7182_INPUT_TYPE_DIFF_CVBS) {
 		/* ADI required writes to make differential CVBS work */
-		adv7180_write(state, 0x005f, 0xa8);
-		adv7180_write(state, 0x005a, 0x90);
-		adv7180_write(state, 0x0060, 0xb0);
-		adv7180_write(state, 0x80b6, 0x08);
-		adv7180_write(state, 0x80c0, 0xa0);
+		adv7180_write(state, ADV7180_REG_RES_CIR, 0xa8);
+		adv7180_write(state, ADV7180_REG_CLAMP_ADJ, 0x90);
+		adv7180_write(state, ADV7180_REG_DIFF_MODE, 0xb0);
+		adv7180_write(state, ADV7180_REG_AGC_ADJ1, 0x08);
+		adv7180_write(state, ADV7180_REG_AGC_ADJ2, 0xa0);
 	} else {
-		adv7180_write(state, 0x005f, 0xf0);
-		adv7180_write(state, 0x005a, 0xd0);
-		adv7180_write(state, 0x0060, 0x10);
-		adv7180_write(state, 0x80b6, 0x9c);
-		adv7180_write(state, 0x80c0, 0x00);
+		adv7180_write(state, ADV7180_REG_RES_CIR, 0xf0);
+		adv7180_write(state, ADV7180_REG_CLAMP_ADJ, 0xd0);
+		adv7180_write(state, ADV7180_REG_DIFF_MODE, 0x10);
+		adv7180_write(state, ADV7180_REG_AGC_ADJ1, 0x9c);
+		adv7180_write(state, ADV7180_REG_AGC_ADJ2, 0x00);
 	}
 
 	return 0;
-- 
1.9.1

[PATCH v4 3/8] media: adv7180: add support for NEWAVMODE

[Steve Longerbeam]

Parse the optional v4l2 endpoint DT node. If the bus type is
V4L2_MBUS_BT656 and the endpoint node specifies "newavmode",
configure the BT.656 bus in NEWAVMODE.

Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>

---

v4: no changes
v3:
- the newavmode endpoint property is now private to adv7180.
---
 .../devicetree/bindings/media/i2c/adv7180.txt      |  4 ++
 drivers/media/i2c/adv7180.c                        | 46 ++++++++++++++++++++--
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/adv7180.txt b/Documentation/devicetree/bindings/media/i2c/adv7180.txt
index 0d50115..6c175d2 100644
--- a/Documentation/devicetree/bindings/media/i2c/adv7180.txt
+++ b/Documentation/devicetree/bindings/media/i2c/adv7180.txt
@@ -15,6 +15,10 @@ Required Properties :
 		"adi,adv7282"
 		"adi,adv7282-m"
 
+Optional Endpoint Properties :
+- newavmode: a boolean property to indicate the BT.656 bus is operating
+  in Analog Device's NEWAVMODE. Valid for BT.656 busses only.
+
 Example:
 
 	i2c0@1c22000 {
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 6e093c22..467953e 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -31,6 +31,7 @@
 #include <media/v4l2-event.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ctrls.h>
+#include <media/v4l2-of.h>
 #include <linux/mutex.h>
 #include <linux/delay.h>
 
@@ -106,6 +107,7 @@
 #define ADV7180_REG_SHAP_FILTER_CTL_1	0x0017
 #define ADV7180_REG_CTRL_2		0x001d
 #define ADV7180_REG_VSYNC_FIELD_CTL_1	0x0031
+#define ADV7180_VSYNC_FIELD_CTL_1_NEWAVMODE 0x02
 #define ADV7180_REG_MANUAL_WIN_CTL_1	0x003d
 #define ADV7180_REG_MANUAL_WIN_CTL_2	0x003e
 #define ADV7180_REG_MANUAL_WIN_CTL_3	0x003f
@@ -214,6 +216,7 @@ struct adv7180_state {
 	struct mutex		mutex; /* mutual excl. when accessing chip */
 	int			irq;
 	v4l2_std_id		curr_norm;
+	bool			newavmode;
 	bool			powered;
 	bool			streaming;
 	u8			input;
@@ -864,9 +867,15 @@ static int adv7180_init(struct adv7180_state *state)
 	if (ret < 0)
 		return ret;
 
-	/* Manually set V bit end position in NTSC mode */
-	return adv7180_write(state, ADV7180_REG_NTSC_V_BIT_END,
-					ADV7180_NTSC_V_BIT_END_MANUAL_NVEND);
+	if (!state->newavmode) {
+		/* Manually set V bit end position in NTSC mode */
+		ret = adv7180_write(state, ADV7180_REG_NTSC_V_BIT_END,
+				    ADV7180_NTSC_V_BIT_END_MANUAL_NVEND);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
 }
 
 static int adv7180_set_std(struct adv7180_state *state, unsigned int std)
@@ -1217,6 +1226,13 @@ static int init_device(struct adv7180_state *state)
 	if (ret)
 		goto out_unlock;
 
+	if (state->newavmode) {
+		ret = adv7180_write(state, ADV7180_REG_VSYNC_FIELD_CTL_1,
+				    ADV7180_VSYNC_FIELD_CTL_1_NEWAVMODE);
+		if (ret < 0)
+			goto out_unlock;
+	}
+
 	ret = adv7180_program_std(state);
 	if (ret)
 		goto out_unlock;
@@ -1257,6 +1273,28 @@ out_unlock:
 	return ret;
 }
 
+static void adv7180_of_parse(struct adv7180_state *state)
+{
+	struct i2c_client *client = state->client;
+	struct device_node *np = client->dev.of_node;
+	struct device_node *endpoint;
+	struct v4l2_of_endpoint	ep;
+
+	endpoint = of_graph_get_next_endpoint(np, NULL);
+	if (!endpoint) {
+		v4l_warn(client, "endpoint node not found\n");
+		return;
+	}
+
+	v4l2_of_parse_endpoint(endpoint, &ep);
+	if (ep.bus_type == V4L2_MBUS_BT656) {
+		if (of_property_read_bool(endpoint, "newavmode"))
+			state->newavmode = true;
+	}
+
+	of_node_put(endpoint);
+}
+
 static int adv7180_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -1279,6 +1317,8 @@ static int adv7180_probe(struct i2c_client *client,
 	state->field = V4L2_FIELD_ALTERNATE;
 	state->chip_info = (struct adv7180_chip_info *)id->driver_data;
 
+	adv7180_of_parse(state);
+
 	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
 		state->csi_client = i2c_new_dummy(client->adapter,
 				ADV7180_DEFAULT_CSI_I2C_ADDR);
-- 
1.9.1

Re: [PATCH v4 3/8] media: adv7180: add support for NEWAVMODE

[Laurent Pinchart]

Hi Steve,

Thank you for the patch.

On Wednesday 03 Aug 2016 11:03:45 Steve Longerbeam wrote:
> Parse the optional v4l2 endpoint DT node. If the bus type is
> V4L2_MBUS_BT656 and the endpoint node specifies "newavmode",
> configure the BT.656 bus in NEWAVMODE.
> 
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> 
> ---
> 
> v4: no changes
> v3:
> - the newavmode endpoint property is now private to adv7180.
> ---
>  .../devicetree/bindings/media/i2c/adv7180.txt      |  4 ++
>  drivers/media/i2c/adv7180.c                        | 46 +++++++++++++++++--
>  2 files changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/adv7180.txt
> b/Documentation/devicetree/bindings/media/i2c/adv7180.txt index
> 0d50115..6c175d2 100644
> --- a/Documentation/devicetree/bindings/media/i2c/adv7180.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/adv7180.txt
> @@ -15,6 +15,10 @@ Required Properties :
>  		"adi,adv7282"
>  		"adi,adv7282-m"
> 
> +Optional Endpoint Properties :
> +- newavmode: a boolean property to indicate the BT.656 bus is operating
> +  in Analog Device's NEWAVMODE. Valid for BT.656 busses only.

This is a vendor-specific property, it should be prefixed with "adi,". Could 
you also explain how this mode works ? I'd like to make sure it qualifies for 
a DT property.

> +
>  Example:
> 
>  	i2c0@1c22000 {
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index 6e093c22..467953e 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -31,6 +31,7 @@
>  #include <media/v4l2-event.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ctrls.h>
> +#include <media/v4l2-of.h>
>  #include <linux/mutex.h>
>  #include <linux/delay.h>
> 
> @@ -106,6 +107,7 @@
>  #define ADV7180_REG_SHAP_FILTER_CTL_1	0x0017
>  #define ADV7180_REG_CTRL_2		0x001d
>  #define ADV7180_REG_VSYNC_FIELD_CTL_1	0x0031
> +#define ADV7180_VSYNC_FIELD_CTL_1_NEWAVMODE 0x02
>  #define ADV7180_REG_MANUAL_WIN_CTL_1	0x003d
>  #define ADV7180_REG_MANUAL_WIN_CTL_2	0x003e
>  #define ADV7180_REG_MANUAL_WIN_CTL_3	0x003f
> @@ -214,6 +216,7 @@ struct adv7180_state {
>  	struct mutex		mutex; /* mutual excl. when accessing chip */
>  	int			irq;
>  	v4l2_std_id		curr_norm;
> +	bool			newavmode;
>  	bool			powered;
>  	bool			streaming;
>  	u8			input;
> @@ -864,9 +867,15 @@ static int adv7180_init(struct adv7180_state *state)
>  	if (ret < 0)
>  		return ret;
> 
> -	/* Manually set V bit end position in NTSC mode */
> -	return adv7180_write(state, ADV7180_REG_NTSC_V_BIT_END,
> -					ADV7180_NTSC_V_BIT_END_MANUAL_NVEND);
> +	if (!state->newavmode) {
> +		/* Manually set V bit end position in NTSC mode */
> +		ret = adv7180_write(state, ADV7180_REG_NTSC_V_BIT_END,
> +				    ADV7180_NTSC_V_BIT_END_MANUAL_NVEND);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
>  }
> 
>  static int adv7180_set_std(struct adv7180_state *state, unsigned int std)
> @@ -1217,6 +1226,13 @@ static int init_device(struct adv7180_state *state)
>  	if (ret)
>  		goto out_unlock;
> 
> +	if (state->newavmode) {
> +		ret = adv7180_write(state, ADV7180_REG_VSYNC_FIELD_CTL_1,
> +				    ADV7180_VSYNC_FIELD_CTL_1_NEWAVMODE);
> +		if (ret < 0)
> +			goto out_unlock;
> +	}
> +
>  	ret = adv7180_program_std(state);
>  	if (ret)
>  		goto out_unlock;
> @@ -1257,6 +1273,28 @@ out_unlock:
>  	return ret;
>  }
> 
> +static void adv7180_of_parse(struct adv7180_state *state)
> +{
> +	struct i2c_client *client = state->client;
> +	struct device_node *np = client->dev.of_node;
> +	struct device_node *endpoint;
> +	struct v4l2_of_endpoint	ep;
> +
> +	endpoint = of_graph_get_next_endpoint(np, NULL);
> +	if (!endpoint) {
> +		v4l_warn(client, "endpoint node not found\n");
> +		return;
> +	}
> +
> +	v4l2_of_parse_endpoint(endpoint, &ep);
> +	if (ep.bus_type == V4L2_MBUS_BT656) {
> +		if (of_property_read_bool(endpoint, "newavmode"))
> +			state->newavmode = true;
> +	}
> +
> +	of_node_put(endpoint);
> +}
> +
>  static int adv7180_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
> @@ -1279,6 +1317,8 @@ static int adv7180_probe(struct i2c_client *client,
>  	state->field = V4L2_FIELD_ALTERNATE;
>  	state->chip_info = (struct adv7180_chip_info *)id->driver_data;
> 
> +	adv7180_of_parse(state);
> +
>  	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
>  		state->csi_client = i2c_new_dummy(client->adapter,
>  				ADV7180_DEFAULT_CSI_I2C_ADDR);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 3/8] media: adv7180: add support for NEWAVMODE

[Steve Longerbeam]

On 10/16/2016 05:18 AM, Laurent Pinchart wrote:
> Hi Steve,
>
> Thank you for the patch.
>
> On Wednesday 03 Aug 2016 11:03:45 Steve Longerbeam wrote:
>> Parse the optional v4l2 endpoint DT node. If the bus type is
>> V4L2_MBUS_BT656 and the endpoint node specifies "newavmode",
>> configure the BT.656 bus in NEWAVMODE.
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>
>> ---
>>
>> v4: no changes
>> v3:
>> - the newavmode endpoint property is now private to adv7180.
>> ---
>>   .../devicetree/bindings/media/i2c/adv7180.txt      |  4 ++
>>   drivers/media/i2c/adv7180.c                        | 46 +++++++++++++++++--
>>   2 files changed, 47 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/adv7180.txt
>> b/Documentation/devicetree/bindings/media/i2c/adv7180.txt index
>> 0d50115..6c175d2 100644
>> --- a/Documentation/devicetree/bindings/media/i2c/adv7180.txt
>> +++ b/Documentation/devicetree/bindings/media/i2c/adv7180.txt
>> @@ -15,6 +15,10 @@ Required Properties :
>>   		"adi,adv7282"
>>   		"adi,adv7282-m"
>>
>> +Optional Endpoint Properties :
>> +- newavmode: a boolean property to indicate the BT.656 bus is operating
>> +  in Analog Device's NEWAVMODE. Valid for BT.656 busses only.
> This is a vendor-specific property, it should be prefixed with "adi,".

Ok, I'll do that in next version.

>   Could
> you also explain how this mode works ? I'd like to make sure it qualifies for
> a DT property.

The blurb in the ADV718x manual is terse:

"When NEWAVMODE is 0 (enabled), EAV/SAV codes are generated to
suit Analog Devices encoders. No adjustments are possible."

"Setting NEWAVMODE to 1 (default) enables the manual position
of the VSYNC, FIELD, and AV codes using Register 0x32 to
Register 0x33 and Register 0xE5 to Register 0xEA. Default register
settings are CCIR656 compliant;"

So it's not clear to me how the generated EAV and SAV codes are
different from standard CCIR656, but apparently they are.

Steve


>
>> +
>>   Example:
>>
>>   	i2c0@1c22000 {
>> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
>> index 6e093c22..467953e 100644
>> --- a/drivers/media/i2c/adv7180.c
>> +++ b/drivers/media/i2c/adv7180.c
>> @@ -31,6 +31,7 @@
>>   #include <media/v4l2-event.h>
>>   #include <media/v4l2-device.h>
>>   #include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-of.h>
>>   #include <linux/mutex.h>
>>   #include <linux/delay.h>
>>
>> @@ -106,6 +107,7 @@
>>   #define ADV7180_REG_SHAP_FILTER_CTL_1	0x0017
>>   #define ADV7180_REG_CTRL_2		0x001d
>>   #define ADV7180_REG_VSYNC_FIELD_CTL_1	0x0031
>> +#define ADV7180_VSYNC_FIELD_CTL_1_NEWAVMODE 0x02
>>   #define ADV7180_REG_MANUAL_WIN_CTL_1	0x003d
>>   #define ADV7180_REG_MANUAL_WIN_CTL_2	0x003e
>>   #define ADV7180_REG_MANUAL_WIN_CTL_3	0x003f
>> @@ -214,6 +216,7 @@ struct adv7180_state {
>>   	struct mutex		mutex; /* mutual excl. when accessing chip */
>>   	int			irq;
>>   	v4l2_std_id		curr_norm;
>> +	bool			newavmode;
>>   	bool			powered;
>>   	bool			streaming;
>>   	u8			input;
>> @@ -864,9 +867,15 @@ static int adv7180_init(struct adv7180_state *state)
>>   	if (ret < 0)
>>   		return ret;
>>
>> -	/* Manually set V bit end position in NTSC mode */
>> -	return adv7180_write(state, ADV7180_REG_NTSC_V_BIT_END,
>> -					ADV7180_NTSC_V_BIT_END_MANUAL_NVEND);
>> +	if (!state->newavmode) {
>> +		/* Manually set V bit end position in NTSC mode */
>> +		ret = adv7180_write(state, ADV7180_REG_NTSC_V_BIT_END,
>> +				    ADV7180_NTSC_V_BIT_END_MANUAL_NVEND);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>> +	return 0;
>>   }
>>
>>   static int adv7180_set_std(struct adv7180_state *state, unsigned int std)
>> @@ -1217,6 +1226,13 @@ static int init_device(struct adv7180_state *state)
>>   	if (ret)
>>   		goto out_unlock;
>>
>> +	if (state->newavmode) {
>> +		ret = adv7180_write(state, ADV7180_REG_VSYNC_FIELD_CTL_1,
>> +				    ADV7180_VSYNC_FIELD_CTL_1_NEWAVMODE);
>> +		if (ret < 0)
>> +			goto out_unlock;
>> +	}
>> +
>>   	ret = adv7180_program_std(state);
>>   	if (ret)
>>   		goto out_unlock;
>> @@ -1257,6 +1273,28 @@ out_unlock:
>>   	return ret;
>>   }
>>
>> +static void adv7180_of_parse(struct adv7180_state *state)
>> +{
>> +	struct i2c_client *client = state->client;
>> +	struct device_node *np = client->dev.of_node;
>> +	struct device_node *endpoint;
>> +	struct v4l2_of_endpoint	ep;
>> +
>> +	endpoint = of_graph_get_next_endpoint(np, NULL);
>> +	if (!endpoint) {
>> +		v4l_warn(client, "endpoint node not found\n");
>> +		return;
>> +	}
>> +
>> +	v4l2_of_parse_endpoint(endpoint, &ep);
>> +	if (ep.bus_type == V4L2_MBUS_BT656) {
>> +		if (of_property_read_bool(endpoint, "newavmode"))
>> +			state->newavmode = true;
>> +	}
>> +
>> +	of_node_put(endpoint);
>> +}
>> +
>>   static int adv7180_probe(struct i2c_client *client,
>>   			 const struct i2c_device_id *id)
>>   {
>> @@ -1279,6 +1317,8 @@ static int adv7180_probe(struct i2c_client *client,
>>   	state->field = V4L2_FIELD_ALTERNATE;
>>   	state->chip_info = (struct adv7180_chip_info *)id->driver_data;
>>
>> +	adv7180_of_parse(state);
>> +
>>   	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
>>   		state->csi_client = i2c_new_dummy(client->adapter,
>>   				ADV7180_DEFAULT_CSI_I2C_ADDR);

Re: [PATCH v4 3/8] media: adv7180: add support for NEWAVMODE

[Laurent Pinchart]

Hi Steve,

On Monday 17 Oct 2016 15:36:14 Steve Longerbeam wrote:
> On 10/16/2016 05:18 AM, Laurent Pinchart wrote:
> > On Wednesday 03 Aug 2016 11:03:45 Steve Longerbeam wrote:
> >> Parse the optional v4l2 endpoint DT node. If the bus type is
> >> V4L2_MBUS_BT656 and the endpoint node specifies "newavmode",
> >> configure the BT.656 bus in NEWAVMODE.
> >> 
> >> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> >> ---
> >> v4: no changes
> >> v3:
> >> - the newavmode endpoint property is now private to adv7180.
> >> ---
> >> .../devicetree/bindings/media/i2c/adv7180.txt      |  4 ++
> >> drivers/media/i2c/adv7180.c                        | 46 ++++++++++++++--
> >> 2 files changed, 47 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/adv7180.txt
> >> b/Documentation/devicetree/bindings/media/i2c/adv7180.txt index
> >> 0d50115..6c175d2 100644
> >> --- a/Documentation/devicetree/bindings/media/i2c/adv7180.txt
> >> +++ b/Documentation/devicetree/bindings/media/i2c/adv7180.txt
> >> @@ -15,6 +15,10 @@ Required Properties :
> >>   		"adi,adv7282"
> >>   		"adi,adv7282-m"
> >> 
> >> +Optional Endpoint Properties :
> >> +- newavmode: a boolean property to indicate the BT.656 bus is operating
> >> +  in Analog Device's NEWAVMODE. Valid for BT.656 busses only.
> > 
> > This is a vendor-specific property, it should be prefixed with "adi,".
> 
> Ok, I'll do that in next version.
> 
> >   Could
> > 
> > you also explain how this mode works ? I'd like to make sure it qualifies
> > for a DT property.
> 
> The blurb in the ADV718x manual is terse:
> 
> "When NEWAVMODE is 0 (enabled), EAV/SAV codes are generated to
> suit Analog Devices encoders. No adjustments are possible."
> 
> "Setting NEWAVMODE to 1 (default) enables the manual position
> of the VSYNC, FIELD, and AV codes using Register 0x32 to
> Register 0x33 and Register 0xE5 to Register 0xEA. Default register
> settings are CCIR656 compliant;"
> 
> So it's not clear to me how the generated EAV and SAV codes are
> different from standard CCIR656, but apparently they are.

Shouldn't we understand how it works to create proper DT bindings ? :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 3/8] media: adv7180: add support for NEWAVMODE

[Hans Verkuil]

On 08/03/2016 08:03 PM, Steve Longerbeam wrote:
> Parse the optional v4l2 endpoint DT node. If the bus type is
> V4L2_MBUS_BT656 and the endpoint node specifies "newavmode",
> configure the BT.656 bus in NEWAVMODE.
> 
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> 
> ---
> 
> v4: no changes
> v3:
> - the newavmode endpoint property is now private to adv7180.
> ---
>  .../devicetree/bindings/media/i2c/adv7180.txt      |  4 ++
>  drivers/media/i2c/adv7180.c                        | 46 ++++++++++++++++++++--
>  2 files changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/adv7180.txt b/Documentation/devicetree/bindings/media/i2c/adv7180.txt
> index 0d50115..6c175d2 100644
> --- a/Documentation/devicetree/bindings/media/i2c/adv7180.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/adv7180.txt
> @@ -15,6 +15,10 @@ Required Properties :
>  		"adi,adv7282"
>  		"adi,adv7282-m"
>  
> +Optional Endpoint Properties :
> +- newavmode: a boolean property to indicate the BT.656 bus is operating
> +  in Analog Device's NEWAVMODE. Valid for BT.656 busses only.

This is too vague.

Based on the ADV7280/ADV7281/ADV7282/ADV7283 Hardware Reference Manual I
would say something like this:

- newavmode: a boolean property to indicate the BT.656 bus is operating
  in Analog Device's NEWAVMODE. Valid for BT.656 busses only. When enabled
  the generated EAV/SAV codes are suitable for Analog Devices encoders.
  Otherwise these codes are setup according to <some standard?>
  See bit 4 of user sub map register 0x31 in the Hardware Reference Manual.

I may have asked this before, but do you actually have hardware that needs
this? If so, it may be useful to give it as an example and explain why it
is needed.

If not, then I wonder if this cannot be dropped until we DO see hardware
that needs it.

Regards,

	Hans


> +
>  Example:
>  
>  	i2c0@1c22000 {
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index 6e093c22..467953e 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -31,6 +31,7 @@
>  #include <media/v4l2-event.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ctrls.h>
> +#include <media/v4l2-of.h>
>  #include <linux/mutex.h>
>  #include <linux/delay.h>
>  
> @@ -106,6 +107,7 @@
>  #define ADV7180_REG_SHAP_FILTER_CTL_1	0x0017
>  #define ADV7180_REG_CTRL_2		0x001d
>  #define ADV7180_REG_VSYNC_FIELD_CTL_1	0x0031
> +#define ADV7180_VSYNC_FIELD_CTL_1_NEWAVMODE 0x02
>  #define ADV7180_REG_MANUAL_WIN_CTL_1	0x003d
>  #define ADV7180_REG_MANUAL_WIN_CTL_2	0x003e
>  #define ADV7180_REG_MANUAL_WIN_CTL_3	0x003f
> @@ -214,6 +216,7 @@ struct adv7180_state {
>  	struct mutex		mutex; /* mutual excl. when accessing chip */
>  	int			irq;
>  	v4l2_std_id		curr_norm;
> +	bool			newavmode;
>  	bool			powered;
>  	bool			streaming;
>  	u8			input;
> @@ -864,9 +867,15 @@ static int adv7180_init(struct adv7180_state *state)
>  	if (ret < 0)
>  		return ret;
>  
> -	/* Manually set V bit end position in NTSC mode */
> -	return adv7180_write(state, ADV7180_REG_NTSC_V_BIT_END,
> -					ADV7180_NTSC_V_BIT_END_MANUAL_NVEND);
> +	if (!state->newavmode) {
> +		/* Manually set V bit end position in NTSC mode */
> +		ret = adv7180_write(state, ADV7180_REG_NTSC_V_BIT_END,
> +				    ADV7180_NTSC_V_BIT_END_MANUAL_NVEND);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
>  }
>  
>  static int adv7180_set_std(struct adv7180_state *state, unsigned int std)
> @@ -1217,6 +1226,13 @@ static int init_device(struct adv7180_state *state)
>  	if (ret)
>  		goto out_unlock;
>  
> +	if (state->newavmode) {
> +		ret = adv7180_write(state, ADV7180_REG_VSYNC_FIELD_CTL_1,
> +				    ADV7180_VSYNC_FIELD_CTL_1_NEWAVMODE);
> +		if (ret < 0)
> +			goto out_unlock;
> +	}
> +
>  	ret = adv7180_program_std(state);
>  	if (ret)
>  		goto out_unlock;
> @@ -1257,6 +1273,28 @@ out_unlock:
>  	return ret;
>  }
>  
> +static void adv7180_of_parse(struct adv7180_state *state)
> +{
> +	struct i2c_client *client = state->client;
> +	struct device_node *np = client->dev.of_node;
> +	struct device_node *endpoint;
> +	struct v4l2_of_endpoint	ep;
> +
> +	endpoint = of_graph_get_next_endpoint(np, NULL);
> +	if (!endpoint) {
> +		v4l_warn(client, "endpoint node not found\n");
> +		return;
> +	}
> +
> +	v4l2_of_parse_endpoint(endpoint, &ep);
> +	if (ep.bus_type == V4L2_MBUS_BT656) {
> +		if (of_property_read_bool(endpoint, "newavmode"))
> +			state->newavmode = true;
> +	}
> +
> +	of_node_put(endpoint);
> +}
> +
>  static int adv7180_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
> @@ -1279,6 +1317,8 @@ static int adv7180_probe(struct i2c_client *client,
>  	state->field = V4L2_FIELD_ALTERNATE;
>  	state->chip_info = (struct adv7180_chip_info *)id->driver_data;
>  
> +	adv7180_of_parse(state);
> +
>  	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
>  		state->csi_client = i2c_new_dummy(client->adapter,
>  				ADV7180_DEFAULT_CSI_I2C_ADDR);
> 

Re: [PATCH v4 3/8] media: adv7180: add support for NEWAVMODE

[Steve Longerbeam]

On 11/14/2016 03:28 AM, Hans Verkuil wrote:
> On 08/03/2016 08:03 PM, Steve Longerbeam wrote:
>> Parse the optional v4l2 endpoint DT node. If the bus type is
>> V4L2_MBUS_BT656 and the endpoint node specifies "newavmode",
>> configure the BT.656 bus in NEWAVMODE.
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>
>> ---
>>
>> v4: no changes
>> v3:
>> - the newavmode endpoint property is now private to adv7180.
>> ---
>>   .../devicetree/bindings/media/i2c/adv7180.txt      |  4 ++
>>   drivers/media/i2c/adv7180.c                        | 46 ++++++++++++++++++++--
>>   2 files changed, 47 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/adv7180.txt b/Documentation/devicetree/bindings/media/i2c/adv7180.txt
>> index 0d50115..6c175d2 100644
>> --- a/Documentation/devicetree/bindings/media/i2c/adv7180.txt
>> +++ b/Documentation/devicetree/bindings/media/i2c/adv7180.txt
>> @@ -15,6 +15,10 @@ Required Properties :
>>   		"adi,adv7282"
>>   		"adi,adv7282-m"
>>   
>> +Optional Endpoint Properties :
>> +- newavmode: a boolean property to indicate the BT.656 bus is operating
>> +  in Analog Device's NEWAVMODE. Valid for BT.656 busses only.
> This is too vague.
>
> Based on the ADV7280/ADV7281/ADV7282/ADV7283 Hardware Reference Manual I
> would say something like this:
>
> - newavmode: a boolean property to indicate the BT.656 bus is operating
>    in Analog Device's NEWAVMODE. Valid for BT.656 busses only. When enabled
>    the generated EAV/SAV codes are suitable for Analog Devices encoders.
>    Otherwise these codes are setup according to <some standard?>
>    See bit 4 of user sub map register 0x31 in the Hardware Reference Manual.
>
> I may have asked this before, but do you actually have hardware that needs
> this? If so, it may be useful to give it as an example and explain why it
> is needed.
>
> If not, then I wonder if this cannot be dropped until we DO see hardware
> that needs it.

Hi Hans, thanks for reviewing this, but at least for imx6, I don't
need this patch anymore.

Recently I dug deeper into the current bt.656 programming in
adv7180.c. The driver manually configures the bus to have 21
blank lines in odd fields, and 22 blank lines in even fields (via
NVEND register) for NTSC.

That leaves 525 - (21 +22) = 482 active lines in NTSC.

After configuring the imx6 host bridge to crop those extra 2 lines,
it is capturing good 720x480 NTSC images now.

So I no longer need this patch to enable NEWAVMODE.

However I still see some issues.

First, adv7180.c attempts to enable  BT.656-4 mode, but according
to the datasheet, that cannot be enabled without first enabling
NEWAVMODE. So the attempt to enable BT.656-4 mode is a no-op,
it is currently doing nothing. So I suggest removing that attempt.

Second, it is wrong for the host bridge to have to make an assumption
about cropping for a sensor. The adv7180 needs to communicate to
hosts about the number of field blanking lines it has configured, maybe
via get_selection. I.e., report full sensor frame via get_fmt, and 720x482
via get_selection.


Steve

[PATCH v4 4/8] media: adv7180: add power pin control

[Steve Longerbeam]

Some targets control the ADV7180 power pin via a gpio, so add
optional support for "powerdown" pin control.

Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
Tested-by: Tim Harvey <tharvey@gateworks.com>
Acked-by: Tim Harvey <tharvey@gateworks.com>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>

---

v4: no changes
v3: no changes

v2:
- placed call to gpiod_get inline in adv7180_probe().
- rename gpio pin to "powerdown".
- document optional powerdown-gpios property in
  Documentation/devicetree/bindings/media/i2c/adv7180.txt.
- include error number in error message on gpiod_get failure.
---
 .../devicetree/bindings/media/i2c/adv7180.txt      |  4 ++++
 drivers/media/i2c/Kconfig                          |  2 +-
 drivers/media/i2c/adv7180.c                        | 27 ++++++++++++++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/adv7180.txt b/Documentation/devicetree/bindings/media/i2c/adv7180.txt
index 6c175d2..ab9ef02 100644
--- a/Documentation/devicetree/bindings/media/i2c/adv7180.txt
+++ b/Documentation/devicetree/bindings/media/i2c/adv7180.txt
@@ -15,6 +15,10 @@ Required Properties :
 		"adi,adv7282"
 		"adi,adv7282-m"
 
+Optional Properties :
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+  if any.
+
 Optional Endpoint Properties :
 - newavmode: a boolean property to indicate the BT.656 bus is operating
   in Analog Device's NEWAVMODE. Valid for BT.656 busses only.
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index ce9006e..6769898 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -187,7 +187,7 @@ comment "Video decoders"
 
 config VIDEO_ADV7180
 	tristate "Analog Devices ADV7180 decoder"
-	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+	depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
 	---help---
 	  Support for the Analog Devices ADV7180 video decoder.
 
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 467953e..b2df181 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -26,6 +26,7 @@
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/gpio/consumer.h>
 #include <linux/videodev2.h>
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-event.h>
@@ -215,6 +216,7 @@ struct adv7180_state {
 	struct media_pad	pad;
 	struct mutex		mutex; /* mutual excl. when accessing chip */
 	int			irq;
+	struct gpio_desc	*pwdn_gpio;
 	v4l2_std_id		curr_norm;
 	bool			newavmode;
 	bool			powered;
@@ -466,6 +468,19 @@ static int adv7180_g_std(struct v4l2_subdev *sd, v4l2_std_id *norm)
 	return 0;
 }
 
+static void adv7180_set_power_pin(struct adv7180_state *state, bool on)
+{
+	if (!state->pwdn_gpio)
+		return;
+
+	if (on) {
+		gpiod_set_value_cansleep(state->pwdn_gpio, 0);
+		usleep_range(5000, 10000);
+	} else {
+		gpiod_set_value_cansleep(state->pwdn_gpio, 1);
+	}
+}
+
 static int adv7180_set_power(struct adv7180_state *state, bool on)
 {
 	u8 val;
@@ -1219,6 +1234,8 @@ static int init_device(struct adv7180_state *state)
 
 	mutex_lock(&state->mutex);
 
+	adv7180_set_power_pin(state, true);
+
 	adv7180_write(state, ADV7180_REG_PWR_MAN, ADV7180_PWR_MAN_RES);
 	usleep_range(5000, 10000);
 
@@ -1319,6 +1336,14 @@ static int adv7180_probe(struct i2c_client *client,
 
 	adv7180_of_parse(state);
 
+	state->pwdn_gpio = devm_gpiod_get_optional(&client->dev, "powerdown",
+						   GPIOD_OUT_HIGH);
+	if (IS_ERR(state->pwdn_gpio)) {
+		ret = PTR_ERR(state->pwdn_gpio);
+		v4l_err(client, "request for power pin failed: %d\n", ret);
+		return ret;
+	}
+
 	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
 		state->csi_client = i2c_new_dummy(client->adapter,
 				ADV7180_DEFAULT_CSI_I2C_ADDR);
@@ -1410,6 +1435,8 @@ static int adv7180_remove(struct i2c_client *client)
 	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2)
 		i2c_unregister_device(state->csi_client);
 
+	adv7180_set_power_pin(state, false);
+
 	mutex_destroy(&state->mutex);
 
 	return 0;
-- 
1.9.1

[PATCH v4 5/8] media: adv7180: implement g_parm

[Steve Longerbeam]

Implement g_parm to return the current standard's frame period.

Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
Tested-by: Tim Harvey <tharvey@gateworks.com>
Acked-by: Tim Harvey <tharvey@gateworks.com>

---
v4: no changes
v3: no changes
v2: no changes
---
 drivers/media/i2c/adv7180.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index b2df181..9705e24 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -764,6 +764,27 @@ static int adv7180_g_mbus_config(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int adv7180_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *a)
+{
+	struct adv7180_state *state = to_state(sd);
+	struct v4l2_captureparm *cparm = &a->parm.capture;
+
+	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	memset(a, 0, sizeof(*a));
+	a->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	if (state->curr_norm & V4L2_STD_525_60) {
+		cparm->timeperframe.numerator = 1001;
+		cparm->timeperframe.denominator = 30000;
+	} else {
+		cparm->timeperframe.numerator = 1;
+		cparm->timeperframe.denominator = 25;
+	}
+
+	return 0;
+}
+
 static int adv7180_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *cropcap)
 {
 	struct adv7180_state *state = to_state(sd);
@@ -822,6 +843,7 @@ static int adv7180_subscribe_event(struct v4l2_subdev *sd,
 static const struct v4l2_subdev_video_ops adv7180_video_ops = {
 	.s_std = adv7180_s_std,
 	.g_std = adv7180_g_std,
+	.g_parm = adv7180_g_parm,
 	.querystd = adv7180_querystd,
 	.g_input_status = adv7180_g_input_status,
 	.s_routing = adv7180_s_routing,
-- 
1.9.1

Re: [PATCH v4 5/8] media: adv7180: implement g_parm

[Hans Verkuil]

On 08/03/2016 08:03 PM, Steve Longerbeam wrote:
> Implement g_parm to return the current standard's frame period.
> 
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> Tested-by: Tim Harvey <tharvey@gateworks.com>
> Acked-by: Tim Harvey <tharvey@gateworks.com>
> 
> ---
> v4: no changes
> v3: no changes
> v2: no changes
> ---
>  drivers/media/i2c/adv7180.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index b2df181..9705e24 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -764,6 +764,27 @@ static int adv7180_g_mbus_config(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static int adv7180_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *a)
> +{
> +	struct adv7180_state *state = to_state(sd);
> +	struct v4l2_captureparm *cparm = &a->parm.capture;
> +
> +	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +		return -EINVAL;
> +
> +	memset(a, 0, sizeof(*a));
> +	a->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;

Don't memset this, it is the responsibility of the caller to do that,
not of the subdev.

The caller may have set other fields in the struct that the memset would
wipe out.

Regards,

	Hans

> +	if (state->curr_norm & V4L2_STD_525_60) {
> +		cparm->timeperframe.numerator = 1001;
> +		cparm->timeperframe.denominator = 30000;
> +	} else {
> +		cparm->timeperframe.numerator = 1;
> +		cparm->timeperframe.denominator = 25;
> +	}
> +
> +	return 0;
> +}
> +
>  static int adv7180_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *cropcap)
>  {
>  	struct adv7180_state *state = to_state(sd);
> @@ -822,6 +843,7 @@ static int adv7180_subscribe_event(struct v4l2_subdev *sd,
>  static const struct v4l2_subdev_video_ops adv7180_video_ops = {
>  	.s_std = adv7180_s_std,
>  	.g_std = adv7180_g_std,
> +	.g_parm = adv7180_g_parm,
>  	.querystd = adv7180_querystd,
>  	.g_input_status = adv7180_g_input_status,
>  	.s_routing = adv7180_s_routing,
> 

[PATCH v4 6/8] media: adv7180: change mbus format to UYVY

[Steve Longerbeam]

Change the media bus format from YUYV8_2X8 to UYVY8_2X8. Colors
now look correct when capturing with the i.mx6 backend.

Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
Tested-by: Tim Harvey <tharvey@gateworks.com>
Acked-by: Tim Harvey <tharvey@gateworks.com>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

---
v4: no changes
v3: no changes
v2: no changes
---
 drivers/media/i2c/adv7180.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 9705e24..abdc519 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -636,7 +636,7 @@ static int adv7180_enum_mbus_code(struct v4l2_subdev *sd,
 	if (code->index != 0)
 		return -EINVAL;
 
-	code->code = MEDIA_BUS_FMT_YUYV8_2X8;
+	code->code = MEDIA_BUS_FMT_UYVY8_2X8;
 
 	return 0;
 }
@@ -646,7 +646,7 @@ static int adv7180_mbus_fmt(struct v4l2_subdev *sd,
 {
 	struct adv7180_state *state = to_state(sd);
 
-	fmt->code = MEDIA_BUS_FMT_YUYV8_2X8;
+	fmt->code = MEDIA_BUS_FMT_UYVY8_2X8;
 	fmt->colorspace = V4L2_COLORSPACE_SMPTE170M;
 	fmt->width = 720;
 	fmt->height = state->curr_norm & V4L2_STD_525_60 ? 480 : 576;
-- 
1.9.1

[PATCH v4 7/8] v4l: Add signal lock status to source change events

[Steve Longerbeam]

Add a signal lock status change to the source changes bitmask.
This indicates there was a signal lock or unlock event detected
at the input of a video decoder.

Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

---

v4:
- converted to rst from DocBook

v3: no changes
v2: no changes
---
 Documentation/media/uapi/v4l/vidioc-dqevent.rst | 9 +++++++++
 Documentation/media/videodev2.h.rst.exceptions  | 1 +
 include/uapi/linux/videodev2.h                  | 1 +
 3 files changed, 11 insertions(+)

diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
index 73c0d5b..7d8a053 100644
--- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst
+++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
@@ -564,6 +564,15 @@ call.
 	  an input. This can come from an input connector or from a video
 	  decoder.
 
+    -  .. row 2
+
+       -  ``V4L2_EVENT_SRC_CH_LOCK_STATUS``
+
+       -  0x0002
+
+       -  This event gets triggered when there is a signal lock or
+	  unlock detected at the input of a video decoder.
+
 
 Return Value
 ============
diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
index 9bb9a6c..f412cc8 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -453,6 +453,7 @@ replace define V4L2_EVENT_CTRL_CH_FLAGS ctrl-changes-flags
 replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags
 
 replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
+replace define V4L2_EVENT_SRC_CH_LOCK_STATUS src-changes-flags
 
 replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ v4l2-event-motion-det
 
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 724f43e..08a153f 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -2078,6 +2078,7 @@ struct v4l2_event_frame_sync {
 };
 
 #define V4L2_EVENT_SRC_CH_RESOLUTION		(1 << 0)
+#define V4L2_EVENT_SRC_CH_LOCK_STATUS		(1 << 1)
 
 struct v4l2_event_src_change {
 	__u32 changes;
-- 
1.9.1

Re: [PATCH v4 7/8] v4l: Add signal lock status to source change events

[Hans Verkuil]

On 08/03/2016 08:03 PM, Steve Longerbeam wrote:
> Add a signal lock status change to the source changes bitmask.
> This indicates there was a signal lock or unlock event detected
> at the input of a video decoder.
> 
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> ---
> 
> v4:
> - converted to rst from DocBook
> 
> v3: no changes
> v2: no changes
> ---
>  Documentation/media/uapi/v4l/vidioc-dqevent.rst | 9 +++++++++
>  Documentation/media/videodev2.h.rst.exceptions  | 1 +
>  include/uapi/linux/videodev2.h                  | 1 +
>  3 files changed, 11 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
> index 73c0d5b..7d8a053 100644
> --- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
> @@ -564,6 +564,15 @@ call.
>  	  an input. This can come from an input connector or from a video
>  	  decoder.
>  
> +    -  .. row 2
> +
> +       -  ``V4L2_EVENT_SRC_CH_LOCK_STATUS``
> +
> +       -  0x0002
> +
> +       -  This event gets triggered when there is a signal lock or
> +	  unlock detected at the input of a video decoder.
> +
>  
>  Return Value
>  ============
> diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
> index 9bb9a6c..f412cc8 100644
> --- a/Documentation/media/videodev2.h.rst.exceptions
> +++ b/Documentation/media/videodev2.h.rst.exceptions
> @@ -453,6 +453,7 @@ replace define V4L2_EVENT_CTRL_CH_FLAGS ctrl-changes-flags
>  replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags
>  
>  replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
> +replace define V4L2_EVENT_SRC_CH_LOCK_STATUS src-changes-flags
>  
>  replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ v4l2-event-motion-det
>  
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 724f43e..08a153f 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -2078,6 +2078,7 @@ struct v4l2_event_frame_sync {
>  };
>  
>  #define V4L2_EVENT_SRC_CH_RESOLUTION		(1 << 0)
> +#define V4L2_EVENT_SRC_CH_LOCK_STATUS		(1 << 1)
>  
>  struct v4l2_event_src_change {
>  	__u32 changes;
> 

Quoting from an old (July) conversation about this:

>> > I'm not entirely sure I like this. Typically losing lock means that this event
>> > is triggered with the V4L2_EVENT_SRC_CH_RESOLUTION flag set, and userspace has
>> > to check the new timings etc., which will fail if there is no lock anymore.
>> >
>> > This information is also available through ENUMINPUT.
>> >
>> > I would need to know more about why you think this is needed, because I don't
>> > see what this adds.
> 
> Hi Hans,
> 
> At least on the ADV718x, a source resolution change (from an 
> autodetected video
> standard change) and a signal lock status change are distinct events. 
> For example
> there can be a temporary loss of input signal lock without a change in 
> detected
> input video standard/resolution.

OK, but what can the application do with that event? If the glitch didn't
affect the video, then it is pointless.

If the lock is lost, then normally you loose video as well. If not, then
applications are not interested in the event.

So I am not convinced...

Regards,

	Hans

Re: [PATCH v4 7/8] v4l: Add signal lock status to source change events

[Devin Heitmueller]

> OK, but what can the application do with that event? If the glitch didn't
> affect the video, then it is pointless.
>
> If the lock is lost, then normally you loose video as well. If not, then
> applications are not interested in the event.

What about free running mode (where some decoders delivers blue or
black video with no signal present)?  In that case it might still be
useful to inform the application so it can show a message that says
something like "No Signal".

Devin


-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH v4 7/8] v4l: Add signal lock status to source change events

[Steve Longerbeam]

On 11/14/2016 03:36 AM, Hans Verkuil wrote:
> On 08/03/2016 08:03 PM, Steve Longerbeam wrote:
>> Add a signal lock status change to the source changes bitmask.
>> This indicates there was a signal lock or unlock event detected
>> at the input of a video decoder.
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>> Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>
>> ---
>>
>> v4:
>> - converted to rst from DocBook
>>
>> v3: no changes
>> v2: no changes
>> ---
>>   Documentation/media/uapi/v4l/vidioc-dqevent.rst | 9 +++++++++
>>   Documentation/media/videodev2.h.rst.exceptions  | 1 +
>>   include/uapi/linux/videodev2.h                  | 1 +
>>   3 files changed, 11 insertions(+)
>>
>> diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>> index 73c0d5b..7d8a053 100644
>> --- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>> +++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>> @@ -564,6 +564,15 @@ call.
>>   	  an input. This can come from an input connector or from a video
>>   	  decoder.
>>   
>> +    -  .. row 2
>> +
>> +       -  ``V4L2_EVENT_SRC_CH_LOCK_STATUS``
>> +
>> +       -  0x0002
>> +
>> +       -  This event gets triggered when there is a signal lock or
>> +	  unlock detected at the input of a video decoder.
>> +
>>   
>>   Return Value
>>   ============
>> diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
>> index 9bb9a6c..f412cc8 100644
>> --- a/Documentation/media/videodev2.h.rst.exceptions
>> +++ b/Documentation/media/videodev2.h.rst.exceptions
>> @@ -453,6 +453,7 @@ replace define V4L2_EVENT_CTRL_CH_FLAGS ctrl-changes-flags
>>   replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags
>>   
>>   replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
>> +replace define V4L2_EVENT_SRC_CH_LOCK_STATUS src-changes-flags
>>   
>>   replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ v4l2-event-motion-det
>>   
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index 724f43e..08a153f 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -2078,6 +2078,7 @@ struct v4l2_event_frame_sync {
>>   };
>>   
>>   #define V4L2_EVENT_SRC_CH_RESOLUTION		(1 << 0)
>> +#define V4L2_EVENT_SRC_CH_LOCK_STATUS		(1 << 1)
>>   
>>   struct v4l2_event_src_change {
>>   	__u32 changes;
>>
> Quoting from an old (July) conversation about this:
>
>>>> I'm not entirely sure I like this. Typically losing lock means that this event
>>>> is triggered with the V4L2_EVENT_SRC_CH_RESOLUTION flag set, and userspace has
>>>> to check the new timings etc., which will fail if there is no lock anymore.
>>>>
>>>> This information is also available through ENUMINPUT.
>>>>
>>>> I would need to know more about why you think this is needed, because I don't
>>>> see what this adds.
>> Hi Hans,
>>
>> At least on the ADV718x, a source resolution change (from an
>> autodetected video
>> standard change) and a signal lock status change are distinct events.
>> For example
>> there can be a temporary loss of input signal lock without a change in
>> detected
>> input video standard/resolution.
> OK, but what can the application do with that event? If the glitch didn't
> affect the video, then it is pointless.

Hi Hans, that's just it, for i.mx6 it does affect video. On i.mx6 a 
temporary
loss of signal from the adv7180 often results in a "split image", or rolling
image from captured frame to the next, and the only way to recover
from this failure is to restart the pipeline (stream off -- stream on). So
the application needs to be informed of this temporary loss of signal
event in order to restart streaming.


>
> If the lock is lost, then normally you loose video as well. If not, then
> applications are not interested in the event.

Yes, the lost lock causes a temporary or permanent loss of video,
but with no other indications from the adv7180 (such as a detected
video standard change). And on i.mx6, the lost lock often (actually
usually) causes a split or rolling image.

Steve

Re: [PATCH v4 7/8] v4l: Add signal lock status to source change events

[Steve Longerbeam]

On 11/14/2016 11:19 AM, Steve Longerbeam wrote:
>
>
> On 11/14/2016 03:36 AM, Hans Verkuil wrote:
>> On 08/03/2016 08:03 PM, Steve Longerbeam wrote:
>>> Add a signal lock status change to the source changes bitmask.
>>> This indicates there was a signal lock or unlock event detected
>>> at the input of a video decoder.
>>>
>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>> Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>>
>>> ---
>>>
>>> v4:
>>> - converted to rst from DocBook
>>>
>>> v3: no changes
>>> v2: no changes
>>> ---
>>>   Documentation/media/uapi/v4l/vidioc-dqevent.rst | 9 +++++++++
>>>   Documentation/media/videodev2.h.rst.exceptions  | 1 +
>>>   include/uapi/linux/videodev2.h                  | 1 +
>>>   3 files changed, 11 insertions(+)
>>>
>>> diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst 
>>> b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>>> index 73c0d5b..7d8a053 100644
>>> --- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>>> +++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>>> @@ -564,6 +564,15 @@ call.
>>>         an input. This can come from an input connector or from a video
>>>         decoder.
>>>   +    -  .. row 2
>>> +
>>> +       -  ``V4L2_EVENT_SRC_CH_LOCK_STATUS``
>>> +
>>> +       -  0x0002
>>> +
>>> +       -  This event gets triggered when there is a signal lock or
>>> +      unlock detected at the input of a video decoder.
>>> +
>>>     Return Value
>>>   ============
>>> diff --git a/Documentation/media/videodev2.h.rst.exceptions 
>>> b/Documentation/media/videodev2.h.rst.exceptions
>>> index 9bb9a6c..f412cc8 100644
>>> --- a/Documentation/media/videodev2.h.rst.exceptions
>>> +++ b/Documentation/media/videodev2.h.rst.exceptions
>>> @@ -453,6 +453,7 @@ replace define V4L2_EVENT_CTRL_CH_FLAGS 
>>> ctrl-changes-flags
>>>   replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags
>>>     replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
>>> +replace define V4L2_EVENT_SRC_CH_LOCK_STATUS src-changes-flags
>>>     replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ 
>>> v4l2-event-motion-det
>>>   diff --git a/include/uapi/linux/videodev2.h 
>>> b/include/uapi/linux/videodev2.h
>>> index 724f43e..08a153f 100644
>>> --- a/include/uapi/linux/videodev2.h
>>> +++ b/include/uapi/linux/videodev2.h
>>> @@ -2078,6 +2078,7 @@ struct v4l2_event_frame_sync {
>>>   };
>>>     #define V4L2_EVENT_SRC_CH_RESOLUTION        (1 << 0)
>>> +#define V4L2_EVENT_SRC_CH_LOCK_STATUS        (1 << 1)
>>>     struct v4l2_event_src_change {
>>>       __u32 changes;
>>>
>> Quoting from an old (July) conversation about this:
>>
>>>>> I'm not entirely sure I like this. Typically losing lock means 
>>>>> that this event
>>>>> is triggered with the V4L2_EVENT_SRC_CH_RESOLUTION flag set, and 
>>>>> userspace has
>>>>> to check the new timings etc., which will fail if there is no lock 
>>>>> anymore.
>>>>>
>>>>> This information is also available through ENUMINPUT.
>>>>>
>>>>> I would need to know more about why you think this is needed, 
>>>>> because I don't
>>>>> see what this adds.
>>> Hi Hans,
>>>
>>> At least on the ADV718x, a source resolution change (from an
>>> autodetected video
>>> standard change) and a signal lock status change are distinct events.
>>> For example
>>> there can be a temporary loss of input signal lock without a change in
>>> detected
>>> input video standard/resolution.
>> OK, but what can the application do with that event? If the glitch 
>> didn't
>> affect the video, then it is pointless.
>
> Hi Hans, that's just it, for i.mx6 it does affect video. On i.mx6 a 
> temporary
> loss of signal from the adv7180 often results in a "split image", or 
> rolling
> image from captured frame to the next, and the only way to recover
> from this failure is to restart the pipeline (stream off -- stream 
> on). So
> the application needs to be informed of this temporary loss of signal
> event in order to restart streaming.
>
>
>>
>> If the lock is lost, then normally you loose video as well. If not, then
>> applications are not interested in the event.
>
> Yes, the lost lock causes a temporary or permanent loss of video,
> but with no other indications from the adv7180 (such as a detected
> video standard change). And on i.mx6, the lost lock often (actually
> usually) causes a split or rolling image.
>

Also it seems the sequence of actions from the application due to a
SRC_CH_LOCK_STATUS event would be in line with the action due to
SRC_CH_RESOLUTION. In other words for SRC_CH_RESOLUTION,
the app requests an update on the detected video standard via
QUERYSTD, and for SRC_CH_LOCK_STATUS, it requests an update
on input status (ENUMINPUTS).

Steve

[PATCH v4 8/8] media: adv7180: enable lock/unlock interrupts

[Steve Longerbeam]

Enable the SD lock/unlock interrupts and send V4L2_EVENT_SRC_CH_LOCK_STATUS
in the interrupt handler on a detected lock/unlock.

Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>

---

v4: no changes
v3: no changes

v2:
- last version of this patch was based on the old reverted autodetect
  code. This version now only enables the interrupt and sends the
  event in the interrupt handler.
---
 drivers/media/i2c/adv7180.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index abdc519..d017b05 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -874,19 +874,29 @@ static const struct v4l2_subdev_ops adv7180_ops = {
 static irqreturn_t adv7180_irq(int irq, void *devid)
 {
 	struct adv7180_state *state = devid;
-	u8 isr3;
+	u8 isr1, isr3;
 
 	mutex_lock(&state->mutex);
+	isr1 = adv7180_read(state, ADV7180_REG_ISR1);
 	isr3 = adv7180_read(state, ADV7180_REG_ISR3);
 	/* clear */
+	adv7180_write(state, ADV7180_REG_ICR1, isr1);
 	adv7180_write(state, ADV7180_REG_ICR3, isr3);
 
-	if (isr3 & ADV7180_IRQ3_AD_CHANGE) {
-		static const struct v4l2_event src_ch = {
+	if ((isr3 & ADV7180_IRQ3_AD_CHANGE) ||
+	    (isr1 & (ADV7180_IRQ1_LOCK | ADV7180_IRQ1_UNLOCK))) {
+		static struct v4l2_event src_ch = {
 			.type = V4L2_EVENT_SOURCE_CHANGE,
-			.u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
 		};
 
+		if (isr3 & ADV7180_IRQ3_AD_CHANGE)
+			src_ch.u.src_change.changes |=
+				V4L2_EVENT_SRC_CH_RESOLUTION;
+
+		if (isr1 & (ADV7180_IRQ1_LOCK | ADV7180_IRQ1_UNLOCK))
+			src_ch.u.src_change.changes |=
+				V4L2_EVENT_SRC_CH_LOCK_STATUS;
+
 		v4l2_subdev_notify_event(&state->sd, &src_ch);
 	}
 	mutex_unlock(&state->mutex);
@@ -1287,7 +1297,9 @@ static int init_device(struct adv7180_state *state)
 		if (ret < 0)
 			goto out_unlock;
 
-		ret = adv7180_write(state, ADV7180_REG_IMR1, 0);
+		/* enable lock/unlock interrupts */
+		ret = adv7180_write(state, ADV7180_REG_IMR1,
+				    ADV7180_IRQ1_LOCK | ADV7180_IRQ1_UNLOCK);
 		if (ret < 0)
 			goto out_unlock;
 
-- 
1.9.1

Re: [PATCH v4 0/8] adv7180 subdev fixes, v4

[Jack Mitchell]

On 03/08/16 19:03, Steve Longerbeam wrote:
> Steve Longerbeam (8):
>   media: adv7180: fix field type
>   media: adv7180: define more registers
>   media: adv7180: add support for NEWAVMODE
>   media: adv7180: add power pin control
>   media: adv7180: implement g_parm
>   media: adv7180: change mbus format to UYVY
>   v4l: Add signal lock status to source change events
>   media: adv7180: enable lock/unlock interrupts
>
>  .../devicetree/bindings/media/i2c/adv7180.txt      |   8 +
>  Documentation/media/uapi/v4l/vidioc-dqevent.rst    |   9 +
>  Documentation/media/videodev2.h.rst.exceptions     |   1 +
>  drivers/media/i2c/Kconfig                          |   2 +-
>  drivers/media/i2c/adv7180.c                        | 200 +++++++++++++++++----
>  include/uapi/linux/videodev2.h                     |   1 +
>  6 files changed, 183 insertions(+), 38 deletions(-)
>

Did anything come of this patchset, I see a few select patches from the 
original (full imx6) series have been merged in but only seems partial?

Cheers,
Jack.

Re: [PATCH v4 0/8] adv7180 subdev fixes, v4

[Hans Verkuil]

On 09/19/2016 04:19 PM, Jack Mitchell wrote:
> 
> 
> On 03/08/16 19:03, Steve Longerbeam wrote:
>> Steve Longerbeam (8):
>>   media: adv7180: fix field type
>>   media: adv7180: define more registers
>>   media: adv7180: add support for NEWAVMODE
>>   media: adv7180: add power pin control
>>   media: adv7180: implement g_parm
>>   media: adv7180: change mbus format to UYVY
>>   v4l: Add signal lock status to source change events
>>   media: adv7180: enable lock/unlock interrupts
>>
>>  .../devicetree/bindings/media/i2c/adv7180.txt      |   8 +
>>  Documentation/media/uapi/v4l/vidioc-dqevent.rst    |   9 +
>>  Documentation/media/videodev2.h.rst.exceptions     |   1 +
>>  drivers/media/i2c/Kconfig                          |   2 +-
>>  drivers/media/i2c/adv7180.c                        | 200 +++++++++++++++++----
>>  include/uapi/linux/videodev2.h                     |   1 +
>>  6 files changed, 183 insertions(+), 38 deletions(-)
>>
> 
> Did anything come of this patchset, I see a few select patches from the 
> original (full imx6) series have been merged in but only seems partial?

I cherry-picked a few patches, but most are still in my TODO list.

I need time to carefully look at the interlaced/NEWAVMODE support. So those
patches won't make 4.9 but will be postponed for 4.10.

Regards,

	Hans