[PATCH] drm: disable deep color when EDID violates spec

[Nicholas Sielicki <nicholas.sielicki@gmail.com>]

As per the HDMI 1.3 and 1.4 spec, "deep color modes" are color depths
greater than 24 bits per pixel. The spec explicitly states, "All Deep
Color modes are optional though if an HDMI Source or Sink supports any
Deep Color mode, it shall support 36-bit mode." (6.2.4 Color Depth
Requirements).

I came across a monitor (Acer X233H) that supplies an illegal EDID where
DC_30bit is set and DC_36bit is not set. The end result is badly garbled
output because the driver is sending 36BPP when the monitor can't handle
it.

Much of the intel hardware is incapable of operating at any
bit-per-component setting outside of 8 or 12, and the spec seems to
imply that if any deep color support is found, then it is a safe
assumption to operate at 12.

This patch ensures that the EDID is within the spec (specifically, that
DC_36bit is set) before committing to going forward with any deep
colors.  There was already a check for this EDID inconsistency, but it
resulted only in a warning and did not fall-back to safer settings.

CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Nicholas Sielicki <nicholas.sielicki@gmail.com>
---
 drivers/gpu/drm/drm_edid.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 336be31ff3de..42ce3f54d2dc 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3772,30 +3772,34 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
 {
 	struct drm_display_info *info = &connector->display_info;
 	unsigned int dc_bpc = 0;
+	u8 modes = 0;
 
 	/* HDMI supports at least 8 bpc */
 	info->bpc = 8;
 
+	/* Ensure all DC modes are unset if we return early */
+	info->edid_hdmi_dc_modes = 0;
+
 	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;
+		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;
+		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;
+		modes |= DRM_EDID_HDMI_DC_48;
 		DRM_DEBUG("%s: HDMI sink does deep color 48.\n",
 			  connector->name);
 	}
@@ -3806,9 +3810,24 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
 		return;
 	}
 
+	/*
+	 * All deep color modes are optional, but if a sink supports any deep
+	 * color mode, it must support 36-bit mode. If this is found not
+	 * to be the case, sink is in violation of HDMI 1.3 / 1.4 spec and it
+	 * is prudent to disable all deep color modes. Return here before
+	 * committing bpc and edid_hdmi_dc_modes.
+	 */
+	if (!(modes & DRM_EDID_HDMI_DC_36)) {
+		DRM_DEBUG("%s: HDMI sink should do DC_36, but does not!\n",
+			  connector->name);
+		return;
+	}
+
+
 	DRM_DEBUG("%s: Assigning HDMI sink color depth as %d bpc.\n",
 		  connector->name, dc_bpc);
 	info->bpc = dc_bpc;
+	info->edid_hdmi_dc_modes = modes;
 
 	/*
 	 * Deep color support mandates RGB444 support for all video
@@ -3823,15 +3842,6 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
 		DRM_DEBUG("%s: HDMI sink does YCRCB444 in deep color.\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);
-	}
 }
 
 static void
@@ -3895,6 +3905,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->edid_hdmi_dc_modes = 0;
 	info->cea_rev = 0;
 	info->max_tmds_clock = 0;
 	info->dvi_dual = false;
-- 
2.11.0

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