HDMI 4k support

HDMI 4k support

[Damien Lespiau]

This series parses one of the dark corners of the EDID to expose "4k x 2k"
modes to userspace. Those modes are part of HDMI 1.4.

To complete the 4k HDMI support, one needs:

  * Hardware able to output a 300 MHz TMDS clock (Haswell can do that) and
    Daniel's patch to bump that limit in the kernel (already queued).

      Author: Daniel Vetter <daniel.vetter@ffwll.ch>
      Date:   Mon Jul 22 18:02:39 2013 +0200

          drm/i915: fix hdmi portclock limits

  * The 2 patches attached here to parse the HDMI VICs in the HDMI vendor
    specific CEA block.

  * A DDX patch (for UXA) already merged (and in 2.21.14)

      Author: Damien Lespiau <damien.lespiau@intel.com>
      Date:   Wed Jul 31 18:50:51 2013 +0100

          uxa/display: Keep the EDID blob around for the lifetime of an output

  * My "Use the TMDS maximum frequency to check mode dot clock" xserver series:

    http://lists.x.org/archives/xorg-devel/2013-August/037297.html

The bug referencing all that and the testing by QA:

  https://bugs.freedesktop.org/show_bug.cgi?id=67030
    
Additionally, edid-decode has been taught about those modes as well:

  http://lists.freedesktop.org/archives/dri-devel/2013-August/043078.html

-- 
Damien

[PATCH 1/2] drm/edid: Fix add_cea_modes() style issues

[Damien Lespiau]

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

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

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 95d6f4b..51342c4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2442,10 +2442,10 @@ 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, u8 *db, u8 len)
 {
 	struct drm_device *dev = connector->dev;
-	u8 * mode, cea_mode;
+	u8 *mode, cea_mode;
 	int modes = 0;
 
 	for (mode = db; mode < db + len; mode++) {
@@ -2502,8 +2502,8 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
 static int
 add_cea_modes(struct drm_connector *connector, 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) {
@@ -2517,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

Re: [PATCH 1/2] drm/edid: Fix add_cea_modes() style issues

[Ville Syrjälä]

On Wed, Aug 07, 2013 at 07:39:13PM +0100, Damien Lespiau wrote:
> A few styles issues have creept in here, fix them before touching this
> code again.

You could also sprinke a bit of constness into these functions while
you're touching them.

> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 95d6f4b..51342c4 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2442,10 +2442,10 @@ 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, u8 *db, u8 len)
>  {
>  	struct drm_device *dev = connector->dev;
> -	u8 * mode, cea_mode;
> +	u8 *mode, cea_mode;


>  	int modes = 0;
>  
>  	for (mode = db; mode < db + len; mode++) {
> @@ -2502,8 +2502,8 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
>  static int
>  add_cea_modes(struct drm_connector *connector, 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) {
> @@ -2517,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

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

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 | 115 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 100 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 51342c4..b43d64f 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("3840x2160", 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,59 @@ do_cea_modes(struct drm_connector *connector, u8 *db, u8 len)
 	return modes;
 }
 
+static int do_hdmi_vsdb_modes(struct drm_connector *connector, 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 */
+	if (len < (10 + offset))
+		goto out;
+
+	vic_len =  db[10 + offset] >> 5;
+	offset += 2;
+
+	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 +2579,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 +2616,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);
 		}
 	}
 
@@ -2570,21 +2670,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

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

[Ville Syrjälä]

On Wed, Aug 07, 2013 at 07:39:14PM +0100, 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.
> 
> 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 | 115 +++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 100 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 51342c4..b43d64f 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("3840x2160", 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, },

We should also add the 1000/1001 variants for #1 and #3. But we
don't want drm_match_cea_mode() to return the HDMI VIC, so a bit of
gymnastics are going to be required. I guess we want some kind of
drm_match_hdmi_mode() since we should also send the HDMI VIC in the
HDMI infoframe (if and when someone adds support for such a thing).

Also the mode #4 isn't supposed to have a 23.97Hz variant, so if we
want to utilize cea_mode_alternate_clock() for drm_match_hdmi_mode()
we need to add an exception there.

> +};
> +
>  /*** DDC fetch and block validation ***/
>  
>  static const u8 edid_header[] = {
> @@ -2465,6 +2495,59 @@ do_cea_modes(struct drm_connector *connector, u8 *db, u8 len)
>  	return modes;
>  }
>  
> +static int do_hdmi_vsdb_modes(struct drm_connector *connector, u8 *db, u8 len)
> +{

Could be 'const u8 *db'

> +	struct drm_device *dev = connector->dev;
> +	int modes = 0, offset = 0, i;
> +	u8 vic_len;
> +
> +	if (len < 8)
> +		goto out;

We skip the 'tag/len' byte for do_cea_modes(), but here we include it.
The resulting indexing is maybe a bit easier to compare w/ the spec,
but one must always keep in mind that the payload length doesn't include
byte 0. Not sure if we should just increase len by 1, or skip the tag
byte here too. Or maybe just add a comment explaining the situation.

> +
> +	/* 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;
> +

Maybe do the final 'offset += 2' already here? It would mean we could
say '8 + offset' below, which would somewhat nicely match the fact that
there are 8 fixed bytes in the payload, and the rest are shifty.

> +	/* the declared length is not long enough for the 2 first bytes
> +	 * of additional video format capabilities */
> +	if (len < (10 + offset))
> +		goto out;
> +
> +	vic_len =  db[10 + offset] >> 5;
> +	offset += 2;
> +
> +	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 +2579,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 +2616,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);

>  		}
>  	}
>  
> @@ -2570,21 +2670,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
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC