[PATCH 0/6] adv7511/adv7604: logging improvements

[PATCH 0/6] adv7511/adv7604: logging improvements

[Hans Verkuil]

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

The first two patches fix type inconsistencies, but make no other changes.
The next two add support for logging InfoFrames to both drivers.
The last two patches fix a logging bug and log two more pieces of information
regarding the colorspace handling of the adv76xx.

Regards,

	Hans

Hans Verkuil (6):
  adv7511: replace uintX_t by uX for consistency
  adv7842: replace uintX_t by uX for consistency
  adv7511: log the currently set infoframes
  adv7604: log infoframes
  adv7604: fix broken saturator check
  adv7604: log alt-gamma and HDMI colorspace

 drivers/media/i2c/Kconfig                |   2 +
 drivers/media/i2c/adv7511.c              | 155 +++++++++++++++++++++++++++----
 drivers/media/i2c/adv7604.c              | 103 +++++++++++++-------
 drivers/media/i2c/adv7842.c              |  22 ++---
 drivers/media/pci/cobalt/cobalt-driver.c |   1 +
 include/media/adv7511.h                  |   7 +-
 include/media/adv7842.h                  |  50 +++++-----
 7 files changed, 251 insertions(+), 89 deletions(-)

-- 
2.1.4

[PATCH 1/6] adv7511: replace uintX_t by uX for consistency

[Hans Verkuil]

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

Currently this driver mixes u8/u16 and uint8_t/uint16_t. Standardize on
u8/u16.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7511.c | 32 ++++++++++++++++----------------
 include/media/adv7511.h     |  6 +++---
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c
index 9032567..d9bb90b 100644
--- a/drivers/media/i2c/adv7511.c
+++ b/drivers/media/i2c/adv7511.c
@@ -77,7 +77,7 @@ struct adv7511_state_edid {
 	u32 blocks;
 	/* Number of segments read */
 	u32 segments;
-	uint8_t data[EDID_MAX_SEGM * 256];
+	u8 data[EDID_MAX_SEGM * 256];
 	/* Number of EDID read retries left */
 	unsigned read_retries;
 	bool complete;
@@ -89,8 +89,8 @@ struct adv7511_state {
 	struct media_pad pad;
 	struct v4l2_ctrl_handler hdl;
 	int chip_revision;
-	uint8_t i2c_edid_addr;
-	uint8_t i2c_cec_addr;
+	u8 i2c_edid_addr;
+	u8 i2c_cec_addr;
 	/* Is the adv7511 powered on? */
 	bool power_on;
 	/* Did we receive hotplug and rx-sense signals? */
@@ -201,7 +201,7 @@ static int adv7511_wr(struct v4l2_subdev *sd, u8 reg, u8 val)
 
 /* To set specific bits in the register, a clear-mask is given (to be AND-ed),
    and then the value-mask (to be OR-ed). */
-static inline void adv7511_wr_and_or(struct v4l2_subdev *sd, u8 reg, uint8_t clr_mask, uint8_t val_mask)
+static inline void adv7511_wr_and_or(struct v4l2_subdev *sd, u8 reg, u8 clr_mask, u8 val_mask)
 {
 	adv7511_wr(sd, reg, (adv7511_rd(sd, reg) & clr_mask) | val_mask);
 }
@@ -223,7 +223,7 @@ static int adv_smbus_read_i2c_block_data(struct i2c_client *client,
 	return ret;
 }
 
-static inline void adv7511_edid_rd(struct v4l2_subdev *sd, uint16_t len, uint8_t *buf)
+static inline void adv7511_edid_rd(struct v4l2_subdev *sd, u16 len, u8 *buf)
 {
 	struct adv7511_state *state = get_adv7511_state(sd);
 	int i;
@@ -248,7 +248,7 @@ static inline bool adv7511_have_rx_sense(struct v4l2_subdev *sd)
 	return adv7511_rd(sd, 0x42) & MASK_ADV7511_MSEN_DETECT;
 }
 
-static void adv7511_csc_conversion_mode(struct v4l2_subdev *sd, uint8_t mode)
+static void adv7511_csc_conversion_mode(struct v4l2_subdev *sd, u8 mode)
 {
 	adv7511_wr_and_or(sd, 0x18, 0x9f, (mode & 0x3)<<5);
 }
@@ -292,7 +292,7 @@ static void adv7511_csc_coeff(struct v4l2_subdev *sd,
 static void adv7511_csc_rgb_full2limit(struct v4l2_subdev *sd, bool enable)
 {
 	if (enable) {
-		uint8_t csc_mode = 0;
+		u8 csc_mode = 0;
 		adv7511_csc_conversion_mode(sd, csc_mode);
 		adv7511_csc_coeff(sd,
 				  4096-564, 0, 0, 256,
@@ -546,8 +546,8 @@ static int adv7511_s_power(struct v4l2_subdev *sd, int on)
 /* Enable interrupts */
 static void adv7511_set_isr(struct v4l2_subdev *sd, bool enable)
 {
-	uint8_t irqs = MASK_ADV7511_HPD_INT | MASK_ADV7511_MSEN_INT;
-	uint8_t irqs_rd;
+	u8 irqs = MASK_ADV7511_HPD_INT | MASK_ADV7511_MSEN_INT;
+	u8 irqs_rd;
 	int retries = 100;
 
 	v4l2_dbg(2, debug, sd, "%s: %s\n", __func__, enable ? "enable" : "disable");
@@ -580,7 +580,7 @@ static void adv7511_set_isr(struct v4l2_subdev *sd, bool enable)
 /* Interrupt handler */
 static int adv7511_isr(struct v4l2_subdev *sd, u32 status, bool *handled)
 {
-	uint8_t irq_status;
+	u8 irq_status;
 
 	/* disable interrupts to prevent a race condition */
 	adv7511_set_isr(sd, false);
@@ -1033,7 +1033,7 @@ static const struct v4l2_subdev_ops adv7511_ops = {
 };
 
 /* ----------------------------------------------------------------------- */
-static void adv7511_dbg_dump_edid(int lvl, int debug, struct v4l2_subdev *sd, int segment, uint8_t *buf)
+static void adv7511_dbg_dump_edid(int lvl, int debug, struct v4l2_subdev *sd, int segment, u8 *buf)
 {
 	if (debug >= lvl) {
 		int i, j;
@@ -1145,7 +1145,7 @@ static void adv7511_check_monitor_present_status(struct v4l2_subdev *sd)
 {
 	struct adv7511_state *state = get_adv7511_state(sd);
 	/* read hotplug and rx-sense state */
-	uint8_t status = adv7511_rd(sd, 0x42);
+	u8 status = adv7511_rd(sd, 0x42);
 
 	v4l2_dbg(1, debug, sd, "%s: status: 0x%x%s%s\n",
 			 __func__,
@@ -1189,9 +1189,9 @@ static void adv7511_check_monitor_present_status(struct v4l2_subdev *sd)
 	}
 }
 
-static bool edid_block_verify_crc(uint8_t *edid_block)
+static bool edid_block_verify_crc(u8 *edid_block)
 {
-	uint8_t sum = 0;
+	u8 sum = 0;
 	int i;
 
 	for (i = 0; i < 128; i++)
@@ -1203,7 +1203,7 @@ static bool edid_verify_crc(struct v4l2_subdev *sd, u32 segment)
 {
 	struct adv7511_state *state = get_adv7511_state(sd);
 	u32 blocks = state->edid.blocks;
-	uint8_t *data = state->edid.data;
+	u8 *data = state->edid.data;
 
 	if (!edid_block_verify_crc(&data[segment * 256]))
 		return false;
@@ -1228,7 +1228,7 @@ static bool edid_verify_header(struct v4l2_subdev *sd, u32 segment)
 static bool adv7511_check_edid_status(struct v4l2_subdev *sd)
 {
 	struct adv7511_state *state = get_adv7511_state(sd);
-	uint8_t edidRdy = adv7511_rd(sd, 0xc5);
+	u8 edidRdy = adv7511_rd(sd, 0xc5);
 
 	v4l2_dbg(1, debug, sd, "%s: edid ready (retries: %d)\n",
 			 __func__, EDID_MAX_RETRIES - state->edid.read_retries);
diff --git a/include/media/adv7511.h b/include/media/adv7511.h
index bb78bed..f351eff 100644
--- a/include/media/adv7511.h
+++ b/include/media/adv7511.h
@@ -40,9 +40,9 @@ struct adv7511_cec_arg {
 };
 
 struct adv7511_platform_data {
-	uint8_t i2c_edid;
-	uint8_t i2c_cec;
-	uint32_t cec_clk;
+	u8 i2c_edid;
+	u8 i2c_cec;
+	u32 cec_clk;
 };
 
 #endif
-- 
2.1.4

[PATCH 2/6] adv7842: replace uintX_t by uX for consistency

[Hans Verkuil]

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

Currently this driver mixes u8/u32 and uint8_t/uint32_t. Standardize on
u8/u32.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7842.c | 22 ++++++++++----------
 include/media/adv7842.h     | 50 ++++++++++++++++++++++-----------------------
 2 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index f5248ba..f14ea78 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -1556,7 +1556,7 @@ static int adv7842_query_dv_timings(struct v4l2_subdev *sd,
 			V4L2_DV_BT_STD_GTF | V4L2_DV_BT_STD_CVT;
 
 	if (is_digital_input(sd)) {
-		uint32_t freq;
+		u32 freq;
 
 		timings->type = V4L2_DV_BT_656_1120;
 
@@ -2334,7 +2334,7 @@ struct adv7842_cfg_read_infoframe {
 static void log_infoframe(struct v4l2_subdev *sd, struct adv7842_cfg_read_infoframe *cri)
 {
 	int i;
-	uint8_t buffer[32];
+	u8 buffer[32];
 	union hdmi_infoframe frame;
 	u8 len;
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
@@ -2407,7 +2407,7 @@ static const char * const prim_mode_txt[] = {
 static int adv7842_sdp_log_status(struct v4l2_subdev *sd)
 {
 	/* SDP (Standard definition processor) block */
-	uint8_t sdp_signal_detected = sdp_read(sd, 0x5A) & 0x01;
+	u8 sdp_signal_detected = sdp_read(sd, 0x5A) & 0x01;
 
 	v4l2_info(sd, "Chip powered %s\n", no_power(sd) ? "off" : "on");
 	v4l2_info(sd, "Prim-mode = 0x%x, video std = 0x%x\n",
@@ -2451,10 +2451,10 @@ static int adv7842_cp_log_status(struct v4l2_subdev *sd)
 	/* CP block */
 	struct adv7842_state *state = to_state(sd);
 	struct v4l2_dv_timings timings;
-	uint8_t reg_io_0x02 = io_read(sd, 0x02);
-	uint8_t reg_io_0x21 = io_read(sd, 0x21);
-	uint8_t reg_rep_0x77 = rep_read(sd, 0x77);
-	uint8_t reg_rep_0x7d = rep_read(sd, 0x7d);
+	u8 reg_io_0x02 = io_read(sd, 0x02);
+	u8 reg_io_0x21 = io_read(sd, 0x21);
+	u8 reg_rep_0x77 = rep_read(sd, 0x77);
+	u8 reg_rep_0x7d = rep_read(sd, 0x7d);
 	bool audio_pll_locked = hdmi_read(sd, 0x04) & 0x01;
 	bool audio_sample_packet_detect = hdmi_read(sd, 0x18) & 0x01;
 	bool audio_mute = io_read(sd, 0x65) & 0x40;
@@ -2526,10 +2526,10 @@ static int adv7842_cp_log_status(struct v4l2_subdev *sd)
 	if (no_cp_signal(sd)) {
 		v4l2_info(sd, "STDI: not locked\n");
 	} else {
-		uint32_t bl = ((cp_read(sd, 0xb1) & 0x3f) << 8) | cp_read(sd, 0xb2);
-		uint32_t lcf = ((cp_read(sd, 0xb3) & 0x7) << 8) | cp_read(sd, 0xb4);
-		uint32_t lcvs = cp_read(sd, 0xb3) >> 3;
-		uint32_t fcl = ((cp_read(sd, 0xb8) & 0x1f) << 8) | cp_read(sd, 0xb9);
+		u32 bl = ((cp_read(sd, 0xb1) & 0x3f) << 8) | cp_read(sd, 0xb2);
+		u32 lcf = ((cp_read(sd, 0xb3) & 0x7) << 8) | cp_read(sd, 0xb4);
+		u32 lcvs = cp_read(sd, 0xb3) >> 3;
+		u32 fcl = ((cp_read(sd, 0xb8) & 0x1f) << 8) | cp_read(sd, 0xb9);
 		char hs_pol = ((cp_read(sd, 0xb5) & 0x10) ?
 				((cp_read(sd, 0xb5) & 0x08) ? '+' : '-') : 'x');
 		char vs_pol = ((cp_read(sd, 0xb5) & 0x40) ?
diff --git a/include/media/adv7842.h b/include/media/adv7842.h
index 1f38db8..bc24970 100644
--- a/include/media/adv7842.h
+++ b/include/media/adv7842.h
@@ -103,35 +103,35 @@ enum adv7842_drive_strength {
 
 struct adv7842_sdp_csc_coeff {
 	bool manual;
-	uint16_t scaling;
-	uint16_t A1;
-	uint16_t A2;
-	uint16_t A3;
-	uint16_t A4;
-	uint16_t B1;
-	uint16_t B2;
-	uint16_t B3;
-	uint16_t B4;
-	uint16_t C1;
-	uint16_t C2;
-	uint16_t C3;
-	uint16_t C4;
+	u16 scaling;
+	u16 A1;
+	u16 A2;
+	u16 A3;
+	u16 A4;
+	u16 B1;
+	u16 B2;
+	u16 B3;
+	u16 B4;
+	u16 C1;
+	u16 C2;
+	u16 C3;
+	u16 C4;
 };
 
 struct adv7842_sdp_io_sync_adjustment {
 	bool adjust;
-	uint16_t hs_beg;
-	uint16_t hs_width;
-	uint16_t de_beg;
-	uint16_t de_end;
-	uint8_t vs_beg_o;
-	uint8_t vs_beg_e;
-	uint8_t vs_end_o;
-	uint8_t vs_end_e;
-	uint8_t de_v_beg_o;
-	uint8_t de_v_beg_e;
-	uint8_t de_v_end_o;
-	uint8_t de_v_end_e;
+	u16 hs_beg;
+	u16 hs_width;
+	u16 de_beg;
+	u16 de_end;
+	u8 vs_beg_o;
+	u8 vs_beg_e;
+	u8 vs_end_o;
+	u8 vs_end_e;
+	u8 de_v_beg_o;
+	u8 de_v_beg_e;
+	u8 de_v_end_o;
+	u8 de_v_end_e;
 };
 
 /* Platform dependent definition */
-- 
2.1.4

[PATCH 3/6] adv7511: log the currently set infoframes

[Hans Verkuil]

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

The adv7511 sets up InfoFrames that are used when transmitting video.
Log the contents of those InfoFrames so it is possible to see exactly what
the transmitter is sending.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/Kconfig                |   1 +
 drivers/media/i2c/adv7511.c              | 123 ++++++++++++++++++++++++++++++-
 drivers/media/pci/cobalt/cobalt-driver.c |   1 +
 include/media/adv7511.h                  |   1 +
 4 files changed, 124 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 36f5563..c92180d 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -424,6 +424,7 @@ config VIDEO_ADV7393
 config VIDEO_ADV7511
 	tristate "Analog Devices ADV7511 encoder"
 	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+	select HDMI
 	---help---
 	  Support for the Analog Devices ADV7511 video encoder.
 
diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c
index d9bb90b..95bcd40 100644
--- a/drivers/media/i2c/adv7511.c
+++ b/drivers/media/i2c/adv7511.c
@@ -91,6 +91,7 @@ struct adv7511_state {
 	int chip_revision;
 	u8 i2c_edid_addr;
 	u8 i2c_cec_addr;
+	u8 i2c_pktmem_addr;
 	/* Is the adv7511 powered on? */
 	bool power_on;
 	/* Did we receive hotplug and rx-sense signals? */
@@ -109,6 +110,7 @@ struct adv7511_state {
 	struct v4l2_ctrl *have_edid0_ctrl;
 	struct v4l2_ctrl *rgb_quantization_range_ctrl;
 	struct i2c_client *i2c_edid;
+	struct i2c_client *i2c_pktmem;
 	struct adv7511_state_edid edid;
 	/* Running counter of the number of detected EDIDs (for debugging) */
 	unsigned edid_detect_counter;
@@ -238,6 +240,35 @@ static inline void adv7511_edid_rd(struct v4l2_subdev *sd, u16 len, u8 *buf)
 		v4l2_err(sd, "%s: i2c read error\n", __func__);
 }
 
+static int adv7511_pktmem_rd(struct v4l2_subdev *sd, u8 reg)
+{
+	struct adv7511_state *state = get_adv7511_state(sd);
+
+	return adv_smbus_read_byte_data(state->i2c_pktmem, reg);
+}
+
+static int adv7511_pktmem_wr(struct v4l2_subdev *sd, u8 reg, u8 val)
+{
+	struct adv7511_state *state = get_adv7511_state(sd);
+	int ret;
+	int i;
+
+	for (i = 0; i < 3; i++) {
+		ret = i2c_smbus_write_byte_data(state->i2c_pktmem, reg, val);
+		if (ret == 0)
+			return 0;
+	}
+	v4l2_err(sd, "%s: i2c write error\n", __func__);
+	return ret;
+}
+
+/* To set specific bits in the register, a clear-mask is given (to be AND-ed),
+   and then the value-mask (to be OR-ed). */
+static inline void adv7511_pktmem_wr_and_or(struct v4l2_subdev *sd, u8 reg, u8 clr_mask, u8 val_mask)
+{
+	adv7511_pktmem_wr(sd, reg, (adv7511_pktmem_rd(sd, reg) & clr_mask) | val_mask);
+}
+
 static inline bool adv7511_have_hotplug(struct v4l2_subdev *sd)
 {
 	return adv7511_rd(sd, 0x42) & MASK_ADV7511_HPD_DETECT;
@@ -415,6 +446,80 @@ static int adv7511_s_register(struct v4l2_subdev *sd, const struct v4l2_dbg_regi
 }
 #endif
 
+struct adv7511_cfg_read_infoframe {
+	const char *desc;
+	u8 present_reg;
+	u8 present_mask;
+	u8 header[3];
+	u16 payload_addr;
+};
+
+static u8 hdmi_infoframe_checksum(u8 *ptr, size_t size)
+{
+	u8 csum = 0;
+	size_t i;
+
+	/* compute checksum */
+	for (i = 0; i < size; i++)
+		csum += ptr[i];
+
+	return 256 - csum;
+}
+
+static void log_infoframe(struct v4l2_subdev *sd, const struct adv7511_cfg_read_infoframe *cri)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct device *dev = &client->dev;
+	union hdmi_infoframe frame;
+	u8 buffer[32];
+	u8 len;
+	int i;
+
+	if (!(adv7511_rd(sd, cri->present_reg) & cri->present_mask)) {
+		v4l2_info(sd, "%s infoframe not transmitted\n", cri->desc);
+		return;
+	}
+
+	memcpy(buffer, cri->header, sizeof(cri->header));
+
+	len = buffer[2];
+
+	if (len + 4 > sizeof(buffer)) {
+		v4l2_err(sd, "%s: invalid %s infoframe length %d\n", __func__, cri->desc, len);
+		return;
+	}
+
+	if (cri->payload_addr >= 0x100) {
+		for (i = 0; i < len; i++)
+			buffer[i + 4] = adv7511_pktmem_rd(sd, cri->payload_addr + i - 0x100);
+	} else {
+		for (i = 0; i < len; i++)
+			buffer[i + 4] = adv7511_rd(sd, cri->payload_addr + i);
+	}
+	buffer[3] = 0;
+	buffer[3] = hdmi_infoframe_checksum(buffer, len + 4);
+
+	if (hdmi_infoframe_unpack(&frame, buffer) < 0) {
+		v4l2_err(sd, "%s: unpack of %s infoframe failed\n", __func__, cri->desc);
+		return;
+	}
+
+	hdmi_infoframe_log(KERN_INFO, dev, &frame);
+}
+
+static void adv7511_log_infoframes(struct v4l2_subdev *sd)
+{
+	static const struct adv7511_cfg_read_infoframe cri[] = {
+		{ "AVI", 0x44, 0x10, { 0x82, 2, 13 }, 0x55 },
+		{ "Audio", 0x44, 0x08, { 0x84, 1, 10 }, 0x73 },
+		{ "SDP", 0x40, 0x40, { 0x83, 1, 25 }, 0x103 },
+	};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(cri); i++)
+		log_infoframe(sd, &cri[i]);
+}
+
 static int adv7511_log_status(struct v4l2_subdev *sd)
 {
 	struct adv7511_state *state = get_adv7511_state(sd);
@@ -480,6 +585,7 @@ static int adv7511_log_status(struct v4l2_subdev *sd)
 			  manual_cts ? "manual" : "automatic", N, CTS);
 		v4l2_info(sd, "VIC: detected %d, sent %d\n",
 			  vic_detect, vic_sent);
+		adv7511_log_infoframes(sd);
 	}
 	if (state->dv_timings.type == V4L2_DV_BT_656_1120)
 		v4l2_print_dv_timings(sd->name, "timings: ",
@@ -488,6 +594,7 @@ static int adv7511_log_status(struct v4l2_subdev *sd)
 		v4l2_info(sd, "no timings set\n");
 	v4l2_info(sd, "i2c edid addr: 0x%x\n", state->i2c_edid_addr);
 	v4l2_info(sd, "i2c cec addr: 0x%x\n", state->i2c_cec_addr);
+	v4l2_info(sd, "i2c pktmem addr: 0x%x\n", state->i2c_pktmem_addr);
 	return 0;
 }
 
@@ -537,6 +644,7 @@ static int adv7511_s_power(struct v4l2_subdev *sd, int on)
 	adv7511_wr(sd, 0xf9, 0x00);
 
 	adv7511_wr(sd, 0x43, state->i2c_edid_addr);
+	adv7511_wr(sd, 0x45, state->i2c_pktmem_addr);
 
 	/* Set number of attempts to read the EDID */
 	adv7511_wr(sd, 0xc9, 0xf);
@@ -1381,6 +1489,7 @@ static int adv7511_probe(struct i2c_client *client, const struct i2c_device_id *
 	/* EDID and CEC i2c addr */
 	state->i2c_edid_addr = state->pdata.i2c_edid << 1;
 	state->i2c_cec_addr = state->pdata.i2c_cec << 1;
+	state->i2c_pktmem_addr = state->pdata.i2c_pktmem << 1;
 
 	state->chip_revision = adv7511_rd(sd, 0x0);
 	chip_id[0] = adv7511_rd(sd, 0xf5);
@@ -1398,12 +1507,19 @@ static int adv7511_probe(struct i2c_client *client, const struct i2c_device_id *
 		goto err_entity;
 	}
 
+	state->i2c_pktmem = i2c_new_dummy(client->adapter, state->i2c_pktmem_addr >> 1);
+	if (state->i2c_pktmem == NULL) {
+		v4l2_err(sd, "failed to register pktmem i2c client\n");
+		err = -ENOMEM;
+		goto err_unreg_edid;
+	}
+
 	adv7511_wr(sd, 0xe2, 0x01); /* power down cec section */
 	state->work_queue = create_singlethread_workqueue(sd->name);
 	if (state->work_queue == NULL) {
 		v4l2_err(sd, "could not create workqueue\n");
 		err = -ENOMEM;
-		goto err_unreg_cec;
+		goto err_unreg_pktmem;
 	}
 
 	INIT_DELAYED_WORK(&state->edid_handler, adv7511_edid_handler);
@@ -1416,7 +1532,9 @@ static int adv7511_probe(struct i2c_client *client, const struct i2c_device_id *
 			  client->addr << 1, client->adapter->name);
 	return 0;
 
-err_unreg_cec:
+err_unreg_pktmem:
+	i2c_unregister_device(state->i2c_pktmem);
+err_unreg_edid:
 	i2c_unregister_device(state->i2c_edid);
 err_entity:
 	media_entity_cleanup(&sd->entity);
@@ -1440,6 +1558,7 @@ static int adv7511_remove(struct i2c_client *client)
 	adv7511_init_setup(sd);
 	cancel_delayed_work(&state->edid_handler);
 	i2c_unregister_device(state->i2c_edid);
+	i2c_unregister_device(state->i2c_pktmem);
 	destroy_workqueue(state->work_queue);
 	v4l2_device_unregister_subdev(sd);
 	media_entity_cleanup(&sd->entity);
diff --git a/drivers/media/pci/cobalt/cobalt-driver.c b/drivers/media/pci/cobalt/cobalt-driver.c
index c2974e6..b994b8e 100644
--- a/drivers/media/pci/cobalt/cobalt-driver.c
+++ b/drivers/media/pci/cobalt/cobalt-driver.c
@@ -602,6 +602,7 @@ static int cobalt_subdevs_hsma_init(struct cobalt *cobalt)
 	static struct adv7511_platform_data adv7511_pdata = {
 		.i2c_edid = 0x7e >> 1,
 		.i2c_cec = 0x7c >> 1,
+		.i2c_pktmem = 0x70 >> 1,
 		.cec_clk = 12000000,
 	};
 	static struct i2c_board_info adv7511_info = {
diff --git a/include/media/adv7511.h b/include/media/adv7511.h
index f351eff..d83b91d 100644
--- a/include/media/adv7511.h
+++ b/include/media/adv7511.h
@@ -42,6 +42,7 @@ struct adv7511_cec_arg {
 struct adv7511_platform_data {
 	u8 i2c_edid;
 	u8 i2c_cec;
+	u8 i2c_pktmem;
 	u32 cec_clk;
 };
 
-- 
2.1.4

[PATCH 4/6] adv7604: log infoframes

[Hans Verkuil]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/Kconfig   |  1 +
 drivers/media/i2c/adv7604.c | 87 ++++++++++++++++++++++++++++++---------------
 2 files changed, 59 insertions(+), 29 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index c92180d..71ee8f5 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -197,6 +197,7 @@ config VIDEO_ADV7183
 config VIDEO_ADV7604
 	tristate "Analog Devices ADV7604 decoder"
 	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && GPIOLIB
+	select HDMI
 	---help---
 	  Support for the Analog Devices ADV7604 video decoder.
 
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index aaa37b0..757b6b5 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -29,6 +29,7 @@
 
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
+#include <linux/hdmi.h>
 #include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -95,6 +96,13 @@ struct adv76xx_format_info {
 	u8 op_format_sel;
 };
 
+struct adv76xx_cfg_read_infoframe {
+	const char *desc;
+	u8 present_mask;
+	u8 head_addr;
+	u8 payload_addr;
+};
+
 struct adv76xx_chip_info {
 	enum adv76xx_type type;
 
@@ -2127,46 +2135,67 @@ static int adv76xx_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
 
 /*********** avi info frame CEA-861-E **************/
 
-static void print_avi_infoframe(struct v4l2_subdev *sd)
+static const struct adv76xx_cfg_read_infoframe adv76xx_cri[] = {
+	{ "AVI", 0x01, 0xe0, 0x00 },
+	{ "Audio", 0x02, 0xe3, 0x1c },
+	{ "SDP", 0x04, 0xe6, 0x2a },
+	{ "Vendor", 0x10, 0xec, 0x54 }
+};
+
+static int adv76xx_read_infoframe(struct v4l2_subdev *sd, int index,
+				  union hdmi_infoframe *frame)
 {
+	uint8_t buffer[32];
+	u8 len;
 	int i;
-	u8 buf[14];
-	u8 avi_len;
-	u8 avi_ver;
 
-	if (!is_hdmi(sd)) {
-		v4l2_info(sd, "receive DVI-D signal (AVI infoframe not supported)\n");
-		return;
+	if (!(io_read(sd, 0x60) & adv76xx_cri[index].present_mask)) {
+		v4l2_info(sd, "%s infoframe not received\n",
+			  adv76xx_cri[index].desc);
+		return -ENOENT;
 	}
-	if (!(io_read(sd, 0x60) & 0x01)) {
-		v4l2_info(sd, "AVI infoframe not received\n");
-		return;
+
+	for (i = 0; i < 3; i++)
+		buffer[i] = infoframe_read(sd,
+					   adv76xx_cri[index].head_addr + i);
+
+	len = buffer[2] + 1;
+
+	if (len + 3 > sizeof(buffer)) {
+		v4l2_err(sd, "%s: invalid %s infoframe length %d\n", __func__,
+			 adv76xx_cri[index].desc, len);
+		return -ENOENT;
 	}
 
-	if (io_read(sd, 0x83) & 0x01) {
-		v4l2_info(sd, "AVI infoframe checksum error has occurred earlier\n");
-		io_write(sd, 0x85, 0x01); /* clear AVI_INF_CKS_ERR_RAW */
-		if (io_read(sd, 0x83) & 0x01) {
-			v4l2_info(sd, "AVI infoframe checksum error still present\n");
-			io_write(sd, 0x85, 0x01); /* clear AVI_INF_CKS_ERR_RAW */
-		}
+	for (i = 0; i < len; i++)
+		buffer[i + 3] = infoframe_read(sd,
+				       adv76xx_cri[index].payload_addr + i);
+
+	if (hdmi_infoframe_unpack(frame, buffer) < 0) {
+		v4l2_err(sd, "%s: unpack of %s infoframe failed\n", __func__,
+			 adv76xx_cri[index].desc);
+		return -ENOENT;
 	}
+	return 0;
+}
 
-	avi_len = infoframe_read(sd, 0xe2);
-	avi_ver = infoframe_read(sd, 0xe1);
-	v4l2_info(sd, "AVI infoframe version %d (%d byte)\n",
-			avi_ver, avi_len);
+static void adv76xx_log_infoframes(struct v4l2_subdev *sd)
+{
+	int i;
 
-	if (avi_ver != 0x02)
+	if (!is_hdmi(sd)) {
+		v4l2_info(sd, "receive DVI-D signal, no infoframes\n");
 		return;
+	}
 
-	for (i = 0; i < 14; i++)
-		buf[i] = infoframe_read(sd, i);
+	for (i = 0; i < ARRAY_SIZE(adv76xx_cri); i++) {
+		union hdmi_infoframe frame;
+		struct i2c_client *client = v4l2_get_subdevdata(sd);
 
-	v4l2_info(sd,
-		"\t%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n",
-		buf[0], buf[1], buf[2], buf[3], buf[4], buf[5], buf[6], buf[7],
-		buf[8], buf[9], buf[10], buf[11], buf[12], buf[13]);
+		if (adv76xx_read_infoframe(sd, i, &frame))
+			return;
+		hdmi_infoframe_log(KERN_INFO, &client->dev, &frame);
+	}
 }
 
 static int adv76xx_log_status(struct v4l2_subdev *sd)
@@ -2302,7 +2331,7 @@ static int adv76xx_log_status(struct v4l2_subdev *sd)
 
 		v4l2_info(sd, "Deep color mode: %s\n", deep_color_mode_txt[(hdmi_read(sd, 0x0b) & 0x60) >> 5]);
 
-		print_avi_infoframe(sd);
+		adv76xx_log_infoframes(sd);
 	}
 
 	return 0;
-- 
2.1.4

[PATCH 5/6] adv7604: fix broken saturator check

[Hans Verkuil]

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

The logging of the saturator status was wrong due to an incorrect
condition.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7604.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 757b6b5..c04e0dd 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -2292,7 +2292,7 @@ static int adv76xx_log_status(struct v4l2_subdev *sd)
 	v4l2_info(sd, "Output color space: %s %s, saturator %s\n",
 			(reg_io_0x02 & 0x02) ? "RGB" : "YCbCr",
 			(reg_io_0x02 & 0x04) ? "(16-235)" : "(0-255)",
-			((reg_io_0x02 & 0x04) ^ (reg_io_0x02 & 0x01)) ?
+			(((reg_io_0x02 >> 2) & 0x01) ^ (reg_io_0x02 & 0x01)) ?
 				"enabled" : "disabled");
 	v4l2_info(sd, "Color space conversion: %s\n",
 			csc_coeff_sel_rb[cp_read(sd, info->cp_csc) >> 4]);
-- 
2.1.4

[PATCH 6/6] adv7604: log alt-gamma and HDMI colorspace

[Hans Verkuil]

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

Log the alternate gamma state and the HDMI colorspace that the adv
device detected.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/adv7604.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index c04e0dd..daf9386 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -2222,6 +2222,14 @@ static int adv76xx_log_status(struct v4l2_subdev *sd)
 		"invalid", "invalid", "invalid", "invalid", "invalid",
 		"invalid", "invalid", "automatic"
 	};
+	static const char * const hdmi_color_space_txt[16] = {
+		"RGB limited range (16-235)", "RGB full range (0-255)",
+		"YCbCr Bt.601 (16-235)", "YCbCr Bt.709 (16-235)",
+		"xvYCC Bt.601", "xvYCC Bt.709",
+		"YCbCr Bt.601 (0-255)", "YCbCr Bt.709 (0-255)",
+		"sYCC", "Adobe YCC 601", "AdobeRGB", "invalid", "invalid",
+		"invalid", "invalid", "invalid"
+	};
 	static const char * const rgb_quantization_range_txt[] = {
 		"Automatic",
 		"RGB limited range (16-235)",
@@ -2289,11 +2297,12 @@ static int adv76xx_log_status(struct v4l2_subdev *sd)
 			rgb_quantization_range_txt[state->rgb_quantization_range]);
 	v4l2_info(sd, "Input color space: %s\n",
 			input_color_space_txt[reg_io_0x02 >> 4]);
-	v4l2_info(sd, "Output color space: %s %s, saturator %s\n",
+	v4l2_info(sd, "Output color space: %s %s, saturator %s, alt-gamma %s\n",
 			(reg_io_0x02 & 0x02) ? "RGB" : "YCbCr",
 			(reg_io_0x02 & 0x04) ? "(16-235)" : "(0-255)",
 			(((reg_io_0x02 >> 2) & 0x01) ^ (reg_io_0x02 & 0x01)) ?
-				"enabled" : "disabled");
+				"enabled" : "disabled",
+			(reg_io_0x02 & 0x08) ? "enabled" : "disabled");
 	v4l2_info(sd, "Color space conversion: %s\n",
 			csc_coeff_sel_rb[cp_read(sd, info->cp_csc) >> 4]);
 
@@ -2330,6 +2339,7 @@ static int adv76xx_log_status(struct v4l2_subdev *sd)
 		v4l2_info(sd, "AV Mute: %s\n", (hdmi_read(sd, 0x04) & 0x40) ? "on" : "off");
 
 		v4l2_info(sd, "Deep color mode: %s\n", deep_color_mode_txt[(hdmi_read(sd, 0x0b) & 0x60) >> 5]);
+		v4l2_info(sd, "HDMI colorspace: %s\n", hdmi_color_space_txt[hdmi_read(sd, 0x53) & 0xf]);
 
 		adv76xx_log_infoframes(sd);
 	}
-- 
2.1.4

Re: [PATCH 4/6] adv7604: log infoframes

[Hans Verkuil]

On 06/07/2015 12:32 PM, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 

Hmm, missing commit log. I'm sure I wrote it at some point in time...

This should be:

Add support for logging the detected InfoFrames for the adv76xx. Helps in
debugging what is actually received on the HDMI link.

Regards,

	Hans

> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/i2c/Kconfig   |  1 +
>  drivers/media/i2c/adv7604.c | 87 ++++++++++++++++++++++++++++++---------------
>  2 files changed, 59 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index c92180d..71ee8f5 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -197,6 +197,7 @@ config VIDEO_ADV7183
>  config VIDEO_ADV7604
>  	tristate "Analog Devices ADV7604 decoder"
>  	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && GPIOLIB
> +	select HDMI
>  	---help---
>  	  Support for the Analog Devices ADV7604 video decoder.
>  
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index aaa37b0..757b6b5 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -29,6 +29,7 @@
>  
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/hdmi.h>
>  #include <linux/i2c.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> @@ -95,6 +96,13 @@ struct adv76xx_format_info {
>  	u8 op_format_sel;
>  };
>  
> +struct adv76xx_cfg_read_infoframe {
> +	const char *desc;
> +	u8 present_mask;
> +	u8 head_addr;
> +	u8 payload_addr;
> +};
> +
>  struct adv76xx_chip_info {
>  	enum adv76xx_type type;
>  
> @@ -2127,46 +2135,67 @@ static int adv76xx_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
>  
>  /*********** avi info frame CEA-861-E **************/
>  
> -static void print_avi_infoframe(struct v4l2_subdev *sd)
> +static const struct adv76xx_cfg_read_infoframe adv76xx_cri[] = {
> +	{ "AVI", 0x01, 0xe0, 0x00 },
> +	{ "Audio", 0x02, 0xe3, 0x1c },
> +	{ "SDP", 0x04, 0xe6, 0x2a },
> +	{ "Vendor", 0x10, 0xec, 0x54 }
> +};
> +
> +static int adv76xx_read_infoframe(struct v4l2_subdev *sd, int index,
> +				  union hdmi_infoframe *frame)
>  {
> +	uint8_t buffer[32];
> +	u8 len;
>  	int i;
> -	u8 buf[14];
> -	u8 avi_len;
> -	u8 avi_ver;
>  
> -	if (!is_hdmi(sd)) {
> -		v4l2_info(sd, "receive DVI-D signal (AVI infoframe not supported)\n");
> -		return;
> +	if (!(io_read(sd, 0x60) & adv76xx_cri[index].present_mask)) {
> +		v4l2_info(sd, "%s infoframe not received\n",
> +			  adv76xx_cri[index].desc);
> +		return -ENOENT;
>  	}
> -	if (!(io_read(sd, 0x60) & 0x01)) {
> -		v4l2_info(sd, "AVI infoframe not received\n");
> -		return;
> +
> +	for (i = 0; i < 3; i++)
> +		buffer[i] = infoframe_read(sd,
> +					   adv76xx_cri[index].head_addr + i);
> +
> +	len = buffer[2] + 1;
> +
> +	if (len + 3 > sizeof(buffer)) {
> +		v4l2_err(sd, "%s: invalid %s infoframe length %d\n", __func__,
> +			 adv76xx_cri[index].desc, len);
> +		return -ENOENT;
>  	}
>  
> -	if (io_read(sd, 0x83) & 0x01) {
> -		v4l2_info(sd, "AVI infoframe checksum error has occurred earlier\n");
> -		io_write(sd, 0x85, 0x01); /* clear AVI_INF_CKS_ERR_RAW */
> -		if (io_read(sd, 0x83) & 0x01) {
> -			v4l2_info(sd, "AVI infoframe checksum error still present\n");
> -			io_write(sd, 0x85, 0x01); /* clear AVI_INF_CKS_ERR_RAW */
> -		}
> +	for (i = 0; i < len; i++)
> +		buffer[i + 3] = infoframe_read(sd,
> +				       adv76xx_cri[index].payload_addr + i);
> +
> +	if (hdmi_infoframe_unpack(frame, buffer) < 0) {
> +		v4l2_err(sd, "%s: unpack of %s infoframe failed\n", __func__,
> +			 adv76xx_cri[index].desc);
> +		return -ENOENT;
>  	}
> +	return 0;
> +}
>  
> -	avi_len = infoframe_read(sd, 0xe2);
> -	avi_ver = infoframe_read(sd, 0xe1);
> -	v4l2_info(sd, "AVI infoframe version %d (%d byte)\n",
> -			avi_ver, avi_len);
> +static void adv76xx_log_infoframes(struct v4l2_subdev *sd)
> +{
> +	int i;
>  
> -	if (avi_ver != 0x02)
> +	if (!is_hdmi(sd)) {
> +		v4l2_info(sd, "receive DVI-D signal, no infoframes\n");
>  		return;
> +	}
>  
> -	for (i = 0; i < 14; i++)
> -		buf[i] = infoframe_read(sd, i);
> +	for (i = 0; i < ARRAY_SIZE(adv76xx_cri); i++) {
> +		union hdmi_infoframe frame;
> +		struct i2c_client *client = v4l2_get_subdevdata(sd);
>  
> -	v4l2_info(sd,
> -		"\t%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n",
> -		buf[0], buf[1], buf[2], buf[3], buf[4], buf[5], buf[6], buf[7],
> -		buf[8], buf[9], buf[10], buf[11], buf[12], buf[13]);
> +		if (adv76xx_read_infoframe(sd, i, &frame))
> +			return;
> +		hdmi_infoframe_log(KERN_INFO, &client->dev, &frame);
> +	}
>  }
>  
>  static int adv76xx_log_status(struct v4l2_subdev *sd)
> @@ -2302,7 +2331,7 @@ static int adv76xx_log_status(struct v4l2_subdev *sd)
>  
>  		v4l2_info(sd, "Deep color mode: %s\n", deep_color_mode_txt[(hdmi_read(sd, 0x0b) & 0x60) >> 5]);
>  
> -		print_avi_infoframe(sd);
> +		adv76xx_log_infoframes(sd);
>  	}
>  
>  	return 0;
>