[PATCH v3 0/9] adv7180 subdev fixes, v3

[PATCH v3 0/9] adv7180 subdev fixes, v3

[Steve Longerbeam]

Steve Longerbeam (9):
  media: adv7180: Fix broken interrupt register access
  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
  media: adv7180: fix field type

 Documentation/DocBook/media/v4l/vidioc-dqevent.xml |  12 +-
 .../devicetree/bindings/media/i2c/adv7180.txt      |   8 +
 drivers/media/i2c/Kconfig                          |   2 +-
 drivers/media/i2c/adv7180.c                        | 232 ++++++++++++++++-----
 include/uapi/linux/videodev2.h                     |   1 +
 5 files changed, 205 insertions(+), 50 deletions(-)

-- 
1.9.1

[PATCH v3 1/9] media: adv7180: Fix broken interrupt register access

[Steve Longerbeam]

Access to the interrupt page registers has been broken since at least
commit 3999e5d01da7 ("[media] adv7180: Do implicit register paging").
That commit forgot to add the interrupt page number to the register
defines.

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>

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

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index b77b0a4..95cbc85 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -100,7 +100,7 @@
 #define ADV7180_REG_IDENT 0x0011
 #define ADV7180_ID_7180 0x18
 
-#define ADV7180_REG_ICONF1		0x0040
+#define ADV7180_REG_ICONF1		0x2040
 #define ADV7180_ICONF1_ACTIVE_LOW	0x01
 #define ADV7180_ICONF1_PSYNC_ONLY	0x10
 #define ADV7180_ICONF1_ACTIVE_TO_CLR	0xC0
@@ -113,15 +113,15 @@
 
 #define ADV7180_IRQ1_LOCK	0x01
 #define ADV7180_IRQ1_UNLOCK	0x02
-#define ADV7180_REG_ISR1	0x0042
-#define ADV7180_REG_ICR1	0x0043
-#define ADV7180_REG_IMR1	0x0044
-#define ADV7180_REG_IMR2	0x0048
+#define ADV7180_REG_ISR1	0x2042
+#define ADV7180_REG_ICR1	0x2043
+#define ADV7180_REG_IMR1	0x2044
+#define ADV7180_REG_IMR2	0x2048
 #define ADV7180_IRQ3_AD_CHANGE	0x08
-#define ADV7180_REG_ISR3	0x004A
-#define ADV7180_REG_ICR3	0x004B
-#define ADV7180_REG_IMR3	0x004C
-#define ADV7180_REG_IMR4	0x50
+#define ADV7180_REG_ISR3	0x204A
+#define ADV7180_REG_ICR3	0x204B
+#define ADV7180_REG_IMR3	0x204C
+#define ADV7180_REG_IMR4	0x2050
 
 #define ADV7180_REG_NTSC_V_BIT_END	0x00E6
 #define ADV7180_NTSC_V_BIT_END_MANUAL_NVEND	0x4F
-- 
1.9.1

[PATCH v3 2/9] 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>

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 95cbc85..cb83ebb 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 v3 3/9] 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>

---

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 cb83ebb..3067d5f 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_INTERLACED;
 	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

[PATCH v3 4/9] 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>

---

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 3067d5f..58f4eca 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 v3 5/9] 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>

---
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 58f4eca..b8a6d94 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

[PATCH v3 6/9] 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>

---
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 b8a6d94..ef2a107 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 v3 7/9] 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>

---
v3: no changes
v2: no changes
---
 Documentation/DocBook/media/v4l/vidioc-dqevent.xml | 12 ++++++++++--
 include/uapi/linux/videodev2.h                     |  1 +
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
index c9c3c77..7758ad7 100644
--- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
@@ -233,8 +233,9 @@
 	    <entry>
 	      <para>This event is triggered when a source parameter change is
 	       detected during runtime by the video device. It can be a
-	       runtime resolution change triggered by a video decoder or the
-	       format change happening on an input connector.
+	       runtime resolution change or signal lock status change
+	       triggered by a video decoder, or the format change happening
+	       on an input connector.
 	       This event requires that the <structfield>id</structfield>
 	       matches the input index (when used with a video device node)
 	       or the pad index (when used with a subdevice node) from which
@@ -461,6 +462,13 @@
 	    from a video decoder.
 	    </entry>
 	  </row>
+	  <row>
+	    <entry><constant>V4L2_EVENT_SRC_CH_LOCK_STATUS</constant></entry>
+	    <entry>0x0002</entry>
+	    <entry>This event gets triggered when there is a signal lock or
+	    unlock detected at the input of a video decoder.
+	    </entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
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

[PATCH v3 8/9] 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>

---

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 ef2a107..6bcc652 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

[PATCH v3 9/9] media: adv7180: fix field type

[Steve Longerbeam]

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, set the field mode to SEQ_BT for PAL, and SEQ_TB for NTSC (there
seems to be conflicting info on field order of PAL vs NTSC, so follow
what is in adv7183.c).

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

---

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 | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 6bcc652..3b8041e 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -718,10 +718,17 @@ 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 =
+				(state->curr_norm & V4L2_STD_525_60) ?
+				V4L2_FIELD_SEQ_TB : V4L2_FIELD_SEQ_BT;
 		break;
 	default:
-		format->format.field = V4L2_FIELD_INTERLACED;
+		if (state->chip_info->flags & ADV7180_FLAG_I2P)
+			format->format.field = V4L2_FIELD_INTERLACED;
+		else
+			format->format.field =
+				(state->curr_norm & V4L2_STD_525_60) ?
+				V4L2_FIELD_SEQ_TB : V4L2_FIELD_SEQ_BT;
 		break;
 	}
 
@@ -1365,8 +1372,14 @@ static int adv7180_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	state->client = client;
-	state->field = V4L2_FIELD_INTERLACED;
 	state->chip_info = (struct adv7180_chip_info *)id->driver_data;
+	state->curr_norm = V4L2_STD_NTSC;
+
+	if (state->chip_info->flags & ADV7180_FLAG_I2P)
+		state->field = V4L2_FIELD_INTERLACED;
+	else
+		state->field = (state->curr_norm & V4L2_STD_525_60) ?
+			V4L2_FIELD_SEQ_TB : V4L2_FIELD_SEQ_BT;
 
 	adv7180_of_parse(state);
 
@@ -1396,7 +1409,6 @@ static int adv7180_probe(struct i2c_client *client,
 
 	state->irq = client->irq;
 	mutex_init(&state->mutex);
-	state->curr_norm = V4L2_STD_NTSC;
 	if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
 		state->powered = true;
 	else
-- 
1.9.1

Re: [PATCH v3 3/9] media: adv7180: add support for NEWAVMODE

[Ian Arkver]

Resend due to HTML email bounce.

On 23/07/16 18:00, 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>
>
> ---
>
> 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 cb83ebb..3067d5f 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
See below re this value.
>   #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;

According to the ADV7180 datasheet, rev. J, page 48 [1], when NEWAVMODE 
is 0,
no adjustments are possible to the output timings, which would imply 
this write
is ignored after your edit.

1: www.analog.com/media/en/technical-documentation/data-sheets/ADV7180.pdf
>   }
>   
>   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;
> +	}
> +

Again according to the DS, NEWAVMODE is set by default and enables a
stream which is by default CCIR656 compliant. Your edit writes 0x02 to
this register which actually clears NEWAVMODE (bit 4).
>   	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);
> +}
> +

I'm not sure what the use case is for clearing NEWAVMODE, in which case
this code and the call below are not needed.

Maybe this particular patch should be dropped?
>   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_INTERLACED;
>   	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,
IanJ

Re: [PATCH v3 3/9] media: adv7180: add support for NEWAVMODE

[Steve Longerbeam]

On 07/25/2016 05:04 AM, Ian Arkver wrote:
> On 23/07/16 18:00, 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>
>>
>> ---
>>
>> 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 cb83ebb..3067d5f 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
>
> See below re this value.
>

Hi Ian, I double-checked the ADV7180 datasheet, this value is
correct. Bit 4, when cleared, _enables_ NEWAVMODE.


>>  #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;
>
> According to the ADV7180 datasheet, rev. J, page 48 [1], when NEWAVMODE is 0,
> no adjustments are possible to the output timings, which would imply this write
> is ignored after your edit.

That's correct, when NEWAVMODE is enabled (by clearing reg 0x31 bit 4), no
adjustments are possible to the output timings:

>From the rev J datasheet, page 91, when reg 0x31 bit 4 is set (NEAVMODE disabled):

"Manual VS/FIELD position controlled by Register 0x32, Register 0x33, and
Register 0xE5 to Register 0xEA"

>
> 1: www.analog.com/media/en/technical-documentation/data-sheets/*ADV7180*.pdf
>
>>  }
>>  
>>  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;
>> +	}
>> +
>
> Again according to the DS, NEWAVMODE is set by default and enables a
> stream which is by default CCIR656 compliant. Your edit writes 0x02 to
> this register which actually clears NEWAVMODE (bit 4).

correct, it clears bit 4 which _enables_ NEWAVMODE.

NEWAVMODE by default is _disabled_ (bit 4 is set).


Steve

Re: [PATCH v3 3/9] media: adv7180: add support for NEWAVMODE

[Ian Arkver]

On 25/07/16 18:55, Steve Longerbeam wrote:
> On 07/25/2016 05:04 AM, Ian Arkver wrote:
>> On 23/07/16 18:00, 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>
>>>
>>> ---
>>>
>>> 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 cb83ebb..3067d5f 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
>> See below re this value.
>>
> Hi Ian, I double-checked the ADV7180 datasheet, this value is
> correct. Bit 4, when cleared, _enables_ NEWAVMODE.

Hah, ok. I'm not familiar enough with the history of this chip and didn't
know what "OLDAVMODE" was. So, to enable NEWAVMODE you clear
the NEWAVMODE bit. That makes perfect sense.

Anyway, I still don't see what NEWAVMODE gets you. As
far as I can see it just locks down the timings and removes the flexibility
the chip otherwise offers to move the BT656 SAV and EAV codes around
relative to the incoming video.

In what circumstances would you need to set the newavmode property
and change this default behaviour? We're not coupling the adv7180
back-to-back with an ADV video encoder here, which is what
NEWAVMODE is for and is presumably why AD recommend it for their
eval boards. We're trying to get a BT656 compliant stream, which is
what the default mode purports to generate.

Regards,
IanJ
>
>>>   #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;
>> According to the ADV7180 datasheet, rev. J, page 48 [1], when NEWAVMODE is 0,
>> no adjustments are possible to the output timings, which would imply this write
>> is ignored after your edit.
> That's correct, when NEWAVMODE is enabled (by clearing reg 0x31 bit 4), no
> adjustments are possible to the output timings:
>
>  From the rev J datasheet, page 91, when reg 0x31 bit 4 is set (NEAVMODE disabled):
>
> "Manual VS/FIELD position controlled by Register 0x32, Register 0x33, and
> Register 0xE5 to Register 0xEA"
>
>> 1: www.analog.com/media/en/technical-documentation/data-sheets/*ADV7180*.pdf
>>
>>>   }
>>>   
>>>   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;
>>> +	}
>>> +
>> Again according to the DS, NEWAVMODE is set by default and enables a
>> stream which is by default CCIR656 compliant. Your edit writes 0x02 to
>> this register which actually clears NEWAVMODE (bit 4).
> correct, it clears bit 4 which _enables_ NEWAVMODE.
>
> NEWAVMODE by default is _disabled_ (bit 4 is set).
>
>
> Steve
>

Re: [PATCH v3 3/9] media: adv7180: add support for NEWAVMODE

[Steve Longerbeam]

On 07/25/2016 12:36 PM, Ian Arkver wrote:
> On 25/07/16 18:55, Steve Longerbeam wrote:
>> On 07/25/2016 05:04 AM, Ian Arkver wrote:
>>> On 23/07/16 18:00, Steve Longerbeam wrote:
>>>> <snip>
>>>> +#define ADV7180_VSYNC_FIELD_CTL_1_NEWAVMODE 0x02
>>> See below re this value.
>>>
>> Hi Ian, I double-checked the ADV7180 datasheet, this value is
>> correct. Bit 4, when cleared, _enables_ NEWAVMODE.
>
> Hah, ok. I'm not familiar enough with the history of this chip and didn't
> know what "OLDAVMODE" was. So, to enable NEWAVMODE you clear
> the NEWAVMODE bit. That makes perfect sense.
>
> Anyway, I still don't see what NEWAVMODE gets you.

Hi Ian,

With video standard auto-detect disabled in the chip (VID_SEL > 2), 
captured NTSC
images by the i.mx6q SabreAuto are corrupted, best I can describe it as 
"extremely
fuzzy". Only when newavmode is enabled do the images look good again, in 
manual
mode. With auto-detect enabled, images look good with or without newavmode.

The strange this is, the auto-detected standard is identical to the 
standard set
explicitly in manual mode (NTSC-M). I did a complete i2c dump of the 
registers
for both auto-detect and manual mode, and found no other differences besides
the auto-detect/manual setting.

Trying to track this down further would probably require a logic 
analyzer on the
bt.656 bus, which I don't have access to.

I will not be debugging this further so NEWAVMODE it will have to remain.

Steve



> As
> far as I can see it just locks down the timings and removes the 
> flexibility
> the chip otherwise offers to move the BT656 SAV and EAV codes around
> relative to the incoming video.
>
> In what circumstances would you need to set the newavmode property
> and change this default behaviour? We're not coupling the adv7180
> back-to-back with an ADV video encoder here, which is what
> NEWAVMODE is for and is presumably why AD recommend it for their
> eval boards. We're trying to get a BT656 compliant stream, which is
> what the default mode purports to generate.
>
> Regards,
> IanJ
>

Re: [PATCH v3 3/9] media: adv7180: add support for NEWAVMODE

[Ian Arkver]

On 25/07/16 23:04, Steve Longerbeam wrote:
>
>
> On 07/25/2016 12:36 PM, Ian Arkver wrote:
>> On 25/07/16 18:55, Steve Longerbeam wrote:
>>> On 07/25/2016 05:04 AM, Ian Arkver wrote:
>>>> On 23/07/16 18:00, Steve Longerbeam wrote:
>>>>> <snip>
>>>>> +#define ADV7180_VSYNC_FIELD_CTL_1_NEWAVMODE 0x02
>>>> See below re this value.
>>>>
>>> Hi Ian, I double-checked the ADV7180 datasheet, this value is
>>> correct. Bit 4, when cleared, _enables_ NEWAVMODE.
>>
>> Hah, ok. I'm not familiar enough with the history of this chip and 
>> didn't
>> know what "OLDAVMODE" was. So, to enable NEWAVMODE you clear
>> the NEWAVMODE bit. That makes perfect sense.
>>
>> Anyway, I still don't see what NEWAVMODE gets you.
>
> Hi Ian,
>
> With video standard auto-detect disabled in the chip (VID_SEL > 2), 
> captured NTSC
> images by the i.mx6q SabreAuto are corrupted, best I can describe it 
> as "extremely
> fuzzy". Only when newavmode is enabled do the images look good again, 
> in manual
> mode. With auto-detect enabled, images look good with or without 
> newavmode.
>
> The strange this is, the auto-detected standard is identical to the 
> standard set
> explicitly in manual mode (NTSC-M). I did a complete i2c dump of the 
> registers
> for both auto-detect and manual mode, and found no other differences 
> besides
> the auto-detect/manual setting.
>
> Trying to track this down further would probably require a logic 
> analyzer on the
> bt.656 bus, which I don't have access to.
>
> I will not be debugging this further so NEWAVMODE it will have to remain.
>
> Steve

OK, interesting. And weird indeed.

I may be interfacing an ADV7280 to the i.MX6 in the August timeframe, 
depending on
project needs etc. I'll see if I hit this with that chip. My test app 
does use autodetect.

Incidentally, looking at the BT656-5 spec and comparing to the tvp5150, 
I see that
the spec calls for 244 and 243 lines per field for NTSC, and the tvp5150 
provides
that number of lines. However this write...

adv7180_write(state, ADV7180_REG_NTSC_V_BIT_END,
             ADV7180_NTSC_V_BIT_END_MANUAL_NVEND);

where NVEND is 0x4f, configures the adv7180 to send only 242 lines in 
each field.
Not sure if this is significant.

Regards,
IanJ.

>
>> As
>> far as I can see it just locks down the timings and removes the 
>> flexibility
>> the chip otherwise offers to move the BT656 SAV and EAV codes around
>> relative to the incoming video.
>>
>> In what circumstances would you need to set the newavmode property
>> and change this default behaviour? We're not coupling the adv7180
>> back-to-back with an ADV video encoder here, which is what
>> NEWAVMODE is for and is presumably why AD recommend it for their
>> eval boards. We're trying to get a BT656 compliant stream, which is
>> what the default mode purports to generate.
>>
>> Regards,
>> IanJ
>>
>
>

Re: [PATCH v3 3/9] media: adv7180: add support for NEWAVMODE

[Steve Longerbeam]

On 07/25/2016 03:24 PM, Ian Arkver wrote:
> On 25/07/16 23:04, Steve Longerbeam wrote:
>>
>>
>> On 07/25/2016 12:36 PM, Ian Arkver wrote:
>>> On 25/07/16 18:55, Steve Longerbeam wrote:
>>>> On 07/25/2016 05:04 AM, Ian Arkver wrote:
>>>>> On 23/07/16 18:00, Steve Longerbeam wrote:
>>>>>> <snip>
>>>>>> +#define ADV7180_VSYNC_FIELD_CTL_1_NEWAVMODE 0x02
>>>>> See below re this value.
>>>>>
>>>> Hi Ian, I double-checked the ADV7180 datasheet, this value is
>>>> correct. Bit 4, when cleared, _enables_ NEWAVMODE.
>>>
>>> Hah, ok. I'm not familiar enough with the history of this chip and didn't
>>> know what "OLDAVMODE" was. So, to enable NEWAVMODE you clear
>>> the NEWAVMODE bit. That makes perfect sense.
>>>
>>> Anyway, I still don't see what NEWAVMODE gets you.
>>
>> Hi Ian,
>>
>> With video standard auto-detect disabled in the chip (VID_SEL > 2), captured NTSC
>> images by the i.mx6q SabreAuto are corrupted, best I can describe it as "extremely
>> fuzzy". Only when newavmode is enabled do the images look good again, in manual
>> mode. With auto-detect enabled, images look good with or without newavmode.
>>
>> The strange this is, the auto-detected standard is identical to the standard set
>> explicitly in manual mode (NTSC-M). I did a complete i2c dump of the registers
>> for both auto-detect and manual mode, and found no other differences besides
>> the auto-detect/manual setting.
>>
>> Trying to track this down further would probably require a logic analyzer on the
>> bt.656 bus, which I don't have access to.
>>
>> I will not be debugging this further so NEWAVMODE it will have to remain.
>>
>> Steve
>
> OK, interesting. And weird indeed.
>
> I may be interfacing an ADV7280 to the i.MX6 in the August timeframe, depending on
> project needs etc. I'll see if I hit this with that chip. My test app does use autodetect.
>
> Incidentally, looking at the BT656-5 spec and comparing to the tvp5150, I see that
> the spec calls for 244 and 243 lines per field for NTSC, and the tvp5150 provides
> that number of lines. However this write...
>
> adv7180_write(state, ADV7180_REG_NTSC_V_BIT_END,
>             ADV7180_NTSC_V_BIT_END_MANUAL_NVEND);
>
> where NVEND is 0x4f, configures the adv7180 to send only 242 lines in each field.
> Not sure if this is significant.

Right. I thought for sure that was the reason for the image problems, but
I tried commenting out that write (default value for ADV7180_REG_NTSC_V_BIT_END)
with NEWAVMODE disabled, and still the images were corrupted in manual mode.

Steve