On Thu, Jan 20, 2022 at 04:16:12PM +0100, Maxime Ripard wrote:
The current code assumes that the RGB444 and YUV444 formats are the same, but the HDMI 2.0 specification states that:
The three DC_XXbit bits above only indicate support for RGB 4:4:4 at that pixel size. Support for YCBCR 4:4:4 in Deep Color modes is indicated with the DC_Y444 bit. If DC_Y444 is set, then YCBCR 4:4:4 is supported for all modes indicated by the DC_XXbit flags.
So if we have YUV444 support and any DC_XXbit flag set but the DC_Y444 flag isn't, we'll assume that we support that deep colour mode for YUV444 which breaks the specification.
In order to fix this, let's split the edid_hdmi_dc_modes field in struct drm_display_info into two fields, one for RGB444 and one for YUV444.
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Fixes: d0c94692e0a3 ("drm/edid: Parse and handle HDMI deep color modes.") Signed-off-by: Maxime Ripard maxime@cerno.tech
Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
Though we seem to be missing clearing of *dc_modes in drm_reset_display_info()...
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 2 +- drivers/gpu/drm/drm_edid.c | 7 ++++--- drivers/gpu/drm/i915/display/intel_hdmi.c | 4 ++-- drivers/gpu/drm/radeon/radeon_connectors.c | 2 +- include/drm/drm_connector.h | 12 +++++++++--- 5 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index c16a2704ced6..f3160b951df3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -175,7 +175,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector *connector)
/* Check if bpc is within clock limit. Try to degrade gracefully otherwise */ if ((bpc == 12) && (mode_clock * 3/2 > max_tmds_clock)) {
if ((connector->display_info.edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_30) &&
if ((connector->display_info.edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30) && (mode_clock * 5/4 <= max_tmds_clock)) bpc = 10; elsediff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 5085ef08c22d..471b577dca79 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5075,21 +5075,21 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
if (hdmi[6] & DRM_EDID_HDMI_DC_30) { dc_bpc = 10;
info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30;
info->edid_hdmi_rgb444_dc_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;
info->edid_hdmi_rgb444_dc_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;
info->edid_hdmi_rgb444_dc_modes |= DRM_EDID_HDMI_DC_48;@@ -5106,6 +5106,7 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
/* YCRCB444 is optional according to spec. */ if (hdmi[6] & DRM_EDID_HDMI_DC_Y444) {
info->edid_hdmi_ycbcr444_dc_modes = info->edid_hdmi_rgb444_dc_modes;diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 96e508ddc4af..52f6dc248453 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -1912,7 +1912,7 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector, if (ycbcr420_output) return hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36; else
return info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36;
return info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_36;@@ -1920,7 +1920,7 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector, if (ycbcr420_output) return hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_30; else
return info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_30;
return info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30;diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index 607ad5620bd9..1546abcadacf 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -204,7 +204,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
/* Check if bpc is within clock limit. Try to degrade gracefully otherwise */ if ((bpc == 12) && (mode_clock * 3/2 > max_tmds_clock)) {
if ((connector->display_info.edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_30) &&
if ((connector->display_info.edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30) && (mode_clock * 5/4 <= max_tmds_clock)) bpc = 10; elsediff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index b501d0badaea..eaf0ef5f1843 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -592,10 +592,16 @@ struct drm_display_info { bool rgb_quant_range_selectable;
/**
* @edid_hdmi_dc_modes: Mask of supported hdmi deep color modes. Even* more stuff redundant with @bus_formats.
* @edid_hdmi_dc_rgb444_modes: Mask of supported hdmi deep color modes* in RGB 4:4:4. Even more stuff redundant with @bus_formats.
- u8 edid_hdmi_dc_modes;
u8 edid_hdmi_rgb444_dc_modes;
/**
* @edid_hdmi_dc_ycbcr444_modes: Mask of supported hdmi deep color* modes in YCbCr 4:4:4. Even more stuff redundant with @bus_formats.*/u8 edid_hdmi_ycbcr444_dc_modes;
/**
- @cea_rev: CEA revision of the HDMI sink.
-- 2.34.1