Skip to contentSkip to conversation

drm/i915/dp: Stop enabling limited color ranges for everything

PATCH In review Lyude 5 participants 6 messages Jan 5, 2017

Subsystems (2)
lore
Share & tools
lore

Lyude

author
#0 lore raw

PATCH 1/1 drm/i915/dp: Stop enabling limited color ranges for everything

Until now, it seems we've been erroneously enabling limited color ranges for the vast majority of DisplayPort monitors. I noticed this after writing a frame dump comparison test for the Chamelium and noticing that every i915 device I had was failing, while amdgpu machines were fine:

https://people.freedesktop.org/~lyudess/archive/01-03-2017/

It looks like this is because the DisplayPort spec tells us to use limited color ranges whenever we detect a CEA mode in use. However, from the looks of it there's another rather confusing part of the spec that got missed: source devices are allowed to use the full range of values for pixels -even- if the sink device declares that it's using a CEA mode. It's up to the sink device to limit the pixel range to the CEA ranges if it needs.

Cc Chris Wilson <[email protected]>
Cc Daniel Vetter <[email protected]>
Cc Jani Nikula <[email protected]>
Cc Ville Syrjälä <[email protected]>

---
I'm really surprised that this bug would have been around as long as it looks like it has been without anyone noticing it, so I figured I'd just send a patch to the mailing list so you guys can point out whether or not this is really the correct thing to do.

 drivers/gpu/drm/i915/intel_dp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_dp.c +5 -2
index d9bc19b..6642abd 100644
@@ -1649,12 +1649,15 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 found:
 	if (intel_dp->color_range_auto) {
 		/*
+		 * We are required to use the limited CEA RGB range when a CEA
+		 * mode is declared in the EDID. However, limiting the pixel
+		 * value range is up to the sink, not the source. So, just
+		 * don't enable limited color ranges.
 		 * See:
 		 * CEA-861-E - 5.1 Default Encoding Parameters
 		 * VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry
 		 */
-		pipe_config->limited_color_range =
-			bpp != 18 && drm_match_cea_mode(adjusted_mode) > 1;
+		pipe_config->limited_color_range = false;
 	} else {
 		pipe_config->limited_color_range =
 			intel_dp->limited_color_range;
Signature · 6 lines
2.9.3

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#1 lore raw
Quoted 7 lines · Lyude

Until now, it seems we've been erroneously enabling limited color ranges for the vast majority of DisplayPort monitors. I noticed this after writing a frame dump comparison test for the Chamelium and noticing that every i915 device I had was failing, while amdgpu machines were fine:

https://people.freedesktop.org/~lyudess/archive/01-03-2017/

Quoted 7 lines

It looks like this is because the DisplayPort spec tells us to use limited color ranges whenever we detect a CEA mode in use. However, from the looks of it there's another rather confusing part of the spec that got missed: source devices are allowed to use the full range of values for pixels -even- if the sink device declares that it's using a CEA mode. It's up to the sink device to limit the pixel range to the CEA ranges if it needs.

DP 1.2a 5.1.1.1 Video Colorimetry

* When a Source device is transmitting an RGB stream with a video timing format called out in CEA-861C Section 5 (except 640 x 480p) as using CEA range RGB, it should use CEA range RGB.

* When a Source device is transmitting a RGB stream with a video timing format called out in CEA-861C Section 5 as using CEA range RGB, it must use the CEA range RGB.

* However, a Source device may transmit all code values from zero to the maximum even when it declares the CEA range in the Main Stream Attributes. It is the responsibility of the Sink device to limit the pixel value range as needed.

The spec has a conflict about SHOULD and MUST. Nevertheless, using CEA range is at least recommended. When using CEA range, the CEA range MUST be declared in Main Stream Attributes for CEA modes, but full range of values MAY be transmitted.

DP 1.4 5.1.1.1 Video Colorimetry

* When a Source device is transmitting an RGB stream with a video timing format called out in CEA-861-F, Section 5 (except 640x480p) as using CEA range RGB, it should use CEA range RGB.

* When a Source device is transmitting an RGB stream with a video timing format called out in CEA-861-F, the device may provide RGB in the VESA range.

* The Sink device shall have the responsibility of limiting the pixel value range, as needed.

The conflict has been resolved, using CEA range is now recommended. However, the source MAY also use VESA range. I take it that means the source could also declare VESA range in Main Stream Attributes.

---

I am not sure if we'd be able to declare CEA range in Main Stream Attributes but transmit VESA range anyway. Feels kind of silly anyway, and IIUC would only be according to DP 1.2a. It boils down to CEA range vs. VESA range.

Quoted 4 lines

I'm really surprised that this bug would have been around as long as it looks like it has been without anyone noticing it, so I figured I'd just send a patch to the mailing list so you guys can point out whether or not this is really the correct thing to do.

We've had the same discussion several times over with HDMI for sure, but I think also with DP. The conclusion has always been that no matter which range we choose as the default, it's going to be wrong for some displays. And when it turns into a contest of who complains the loudest, it's a no brainer to go by spec. It's never been less than recommended to go with CEA range for CEA modes, regardless of the conflict in DP 1.2a.

No matter what we do here, the question remains what to do with Chamelium. Changing the color range is really a workaround for Chamelium, not a fix. Using CEA range is perfectly fine per DP spec.

BR,
Jani.

Quoted 26 lines
 drivers/gpu/drm/i915/intel_dp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_dp.c +5 -2
index d9bc19b..6642abd 100644
@@ -1649,12 +1649,15 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 found:
 	if (intel_dp->color_range_auto) {
 		/*
+		 * We are required to use the limited CEA RGB range when a CEA
+		 * mode is declared in the EDID. However, limiting the pixel
+		 * value range is up to the sink, not the source. So, just
+		 * don't enable limited color ranges.
 		 * See:
 		 * CEA-861-E - 5.1 Default Encoding Parameters
 		 * VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry
 		 */
-		pipe_config->limited_color_range =
-			bpp != 18 && drm_match_cea_mode(adjusted_mode) > 1;
+		pipe_config->limited_color_range = false;
 	} else {
 		pipe_config->limited_color_range =
 			intel_dp->limited_color_range;
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#2 lore raw
Quoted 3 lines · Jani Nikula

No matter what we do here, the question remains what to do with Chamelium. Changing the color range is really a workaround for Chamelium, not a fix. Using CEA range is perfectly fine per DP spec.

Can we just set a non-CEA mode/edid for chamelium, problem solved? We want to do that anyway for HDMI, where you really have to do the limited range dance to make stuff display correctly.
-Daniel

Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#3 lore raw

Hi,

Quoted 5 lines · Daniel Vetter
Quoted 3 lines · Jani Nikula

No matter what we do here, the question remains what to do with Chamelium. Changing the color range is really a workaround for Chamelium, not a fix. Using CEA range is perfectly fine per DP spec.

Quoted 13 lines

Can we just set a non-CEA mode/edid for chamelium, problem solved? We want to do that anyway for HDMI, where you really have to do the limited range dance to make stuff display correctly.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

I shortly want to emphasize on this:
https://lists.freedesktop.org/archives/intel-gfx/2015-November/081516.html

No matter what you sync with the Port - userspace must have a chance to know what happens with its DATA. It is a huge problem when the driver clamps by itself assuming everything it gets would be full range. Or when the Output device clamps and does not scale, then you loose, e.g. 0 ..16 and 235.. 255. On the other hand such scenarios make userspace scale data - especially video data - twice. One time to full range and later down in the driver when using CEA mode, e.g. Limited 16:235.

Please keep that in mind. It's problematic already nowadays when a 1:1 output without color altering is high importance.

Thanks very much
Peter

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#4 lore raw
In reply to Daniel Vetter

Hi,

Quoted 8 lines · Daniel Vetter
Quoted 3 lines · Jani Nikula

No matter what we do here, the question remains what to do with Chamelium. Changing the color range is really a workaround for Chamelium, not a fix. Using CEA range is perfectly fine per DP spec.

Can we just set a non-CEA mode/edid for chamelium, problem solved? We want to do that anyway for HDMI, where you really have to do the limited range dance to make stuff display correctly.

Or, if you set the 'Broadcast RGB' connector property to 'Full', you'll never hit the color_range_auto branch in the first place ...

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

author
#5 lore raw
In reply to Daniel Vetter
Quoted 6 lines · Daniel Vetter
Quoted 4 lines · Jani Nikula

No matter what we do here, the question remains what to do with Chamelium. Changing the color range is really a workaround for Chamelium, not a fix. Using CEA range is perfectly fine per DP spec.

Of course, I had just figured this was actually a bug, but I guess not :). I'll just make sure RGB Broadcast is set in the igt tests then

Quoted 6 lines

Can we just set a non-CEA mode/edid for chamelium, problem solved? We want
to do that anyway for HDMI, where you really have to do the limited range
dance to make stuff display correctly.
-Daniel

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx