From: Ville Syrjälä ville.syrjala@linux.intel.com
This series aims to clean up the way we fill out the display_info structure a bit. Mainly doing a clean separation of the audio and video related bits.
My aim is getting i915 to respect the HDMI sink TMDS clock limit (currently we respect only the DP++ dongle limit, and the source limit). But while doing that, I noticed that the display_info stuff was a bit of mess, hence it resulted in this bigger series.
Entire series available here: git://github.com/vsyrjala/linux.git hdmi_sink_tmds_limit_3
Ville Syrjälä (10): drm/edid: Clear old audio latency values before parsing the new EDID drm/edid: Clear old dvi_dual/max_tmds_clock before parsing the new EDID drm/edid: Make max_tmds_clock kHz instead of MHz drm/edid: Move dvi_dual/max_tmds_clock to drm_display_info drm/edid: Don't pass around drm_display_info needlessly drm/edid: Reduce the number of times we parse the CEA extension block drm/edid: Clear the old cea_rev when there's no CEA extension in the new EDID drm/edid: Move dvi_dual/max_tmds_clock parsing out from drm_edid_to_eld() drm/i915: Replace a bunch of connector->base.display_info with a local variable drm/i915: Account for sink max TMDS clock when checking the port clock
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 4 +- drivers/gpu/drm/drm_edid.c | 258 +++++++++++++------------ drivers/gpu/drm/i915/intel_display.c | 14 +- drivers/gpu/drm/i915/intel_hdmi.c | 9 +- drivers/gpu/drm/radeon/radeon_connectors.c | 4 +- include/drm/drm_crtc.h | 7 +- 6 files changed, 152 insertions(+), 144 deletions(-)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Clear out stale audio latency information (potentially from a previous EDID) before constructing the ELD from the EDID.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 7df26d4b7ad8..05ef93c8fa44 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3353,6 +3353,13 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
memset(eld, 0, sizeof(connector->eld));
+ connector->latency_present[0] = false; + connector->latency_present[1] = false; + connector->video_latency[0] = 0; + connector->audio_latency[0] = 0; + connector->video_latency[1] = 0; + connector->audio_latency[1] = 0; + cea = drm_find_cea_extension(edid); if (!cea) { DRM_DEBUG_KMS("ELD: no CEA Extension found\n");
From: Ville Syrjälä ville.syrjala@linux.intel.com
Clear out old max_tmds_clock and dvi_dual information (possibly from a previous EDID) before parsing the current EDID. Tne current EDID might not even have these in its HDMI VSDB, which would mean that we'd leave the old stale values in place.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 05ef93c8fa44..40e7f1a863a6 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3360,6 +3360,9 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) connector->video_latency[1] = 0; connector->audio_latency[1] = 0;
+ connector->max_tmds_clock = 0; + connector->dvi_dual = false; + cea = drm_find_cea_extension(edid); if (!cea) { DRM_DEBUG_KMS("ELD: no CEA Extension found\n");
From: Ville Syrjälä ville.syrjala@linux.intel.com
We generally store clocks in kHz, so let's do that for the HDMI max TMDS clock value as well. Less surpising.
Cc: Alex Deucher alexander.deucher@amd.com Cc: "Christian König" christian.koenig@amd.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 2 +- drivers/gpu/drm/drm_edid.c | 2 +- drivers/gpu/drm/radeon/radeon_connectors.c | 2 +- include/drm/drm_crtc.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index ff0b55a65ca3..a56eeaa213f4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -173,7 +173,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector *connector) mode_clock = amdgpu_connector->pixelclock_for_modeset;
/* Maximum allowable input clock in kHz */ - max_tmds_clock = connector->max_tmds_clock * 1000; + max_tmds_clock = connector->max_tmds_clock;
DRM_DEBUG("%s: hdmi mode dotclock %d kHz, max tmds input clock %d kHz.\n", connector->name, mode_clock, max_tmds_clock); diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 40e7f1a863a6..f0f08f56ea97 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3257,7 +3257,7 @@ parse_hdmi_vsdb(struct drm_connector *connector, const u8 *db) connector->dvi_dual = db[6] & 1; } if (len >= 7) - connector->max_tmds_clock = db[7] * 5; + connector->max_tmds_clock = db[7] * 5000; if (len >= 8) { connector->latency_present[0] = db[8] >> 7; connector->latency_present[1] = (db[8] >> 6) & 1; diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index b79f3b002471..db5488732e31 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -203,7 +203,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector) mode_clock = radeon_connector->pixelclock_for_modeset;
/* Maximum allowable input clock in kHz */ - max_tmds_clock = connector->max_tmds_clock * 1000; + max_tmds_clock = connector->max_tmds_clock;
DRM_DEBUG("%s: hdmi mode dotclock %d kHz, max tmds input clock %d kHz.\n", connector->name, mode_clock, max_tmds_clock); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 3edeaf88ebc0..e7cf08d9df10 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1366,7 +1366,7 @@ struct drm_connector { /* EDID bits */ uint8_t eld[MAX_ELD_BYTES]; bool dvi_dual; - int max_tmds_clock; /* in MHz */ + int max_tmds_clock; /* in kHz */ bool latency_present[2]; int video_latency[2]; /* [0]: progressive, [1]: interlaced */ int audio_latency[2];
From: Ville Syrjälä ville.syrjala@linux.intel.com
We have the drm_display_info for storing information about the sink, so let's move dvi_dual and max_tmds_clock in there.
Cc: Alex Deucher alexander.deucher@amd.com Cc: "Christian König" christian.koenig@amd.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 4 ++-- drivers/gpu/drm/drm_edid.c | 14 ++++++++------ drivers/gpu/drm/radeon/radeon_connectors.c | 4 ++-- include/drm/drm_crtc.h | 7 +++---- 4 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index a56eeaa213f4..3df2f6a56b46 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -168,12 +168,12 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector *connector) }
/* Any defined maximum tmds clock limit we must not exceed? */ - if (connector->max_tmds_clock > 0) { + if (connector->display_info.max_tmds_clock > 0) { /* mode_clock is clock in kHz for mode to be modeset on this connector */ mode_clock = amdgpu_connector->pixelclock_for_modeset;
/* Maximum allowable input clock in kHz */ - max_tmds_clock = connector->max_tmds_clock; + max_tmds_clock = connector->display_info.max_tmds_clock;
DRM_DEBUG("%s: hdmi mode dotclock %d kHz, max tmds input clock %d kHz.\n", connector->name, mode_clock, max_tmds_clock); diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index f0f08f56ea97..abd6d3c77ef2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3250,14 +3250,15 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) static void parse_hdmi_vsdb(struct drm_connector *connector, const u8 *db) { + struct drm_display_info *info = &connector->display_info; u8 len = cea_db_payload_len(db);
if (len >= 6) { connector->eld[5] |= (db[6] >> 7) << 1; /* Supports_AI */ - connector->dvi_dual = db[6] & 1; + info->dvi_dual = db[6] & 1; } if (len >= 7) - connector->max_tmds_clock = db[7] * 5000; + info->max_tmds_clock = db[7] * 5000; if (len >= 8) { connector->latency_present[0] = db[8] >> 7; connector->latency_present[1] = (db[8] >> 6) & 1; @@ -3276,8 +3277,8 @@ parse_hdmi_vsdb(struct drm_connector *connector, const u8 *db) "latency present %d %d, " "video latency %d %d, " "audio latency %d %d\n", - connector->dvi_dual, - connector->max_tmds_clock, + info->dvi_dual, + info->max_tmds_clock, (int) connector->latency_present[0], (int) connector->latency_present[1], connector->video_latency[0], @@ -3344,6 +3345,7 @@ EXPORT_SYMBOL(drm_edid_get_monitor_name); */ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) { + struct drm_display_info *info = &connector->display_info; uint8_t *eld = connector->eld; u8 *cea; u8 *db; @@ -3360,8 +3362,8 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) connector->video_latency[1] = 0; connector->audio_latency[1] = 0;
- connector->max_tmds_clock = 0; - connector->dvi_dual = false; + info->max_tmds_clock = 0; + info->dvi_dual = false;
cea = drm_find_cea_extension(edid); if (!cea) { diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index db5488732e31..50e96d2c593d 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -198,12 +198,12 @@ int radeon_get_monitor_bpc(struct drm_connector *connector) }
/* Any defined maximum tmds clock limit we must not exceed? */ - if (connector->max_tmds_clock > 0) { + if (connector->display_info.max_tmds_clock > 0) { /* mode_clock is clock in kHz for mode to be modeset on this connector */ mode_clock = radeon_connector->pixelclock_for_modeset;
/* Maximum allowable input clock in kHz */ - max_tmds_clock = connector->max_tmds_clock; + max_tmds_clock = connector->display_info.max_tmds_clock;
DRM_DEBUG("%s: hdmi mode dotclock %d kHz, max tmds input clock %d kHz.\n", connector->name, mode_clock, max_tmds_clock); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index e7cf08d9df10..b1c54b322b4f 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -150,6 +150,9 @@ struct drm_display_info { unsigned int num_bus_formats; u32 bus_flags;
+ int max_tmds_clock; /* in kHz */ + bool dvi_dual; + /* Mask of supported hdmi deep color modes */ u8 edid_hdmi_dc_modes;
@@ -1268,8 +1271,6 @@ struct drm_encoder { * @encoder_ids: valid encoders for this connector * @encoder: encoder driving this connector, if any * @eld: EDID-like data, if present - * @dvi_dual: dual link DVI, if found - * @max_tmds_clock: max clock rate, if found * @latency_present: AV delay info from ELD, if found * @video_latency: video latency info from ELD, if found * @audio_latency: audio latency info from ELD, if found @@ -1365,8 +1366,6 @@ struct drm_connector {
/* EDID bits */ uint8_t eld[MAX_ELD_BYTES]; - bool dvi_dual; - int max_tmds_clock; /* in kHz */ bool latency_present[2]; int video_latency[2]; /* [0]: progressive, [1]: interlaced */ int audio_latency[2];
From: Ville Syrjälä ville.syrjala@linux.intel.com
We already pass the connector to drm_add_display_info() and drm_assign_hdmi_deep_color_info(), so passing the connector->display_info also is pointless.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index abd6d3c77ef2..d8c7da56ab75 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3732,17 +3732,15 @@ EXPORT_SYMBOL(drm_rgb_quant_range_selectable); * drm_assign_hdmi_deep_color_info - detect whether monitor supports * hdmi deep color modes and update drm_display_info if so. * @edid: monitor EDID information - * @info: Updated with maximum supported deep color bpc and color format - * if deep color supported. * @connector: DRM connector, used only for debug output * * Parse the CEA extension according to CEA-861-B. * Return true if HDMI deep color supported, false if not or unknown. */ static bool drm_assign_hdmi_deep_color_info(struct edid *edid, - struct drm_display_info *info, struct drm_connector *connector) { + struct drm_display_info *info = &connector->display_info; u8 *edid_ext, *hdmi; int i; int start_offset, end_offset; @@ -3832,7 +3830,6 @@ static bool drm_assign_hdmi_deep_color_info(struct edid *edid, /** * drm_add_display_info - pull display info out if present * @edid: EDID data - * @info: display info (attached to connector) * @connector: connector whose edid is used to build display info * * Grab any available display info and stuff it into the drm_display_info @@ -3840,9 +3837,9 @@ static bool drm_assign_hdmi_deep_color_info(struct edid *edid, * color spaces. */ static void drm_add_display_info(struct edid *edid, - struct drm_display_info *info, struct drm_connector *connector) { + struct drm_display_info *info = &connector->display_info; u8 *edid_ext;
info->width_mm = edid->width_cm * 10; @@ -3872,7 +3869,7 @@ static void drm_add_display_info(struct edid *edid, }
/* HDMI deep color modes supported? Assign to info, if so */ - drm_assign_hdmi_deep_color_info(edid, info, connector); + drm_assign_hdmi_deep_color_info(edid, connector);
/* Only defined for 1.4 with digital displays */ if (edid->revision < 4) @@ -4092,7 +4089,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75)) edid_fixup_preferred(connector, quirks);
- drm_add_display_info(edid, &connector->display_info, connector); + drm_add_display_info(edid, connector);
if (quirks & EDID_QUIRK_FORCE_8BPC) connector->display_info.bpc = 8;
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of parsing parts of the CEA extension block in two places to determine supported color formats and whatnot, let's just consolidate it to one function. This also makes it possible to neatly flatten drm_assign_hdmi_deep_color_info().
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 192 +++++++++++++++++++++------------------------ 1 file changed, 89 insertions(+), 103 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index d8c7da56ab75..82ab57c4d6c9 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3728,119 +3728,119 @@ bool drm_rgb_quant_range_selectable(struct edid *edid) } EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
-/** - * drm_assign_hdmi_deep_color_info - detect whether monitor supports - * hdmi deep color modes and update drm_display_info if so. - * @edid: monitor EDID information - * @connector: DRM connector, used only for debug output - * - * Parse the CEA extension according to CEA-861-B. - * Return true if HDMI deep color supported, false if not or unknown. - */ -static bool drm_assign_hdmi_deep_color_info(struct edid *edid, - struct drm_connector *connector) +static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, + const u8 *hdmi) { struct drm_display_info *info = &connector->display_info; - u8 *edid_ext, *hdmi; - int i; - int start_offset, end_offset; unsigned int dc_bpc = 0;
- edid_ext = drm_find_cea_extension(edid); - if (!edid_ext) - return false; + /* HDMI supports at least 8 bpc */ + info->bpc = 8;
- if (cea_db_offsets(edid_ext, &start_offset, &end_offset)) - return false; + 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; + 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; + 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; + DRM_DEBUG("%s: HDMI sink does deep color 48.\n", + connector->name); + } + + if (dc_bpc == 0) { + DRM_DEBUG("%s: No deep color support on this HDMI sink.\n", + connector->name); + return; + } + + DRM_DEBUG("%s: Assigning HDMI sink color depth as %d bpc.\n", + connector->name, dc_bpc); + info->bpc = dc_bpc;
/* - * Because HDMI identifier is in Vendor Specific Block, - * search it from all data blocks of CEA extension. + * Deep color support mandates RGB444 support for all video + * modes and forbids YCRCB422 support for all video modes per + * HDMI 1.3 spec. */ - for_each_cea_db(edid_ext, i, start_offset, end_offset) { - if (cea_db_is_hdmi_vsdb(&edid_ext[i])) { - /* HDMI supports at least 8 bpc */ - info->bpc = 8; - - hdmi = &edid_ext[i]; - if (cea_db_payload_len(hdmi) < 6) - return false; - - if (hdmi[6] & DRM_EDID_HDMI_DC_30) { - dc_bpc = 10; - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30; - DRM_DEBUG("%s: HDMI sink does deep color 30.\n", - connector->name); - } + info->color_formats = DRM_COLOR_FORMAT_RGB444;
- if (hdmi[6] & DRM_EDID_HDMI_DC_36) { - dc_bpc = 12; - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36; - DRM_DEBUG("%s: HDMI sink does deep color 36.\n", - connector->name); - } + /* YCRCB444 is optional according to spec. */ + if (hdmi[6] & DRM_EDID_HDMI_DC_Y444) { + info->color_formats |= DRM_COLOR_FORMAT_YCRCB444; + DRM_DEBUG("%s: HDMI sink does YCRCB444 in deep color.\n", + connector->name); + }
- if (hdmi[6] & DRM_EDID_HDMI_DC_48) { - dc_bpc = 16; - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48; - DRM_DEBUG("%s: HDMI sink does deep color 48.\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); + } +}
- if (dc_bpc > 0) { - DRM_DEBUG("%s: Assigning HDMI sink color depth as %d bpc.\n", - connector->name, dc_bpc); - info->bpc = dc_bpc; - - /* - * Deep color support mandates RGB444 support for all video - * modes and forbids YCRCB422 support for all video modes per - * HDMI 1.3 spec. - */ - info->color_formats = DRM_COLOR_FORMAT_RGB444; - - /* YCRCB444 is optional according to spec. */ - if (hdmi[6] & DRM_EDID_HDMI_DC_Y444) { - info->color_formats |= DRM_COLOR_FORMAT_YCRCB444; - DRM_DEBUG("%s: HDMI sink does YCRCB444 in deep color.\n", - connector->name); - } +static void drm_parse_cea_ext(struct drm_connector *connector, + struct edid *edid) +{ + struct drm_display_info *info = &connector->display_info; + const u8 *edid_ext; + int i, start, end;
- /* - * 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); - } + edid_ext = drm_find_cea_extension(edid); + if (!edid_ext) + return;
- return true; - } - else { - DRM_DEBUG("%s: No deep color support on this HDMI sink.\n", - connector->name); - } - } - } + info->cea_rev = edid_ext[1];
- return false; + /* The existence of a CEA block should imply RGB support */ + info->color_formats = DRM_COLOR_FORMAT_RGB444; + if (edid_ext[3] & EDID_CEA_YCRCB444) + info->color_formats |= DRM_COLOR_FORMAT_YCRCB444; + if (edid_ext[3] & EDID_CEA_YCRCB422) + info->color_formats |= DRM_COLOR_FORMAT_YCRCB422; + + if (cea_db_offsets(edid_ext, &start, &end)) + return; + + for_each_cea_db(edid_ext, i, start, end) { + const u8 *db = &edid_ext[i]; + + if (!cea_db_is_hdmi_vsdb(db)) + continue; + + drm_parse_hdmi_deep_color_info(connector, db); + } }
/** * drm_add_display_info - pull display info out if present - * @edid: EDID data * @connector: connector whose edid is used to build display info + * @edid: EDID data * * Grab any available display info and stuff it into the drm_display_info * structure that's part of the connector. Useful for tracking bpp and * color spaces. */ -static void drm_add_display_info(struct edid *edid, - struct drm_connector *connector) +static void drm_add_display_info(struct drm_connector *connector, + struct edid *edid) { struct drm_display_info *info = &connector->display_info; - u8 *edid_ext;
info->width_mm = edid->width_cm * 10; info->height_mm = edid->height_cm * 10; @@ -3855,21 +3855,7 @@ static void drm_add_display_info(struct edid *edid, if (!(edid->input & DRM_EDID_INPUT_DIGITAL)) return;
- /* Get data from CEA blocks if present */ - edid_ext = drm_find_cea_extension(edid); - if (edid_ext) { - info->cea_rev = edid_ext[1]; - - /* The existence of a CEA block should imply RGB support */ - info->color_formats = DRM_COLOR_FORMAT_RGB444; - if (edid_ext[3] & EDID_CEA_YCRCB444) - info->color_formats |= DRM_COLOR_FORMAT_YCRCB444; - if (edid_ext[3] & EDID_CEA_YCRCB422) - info->color_formats |= DRM_COLOR_FORMAT_YCRCB422; - } - - /* HDMI deep color modes supported? Assign to info, if so */ - drm_assign_hdmi_deep_color_info(edid, connector); + drm_parse_cea_ext(connector, edid);
/* Only defined for 1.4 with digital displays */ if (edid->revision < 4) @@ -4089,7 +4075,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75)) edid_fixup_preferred(connector, quirks);
- drm_add_display_info(edid, connector); + drm_add_display_info(connector, edid);
if (quirks & EDID_QUIRK_FORCE_8BPC) connector->display_info.bpc = 8;
From: Ville Syrjälä ville.syrjala@linux.intel.com
It's not a good idea to leave stale cea_rev in the drm_display_info. The current EDID might not even have a CEA ext block in which case we'd end up leaving the stale value in place.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 82ab57c4d6c9..452e8c51a729 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3848,6 +3848,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->cea_rev = 0;
if (edid->revision < 3) return;
From: Ville Syrjälä ville.syrjala@linux.intel.com
drm_edid_to_eld() is just mean to cook up the ELD for the audio driver, so having it parse non-audio related stuff seems just wrong, and potentially could lead to that information not being even filled out if the function doesn't even get called. Let's move that stuff to the place where we parse the color formats and whatnot from the CEA ext block.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 64 +++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 452e8c51a729..3cf8e44e040d 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3248,17 +3248,12 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) }
static void -parse_hdmi_vsdb(struct drm_connector *connector, const u8 *db) +drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) { - struct drm_display_info *info = &connector->display_info; u8 len = cea_db_payload_len(db);
- if (len >= 6) { + if (len >= 6) connector->eld[5] |= (db[6] >> 7) << 1; /* Supports_AI */ - info->dvi_dual = db[6] & 1; - } - if (len >= 7) - info->max_tmds_clock = db[7] * 5000; if (len >= 8) { connector->latency_present[0] = db[8] >> 7; connector->latency_present[1] = (db[8] >> 6) & 1; @@ -3272,19 +3267,15 @@ parse_hdmi_vsdb(struct drm_connector *connector, const u8 *db) if (len >= 12) connector->audio_latency[1] = db[12];
- DRM_DEBUG_KMS("HDMI: DVI dual %d, " - "max TMDS clock %d, " - "latency present %d %d, " - "video latency %d %d, " - "audio latency %d %d\n", - info->dvi_dual, - info->max_tmds_clock, - (int) connector->latency_present[0], - (int) connector->latency_present[1], - connector->video_latency[0], - connector->video_latency[1], - connector->audio_latency[0], - connector->audio_latency[1]); + DRM_DEBUG_KMS("HDMI: latency present %d %d, " + "video latency %d %d, " + "audio latency %d %d\n", + connector->latency_present[0], + connector->latency_present[1], + connector->video_latency[0], + connector->video_latency[1], + connector->audio_latency[0], + connector->audio_latency[1]); }
static void @@ -3345,7 +3336,6 @@ EXPORT_SYMBOL(drm_edid_get_monitor_name); */ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) { - struct drm_display_info *info = &connector->display_info; uint8_t *eld = connector->eld; u8 *cea; u8 *db; @@ -3362,9 +3352,6 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) connector->video_latency[1] = 0; connector->audio_latency[1] = 0;
- info->max_tmds_clock = 0; - info->dvi_dual = false; - cea = drm_find_cea_extension(edid); if (!cea) { DRM_DEBUG_KMS("ELD: no CEA Extension found\n"); @@ -3414,7 +3401,7 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) case VENDOR_BLOCK: /* HDMI Vendor-Specific Data Block */ if (cea_db_is_hdmi_vsdb(db)) - parse_hdmi_vsdb(connector, db); + drm_parse_hdmi_vsdb_audio(connector, db); break; default: break; @@ -3795,6 +3782,25 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, } }
+static void +drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db) +{ + struct drm_display_info *info = &connector->display_info; + u8 len = cea_db_payload_len(db); + + if (len >= 6) + info->dvi_dual = db[6] & 1; + if (len >= 7) + info->max_tmds_clock = db[7] * 5000; + + DRM_DEBUG_KMS("HDMI: DVI dual %d, " + "max TMDS clock %d kHz\n", + info->dvi_dual, + info->max_tmds_clock); + + drm_parse_hdmi_deep_color_info(connector, db); +} + static void drm_parse_cea_ext(struct drm_connector *connector, struct edid *edid) { @@ -3821,10 +3827,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, for_each_cea_db(edid_ext, i, start, end) { const u8 *db = &edid_ext[i];
- if (!cea_db_is_hdmi_vsdb(db)) - continue; - - drm_parse_hdmi_deep_color_info(connector, db); + if (cea_db_is_hdmi_vsdb(db)) + drm_parse_hdmi_vsdb_video(connector, db); } }
@@ -3849,6 +3853,8 @@ static void drm_add_display_info(struct drm_connector *connector, info->bpc = 0; info->color_formats = 0; info->cea_rev = 0; + info->max_tmds_clock = 0; + info->dvi_dual = false;
if (edid->revision < 3) return;
From: Ville Syrjälä ville.syrjala@linux.intel.com
Reduce the eyesore with a local variable.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a8e8cc8dfae9..b9cdf9060da6 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12090,22 +12090,22 @@ static void connected_sink_compute_bpp(struct intel_connector *connector, struct intel_crtc_state *pipe_config) { + const struct drm_display_info *info = &connector->base.display_info; int bpp = pipe_config->pipe_bpp;
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] checking for sink bpp constrains\n", - connector->base.base.id, - connector->base.name); + connector->base.base.id, + connector->base.name);
/* Don't use an invalid EDID bpc value */ - if (connector->base.display_info.bpc && - connector->base.display_info.bpc * 3 < bpp) { + if (info->bpc != 0 && info->bpc * 3 < bpp) { DRM_DEBUG_KMS("clamping display bpp (was %d) to EDID reported max of %d\n", - bpp, connector->base.display_info.bpc*3); - pipe_config->pipe_bpp = connector->base.display_info.bpc*3; + bpp, info->bpc * 3); + pipe_config->pipe_bpp = info->bpc * 3; }
/* Clamp bpp to default limit on screens without EDID 1.4 */ - if (connector->base.display_info.bpc == 0) { + if (info->bpc == 0) { int type = connector->base.connector_type; int clamp_bpp = 24;
From: Ville Syrjälä ville.syrjala@linux.intel.com
It's perfectly legal for the sink to support 12bpc only for some lower resolution modes, while the higher resolution modes can only be used with 8bpc. So let's take the sink's max TMDS clock into account before we go and decide that a particular mode can be used with 12bpc.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 4df9f384910c..8b013301cf11 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1204,10 +1204,17 @@ static int hdmi_port_clock_limit(struct intel_hdmi *hdmi, int max_tmds_clock = intel_hdmi_source_max_tmds_clock(to_i915(dev));
if (respect_downstream_limits) { + struct intel_connector *connector = hdmi->attached_connector; + const struct drm_display_info *info = &connector->base.display_info; + if (hdmi->dp_dual_mode.max_tmds_clock) max_tmds_clock = min(max_tmds_clock, hdmi->dp_dual_mode.max_tmds_clock); - if (!hdmi->has_hdmi_sink) + + if (info->max_tmds_clock) + max_tmds_clock = min(max_tmds_clock, + info->max_tmds_clock); + else if (!hdmi->has_hdmi_sink) max_tmds_clock = min(max_tmds_clock, 165000); }
Looks reasonable to me, adding a few more AMD people which probably want to take a look as well.
Patch #3 and #4 are Reviewed-by: Christian König christian.koenig@amd.com since they touch the radeon/amdgpu driver.
Patch #1, #2 and #5 - #8 are Acked-by: Christian König christian.koenig@amd.com.
Regards, Christian.
Am 03.08.2016 um 08:33 schrieb ville.syrjala@linux.intel.com:
From: Ville Syrjälä ville.syrjala@linux.intel.com
This series aims to clean up the way we fill out the display_info structure a bit. Mainly doing a clean separation of the audio and video related bits.
My aim is getting i915 to respect the HDMI sink TMDS clock limit (currently we respect only the DP++ dongle limit, and the source limit). But while doing that, I noticed that the display_info stuff was a bit of mess, hence it resulted in this bigger series.
Entire series available here: git://github.com/vsyrjala/linux.git hdmi_sink_tmds_limit_3
Ville Syrjälä (10): drm/edid: Clear old audio latency values before parsing the new EDID drm/edid: Clear old dvi_dual/max_tmds_clock before parsing the new EDID drm/edid: Make max_tmds_clock kHz instead of MHz drm/edid: Move dvi_dual/max_tmds_clock to drm_display_info drm/edid: Don't pass around drm_display_info needlessly drm/edid: Reduce the number of times we parse the CEA extension block drm/edid: Clear the old cea_rev when there's no CEA extension in the new EDID drm/edid: Move dvi_dual/max_tmds_clock parsing out from drm_edid_to_eld() drm/i915: Replace a bunch of connector->base.display_info with a local variable drm/i915: Account for sink max TMDS clock when checking the port clock
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 4 +- drivers/gpu/drm/drm_edid.c | 258 +++++++++++++------------ drivers/gpu/drm/i915/intel_display.c | 14 +- drivers/gpu/drm/i915/intel_hdmi.c | 9 +- drivers/gpu/drm/radeon/radeon_connectors.c | 4 +- include/drm/drm_crtc.h | 7 +- 6 files changed, 152 insertions(+), 144 deletions(-)
dri-devel@lists.freedesktop.org
-
Christian König -
ville.syrjala＠linux.intel.com