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

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