On Tue, Jan 11, 2022 at 04:55:35PM +0100, Maxime Ripard wrote:
Hi Ville,
Thanks for your review
On Wed, Dec 15, 2021 at 03:48:39PM +0200, Ville Syrjälä wrote:
On Wed, Dec 15, 2021 at 01:43:53PM +0100, Maxime Ripard wrote:
The current code, when parsing the EDID Deep Color depths, that the YUV422 cannot be used, referring to the HDMI 1.3 Specification.
This specification, in its section 6.2.4, indeed states:
For each supported Deep Color mode, RGB 4:4:4 shall be supported and optionally YCBCR 4:4:4 may be supported.
YCBCR 4:2:2 is not permitted for any Deep Color mode.
This indeed can be interpreted like the code does, but the HDMI 1.4 specification further clarifies that statement in its section 6.2.4:
For each supported Deep Color mode, RGB 4:4:4 shall be supported and optionally YCBCR 4:4:4 may be supported.
YCBCR 4:2:2 is also 36-bit mode but does not require the further use of the Deep Color modes described in section 6.5.2 and 6.5.3.
This means that, even though YUV422 can be used with 12 bit per color, it shouldn't be treated as a deep color mode.
This deviates from the interpretation of the code and comment, so let's fix those.
Fixes: d0c94692e0a3 ("drm/edid: Parse and handle HDMI deep color modes.") Signed-off-by: Maxime Ripard maxime@cerno.tech
drivers/gpu/drm/drm_edid.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 12893e7be89b..e57d1b8cdaaa 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5106,10 +5106,9 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
/* * Deep color support mandates RGB444 support for all video
* modes and forbids YCRCB422 support for all video modes per* HDMI 1.3 spec.
* modes.
- info->color_formats = DRM_COLOR_FORMAT_RGB444;
info->color_formats |= DRM_COLOR_FORMAT_RGB444;
/* YCRCB444 is optional according to spec. */ if (hdmi[6] & DRM_EDID_HDMI_DC_Y444) {
This whole code seems pretty much wrong. What it tries to do (I think) is make sure we don't use deep color with YCbCr 4:4:4 unless supported. But what it actually does is also disable YCbCr 4:4:4 8bpc when deep color is not supported for YCbCr 4:4:4.
I think what we want is to just get rid of this color_formats stuff here entirely and instead have some kind of separate tracking of RGB 4:4:4 vs. YCbCr 4:4:4 deep color capabilities.
I'm sorry, I'm not entirely sure to understand what you're suggesting here. Do you want to get ride of info->color_formats entirely?
I think color_formats can stay (assuming it's actually useful for something), but we need separate tracking for supported RGB 4:4:4 vs. YCbCr 4:4:4 deep color modes.