* [PATCH 00/11] adv7180 subdev fixes
@ 2016-07-06 22:59 Steve Longerbeam
2016-07-06 22:59 ` [PATCH 01/11] media: adv7180: Fix broken interrupt register access Steve Longerbeam
` (12 more replies)
0 siblings, 13 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-06 22:59 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Steve Longerbeam (11):
media: adv7180: Fix broken interrupt register access
Revert "[media] adv7180: fix broken standards handling"
media: adv7180: add power pin control
media: adv7180: implement g_parm
media: adv7180: init chip with AD recommended register settings
media: adv7180: add bt.656-4 OF property
media: adv7180: change mbus format to UYVY
adv7180: send V4L2_EVENT_SOURCE_CHANGE on std change
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 +-
drivers/media/i2c/Kconfig | 2 +-
drivers/media/i2c/adv7180.c | 409 +++++++++++++++------
include/uapi/linux/videodev2.h | 1 +
4 files changed, 308 insertions(+), 116 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH 01/11] media: adv7180: Fix broken interrupt register access
2016-07-06 22:59 [PATCH 00/11] adv7180 subdev fixes Steve Longerbeam
@ 2016-07-06 22:59 ` Steve Longerbeam
2016-07-07 14:44 ` Tim Harvey
2016-07-07 15:37 ` Lars-Peter Clausen
2016-07-06 22:59 ` [PATCH 02/11] Revert "[media] adv7180: fix broken standards handling" Steve Longerbeam
` (11 subsequent siblings)
12 siblings, 2 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-06 22:59 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Access to the interrupt page registers has been broken since
at least 3999e5d01da74f1a22afbb0b61b3992fea301478. That commit
forgot to add the inerrupt page number to the register defines.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 02/11] Revert "[media] adv7180: fix broken standards handling"
2016-07-06 22:59 [PATCH 00/11] adv7180 subdev fixes Steve Longerbeam
2016-07-06 22:59 ` [PATCH 01/11] media: adv7180: Fix broken interrupt register access Steve Longerbeam
@ 2016-07-06 22:59 ` Steve Longerbeam
2016-07-07 14:48 ` Tim Harvey
2016-07-07 15:45 ` Lars-Peter Clausen
2016-07-06 22:59 ` [PATCH 03/11] media: adv7180: add power pin control Steve Longerbeam
` (10 subsequent siblings)
12 siblings, 2 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-06 22:59 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Autodetect was likely broken only because access to the
interrupt registers were broken, so there were no standard
change interrupts. After fixing that, and reverting this,
autodetect seems to work just fine on an i.mx6q SabreAuto.
This reverts commit 937feeed3f0ae8a0389d5732f6db63dd912acd99.
---
drivers/media/i2c/adv7180.c | 118 ++++++++++++++------------------------------
1 file changed, 38 insertions(+), 80 deletions(-)
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 95cbc85..967303a 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -26,9 +26,8 @@
#include <linux/i2c.h>
#include <linux/slab.h>
#include <linux/of.h>
-#include <linux/videodev2.h>
#include <media/v4l2-ioctl.h>
-#include <media/v4l2-event.h>
+#include <linux/videodev2.h>
#include <media/v4l2-device.h>
#include <media/v4l2-ctrls.h>
#include <linux/mutex.h>
@@ -193,8 +192,8 @@ struct adv7180_state {
struct mutex mutex; /* mutual excl. when accessing chip */
int irq;
v4l2_std_id curr_norm;
+ bool autodetect;
bool powered;
- bool streaming;
u8 input;
struct i2c_client *client;
@@ -339,26 +338,12 @@ static int adv7180_querystd(struct v4l2_subdev *sd, v4l2_std_id *std)
if (err)
return err;
- if (state->streaming) {
- err = -EBUSY;
- goto unlock;
- }
-
- err = adv7180_set_video_standard(state,
- ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM);
- if (err)
- goto unlock;
-
- msleep(100);
- __adv7180_status(state, NULL, std);
-
- err = v4l2_std_to_adv7180(state->curr_norm);
- if (err < 0)
- goto unlock;
-
- err = adv7180_set_video_standard(state, err);
+ /* when we are interrupt driven we know the state */
+ if (!state->autodetect || state->irq > 0)
+ *std = state->curr_norm;
+ else
+ err = __adv7180_status(state, NULL, std);
-unlock:
mutex_unlock(&state->mutex);
return err;
}
@@ -402,13 +387,23 @@ static int adv7180_program_std(struct adv7180_state *state)
{
int ret;
- ret = v4l2_std_to_adv7180(state->curr_norm);
- if (ret < 0)
- return ret;
+ if (state->autodetect) {
+ ret = adv7180_set_video_standard(state,
+ ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM);
+ if (ret < 0)
+ return ret;
+
+ __adv7180_status(state, NULL, &state->curr_norm);
+ } else {
+ ret = v4l2_std_to_adv7180(state->curr_norm);
+ if (ret < 0)
+ return ret;
+
+ ret = adv7180_set_video_standard(state, ret);
+ if (ret < 0)
+ return ret;
+ }
- ret = adv7180_set_video_standard(state, ret);
- if (ret < 0)
- return ret;
return 0;
}
@@ -420,12 +415,18 @@ static int adv7180_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
if (ret)
return ret;
- /* Make sure we can support this std */
- ret = v4l2_std_to_adv7180(std);
- if (ret < 0)
- goto out;
+ /* all standards -> autodetect */
+ if (std == V4L2_STD_ALL) {
+ state->autodetect = true;
+ } else {
+ /* Make sure we can support this std */
+ ret = v4l2_std_to_adv7180(std);
+ if (ret < 0)
+ goto out;
- state->curr_norm = std;
+ state->curr_norm = std;
+ state->autodetect = false;
+ }
ret = adv7180_program_std(state);
out:
@@ -746,40 +747,6 @@ static int adv7180_g_tvnorms(struct v4l2_subdev *sd, v4l2_std_id *norm)
return 0;
}
-static int adv7180_s_stream(struct v4l2_subdev *sd, int enable)
-{
- struct adv7180_state *state = to_state(sd);
- int ret;
-
- /* It's always safe to stop streaming, no need to take the lock */
- if (!enable) {
- state->streaming = enable;
- return 0;
- }
-
- /* Must wait until querystd released the lock */
- ret = mutex_lock_interruptible(&state->mutex);
- if (ret)
- return ret;
- state->streaming = enable;
- mutex_unlock(&state->mutex);
- return 0;
-}
-
-static int adv7180_subscribe_event(struct v4l2_subdev *sd,
- struct v4l2_fh *fh,
- struct v4l2_event_subscription *sub)
-{
- switch (sub->type) {
- case V4L2_EVENT_SOURCE_CHANGE:
- return v4l2_src_change_event_subdev_subscribe(sd, fh, sub);
- case V4L2_EVENT_CTRL:
- return v4l2_ctrl_subdev_subscribe_event(sd, fh, sub);
- default:
- return -EINVAL;
- }
-}
-
static const struct v4l2_subdev_video_ops adv7180_video_ops = {
.s_std = adv7180_s_std,
.g_std = adv7180_g_std,
@@ -789,13 +756,10 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops = {
.g_mbus_config = adv7180_g_mbus_config,
.cropcap = adv7180_cropcap,
.g_tvnorms = adv7180_g_tvnorms,
- .s_stream = adv7180_s_stream,
};
static const struct v4l2_subdev_core_ops adv7180_core_ops = {
.s_power = adv7180_s_power,
- .subscribe_event = adv7180_subscribe_event,
- .unsubscribe_event = v4l2_event_subdev_unsubscribe,
};
static const struct v4l2_subdev_pad_ops adv7180_pad_ops = {
@@ -820,14 +784,8 @@ static irqreturn_t adv7180_irq(int irq, void *devid)
/* clear */
adv7180_write(state, ADV7180_REG_ICR3, isr3);
- if (isr3 & ADV7180_IRQ3_AD_CHANGE) {
- static const struct v4l2_event src_ch = {
- .type = V4L2_EVENT_SOURCE_CHANGE,
- .u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
- };
-
- v4l2_subdev_notify_event(&state->sd, &src_ch);
- }
+ if (isr3 & ADV7180_IRQ3_AD_CHANGE && state->autodetect)
+ __adv7180_status(state, NULL, &state->curr_norm);
mutex_unlock(&state->mutex);
return IRQ_HANDLED;
@@ -1272,7 +1230,7 @@ static int adv7180_probe(struct i2c_client *client,
state->irq = client->irq;
mutex_init(&state->mutex);
- state->curr_norm = V4L2_STD_NTSC;
+ state->autodetect = true;
if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
state->powered = true;
else
@@ -1280,7 +1238,7 @@ static int adv7180_probe(struct i2c_client *client,
state->input = 0;
sd = &state->sd;
v4l2_i2c_subdev_init(sd, client, &adv7180_ops);
- sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
+ sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
ret = adv7180_init_controls(state);
if (ret)
--
1.9.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 03/11] media: adv7180: add power pin control
2016-07-06 22:59 [PATCH 00/11] adv7180 subdev fixes Steve Longerbeam
2016-07-06 22:59 ` [PATCH 01/11] media: adv7180: Fix broken interrupt register access Steve Longerbeam
2016-07-06 22:59 ` [PATCH 02/11] Revert "[media] adv7180: fix broken standards handling" Steve Longerbeam
@ 2016-07-06 22:59 ` Steve Longerbeam
2016-07-07 15:04 ` Tim Harvey
2016-07-07 15:35 ` Lars-Peter Clausen
2016-07-06 22:59 ` [PATCH 04/11] media: adv7180: implement g_parm Steve Longerbeam
` (9 subsequent siblings)
12 siblings, 2 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-06 22:59 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Some targets control the ADV7180 power pin via a gpio, so add
support for "pwdn-gpio" pin control.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/media/i2c/Kconfig | 2 +-
drivers/media/i2c/adv7180.c | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 993dc50..80d39f6 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 967303a..38e5161 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 <media/v4l2-ioctl.h>
#include <linux/videodev2.h>
#include <media/v4l2-device.h>
@@ -191,6 +192,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 autodetect;
bool powered;
@@ -443,6 +445,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;
@@ -1143,6 +1158,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);
@@ -1190,6 +1207,20 @@ out_unlock:
return ret;
}
+static int adv7180_of_parse(struct adv7180_state *state)
+{
+ struct i2c_client *client = state->client;
+
+ state->pwdn_gpio = devm_gpiod_get_optional(&client->dev, "pwdn",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(state->pwdn_gpio)) {
+ v4l_err(client, "request for power pin failed\n");
+ return PTR_ERR(state->pwdn_gpio);
+ }
+
+ return 0;
+}
+
static int adv7180_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -1212,6 +1243,10 @@ static int adv7180_probe(struct i2c_client *client,
state->field = V4L2_FIELD_INTERLACED;
state->chip_info = (struct adv7180_chip_info *)id->driver_data;
+ ret = adv7180_of_parse(state);
+ if (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);
@@ -1303,6 +1338,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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 04/11] media: adv7180: implement g_parm
2016-07-06 22:59 [PATCH 00/11] adv7180 subdev fixes Steve Longerbeam
` (2 preceding siblings ...)
2016-07-06 22:59 ` [PATCH 03/11] media: adv7180: add power pin control Steve Longerbeam
@ 2016-07-06 22:59 ` Steve Longerbeam
2016-07-07 15:04 ` Tim Harvey
2016-07-06 22:59 ` [PATCH 05/11] media: adv7180: init chip with AD recommended register settings Steve Longerbeam
` (8 subsequent siblings)
12 siblings, 1 reply; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-06 22:59 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Implement g_parm to return the current standard's frame period.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
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 38e5161..42816d4 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -741,6 +741,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);
@@ -765,6 +786,7 @@ static int adv7180_g_tvnorms(struct v4l2_subdev *sd, v4l2_std_id *norm)
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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 05/11] media: adv7180: init chip with AD recommended register settings
2016-07-06 22:59 [PATCH 00/11] adv7180 subdev fixes Steve Longerbeam
` (3 preceding siblings ...)
2016-07-06 22:59 ` [PATCH 04/11] media: adv7180: implement g_parm Steve Longerbeam
@ 2016-07-06 22:59 ` Steve Longerbeam
2016-07-07 15:23 ` Tim Harvey
2016-07-07 15:29 ` Lars-Peter Clausen
2016-07-06 22:59 ` [PATCH 06/11] media: adv7180: add bt.656-4 OF property Steve Longerbeam
` (7 subsequent siblings)
12 siblings, 2 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-06 22:59 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Define and load register tables that conform to Analog Device's
recommended register settings. It loads the default single-ended
CVBS on Ain1 configuration for both ADV7180 and ADV7182 chips.
New register addresses have been defined for the tables. Those new
defines are also used in existing locations where hard-coded addresses
were used.
Note this patch also enables NEWAVMODE, which is also recommended by
Analog Devices. This will likely break any current backends using this
subdev that are expecting different or manually configured AV codes.
Note also that bt.656-4 support has been removed in this patch, but it
will be brought back in a subsequent patch.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/media/i2c/adv7180.c | 168 ++++++++++++++++++++++++++++++++++----------
1 file changed, 130 insertions(+), 38 deletions(-)
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 42816d4..92e2f37 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
@@ -209,6 +230,11 @@ struct adv7180_state {
struct adv7180_state, \
ctrl_hdl)->sd)
+struct adv7180_reg_tbl_t {
+ unsigned int reg;
+ unsigned int val;
+};
+
static int adv7180_select_page(struct adv7180_state *state, unsigned int page)
{
if (state->register_page != page) {
@@ -235,6 +261,20 @@ static int adv7180_read(struct adv7180_state *state, unsigned int reg)
return i2c_smbus_read_byte_data(state->client, reg & 0xff);
}
+static int adv7180_load_reg_tbl(struct adv7180_state *state,
+ const struct adv7180_reg_tbl_t *tbl, int n)
+{
+ int ret, i;
+
+ for (i = 0; i < n; i++) {
+ ret = adv7180_write(state, tbl[i].reg, tbl[i].val);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static int adv7180_csi_write(struct adv7180_state *state, unsigned int reg,
unsigned int value)
{
@@ -828,19 +868,36 @@ static irqreturn_t adv7180_irq(int irq, void *devid)
return IRQ_HANDLED;
}
+/*
+ * This register table conforms to Analog Device's Register Settings
+ * Recommendation for the ADV7180. It configures single-ended CVBS
+ * input on Ain1, and enables NEWAVMODE.
+ */
+static const struct adv7180_reg_tbl_t adv7180_single_ended_cvbs[] = {
+ /* Set analog mux for CVBS on Ain1 */
+ { ADV7180_REG_INPUT_CONTROL, 0x00 },
+ /* ADI Required Write: Reset Clamp Circuitry */
+ { ADV7180_REG_ANALOG_CLAMP_CTL, 0x30 },
+ /* Enable SFL Output */
+ { ADV7180_REG_EXTENDED_OUTPUT_CONTROL, 0x57 },
+ /* Select SH1 Chroma Shaping Filter */
+ { ADV7180_REG_SHAP_FILTER_CTL_1, 0x41 },
+ /* Enable NEWAVMODE */
+ { ADV7180_REG_VSYNC_FIELD_CTL_1, 0x02 },
+ /* ADI Required Write: optimize windowing function Step 1,2,3 */
+ { ADV7180_REG_MANUAL_WIN_CTL_1, 0xA2 },
+ { ADV7180_REG_MANUAL_WIN_CTL_2, 0x6A },
+ { ADV7180_REG_MANUAL_WIN_CTL_3, 0xA0 },
+ /* ADI Required Write: Enable ADC step 1,2,3 */
+ { 0x8055, 0x81 }, /* undocumented register 0x55 */
+ /* Recommended AFE I BIAS Setting for CVBS mode */
+ { ADV7180_REG_CVBS_TRIM, 0x0D },
+};
+
static int adv7180_init(struct adv7180_state *state)
{
- int ret;
-
- /* ITU-R BT.656-4 compatible */
- ret = adv7180_write(state, ADV7180_REG_EXTENDED_OUTPUT_CONTROL,
- ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS);
- 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);
+ return adv7180_load_reg_tbl(state, adv7180_single_ended_cvbs,
+ ARRAY_SIZE(adv7180_single_ended_cvbs));
}
static int adv7180_set_std(struct adv7180_state *state, unsigned int std)
@@ -862,8 +919,48 @@ static int adv7180_select_input(struct adv7180_state *state, unsigned int input)
return adv7180_write(state, ADV7180_REG_INPUT_CONTROL, ret);
}
+/*
+ * This register table conforms to Analog Device's Register Settings
+ * Recommendation revision C for the ADV7182. It configures single-ended
+ * CVBS inputs on Ain1, and enables NEWAVMODE.
+ */
+static const struct adv7180_reg_tbl_t adv7182_single_ended_cvbs[] = {
+ /* Exit Power Down Mode */
+ { ADV7180_REG_PWR_MAN, 0x00 },
+ /* Enable ADV7182 for 28.63636 MHz Crystal Clock Input */
+ { ADV7180_REG_STATUS3, 0x00 },
+ /* Set optimized IBIAS for single-ended CVBS input */
+ { ADV7180_REG_CVBS_TRIM, 0xCD },
+ /* Switch to single-ended CVBS on AIN1 */
+ { ADV7180_REG_INPUT_CONTROL, 0x00 },
+ /* ADI Required Write: Reset Current Clamp Circuitry steps 1,2,3,4 */
+ { ADV7180_REG_RST_CLAMP, 0x00 },
+ { ADV7180_REG_RST_CLAMP, 0xFF },
+ /* Select SH1 Chroma Shaping Filter */
+ { ADV7180_REG_SHAP_FILTER_CTL_1, 0x41 },
+ /* Enable Pixel & Sync output drivers */
+ { ADV7180_REG_OUTPUT_CONTROL, 0x0C },
+ /* Power-up INTRQ, HS and VS/FIELD/SFL pad */
+ { ADV7180_REG_EXTENDED_OUTPUT_CONTROL, 0x07 },
+ /* Enable LLC Output Driver */
+ { ADV7180_REG_CTRL_2, 0x40 },
+ /* Optimize ACE Performance */
+ { ADV7180_REG_ACE_CTRL5, 0x00 },
+ /* Enable ACE Feature */
+ { ADV7180_REG_ACE_CTRL1, 0x80 },
+ /* Enable NEWAVMODE */
+ { ADV7180_REG_VSYNC_FIELD_CTL_1, 0x02 },
+};
+
static int adv7182_init(struct adv7180_state *state)
{
+ int ret;
+
+ ret = adv7180_load_reg_tbl(state, adv7182_single_ended_cvbs,
+ ARRAY_SIZE(adv7182_single_ended_cvbs));
+ if (ret)
+ return ret;
+
if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2)
adv7180_write(state, ADV7180_REG_CSI_SLAVE_ADDR,
ADV7180_DEFAULT_CSI_I2C_ADDR << 1);
@@ -881,20 +978,15 @@ 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);
- else
- adv7180_write(state, 0x0004, 0x07);
- adv7180_write(state, 0x0003, 0x0c);
- adv7180_write(state, 0x001d, 0x40);
+ adv7180_write(state, ADV7180_REG_EXTENDED_OUTPUT_CONTROL,
+ 0x17);
}
- adv7180_write(state, 0x0013, 0x00);
-
return 0;
}
@@ -967,8 +1059,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);
@@ -976,10 +1068,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;
}
@@ -989,21 +1081,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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 06/11] media: adv7180: add bt.656-4 OF property
2016-07-06 22:59 [PATCH 00/11] adv7180 subdev fixes Steve Longerbeam
` (4 preceding siblings ...)
2016-07-06 22:59 ` [PATCH 05/11] media: adv7180: init chip with AD recommended register settings Steve Longerbeam
@ 2016-07-06 22:59 ` Steve Longerbeam
2016-07-07 14:52 ` Lars-Peter Clausen
2016-07-06 23:00 ` [PATCH 07/11] media: adv7180: change mbus format to UYVY Steve Longerbeam
` (6 subsequent siblings)
12 siblings, 1 reply; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-06 22:59 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Add a device tree boolean property "bt656-4" to allow setting
the ITU-R BT.656-4 compatible bit.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/media/i2c/adv7180.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 92e2f37..fff887c 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -58,7 +58,7 @@
#define ADV7180_REG_OUTPUT_CONTROL 0x0003
#define ADV7180_REG_EXTENDED_OUTPUT_CONTROL 0x0004
-#define ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS 0xC5
+#define ADV7180_EXTENDED_OUTPUT_CONTROL_BT656_4 0x80
#define ADV7180_REG_AUTODETECT_ENABLE 0x0007
#define ADV7180_AUTODETECT_DEFAULT 0x7f
@@ -216,6 +216,7 @@ struct adv7180_state {
struct gpio_desc *pwdn_gpio;
v4l2_std_id curr_norm;
bool autodetect;
+ bool bt656_4; /* use bt.656-4 standard for NTSC */
bool powered;
u8 input;
@@ -1281,6 +1282,17 @@ static int init_device(struct adv7180_state *state)
if (ret)
goto out_unlock;
+ if (state->bt656_4) {
+ ret = adv7180_read(state, ADV7180_REG_EXTENDED_OUTPUT_CONTROL);
+ if (ret < 0)
+ goto out_unlock;
+ ret |= ADV7180_EXTENDED_OUTPUT_CONTROL_BT656_4;
+ ret = adv7180_write(state, ADV7180_REG_EXTENDED_OUTPUT_CONTROL,
+ ret);
+ if (ret < 0)
+ goto out_unlock;
+ }
+
ret = adv7180_program_std(state);
if (ret)
goto out_unlock;
@@ -1332,6 +1344,10 @@ static int adv7180_of_parse(struct adv7180_state *state)
return PTR_ERR(state->pwdn_gpio);
}
+ /* select ITU-R BT.656-4 compatible? */
+ if (of_property_read_bool(client->dev.of_node, "bt656-4"))
+ state->bt656_4 = true;
+
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 07/11] media: adv7180: change mbus format to UYVY
2016-07-06 22:59 [PATCH 00/11] adv7180 subdev fixes Steve Longerbeam
` (5 preceding siblings ...)
2016-07-06 22:59 ` [PATCH 06/11] media: adv7180: add bt.656-4 OF property Steve Longerbeam
@ 2016-07-06 23:00 ` Steve Longerbeam
2016-07-07 15:18 ` Lars-Peter Clausen
2016-07-07 15:25 ` Tim Harvey
2016-07-06 23:00 ` [PATCH 08/11] adv7180: send V4L2_EVENT_SOURCE_CHANGE on std change Steve Longerbeam
` (5 subsequent siblings)
12 siblings, 2 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-06 23:00 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Change the media bus format from YUYV8_2X8 to UYVY8_2X8. Colors
now look correct when capturing with the i.mx6 backend. The other
option is to set the SWPC bit in register 0x27 to swap the Cr and Cb
output samples.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
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 fff887c..427695d 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -654,7 +654,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;
}
@@ -664,7 +664,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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 08/11] adv7180: send V4L2_EVENT_SOURCE_CHANGE on std change
2016-07-06 22:59 [PATCH 00/11] adv7180 subdev fixes Steve Longerbeam
` (6 preceding siblings ...)
2016-07-06 23:00 ` [PATCH 07/11] media: adv7180: change mbus format to UYVY Steve Longerbeam
@ 2016-07-06 23:00 ` Steve Longerbeam
2016-07-07 15:27 ` Tim Harvey
2016-07-06 23:00 ` [PATCH 09/11] v4l: Add signal lock status to source change events Steve Longerbeam
` (4 subsequent siblings)
12 siblings, 1 reply; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-06 23:00 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Subscribe to the V4L2_EVENT_SOURCE_CHANGE event and send
V4L2_EVENT_SRC_CH_RESOLUTION in the interrupt handler on a
detected standard change.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/media/i2c/adv7180.c | 33 ++++++++++++++++++++++++++++++---
1 file changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 427695d..f76a0e7 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -28,6 +28,7 @@
#include <linux/of.h>
#include <linux/gpio/consumer.h>
#include <media/v4l2-ioctl.h>
+#include <media/v4l2-event.h>
#include <linux/videodev2.h>
#include <media/v4l2-device.h>
#include <media/v4l2-ctrls.h>
@@ -824,6 +825,20 @@ static int adv7180_g_tvnorms(struct v4l2_subdev *sd, v4l2_std_id *norm)
return 0;
}
+static int adv7180_subscribe_event(struct v4l2_subdev *sd,
+ struct v4l2_fh *fh,
+ struct v4l2_event_subscription *sub)
+{
+ switch (sub->type) {
+ case V4L2_EVENT_SOURCE_CHANGE:
+ return v4l2_src_change_event_subdev_subscribe(sd, fh, sub);
+ case V4L2_EVENT_CTRL:
+ return v4l2_ctrl_subdev_subscribe_event(sd, fh, sub);
+ default:
+ return -EINVAL;
+ }
+}
+
static const struct v4l2_subdev_video_ops adv7180_video_ops = {
.s_std = adv7180_s_std,
.g_std = adv7180_g_std,
@@ -838,6 +853,8 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops = {
static const struct v4l2_subdev_core_ops adv7180_core_ops = {
.s_power = adv7180_s_power,
+ .subscribe_event = adv7180_subscribe_event,
+ .unsubscribe_event = v4l2_event_subdev_unsubscribe,
};
static const struct v4l2_subdev_pad_ops adv7180_pad_ops = {
@@ -862,8 +879,18 @@ static irqreturn_t adv7180_irq(int irq, void *devid)
/* clear */
adv7180_write(state, ADV7180_REG_ICR3, isr3);
- if (isr3 & ADV7180_IRQ3_AD_CHANGE && state->autodetect)
- __adv7180_status(state, NULL, &state->curr_norm);
+ if (isr3 & ADV7180_IRQ3_AD_CHANGE) {
+ static const struct v4l2_event src_ch = {
+ .type = V4L2_EVENT_SOURCE_CHANGE,
+ .u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
+ };
+
+ v4l2_subdev_notify_event(&state->sd, &src_ch);
+
+ if (state->autodetect)
+ __adv7180_status(state, NULL, &state->curr_norm);
+ }
+
mutex_unlock(&state->mutex);
return IRQ_HANDLED;
@@ -1403,7 +1430,7 @@ static int adv7180_probe(struct i2c_client *client,
state->input = 0;
sd = &state->sd;
v4l2_i2c_subdev_init(sd, client, &adv7180_ops);
- sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+ sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
ret = adv7180_init_controls(state);
if (ret)
--
1.9.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 09/11] v4l: Add signal lock status to source change events
2016-07-06 22:59 [PATCH 00/11] adv7180 subdev fixes Steve Longerbeam
` (7 preceding siblings ...)
2016-07-06 23:00 ` [PATCH 08/11] adv7180: send V4L2_EVENT_SOURCE_CHANGE on std change Steve Longerbeam
@ 2016-07-06 23:00 ` Steve Longerbeam
2016-07-06 23:00 ` [PATCH 10/11] media: adv7180: enable lock/unlock interrupts Steve Longerbeam
` (3 subsequent siblings)
12 siblings, 0 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-06 23:00 UTC (permalink / raw)
To: linux-media; +Cc: 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>
---
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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 10/11] media: adv7180: enable lock/unlock interrupts
2016-07-06 22:59 [PATCH 00/11] adv7180 subdev fixes Steve Longerbeam
` (8 preceding siblings ...)
2016-07-06 23:00 ` [PATCH 09/11] v4l: Add signal lock status to source change events Steve Longerbeam
@ 2016-07-06 23:00 ` Steve Longerbeam
2016-07-06 23:00 ` [PATCH 11/11] media: adv7180: fix field type Steve Longerbeam
` (2 subsequent siblings)
12 siblings, 0 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-06 23:00 UTC (permalink / raw)
To: linux-media; +Cc: 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. Keep track of current
input lock status with state->curr_status.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/media/i2c/adv7180.c | 35 +++++++++++++++++++++++++++--------
1 file changed, 27 insertions(+), 8 deletions(-)
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index f76a0e7..4c2623f 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -216,6 +216,7 @@ struct adv7180_state {
int irq;
struct gpio_desc *pwdn_gpio;
v4l2_std_id curr_norm;
+ u32 curr_status; /* lock status */
bool autodetect;
bool bt656_4; /* use bt.656-4 standard for NTSC */
bool powered;
@@ -422,7 +423,12 @@ static int adv7180_g_input_status(struct v4l2_subdev *sd, u32 *status)
if (ret)
return ret;
- ret = __adv7180_status(state, status, NULL);
+ /* when we are interrupt driven we know the input lock status */
+ if (!state->autodetect || state->irq > 0)
+ *status = state->curr_status;
+ else
+ ret = __adv7180_status(state, status, NULL);
+
mutex_unlock(&state->mutex);
return ret;
}
@@ -437,7 +443,7 @@ static int adv7180_program_std(struct adv7180_state *state)
if (ret < 0)
return ret;
- __adv7180_status(state, NULL, &state->curr_norm);
+ __adv7180_status(state, &state->curr_status, &state->curr_norm);
} else {
ret = v4l2_std_to_adv7180(state->curr_norm);
if (ret < 0)
@@ -872,23 +878,34 @@ 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);
if (state->autodetect)
- __adv7180_status(state, NULL, &state->curr_norm);
+ __adv7180_status(state, &state->curr_status,
+ &state->curr_norm);
}
mutex_unlock(&state->mutex);
@@ -1335,7 +1352,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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 11/11] media: adv7180: fix field type
2016-07-06 22:59 [PATCH 00/11] adv7180 subdev fixes Steve Longerbeam
` (9 preceding siblings ...)
2016-07-06 23:00 ` [PATCH 10/11] media: adv7180: enable lock/unlock interrupts Steve Longerbeam
@ 2016-07-06 23:00 ` Steve Longerbeam
2016-07-07 14:37 ` [PATCH 00/11] adv7180 subdev fixes Tim Harvey
2016-07-20 0:03 ` [PATCH v2 00/10] adv7180 subdev fixes, v2 Steve Longerbeam
12 siblings, 0 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-06 23:00 UTC (permalink / raw)
To: linux-media; +Cc: 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>
---
drivers/media/i2c/adv7180.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 4c2623f..c12fca7 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -743,10 +743,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;
}
@@ -1416,9 +1423,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;
+ 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;
+
ret = adv7180_of_parse(state);
if (ret)
return ret;
--
1.9.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* Re: [PATCH 00/11] adv7180 subdev fixes
2016-07-06 22:59 [PATCH 00/11] adv7180 subdev fixes Steve Longerbeam
` (10 preceding siblings ...)
2016-07-06 23:00 ` [PATCH 11/11] media: adv7180: fix field type Steve Longerbeam
@ 2016-07-07 14:37 ` Tim Harvey
2016-07-20 0:03 ` [PATCH v2 00/10] adv7180 subdev fixes, v2 Steve Longerbeam
12 siblings, 0 replies; 55+ messages in thread
From: Tim Harvey @ 2016-07-07 14:37 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media, Lars-Peter Clausen
On Wed, Jul 6, 2016 at 3:59 PM, Steve Longerbeam <slongerbeam@gmail.com> wrote:
> Steve Longerbeam (11):
> media: adv7180: Fix broken interrupt register access
> Revert "[media] adv7180: fix broken standards handling"
> media: adv7180: add power pin control
> media: adv7180: implement g_parm
> media: adv7180: init chip with AD recommended register settings
> media: adv7180: add bt.656-4 OF property
> media: adv7180: change mbus format to UYVY
> adv7180: send V4L2_EVENT_SOURCE_CHANGE on std change
> 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 +-
> drivers/media/i2c/Kconfig | 2 +-
> drivers/media/i2c/adv7180.c | 409 +++++++++++++++------
> include/uapi/linux/videodev2.h | 1 +
> 4 files changed, 308 insertions(+), 116 deletions(-)
>
Steve,
I've tested this series and will provide my ack/tested-by in the
individual patches.
These are well grouped but per the MAINTAINERS file they should all be
sent to 'Lars-Peter Clausen <lars@metafoo.de>' with a cc of
'linux-media@vger.kernel.org' or they are liable to be missed by the
proper maintainer:
ANALOG DEVICES INC ADV7180 DRIVER
M: Lars-Peter Clausen <lars@metafoo.de>
L: linux-media@vger.kernel.org
W: http://ez.analog.com/community/linux-device-drivers
S: Supported
F: drivers/media/i2c/adv7180.c
I've cc'd Lars-Peter Clausen so that he see's the patches. Also, the
patch that reverts a previous patch should cc the people that wrote,
tested, ack'd and signed off on that patch. I will add them when I add
my review/ack to that patch.
Regards,
Tim
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 01/11] media: adv7180: Fix broken interrupt register access
2016-07-06 22:59 ` [PATCH 01/11] media: adv7180: Fix broken interrupt register access Steve Longerbeam
@ 2016-07-07 14:44 ` Tim Harvey
2016-07-07 15:37 ` Lars-Peter Clausen
1 sibling, 0 replies; 55+ messages in thread
From: Tim Harvey @ 2016-07-07 14:44 UTC (permalink / raw)
To: Steve Longerbeam
Cc: linux-media, Lars-Peter Clausen, Federico Vaga, Hans Verkuil,
Mauro Carvalho Chehab, Niklas Söderlund
On Wed, Jul 6, 2016 at 3:59 PM, Steve Longerbeam <slongerbeam@gmail.com> wrote:
> Access to the interrupt page registers has been broken since
> at least 3999e5d01da74f1a22afbb0b61b3992fea301478. That commit
> forgot to add the inerrupt page number to the register defines.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Tested on an IMX6 Gateworks Ventana with IMX6 capture drivers from Steve [1]
Tested-by: Tim Harvey <tharvey@gateworks.com>
Acked-by: Tim Harvey <tharvey@gateworks.com>
Added to Cc list those who signed-off and acked
3999e5d01da74f1a22afbb0b61b3992fea301478
[1] - http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/102914
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 02/11] Revert "[media] adv7180: fix broken standards handling"
2016-07-06 22:59 ` [PATCH 02/11] Revert "[media] adv7180: fix broken standards handling" Steve Longerbeam
@ 2016-07-07 14:48 ` Tim Harvey
2016-07-07 15:45 ` Lars-Peter Clausen
1 sibling, 0 replies; 55+ messages in thread
From: Tim Harvey @ 2016-07-07 14:48 UTC (permalink / raw)
To: Steve Longerbeam
Cc: linux-media, Hans Verkuil, Federico Vaga, Niklas Söderlund,
Lars-Peter Clausen, Mauro Carvalho Chehab
On Wed, Jul 6, 2016 at 3:59 PM, Steve Longerbeam <slongerbeam@gmail.com> wrote:
> Autodetect was likely broken only because access to the
> interrupt registers were broken, so there were no standard
> change interrupts. After fixing that, and reverting this,
> autodetect seems to work just fine on an i.mx6q SabreAuto.
>
> This reverts commit 937feeed3f0ae8a0389d5732f6db63dd912acd99.
> ---
> drivers/media/i2c/adv7180.c | 118 ++++++++++++++------------------------------
> 1 file changed, 38 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index 95cbc85..967303a 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -26,9 +26,8 @@
> #include <linux/i2c.h>
> #include <linux/slab.h>
> #include <linux/of.h>
> -#include <linux/videodev2.h>
> #include <media/v4l2-ioctl.h>
> -#include <media/v4l2-event.h>
> +#include <linux/videodev2.h>
> #include <media/v4l2-device.h>
> #include <media/v4l2-ctrls.h>
> #include <linux/mutex.h>
> @@ -193,8 +192,8 @@ struct adv7180_state {
> struct mutex mutex; /* mutual excl. when accessing chip */
> int irq;
> v4l2_std_id curr_norm;
> + bool autodetect;
> bool powered;
> - bool streaming;
> u8 input;
>
> struct i2c_client *client;
> @@ -339,26 +338,12 @@ static int adv7180_querystd(struct v4l2_subdev *sd, v4l2_std_id *std)
> if (err)
> return err;
>
> - if (state->streaming) {
> - err = -EBUSY;
> - goto unlock;
> - }
> -
> - err = adv7180_set_video_standard(state,
> - ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM);
> - if (err)
> - goto unlock;
> -
> - msleep(100);
> - __adv7180_status(state, NULL, std);
> -
> - err = v4l2_std_to_adv7180(state->curr_norm);
> - if (err < 0)
> - goto unlock;
> -
> - err = adv7180_set_video_standard(state, err);
> + /* when we are interrupt driven we know the state */
> + if (!state->autodetect || state->irq > 0)
> + *std = state->curr_norm;
> + else
> + err = __adv7180_status(state, NULL, std);
>
> -unlock:
> mutex_unlock(&state->mutex);
> return err;
> }
> @@ -402,13 +387,23 @@ static int adv7180_program_std(struct adv7180_state *state)
> {
> int ret;
>
> - ret = v4l2_std_to_adv7180(state->curr_norm);
> - if (ret < 0)
> - return ret;
> + if (state->autodetect) {
> + ret = adv7180_set_video_standard(state,
> + ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM);
> + if (ret < 0)
> + return ret;
> +
> + __adv7180_status(state, NULL, &state->curr_norm);
> + } else {
> + ret = v4l2_std_to_adv7180(state->curr_norm);
> + if (ret < 0)
> + return ret;
> +
> + ret = adv7180_set_video_standard(state, ret);
> + if (ret < 0)
> + return ret;
> + }
>
> - ret = adv7180_set_video_standard(state, ret);
> - if (ret < 0)
> - return ret;
> return 0;
> }
>
> @@ -420,12 +415,18 @@ static int adv7180_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
> if (ret)
> return ret;
>
> - /* Make sure we can support this std */
> - ret = v4l2_std_to_adv7180(std);
> - if (ret < 0)
> - goto out;
> + /* all standards -> autodetect */
> + if (std == V4L2_STD_ALL) {
> + state->autodetect = true;
> + } else {
> + /* Make sure we can support this std */
> + ret = v4l2_std_to_adv7180(std);
> + if (ret < 0)
> + goto out;
>
> - state->curr_norm = std;
> + state->curr_norm = std;
> + state->autodetect = false;
> + }
>
> ret = adv7180_program_std(state);
> out:
> @@ -746,40 +747,6 @@ static int adv7180_g_tvnorms(struct v4l2_subdev *sd, v4l2_std_id *norm)
> return 0;
> }
>
> -static int adv7180_s_stream(struct v4l2_subdev *sd, int enable)
> -{
> - struct adv7180_state *state = to_state(sd);
> - int ret;
> -
> - /* It's always safe to stop streaming, no need to take the lock */
> - if (!enable) {
> - state->streaming = enable;
> - return 0;
> - }
> -
> - /* Must wait until querystd released the lock */
> - ret = mutex_lock_interruptible(&state->mutex);
> - if (ret)
> - return ret;
> - state->streaming = enable;
> - mutex_unlock(&state->mutex);
> - return 0;
> -}
> -
> -static int adv7180_subscribe_event(struct v4l2_subdev *sd,
> - struct v4l2_fh *fh,
> - struct v4l2_event_subscription *sub)
> -{
> - switch (sub->type) {
> - case V4L2_EVENT_SOURCE_CHANGE:
> - return v4l2_src_change_event_subdev_subscribe(sd, fh, sub);
> - case V4L2_EVENT_CTRL:
> - return v4l2_ctrl_subdev_subscribe_event(sd, fh, sub);
> - default:
> - return -EINVAL;
> - }
> -}
> -
> static const struct v4l2_subdev_video_ops adv7180_video_ops = {
> .s_std = adv7180_s_std,
> .g_std = adv7180_g_std,
> @@ -789,13 +756,10 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops = {
> .g_mbus_config = adv7180_g_mbus_config,
> .cropcap = adv7180_cropcap,
> .g_tvnorms = adv7180_g_tvnorms,
> - .s_stream = adv7180_s_stream,
> };
>
> static const struct v4l2_subdev_core_ops adv7180_core_ops = {
> .s_power = adv7180_s_power,
> - .subscribe_event = adv7180_subscribe_event,
> - .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> };
>
> static const struct v4l2_subdev_pad_ops adv7180_pad_ops = {
> @@ -820,14 +784,8 @@ static irqreturn_t adv7180_irq(int irq, void *devid)
> /* clear */
> adv7180_write(state, ADV7180_REG_ICR3, isr3);
>
> - if (isr3 & ADV7180_IRQ3_AD_CHANGE) {
> - static const struct v4l2_event src_ch = {
> - .type = V4L2_EVENT_SOURCE_CHANGE,
> - .u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
> - };
> -
> - v4l2_subdev_notify_event(&state->sd, &src_ch);
> - }
> + if (isr3 & ADV7180_IRQ3_AD_CHANGE && state->autodetect)
> + __adv7180_status(state, NULL, &state->curr_norm);
> mutex_unlock(&state->mutex);
>
> return IRQ_HANDLED;
> @@ -1272,7 +1230,7 @@ static int adv7180_probe(struct i2c_client *client,
>
> state->irq = client->irq;
> mutex_init(&state->mutex);
> - state->curr_norm = V4L2_STD_NTSC;
> + state->autodetect = true;
> if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
> state->powered = true;
> else
> @@ -1280,7 +1238,7 @@ static int adv7180_probe(struct i2c_client *client,
> state->input = 0;
> sd = &state->sd;
> v4l2_i2c_subdev_init(sd, client, &adv7180_ops);
> - sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> + sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
>
> ret = adv7180_init_controls(state);
> if (ret)
> --
Tested on an IMX6 Gateworks Ventana with IMX6 capture drivers from
Steve [1]. Verified that interrupts work via signal lock/unlock.
Tested-by: Tim Harvey <tharvey@gateworks.com>
Acked-by: Tim Harvey <tharvey@gateworks.com>
Added to Cc list those who signed-off and acked
937feeed3f0ae8a0389d5732f6db63dd912acd99:
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Federico Vaga <federico.vaga@gmail.com>
Cc: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Regards,
Tim
[1] - http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/102914
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/11] media: adv7180: add bt.656-4 OF property
2016-07-06 22:59 ` [PATCH 06/11] media: adv7180: add bt.656-4 OF property Steve Longerbeam
@ 2016-07-07 14:52 ` Lars-Peter Clausen
2016-07-09 18:59 ` Steve Longerbeam
0 siblings, 1 reply; 55+ messages in thread
From: Lars-Peter Clausen @ 2016-07-07 14:52 UTC (permalink / raw)
To: Steve Longerbeam, linux-media; +Cc: Steve Longerbeam
On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
> Add a device tree boolean property "bt656-4" to allow setting
> the ITU-R BT.656-4 compatible bit.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
Hi,
Thanks for the patch.
> ---
> drivers/media/i2c/adv7180.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index 92e2f37..fff887c 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -58,7 +58,7 @@
>
> #define ADV7180_REG_OUTPUT_CONTROL 0x0003
> #define ADV7180_REG_EXTENDED_OUTPUT_CONTROL 0x0004
> -#define ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS 0xC5
> +#define ADV7180_EXTENDED_OUTPUT_CONTROL_BT656_4 0x80
>
> #define ADV7180_REG_AUTODETECT_ENABLE 0x0007
> #define ADV7180_AUTODETECT_DEFAULT 0x7f
> @@ -216,6 +216,7 @@ struct adv7180_state {
> struct gpio_desc *pwdn_gpio;
> v4l2_std_id curr_norm;
> bool autodetect;
> + bool bt656_4; /* use bt.656-4 standard for NTSC */
> bool powered;
> u8 input;
>
> @@ -1281,6 +1282,17 @@ static int init_device(struct adv7180_state *state)
> if (ret)
> goto out_unlock;
>
> + if (state->bt656_4) {
> + ret = adv7180_read(state, ADV7180_REG_EXTENDED_OUTPUT_CONTROL);
> + if (ret < 0)
> + goto out_unlock;
> + ret |= ADV7180_EXTENDED_OUTPUT_CONTROL_BT656_4;
> + ret = adv7180_write(state, ADV7180_REG_EXTENDED_OUTPUT_CONTROL,
> + ret);
> + if (ret < 0)
> + goto out_unlock;
> + }
> +
> ret = adv7180_program_std(state);
> if (ret)
> goto out_unlock;
> @@ -1332,6 +1344,10 @@ static int adv7180_of_parse(struct adv7180_state *state)
> return PTR_ERR(state->pwdn_gpio);
> }
>
> + /* select ITU-R BT.656-4 compatible? */
> + if (of_property_read_bool(client->dev.of_node, "bt656-4"))
> + state->bt656_4 = true;
This property needs to be documented. In my opinion it should also be a
property of the endpoint sub-node rather than the toplevel device node since
this is a configuration of the endpoint format.
> +
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/11] media: adv7180: add power pin control
2016-07-06 22:59 ` [PATCH 03/11] media: adv7180: add power pin control Steve Longerbeam
@ 2016-07-07 15:04 ` Tim Harvey
2016-07-07 15:35 ` Lars-Peter Clausen
1 sibling, 0 replies; 55+ messages in thread
From: Tim Harvey @ 2016-07-07 15:04 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media, Steve Longerbeam, Lars-Peter Clausen
On Wed, Jul 6, 2016 at 3:59 PM, Steve Longerbeam <slongerbeam@gmail.com> wrote:
> Some targets control the ADV7180 power pin via a gpio, so add
> support for "pwdn-gpio" pin control.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> drivers/media/i2c/Kconfig | 2 +-
> drivers/media/i2c/adv7180.c | 37 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 993dc50..80d39f6 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 967303a..38e5161 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 <media/v4l2-ioctl.h>
> #include <linux/videodev2.h>
> #include <media/v4l2-device.h>
> @@ -191,6 +192,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 autodetect;
> bool powered;
> @@ -443,6 +445,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;
> @@ -1143,6 +1158,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);
>
> @@ -1190,6 +1207,20 @@ out_unlock:
> return ret;
> }
>
> +static int adv7180_of_parse(struct adv7180_state *state)
> +{
> + struct i2c_client *client = state->client;
> +
> + state->pwdn_gpio = devm_gpiod_get_optional(&client->dev, "pwdn",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(state->pwdn_gpio)) {
> + v4l_err(client, "request for power pin failed\n");
> + return PTR_ERR(state->pwdn_gpio);
> + }
> +
> + return 0;
> +}
> +
> static int adv7180_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> @@ -1212,6 +1243,10 @@ static int adv7180_probe(struct i2c_client *client,
> state->field = V4L2_FIELD_INTERLACED;
> state->chip_info = (struct adv7180_chip_info *)id->driver_data;
>
> + ret = adv7180_of_parse(state);
> + if (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);
> @@ -1303,6 +1338,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;
> --
Steve,
For completeness, you also need to provide a patch to
Documentation/devicetree/bindings/media/i2c/adv7180.txt adding the
Optional property.
Tested on an IMX6 Gateworks Ventana with IMX6 capture drivers [1].
Verified that adv7180_set_power_pin() gets called properly.
Tested-by: Tim Harvey <tharvey@gateworks.com>
Acked-by: Tim Harvey <tharvey@gateworks.com>
Added to Cc:
Cc: Lars-Peter Clausen <lars@metafoo.de>
Regards,
Tim
[1] - http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/102914
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 04/11] media: adv7180: implement g_parm
2016-07-06 22:59 ` [PATCH 04/11] media: adv7180: implement g_parm Steve Longerbeam
@ 2016-07-07 15:04 ` Tim Harvey
0 siblings, 0 replies; 55+ messages in thread
From: Tim Harvey @ 2016-07-07 15:04 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media, Steve Longerbeam, Lars-Peter Clausen
On Wed, Jul 6, 2016 at 3:59 PM, Steve Longerbeam <slongerbeam@gmail.com> wrote:
> Implement g_parm to return the current standard's frame period.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> 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 38e5161..42816d4 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -741,6 +741,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);
> @@ -765,6 +786,7 @@ static int adv7180_g_tvnorms(struct v4l2_subdev *sd, v4l2_std_id *norm)
> 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,
> --
Steve,
Tested on an IMX6 Gateworks Ventana with IMX6 capture drivers [1].
Tested-by: Tim Harvey <tharvey@gateworks.com>
Acked-by: Tim Harvey <tharvey@gateworks.com>
Added to Cc:
Cc: Lars-Peter Clausen <lars@metafoo.de>
Regards,
Tim
[1] - http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/102914
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 07/11] media: adv7180: change mbus format to UYVY
2016-07-06 23:00 ` [PATCH 07/11] media: adv7180: change mbus format to UYVY Steve Longerbeam
@ 2016-07-07 15:18 ` Lars-Peter Clausen
2016-07-08 10:52 ` Niklas Söderlund
2016-07-07 15:25 ` Tim Harvey
1 sibling, 1 reply; 55+ messages in thread
From: Lars-Peter Clausen @ 2016-07-07 15:18 UTC (permalink / raw)
To: Steve Longerbeam, linux-media; +Cc: Steve Longerbeam, Niklas Söderlund
On 07/07/2016 01:00 AM, Steve Longerbeam wrote:
> Change the media bus format from YUYV8_2X8 to UYVY8_2X8. Colors
> now look correct when capturing with the i.mx6 backend. The other
> option is to set the SWPC bit in register 0x27 to swap the Cr and Cb
> output samples.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
The patch is certainly correct from the technical point of view. But we need
to be careful not to break any existing platforms which rely on this
setting. So the alternative solution of changing the default output order is
not an option.
Looking at things it seems like the Renesas vin driver, which is used in
combination with the adv7180 on some boards, uses the return value from
enum_mbus_code to setup the video pipeline. Adding Niklas to Cc, maybe he
can help to test this.
But otherwise
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> 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 fff887c..427695d 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -654,7 +654,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;
> }
> @@ -664,7 +664,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;
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 05/11] media: adv7180: init chip with AD recommended register settings
2016-07-06 22:59 ` [PATCH 05/11] media: adv7180: init chip with AD recommended register settings Steve Longerbeam
@ 2016-07-07 15:23 ` Tim Harvey
2016-07-07 15:29 ` Lars-Peter Clausen
1 sibling, 0 replies; 55+ messages in thread
From: Tim Harvey @ 2016-07-07 15:23 UTC (permalink / raw)
To: Steve Longerbeam
Cc: linux-media, Steve Longerbeam, Sergei Shtylyov, Simon Horman
On Wed, Jul 6, 2016 at 3:59 PM, Steve Longerbeam <slongerbeam@gmail.com> wrote:
> Define and load register tables that conform to Analog Device's
> recommended register settings. It loads the default single-ended
> CVBS on Ain1 configuration for both ADV7180 and ADV7182 chips.
>
> New register addresses have been defined for the tables. Those new
> defines are also used in existing locations where hard-coded addresses
> were used.
>
> Note this patch also enables NEWAVMODE, which is also recommended by
> Analog Devices. This will likely break any current backends using this
> subdev that are expecting different or manually configured AV codes.
>
> Note also that bt.656-4 support has been removed in this patch, but it
> will be brought back in a subsequent patch.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> drivers/media/i2c/adv7180.c | 168 ++++++++++++++++++++++++++++++++++----------
> 1 file changed, 130 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index 42816d4..92e2f37 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
>
> @@ -209,6 +230,11 @@ struct adv7180_state {
> struct adv7180_state, \
> ctrl_hdl)->sd)
>
> +struct adv7180_reg_tbl_t {
> + unsigned int reg;
> + unsigned int val;
> +};
> +
> static int adv7180_select_page(struct adv7180_state *state, unsigned int page)
> {
> if (state->register_page != page) {
> @@ -235,6 +261,20 @@ static int adv7180_read(struct adv7180_state *state, unsigned int reg)
> return i2c_smbus_read_byte_data(state->client, reg & 0xff);
> }
>
> +static int adv7180_load_reg_tbl(struct adv7180_state *state,
> + const struct adv7180_reg_tbl_t *tbl, int n)
> +{
> + int ret, i;
> +
> + for (i = 0; i < n; i++) {
> + ret = adv7180_write(state, tbl[i].reg, tbl[i].val);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int adv7180_csi_write(struct adv7180_state *state, unsigned int reg,
> unsigned int value)
> {
> @@ -828,19 +868,36 @@ static irqreturn_t adv7180_irq(int irq, void *devid)
> return IRQ_HANDLED;
> }
>
> +/*
> + * This register table conforms to Analog Device's Register Settings
> + * Recommendation for the ADV7180. It configures single-ended CVBS
> + * input on Ain1, and enables NEWAVMODE.
> + */
> +static const struct adv7180_reg_tbl_t adv7180_single_ended_cvbs[] = {
> + /* Set analog mux for CVBS on Ain1 */
> + { ADV7180_REG_INPUT_CONTROL, 0x00 },
> + /* ADI Required Write: Reset Clamp Circuitry */
> + { ADV7180_REG_ANALOG_CLAMP_CTL, 0x30 },
> + /* Enable SFL Output */
> + { ADV7180_REG_EXTENDED_OUTPUT_CONTROL, 0x57 },
> + /* Select SH1 Chroma Shaping Filter */
> + { ADV7180_REG_SHAP_FILTER_CTL_1, 0x41 },
> + /* Enable NEWAVMODE */
> + { ADV7180_REG_VSYNC_FIELD_CTL_1, 0x02 },
> + /* ADI Required Write: optimize windowing function Step 1,2,3 */
> + { ADV7180_REG_MANUAL_WIN_CTL_1, 0xA2 },
> + { ADV7180_REG_MANUAL_WIN_CTL_2, 0x6A },
> + { ADV7180_REG_MANUAL_WIN_CTL_3, 0xA0 },
> + /* ADI Required Write: Enable ADC step 1,2,3 */
> + { 0x8055, 0x81 }, /* undocumented register 0x55 */
> + /* Recommended AFE I BIAS Setting for CVBS mode */
> + { ADV7180_REG_CVBS_TRIM, 0x0D },
> +};
> +
> static int adv7180_init(struct adv7180_state *state)
> {
> - int ret;
> -
> - /* ITU-R BT.656-4 compatible */
> - ret = adv7180_write(state, ADV7180_REG_EXTENDED_OUTPUT_CONTROL,
> - ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS);
> - 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);
> + return adv7180_load_reg_tbl(state, adv7180_single_ended_cvbs,
> + ARRAY_SIZE(adv7180_single_ended_cvbs));
> }
>
> static int adv7180_set_std(struct adv7180_state *state, unsigned int std)
> @@ -862,8 +919,48 @@ static int adv7180_select_input(struct adv7180_state *state, unsigned int input)
> return adv7180_write(state, ADV7180_REG_INPUT_CONTROL, ret);
> }
>
> +/*
> + * This register table conforms to Analog Device's Register Settings
> + * Recommendation revision C for the ADV7182. It configures single-ended
> + * CVBS inputs on Ain1, and enables NEWAVMODE.
> + */
> +static const struct adv7180_reg_tbl_t adv7182_single_ended_cvbs[] = {
> + /* Exit Power Down Mode */
> + { ADV7180_REG_PWR_MAN, 0x00 },
> + /* Enable ADV7182 for 28.63636 MHz Crystal Clock Input */
> + { ADV7180_REG_STATUS3, 0x00 },
> + /* Set optimized IBIAS for single-ended CVBS input */
> + { ADV7180_REG_CVBS_TRIM, 0xCD },
> + /* Switch to single-ended CVBS on AIN1 */
> + { ADV7180_REG_INPUT_CONTROL, 0x00 },
> + /* ADI Required Write: Reset Current Clamp Circuitry steps 1,2,3,4 */
> + { ADV7180_REG_RST_CLAMP, 0x00 },
> + { ADV7180_REG_RST_CLAMP, 0xFF },
> + /* Select SH1 Chroma Shaping Filter */
> + { ADV7180_REG_SHAP_FILTER_CTL_1, 0x41 },
> + /* Enable Pixel & Sync output drivers */
> + { ADV7180_REG_OUTPUT_CONTROL, 0x0C },
> + /* Power-up INTRQ, HS and VS/FIELD/SFL pad */
> + { ADV7180_REG_EXTENDED_OUTPUT_CONTROL, 0x07 },
> + /* Enable LLC Output Driver */
> + { ADV7180_REG_CTRL_2, 0x40 },
> + /* Optimize ACE Performance */
> + { ADV7180_REG_ACE_CTRL5, 0x00 },
> + /* Enable ACE Feature */
> + { ADV7180_REG_ACE_CTRL1, 0x80 },
> + /* Enable NEWAVMODE */
> + { ADV7180_REG_VSYNC_FIELD_CTL_1, 0x02 },
> +};
> +
> static int adv7182_init(struct adv7180_state *state)
> {
> + int ret;
> +
> + ret = adv7180_load_reg_tbl(state, adv7182_single_ended_cvbs,
> + ARRAY_SIZE(adv7182_single_ended_cvbs));
> + if (ret)
> + return ret;
> +
> if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2)
> adv7180_write(state, ADV7180_REG_CSI_SLAVE_ADDR,
> ADV7180_DEFAULT_CSI_I2C_ADDR << 1);
> @@ -881,20 +978,15 @@ 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);
> - else
> - adv7180_write(state, 0x0004, 0x07);
> - adv7180_write(state, 0x0003, 0x0c);
> - adv7180_write(state, 0x001d, 0x40);
> + adv7180_write(state, ADV7180_REG_EXTENDED_OUTPUT_CONTROL,
> + 0x17);
> }
>
> - adv7180_write(state, 0x0013, 0x00);
> -
> return 0;
> }
>
> @@ -967,8 +1059,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);
>
> @@ -976,10 +1068,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;
> }
>
> @@ -989,21 +1081,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;
> --
Steve,
Tested on an IMX6 Gateworks Ventana with IMX6 capture drivers [1].
Tested-by: Tim Harvey <tharvey@gateworks.com>
Acked-by: Tim Harvey <tharvey@gateworks.com>
Added to Cc:
Cc: Lars-Peter Clausen <lars@metafoo.de>
Also adding Cc's to the people who are using the adv7180 on other
boards (renesas r8a779* boards) so we can get some feedback and/or
Tested-by from them:
Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Cc: Simon Horman <horms+renesas@verge.net.au>
I'm wondering if those boards need the bt656-4 which was previously
being enabled per your comment and is now moved to a dt-prop per a
subsequent patch.
Regards,
Tim
[1] - http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/102914
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 07/11] media: adv7180: change mbus format to UYVY
2016-07-06 23:00 ` [PATCH 07/11] media: adv7180: change mbus format to UYVY Steve Longerbeam
2016-07-07 15:18 ` Lars-Peter Clausen
@ 2016-07-07 15:25 ` Tim Harvey
1 sibling, 0 replies; 55+ messages in thread
From: Tim Harvey @ 2016-07-07 15:25 UTC (permalink / raw)
To: Steve Longerbeam
Cc: linux-media, Steve Longerbeam, Lars-Peter Clausen,
Sergei Shtylyov, Simon Horman
On Wed, Jul 6, 2016 at 4:00 PM, Steve Longerbeam <slongerbeam@gmail.com> wrote:
> Change the media bus format from YUYV8_2X8 to UYVY8_2X8. Colors
> now look correct when capturing with the i.mx6 backend. The other
> option is to set the SWPC bit in register 0x27 to swap the Cr and Cb
> output samples.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> 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 fff887c..427695d 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -654,7 +654,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;
> }
> @@ -664,7 +664,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;
> --
Steve,
Tested on an IMX6 Gateworks Ventana with IMX6 capture drivers [1].
Tested-by: Tim Harvey <tharvey@gateworks.com>
Acked-by: Tim Harvey <tharvey@gateworks.com>
Added to Cc:
Cc: Lars-Peter Clausen <lars@metafoo.de>
Also adding Cc's to the people who are using the adv7180 on other
boards (renesas r8a779* boards) so we can get some feedback and/or
Tested-by from them:
Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Cc: Simon Horman <horms+renesas@verge.net.au>
Regards,
Tim
[1] - http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/102914
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 08/11] adv7180: send V4L2_EVENT_SOURCE_CHANGE on std change
2016-07-06 23:00 ` [PATCH 08/11] adv7180: send V4L2_EVENT_SOURCE_CHANGE on std change Steve Longerbeam
@ 2016-07-07 15:27 ` Tim Harvey
0 siblings, 0 replies; 55+ messages in thread
From: Tim Harvey @ 2016-07-07 15:27 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media, Steve Longerbeam, Lars-Peter Clausen
On Wed, Jul 6, 2016 at 4:00 PM, Steve Longerbeam <slongerbeam@gmail.com> wrote:
> Subscribe to the V4L2_EVENT_SOURCE_CHANGE event and send
> V4L2_EVENT_SRC_CH_RESOLUTION in the interrupt handler on a
> detected standard change.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> drivers/media/i2c/adv7180.c | 33 ++++++++++++++++++++++++++++++---
> 1 file changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index 427695d..f76a0e7 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -28,6 +28,7 @@
> #include <linux/of.h>
> #include <linux/gpio/consumer.h>
> #include <media/v4l2-ioctl.h>
> +#include <media/v4l2-event.h>
> #include <linux/videodev2.h>
> #include <media/v4l2-device.h>
> #include <media/v4l2-ctrls.h>
> @@ -824,6 +825,20 @@ static int adv7180_g_tvnorms(struct v4l2_subdev *sd, v4l2_std_id *norm)
> return 0;
> }
>
> +static int adv7180_subscribe_event(struct v4l2_subdev *sd,
> + struct v4l2_fh *fh,
> + struct v4l2_event_subscription *sub)
> +{
> + switch (sub->type) {
> + case V4L2_EVENT_SOURCE_CHANGE:
> + return v4l2_src_change_event_subdev_subscribe(sd, fh, sub);
> + case V4L2_EVENT_CTRL:
> + return v4l2_ctrl_subdev_subscribe_event(sd, fh, sub);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> static const struct v4l2_subdev_video_ops adv7180_video_ops = {
> .s_std = adv7180_s_std,
> .g_std = adv7180_g_std,
> @@ -838,6 +853,8 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops = {
>
> static const struct v4l2_subdev_core_ops adv7180_core_ops = {
> .s_power = adv7180_s_power,
> + .subscribe_event = adv7180_subscribe_event,
> + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> };
>
> static const struct v4l2_subdev_pad_ops adv7180_pad_ops = {
> @@ -862,8 +879,18 @@ static irqreturn_t adv7180_irq(int irq, void *devid)
> /* clear */
> adv7180_write(state, ADV7180_REG_ICR3, isr3);
>
> - if (isr3 & ADV7180_IRQ3_AD_CHANGE && state->autodetect)
> - __adv7180_status(state, NULL, &state->curr_norm);
> + if (isr3 & ADV7180_IRQ3_AD_CHANGE) {
> + static const struct v4l2_event src_ch = {
> + .type = V4L2_EVENT_SOURCE_CHANGE,
> + .u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
> + };
> +
> + v4l2_subdev_notify_event(&state->sd, &src_ch);
> +
> + if (state->autodetect)
> + __adv7180_status(state, NULL, &state->curr_norm);
> + }
> +
> mutex_unlock(&state->mutex);
>
> return IRQ_HANDLED;
> @@ -1403,7 +1430,7 @@ static int adv7180_probe(struct i2c_client *client,
> state->input = 0;
> sd = &state->sd;
> v4l2_i2c_subdev_init(sd, client, &adv7180_ops);
> - sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> + sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
>
> ret = adv7180_init_controls(state);
> if (ret)
Steve,
Tested on an IMX6 Gateworks Ventana with IMX6 capture drivers [1].
Tested-by: Tim Harvey <tharvey@gateworks.com>
Added to Cc:
Cc: Lars-Peter Clausen <lars@metafoo.de>
Regards,
Tim
[1] - http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/102914
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 05/11] media: adv7180: init chip with AD recommended register settings
2016-07-06 22:59 ` [PATCH 05/11] media: adv7180: init chip with AD recommended register settings Steve Longerbeam
2016-07-07 15:23 ` Tim Harvey
@ 2016-07-07 15:29 ` Lars-Peter Clausen
1 sibling, 0 replies; 55+ messages in thread
From: Lars-Peter Clausen @ 2016-07-07 15:29 UTC (permalink / raw)
To: Steve Longerbeam, linux-media; +Cc: Steve Longerbeam
On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
> Define and load register tables that conform to Analog Device's
> recommended register settings. It loads the default single-ended
> CVBS on Ain1 configuration for both ADV7180 and ADV7182 chips.
The driver should already setup the recommended configuration based on the
current mode. This patch seems to add a set of register writes that only
apply for a specific mode.
[...]
> Note this patch also enables NEWAVMODE, which is also recommended by
> Analog Devices. This will likely break any current backends using this
> subdev that are expecting different or manually configured AV codes.
>
> Note also that bt.656-4 support has been removed in this patch, but it
> will be brought back in a subsequent patch.
I'm not sure if we can or should break backwards compatibility in this way.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/11] media: adv7180: add power pin control
2016-07-06 22:59 ` [PATCH 03/11] media: adv7180: add power pin control Steve Longerbeam
2016-07-07 15:04 ` Tim Harvey
@ 2016-07-07 15:35 ` Lars-Peter Clausen
1 sibling, 0 replies; 55+ messages in thread
From: Lars-Peter Clausen @ 2016-07-07 15:35 UTC (permalink / raw)
To: Steve Longerbeam, linux-media; +Cc: Steve Longerbeam
> @@ -1190,6 +1207,20 @@ out_unlock:
> return ret;
> }
>
> +static int adv7180_of_parse(struct adv7180_state *state)
Since there is nothing of specific in here anymore the name should be
changed, or maybe just inline the code directly in probe.
> +{
> + struct i2c_client *client = state->client;
> +
> + state->pwdn_gpio = devm_gpiod_get_optional(&client->dev, "pwdn",
I'd use "powerdown", vowels don't cost extra ;):
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(state->pwdn_gpio)) {
> + v4l_err(client, "request for power pin failed\n");
Include the error number in the message.
> + return PTR_ERR(state->pwdn_gpio);
> + }
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 01/11] media: adv7180: Fix broken interrupt register access
2016-07-06 22:59 ` [PATCH 01/11] media: adv7180: Fix broken interrupt register access Steve Longerbeam
2016-07-07 14:44 ` Tim Harvey
@ 2016-07-07 15:37 ` Lars-Peter Clausen
1 sibling, 0 replies; 55+ messages in thread
From: Lars-Peter Clausen @ 2016-07-07 15:37 UTC (permalink / raw)
To: Steve Longerbeam, linux-media; +Cc: Steve Longerbeam
On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
> Access to the interrupt page registers has been broken since
> at least 3999e5d01da74f1a22afbb0b61b3992fea301478. That commit
> forgot to add the inerrupt page number to the register defines.
typo: interrupt
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
Looks good, thanks.
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 02/11] Revert "[media] adv7180: fix broken standards handling"
2016-07-06 22:59 ` [PATCH 02/11] Revert "[media] adv7180: fix broken standards handling" Steve Longerbeam
2016-07-07 14:48 ` Tim Harvey
@ 2016-07-07 15:45 ` Lars-Peter Clausen
2016-07-09 18:56 ` Steve Longerbeam
1 sibling, 1 reply; 55+ messages in thread
From: Lars-Peter Clausen @ 2016-07-07 15:45 UTC (permalink / raw)
To: Steve Longerbeam, linux-media; +Cc: Steve Longerbeam, Hans Verkuil
On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
> Autodetect was likely broken only because access to the
> interrupt registers were broken, so there were no standard
> change interrupts. After fixing that, and reverting this,
> autodetect seems to work just fine on an i.mx6q SabreAuto.
>
> This reverts commit 937feeed3f0ae8a0389d5732f6db63dd912acd99.
The brokenness the commit refers to is conceptual not functional. The driver
simply implemented the API incorrect. A subdev driver is not allowed to
automatically switch the output format/resolution randomly. In the best case
this will confuse the receiver which is not prepared to receive the changed
resolution, in the worst case it will cause buffer overruns with hardware
that has no boundary checks. This is why this was removed from the driver.
The correct sequence is for the driver to generate a change notification and
then have userspace react to that notification by stopping the current
stream, query the new format/resolution, reconfigure the video pipeline for
the new format/resolution and re-start the stream.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 07/11] media: adv7180: change mbus format to UYVY
2016-07-07 15:18 ` Lars-Peter Clausen
@ 2016-07-08 10:52 ` Niklas Söderlund
0 siblings, 0 replies; 55+ messages in thread
From: Niklas Söderlund @ 2016-07-08 10:52 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: Steve Longerbeam, linux-media, Steve Longerbeam
On 2016-07-07 17:18:25 +0200, Lars-Peter Clausen wrote:
> On 07/07/2016 01:00 AM, Steve Longerbeam wrote:
> > Change the media bus format from YUYV8_2X8 to UYVY8_2X8. Colors
> > now look correct when capturing with the i.mx6 backend. The other
> > option is to set the SWPC bit in register 0x27 to swap the Cr and Cb
> > output samples.
> >
> > Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>
> The patch is certainly correct from the technical point of view. But we need
> to be careful not to break any existing platforms which rely on this
> setting. So the alternative solution of changing the default output order is
> not an option.
>
> Looking at things it seems like the Renesas vin driver, which is used in
> combination with the adv7180 on some boards, uses the return value from
> enum_mbus_code to setup the video pipeline. Adding Niklas to Cc, maybe he
> can help to test this.
Yes this change will make the rcar-vin driver fail to probe since it do
not recognise the MEDIA_BUS_FMT_UYVY8_2X8 format.
There is a error in the Renesas VIN driver where it looks for the wrong
media format YUVU but expects UYVY data. This error have masked the bug
in adv7180 fixed in this commit.
I have have sent a patch '[media] rcar-vin: add legacy mode for wrong
media bus formats' which tries to remedy this. I would like to see that
patched (or similar solution) merged before this one as to not break the
Renesas R-Car2 Koelsch board which uses the adv7180.
If that can be arranged
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>
> But otherwise
>
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
>
> > ---
> > 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 fff887c..427695d 100644
> > --- a/drivers/media/i2c/adv7180.c
> > +++ b/drivers/media/i2c/adv7180.c
> > @@ -654,7 +654,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;
> > }
> > @@ -664,7 +664,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;
> >
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 02/11] Revert "[media] adv7180: fix broken standards handling"
2016-07-07 15:45 ` Lars-Peter Clausen
@ 2016-07-09 18:56 ` Steve Longerbeam
0 siblings, 0 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-09 18:56 UTC (permalink / raw)
To: Lars-Peter Clausen, linux-media; +Cc: Steve Longerbeam, Hans Verkuil
On 07/07/2016 08:45 AM, Lars-Peter Clausen wrote:
> On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
>> Autodetect was likely broken only because access to the
>> interrupt registers were broken, so there were no standard
>> change interrupts. After fixing that, and reverting this,
>> autodetect seems to work just fine on an i.mx6q SabreAuto.
>>
>> This reverts commit 937feeed3f0ae8a0389d5732f6db63dd912acd99.
> The brokenness the commit refers to is conceptual not functional. The driver
> simply implemented the API incorrect. A subdev driver is not allowed to
> automatically switch the output format/resolution randomly. In the best case
> this will confuse the receiver which is not prepared to receive the changed
> resolution, in the worst case it will cause buffer overruns with hardware
> that has no boundary checks. This is why this was removed from the driver.
>
> The correct sequence is for the driver to generate a change notification and
> then have userspace react to that notification by stopping the current
> stream, query the new format/resolution, reconfigure the video pipeline for
> the new format/resolution and re-start the stream.
Hi Lars, ok thanks for the clarification. Yes I agree that makes sense.
I will undo the revert in the next version and retest.
Steve
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/11] media: adv7180: add bt.656-4 OF property
2016-07-07 14:52 ` Lars-Peter Clausen
@ 2016-07-09 18:59 ` Steve Longerbeam
2016-07-09 21:10 ` Steve Longerbeam
0 siblings, 1 reply; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-09 18:59 UTC (permalink / raw)
To: Lars-Peter Clausen, linux-media; +Cc: Steve Longerbeam
On 07/07/2016 07:52 AM, Lars-Peter Clausen wrote:
> On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
>> Add a device tree boolean property "bt656-4" to allow setting
>> the ITU-R BT.656-4 compatible bit.
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>
> + /* select ITU-R BT.656-4 compatible? */
> + if (of_property_read_bool(client->dev.of_node, "bt656-4"))
> + state->bt656_4 = true;
> This property needs to be documented. In my opinion it should also be a
> property of the endpoint sub-node rather than the toplevel device node since
> this is a configuration of the endpoint format.
Agreed, it's really a config of the backend capture endpoint. I'll move it
there and also document it in
Documentation/devicetree/bindings/media/i2c/adv7180.txt.
Steve
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/11] media: adv7180: add bt.656-4 OF property
2016-07-09 18:59 ` Steve Longerbeam
@ 2016-07-09 21:10 ` Steve Longerbeam
2016-07-09 21:36 ` Steve Longerbeam
0 siblings, 1 reply; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-09 21:10 UTC (permalink / raw)
To: Steve Longerbeam, Lars-Peter Clausen, linux-media
On 07/09/2016 11:59 AM, Steve Longerbeam wrote:
>
>
> On 07/07/2016 07:52 AM, Lars-Peter Clausen wrote:
>> On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
>>> Add a device tree boolean property "bt656-4" to allow setting
>>> the ITU-R BT.656-4 compatible bit.
>>>
>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>
>> + /* select ITU-R BT.656-4 compatible? */
>> + if (of_property_read_bool(client->dev.of_node, "bt656-4"))
>> + state->bt656_4 = true;
>> This property needs to be documented. In my opinion it should also be a
>> property of the endpoint sub-node rather than the toplevel device
>> node since
>> this is a configuration of the endpoint format.
>
> Agreed, it's really a config of the backend capture endpoint. I'll
> move it
> there and also document it in
> Documentation/devicetree/bindings/media/i2c/adv7180.txt.
er, scratch that. ITU-R BT.656 compatibility is really a property of the
bt.656 bus. It
should be added to v4l2 endpoints and parsed by
v4l2_of_parse_endpoint(). I've created
a patch to add a "bt656-mode" property to v4l2 endpoints and will copy
all the maintainers.
Steve
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/11] media: adv7180: add bt.656-4 OF property
2016-07-09 21:10 ` Steve Longerbeam
@ 2016-07-09 21:36 ` Steve Longerbeam
2016-07-10 12:10 ` Lars-Peter Clausen
0 siblings, 1 reply; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-09 21:36 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: linux-media
On 07/09/2016 02:10 PM, Steve Longerbeam wrote:
>
>
> On 07/09/2016 11:59 AM, Steve Longerbeam wrote:
>>
>>
>> On 07/07/2016 07:52 AM, Lars-Peter Clausen wrote:
>>> On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
>>>> Add a device tree boolean property "bt656-4" to allow setting
>>>> the ITU-R BT.656-4 compatible bit.
>>>>
>>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>>
>>> + /* select ITU-R BT.656-4 compatible? */
>>> + if (of_property_read_bool(client->dev.of_node, "bt656-4"))
>>> + state->bt656_4 = true;
>>> This property needs to be documented. In my opinion it should also be a
>>> property of the endpoint sub-node rather than the toplevel device
>>> node since
>>> this is a configuration of the endpoint format.
>>
>> Agreed, it's really a config of the backend capture endpoint. I'll
>> move it
>> there and also document it in
>> Documentation/devicetree/bindings/media/i2c/adv7180.txt.
>
> er, scratch that. ITU-R BT.656 compatibility is really a property of
> the bt.656 bus. It
> should be added to v4l2 endpoints and parsed by
> v4l2_of_parse_endpoint(). I've created
> a patch to add a "bt656-mode" property to v4l2 endpoints and will copy
> all the maintainers.
But I'm going back and forth on whether this property should be added to
the adv7180's
local endpoint, or to the remote endpoint. I'm leaning towards the
remote endpoint, since
this is more about how the backend wants the bus configured.
Steve
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/11] media: adv7180: add bt.656-4 OF property
2016-07-09 21:36 ` Steve Longerbeam
@ 2016-07-10 12:10 ` Lars-Peter Clausen
2016-07-10 12:55 ` Hans Verkuil
0 siblings, 1 reply; 55+ messages in thread
From: Lars-Peter Clausen @ 2016-07-10 12:10 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media
On 07/09/2016 11:36 PM, Steve Longerbeam wrote:
>
>
> On 07/09/2016 02:10 PM, Steve Longerbeam wrote:
>>
>>
>> On 07/09/2016 11:59 AM, Steve Longerbeam wrote:
>>>
>>>
>>> On 07/07/2016 07:52 AM, Lars-Peter Clausen wrote:
>>>> On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
>>>>> Add a device tree boolean property "bt656-4" to allow setting
>>>>> the ITU-R BT.656-4 compatible bit.
>>>>>
>>>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>>>
>>>> + /* select ITU-R BT.656-4 compatible? */
>>>> + if (of_property_read_bool(client->dev.of_node, "bt656-4"))
>>>> + state->bt656_4 = true;
>>>> This property needs to be documented. In my opinion it should also be a
>>>> property of the endpoint sub-node rather than the toplevel device node
>>>> since
>>>> this is a configuration of the endpoint format.
>>>
>>> Agreed, it's really a config of the backend capture endpoint. I'll move it
>>> there and also document it in
>>> Documentation/devicetree/bindings/media/i2c/adv7180.txt.
>>
>> er, scratch that. ITU-R BT.656 compatibility is really a property of the
>> bt.656 bus. It
>> should be added to v4l2 endpoints and parsed by v4l2_of_parse_endpoint().
>> I've created
>> a patch to add a "bt656-mode" property to v4l2 endpoints and will copy all
>> the maintainers.
>
> But I'm going back and forth on whether this property should be added to the
> adv7180's
> local endpoint, or to the remote endpoint. I'm leaning towards the remote
> endpoint, since
> this is more about how the backend wants the bus configured.
I think it should be set for both, as both endpoints need to agree on the
format.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/11] media: adv7180: add bt.656-4 OF property
2016-07-10 12:10 ` Lars-Peter Clausen
@ 2016-07-10 12:55 ` Hans Verkuil
2016-07-10 14:17 ` Ian Arkver
0 siblings, 1 reply; 55+ messages in thread
From: Hans Verkuil @ 2016-07-10 12:55 UTC (permalink / raw)
To: Lars-Peter Clausen, Steve Longerbeam; +Cc: linux-media
On 07/10/2016 02:10 PM, Lars-Peter Clausen wrote:
> On 07/09/2016 11:36 PM, Steve Longerbeam wrote:
>>
>>
>> On 07/09/2016 02:10 PM, Steve Longerbeam wrote:
>>>
>>>
>>> On 07/09/2016 11:59 AM, Steve Longerbeam wrote:
>>>>
>>>>
>>>> On 07/07/2016 07:52 AM, Lars-Peter Clausen wrote:
>>>>> On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
>>>>>> Add a device tree boolean property "bt656-4" to allow setting
>>>>>> the ITU-R BT.656-4 compatible bit.
>>>>>>
>>>>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>>>>
>>>>> + /* select ITU-R BT.656-4 compatible? */
Please don't use the '-4': BT.656 is sufficient. The -4 is just the version
number of the standard (and 5 is the latest anyway).
>>>>> + if (of_property_read_bool(client->dev.of_node, "bt656-4"))
>>>>> + state->bt656_4 = true;
>>>>> This property needs to be documented. In my opinion it should also be a
>>>>> property of the endpoint sub-node rather than the toplevel device node
>>>>> since
>>>>> this is a configuration of the endpoint format.
>>>>
>>>> Agreed, it's really a config of the backend capture endpoint. I'll move it
>>>> there and also document it in
>>>> Documentation/devicetree/bindings/media/i2c/adv7180.txt.
>>>
>>> er, scratch that. ITU-R BT.656 compatibility is really a property of the
>>> bt.656 bus. It
>>> should be added to v4l2 endpoints and parsed by v4l2_of_parse_endpoint().
>>> I've created
>>> a patch to add a "bt656-mode" property to v4l2 endpoints and will copy all
>>> the maintainers.
>>
>> But I'm going back and forth on whether this property should be added to the
>> adv7180's
>> local endpoint, or to the remote endpoint. I'm leaning towards the remote
>> endpoint, since
>> this is more about how the backend wants the bus configured.
>
> I think it should be set for both, as both endpoints need to agree on the
> format.
Is it needed at all? Setting the polarity flags to H/VSYNC_ACTIVE_HIGH/LOW
will already signal BT.656 mode. See include/media/v4l2-mediabus.h and
v4l2-of.c.
I haven't looked in detail at adv7180, so I may have missed something, but this
looks strange.
Regards,
Hans
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/11] media: adv7180: add bt.656-4 OF property
2016-07-10 12:55 ` Hans Verkuil
@ 2016-07-10 14:17 ` Ian Arkver
2016-07-10 14:30 ` Hans Verkuil
0 siblings, 1 reply; 55+ messages in thread
From: Ian Arkver @ 2016-07-10 14:17 UTC (permalink / raw)
To: Hans Verkuil, Lars-Peter Clausen, Steve Longerbeam; +Cc: linux-media
On 10/07/16 13:55, Hans Verkuil wrote:
> On 07/10/2016 02:10 PM, Lars-Peter Clausen wrote:
>> On 07/09/2016 11:36 PM, Steve Longerbeam wrote:
>>>
>>> On 07/09/2016 02:10 PM, Steve Longerbeam wrote:
>>>>
>>>> On 07/09/2016 11:59 AM, Steve Longerbeam wrote:
>>>>>
>>>>> On 07/07/2016 07:52 AM, Lars-Peter Clausen wrote:
>>>>>> On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
>>>>>>> Add a device tree boolean property "bt656-4" to allow setting
>>>>>>> the ITU-R BT.656-4 compatible bit.
>>>>>>>
>>>>>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>>>>> + /* select ITU-R BT.656-4 compatible? */
> Please don't use the '-4': BT.656 is sufficient. The -4 is just the version
> number of the standard (and 5 is the latest anyway).
From the ADV7180 DS [1]:
BT.656-4, ITU-R BT.656-4 Enable, Address 0x04[7]
Between Revision 3 and Revision 4 of the ITU-R BT.656 standards, the ITU
has changed the toggling position for the V bit within the SAV EAV codes
for NTSC. The ITU-R BT.656-4 standard bit allows the user to select an
output mode that is compliant with either the previous or new standard.
For further information, visit the International Telecommunication Union
website.
Note that the standard change only affects NTSC and has no bearing on PAL.
When ITU-R BT.656-4 is 0 (default), the ITU-R BT.656-3 specification is
used. The V bit goes low at EAV of Line 10 and Line 273.
When ITU-R BT.656-4 is 1, the ITU-R BT.656-4 specification is used. The
V bit goes low at EAV of Line 20 and Line 283.
[1]www.analog.com/media/en/technical-documentation/data-sheets/*ADV7180*.pdf
>
>>>>>> + if (of_property_read_bool(client->dev.of_node, "bt656-4"))
>>>>>> + state->bt656_4 = true;
>>>>>> This property needs to be documented. In my opinion it should also be a
>>>>>> property of the endpoint sub-node rather than the toplevel device node
>>>>>> since
>>>>>> this is a configuration of the endpoint format.
>>>>> Agreed, it's really a config of the backend capture endpoint. I'll move it
>>>>> there and also document it in
>>>>> Documentation/devicetree/bindings/media/i2c/adv7180.txt.
>>>> er, scratch that. ITU-R BT.656 compatibility is really a property of the
>>>> bt.656 bus. It
>>>> should be added to v4l2 endpoints and parsed by v4l2_of_parse_endpoint().
>>>> I've created
>>>> a patch to add a "bt656-mode" property to v4l2 endpoints and will copy all
>>>> the maintainers.
>>> But I'm going back and forth on whether this property should be added to the
>>> adv7180's
>>> local endpoint, or to the remote endpoint. I'm leaning towards the remote
>>> endpoint, since
>>> this is more about how the backend wants the bus configured.
>> I think it should be set for both, as both endpoints need to agree on the
>> format.
> Is it needed at all? Setting the polarity flags to H/VSYNC_ACTIVE_HIGH/LOW
> will already signal BT.656 mode. See include/media/v4l2-mediabus.h and
> v4l2-of.c.
>
> I haven't looked in detail at adv7180, so I may have missed something, but this
> looks strange.
>
> Regards,
>
> Hans
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/11] media: adv7180: add bt.656-4 OF property
2016-07-10 14:17 ` Ian Arkver
@ 2016-07-10 14:30 ` Hans Verkuil
2016-07-10 22:34 ` Steve Longerbeam
0 siblings, 1 reply; 55+ messages in thread
From: Hans Verkuil @ 2016-07-10 14:30 UTC (permalink / raw)
To: Ian Arkver, Lars-Peter Clausen, Steve Longerbeam; +Cc: linux-media
On 07/10/2016 04:17 PM, Ian Arkver wrote:
> On 10/07/16 13:55, Hans Verkuil wrote:
>> On 07/10/2016 02:10 PM, Lars-Peter Clausen wrote:
>>> On 07/09/2016 11:36 PM, Steve Longerbeam wrote:
>>>>
>>>> On 07/09/2016 02:10 PM, Steve Longerbeam wrote:
>>>>>
>>>>> On 07/09/2016 11:59 AM, Steve Longerbeam wrote:
>>>>>>
>>>>>> On 07/07/2016 07:52 AM, Lars-Peter Clausen wrote:
>>>>>>> On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
>>>>>>>> Add a device tree boolean property "bt656-4" to allow setting
>>>>>>>> the ITU-R BT.656-4 compatible bit.
>>>>>>>>
>>>>>>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>>>>>> + /* select ITU-R BT.656-4 compatible? */
>> Please don't use the '-4': BT.656 is sufficient. The -4 is just the version
>> number of the standard (and 5 is the latest anyway).
> From the ADV7180 DS [1]:
>
> BT.656-4, ITU-R BT.656-4 Enable, Address 0x04[7]
>
> Between Revision 3 and Revision 4 of the ITU-R BT.656 standards, the ITU
> has changed the toggling position for the V bit within the SAV EAV codes
> for NTSC. The ITU-R BT.656-4 standard bit allows the user to select an
> output mode that is compliant with either the previous or new standard.
> For further information, visit the International Telecommunication Union
> website.
>
> Note that the standard change only affects NTSC and has no bearing on PAL.
>
> When ITU-R BT.656-4 is 0 (default), the ITU-R BT.656-3 specification is
> used. The V bit goes low at EAV of Line 10 and Line 273.
>
> When ITU-R BT.656-4 is 1, the ITU-R BT.656-4 specification is used. The
> V bit goes low at EAV of Line 20 and Line 283.
>
> [1]www.analog.com/media/en/technical-documentation/data-sheets/*ADV7180*.pdf
Rev 4 came in in 1998. I would say that that is the default. We have had any
problems with this and I would need some proof that this is really needed.
Regards,
Hans
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/11] media: adv7180: add bt.656-4 OF property
2016-07-10 14:30 ` Hans Verkuil
@ 2016-07-10 22:34 ` Steve Longerbeam
2016-07-11 7:06 ` Ian Arkver
0 siblings, 1 reply; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-10 22:34 UTC (permalink / raw)
To: Hans Verkuil, Ian Arkver, Lars-Peter Clausen; +Cc: linux-media
On 07/10/2016 07:30 AM, Hans Verkuil wrote:
> On 07/10/2016 04:17 PM, Ian Arkver wrote:
>> On 10/07/16 13:55, Hans Verkuil wrote:
>>> On 07/10/2016 02:10 PM, Lars-Peter Clausen wrote:
>>>> On 07/09/2016 11:36 PM, Steve Longerbeam wrote:
>>>>> On 07/09/2016 02:10 PM, Steve Longerbeam wrote:
>>>>>> On 07/09/2016 11:59 AM, Steve Longerbeam wrote:
>>>>>>> On 07/07/2016 07:52 AM, Lars-Peter Clausen wrote:
>>>>>>>> On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
>>>>>>>>> Add a device tree boolean property "bt656-4" to allow setting
>>>>>>>>> the ITU-R BT.656-4 compatible bit.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>>>>>>> + /* select ITU-R BT.656-4 compatible? */
>>> Please don't use the '-4': BT.656 is sufficient. The -4 is just the version
>>> number of the standard (and 5 is the latest anyway).
>> From the ADV7180 DS [1]:
>>
>> BT.656-4, ITU-R BT.656-4 Enable, Address 0x04[7]
>>
>> Between Revision 3 and Revision 4 of the ITU-R BT.656 standards, the ITU
>> has changed the toggling position for the V bit within the SAV EAV codes
>> for NTSC. The ITU-R BT.656-4 standard bit allows the user to select an
>> output mode that is compliant with either the previous or new standard.
>> For further information, visit the International Telecommunication Union
>> website.
>>
>> Note that the standard change only affects NTSC and has no bearing on PAL.
>>
>> When ITU-R BT.656-4 is 0 (default), the ITU-R BT.656-3 specification is
>> used. The V bit goes low at EAV of Line 10 and Line 273.
>>
>> When ITU-R BT.656-4 is 1, the ITU-R BT.656-4 specification is used. The
>> V bit goes low at EAV of Line 20 and Line 283.
>>
>> [1]www.analog.com/media/en/technical-documentation/data-sheets/*ADV7180*.pdf
> Rev 4 came in in 1998. I would say that that is the default. We have had any
> problems with this and I would need some proof that this is really needed.
Hi Hans, yeah I agree with you that new capture drivers should not
expect BT.656-3 compatible buss' any longer. The problem with i.MX6
however, is that the IPU CSI CCIR_CODE registers, which inform the IPU
about the AV code positions, are very poorly documented. In order for
i.MX6 to support BT.656-4 it would require very time-consuming reverse
engineering to find the right values to plug into those registers to conform
to the BT.656-4 AV code positions.
Steve
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/11] media: adv7180: add bt.656-4 OF property
2016-07-10 22:34 ` Steve Longerbeam
@ 2016-07-11 7:06 ` Ian Arkver
2016-07-11 22:03 ` Steve Longerbeam
0 siblings, 1 reply; 55+ messages in thread
From: Ian Arkver @ 2016-07-11 7:06 UTC (permalink / raw)
To: Steve Longerbeam, Hans Verkuil, Lars-Peter Clausen; +Cc: linux-media
On 10/07/16 23:34, Steve Longerbeam wrote:
>
>
> On 07/10/2016 07:30 AM, Hans Verkuil wrote:
>> On 07/10/2016 04:17 PM, Ian Arkver wrote:
>>> On 10/07/16 13:55, Hans Verkuil wrote:
>>>> On 07/10/2016 02:10 PM, Lars-Peter Clausen wrote:
>>>>> On 07/09/2016 11:36 PM, Steve Longerbeam wrote:
>>>>>> On 07/09/2016 02:10 PM, Steve Longerbeam wrote:
>>>>>>> On 07/09/2016 11:59 AM, Steve Longerbeam wrote:
>>>>>>>> On 07/07/2016 07:52 AM, Lars-Peter Clausen wrote:
>>>>>>>>> On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
>>>>>>>>>> Add a device tree boolean property "bt656-4" to allow setting
>>>>>>>>>> the ITU-R BT.656-4 compatible bit.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>>>>>>>> + /* select ITU-R BT.656-4 compatible? */
>>>> Please don't use the '-4': BT.656 is sufficient. The -4 is just the
>>>> version
>>>> number of the standard (and 5 is the latest anyway).
>>> From the ADV7180 DS [1]:
>>>
>>> BT.656-4, ITU-R BT.656-4 Enable, Address 0x04[7]
>>>
>>> Between Revision 3 and Revision 4 of the ITU-R BT.656 standards, the
>>> ITU
>>> has changed the toggling position for the V bit within the SAV EAV
>>> codes
>>> for NTSC. The ITU-R BT.656-4 standard bit allows the user to select an
>>> output mode that is compliant with either the previous or new standard.
>>> For further information, visit the International Telecommunication
>>> Union
>>> website.
>>>
>>> Note that the standard change only affects NTSC and has no bearing
>>> on PAL.
>>>
>>> When ITU-R BT.656-4 is 0 (default), the ITU-R BT.656-3 specification is
>>> used. The V bit goes low at EAV of Line 10 and Line 273.
>>>
>>> When ITU-R BT.656-4 is 1, the ITU-R BT.656-4 specification is used. The
>>> V bit goes low at EAV of Line 20 and Line 283.
>>>
>>> [1]www.analog.com/media/en/technical-documentation/data-sheets/*ADV7180*.pdf
>>>
>> Rev 4 came in in 1998. I would say that that is the default. We have
>> had any
>> problems with this and I would need some proof that this is really
>> needed.
>
> Hi Hans, yeah I agree with you that new capture drivers should not
> expect BT.656-3 compatible buss' any longer. The problem with i.MX6
> however, is that the IPU CSI CCIR_CODE registers, which inform the IPU
> about the AV code positions, are very poorly documented. In order for
> i.MX6 to support BT.656-4 it would require very time-consuming reverse
> engineering to find the right values to plug into those registers to
> conform
> to the BT.656-4 AV code positions.
>
> Steve
>
Hi Steve,
Once you dsicover that the 3-bit fields in each CCIR_CODE register
correspond to the H, V and F flags in the SAV/EAV code (in that order,
MSbit->LSbit), those registers make more sense. Pity that's not
mentioned in the Ref Manual.
However, I don't think that's relevant here, since the spec revision
didn't change the codes, but just moved the lines the V bit is sent on.
I believe the spec change switched the NTSC timing from VSYNC to VBLANK,
but the net effect was 10 fewer "active" video lines per field. Looking
at a sample of one other video decoder (tvp5150), the default seems to
be to change V on lines 20 and 283, as per the newer revision of the
spec., though again bets may have been hedged with a programmable override.
In any case, I'm wondering if your extra ten lines per field are more
related to this snippet from calc_default_crop in imx-camif.c, which
seems like it would break other decoder front ends and adv7180 in
bt656-4 mode...
/*
* FIXME: For NTSC standards, top must be set to an
* offset of 13 lines to match fixed CCIR programming
* in the IPU.
*/
if (std != V4L2_STD_UNKNOWN && (std & V4L2_STD_525_60))
rect->top = 13;
I believe tvp5150 at least sends 243 active lines per field for an NTSC
source and the top 3 lines are generally dropped.
Regards,
Ian
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/11] media: adv7180: add bt.656-4 OF property
2016-07-11 7:06 ` Ian Arkver
@ 2016-07-11 22:03 ` Steve Longerbeam
2016-07-12 10:25 ` Ian Arkver
0 siblings, 1 reply; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-11 22:03 UTC (permalink / raw)
To: Ian Arkver, Hans Verkuil, Lars-Peter Clausen; +Cc: linux-media
On 07/11/2016 12:06 AM, Ian Arkver wrote:
> On 10/07/16 23:34, Steve Longerbeam wrote:
>>
>>
>> On 07/10/2016 07:30 AM, Hans Verkuil wrote:
>>> On 07/10/2016 04:17 PM, Ian Arkver wrote:
>>>> On 10/07/16 13:55, Hans Verkuil wrote:
>>>>> On 07/10/2016 02:10 PM, Lars-Peter Clausen wrote:
>>>>>> On 07/09/2016 11:36 PM, Steve Longerbeam wrote:
>>>>>>> On 07/09/2016 02:10 PM, Steve Longerbeam wrote:
>>>>>>>> On 07/09/2016 11:59 AM, Steve Longerbeam wrote:
>>>>>>>>> On 07/07/2016 07:52 AM, Lars-Peter Clausen wrote:
>>>>>>>>>> On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
>>>>>>>>>>> Add a device tree boolean property "bt656-4" to allow setting
>>>>>>>>>>> the ITU-R BT.656-4 compatible bit.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>>>>>>>>> + /* select ITU-R BT.656-4 compatible? */
>>>>> Please don't use the '-4': BT.656 is sufficient. The -4 is just the version
>>>>> number of the standard (and 5 is the latest anyway).
>>>> From the ADV7180 DS [1]:
>>>>
>>>> BT.656-4, ITU-R BT.656-4 Enable, Address 0x04[7]
>>>>
>>>> Between Revision 3 and Revision 4 of the ITU-R BT.656 standards, the ITU
>>>> has changed the toggling position for the V bit within the SAV EAV codes
>>>> for NTSC. The ITU-R BT.656-4 standard bit allows the user to select an
>>>> output mode that is compliant with either the previous or new standard.
>>>> For further information, visit the International Telecommunication Union
>>>> website.
>>>>
>>>> Note that the standard change only affects NTSC and has no bearing on PAL.
>>>>
>>>> When ITU-R BT.656-4 is 0 (default), the ITU-R BT.656-3 specification is
>>>> used. The V bit goes low at EAV of Line 10 and Line 273.
>>>>
>>>> When ITU-R BT.656-4 is 1, the ITU-R BT.656-4 specification is used. The
>>>> V bit goes low at EAV of Line 20 and Line 283.
>>>>
>>>> [1]www.analog.com/media/en/technical-documentation/data-sheets/*ADV7180*.pdf
>>> Rev 4 came in in 1998. I would say that that is the default. We have had any
>>> problems with this and I would need some proof that this is really needed.
>>
>> Hi Hans, yeah I agree with you that new capture drivers should not
>> expect BT.656-3 compatible buss' any longer. The problem with i.MX6
>> however, is that the IPU CSI CCIR_CODE registers, which inform the IPU
>> about the AV code positions, are very poorly documented. In order for
>> i.MX6 to support BT.656-4 it would require very time-consuming reverse
>> engineering to find the right values to plug into those registers to conform
>> to the BT.656-4 AV code positions.
>>
>> Steve
>>
> Hi Steve,
>
> Once you dsicover that the 3-bit fields in each CCIR_CODE register correspond to the H, V and F flags in the SAV/EAV code (in that order, MSbit->LSbit), those registers make more sense. Pity that's
> not mentioned in the Ref Manual.
Hi Ian, that's a plausible theory, but it doesn't work. I looked at the value
programmed to CCIR_CODE_1 register (according to imx6 ref manual is
values for first field), for NTSC : 0xD07DF. Comparing with the definition of
the H/V/F bits in the AV codes in the bt.656 spec:
F = 1 during field 2, 0 during field 1
V = 1 during field blanking, 0 elsewhere
H = 1 in EAV, 0 in SAV
There's no correspondence, for example F bit should be zero everywhere in
CCIR_CODE_1. And wutz this "first/second blanking line command" stuff about?
None of it makes any sense to me.
> However, I don't think that's relevant here, since the spec revision didn't change the codes, but just moved the lines the V bit is sent on. I believe the spec change switched the NTSC timing from
> VSYNC to VBLANK, but the net effect was 10 fewer "active" video lines per field. Looking at a sample of one other video decoder (tvp5150), the default seems to be to change V on lines 20 and 283, as
> per the newer revision of the spec., though again bets may have been hedged with a programmable override.
>
> In any case, I'm wondering if your extra ten lines per field are more related to this snippet from calc_default_crop in imx-camif.c, which seems like it would break other decoder front ends and
> adv7180 in bt656-4 mode...
>
> /*
> * FIXME: For NTSC standards, top must be set to an
> * offset of 13 lines to match fixed CCIR programming
> * in the IPU.
> */
> if (std != V4L2_STD_UNKNOWN && (std & V4L2_STD_525_60))
> rect->top = 13;
could be. I'll try removing that FIXME block and try with bt.656-4 mode.
Steve
>
> I believe tvp5150 at least sends 243 active lines per field for an NTSC source and the top 3 lines are generally dropped.
>
> Regards,
> Ian
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/11] media: adv7180: add bt.656-4 OF property
2016-07-11 22:03 ` Steve Longerbeam
@ 2016-07-12 10:25 ` Ian Arkver
2016-07-12 17:26 ` Steve Longerbeam
0 siblings, 1 reply; 55+ messages in thread
From: Ian Arkver @ 2016-07-12 10:25 UTC (permalink / raw)
To: Steve Longerbeam
Cc: linux-media, Philipp Zabel, Lars-Peter Clausen, Hans Verkuil
This conversation has drifted off topic, sorry.
It now relates to code in drivers/gpu/ipu-v3/ipu-csi.c, so adding
Philipp Zabel.
On 11/07/16 23:03, Steve Longerbeam wrote:
> On 07/11/2016 12:06 AM, Ian Arkver wrote:
>> On 10/07/16 23:34, Steve Longerbeam wrote:
>>>
>>> On 07/10/2016 07:30 AM, Hans Verkuil wrote:
>>>> On 07/10/2016 04:17 PM, Ian Arkver wrote:
>>>>> On 10/07/16 13:55, Hans Verkuil wrote:
>>>>>> On 07/10/2016 02:10 PM, Lars-Peter Clausen wrote:
>>>>>>> On 07/09/2016 11:36 PM, Steve Longerbeam wrote:
>>>>>>>> On 07/09/2016 02:10 PM, Steve Longerbeam wrote:
>>>>>>>>> On 07/09/2016 11:59 AM, Steve Longerbeam wrote:
>>>>>>>>>> On 07/07/2016 07:52 AM, Lars-Peter Clausen wrote:
>>>>>>>>>>> On 07/07/2016 12:59 AM, Steve Longerbeam wrote:
>>>>>>>>>>>> Add a device tree boolean property "bt656-4" to allow setting
>>>>>>>>>>>> the ITU-R BT.656-4 compatible bit.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>>>>>>>>>> + /* select ITU-R BT.656-4 compatible? */
>>>>>> Please don't use the '-4': BT.656 is sufficient. The -4 is just the version
>>>>>> number of the standard (and 5 is the latest anyway).
>>>>> From the ADV7180 DS [1]:
>>>>>
>>>>> BT.656-4, ITU-R BT.656-4 Enable, Address 0x04[7]
>>>>>
>>>>> Between Revision 3 and Revision 4 of the ITU-R BT.656 standards, the ITU
>>>>> has changed the toggling position for the V bit within the SAV EAV codes
>>>>> for NTSC. The ITU-R BT.656-4 standard bit allows the user to select an
>>>>> output mode that is compliant with either the previous or new standard.
>>>>> For further information, visit the International Telecommunication Union
>>>>> website.
>>>>>
>>>>> Note that the standard change only affects NTSC and has no bearing on PAL.
>>>>>
>>>>> When ITU-R BT.656-4 is 0 (default), the ITU-R BT.656-3 specification is
>>>>> used. The V bit goes low at EAV of Line 10 and Line 273.
>>>>>
>>>>> When ITU-R BT.656-4 is 1, the ITU-R BT.656-4 specification is used. The
>>>>> V bit goes low at EAV of Line 20 and Line 283.
>>>>>
>>>>> [1]www.analog.com/media/en/technical-documentation/data-sheets/*ADV7180*.pdf
>>>> Rev 4 came in in 1998. I would say that that is the default. We have had any
>>>> problems with this and I would need some proof that this is really needed.
>>> Hi Hans, yeah I agree with you that new capture drivers should not
>>> expect BT.656-3 compatible buss' any longer. The problem with i.MX6
>>> however, is that the IPU CSI CCIR_CODE registers, which inform the IPU
>>> about the AV code positions, are very poorly documented. In order for
>>> i.MX6 to support BT.656-4 it would require very time-consuming reverse
>>> engineering to find the right values to plug into those registers to conform
>>> to the BT.656-4 AV code positions.
>>>
>>> Steve
>>>
>> Hi Steve,
>>
>> Once you dsicover that the 3-bit fields in each CCIR_CODE register correspond to the H, V and F flags in the SAV/EAV code (in that order, MSbit->LSbit), those registers make more sense. Pity that's
>> not mentioned in the Ref Manual.
> Hi Ian, that's a plausible theory, but it doesn't work. I looked at the value
> programmed to CCIR_CODE_1 register (according to imx6 ref manual is
> values for first field), for NTSC : 0xD07DF. Comparing with the definition of
> the H/V/F bits in the AV codes in the bt.656 spec:
>
> F = 1 during field 2, 0 during field 1
> V = 1 during field blanking, 0 elsewhere
> H = 1 in EAV, 0 in SAV
>
> There's no correspondence, for example F bit should be zero everywhere in
> CCIR_CODE_1. And wutz this "first/second blanking line command" stuff about?
> None of it makes any sense to me.
Hi,
d07df = 001 101 xxxx 011 111 011 111
H V F
CSI0_STRT_FLD0_ACTV 0 0 1
CSI0_END_FLD0_ACTV 1 0 1
CSI0_STRT_FLD0_BLNK_2ND 0 1 1
CSI0_END_FLD0_BLNK_2ND 1 1 1
CSI0_STRT_FLD0_BLNK_1ST 0 1 1
CSI0_END_FLD0_BLNK_1ST 1 1 1
40596 = 000 100 xxxx 010 110 010 110
405A6 = 000 100 xxxx 010 110 100 110
H V F
CSI0_STRT_FLD1_ACTV 0 0 0
CSI0_END_FLD1_ACTV 1 0 0
CSI0_STRT_FLD1_BLNK_2ND 0 1 0
CSI0_END_FLD1_BLNK_2ND 1 1 0
CSI0_STRT_FLD1_BLNK_1ST 0 1 0 or 1 0 0 ?
CSI0_END_FLD1_BLNK_1ST 1 1 0
For PAL: IPU_CSI0_CCIR_CODE_1 = 0x40596, IPU_CSI0_CCIR_CODE_1 = 0xd07df
For NTSC: IPU_CSI0_CCIR_CODE_1 = 0xd07df, IPU_CSI0_CCIR_CODE_1 = 0x405A6
So, for the PAL case the field values make sense to me. The F bit is
constant throughout each field.
I'm not sure why the NTSC case has 405A6 instead of 40596 again. This
looks like a bug to me.
If you look back at the original Freescale capture driver[1], they use
40596 for both PAL and NTSC.
The values are swapped between NTSC and PAL to account for the differing
field order. I think using
the capture width and height to do this is poor as any "bt.656-like"
source with non standard
dimensions will end up in the dev_err("Unsupported") case. Also, looking
at BT.1120-8, for interlaced
video the same SAV and EAV codes as PAL are used, so
IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR and
IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR modes are currently broken.
I think the 1st and 2nd blanking values might be needed for some odd non
656 streams maybe? Also for
progressive (originally) the value 40030 is used which is
40030 = 000 100 xxxx 000 000 110 000
This leaves the 2nd blank values empty. In the HW I'd guess both fire on
SAV but there's a fixed
event priority in the state machine, or it ignores events where both
STRT and END fire on the same
clock tick.
btw: The patch I sent you a while back[2] dumps the CCIR values in octal
which makes them
a bit more readable. It also fixes up theCSI0_STRT_FLD0_BLNK_1ST value
for progressive bt.656 to
be {0 1 0}, which matches what's seen in the stream. In theory the
blanking end value should
be {1 1 0}, but it seems to work OK with the same code as unblanked SAV
{0 0 0}. If it's useful
I can rebase and post that patch to the ML.
Maybe I should create an RFC patch for ipu-csi.c and stop hijacking this
thread?
1:
https://github.com/Freescale/linux-fslc/blob/3.14-1.1.x-imx/drivers/mxc/ipu3/ipu_capture.c#L168
2: 0003-ipu_csi-more-debug.-Fix-Blanking-SAV-for-progressive.patch
>
>> However, I don't think that's relevant here, since the spec revision didn't change the codes, but just moved the lines the V bit is sent on. I believe the spec change switched the NTSC timing from
>> VSYNC to VBLANK, but the net effect was 10 fewer "active" video lines per field. Looking at a sample of one other video decoder (tvp5150), the default seems to be to change V on lines 20 and 283, as
>> per the newer revision of the spec., though again bets may have been hedged with a programmable override.
>>
>> In any case, I'm wondering if your extra ten lines per field are more related to this snippet from calc_default_crop in imx-camif.c, which seems like it would break other decoder front ends and
>> adv7180 in bt656-4 mode...
>>
>> /*
>> * FIXME: For NTSC standards, top must be set to an
>> * offset of 13 lines to match fixed CCIR programming
>> * in the IPU.
>> */
>> if (std != V4L2_STD_UNKNOWN && (std & V4L2_STD_525_60))
>> rect->top = 13;
> could be. I'll try removing that FIXME block and try with bt.656-4 mode.
>
> Steve
>
>> I believe tvp5150 at least sends 243 active lines per field for an NTSC source and the top 3 lines are generally dropped.
>>
>> Regards,
>> Ian
All the best,
Ian
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/11] media: adv7180: add bt.656-4 OF property
2016-07-12 10:25 ` Ian Arkver
@ 2016-07-12 17:26 ` Steve Longerbeam
0 siblings, 0 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-12 17:26 UTC (permalink / raw)
To: Ian Arkver; +Cc: linux-media, Philipp Zabel, Lars-Peter Clausen, Hans Verkuil
Hi Ian,
On 07/12/2016 03:25 AM, Ian Arkver wrote:
> This conversation has drifted off topic, sorry.
> It now relates to code in drivers/gpu/ipu-v3/ipu-csi.c, so adding Philipp Zabel.
>
<snip>
> On 11/07/16 23:03, Steve Longerbeam wrote:
>> On 07/11/2016 12:06 AM, Ian Arkver wrote:
>>>
>>> Hi Steve,
>>>
>>> Once you dsicover that the 3-bit fields in each CCIR_CODE register correspond to the H, V and F flags in the SAV/EAV code (in that order, MSbit->LSbit), those registers make more sense. Pity that's
>>> not mentioned in the Ref Manual.
>> Hi Ian, that's a plausible theory, but it doesn't work. I looked at the value
>> programmed to CCIR_CODE_1 register (according to imx6 ref manual is
>> values for first field), for NTSC : 0xD07DF. Comparing with the definition of
>> the H/V/F bits in the AV codes in the bt.656 spec:
>>
>> F = 1 during field 2, 0 during field 1
>> V = 1 during field blanking, 0 elsewhere
>> H = 1 in EAV, 0 in SAV
>>
>> There's no correspondence, for example F bit should be zero everywhere in
>> CCIR_CODE_1. And wutz this "first/second blanking line command" stuff about?
>> None of it makes any sense to me.
> Hi,
>
> d07df = 001 101 xxxx 011 111 011 111
Oops, I was misreading the register fields in the ref manual.
I misread the reserved field starting at bit 12 as 3 bits long, but
it is 4 bits long. The H,V,F bits for those CCIR_CODE values make
sense now.
So this clears up a lot for me. Due to confusion surrounding these
registers, one of my theories as to what these registers were doing
was that they were telling the CSI actually where to look for the SAV/EAV
codes in the stream, as a line number or something. But I see now that
the registers are simply telling the IPU about the standard, so they are
more like "set it and forget it" registers, and would only be set to something
other than what the standard specifies in order to deal with non-standard
streams (like adv718x "NEWAVMODE", see below).
So the IPU must be detecting the AV codes using the AV code preamble,
which is a good thing.
Also, it clears up how to deal with bt.656-3 vs. bt.656-4 in the imx6 backend a bit
more. The CCIR code registers do not need to be touched when switching between
bt.656-3 and bt.656-4. It's more a matter of the number of active lines the decoder
sends (bt.656-3 has 10 extra active lines).
>
> H V F
> CSI0_STRT_FLD0_ACTV 0 0 1
> CSI0_END_FLD0_ACTV 1 0 1
> CSI0_STRT_FLD0_BLNK_2ND 0 1 1
> CSI0_END_FLD0_BLNK_2ND 1 1 1
> CSI0_STRT_FLD0_BLNK_1ST 0 1 1
> CSI0_END_FLD0_BLNK_1ST 1 1 1
>
> 40596 = 000 100 xxxx 010 110 010 110
> 405A6 = 000 100 xxxx 010 110 100 110
>
> H V F
> CSI0_STRT_FLD1_ACTV 0 0 0
> CSI0_END_FLD1_ACTV 1 0 0
> CSI0_STRT_FLD1_BLNK_2ND 0 1 0
> CSI0_END_FLD1_BLNK_2ND 1 1 0
> CSI0_STRT_FLD1_BLNK_1ST 0 1 0 or 1 0 0 ?
> CSI0_END_FLD1_BLNK_1ST 1 1 0
>
> For PAL: IPU_CSI0_CCIR_CODE_1 = 0x40596, IPU_CSI0_CCIR_CODE_1 = 0xd07df
> For NTSC: IPU_CSI0_CCIR_CODE_1 = 0xd07df, IPU_CSI0_CCIR_CODE_1 = 0x405A6
>
> So, for the PAL case the field values make sense to me. The F bit is constant throughout each field.
> I'm not sure why the NTSC case has 405A6 instead of 40596 again. This looks like a bug to me.
Actually we made this change to deal with "NEWAVMODE" setting in the adv7180. But now
I see that NEWAVMODE breaks the bt.656 standard, and will create problems for other backends.
So I will remove the NEWAVMODE patch to adv7180, and revert the patch that sets the above
0x405A6 value in IPU_CSI0_CCIR_CODE_1.
> If you look back at the original Freescale capture driver[1], they use 40596 for both PAL and NTSC.
>
> The values are swapped between NTSC and PAL to account for the differing field order. I think using
> the capture width and height to do this is poor as any "bt.656-like" source with non standard
> dimensions will end up in the dev_err("Unsupported") case.
yes this code was inherited originally from Freescale. There's needs to be a better
API for that.
> Also, looking at BT.1120-8, for interlaced
> video the same SAV and EAV codes as PAL are used, so IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR and
> IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR modes are currently broken.
right, I haven't even touched bt.1120 support. I don't have any h/w to test.
>
> I think the 1st and 2nd blanking values might be needed for some odd non 656 streams maybe?
> Also for progressive (originally) the value 40030 is used which is
>
> 40030 = 000 100 xxxx 000 000 110 000
>
> This leaves the 2nd blank values empty. In the HW I'd guess both fire on SAV but there's a fixed
> event priority in the state machine, or it ignores events where both STRT and END fire on the same
> clock tick.
Maybe the "1st" and "2nd" blanking fields just provide alternative matches
for the received AV codes.
>
> btw: The patch I sent you a while back[2] dumps the CCIR values in octal which makes them
> a bit more readable. It also fixes up theCSI0_STRT_FLD0_BLNK_1ST value for progressive bt.656 to
> be {0 1 0}, which matches what's seen in the stream. In theory the blanking end value should
> be {1 1 0}, but it seems to work OK with the same code as unblanked SAV {0 0 0}. If it's useful
> I can rebase and post that patch to the ML.
>
> Maybe I should create an RFC patch for ipu-csi.c and stop hijacking this thread?
Sure please do.
>
> 1: https://github.com/Freescale/linux-fslc/blob/3.14-1.1.x-imx/drivers/mxc/ipu3/ipu_capture.c#L168
> 2: 0003-ipu_csi-more-debug.-Fix-Blanking-SAV-for-progressive.patch
>
>
>>
>>> However, I don't think that's relevant here, since the spec revision didn't change the codes, but just moved the lines the V bit is sent on. I believe the spec change switched the NTSC timing from
>>> VSYNC to VBLANK, but the net effect was 10 fewer "active" video lines per field. Looking at a sample of one other video decoder (tvp5150), the default seems to be to change V on lines 20 and 283, as
>>> per the newer revision of the spec., though again bets may have been hedged with a programmable override.
>>>
>>> In any case, I'm wondering if your extra ten lines per field are more related to this snippet from calc_default_crop in imx-camif.c, which seems like it would break other decoder front ends and
>>> adv7180 in bt656-4 mode...
>>>
>>> /*
>>> * FIXME: For NTSC standards, top must be set to an
>>> * offset of 13 lines to match fixed CCIR programming
>>> * in the IPU.
>>> */
>>> if (std != V4L2_STD_UNKNOWN && (std & V4L2_STD_525_60))
>>> rect->top = 13;
>> could be. I'll try removing that FIXME block and try with bt.656-4 mode.
Setting rect->top = 3 with bt.656-4 produces stable video. And I will fix
the FIXME comment, which reflects my confusion about the CCIR code registers.
Steve
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH v2 00/10] adv7180 subdev fixes, v2
2016-07-06 22:59 [PATCH 00/11] adv7180 subdev fixes Steve Longerbeam
` (11 preceding siblings ...)
2016-07-07 14:37 ` [PATCH 00/11] adv7180 subdev fixes Tim Harvey
@ 2016-07-20 0:03 ` Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 01/10] v4l: of: add "newavmode" property for Analog Devices codecs Steve Longerbeam
` (9 more replies)
12 siblings, 10 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-20 0:03 UTC (permalink / raw)
To: lars; +Cc: linux-media, linux-kernel, Steve Longerbeam
This version adds a new v4l2 endpoint property and BT.656 bus flag
for the "NEWAVMODE" setting of Analog Devices decoders. The i.MX6
capture backend is not able to sync on the bt.656 stream from the
ADV7180 when the latter is in manual video standard setting mode,
unless NEWWAVMODE is used in conjunction. The backend needs to be
aware of NEWAVMODE so that it can make adjustments to the AV code
detection.
That's the biggest addition in this version, besides the requested
feedback changes from last version.
Steve Longerbeam (10):
v4l: of: add "newavmode" property for Analog Devices codecs
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 | 5 +
.../devicetree/bindings/media/video-interfaces.txt | 2 +
drivers/media/i2c/Kconfig | 2 +-
drivers/media/i2c/adv7180.c | 233 ++++++++++++++++-----
drivers/media/v4l2-core/v4l2-of.c | 4 +
include/media/v4l2-mediabus.h | 5 +
include/uapi/linux/videodev2.h | 1 +
8 files changed, 214 insertions(+), 50 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH v2 01/10] v4l: of: add "newavmode" property for Analog Devices codecs
2016-07-20 0:03 ` [PATCH v2 00/10] adv7180 subdev fixes, v2 Steve Longerbeam
@ 2016-07-20 0:03 ` Steve Longerbeam
2016-07-20 7:37 ` Hans Verkuil
2016-07-20 0:03 ` [PATCH v2 02/10] media: adv7180: Fix broken interrupt register access Steve Longerbeam
` (8 subsequent siblings)
9 siblings, 1 reply; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-20 0:03 UTC (permalink / raw)
To: lars
Cc: linux-media, linux-kernel, Steve Longerbeam,
Mauro Carvalho Chehab, Javier Martinez Canillas,
Laurent Pinchart, Sakari Ailus
This patch adds a "newavmode" boolean property as part of the v4l2 endpoint
properties. This indicates an Analog Devices decoder is generating EAV/SAV
codes to suit Analog Devices encoders.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Cc: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
---
Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
drivers/media/v4l2-core/v4l2-of.c | 4 ++++
include/media/v4l2-mediabus.h | 5 +++++
3 files changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 9cd2a36..6f2df51 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -88,6 +88,8 @@ Optional endpoint properties
- field-even-active: field signal level during the even field data transmission.
- pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock
signal.
+- newavmode: a boolean property to indicate an Analog Devices decoder is
+ operating in NEWAVMODE. Valid for BT.656 busses only.
- sync-on-green-active: active state of Sync-on-green (SoG) signal, 0/1 for
LOW/HIGH respectively.
- data-lanes: an array of physical data lane indexes. Position of an entry
diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
index 93b3368..719a7d1 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -109,6 +109,10 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node,
flags |= v ? V4L2_MBUS_DATA_ACTIVE_HIGH :
V4L2_MBUS_DATA_ACTIVE_LOW;
+ if (endpoint->bus_type == V4L2_MBUS_BT656 &&
+ of_get_property(node, "newavmode", &v))
+ flags |= V4L2_MBUS_NEWAVMODE;
+
if (of_get_property(node, "slave-mode", &v))
flags |= V4L2_MBUS_SLAVE;
else
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 34cc99e..0bd5f0e 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -43,6 +43,11 @@
/* Active state of Sync-on-green (SoG) signal, 0/1 for LOW/HIGH respectively. */
#define V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH (1 << 12)
#define V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW (1 << 13)
+/*
+ * BT.656 specific flags
+ */
+/* Analog Device's NEWAVMODE */
+#define V4L2_MBUS_NEWAVMODE (1 << 14)
/* Serial flags */
/* How many lanes the client can use */
--
1.9.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 02/10] media: adv7180: Fix broken interrupt register access
2016-07-20 0:03 ` [PATCH v2 00/10] adv7180 subdev fixes, v2 Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 01/10] v4l: of: add "newavmode" property for Analog Devices codecs Steve Longerbeam
@ 2016-07-20 0:03 ` Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 03/10] media: adv7180: define more registers Steve Longerbeam
` (7 subsequent siblings)
9 siblings, 0 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-20 0:03 UTC (permalink / raw)
To: lars; +Cc: linux-media, linux-kernel, 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>
---
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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 03/10] media: adv7180: define more registers
2016-07-20 0:03 ` [PATCH v2 00/10] adv7180 subdev fixes, v2 Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 01/10] v4l: of: add "newavmode" property for Analog Devices codecs Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 02/10] media: adv7180: Fix broken interrupt register access Steve Longerbeam
@ 2016-07-20 0:03 ` Steve Longerbeam
2016-07-20 8:54 ` Lars-Peter Clausen
2016-07-20 0:03 ` [PATCH v2 04/10] media: adv7180: add support for NEWAVMODE Steve Longerbeam
` (6 subsequent siblings)
9 siblings, 1 reply; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-20 0:03 UTC (permalink / raw)
To: lars; +Cc: linux-media, linux-kernel, Steve Longerbeam
Replace hard-coded addresses with new register macro defines. No
functional changes.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 04/10] media: adv7180: add support for NEWAVMODE
2016-07-20 0:03 ` [PATCH v2 00/10] adv7180 subdev fixes, v2 Steve Longerbeam
` (2 preceding siblings ...)
2016-07-20 0:03 ` [PATCH v2 03/10] media: adv7180: define more registers Steve Longerbeam
@ 2016-07-20 0:03 ` Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 05/10] media: adv7180: add power pin control Steve Longerbeam
` (5 subsequent siblings)
9 siblings, 0 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-20 0:03 UTC (permalink / raw)
To: lars; +Cc: linux-media, linux-kernel, Steve Longerbeam
Parse the optional v4l2 endpoint DT node. If the V4L2_MBUS_NEWAVMODE
parallel bus flag is set, configure the BT.656 bus in NEWAVMODE.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/media/i2c/adv7180.c | 47 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 44 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index cb83ebb..667931e9 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;
@@ -740,6 +743,8 @@ static int adv7180_g_mbus_config(struct v4l2_subdev *sd,
*/
cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
V4L2_MBUS_DATA_ACTIVE_HIGH;
+ if (state->newavmode)
+ cfg->flags |= V4L2_MBUS_NEWAVMODE;
cfg->type = V4L2_MBUS_BT656;
}
@@ -864,9 +869,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 +1228,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 +1275,27 @@ 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);
+ of_node_put(endpoint);
+
+ if (ep.bus_type == V4L2_MBUS_BT656 &&
+ ep.bus.parallel.flags & V4L2_MBUS_NEWAVMODE)
+ state->newavmode = true;
+}
+
static int adv7180_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -1279,6 +1318,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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 05/10] media: adv7180: add power pin control
2016-07-20 0:03 ` [PATCH v2 00/10] adv7180 subdev fixes, v2 Steve Longerbeam
` (3 preceding siblings ...)
2016-07-20 0:03 ` [PATCH v2 04/10] media: adv7180: add support for NEWAVMODE Steve Longerbeam
@ 2016-07-20 0:03 ` Steve Longerbeam
2016-07-20 8:53 ` Lars-Peter Clausen
2016-07-20 0:03 ` [PATCH v2 06/10] media: adv7180: implement g_parm Steve Longerbeam
` (4 subsequent siblings)
9 siblings, 1 reply; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-20 0:03 UTC (permalink / raw)
To: lars; +Cc: linux-media, linux-kernel, 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>
Cc: Lars-Peter Clausen <lars@metafoo.de>
---
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 | 5 ++++
drivers/media/i2c/Kconfig | 2 +-
drivers/media/i2c/adv7180.c | 27 ++++++++++++++++++++++
3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/media/i2c/adv7180.txt b/Documentation/devicetree/bindings/media/i2c/adv7180.txt
index 0d50115..4da486f 100644
--- a/Documentation/devicetree/bindings/media/i2c/adv7180.txt
+++ b/Documentation/devicetree/bindings/media/i2c/adv7180.txt
@@ -15,6 +15,11 @@ Required Properties :
"adi,adv7282"
"adi,adv7282-m"
+Optional Properties :
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+ if any.
+
+
Example:
i2c0@1c22000 {
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 667931e9..8612d21 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;
@@ -1221,6 +1236,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);
@@ -1320,6 +1337,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);
@@ -1411,6 +1436,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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 06/10] media: adv7180: implement g_parm
2016-07-20 0:03 ` [PATCH v2 00/10] adv7180 subdev fixes, v2 Steve Longerbeam
` (4 preceding siblings ...)
2016-07-20 0:03 ` [PATCH v2 05/10] media: adv7180: add power pin control Steve Longerbeam
@ 2016-07-20 0:03 ` Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 07/10] media: adv7180: change mbus format to UYVY Steve Longerbeam
` (3 subsequent siblings)
9 siblings, 0 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-20 0:03 UTC (permalink / raw)
To: lars; +Cc: linux-media, linux-kernel, 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>
---
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 8612d21..8259549 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -766,6 +766,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);
@@ -824,6 +845,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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 07/10] media: adv7180: change mbus format to UYVY
2016-07-20 0:03 ` [PATCH v2 00/10] adv7180 subdev fixes, v2 Steve Longerbeam
` (5 preceding siblings ...)
2016-07-20 0:03 ` [PATCH v2 06/10] media: adv7180: implement g_parm Steve Longerbeam
@ 2016-07-20 0:03 ` Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 08/10] v4l: Add signal lock status to source change events Steve Longerbeam
` (2 subsequent siblings)
9 siblings, 0 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-20 0:03 UTC (permalink / raw)
To: lars; +Cc: linux-media, linux-kernel, 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>
---
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 8259549..0f0568c 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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 08/10] v4l: Add signal lock status to source change events
2016-07-20 0:03 ` [PATCH v2 00/10] adv7180 subdev fixes, v2 Steve Longerbeam
` (6 preceding siblings ...)
2016-07-20 0:03 ` [PATCH v2 07/10] media: adv7180: change mbus format to UYVY Steve Longerbeam
@ 2016-07-20 0:03 ` Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 09/10] media: adv7180: enable lock/unlock interrupts Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 10/10] media: adv7180: fix field type Steve Longerbeam
9 siblings, 0 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-20 0:03 UTC (permalink / raw)
To: lars; +Cc: linux-media, linux-kernel, Steve Longerbeam, Mauro Carvalho Chehab
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>
---
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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 09/10] media: adv7180: enable lock/unlock interrupts
2016-07-20 0:03 ` [PATCH v2 00/10] adv7180 subdev fixes, v2 Steve Longerbeam
` (7 preceding siblings ...)
2016-07-20 0:03 ` [PATCH v2 08/10] v4l: Add signal lock status to source change events Steve Longerbeam
@ 2016-07-20 0:03 ` Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 10/10] media: adv7180: fix field type Steve Longerbeam
9 siblings, 0 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-20 0:03 UTC (permalink / raw)
To: lars; +Cc: linux-media, linux-kernel, 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>
---
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 0f0568c..61f2140 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -876,19 +876,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);
@@ -1289,7 +1299,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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 10/10] media: adv7180: fix field type
2016-07-20 0:03 ` [PATCH v2 00/10] adv7180 subdev fixes, v2 Steve Longerbeam
` (8 preceding siblings ...)
2016-07-20 0:03 ` [PATCH v2 09/10] media: adv7180: enable lock/unlock interrupts Steve Longerbeam
@ 2016-07-20 0:03 ` Steve Longerbeam
9 siblings, 0 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-20 0:03 UTC (permalink / raw)
To: lars; +Cc: linux-media, linux-kernel, 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>
---
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 61f2140..58e1f53 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;
}
@@ -1366,8 +1373,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);
@@ -1397,7 +1410,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
^ permalink raw reply related [flat|nested] 55+ messages in thread
* Re: [PATCH v2 01/10] v4l: of: add "newavmode" property for Analog Devices codecs
2016-07-20 0:03 ` [PATCH v2 01/10] v4l: of: add "newavmode" property for Analog Devices codecs Steve Longerbeam
@ 2016-07-20 7:37 ` Hans Verkuil
2016-07-20 17:14 ` Steve Longerbeam
0 siblings, 1 reply; 55+ messages in thread
From: Hans Verkuil @ 2016-07-20 7:37 UTC (permalink / raw)
To: Steve Longerbeam, lars
Cc: linux-media, linux-kernel, Steve Longerbeam,
Mauro Carvalho Chehab, Javier Martinez Canillas,
Laurent Pinchart, Sakari Ailus
On 07/20/2016 02:03 AM, Steve Longerbeam wrote:
> This patch adds a "newavmode" boolean property as part of the v4l2 endpoint
> properties. This indicates an Analog Devices decoder is generating EAV/SAV
> codes to suit Analog Devices encoders.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
> drivers/media/v4l2-core/v4l2-of.c | 4 ++++
> include/media/v4l2-mediabus.h | 5 +++++
> 3 files changed, 11 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
> index 9cd2a36..6f2df51 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -88,6 +88,8 @@ Optional endpoint properties
> - field-even-active: field signal level during the even field data transmission.
> - pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock
> signal.
> +- newavmode: a boolean property to indicate an Analog Devices decoder is
> + operating in NEWAVMODE. Valid for BT.656 busses only.
This property is adv7180 specific and does not belong here.
Add this to Documentation/devicetree/bindings/media/i2c/adv7180.txt instead.
Nacked-by: Hans Verkuil <hans.verkuil@cisco.com>
Regards,
Hans
> - sync-on-green-active: active state of Sync-on-green (SoG) signal, 0/1 for
> LOW/HIGH respectively.
> - data-lanes: an array of physical data lane indexes. Position of an entry
> diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
> index 93b3368..719a7d1 100644
> --- a/drivers/media/v4l2-core/v4l2-of.c
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -109,6 +109,10 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node,
> flags |= v ? V4L2_MBUS_DATA_ACTIVE_HIGH :
> V4L2_MBUS_DATA_ACTIVE_LOW;
>
> + if (endpoint->bus_type == V4L2_MBUS_BT656 &&
> + of_get_property(node, "newavmode", &v))
> + flags |= V4L2_MBUS_NEWAVMODE;
> +
> if (of_get_property(node, "slave-mode", &v))
> flags |= V4L2_MBUS_SLAVE;
> else
> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> index 34cc99e..0bd5f0e 100644
> --- a/include/media/v4l2-mediabus.h
> +++ b/include/media/v4l2-mediabus.h
> @@ -43,6 +43,11 @@
> /* Active state of Sync-on-green (SoG) signal, 0/1 for LOW/HIGH respectively. */
> #define V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH (1 << 12)
> #define V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW (1 << 13)
> +/*
> + * BT.656 specific flags
> + */
> +/* Analog Device's NEWAVMODE */
> +#define V4L2_MBUS_NEWAVMODE (1 << 14)
>
> /* Serial flags */
> /* How many lanes the client can use */
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 05/10] media: adv7180: add power pin control
2016-07-20 0:03 ` [PATCH v2 05/10] media: adv7180: add power pin control Steve Longerbeam
@ 2016-07-20 8:53 ` Lars-Peter Clausen
0 siblings, 0 replies; 55+ messages in thread
From: Lars-Peter Clausen @ 2016-07-20 8:53 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media, linux-kernel, Steve Longerbeam
On 07/20/2016 02:03 AM, Steve Longerbeam wrote:
> 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>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
Looks good, thanks.
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 03/10] media: adv7180: define more registers
2016-07-20 0:03 ` [PATCH v2 03/10] media: adv7180: define more registers Steve Longerbeam
@ 2016-07-20 8:54 ` Lars-Peter Clausen
0 siblings, 0 replies; 55+ messages in thread
From: Lars-Peter Clausen @ 2016-07-20 8:54 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media, linux-kernel, Steve Longerbeam
On 07/20/2016 02:03 AM, Steve Longerbeam wrote:
> 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>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 01/10] v4l: of: add "newavmode" property for Analog Devices codecs
2016-07-20 7:37 ` Hans Verkuil
@ 2016-07-20 17:14 ` Steve Longerbeam
0 siblings, 0 replies; 55+ messages in thread
From: Steve Longerbeam @ 2016-07-20 17:14 UTC (permalink / raw)
To: Hans Verkuil, Steve Longerbeam, lars
Cc: linux-media, linux-kernel, Mauro Carvalho Chehab,
Javier Martinez Canillas, Laurent Pinchart, Sakari Ailus
On 07/20/2016 12:37 AM, Hans Verkuil wrote:
> On 07/20/2016 02:03 AM, Steve Longerbeam wrote:
>> This patch adds a "newavmode" boolean property as part of the v4l2 endpoint
>> properties. This indicates an Analog Devices decoder is generating EAV/SAV
>> codes to suit Analog Devices encoders.
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>> Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>> ---
>> Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
>> drivers/media/v4l2-core/v4l2-of.c | 4 ++++
>> include/media/v4l2-mediabus.h | 5 +++++
>> 3 files changed, 11 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
>> index 9cd2a36..6f2df51 100644
>> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
>> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
>> @@ -88,6 +88,8 @@ Optional endpoint properties
>> - field-even-active: field signal level during the even field data transmission.
>> - pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock
>> signal.
>> +- newavmode: a boolean property to indicate an Analog Devices decoder is
>> + operating in NEWAVMODE. Valid for BT.656 busses only.
> This property is adv7180 specific and does not belong here.
Hi Hans, yes that was my initial reaction to this idea as well, but
you didn't respond to my first request for comment.
>
> Add this to Documentation/devicetree/bindings/media/i2c/adv7180.txt instead.
Yes, I'll move this property into adv7180.
Steve
^ permalink raw reply [flat|nested] 55+ messages in thread
end of thread, other threads:[~2016-07-20 17:14 UTC | newest]
Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-06 22:59 [PATCH 00/11] adv7180 subdev fixes Steve Longerbeam
2016-07-06 22:59 ` [PATCH 01/11] media: adv7180: Fix broken interrupt register access Steve Longerbeam
2016-07-07 14:44 ` Tim Harvey
2016-07-07 15:37 ` Lars-Peter Clausen
2016-07-06 22:59 ` [PATCH 02/11] Revert "[media] adv7180: fix broken standards handling" Steve Longerbeam
2016-07-07 14:48 ` Tim Harvey
2016-07-07 15:45 ` Lars-Peter Clausen
2016-07-09 18:56 ` Steve Longerbeam
2016-07-06 22:59 ` [PATCH 03/11] media: adv7180: add power pin control Steve Longerbeam
2016-07-07 15:04 ` Tim Harvey
2016-07-07 15:35 ` Lars-Peter Clausen
2016-07-06 22:59 ` [PATCH 04/11] media: adv7180: implement g_parm Steve Longerbeam
2016-07-07 15:04 ` Tim Harvey
2016-07-06 22:59 ` [PATCH 05/11] media: adv7180: init chip with AD recommended register settings Steve Longerbeam
2016-07-07 15:23 ` Tim Harvey
2016-07-07 15:29 ` Lars-Peter Clausen
2016-07-06 22:59 ` [PATCH 06/11] media: adv7180: add bt.656-4 OF property Steve Longerbeam
2016-07-07 14:52 ` Lars-Peter Clausen
2016-07-09 18:59 ` Steve Longerbeam
2016-07-09 21:10 ` Steve Longerbeam
2016-07-09 21:36 ` Steve Longerbeam
2016-07-10 12:10 ` Lars-Peter Clausen
2016-07-10 12:55 ` Hans Verkuil
2016-07-10 14:17 ` Ian Arkver
2016-07-10 14:30 ` Hans Verkuil
2016-07-10 22:34 ` Steve Longerbeam
2016-07-11 7:06 ` Ian Arkver
2016-07-11 22:03 ` Steve Longerbeam
2016-07-12 10:25 ` Ian Arkver
2016-07-12 17:26 ` Steve Longerbeam
2016-07-06 23:00 ` [PATCH 07/11] media: adv7180: change mbus format to UYVY Steve Longerbeam
2016-07-07 15:18 ` Lars-Peter Clausen
2016-07-08 10:52 ` Niklas Söderlund
2016-07-07 15:25 ` Tim Harvey
2016-07-06 23:00 ` [PATCH 08/11] adv7180: send V4L2_EVENT_SOURCE_CHANGE on std change Steve Longerbeam
2016-07-07 15:27 ` Tim Harvey
2016-07-06 23:00 ` [PATCH 09/11] v4l: Add signal lock status to source change events Steve Longerbeam
2016-07-06 23:00 ` [PATCH 10/11] media: adv7180: enable lock/unlock interrupts Steve Longerbeam
2016-07-06 23:00 ` [PATCH 11/11] media: adv7180: fix field type Steve Longerbeam
2016-07-07 14:37 ` [PATCH 00/11] adv7180 subdev fixes Tim Harvey
2016-07-20 0:03 ` [PATCH v2 00/10] adv7180 subdev fixes, v2 Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 01/10] v4l: of: add "newavmode" property for Analog Devices codecs Steve Longerbeam
2016-07-20 7:37 ` Hans Verkuil
2016-07-20 17:14 ` Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 02/10] media: adv7180: Fix broken interrupt register access Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 03/10] media: adv7180: define more registers Steve Longerbeam
2016-07-20 8:54 ` Lars-Peter Clausen
2016-07-20 0:03 ` [PATCH v2 04/10] media: adv7180: add support for NEWAVMODE Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 05/10] media: adv7180: add power pin control Steve Longerbeam
2016-07-20 8:53 ` Lars-Peter Clausen
2016-07-20 0:03 ` [PATCH v2 06/10] media: adv7180: implement g_parm Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 07/10] media: adv7180: change mbus format to UYVY Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 08/10] v4l: Add signal lock status to source change events Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 09/10] media: adv7180: enable lock/unlock interrupts Steve Longerbeam
2016-07-20 0:03 ` [PATCH v2 10/10] media: adv7180: fix field type Steve Longerbeam
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.