[PATCH 00/10] drm/edid: Clean up display_info stuff

[PATCH 00/10] drm/edid: Clean up display_info stuff

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

This series aims to clean up the way we fill out the display_info 
structure a bit. Mainly doing a clean separation of the audio and
video related bits.

My aim is getting i915 to respect the HDMI sink TMDS clock limit
(currently we respect only the DP++ dongle limit, and the source
limit). But while doing that, I noticed that the display_info stuff
was a bit of mess, hence it resulted in this bigger series.

Entire series available here:
git://github.com/vsyrjala/linux.git hdmi_sink_tmds_limit_3

Ville Syrjälä (10):
  drm/edid: Clear old audio latency values before parsing the new EDID
  drm/edid: Clear old dvi_dual/max_tmds_clock before parsing the new
    EDID
  drm/edid: Make max_tmds_clock kHz instead of MHz
  drm/edid: Move dvi_dual/max_tmds_clock to drm_display_info
  drm/edid: Don't pass around drm_display_info needlessly
  drm/edid: Reduce the number of times we parse the CEA extension block
  drm/edid: Clear the old cea_rev when there's no CEA extension in the
    new EDID
  drm/edid: Move dvi_dual/max_tmds_clock parsing out from
    drm_edid_to_eld()
  drm/i915: Replace a bunch of connector->base.display_info with a local
    variable
  drm/i915: Account for sink max TMDS clock when checking the port clock

 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c |   4 +-
 drivers/gpu/drm/drm_edid.c                     | 258 +++++++++++++------------
 drivers/gpu/drm/i915/intel_display.c           |  14 +-
 drivers/gpu/drm/i915/intel_hdmi.c              |   9 +-
 drivers/gpu/drm/radeon/radeon_connectors.c     |   4 +-
 include/drm/drm_crtc.h                         |   7 +-
 6 files changed, 152 insertions(+), 144 deletions(-)

-- 
2.7.4

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

[PATCH 01/10] drm/edid: Clear old audio latency values before parsing the new EDID

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Clear out stale audio latency information (potentially from a previous
EDID) before constructing the ELD from the EDID.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7df26d4b7ad8..05ef93c8fa44 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3353,6 +3353,13 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 
 	memset(eld, 0, sizeof(connector->eld));
 
+	connector->latency_present[0] = false;
+	connector->latency_present[1] = false;
+	connector->video_latency[0] = 0;
+	connector->audio_latency[0] = 0;
+	connector->video_latency[1] = 0;
+	connector->audio_latency[1] = 0;
+
 	cea = drm_find_cea_extension(edid);
 	if (!cea) {
 		DRM_DEBUG_KMS("ELD: no CEA Extension found\n");
-- 
2.7.4

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

[PATCH 02/10] drm/edid: Clear old dvi_dual/max_tmds_clock before parsing the new EDID

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Clear out old max_tmds_clock and dvi_dual information (possibly from a
previous EDID) before parsing the current EDID. Tne current EDID might
not even have these in its HDMI VSDB, which would mean that we'd leave
the old stale values in place.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 05ef93c8fa44..40e7f1a863a6 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3360,6 +3360,9 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 	connector->video_latency[1] = 0;
 	connector->audio_latency[1] = 0;
 
+	connector->max_tmds_clock = 0;
+	connector->dvi_dual = false;
+
 	cea = drm_find_cea_extension(edid);
 	if (!cea) {
 		DRM_DEBUG_KMS("ELD: no CEA Extension found\n");
-- 
2.7.4

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

[PATCH 03/10] drm/edid: Make max_tmds_clock kHz instead of MHz

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We generally store clocks in kHz, so let's do that for the
HDMI max TMDS clock value as well. Less surpising.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 2 +-
 drivers/gpu/drm/drm_edid.c                     | 2 +-
 drivers/gpu/drm/radeon/radeon_connectors.c     | 2 +-
 include/drm/drm_crtc.h                         | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index ff0b55a65ca3..a56eeaa213f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -173,7 +173,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector *connector)
 			mode_clock = amdgpu_connector->pixelclock_for_modeset;
 
 			/* Maximum allowable input clock in kHz */
-			max_tmds_clock = connector->max_tmds_clock * 1000;
+			max_tmds_clock = connector->max_tmds_clock;
 
 			DRM_DEBUG("%s: hdmi mode dotclock %d kHz, max tmds input clock %d kHz.\n",
 				  connector->name, mode_clock, max_tmds_clock);
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 40e7f1a863a6..f0f08f56ea97 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3257,7 +3257,7 @@ parse_hdmi_vsdb(struct drm_connector *connector, const u8 *db)
 		connector->dvi_dual = db[6] & 1;
 	}
 	if (len >= 7)
-		connector->max_tmds_clock = db[7] * 5;
+		connector->max_tmds_clock = db[7] * 5000;
 	if (len >= 8) {
 		connector->latency_present[0] = db[8] >> 7;
 		connector->latency_present[1] = (db[8] >> 6) & 1;
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index b79f3b002471..db5488732e31 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -203,7 +203,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
 			mode_clock = radeon_connector->pixelclock_for_modeset;
 
 			/* Maximum allowable input clock in kHz */
-			max_tmds_clock = connector->max_tmds_clock * 1000;
+			max_tmds_clock = connector->max_tmds_clock;
 
 			DRM_DEBUG("%s: hdmi mode dotclock %d kHz, max tmds input clock %d kHz.\n",
 					  connector->name, mode_clock, max_tmds_clock);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 3edeaf88ebc0..e7cf08d9df10 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1366,7 +1366,7 @@ struct drm_connector {
 	/* EDID bits */
 	uint8_t eld[MAX_ELD_BYTES];
 	bool dvi_dual;
-	int max_tmds_clock;	/* in MHz */
+	int max_tmds_clock;	/* in kHz */
 	bool latency_present[2];
 	int video_latency[2];	/* [0]: progressive, [1]: interlaced */
 	int audio_latency[2];
-- 
2.7.4

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

[PATCH 04/10] drm/edid: Move dvi_dual/max_tmds_clock to drm_display_info

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We have the drm_display_info for storing information about the sink, so
let's move dvi_dual and max_tmds_clock in there.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c |  4 ++--
 drivers/gpu/drm/drm_edid.c                     | 14 ++++++++------
 drivers/gpu/drm/radeon/radeon_connectors.c     |  4 ++--
 include/drm/drm_crtc.h                         |  7 +++----
 4 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index a56eeaa213f4..3df2f6a56b46 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -168,12 +168,12 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector *connector)
 		}
 
 		/* Any defined maximum tmds clock limit we must not exceed? */
-		if (connector->max_tmds_clock > 0) {
+		if (connector->display_info.max_tmds_clock > 0) {
 			/* mode_clock is clock in kHz for mode to be modeset on this connector */
 			mode_clock = amdgpu_connector->pixelclock_for_modeset;
 
 			/* Maximum allowable input clock in kHz */
-			max_tmds_clock = connector->max_tmds_clock;
+			max_tmds_clock = connector->display_info.max_tmds_clock;
 
 			DRM_DEBUG("%s: hdmi mode dotclock %d kHz, max tmds input clock %d kHz.\n",
 				  connector->name, mode_clock, max_tmds_clock);
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index f0f08f56ea97..abd6d3c77ef2 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3250,14 +3250,15 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
 static void
 parse_hdmi_vsdb(struct drm_connector *connector, const u8 *db)
 {
+	struct drm_display_info *info = &connector->display_info;
 	u8 len = cea_db_payload_len(db);
 
 	if (len >= 6) {
 		connector->eld[5] |= (db[6] >> 7) << 1;  /* Supports_AI */
-		connector->dvi_dual = db[6] & 1;
+		info->dvi_dual = db[6] & 1;
 	}
 	if (len >= 7)
-		connector->max_tmds_clock = db[7] * 5000;
+		info->max_tmds_clock = db[7] * 5000;
 	if (len >= 8) {
 		connector->latency_present[0] = db[8] >> 7;
 		connector->latency_present[1] = (db[8] >> 6) & 1;
@@ -3276,8 +3277,8 @@ parse_hdmi_vsdb(struct drm_connector *connector, const u8 *db)
 		    "latency present %d %d, "
 		    "video latency %d %d, "
 		    "audio latency %d %d\n",
-		    connector->dvi_dual,
-		    connector->max_tmds_clock,
+		    info->dvi_dual,
+		    info->max_tmds_clock,
 	      (int) connector->latency_present[0],
 	      (int) connector->latency_present[1],
 		    connector->video_latency[0],
@@ -3344,6 +3345,7 @@ EXPORT_SYMBOL(drm_edid_get_monitor_name);
  */
 void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 {
+	struct drm_display_info *info = &connector->display_info;
 	uint8_t *eld = connector->eld;
 	u8 *cea;
 	u8 *db;
@@ -3360,8 +3362,8 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 	connector->video_latency[1] = 0;
 	connector->audio_latency[1] = 0;
 
-	connector->max_tmds_clock = 0;
-	connector->dvi_dual = false;
+	info->max_tmds_clock = 0;
+	info->dvi_dual = false;
 
 	cea = drm_find_cea_extension(edid);
 	if (!cea) {
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index db5488732e31..50e96d2c593d 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -198,12 +198,12 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
 		}
 
 		/* Any defined maximum tmds clock limit we must not exceed? */
-		if (connector->max_tmds_clock > 0) {
+		if (connector->display_info.max_tmds_clock > 0) {
 			/* mode_clock is clock in kHz for mode to be modeset on this connector */
 			mode_clock = radeon_connector->pixelclock_for_modeset;
 
 			/* Maximum allowable input clock in kHz */
-			max_tmds_clock = connector->max_tmds_clock;
+			max_tmds_clock = connector->display_info.max_tmds_clock;
 
 			DRM_DEBUG("%s: hdmi mode dotclock %d kHz, max tmds input clock %d kHz.\n",
 					  connector->name, mode_clock, max_tmds_clock);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index e7cf08d9df10..b1c54b322b4f 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -150,6 +150,9 @@ struct drm_display_info {
 	unsigned int num_bus_formats;
 	u32 bus_flags;
 
+	int max_tmds_clock;	/* in kHz */
+	bool dvi_dual;
+
 	/* Mask of supported hdmi deep color modes */
 	u8 edid_hdmi_dc_modes;
 
@@ -1268,8 +1271,6 @@ struct drm_encoder {
  * @encoder_ids: valid encoders for this connector
  * @encoder: encoder driving this connector, if any
  * @eld: EDID-like data, if present
- * @dvi_dual: dual link DVI, if found
- * @max_tmds_clock: max clock rate, if found
  * @latency_present: AV delay info from ELD, if found
  * @video_latency: video latency info from ELD, if found
  * @audio_latency: audio latency info from ELD, if found
@@ -1365,8 +1366,6 @@ struct drm_connector {
 
 	/* EDID bits */
 	uint8_t eld[MAX_ELD_BYTES];
-	bool dvi_dual;
-	int max_tmds_clock;	/* in kHz */
 	bool latency_present[2];
 	int video_latency[2];	/* [0]: progressive, [1]: interlaced */
 	int audio_latency[2];
-- 
2.7.4

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

[PATCH 05/10] drm/edid: Don't pass around drm_display_info needlessly

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We already pass the connector to drm_add_display_info() and
drm_assign_hdmi_deep_color_info(), so passing the
connector->display_info also is pointless.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index abd6d3c77ef2..d8c7da56ab75 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3732,17 +3732,15 @@ EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
  * drm_assign_hdmi_deep_color_info - detect whether monitor supports
  * hdmi deep color modes and update drm_display_info if so.
  * @edid: monitor EDID information
- * @info: Updated with maximum supported deep color bpc and color format
- *        if deep color supported.
  * @connector: DRM connector, used only for debug output
  *
  * Parse the CEA extension according to CEA-861-B.
  * Return true if HDMI deep color supported, false if not or unknown.
  */
 static bool drm_assign_hdmi_deep_color_info(struct edid *edid,
-                                            struct drm_display_info *info,
                                             struct drm_connector *connector)
 {
+	struct drm_display_info *info = &connector->display_info;
 	u8 *edid_ext, *hdmi;
 	int i;
 	int start_offset, end_offset;
@@ -3832,7 +3830,6 @@ static bool drm_assign_hdmi_deep_color_info(struct edid *edid,
 /**
  * drm_add_display_info - pull display info out if present
  * @edid: EDID data
- * @info: display info (attached to connector)
  * @connector: connector whose edid is used to build display info
  *
  * Grab any available display info and stuff it into the drm_display_info
@@ -3840,9 +3837,9 @@ static bool drm_assign_hdmi_deep_color_info(struct edid *edid,
  * color spaces.
  */
 static void drm_add_display_info(struct edid *edid,
-                                 struct drm_display_info *info,
                                  struct drm_connector *connector)
 {
+	struct drm_display_info *info = &connector->display_info;
 	u8 *edid_ext;
 
 	info->width_mm = edid->width_cm * 10;
@@ -3872,7 +3869,7 @@ static void drm_add_display_info(struct edid *edid,
 	}
 
 	/* HDMI deep color modes supported? Assign to info, if so */
-	drm_assign_hdmi_deep_color_info(edid, info, connector);
+	drm_assign_hdmi_deep_color_info(edid, connector);
 
 	/* Only defined for 1.4 with digital displays */
 	if (edid->revision < 4)
@@ -4092,7 +4089,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
 	if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75))
 		edid_fixup_preferred(connector, quirks);
 
-	drm_add_display_info(edid, &connector->display_info, connector);
+	drm_add_display_info(edid, connector);
 
 	if (quirks & EDID_QUIRK_FORCE_8BPC)
 		connector->display_info.bpc = 8;
-- 
2.7.4

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

[PATCH 06/10] drm/edid: Reduce the number of times we parse the CEA extension block

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Instead of parsing parts of the CEA extension block in two places
to determine supported color formats and whatnot, let's just
consolidate it to one function. This also makes it possible to neatly
flatten drm_assign_hdmi_deep_color_info().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 192 +++++++++++++++++++++------------------------
 1 file changed, 89 insertions(+), 103 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index d8c7da56ab75..82ab57c4d6c9 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3728,119 +3728,119 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
 }
 EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
 
-/**
- * drm_assign_hdmi_deep_color_info - detect whether monitor supports
- * hdmi deep color modes and update drm_display_info if so.
- * @edid: monitor EDID information
- * @connector: DRM connector, used only for debug output
- *
- * Parse the CEA extension according to CEA-861-B.
- * Return true if HDMI deep color supported, false if not or unknown.
- */
-static bool drm_assign_hdmi_deep_color_info(struct edid *edid,
-                                            struct drm_connector *connector)
+static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
+					   const u8 *hdmi)
 {
 	struct drm_display_info *info = &connector->display_info;
-	u8 *edid_ext, *hdmi;
-	int i;
-	int start_offset, end_offset;
 	unsigned int dc_bpc = 0;
 
-	edid_ext = drm_find_cea_extension(edid);
-	if (!edid_ext)
-		return false;
+	/* HDMI supports at least 8 bpc */
+	info->bpc = 8;
 
-	if (cea_db_offsets(edid_ext, &start_offset, &end_offset))
-		return false;
+	if (cea_db_payload_len(hdmi) < 6)
+		return;
+
+	if (hdmi[6] & DRM_EDID_HDMI_DC_30) {
+		dc_bpc = 10;
+		info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30;
+		DRM_DEBUG("%s: HDMI sink does deep color 30.\n",
+			  connector->name);
+	}
+
+	if (hdmi[6] & DRM_EDID_HDMI_DC_36) {
+		dc_bpc = 12;
+		info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36;
+		DRM_DEBUG("%s: HDMI sink does deep color 36.\n",
+			  connector->name);
+	}
+
+	if (hdmi[6] & DRM_EDID_HDMI_DC_48) {
+		dc_bpc = 16;
+		info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48;
+		DRM_DEBUG("%s: HDMI sink does deep color 48.\n",
+			  connector->name);
+	}
+
+	if (dc_bpc == 0) {
+		DRM_DEBUG("%s: No deep color support on this HDMI sink.\n",
+			  connector->name);
+		return;
+	}
+
+	DRM_DEBUG("%s: Assigning HDMI sink color depth as %d bpc.\n",
+		  connector->name, dc_bpc);
+	info->bpc = dc_bpc;
 
 	/*
-	 * Because HDMI identifier is in Vendor Specific Block,
-	 * search it from all data blocks of CEA extension.
+	 * Deep color support mandates RGB444 support for all video
+	 * modes and forbids YCRCB422 support for all video modes per
+	 * HDMI 1.3 spec.
 	 */
-	for_each_cea_db(edid_ext, i, start_offset, end_offset) {
-		if (cea_db_is_hdmi_vsdb(&edid_ext[i])) {
-			/* HDMI supports at least 8 bpc */
-			info->bpc = 8;
-
-			hdmi = &edid_ext[i];
-			if (cea_db_payload_len(hdmi) < 6)
-				return false;
-
-			if (hdmi[6] & DRM_EDID_HDMI_DC_30) {
-				dc_bpc = 10;
-				info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30;
-				DRM_DEBUG("%s: HDMI sink does deep color 30.\n",
-						  connector->name);
-			}
+	info->color_formats = DRM_COLOR_FORMAT_RGB444;
 
-			if (hdmi[6] & DRM_EDID_HDMI_DC_36) {
-				dc_bpc = 12;
-				info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36;
-				DRM_DEBUG("%s: HDMI sink does deep color 36.\n",
-						  connector->name);
-			}
+	/* YCRCB444 is optional according to spec. */
+	if (hdmi[6] & DRM_EDID_HDMI_DC_Y444) {
+		info->color_formats |= DRM_COLOR_FORMAT_YCRCB444;
+		DRM_DEBUG("%s: HDMI sink does YCRCB444 in deep color.\n",
+			  connector->name);
+	}
 
-			if (hdmi[6] & DRM_EDID_HDMI_DC_48) {
-				dc_bpc = 16;
-				info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48;
-				DRM_DEBUG("%s: HDMI sink does deep color 48.\n",
-						  connector->name);
-			}
+	/*
+	 * Spec says that if any deep color mode is supported at all,
+	 * then deep color 36 bit must be supported.
+	 */
+	if (!(hdmi[6] & DRM_EDID_HDMI_DC_36)) {
+		DRM_DEBUG("%s: HDMI sink should do DC_36, but does not!\n",
+			  connector->name);
+	}
+}
 
-			if (dc_bpc > 0) {
-				DRM_DEBUG("%s: Assigning HDMI sink color depth as %d bpc.\n",
-						  connector->name, dc_bpc);
-				info->bpc = dc_bpc;
-
-				/*
-				 * Deep color support mandates RGB444 support for all video
-				 * modes and forbids YCRCB422 support for all video modes per
-				 * HDMI 1.3 spec.
-				 */
-				info->color_formats = DRM_COLOR_FORMAT_RGB444;
-
-				/* YCRCB444 is optional according to spec. */
-				if (hdmi[6] & DRM_EDID_HDMI_DC_Y444) {
-					info->color_formats |= DRM_COLOR_FORMAT_YCRCB444;
-					DRM_DEBUG("%s: HDMI sink does YCRCB444 in deep color.\n",
-							  connector->name);
-				}
+static void drm_parse_cea_ext(struct drm_connector *connector,
+			      struct edid *edid)
+{
+	struct drm_display_info *info = &connector->display_info;
+	const u8 *edid_ext;
+	int i, start, end;
 
-				/*
-				 * Spec says that if any deep color mode is supported at all,
-				 * then deep color 36 bit must be supported.
-				 */
-				if (!(hdmi[6] & DRM_EDID_HDMI_DC_36)) {
-					DRM_DEBUG("%s: HDMI sink should do DC_36, but does not!\n",
-							  connector->name);
-				}
+	edid_ext = drm_find_cea_extension(edid);
+	if (!edid_ext)
+		return;
 
-				return true;
-			}
-			else {
-				DRM_DEBUG("%s: No deep color support on this HDMI sink.\n",
-						  connector->name);
-			}
-		}
-	}
+	info->cea_rev = edid_ext[1];
 
-	return false;
+	/* The existence of a CEA block should imply RGB support */
+	info->color_formats = DRM_COLOR_FORMAT_RGB444;
+	if (edid_ext[3] & EDID_CEA_YCRCB444)
+		info->color_formats |= DRM_COLOR_FORMAT_YCRCB444;
+	if (edid_ext[3] & EDID_CEA_YCRCB422)
+		info->color_formats |= DRM_COLOR_FORMAT_YCRCB422;
+
+	if (cea_db_offsets(edid_ext, &start, &end))
+		return;
+
+	for_each_cea_db(edid_ext, i, start, end) {
+		const u8 *db = &edid_ext[i];
+
+		if (!cea_db_is_hdmi_vsdb(db))
+			continue;
+
+		drm_parse_hdmi_deep_color_info(connector, db);
+	}
 }
 
 /**
  * drm_add_display_info - pull display info out if present
- * @edid: EDID data
  * @connector: connector whose edid is used to build display info
+ * @edid: EDID data
  *
  * Grab any available display info and stuff it into the drm_display_info
  * structure that's part of the connector.  Useful for tracking bpp and
  * color spaces.
  */
-static void drm_add_display_info(struct edid *edid,
-                                 struct drm_connector *connector)
+static void drm_add_display_info(struct drm_connector *connector,
+				 struct edid *edid)
 {
 	struct drm_display_info *info = &connector->display_info;
-	u8 *edid_ext;
 
 	info->width_mm = edid->width_cm * 10;
 	info->height_mm = edid->height_cm * 10;
@@ -3855,21 +3855,7 @@ static void drm_add_display_info(struct edid *edid,
 	if (!(edid->input & DRM_EDID_INPUT_DIGITAL))
 		return;
 
-	/* Get data from CEA blocks if present */
-	edid_ext = drm_find_cea_extension(edid);
-	if (edid_ext) {
-		info->cea_rev = edid_ext[1];
-
-		/* The existence of a CEA block should imply RGB support */
-		info->color_formats = DRM_COLOR_FORMAT_RGB444;
-		if (edid_ext[3] & EDID_CEA_YCRCB444)
-			info->color_formats |= DRM_COLOR_FORMAT_YCRCB444;
-		if (edid_ext[3] & EDID_CEA_YCRCB422)
-			info->color_formats |= DRM_COLOR_FORMAT_YCRCB422;
-	}
-
-	/* HDMI deep color modes supported? Assign to info, if so */
-	drm_assign_hdmi_deep_color_info(edid, connector);
+	drm_parse_cea_ext(connector, edid);
 
 	/* Only defined for 1.4 with digital displays */
 	if (edid->revision < 4)
@@ -4089,7 +4075,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
 	if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75))
 		edid_fixup_preferred(connector, quirks);
 
-	drm_add_display_info(edid, connector);
+	drm_add_display_info(connector, edid);
 
 	if (quirks & EDID_QUIRK_FORCE_8BPC)
 		connector->display_info.bpc = 8;
-- 
2.7.4

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

[PATCH 07/10] drm/edid: Clear the old cea_rev when there's no CEA extension in the new EDID

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

It's not a good idea to leave stale cea_rev in the drm_display_info. The
current EDID might not even have a CEA ext block in which case we'd end
up leaving the stale value in place.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 82ab57c4d6c9..452e8c51a729 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3848,6 +3848,7 @@ static void drm_add_display_info(struct drm_connector *connector,
 	/* driver figures it out in this case */
 	info->bpc = 0;
 	info->color_formats = 0;
+	info->cea_rev = 0;
 
 	if (edid->revision < 3)
 		return;
-- 
2.7.4

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

[PATCH 08/10] drm/edid: Move dvi_dual/max_tmds_clock parsing out from drm_edid_to_eld()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

drm_edid_to_eld() is just mean to cook up the ELD for the audio driver,
so having it parse non-audio related stuff seems just wrong, and
potentially could lead to that information not being even filled out
if the function doesn't even get called. Let's move that stuff to the
place where we parse the color formats and whatnot from the CEA ext
block.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 64 +++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 452e8c51a729..3cf8e44e040d 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3248,17 +3248,12 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
 }
 
 static void
-parse_hdmi_vsdb(struct drm_connector *connector, const u8 *db)
+drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db)
 {
-	struct drm_display_info *info = &connector->display_info;
 	u8 len = cea_db_payload_len(db);
 
-	if (len >= 6) {
+	if (len >= 6)
 		connector->eld[5] |= (db[6] >> 7) << 1;  /* Supports_AI */
-		info->dvi_dual = db[6] & 1;
-	}
-	if (len >= 7)
-		info->max_tmds_clock = db[7] * 5000;
 	if (len >= 8) {
 		connector->latency_present[0] = db[8] >> 7;
 		connector->latency_present[1] = (db[8] >> 6) & 1;
@@ -3272,19 +3267,15 @@ parse_hdmi_vsdb(struct drm_connector *connector, const u8 *db)
 	if (len >= 12)
 		connector->audio_latency[1] = db[12];
 
-	DRM_DEBUG_KMS("HDMI: DVI dual %d, "
-		    "max TMDS clock %d, "
-		    "latency present %d %d, "
-		    "video latency %d %d, "
-		    "audio latency %d %d\n",
-		    info->dvi_dual,
-		    info->max_tmds_clock,
-	      (int) connector->latency_present[0],
-	      (int) connector->latency_present[1],
-		    connector->video_latency[0],
-		    connector->video_latency[1],
-		    connector->audio_latency[0],
-		    connector->audio_latency[1]);
+	DRM_DEBUG_KMS("HDMI: latency present %d %d, "
+		      "video latency %d %d, "
+		      "audio latency %d %d\n",
+		      connector->latency_present[0],
+		      connector->latency_present[1],
+		      connector->video_latency[0],
+		      connector->video_latency[1],
+		      connector->audio_latency[0],
+		      connector->audio_latency[1]);
 }
 
 static void
@@ -3345,7 +3336,6 @@ EXPORT_SYMBOL(drm_edid_get_monitor_name);
  */
 void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 {
-	struct drm_display_info *info = &connector->display_info;
 	uint8_t *eld = connector->eld;
 	u8 *cea;
 	u8 *db;
@@ -3362,9 +3352,6 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 	connector->video_latency[1] = 0;
 	connector->audio_latency[1] = 0;
 
-	info->max_tmds_clock = 0;
-	info->dvi_dual = false;
-
 	cea = drm_find_cea_extension(edid);
 	if (!cea) {
 		DRM_DEBUG_KMS("ELD: no CEA Extension found\n");
@@ -3414,7 +3401,7 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 			case VENDOR_BLOCK:
 				/* HDMI Vendor-Specific Data Block */
 				if (cea_db_is_hdmi_vsdb(db))
-					parse_hdmi_vsdb(connector, db);
+					drm_parse_hdmi_vsdb_audio(connector, db);
 				break;
 			default:
 				break;
@@ -3795,6 +3782,25 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
 	}
 }
 
+static void
+drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db)
+{
+	struct drm_display_info *info = &connector->display_info;
+	u8 len = cea_db_payload_len(db);
+
+	if (len >= 6)
+		info->dvi_dual = db[6] & 1;
+	if (len >= 7)
+		info->max_tmds_clock = db[7] * 5000;
+
+	DRM_DEBUG_KMS("HDMI: DVI dual %d, "
+		      "max TMDS clock %d kHz\n",
+		      info->dvi_dual,
+		      info->max_tmds_clock);
+
+	drm_parse_hdmi_deep_color_info(connector, db);
+}
+
 static void drm_parse_cea_ext(struct drm_connector *connector,
 			      struct edid *edid)
 {
@@ -3821,10 +3827,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
 	for_each_cea_db(edid_ext, i, start, end) {
 		const u8 *db = &edid_ext[i];
 
-		if (!cea_db_is_hdmi_vsdb(db))
-			continue;
-
-		drm_parse_hdmi_deep_color_info(connector, db);
+		if (cea_db_is_hdmi_vsdb(db))
+			drm_parse_hdmi_vsdb_video(connector, db);
 	}
 }
 
@@ -3849,6 +3853,8 @@ static void drm_add_display_info(struct drm_connector *connector,
 	info->bpc = 0;
 	info->color_formats = 0;
 	info->cea_rev = 0;
+	info->max_tmds_clock = 0;
+	info->dvi_dual = false;
 
 	if (edid->revision < 3)
 		return;
-- 
2.7.4

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

[PATCH 09/10] drm/i915: Replace a bunch of connector->base.display_info with a local variable

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reduce the eyesore with a local variable.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a8e8cc8dfae9..b9cdf9060da6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12090,22 +12090,22 @@ static void
 connected_sink_compute_bpp(struct intel_connector *connector,
 			   struct intel_crtc_state *pipe_config)
 {
+	const struct drm_display_info *info = &connector->base.display_info;
 	int bpp = pipe_config->pipe_bpp;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] checking for sink bpp constrains\n",
-		connector->base.base.id,
-		connector->base.name);
+		      connector->base.base.id,
+		      connector->base.name);
 
 	/* Don't use an invalid EDID bpc value */
-	if (connector->base.display_info.bpc &&
-	    connector->base.display_info.bpc * 3 < bpp) {
+	if (info->bpc != 0 && info->bpc * 3 < bpp) {
 		DRM_DEBUG_KMS("clamping display bpp (was %d) to EDID reported max of %d\n",
-			      bpp, connector->base.display_info.bpc*3);
-		pipe_config->pipe_bpp = connector->base.display_info.bpc*3;
+			      bpp, info->bpc * 3);
+		pipe_config->pipe_bpp = info->bpc * 3;
 	}
 
 	/* Clamp bpp to default limit on screens without EDID 1.4 */
-	if (connector->base.display_info.bpc == 0) {
+	if (info->bpc == 0) {
 		int type = connector->base.connector_type;
 		int clamp_bpp = 24;
 
-- 
2.7.4

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

[PATCH 10/10] drm/i915: Account for sink max TMDS clock when checking the port clock

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

It's perfectly legal for the sink to support 12bpc only for
some lower resolution modes, while the higher resolution modes
can only be used with 8bpc. So let's take the sink's max TMDS clock
into account before we go and decide that a particular mode can
be used with 12bpc.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 4df9f384910c..8b013301cf11 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1204,10 +1204,17 @@ static int hdmi_port_clock_limit(struct intel_hdmi *hdmi,
 	int max_tmds_clock = intel_hdmi_source_max_tmds_clock(to_i915(dev));
 
 	if (respect_downstream_limits) {
+		struct intel_connector *connector = hdmi->attached_connector;
+		const struct drm_display_info *info = &connector->base.display_info;
+
 		if (hdmi->dp_dual_mode.max_tmds_clock)
 			max_tmds_clock = min(max_tmds_clock,
 					     hdmi->dp_dual_mode.max_tmds_clock);
-		if (!hdmi->has_hdmi_sink)
+
+		if (info->max_tmds_clock)
+			max_tmds_clock = min(max_tmds_clock,
+					     info->max_tmds_clock);
+		else if (!hdmi->has_hdmi_sink)
 			max_tmds_clock = min(max_tmds_clock, 165000);
 	}
 
-- 
2.7.4

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

Re: [PATCH 00/10] drm/edid: Clean up display_info stuff

[Christian König]

Looks reasonable to me, adding a few more AMD people which probably want 
to take a look as well.

Patch #3 and #4 are Reviewed-by: Christian König 
<christian.koenig@amd.com> since they touch the radeon/amdgpu driver.

Patch #1, #2 and #5 - #8 are Acked-by: Christian König 
<christian.koenig@amd.com>.

Regards,
Christian.

Am 03.08.2016 um 08:33 schrieb ville.syrjala@linux.intel.com:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> This series aims to clean up the way we fill out the display_info
> structure a bit. Mainly doing a clean separation of the audio and
> video related bits.
>
> My aim is getting i915 to respect the HDMI sink TMDS clock limit
> (currently we respect only the DP++ dongle limit, and the source
> limit). But while doing that, I noticed that the display_info stuff
> was a bit of mess, hence it resulted in this bigger series.
>
> Entire series available here:
> git://github.com/vsyrjala/linux.git hdmi_sink_tmds_limit_3
>
> Ville Syrjälä (10):
>    drm/edid: Clear old audio latency values before parsing the new EDID
>    drm/edid: Clear old dvi_dual/max_tmds_clock before parsing the new
>      EDID
>    drm/edid: Make max_tmds_clock kHz instead of MHz
>    drm/edid: Move dvi_dual/max_tmds_clock to drm_display_info
>    drm/edid: Don't pass around drm_display_info needlessly
>    drm/edid: Reduce the number of times we parse the CEA extension block
>    drm/edid: Clear the old cea_rev when there's no CEA extension in the
>      new EDID
>    drm/edid: Move dvi_dual/max_tmds_clock parsing out from
>      drm_edid_to_eld()
>    drm/i915: Replace a bunch of connector->base.display_info with a local
>      variable
>    drm/i915: Account for sink max TMDS clock when checking the port clock
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c |   4 +-
>   drivers/gpu/drm/drm_edid.c                     | 258 +++++++++++++------------
>   drivers/gpu/drm/i915/intel_display.c           |  14 +-
>   drivers/gpu/drm/i915/intel_hdmi.c              |   9 +-
>   drivers/gpu/drm/radeon/radeon_connectors.c     |   4 +-
>   include/drm/drm_crtc.h                         |   7 +-
>   6 files changed, 152 insertions(+), 144 deletions(-)
>

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