HDMI 4k support v2

HDMI 4k support v2

[Damien Lespiau]

Following up the first instance of this series:
  http://lists.freedesktop.org/archives/dri-devel/2013-August/043125.html

Here is a v2 with Ville's review pass and a few additions:
  - Alternate clock modes for 4k resolutions
  - HDMI vendor specific infoframe support in drivers/video/hdmi.c
  - Enabling of those vendor specific infoframes in the i915 driver

Along the way, a tegra patch was needed for a small consolidation of the code
packing vendor specific infoframes. This patch has only been compile-tested.

On Intel, it's possible to read back the programmed infoframe buffers with
intel-gpu-tools' intel_infoframe and this gives:

Vendor InfoFrame:
- frequency: every vsync
- raw:
  f4050181 000c0347 00000320 00000000
  00000000 00000000 00000000 00000000
  0000004c 00000000 00000000 00000000
  00000000 00000000 00000000 00000000
- vendor Id: 0x000c03 (HDMI)
- video format: 0x001 
- HDMI VIC: 3

after a $ xrandr --output HDMI1 --mode 3840x2160 -r 24

-- 
Damien

[PATCH 01/12] drm: Don't export drm_find_cea_extension() any more

[Damien Lespiau]

This function is only used inside drm_edid.c.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 5 ++---
 include/drm/drm_crtc.h     | 1 -
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index dfc7a1b..e014785 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2298,10 +2298,10 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
 #define EDID_CEA_YCRCB422	(1 << 4)
 #define EDID_CEA_VCDB_QS	(1 << 6)
 
-/**
+/*
  * Search EDID for CEA extension block.
  */
-u8 *drm_find_cea_extension(struct edid *edid)
+static u8 *drm_find_cea_extension(struct edid *edid)
 {
 	u8 *edid_ext = NULL;
 	int i;
@@ -2322,7 +2322,6 @@ u8 *drm_find_cea_extension(struct edid *edid)
 
 	return edid_ext;
 }
-EXPORT_SYMBOL(drm_find_cea_extension);
 
 /*
  * Calculate the alternate clock for the CEA mode
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index fa12a2f..f3ecc6f 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1050,7 +1050,6 @@ extern int drm_mode_gamma_get_ioctl(struct drm_device *dev,
 				    void *data, struct drm_file *file_priv);
 extern int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 				    void *data, struct drm_file *file_priv);
-extern u8 *drm_find_cea_extension(struct edid *edid);
 extern u8 drm_match_cea_mode(const struct drm_display_mode *to_match);
 extern bool drm_detect_hdmi_monitor(struct edid *edid);
 extern bool drm_detect_monitor_audio(struct edid *edid);
-- 
1.8.3.1

[PATCH 02/12] drm/edid: Fix add_cea_modes() style issues

[Damien Lespiau]

A few styles issues have crept in here, fix them before touching this
code again.

v2: constify arguments that can be (Ville Syrjälä)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index e014785..9e9b6ed 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2301,7 +2301,7 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
 /*
  * Search EDID for CEA extension block.
  */
-static u8 *drm_find_cea_extension(struct edid *edid)
+static u8 *drm_find_cea_extension(const struct edid *edid)
 {
 	u8 *edid_ext = NULL;
 	int i;
@@ -2441,10 +2441,11 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
 }
 
 static int
-do_cea_modes (struct drm_connector *connector, u8 *db, u8 len)
+do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
 {
 	struct drm_device *dev = connector->dev;
-	u8 * mode, cea_mode;
+	const u8 *mode;
+	u8 cea_mode;
 	int modes = 0;
 
 	for (mode = db; mode < db + len; mode++) {
@@ -2499,10 +2500,10 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
 	for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1)
 
 static int
-add_cea_modes(struct drm_connector *connector, struct edid *edid)
+add_cea_modes(struct drm_connector *connector, const struct edid *edid)
 {
-	u8 * cea = drm_find_cea_extension(edid);
-	u8 * db, dbl;
+	u8 *cea = drm_find_cea_extension(edid);
+	u8 *db, dbl;
 	int modes = 0;
 
 	if (cea && cea_revision(cea) >= 3) {
@@ -2516,7 +2517,7 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid)
 			dbl = cea_db_payload_len(db);
 
 			if (cea_db_tag(db) == VIDEO_BLOCK)
-				modes += do_cea_modes (connector, db+1, dbl);
+				modes += do_cea_modes(connector, db + 1, dbl);
 		}
 	}
 
-- 
1.8.3.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/12] drm/edid: Parse the HDMI CEA block and look for 4k modes

[Damien Lespiau]

HDMI 1.4 adds 4 "4k x 2k" modes in the the CEA vendor specific block.

With this commit, we now parse this block and expose the 4k modes that
we find there.

v2: Fix the "4096x2160" string (nice catch!), add comments about
    do_hdmi_vsdb_modes() arguments and make it clearer that offset is
    relative to the end of the required fields of the HDMI VSDB
    (Ville Syrjälä)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Tested-by: Cancan Feng <cancan.feng@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67030
---
 drivers/gpu/drm/drm_edid.c | 124 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 109 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9e9b6ed..0faa08e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -931,6 +931,36 @@ static const struct drm_display_mode edid_cea_modes[] = {
 	 .vrefresh = 100, },
 };
 
+/*
+ * HDMI 1.4 4k modes.
+ */
+static const struct drm_display_mode edid_4k_modes[] = {
+	/* 1 - 3840x2160@30Hz */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
+		   3840, 4016, 4104, 4400, 0,
+		   2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 30, },
+	/* 2 - 3840x2160@25Hz */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
+		   3840, 4896, 4984, 5280, 0,
+		   2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 25, },
+	/* 3 - 3840x2160@24Hz */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
+		   3840, 5116, 5204, 5500, 0,
+		   2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 24, },
+	/* 4 - 4096x2160@24Hz (SMPTE) */
+	{ DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000,
+		   4096, 5116, 5204, 5500, 0,
+		   2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 24, },
+};
+
 /*** DDC fetch and block validation ***/
 
 static const u8 edid_header[] = {
@@ -2465,6 +2495,68 @@ do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
 	return modes;
 }
 
+/*
+ * do_hdmi_vsdb_modes - Parse the HDMI Vendor Specific data block
+ * @connector: connector corresponding to the HDMI sink
+ * @db: start of the CEA vendor specific block
+ * @len: length of the CEA block payload, ie. one can access up to db[len]
+ *
+ * Parses the HDMI VSDB looking for modes to add to @connector.
+ */
+static int
+do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len)
+{
+	struct drm_device *dev = connector->dev;
+	int modes = 0, offset = 0, i;
+	u8 vic_len;
+
+	if (len < 8)
+		goto out;
+
+	/* no HDMI_Video_Present */
+	if (!(db[8] & (1 << 5)))
+		goto out;
+
+	/* Latency_Fields_Present */
+	if (db[8] & (1 << 7))
+		offset += 2;
+
+	/* I_Latency_Fields_Present */
+	if (db[8] & (1 << 6))
+		offset += 2;
+
+	/* the declared length is not long enough for the 2 first bytes
+	 * of additional video format capabilities */
+	offset += 2;
+	if (len < (8 + offset))
+		goto out;
+
+	vic_len = db[8 + offset] >> 5;
+
+	for (i = 0; i < vic_len && len >= (9 + offset + i); i++) {
+		struct drm_display_mode *newmode;
+		u8 vic;
+
+		vic = db[9 + offset + i];
+
+		vic--; /* VICs start at 1 */
+		if (vic >= ARRAY_SIZE(edid_4k_modes)) {
+			DRM_ERROR("Unknow HDMI VIC: %d\n", vic);
+			continue;
+		}
+
+		newmode = drm_mode_duplicate(dev, &edid_4k_modes[vic]);
+		if (!newmode)
+			continue;
+
+		drm_mode_probed_add(connector, newmode);
+		modes++;
+	}
+
+out:
+	return modes;
+}
+
 static int
 cea_db_payload_len(const u8 *db)
 {
@@ -2496,6 +2588,21 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
 	return 0;
 }
 
+static bool cea_db_is_hdmi_vsdb(const u8 *db)
+{
+	int hdmi_id;
+
+	if (cea_db_tag(db) != VENDOR_BLOCK)
+		return false;
+
+	if (cea_db_payload_len(db) < 5)
+		return false;
+
+	hdmi_id = db[1] | (db[2] << 8) | (db[3] << 16);
+
+	return hdmi_id == HDMI_IDENTIFIER;
+}
+
 #define for_each_cea_db(cea, i, start, end) \
 	for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1)
 
@@ -2518,6 +2625,8 @@ add_cea_modes(struct drm_connector *connector, const struct edid *edid)
 
 			if (cea_db_tag(db) == VIDEO_BLOCK)
 				modes += do_cea_modes(connector, db + 1, dbl);
+			else if (cea_db_is_hdmi_vsdb(db))
+				modes += do_hdmi_vsdb_modes(connector, db, dbl);
 		}
 	}
 
@@ -2570,21 +2679,6 @@ monitor_name(struct detailed_timing *t, void *data)
 		*(u8 **)data = t->data.other_data.data.str.str;
 }
 
-static bool cea_db_is_hdmi_vsdb(const u8 *db)
-{
-	int hdmi_id;
-
-	if (cea_db_tag(db) != VENDOR_BLOCK)
-		return false;
-
-	if (cea_db_payload_len(db) < 5)
-		return false;
-
-	hdmi_id = db[1] | (db[2] << 8) | (db[3] << 16);
-
-	return hdmi_id == HDMI_IDENTIFIER;
-}
-
 /**
  * drm_edid_to_eld - build ELD from EDID
  * @connector: connector corresponding to the HDMI/DP sink
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 04/12] drm: Add support for alternate clocks of 4k modes

[Damien Lespiau]

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 68 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 62 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0faa08e..606335f 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2409,6 +2409,54 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
 }
 EXPORT_SYMBOL(drm_match_cea_mode);
 
+/*
+ * Calculate the alternate clock for HDMI modes (those from the HDMI vendor
+ * specific block).
+ *
+ * It's almost like cea_mode_alternate_clock(), we just need to add an
+ * exception for the VIC 4 mode (4096x2160@24Hz): no alternate clock for this
+ * one.
+ */
+static unsigned int
+hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode)
+{
+	if (hdmi_mode->vdisplay == 4096 && hdmi_mode->hdisplay == 2160)
+		return hdmi_mode->clock;
+
+	return cea_mode_alternate_clock(hdmi_mode);
+}
+
+/*
+ * drm_match_cea_mode - look for a HDMI mode matching given mode
+ * @to_match: display mode
+ *
+ * An HDMI mode is one defined in the HDMI vendor specific block.
+ *
+ * Returns the HDMI Video ID (VIC) of the mode or 0 if it isn't one.
+ */
+static u8 drm_match_hmdi_mode(const struct drm_display_mode *to_match)
+{
+	u8 mode;
+
+	if (!to_match->clock)
+		return 0;
+
+	for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) {
+		const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode];
+		unsigned int clock1, clock2;
+
+		/* Make sure to also match alternate clocks */
+		clock1 = hdmi_mode->clock;
+		clock2 = hdmi_mode_alternate_clock(hdmi_mode);
+
+		if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) ||
+		     KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) &&
+		    drm_mode_equal_no_clocks(to_match, hdmi_mode))
+			return mode + 1;
+	}
+	return 0;
+}
+
 static int
 add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
 {
@@ -2426,18 +2474,26 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
 	 * with the alternate clock for certain CEA modes.
 	 */
 	list_for_each_entry(mode, &connector->probed_modes, head) {
-		const struct drm_display_mode *cea_mode;
+		const struct drm_display_mode *cea_mode = NULL;
 		struct drm_display_mode *newmode;
-		u8 cea_mode_idx = drm_match_cea_mode(mode) - 1;
+		u8 mode_idx = drm_match_cea_mode(mode) - 1;
 		unsigned int clock1, clock2;
 
-		if (cea_mode_idx >= ARRAY_SIZE(edid_cea_modes))
-			continue;
+		if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
+			cea_mode = &edid_cea_modes[mode_idx];
+			clock2 = cea_mode_alternate_clock(cea_mode);
+		} else {
+			mode_idx = drm_match_hmdi_mode(mode) - 1;
+			if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
+				cea_mode = &edid_4k_modes[mode_idx];
+				clock2 = hdmi_mode_alternate_clock(cea_mode);
+			}
+		}
 
-		cea_mode = &edid_cea_modes[cea_mode_idx];
+		if (!cea_mode)
+			continue;
 
 		clock1 = cea_mode->clock;
-		clock2 = cea_mode_alternate_clock(cea_mode);
 
 		if (clock1 == clock2)
 			continue;
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 05/12] video/hdmi: Don't let the user of this API create invalid infoframes

[Damien Lespiau]

To set the active aspect ratio value in the AVI infoframe today, you not
only have to set the active_aspect field, but also the active_info_valid
bit. Out of the 1 user of this API, we had 100% misuse, forgetting the
_valid bit. This was fixed in:

  Author: Damien Lespiau <damien.lespiau@intel.com>
  Date:   Tue Aug 6 20:32:17 2013 +0100

      drm: Don't generate invalid AVI infoframes for CEA modes

We can do better and derive the _valid bit from the user wanting to set
the active aspect ratio.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 1 -
 drivers/video/hdmi.c       | 4 +++-
 include/linux/hdmi.h       | 1 -
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 606335f..ec47eb8 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3258,7 +3258,6 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 	frame->video_code = drm_match_cea_mode(mode);
 
 	frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
-	frame->active_info_valid = 1;
 	frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
 
 	return 0;
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 635d569..e36da36 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -96,7 +96,9 @@ ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe *frame, void *buffer,
 
 	ptr[0] = ((frame->colorspace & 0x3) << 5) | (frame->scan_mode & 0x3);
 
-	if (frame->active_info_valid)
+	/* Data byte 1, bit 4 has to be set if we provide the active format
+	 * aspect ratio */
+	if (frame->active_aspect & 0xf)
 		ptr[0] |= BIT(4);
 
 	if (frame->horizontal_bar_valid)
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index bc6743e..931474c6 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -109,7 +109,6 @@ struct hdmi_avi_infoframe {
 	unsigned char version;
 	unsigned char length;
 	enum hdmi_colorspace colorspace;
-	bool active_info_valid;
 	bool horizontal_bar_valid;
 	bool vertical_bar_valid;
 	enum hdmi_scan_mode scan_mode;
-- 
1.8.3.1

[PATCH 06/12] video/hdmi: Derive the bar data valid bit from the bar data fields

[Damien Lespiau]

Just like:

  Author: Damien Lespiau <damien.lespiau@intel.com>
  Date:   Mon Aug 12 11:53:24 2013 +0100

      video/hdmi: Don't let the user of this API create invalid infoframes

But this time for the horizontal/vertical bar data present bits.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/video/hdmi.c | 5 +++--
 include/linux/hdmi.h | 2 --
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index e36da36..ac84215 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -101,10 +101,11 @@ ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe *frame, void *buffer,
 	if (frame->active_aspect & 0xf)
 		ptr[0] |= BIT(4);
 
-	if (frame->horizontal_bar_valid)
+	/* Bit 3 and 2 indicate if we transmit horizontal/vertical bar data */
+	if (frame->top_bar || frame->bottom_bar)
 		ptr[0] |= BIT(3);
 
-	if (frame->vertical_bar_valid)
+	if (frame->left_bar || frame->right_bar)
 		ptr[0] |= BIT(2);
 
 	ptr[1] = ((frame->colorimetry & 0x3) << 6) |
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 931474c6..b98340b 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -109,8 +109,6 @@ struct hdmi_avi_infoframe {
 	unsigned char version;
 	unsigned char length;
 	enum hdmi_colorspace colorspace;
-	bool horizontal_bar_valid;
-	bool vertical_bar_valid;
 	enum hdmi_scan_mode scan_mode;
 	enum hdmi_colorimetry colorimetry;
 	enum hdmi_picture_aspect picture_aspect;
-- 
1.8.3.1

[PATCH 07/12] video/hdmi: Introduce helpers for the HDMI vendor specific infoframe

[Damien Lespiau]

Provide the programming model than the other infoframe types.

The generic _pack() function can't handle those yet as we need to move
the vendor OUI in the generic hdmi_vendor_infoframe structure to know
which kind of vendor infoframe we are dealing with.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/video/hdmi.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/hdmi.h | 25 ++++++++++++++++
 2 files changed, 107 insertions(+)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index ac84215..2059f7b 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -286,6 +286,88 @@ ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
 EXPORT_SYMBOL(hdmi_audio_infoframe_pack);
 
 /**
+ * hdmi_hdmi_infoframe_init() - initialize an HDMI vendor infoframe
+ * @frame: HDMI vendor infoframe
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame)
+{
+	memset(frame, 0, sizeof(*frame));
+
+	frame->type = HDMI_INFOFRAME_TYPE_VENDOR;
+	frame->version = 1;
+	frame->length = 5; /* we can hardcode the size for now as we don't
+			    support neither 3D_Ext_Data nor 3D_Metadata_* */
+
+	/* 0 is a valid value for s3d_struct, so we use a special "not set"
+	 * value */
+	frame->s3d_struct = HDMI_3D_STRUCTURE_INVALID;
+
+	return 0;
+}
+EXPORT_SYMBOL(hdmi_hdmi_infoframe_init);
+
+/**
+ * hdmi_hmdi_infoframe_pack() - write a HDMI vendor infoframe to binary buffer
+ * @frame: HDMI infoframe
+ * @buffer: destination buffer
+ * @size: size of buffer
+ *
+ * Packs the information contained in the @frame structure into a binary
+ * representation that can be written into the corresponding controller
+ * registers. Also computes the checksum as required by section 5.3.5 of
+ * the HDMI 1.4 specification.
+ *
+ * Returns the number of bytes packed into the binary buffer or a negative
+ * error code on failure.
+ */
+ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
+				 void *buffer, size_t size)
+{
+	u8 *ptr = buffer;
+	size_t length;
+
+	/* empty info frame */
+	if (frame->vic == 0 && frame->s3d_struct == HDMI_3D_STRUCTURE_INVALID)
+		return -EINVAL;
+
+	/* only one of those can be supplied */
+	if (frame->vic != 0 && frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID)
+		return -EINVAL;
+
+	length = HDMI_INFOFRAME_HEADER_SIZE + frame->length;
+
+	if (size < length)
+		return -ENOSPC;
+
+	memset(buffer, 0, size);
+
+	ptr[0] = frame->type;
+	ptr[1] = frame->version;
+	ptr[2] = frame->length;
+	ptr[3] = 0; /* checksum */
+
+	/* HDMI OUI */
+	ptr[4] = 0x03;
+	ptr[5] = 0x0c;
+	ptr[6] = 0x00;
+
+	if (frame->vic) {
+		ptr[7] = 0x1 << 5;	/* video format */
+		ptr[8] = frame->vic;
+	} else {
+		ptr[7] = 0x2 << 5;	/* video format */
+		ptr[8] = (frame->s3d_struct & 0xf) << 4;
+	}
+
+	hdmi_infoframe_checksum(buffer, length);
+
+	return length;
+}
+EXPORT_SYMBOL(hdmi_hdmi_infoframe_pack);
+
+/**
  * hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary
  *                                buffer
  * @frame: HDMI vendor infoframe
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index b98340b..f5098a8 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -234,11 +234,36 @@ struct hdmi_vendor_infoframe {
 ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
 				   void *buffer, size_t size);
 
+enum hdmi_3d_structure {
+	HDMI_3D_STRUCTURE_INVALID = -1,
+	HDMI_3D_STRUCTURE_FRAME_PACKING = 0,
+	HDMI_3D_STRUCTURE_FIELD_ALTERNATIVE,
+	HDMI_3D_STRUCTURE_LINE_ALTERNATIVE,
+	HDMI_3D_STRUCTURE_SIDE_BY_SIDE_FULL,
+	HDMI_3D_STRUCTURE_L_DEPTH,
+	HDMI_3D_STRUCTURE_L_DEPTH_GFX_GFX_DEPTH,
+	HDMI_3D_STRUCTURE_TOP_BOTTOM,
+	HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF,
+};
+
+struct hdmi_hdmi_infoframe {
+	enum hdmi_infoframe_type type;
+	unsigned char version;
+	unsigned char length;
+	u8 vic;
+	enum hdmi_3d_structure s3d_struct;
+};
+
+int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame);
+ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
+				 void *buffer, size_t size);
+
 union hdmi_infoframe {
 	struct hdmi_any_infoframe any;
 	struct hdmi_avi_infoframe avi;
 	struct hdmi_spd_infoframe spd;
 	struct hdmi_vendor_infoframe vendor;
+	struct hdmi_hdmi_infoframe hdmi;
 	struct hdmi_audio_infoframe audio;
 };
 
-- 
1.8.3.1

[PATCH 08/12] gpu: host1x: Port the HDMI vendor infoframe code the common helpers

[Damien Lespiau]

I just wrote the bits to define and pack HDMI vendor specific infoframe.
Port the host1x driver to use those so I can refactor the infoframe code
a bit more.

Cc: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Terje Bergström <tbergstrom-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Signed-off-by: Damien Lespiau <damien.lespiau-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/host1x/drm/hdmi.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/host1x/drm/hdmi.c b/drivers/gpu/host1x/drm/hdmi.c
index 01097da..b548918 100644
--- a/drivers/gpu/host1x/drm/hdmi.c
+++ b/drivers/gpu/host1x/drm/hdmi.c
@@ -539,7 +539,7 @@ static void tegra_hdmi_setup_audio_infoframe(struct tegra_hdmi *hdmi)
 
 static void tegra_hdmi_setup_stereo_infoframe(struct tegra_hdmi *hdmi)
 {
-	struct hdmi_vendor_infoframe frame;
+	struct hdmi_hdmi_infoframe frame;
 	unsigned long value;
 	u8 buffer[10];
 	ssize_t err;
@@ -551,26 +551,10 @@ static void tegra_hdmi_setup_stereo_infoframe(struct tegra_hdmi *hdmi)
 		return;
 	}
 
-	memset(&frame, 0, sizeof(frame));
+	hdmi_hdmi_infoframe_init(&frame);
+	frame.s3d_struct = HDMI_3D_STRUCTURE_FRAME_PACKING;
 
-	frame.type = HDMI_INFOFRAME_TYPE_VENDOR;
-	frame.version = 0x01;
-	frame.length = 6;
-
-	frame.data[0] = 0x03; /* regid0 */
-	frame.data[1] = 0x0c; /* regid1 */
-	frame.data[2] = 0x00; /* regid2 */
-	frame.data[3] = 0x02 << 5; /* video format */
-
-	/* TODO: 74 MHz limit? */
-	if (1) {
-		frame.data[4] = 0x00 << 4; /* 3D structure */
-	} else {
-		frame.data[4] = 0x08 << 4; /* 3D structure */
-		frame.data[5] = 0x00 << 4; /* 3D ext. data */
-	}
-
-	err = hdmi_vendor_infoframe_pack(&frame, buffer, sizeof(buffer));
+	err = hdmi_hdmi_infoframe_pack(&frame, buffer, sizeof(buffer));
 	if (err < 0) {
 		dev_err(hdmi->dev, "failed to pack vendor infoframe: %zd\n",
 			err);
-- 
1.8.3.1

[PATCH 09/12] drm/edid: Move HDMI_IDENTIFIER to hdmi.h

[Damien Lespiau]

We'll need the HDMI OUI for the HDMI vendor infoframe data, so let's
move the DRM one to hdmi.h, might as well use the hdmi header to store
some hdmi defines.

(Note that, in fact, infoframes are part of the CEA-861 standard, and
only the HDMI vendor specific infoframe is special to HDMI, but
details..)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 1 -
 include/linux/hdmi.h       | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index ec47eb8..9a07a33 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2317,7 +2317,6 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
 	return closure.modes;
 }
 
-#define HDMI_IDENTIFIER 0x000C03
 #define AUDIO_BLOCK	0x01
 #define VIDEO_BLOCK     0x02
 #define VENDOR_BLOCK    0x03
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index f5098a8..53bbf0d 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -18,6 +18,7 @@ enum hdmi_infoframe_type {
 	HDMI_INFOFRAME_TYPE_AUDIO = 0x84,
 };
 
+#define HDMI_IDENTIFIER 0x000c03
 #define HDMI_INFOFRAME_HEADER_SIZE  4
 #define HDMI_AVI_INFOFRAME_SIZE    13
 #define HDMI_SPD_INFOFRAME_SIZE    25
-- 
1.8.3.1

[PATCH 10/12] video/hdmi: Hook the HDMI vendor infoframe with the generic _pack()

[Damien Lespiau]

With this last bit, hdmi_infoframe_pack() is now able to pack any
infoframe we support.

At the same time, because it's impractical to make two commits out of
this, we get rid of the version that encourages the open coding of the
vendor infoframe packing. We can do so because the only user of this API
has been ported in:

  Author: Damien Lespiau <damien.lespiau@intel.com>
  Date:   Mon Aug 12 18:08:37 2013 +0100

      gpu: host1x: Port the HDMI vendor infoframe code the common helpers

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/video/hdmi.c | 45 +++++++++------------------------------------
 include/linux/hdmi.h | 24 ++++++++++++------------
 2 files changed, 21 insertions(+), 48 deletions(-)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 2059f7b..073f005 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -300,6 +300,7 @@ int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame)
 	frame->length = 5; /* we can hardcode the size for now as we don't
 			    support neither 3D_Ext_Data nor 3D_Metadata_* */
 
+	frame->oui = HDMI_IDENTIFIER;
 	/* 0 is a valid value for s3d_struct, so we use a special "not set"
 	 * value */
 	frame->s3d_struct = HDMI_3D_STRUCTURE_INVALID;
@@ -367,46 +368,18 @@ ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
 }
 EXPORT_SYMBOL(hdmi_hdmi_infoframe_pack);
 
-/**
- * hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary
- *                                buffer
- * @frame: HDMI vendor infoframe
- * @buffer: destination buffer
- * @size: size of buffer
- *
- * Packs the information contained in the @frame structure into a binary
- * representation that can be written into the corresponding controller
- * registers. Also computes the checksum as required by section 5.3.5 of
- * the HDMI 1.4 specification.
- *
- * Returns the number of bytes packed into the binary buffer or a negative
- * error code on failure.
+/*
+ * hdmi_vendor_infoframe_pack() - write a vendor infoframe to binary buffer
  */
-ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
-				   void *buffer, size_t size)
+static ssize_t hdmi_vendor_infoframe_pack(union hdmi_vendor_infoframe *frame,
+					  void *buffer, size_t size)
 {
-	u8 *ptr = buffer;
-	size_t length;
-
-	length = HDMI_INFOFRAME_HEADER_SIZE + frame->length;
-
-	if (size < length)
-		return -ENOSPC;
-
-	memset(buffer, 0, size);
-
-	ptr[0] = frame->type;
-	ptr[1] = frame->version;
-	ptr[2] = frame->length;
-	ptr[3] = 0; /* checksum */
-
-	memcpy(&ptr[HDMI_INFOFRAME_HEADER_SIZE], frame->data, frame->length);
-
-	hdmi_infoframe_checksum(buffer, length);
+	/* we only know about HDMI vendor infoframes */
+	if (frame->any.oui != HDMI_IDENTIFIER)
+		return -EINVAL;
 
-	return length;
+	return hdmi_hdmi_infoframe_pack(&frame->hdmi, buffer, size);
 }
-EXPORT_SYMBOL(hdmi_vendor_infoframe_pack);
 
 /**
  * hdmi_infoframe_pack() - write a HDMI infoframe to binary buffer
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 53bbf0d..5c572e9 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -225,16 +225,6 @@ int hdmi_audio_infoframe_init(struct hdmi_audio_infoframe *frame);
 ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
 				  void *buffer, size_t size);
 
-struct hdmi_vendor_infoframe {
-	enum hdmi_infoframe_type type;
-	unsigned char version;
-	unsigned char length;
-	u8 data[27];
-};
-
-ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
-				   void *buffer, size_t size);
-
 enum hdmi_3d_structure {
 	HDMI_3D_STRUCTURE_INVALID = -1,
 	HDMI_3D_STRUCTURE_FRAME_PACKING = 0,
@@ -251,6 +241,7 @@ struct hdmi_hdmi_infoframe {
 	enum hdmi_infoframe_type type;
 	unsigned char version;
 	unsigned char length;
+	int oui;
 	u8 vic;
 	enum hdmi_3d_structure s3d_struct;
 };
@@ -259,12 +250,21 @@ int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame);
 ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
 				 void *buffer, size_t size);
 
+union hdmi_vendor_infoframe {
+	struct {
+		enum hdmi_infoframe_type type;
+		unsigned char version;
+		unsigned char length;
+		int oui;
+	} any;
+	struct hdmi_hdmi_infoframe hdmi;
+};
+
 union hdmi_infoframe {
 	struct hdmi_any_infoframe any;
 	struct hdmi_avi_infoframe avi;
 	struct hdmi_spd_infoframe spd;
-	struct hdmi_vendor_infoframe vendor;
-	struct hdmi_hdmi_infoframe hdmi;
+	union hdmi_vendor_infoframe vendor;
 	struct hdmi_audio_infoframe audio;
 };
 
-- 
1.8.3.1

[PATCH 11/12] drm: Add a helper to forge HDMI vendor infoframes

[Damien Lespiau]

This can then be used by DRM drivers to setup their vendor infoframes.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 36 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_edid.h     |  4 ++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9a07a33..83e1202 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3262,3 +3262,39 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 	return 0;
 }
 EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
+
+/**
+ * drm_hdmi_vendor_infoframe_from_display_mode() - fill an HDMI infoframe with
+ * data from a DRM display mode
+ * @frame: HDMI vendor infoframe
+ * @mode: DRM display mode
+ *
+ * Note that there's is a need to send HDMI vendor infoframes only when using a
+ * 4k or stereoscopic 3D mode. So when giving any other mode as input this
+ * function will return -EINVAL, error that can be safely ignored.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int
+drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_hdmi_infoframe *frame,
+					    const struct drm_display_mode *mode)
+{
+	int err;
+	u8 vic;
+
+	if (!frame || !mode)
+		return -EINVAL;
+
+	vic = drm_match_hmdi_mode(mode);
+	if (!vic)
+		return -EINVAL;
+
+	err = hdmi_hdmi_infoframe_init(frame);
+	if (err < 0)
+		return err;
+
+	frame->vic = vic;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index fc481fc..a204e31 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -256,6 +256,7 @@ struct drm_encoder;
 struct drm_connector;
 struct drm_display_mode;
 struct hdmi_avi_infoframe;
+struct hdmi_hdmi_infoframe;
 
 void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid);
 int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads);
@@ -268,5 +269,8 @@ int drm_load_edid_firmware(struct drm_connector *connector);
 int
 drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 					 const struct drm_display_mode *mode);
+int
+drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_hdmi_infoframe *frame,
+					    const struct drm_display_mode *mode);
 
 #endif /* __DRM_EDID_H__ */
-- 
1.8.3.1

[PATCH 12/12] drm/i915/hdmi: Write HDMI vendor specific infoframes

[Damien Lespiau]

With all the common infoframe bits now in place, we can finally write
the vendor specific infoframes in our driver.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h   |  2 ++
 drivers/gpu/drm/i915/intel_hdmi.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 17f6252..33427fd1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4157,6 +4157,8 @@
 	 _TRANSCODER(trans, HSW_VIDEO_DIP_CTL_A, HSW_VIDEO_DIP_CTL_B)
 #define HSW_TVIDEO_DIP_AVI_DATA(trans) \
 	 _TRANSCODER(trans, HSW_VIDEO_DIP_AVI_DATA_A, HSW_VIDEO_DIP_AVI_DATA_B)
+#define HSW_TVIDEO_DIP_VS_DATA(trans) \
+	 _TRANSCODER(trans, HSW_VIDEO_DIP_VS_DATA_A, HSW_VIDEO_DIP_VS_DATA_B)
 #define HSW_TVIDEO_DIP_SPD_DATA(trans) \
 	 _TRANSCODER(trans, HSW_VIDEO_DIP_SPD_DATA_A, HSW_VIDEO_DIP_SPD_DATA_B)
 #define HSW_TVIDEO_DIP_GCP(trans) \
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index a619d94..4148cc8 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -74,6 +74,8 @@ static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
 		return VIDEO_DIP_SELECT_AVI;
 	case HDMI_INFOFRAME_TYPE_SPD:
 		return VIDEO_DIP_SELECT_SPD;
+	case HDMI_INFOFRAME_TYPE_VENDOR:
+		return VIDEO_DIP_SELECT_VENDOR;
 	default:
 		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
 		return 0;
@@ -87,6 +89,8 @@ static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
 		return VIDEO_DIP_ENABLE_AVI;
 	case HDMI_INFOFRAME_TYPE_SPD:
 		return VIDEO_DIP_ENABLE_SPD;
+	case HDMI_INFOFRAME_TYPE_VENDOR:
+		return VIDEO_DIP_ENABLE_VENDOR;
 	default:
 		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
 		return 0;
@@ -100,6 +104,8 @@ static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
 		return VIDEO_DIP_ENABLE_AVI_HSW;
 	case HDMI_INFOFRAME_TYPE_SPD:
 		return VIDEO_DIP_ENABLE_SPD_HSW;
+	case HDMI_INFOFRAME_TYPE_VENDOR:
+		return VIDEO_DIP_ENABLE_VS_HSW;
 	default:
 		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
 		return 0;
@@ -114,6 +120,8 @@ static u32 hsw_infoframe_data_reg(enum hdmi_infoframe_type type,
 		return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder);
 	case HDMI_INFOFRAME_TYPE_SPD:
 		return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder);
+	case HDMI_INFOFRAME_TYPE_VENDOR:
+		return HSW_TVIDEO_DIP_VS_DATA(cpu_transcoder);
 	default:
 		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
 		return 0;
@@ -392,6 +400,21 @@ static void intel_hdmi_set_spd_infoframe(struct drm_encoder *encoder)
 	intel_write_infoframe(encoder, &frame);
 }
 
+static void
+intel_hdmi_set_hdmi_infoframe(struct drm_encoder *encoder,
+			      struct drm_display_mode *adjusted_mode)
+{
+	union hdmi_infoframe frame;
+	int ret;
+
+	ret = drm_hdmi_vendor_infoframe_from_display_mode(&frame.vendor.hdmi,
+							  adjusted_mode);
+	if (ret < 0)
+		return;
+
+	intel_write_infoframe(encoder, &frame);
+}
+
 static void g4x_set_infoframes(struct drm_encoder *encoder,
 			       struct drm_display_mode *adjusted_mode)
 {
@@ -454,6 +477,7 @@ static void g4x_set_infoframes(struct drm_encoder *encoder,
 
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
+	intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode);
 }
 
 static void ibx_set_infoframes(struct drm_encoder *encoder,
@@ -515,6 +539,7 @@ static void ibx_set_infoframes(struct drm_encoder *encoder,
 
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
+	intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode);
 }
 
 static void cpt_set_infoframes(struct drm_encoder *encoder,
@@ -550,6 +575,7 @@ static void cpt_set_infoframes(struct drm_encoder *encoder,
 
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
+	intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode);
 }
 
 static void vlv_set_infoframes(struct drm_encoder *encoder,
@@ -584,6 +610,7 @@ static void vlv_set_infoframes(struct drm_encoder *encoder,
 
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
+	intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode);
 }
 
 static void hsw_set_infoframes(struct drm_encoder *encoder,
@@ -611,6 +638,7 @@ static void hsw_set_infoframes(struct drm_encoder *encoder,
 
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
+	intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode);
 }
 
 static void intel_hdmi_mode_set(struct intel_encoder *encoder)
-- 
1.8.3.1

Re: [PATCH 03/12] drm/edid: Parse the HDMI CEA block and look for 4k modes

[Simon Farnsworth]

[-- Attachment #1.1: Type: text/plain, Size: 2960 bytes --]

Minor typo - feel free to ignore:

On Wednesday 14 August 2013 00:17:19 Damien Lespiau wrote:
> HDMI 1.4 adds 4 "4k x 2k" modes in the the CEA vendor specific block.
> 
> With this commit, we now parse this block and expose the 4k modes that
> we find there.
> 
> v2: Fix the "4096x2160" string (nice catch!), add comments about
>     do_hdmi_vsdb_modes() arguments and make it clearer that offset is
>     relative to the end of the required fields of the HDMI VSDB
>     (Ville Syrjälä)
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> Tested-by: Cancan Feng <cancan.feng@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67030
> ---
>  drivers/gpu/drm/drm_edid.c | 124 +++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 109 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 9e9b6ed..0faa08e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c

<snip>

> @@ -2465,6 +2495,68 @@ do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
>  	return modes;
>  }
>  
> +/*
> + * do_hdmi_vsdb_modes - Parse the HDMI Vendor Specific data block
> + * @connector: connector corresponding to the HDMI sink
> + * @db: start of the CEA vendor specific block
> + * @len: length of the CEA block payload, ie. one can access up to db[len]
> + *
> + * Parses the HDMI VSDB looking for modes to add to @connector.
> + */
> +static int
> +do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len)
> +{
> +	struct drm_device *dev = connector->dev;
> +	int modes = 0, offset = 0, i;
> +	u8 vic_len;
> +
> +	if (len < 8)
> +		goto out;
> +
> +	/* no HDMI_Video_Present */
> +	if (!(db[8] & (1 << 5)))
> +		goto out;
> +
> +	/* Latency_Fields_Present */
> +	if (db[8] & (1 << 7))
> +		offset += 2;
> +
> +	/* I_Latency_Fields_Present */
> +	if (db[8] & (1 << 6))
> +		offset += 2;
> +
> +	/* the declared length is not long enough for the 2 first bytes
> +	 * of additional video format capabilities */
> +	offset += 2;
> +	if (len < (8 + offset))
> +		goto out;
> +
> +	vic_len = db[8 + offset] >> 5;
> +
> +	for (i = 0; i < vic_len && len >= (9 + offset + i); i++) {
> +		struct drm_display_mode *newmode;
> +		u8 vic;
> +
> +		vic = db[9 + offset + i];
> +
> +		vic--; /* VICs start at 1 */
> +		if (vic >= ARRAY_SIZE(edid_4k_modes)) {
> +			DRM_ERROR("Unknow HDMI VIC: %d\n", vic);
                       ^^^^^^
Missing "n" - should be Unknown
> +			continue;
> +		}
> +
> +		newmode = drm_mode_duplicate(dev, &edid_4k_modes[vic]);
> +		if (!newmode)
> +			continue;
> +
> +		drm_mode_probed_add(connector, newmode);
> +		modes++;
> +	}
> +
> +out:
> +	return modes;
> +}
> +
>  static int
>  cea_db_payload_len(const u8 *db)
>  {
<snip>
-- 
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 04/12] drm: Add support for alternate clocks of 4k modes

[Simon Farnsworth]

[-- Attachment #1.1: Type: text/plain, Size: 3808 bytes --]

A few minor things:

On Wednesday 14 August 2013 00:17:20 Damien Lespiau wrote:
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 68 ++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 62 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 0faa08e..606335f 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2409,6 +2409,54 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
>  }
>  EXPORT_SYMBOL(drm_match_cea_mode);
>  
> +/*
> + * Calculate the alternate clock for HDMI modes (those from the HDMI vendor
> + * specific block).
> + *
> + * It's almost like cea_mode_alternate_clock(), we just need to add an
> + * exception for the VIC 4 mode (4096x2160@24Hz): no alternate clock for this
> + * one.
> + */
> +static unsigned int
> +hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode)
> +{
> +	if (hdmi_mode->vdisplay == 4096 && hdmi_mode->hdisplay == 2160)
> +		return hdmi_mode->clock;
> +
> +	return cea_mode_alternate_clock(hdmi_mode);
> +}
> +
> +/*
> + * drm_match_cea_mode - look for a HDMI mode matching given mode

Here we document drm_match_cea_mode, but the function is drm_match_hdmi_mode.

> + * @to_match: display mode
> + *
> + * An HDMI mode is one defined in the HDMI vendor specific block.
> + *
> + * Returns the HDMI Video ID (VIC) of the mode or 0 if it isn't one.
> + */
> +static u8 drm_match_hmdi_mode(const struct drm_display_mode *to_match)
                       ^^^^
Surely should be hdmi, not hmdi?
> +{
> +	u8 mode;
> +
> +	if (!to_match->clock)
> +		return 0;
> +
> +	for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) {
> +		const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode];
> +		unsigned int clock1, clock2;
> +
> +		/* Make sure to also match alternate clocks */
> +		clock1 = hdmi_mode->clock;
> +		clock2 = hdmi_mode_alternate_clock(hdmi_mode);
> +
> +		if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) ||
> +		     KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) &&
> +		    drm_mode_equal_no_clocks(to_match, hdmi_mode))
> +			return mode + 1;
> +	}
> +	return 0;
> +}
> +
>  static int
>  add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
>  {
> @@ -2426,18 +2474,26 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
>  	 * with the alternate clock for certain CEA modes.
>  	 */
>  	list_for_each_entry(mode, &connector->probed_modes, head) {
> -		const struct drm_display_mode *cea_mode;
> +		const struct drm_display_mode *cea_mode = NULL;
>  		struct drm_display_mode *newmode;
> -		u8 cea_mode_idx = drm_match_cea_mode(mode) - 1;
> +		u8 mode_idx = drm_match_cea_mode(mode) - 1;
>  		unsigned int clock1, clock2;
>  
> -		if (cea_mode_idx >= ARRAY_SIZE(edid_cea_modes))
> -			continue;
> +		if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
> +			cea_mode = &edid_cea_modes[mode_idx];
> +			clock2 = cea_mode_alternate_clock(cea_mode);
> +		} else {
> +			mode_idx = drm_match_hmdi_mode(mode) - 1;
                                 ^^^^
Same typo here - hmdi for hdmi.

> +			if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
> +				cea_mode = &edid_4k_modes[mode_idx];
> +				clock2 = hdmi_mode_alternate_clock(cea_mode);
> +			}
> +		}
>  
> -		cea_mode = &edid_cea_modes[cea_mode_idx];
> +		if (!cea_mode)
> +			continue;
>  
>  		clock1 = cea_mode->clock;
> -		clock2 = cea_mode_alternate_clock(cea_mode);
>  
>  		if (clock1 == clock2)
>  			continue;
> 
-- 
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 11/12] drm: Add a helper to forge HDMI vendor infoframes

[Simon Farnsworth]

[-- Attachment #1.1: Type: text/plain, Size: 1847 bytes --]

More typo fallout:

On Wednesday 14 August 2013 00:17:27 Damien Lespiau wrote:
> This can then be used by DRM drivers to setup their vendor infoframes.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 36 ++++++++++++++++++++++++++++++++++++
>  include/drm/drm_edid.h     |  4 ++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 9a07a33..83e1202 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3262,3 +3262,39 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
> +
> +/**
> + * drm_hdmi_vendor_infoframe_from_display_mode() - fill an HDMI infoframe with
> + * data from a DRM display mode
> + * @frame: HDMI vendor infoframe
> + * @mode: DRM display mode
> + *
> + * Note that there's is a need to send HDMI vendor infoframes only when using a
> + * 4k or stereoscopic 3D mode. So when giving any other mode as input this
> + * function will return -EINVAL, error that can be safely ignored.
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int
> +drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_hdmi_infoframe *frame,
> +					    const struct drm_display_mode *mode)
> +{
> +	int err;
> +	u8 vic;
> +
> +	if (!frame || !mode)
> +		return -EINVAL;
> +
> +	vic = drm_match_hmdi_mode(mode);
                    ^^^^
Should be hdmi again.

> +	if (!vic)
> +		return -EINVAL;
> +
> +	err = hdmi_hdmi_infoframe_init(frame);
> +	if (err < 0)
> +		return err;
> +
> +	frame->vic = vic;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);

<snip>
-- 
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: HDMI 4k support v2

[Simon Farnsworth]

[-- Attachment #1.1: Type: text/plain, Size: 1259 bytes --]

On Wednesday 14 August 2013 00:17:16 Damien Lespiau wrote:
> Following up the first instance of this series:
>   http://lists.freedesktop.org/archives/dri-devel/2013-August/043125.html
> 
> Here is a v2 with Ville's review pass and a few additions:
>   - Alternate clock modes for 4k resolutions
>   - HDMI vendor specific infoframe support in drivers/video/hdmi.c
>   - Enabling of those vendor specific infoframes in the i915 driver
> 
> Along the way, a tegra patch was needed for a small consolidation of the code
> packing vendor specific infoframes. This patch has only been compile-tested.
> 
> On Intel, it's possible to read back the programmed infoframe buffers with
> intel-gpu-tools' intel_infoframe and this gives:
> 
> Vendor InfoFrame:
> - frequency: every vsync
> - raw:
>   f4050181 000c0347 00000320 00000000
>   00000000 00000000 00000000 00000000
>   0000004c 00000000 00000000 00000000
>   00000000 00000000 00000000 00000000
> - vendor Id: 0x000c03 (HDMI)
> - video format: 0x001 
> - HDMI VIC: 3
> 
> after a $ xrandr --output HDMI1 --mode 3840x2160 -r 24
> 
>
With the typos fixed, feel free to add:

Reviewed-by: Simon Farnsworth <simon.farnsworth@onelan.co.uk>
-- 
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/12] drm/edid: Fix add_cea_modes() style issues

[Ville Syrjälä]

On Wed, Aug 14, 2013 at 12:17:18AM +0100, Damien Lespiau wrote:
> A few styles issues have crept in here, fix them before touching this
> code again.
> 
> v2: constify arguments that can be (Ville Syrjälä)
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index e014785..9e9b6ed 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2301,7 +2301,7 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
>  /*
>   * Search EDID for CEA extension block.
>   */
> -static u8 *drm_find_cea_extension(struct edid *edid)
> +static u8 *drm_find_cea_extension(const struct edid *edid)

This guy still casts away the const internally. Not sure if something
will complain about that. So maybe just drop this bit for now.

A long time ago I posted a big contify everything in drm_edid.c patch,
and it did in a way that didn't ad new warnings even w/ -Wcast-qual.
Maybe I should resurrect it.

>  {
>  	u8 *edid_ext = NULL;
>  	int i;
> @@ -2441,10 +2441,11 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
>  }
>  
>  static int
> -do_cea_modes (struct drm_connector *connector, u8 *db, u8 len)
> +do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
>  {
>  	struct drm_device *dev = connector->dev;
> -	u8 * mode, cea_mode;
> +	const u8 *mode;
> +	u8 cea_mode;
>  	int modes = 0;
>  
>  	for (mode = db; mode < db + len; mode++) {
> @@ -2499,10 +2500,10 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
>  	for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1)
>  
>  static int
> -add_cea_modes(struct drm_connector *connector, struct edid *edid)
> +add_cea_modes(struct drm_connector *connector, const struct edid *edid)
>  {
> -	u8 * cea = drm_find_cea_extension(edid);
> -	u8 * db, dbl;
> +	u8 *cea = drm_find_cea_extension(edid);
> +	u8 *db, dbl;

cea and db could be const here.

>  	int modes = 0;
>  
>  	if (cea && cea_revision(cea) >= 3) {
> @@ -2516,7 +2517,7 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid)
>  			dbl = cea_db_payload_len(db);
>  
>  			if (cea_db_tag(db) == VIDEO_BLOCK)
> -				modes += do_cea_modes (connector, db+1, dbl);
> +				modes += do_cea_modes(connector, db + 1, dbl);
>  		}
>  	}
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

Re: [Intel-gfx] [PATCH 04/12] drm: Add support for alternate clocks of 4k modes

[Ville Syrjälä]

On Wed, Aug 14, 2013 at 12:17:20AM +0100, Damien Lespiau wrote:
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 68 ++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 62 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 0faa08e..606335f 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2409,6 +2409,54 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
>  }
>  EXPORT_SYMBOL(drm_match_cea_mode);
>  
> +/*
> + * Calculate the alternate clock for HDMI modes (those from the HDMI vendor
> + * specific block).
> + *
> + * It's almost like cea_mode_alternate_clock(), we just need to add an
> + * exception for the VIC 4 mode (4096x2160@24Hz): no alternate clock for this
> + * one.
> + */
> +static unsigned int
> +hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode)
> +{
> +	if (hdmi_mode->vdisplay == 4096 && hdmi_mode->hdisplay == 2160)
> +		return hdmi_mode->clock;
> +
> +	return cea_mode_alternate_clock(hdmi_mode);
> +}
> +
> +/*
> + * drm_match_cea_mode - look for a HDMI mode matching given mode
                ^^^

Fix that, and you get:

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> + * @to_match: display mode
> + *
> + * An HDMI mode is one defined in the HDMI vendor specific block.
> + *
> + * Returns the HDMI Video ID (VIC) of the mode or 0 if it isn't one.
> + */
> +static u8 drm_match_hmdi_mode(const struct drm_display_mode *to_match)
> +{
> +	u8 mode;
> +
> +	if (!to_match->clock)
> +		return 0;
> +
> +	for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) {
> +		const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode];
> +		unsigned int clock1, clock2;
> +
> +		/* Make sure to also match alternate clocks */
> +		clock1 = hdmi_mode->clock;
> +		clock2 = hdmi_mode_alternate_clock(hdmi_mode);
> +
> +		if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) ||
> +		     KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) &&
> +		    drm_mode_equal_no_clocks(to_match, hdmi_mode))
> +			return mode + 1;
> +	}
> +	return 0;
> +}
> +
>  static int
>  add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
>  {
> @@ -2426,18 +2474,26 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
>  	 * with the alternate clock for certain CEA modes.
>  	 */
>  	list_for_each_entry(mode, &connector->probed_modes, head) {
> -		const struct drm_display_mode *cea_mode;
> +		const struct drm_display_mode *cea_mode = NULL;
>  		struct drm_display_mode *newmode;
> -		u8 cea_mode_idx = drm_match_cea_mode(mode) - 1;
> +		u8 mode_idx = drm_match_cea_mode(mode) - 1;
>  		unsigned int clock1, clock2;
>  
> -		if (cea_mode_idx >= ARRAY_SIZE(edid_cea_modes))
> -			continue;
> +		if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
> +			cea_mode = &edid_cea_modes[mode_idx];
> +			clock2 = cea_mode_alternate_clock(cea_mode);
> +		} else {
> +			mode_idx = drm_match_hmdi_mode(mode) - 1;
> +			if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
> +				cea_mode = &edid_4k_modes[mode_idx];
> +				clock2 = hdmi_mode_alternate_clock(cea_mode);
> +			}
> +		}
>  
> -		cea_mode = &edid_cea_modes[cea_mode_idx];
> +		if (!cea_mode)
> +			continue;
>  
>  		clock1 = cea_mode->clock;
> -		clock2 = cea_mode_alternate_clock(cea_mode);
>  
>  		if (clock1 == clock2)
>  			continue;
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 06/12] video/hdmi: Derive the bar data valid bit from the bar data fields

[Ville Syrjälä]

On Wed, Aug 14, 2013 at 12:17:22AM +0100, Damien Lespiau wrote:
> Just like:
> 
>   Author: Damien Lespiau <damien.lespiau@intel.com>
>   Date:   Mon Aug 12 11:53:24 2013 +0100
> 
>       video/hdmi: Don't let the user of this API create invalid infoframes
> 
> But this time for the horizontal/vertical bar data present bits.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/video/hdmi.c | 5 +++--
>  include/linux/hdmi.h | 2 --
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index e36da36..ac84215 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -101,10 +101,11 @@ ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe *frame, void *buffer,
>  	if (frame->active_aspect & 0xf)
>  		ptr[0] |= BIT(4);
>  
> -	if (frame->horizontal_bar_valid)
> +	/* Bit 3 and 2 indicate if we transmit horizontal/vertical bar data */
> +	if (frame->top_bar || frame->bottom_bar)
>  		ptr[0] |= BIT(3);
>  
> -	if (frame->vertical_bar_valid)
> +	if (frame->left_bar || frame->right_bar)
>  		ptr[0] |= BIT(2);

Technically top=0,bottom=0 or left=0,right=0 is a valid bar setup,
but it would indicate that the entire picture is made up of the
bottom/right bar. I guess no one would really want to use such a
setup, and even if they do, they could just use some N!=0 for
both bars to achieve the same effect. So we don't seem to lose
anything by doing this.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  	ptr[1] = ((frame->colorimetry & 0x3) << 6) |
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 931474c6..b98340b 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -109,8 +109,6 @@ struct hdmi_avi_infoframe {
>  	unsigned char version;
>  	unsigned char length;
>  	enum hdmi_colorspace colorspace;
> -	bool horizontal_bar_valid;
> -	bool vertical_bar_valid;
>  	enum hdmi_scan_mode scan_mode;
>  	enum hdmi_colorimetry colorimetry;
>  	enum hdmi_picture_aspect picture_aspect;
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 07/12] video/hdmi: Introduce helpers for the HDMI vendor specific infoframe

[Ville Syrjälä]

On Wed, Aug 14, 2013 at 12:17:23AM +0100, Damien Lespiau wrote:
> Provide the programming model than the other infoframe types.
> 
> The generic _pack() function can't handle those yet as we need to move
> the vendor OUI in the generic hdmi_vendor_infoframe structure to know
> which kind of vendor infoframe we are dealing with.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/video/hdmi.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/hdmi.h | 25 ++++++++++++++++
>  2 files changed, 107 insertions(+)
> 
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index ac84215..2059f7b 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -286,6 +286,88 @@ ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
>  EXPORT_SYMBOL(hdmi_audio_infoframe_pack);
>  
>  /**
> + * hdmi_hdmi_infoframe_init() - initialize an HDMI vendor infoframe
> + * @frame: HDMI vendor infoframe
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame)
> +{
> +	memset(frame, 0, sizeof(*frame));
> +
> +	frame->type = HDMI_INFOFRAME_TYPE_VENDOR;
> +	frame->version = 1;
> +	frame->length = 5; /* we can hardcode the size for now as we don't
> +			    support neither 3D_Ext_Data nor 3D_Metadata_* */
> +
> +	/* 0 is a valid value for s3d_struct, so we use a special "not set"
> +	 * value */
> +	frame->s3d_struct = HDMI_3D_STRUCTURE_INVALID;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(hdmi_hdmi_infoframe_init);
> +
> +/**
> + * hdmi_hmdi_infoframe_pack() - write a HDMI vendor infoframe to binary buffer
> + * @frame: HDMI infoframe
> + * @buffer: destination buffer
> + * @size: size of buffer
> + *
> + * Packs the information contained in the @frame structure into a binary
> + * representation that can be written into the corresponding controller
> + * registers. Also computes the checksum as required by section 5.3.5 of
> + * the HDMI 1.4 specification.
> + *
> + * Returns the number of bytes packed into the binary buffer or a negative
> + * error code on failure.
> + */
> +ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
> +				 void *buffer, size_t size)
> +{
> +	u8 *ptr = buffer;
> +	size_t length;
> +
> +	/* empty info frame */
> +	if (frame->vic == 0 && frame->s3d_struct == HDMI_3D_STRUCTURE_INVALID)
> +		return -EINVAL;
> +
> +	/* only one of those can be supplied */
> +	if (frame->vic != 0 && frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID)
> +		return -EINVAL;
> +
> +	length = HDMI_INFOFRAME_HEADER_SIZE + frame->length;
> +
> +	if (size < length)
> +		return -ENOSPC;
> +
> +	memset(buffer, 0, size);
> +
> +	ptr[0] = frame->type;
> +	ptr[1] = frame->version;
> +	ptr[2] = frame->length;
> +	ptr[3] = 0; /* checksum */
> +
> +	/* HDMI OUI */
> +	ptr[4] = 0x03;
> +	ptr[5] = 0x0c;
> +	ptr[6] = 0x00;
> +
> +	if (frame->vic) {
> +		ptr[7] = 0x1 << 5;	/* video format */
> +		ptr[8] = frame->vic;
> +	} else {
> +		ptr[7] = 0x2 << 5;	/* video format */
> +		ptr[8] = (frame->s3d_struct & 0xf) << 4;
> +	}
> +
> +	hdmi_infoframe_checksum(buffer, length);
> +
> +	return length;
> +}
> +EXPORT_SYMBOL(hdmi_hdmi_infoframe_pack);
> +
> +/**
>   * hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary
>   *                                buffer
>   * @frame: HDMI vendor infoframe
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index b98340b..f5098a8 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -234,11 +234,36 @@ struct hdmi_vendor_infoframe {
>  ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
>  				   void *buffer, size_t size);
>  
> +enum hdmi_3d_structure {
> +	HDMI_3D_STRUCTURE_INVALID = -1,
> +	HDMI_3D_STRUCTURE_FRAME_PACKING = 0,
> +	HDMI_3D_STRUCTURE_FIELD_ALTERNATIVE,
> +	HDMI_3D_STRUCTURE_LINE_ALTERNATIVE,
> +	HDMI_3D_STRUCTURE_SIDE_BY_SIDE_FULL,
> +	HDMI_3D_STRUCTURE_L_DEPTH,
> +	HDMI_3D_STRUCTURE_L_DEPTH_GFX_GFX_DEPTH,
> +	HDMI_3D_STRUCTURE_TOP_BOTTOM,

TOP_AND_BOTTOM maybe?

> +	HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF,

The value of this should be 8. Also according to the spec
we must send 3d_ext_data when when 3d_structure >= 8, so
either we need a bit more code, or we need to disallow
3d_structure >= 8 for the time being.

> +};
> +
> +struct hdmi_hdmi_infoframe {
> +	enum hdmi_infoframe_type type;
> +	unsigned char version;
> +	unsigned char length;
> +	u8 vic;
> +	enum hdmi_3d_structure s3d_struct;
> +};
> +
> +int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame);
> +ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
> +				 void *buffer, size_t size);
> +
>  union hdmi_infoframe {
>  	struct hdmi_any_infoframe any;
>  	struct hdmi_avi_infoframe avi;
>  	struct hdmi_spd_infoframe spd;
>  	struct hdmi_vendor_infoframe vendor;
> +	struct hdmi_hdmi_infoframe hdmi;
>  	struct hdmi_audio_infoframe audio;
>  };
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

Re: [Intel-gfx] [PATCH 10/12] video/hdmi: Hook the HDMI vendor infoframe with the generic _pack()

[Ville Syrjälä]

On Wed, Aug 14, 2013 at 12:17:26AM +0100, Damien Lespiau wrote:
> With this last bit, hdmi_infoframe_pack() is now able to pack any
> infoframe we support.
> 
> At the same time, because it's impractical to make two commits out of
> this, we get rid of the version that encourages the open coding of the
> vendor infoframe packing. We can do so because the only user of this API
> has been ported in:
> 
>   Author: Damien Lespiau <damien.lespiau@intel.com>
>   Date:   Mon Aug 12 18:08:37 2013 +0100
> 
>       gpu: host1x: Port the HDMI vendor infoframe code the common helpers
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/video/hdmi.c | 45 +++++++++------------------------------------
>  include/linux/hdmi.h | 24 ++++++++++++------------
>  2 files changed, 21 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index 2059f7b..073f005 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -300,6 +300,7 @@ int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame)
>  	frame->length = 5; /* we can hardcode the size for now as we don't
>  			    support neither 3D_Ext_Data nor 3D_Metadata_* */
>  
> +	frame->oui = HDMI_IDENTIFIER;
>  	/* 0 is a valid value for s3d_struct, so we use a special "not set"
>  	 * value */
>  	frame->s3d_struct = HDMI_3D_STRUCTURE_INVALID;
> @@ -367,46 +368,18 @@ ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
>  }
>  EXPORT_SYMBOL(hdmi_hdmi_infoframe_pack);
>  
> -/**
> - * hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary
> - *                                buffer
> - * @frame: HDMI vendor infoframe
> - * @buffer: destination buffer
> - * @size: size of buffer
> - *
> - * Packs the information contained in the @frame structure into a binary
> - * representation that can be written into the corresponding controller
> - * registers. Also computes the checksum as required by section 5.3.5 of
> - * the HDMI 1.4 specification.
> - *
> - * Returns the number of bytes packed into the binary buffer or a negative
> - * error code on failure.
> +/*
> + * hdmi_vendor_infoframe_pack() - write a vendor infoframe to binary buffer
>   */
> -ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
> -				   void *buffer, size_t size)
> +static ssize_t hdmi_vendor_infoframe_pack(union hdmi_vendor_infoframe *frame,
> +					  void *buffer, size_t size)
>  {
> -	u8 *ptr = buffer;
> -	size_t length;
> -
> -	length = HDMI_INFOFRAME_HEADER_SIZE + frame->length;
> -
> -	if (size < length)
> -		return -ENOSPC;
> -
> -	memset(buffer, 0, size);
> -
> -	ptr[0] = frame->type;
> -	ptr[1] = frame->version;
> -	ptr[2] = frame->length;
> -	ptr[3] = 0; /* checksum */
> -
> -	memcpy(&ptr[HDMI_INFOFRAME_HEADER_SIZE], frame->data, frame->length);
> -
> -	hdmi_infoframe_checksum(buffer, length);
> +	/* we only know about HDMI vendor infoframes */
> +	if (frame->any.oui != HDMI_IDENTIFIER)
> +		return -EINVAL;
>  
> -	return length;
> +	return hdmi_hdmi_infoframe_pack(&frame->hdmi, buffer, size);
>  }
> -EXPORT_SYMBOL(hdmi_vendor_infoframe_pack);
>  
>  /**
>   * hdmi_infoframe_pack() - write a HDMI infoframe to binary buffer
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 53bbf0d..5c572e9 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -225,16 +225,6 @@ int hdmi_audio_infoframe_init(struct hdmi_audio_infoframe *frame);
>  ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
>  				  void *buffer, size_t size);
>  
> -struct hdmi_vendor_infoframe {
> -	enum hdmi_infoframe_type type;
> -	unsigned char version;
> -	unsigned char length;
> -	u8 data[27];
> -};
> -
> -ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
> -				   void *buffer, size_t size);
> -
>  enum hdmi_3d_structure {
>  	HDMI_3D_STRUCTURE_INVALID = -1,
>  	HDMI_3D_STRUCTURE_FRAME_PACKING = 0,
> @@ -251,6 +241,7 @@ struct hdmi_hdmi_infoframe {
>  	enum hdmi_infoframe_type type;
>  	unsigned char version;
>  	unsigned char length;
> +	int oui;

unsigned perhaps? Same deal below in the 'any' struct.

Doesn't really matter I guess, so w/ or w/o that change:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	u8 vic;
>  	enum hdmi_3d_structure s3d_struct;
>  };
> @@ -259,12 +250,21 @@ int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame);
>  ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
>  				 void *buffer, size_t size);
>  
> +union hdmi_vendor_infoframe {
> +	struct {
> +		enum hdmi_infoframe_type type;
> +		unsigned char version;
> +		unsigned char length;
> +		int oui;
> +	} any;
> +	struct hdmi_hdmi_infoframe hdmi;
> +};
> +
>  union hdmi_infoframe {
>  	struct hdmi_any_infoframe any;
>  	struct hdmi_avi_infoframe avi;
>  	struct hdmi_spd_infoframe spd;
> -	struct hdmi_vendor_infoframe vendor;
> -	struct hdmi_hdmi_infoframe hdmi;
> +	union hdmi_vendor_infoframe vendor;
>  	struct hdmi_audio_infoframe audio;
>  };
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 08/12] gpu: host1x: Port the HDMI vendor infoframe code the common helpers

[Ville Syrjälä]

On Wed, Aug 14, 2013 at 12:17:24AM +0100, Damien Lespiau wrote:
> I just wrote the bits to define and pack HDMI vendor specific infoframe.
> Port the host1x driver to use those so I can refactor the infoframe code
> a bit more.
> 
> Cc: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Terje Bergström <tbergstrom-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> 
> Signed-off-by: Damien Lespiau <damien.lespiau-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/gpu/host1x/drm/hdmi.c | 24 ++++--------------------
>  1 file changed, 4 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/host1x/drm/hdmi.c b/drivers/gpu/host1x/drm/hdmi.c
> index 01097da..b548918 100644
> --- a/drivers/gpu/host1x/drm/hdmi.c
> +++ b/drivers/gpu/host1x/drm/hdmi.c
> @@ -539,7 +539,7 @@ static void tegra_hdmi_setup_audio_infoframe(struct tegra_hdmi *hdmi)
>  
>  static void tegra_hdmi_setup_stereo_infoframe(struct tegra_hdmi *hdmi)
>  {
> -	struct hdmi_vendor_infoframe frame;
> +	struct hdmi_hdmi_infoframe frame;
>  	unsigned long value;
>  	u8 buffer[10];
>  	ssize_t err;
> @@ -551,26 +551,10 @@ static void tegra_hdmi_setup_stereo_infoframe(struct tegra_hdmi *hdmi)
>  		return;
>  	}
>  
> -	memset(&frame, 0, sizeof(frame));
> +	hdmi_hdmi_infoframe_init(&frame);
> +	frame.s3d_struct = HDMI_3D_STRUCTURE_FRAME_PACKING;
>  
> -	frame.type = HDMI_INFOFRAME_TYPE_VENDOR;
> -	frame.version = 0x01;
> -	frame.length = 6;

This changes the length of the infoframe from 6 to 5, which is enough
for "frame packing", but maybe the commit msg should still mention that
small detail.

> -
> -	frame.data[0] = 0x03; /* regid0 */
> -	frame.data[1] = 0x0c; /* regid1 */
> -	frame.data[2] = 0x00; /* regid2 */
> -	frame.data[3] = 0x02 << 5; /* video format */
> -
> -	/* TODO: 74 MHz limit? */
> -	if (1) {
> -		frame.data[4] = 0x00 << 4; /* 3D structure */
> -	} else {
> -		frame.data[4] = 0x08 << 4; /* 3D structure */
> -		frame.data[5] = 0x00 << 4; /* 3D ext. data */
> -	}
> -
> -	err = hdmi_vendor_infoframe_pack(&frame, buffer, sizeof(buffer));
> +	err = hdmi_hdmi_infoframe_pack(&frame, buffer, sizeof(buffer));
>  	if (err < 0) {
>  		dev_err(hdmi->dev, "failed to pack vendor infoframe: %zd\n",
>  			err);
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

Re: [Intel-gfx] [PATCH 07/12] video/hdmi: Introduce helpers for the HDMI vendor specific infoframe

[Ville Syrjälä]

On Wed, Aug 14, 2013 at 12:17:23AM +0100, Damien Lespiau wrote:
> Provide the programming model than the other infoframe types.
> 
> The generic _pack() function can't handle those yet as we need to move
> the vendor OUI in the generic hdmi_vendor_infoframe structure to know
> which kind of vendor infoframe we are dealing with.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/video/hdmi.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/hdmi.h | 25 ++++++++++++++++
>  2 files changed, 107 insertions(+)
> 
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index ac84215..2059f7b 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -286,6 +286,88 @@ ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
>  EXPORT_SYMBOL(hdmi_audio_infoframe_pack);
>  
>  /**
> + * hdmi_hdmi_infoframe_init() - initialize an HDMI vendor infoframe
> + * @frame: HDMI vendor infoframe
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame)
> +{
> +	memset(frame, 0, sizeof(*frame));
> +
> +	frame->type = HDMI_INFOFRAME_TYPE_VENDOR;
> +	frame->version = 1;
> +	frame->length = 5; /* we can hardcode the size for now as we don't
> +			    support neither 3D_Ext_Data nor 3D_Metadata_* */
> +
> +	/* 0 is a valid value for s3d_struct, so we use a special "not set"
> +	 * value */
> +	frame->s3d_struct = HDMI_3D_STRUCTURE_INVALID;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(hdmi_hdmi_infoframe_init);
> +
> +/**
> + * hdmi_hmdi_infoframe_pack() - write a HDMI vendor infoframe to binary buffer
           ^^^^

Another hmdi typo that wasn't spotted yet :)

> + * @frame: HDMI infoframe
> + * @buffer: destination buffer
> + * @size: size of buffer
> + *
> + * Packs the information contained in the @frame structure into a binary
> + * representation that can be written into the corresponding controller
> + * registers. Also computes the checksum as required by section 5.3.5 of
> + * the HDMI 1.4 specification.
> + *
> + * Returns the number of bytes packed into the binary buffer or a negative
> + * error code on failure.
> + */
> +ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
> +				 void *buffer, size_t size)
> +{
> +	u8 *ptr = buffer;
> +	size_t length;
> +
> +	/* empty info frame */
> +	if (frame->vic == 0 && frame->s3d_struct == HDMI_3D_STRUCTURE_INVALID)
> +		return -EINVAL;
> +
> +	/* only one of those can be supplied */
> +	if (frame->vic != 0 && frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID)
> +		return -EINVAL;
> +
> +	length = HDMI_INFOFRAME_HEADER_SIZE + frame->length;
> +
> +	if (size < length)
> +		return -ENOSPC;
> +
> +	memset(buffer, 0, size);
> +
> +	ptr[0] = frame->type;
> +	ptr[1] = frame->version;
> +	ptr[2] = frame->length;
> +	ptr[3] = 0; /* checksum */
> +
> +	/* HDMI OUI */
> +	ptr[4] = 0x03;
> +	ptr[5] = 0x0c;
> +	ptr[6] = 0x00;
> +
> +	if (frame->vic) {
> +		ptr[7] = 0x1 << 5;	/* video format */
> +		ptr[8] = frame->vic;
> +	} else {
> +		ptr[7] = 0x2 << 5;	/* video format */
> +		ptr[8] = (frame->s3d_struct & 0xf) << 4;
> +	}
> +
> +	hdmi_infoframe_checksum(buffer, length);
> +
> +	return length;
> +}
> +EXPORT_SYMBOL(hdmi_hdmi_infoframe_pack);
> +
> +/**
>   * hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary
>   *                                buffer
>   * @frame: HDMI vendor infoframe
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index b98340b..f5098a8 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -234,11 +234,36 @@ struct hdmi_vendor_infoframe {
>  ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
>  				   void *buffer, size_t size);
>  
> +enum hdmi_3d_structure {
> +	HDMI_3D_STRUCTURE_INVALID = -1,
> +	HDMI_3D_STRUCTURE_FRAME_PACKING = 0,
> +	HDMI_3D_STRUCTURE_FIELD_ALTERNATIVE,
> +	HDMI_3D_STRUCTURE_LINE_ALTERNATIVE,
> +	HDMI_3D_STRUCTURE_SIDE_BY_SIDE_FULL,
> +	HDMI_3D_STRUCTURE_L_DEPTH,
> +	HDMI_3D_STRUCTURE_L_DEPTH_GFX_GFX_DEPTH,
> +	HDMI_3D_STRUCTURE_TOP_BOTTOM,
> +	HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF,
> +};
> +
> +struct hdmi_hdmi_infoframe {
> +	enum hdmi_infoframe_type type;
> +	unsigned char version;
> +	unsigned char length;
> +	u8 vic;
> +	enum hdmi_3d_structure s3d_struct;
> +};
> +
> +int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame);
> +ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
> +				 void *buffer, size_t size);
> +
>  union hdmi_infoframe {
>  	struct hdmi_any_infoframe any;
>  	struct hdmi_avi_infoframe avi;
>  	struct hdmi_spd_infoframe spd;
>  	struct hdmi_vendor_infoframe vendor;
> +	struct hdmi_hdmi_infoframe hdmi;
>  	struct hdmi_audio_infoframe audio;
>  };
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: HDMI 4k support v2

[Ville Syrjälä]

On Wed, Aug 14, 2013 at 12:17:16AM +0100, Damien Lespiau wrote:
> Following up the first instance of this series:
>   http://lists.freedesktop.org/archives/dri-devel/2013-August/043125.html
> 
> Here is a v2 with Ville's review pass and a few additions:
>   - Alternate clock modes for 4k resolutions
>   - HDMI vendor specific infoframe support in drivers/video/hdmi.c
>   - Enabling of those vendor specific infoframes in the i915 driver

The patches I didn't explicitly comment on (01,05,09,12 by my reckoning) are:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> Along the way, a tegra patch was needed for a small consolidation of the code
> packing vendor specific infoframes. This patch has only been compile-tested.
> 
> On Intel, it's possible to read back the programmed infoframe buffers with
> intel-gpu-tools' intel_infoframe and this gives:
> 
> Vendor InfoFrame:
> - frequency: every vsync
> - raw:
>   f4050181 000c0347 00000320 00000000
>   00000000 00000000 00000000 00000000
>   0000004c 00000000 00000000 00000000
>   00000000 00000000 00000000 00000000
> - vendor Id: 0x000c03 (HDMI)
> - video format: 0x001 
> - HDMI VIC: 3
> 
> after a $ xrandr --output HDMI1 --mode 3840x2160 -r 24
> 
> -- 
> Damien
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

[PATCH 03/12] drm/edid: Parse the HDMI CEA block and look for 4k modes

[Damien Lespiau]

HDMI 1.4 adds 4 "4k x 2k" modes in the the CEA vendor specific block.

With this commit, we now parse this block and expose the 4k modes that
we find there.

v2: Fix the "4096x2160" string (nice catch!), add comments about
    do_hdmi_vsdb_modes() arguments and make it clearer that offset is
    relative to the end of the required fields of the HDMI VSDB
    (Ville Syrjälä)

v3: Fix 'Unknow' typo (Simon Farnsworth)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Tested-by: Cancan Feng <cancan.feng@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67030
Reviewed-by: Simon Farnsworth <simon.farnsworth@onelan.co.uk>
---
 drivers/gpu/drm/drm_edid.c | 124 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 109 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index bb25ee2..9de573c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -931,6 +931,36 @@ static const struct drm_display_mode edid_cea_modes[] = {
 	 .vrefresh = 100, },
 };
 
+/*
+ * HDMI 1.4 4k modes.
+ */
+static const struct drm_display_mode edid_4k_modes[] = {
+	/* 1 - 3840x2160@30Hz */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
+		   3840, 4016, 4104, 4400, 0,
+		   2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 30, },
+	/* 2 - 3840x2160@25Hz */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
+		   3840, 4896, 4984, 5280, 0,
+		   2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 25, },
+	/* 3 - 3840x2160@24Hz */
+	{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
+		   3840, 5116, 5204, 5500, 0,
+		   2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 24, },
+	/* 4 - 4096x2160@24Hz (SMPTE) */
+	{ DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000,
+		   4096, 5116, 5204, 5500, 0,
+		   2160, 2168, 2178, 2250, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 24, },
+};
+
 /*** DDC fetch and block validation ***/
 
 static const u8 edid_header[] = {
@@ -2465,6 +2495,68 @@ do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
 	return modes;
 }
 
+/*
+ * do_hdmi_vsdb_modes - Parse the HDMI Vendor Specific data block
+ * @connector: connector corresponding to the HDMI sink
+ * @db: start of the CEA vendor specific block
+ * @len: length of the CEA block payload, ie. one can access up to db[len]
+ *
+ * Parses the HDMI VSDB looking for modes to add to @connector.
+ */
+static int
+do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len)
+{
+	struct drm_device *dev = connector->dev;
+	int modes = 0, offset = 0, i;
+	u8 vic_len;
+
+	if (len < 8)
+		goto out;
+
+	/* no HDMI_Video_Present */
+	if (!(db[8] & (1 << 5)))
+		goto out;
+
+	/* Latency_Fields_Present */
+	if (db[8] & (1 << 7))
+		offset += 2;
+
+	/* I_Latency_Fields_Present */
+	if (db[8] & (1 << 6))
+		offset += 2;
+
+	/* the declared length is not long enough for the 2 first bytes
+	 * of additional video format capabilities */
+	offset += 2;
+	if (len < (8 + offset))
+		goto out;
+
+	vic_len = db[8 + offset] >> 5;
+
+	for (i = 0; i < vic_len && len >= (9 + offset + i); i++) {
+		struct drm_display_mode *newmode;
+		u8 vic;
+
+		vic = db[9 + offset + i];
+
+		vic--; /* VICs start at 1 */
+		if (vic >= ARRAY_SIZE(edid_4k_modes)) {
+			DRM_ERROR("Unknown HDMI VIC: %d\n", vic);
+			continue;
+		}
+
+		newmode = drm_mode_duplicate(dev, &edid_4k_modes[vic]);
+		if (!newmode)
+			continue;
+
+		drm_mode_probed_add(connector, newmode);
+		modes++;
+	}
+
+out:
+	return modes;
+}
+
 static int
 cea_db_payload_len(const u8 *db)
 {
@@ -2496,6 +2588,21 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
 	return 0;
 }
 
+static bool cea_db_is_hdmi_vsdb(const u8 *db)
+{
+	int hdmi_id;
+
+	if (cea_db_tag(db) != VENDOR_BLOCK)
+		return false;
+
+	if (cea_db_payload_len(db) < 5)
+		return false;
+
+	hdmi_id = db[1] | (db[2] << 8) | (db[3] << 16);
+
+	return hdmi_id == HDMI_IDENTIFIER;
+}
+
 #define for_each_cea_db(cea, i, start, end) \
 	for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1)
 
@@ -2519,6 +2626,8 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid)
 
 			if (cea_db_tag(db) == VIDEO_BLOCK)
 				modes += do_cea_modes(connector, db + 1, dbl);
+			else if (cea_db_is_hdmi_vsdb(db))
+				modes += do_hdmi_vsdb_modes(connector, db, dbl);
 		}
 	}
 
@@ -2571,21 +2680,6 @@ monitor_name(struct detailed_timing *t, void *data)
 		*(u8 **)data = t->data.other_data.data.str.str;
 }
 
-static bool cea_db_is_hdmi_vsdb(const u8 *db)
-{
-	int hdmi_id;
-
-	if (cea_db_tag(db) != VENDOR_BLOCK)
-		return false;
-
-	if (cea_db_payload_len(db) < 5)
-		return false;
-
-	hdmi_id = db[1] | (db[2] << 8) | (db[3] << 16);
-
-	return hdmi_id == HDMI_IDENTIFIER;
-}
-
 /**
  * drm_edid_to_eld - build ELD from EDID
  * @connector: connector corresponding to the HDMI/DP sink
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx