Attachment 128785 Details for Bug 99250 – drm: disable deep color when EDID violates spec

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

0001-drm-disable-deep-color-when-EDID-violates-spec.patch (text/plain), 4.14 KB, created by Nicholas Sielicki on 2017-01-05 21:43:31 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nicholas Sielicki

Created: 2017-01-05 21:43:31 UTC

Size: 4.14 KB

patch

obsolete

>From e2547a884b4c1949d6be7a3fa8019b3e0f519e19 Mon Sep 17 00:00:00 2001
>From: Nicholas Sielicki <sielicki@yandex.com>
>Date: Wed, 4 Jan 2017 17:26:47 -0600
>Subject: [PATCH] drm: disable deep color when EDID violates spec
>
>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.
>
>Signed-off-by: Nicholas Sielicki <sielicki@yandex.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
>

Actions: View | Splinter Review

Attachments on bug 99250: 128711 | 128712 | 128761 | 128762 | 128785

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.