On 2020-01-09 4:13 p.m., Mario Kleiner wrote: > On Thu, Jan 9, 2020 at 7:44 PM Harry Wentland <hwentlan@xxxxxxx > <mailto:hwentlan@xxxxxxx>> wrote: > > On 2020-01-09 10:20 a.m., Mario Kleiner wrote: > > If the current eDP link settings, as read from hw, provide a higher > > bandwidth than the verified_link_cap ones (= reported_link_cap), then > > override verified_link_cap with current settings. > > > > These initial current eDP link settings have been set up by > > firmware during boot, so they should work on the eDP panel. > > Therefore use them if the firmware thinks they are good and > > they provide higher link bandwidth, e.g., to enable higher > > resolutions / color depths. > > > > > > Hi Harry, happy new year! > Frohes Neues. :) > This only works when taking over from UEFI, so on boot or resume from > hibernate. This wouldn't work on a normal suspend/resume. > > > See the other thread i just cc'ed you on. Depends if > dc_link_detect_helper() gets skipped/early returns or not on EDP. Some > if statement suggests it might get skipped on EDP + resume? > You've likely looked at the code more closely while debugging this than I have recently. It looks like we indeed skip detection if we've previously detected the eDP sink. > > Can you check if setting link->dc->config.optimize_edp_link_rate (see > first if statement in detect_edp_sink_caps) fixes this? I imagine we > need to read the reported settings from DP_SUPPORTED_LINK_RATES and fail > to do so. > > > Tried that already (see other mail), replacing the whole if statement > with a if (true) to force reading DP_SUPPORTED_LINK_RATES. The whole > table reads back as all-zero, and versions are DP 1.1, eDP 1.3, not 1.4+ > as what seems to be required. The use the classic link bw stuff, but > with a non-standard link bandwidth multiplier of 0xc, and a reported > DP_MAX_LINK_RATE of 0xa, contradicting the 0xc setting that the firmware > sets at bootup. > > Seems to be a very Apple thing... Indeed. I think it was a funky panel that was "ahead of its time" and ahead of the spec. I would prefer a DPCD quirk for this panel that updates the reported DP caps, rather than picking the "current" ones from the FW lightup. Harry > -mario > > > > Thanks, > Harry > > > This fixes a problem found on the MacBookPro 2017 Retina panel: > > > > The panel reports 10 bpc color depth in its EDID, and the > > firmware chooses link settings at boot which support enough > > bandwidth for 10 bpc (324000 kbit/sec aka LINK_RATE_RBR2), > > but the DP_MAX_LINK_RATE dpcd register only reports 2.7 Gbps > > as possible, so verified_link_cap is only good for 2.7 Gbps > > and 8 bpc, not providing the full color depth of the panel. > > > > Signed-off-by: Mario Kleiner <mario.kleiner.de@xxxxxxxxx > <mailto:mario.kleiner.de@xxxxxxxxx>> > > Cc: Alex Deucher <alexander.deucher@xxxxxxx > <mailto:alexander.deucher@xxxxxxx>> > > --- > > drivers/gpu/drm/amd/display/dc/core/dc_link.c | 21 > +++++++++++++++++++ > > 1 file changed, 21 insertions(+) > > > > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c > b/drivers/gpu/drm/amd/display/dc/core/dc_link.c > > index 5ea4a1675259..f3acdb8fead5 100644 > > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c > > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c > > @@ -819,6 +819,27 @@ static bool dc_link_detect_helper(struct > dc_link *link, > > case SIGNAL_TYPE_EDP: { > > detect_edp_sink_caps(link); > > read_current_link_settings_on_detect(link); > > + > > + /* If cur_link_settings provides higher > bandwidth than > > + * verified_link_cap, then use > cur_link_settings as new > > + * verified_link_cap, as it obviously works > according to > > + * firmware boot setup. > > + * > > + * This has been observed on the Apple > MacBookPro 2017 > > + * Retina panel, which boots with a link > setting higher > > + * than what dpcd[DP_MAX_LINK_RATE] claims > as possible. > > + * Overriding allows to run the panel at 10 > bpc / 30 bit. > > + */ > > + if (dc_link_bandwidth_kbps(link, > &link->cur_link_settings) > > > + dc_link_bandwidth_kbps(link, > &link->verified_link_cap)) { > > + DC_LOG_DETECTION_DP_CAPS( > > + "eDP current link setting bw %d kbps > > verified_link_cap %d kbps. Override.", > > + dc_link_bandwidth_kbps(link, > &link->cur_link_settings), > > + dc_link_bandwidth_kbps(link, > &link->verified_link_cap)); > > + > > + link->verified_link_cap = > link->cur_link_settings; > > + } > > + > > sink_caps.transaction_type = > DDC_TRANSACTION_TYPE_I2C_OVER_AUX; > > sink_caps.signal = SIGNAL_TYPE_EDP; > > break; > > > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
