On 2017-01-10 03:41 PM, Harry Wentland wrote:
On 2017-01-10 03:10 PM, Alex Deucher wrote:
On Tue, Jan 10, 2017 at 3:02 PM, Ernst Sjöstrand <ernstp@xxxxxxxxx>
wrote:
I kindof assume DP is the default connection these days and with
Freesync
you use
DP or course, but this question was specifically for HDMI.
I guess this patch doesn't affect deep color over DP?
Anyway, only 17 of those monitors have FreeSync but almost all have
DP, so
perhaps they only support
10 bpc when connected with DP?
We see 10 bpc only over HDMI a lot apparently. I guess a lot of
monitor vendors do the minimum necessary for deep color.
Yes, apparently there are a bunch of HDMI displays that we drive in
10bpc that might or might not report 12bpc support. From talking to our
HDMI guys it sounds like this is a slightly ambiguous area in the spec,
despite what the HDMI spec quote mentioned by Nicholas said. I'll see if
I can get more info.
Adding Ernst, Nicholas, Ville, Alex again.
So apparently the spec is pretty clear and there's a sink compliance
test that should cover this. I don't really think it makes sense for the
source device to babysit the sink's behavior, though.
In general we might want to try for a solution that gives the best user
experience and highest interoperability, so check what sink supports,
check what source supports, check what cable can do, and do an
intersection of all to give the best user experience.
I suggest to block 10-bit on driver's that can't handle this.
Harry
I'm not sure it makes sense to block all deep color modes in this case
for all drivers. Other HW (e.g. AMD's) is perfectly capable of driving
10-bit. Would it make sense to just block this on the i915 side as Ville
alluded to on another thread?
Harry
Alex
Regards
//Ernst
2017-01-10 20:54 GMT+01:00 Alex Deucher <alexdeucher@xxxxxxxxx>:
On Tue, Jan 10, 2017 at 12:46 PM, Ville Syrjälä
<ville.syrjala@xxxxxxxxxxxxxxx> wrote:
On Tue, Jan 10, 2017 at 12:27:45PM -0500, Alex Deucher wrote:
On Tue, Jan 10, 2017 at 6:33 AM, Ernst Sjöstrand <ernstp@xxxxxxxxx>
wrote:
Isn't 10bpc very common among monitors, and 12bpc very rare? Or
maybe
I'm
confusing the transport layer with the presentation capabilities...?
Here are 201 monitors that claim 10bpc:
http://pricespy.co.uk/category.php?l=s300859434&o=eg_401#prodlista
FWIW, I've almost never seen a monitor that supports 12 bpc, but I've
see quite a few 10 bpc monitors.
I've seen plenty of monitors that do 10bpc over DP, but I've never
seen
anything that does 10bpc over HDMI. 12bpc over HDMI is pretty common
in "proper" TVs in my experience.
I can talk to our display team to
see what other OSes do.
Thanks. That should give us some idea if anyone ever tried 10bpc
over HDMI on these things.
We apparently see this pretty commonly, especially with freesync
monitors, and we support it. It seems to be pretty prevalent because
you can support a higher refresh rate with a lower bpc.
Alex
Alex
Regards
//Ernst
2017-01-10 11:52 GMT+01:00 Ville Syrjälä
<ville.syrjala@xxxxxxxxxxxxxxx>:
On Thu, Jan 05, 2017 at 05:45:23PM -0600, Nicholas Sielicki wrote:
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@xxxxxxxxxxxxxxx>
Signed-off-by: Nicholas Sielicki <nicholas.sielicki@xxxxxxxxx>
---
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;
Clearing this in drm_add_display_info() should be sufficient since
this guy doesn't get called from anywhere else. So this part could
be droppped.
Otherwise this feels like a decent way to handle the problem. We
could of course try to use the 10bpc (or whatever) deep color modes
the sink claims to support, but given that the people designing the
thing didn't bother reading the spec, it seems safer to just
disable
deep color support entirely.
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
+
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
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
