From: Mario Kleiner <mario.kleiner.de@gmail.com> To: Harry Wentland <hwentlan@amd.com> Cc: Alex Deucher <alexander.deucher@amd.com>, mario.kleiner.de@gmail.de, amd-gfx list <amd-gfx@lists.freedesktop.org>, dri-devel <dri-devel@lists.freedesktop.org> Subject: Re: [PATCH 2/2] drm/amd/display: Allow current eDP link settings to override verified ones. Date: Thu, 9 Jan 2020 22:13:23 +0100 [thread overview] Message-ID: <CAEsyxygx+2p+i91bvYBLVfq-9qog-SLQ_KdHBTmSyq4Zfr09jg@mail.gmail.com> (raw) In-Reply-To: <9238371c-fc93-2a65-c3e5-df6b3d1270dd@amd.com> [-- Attachment #1.1: Type: text/plain, Size: 4481 bytes --] On Thu, Jan 9, 2020 at 7:44 PM Harry Wentland <hwentlan@amd.com> 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! 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? > 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... -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@gmail.com> > > Cc: Alex Deucher <alexander.deucher@amd.com> > > --- > > 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; > > > [-- Attachment #1.2: Type: text/html, Size: 6145 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Mario Kleiner <mario.kleiner.de@gmail.com> To: Harry Wentland <hwentlan@amd.com> Cc: Alex Deucher <alexander.deucher@amd.com>, mario.kleiner.de@gmail.de, amd-gfx list <amd-gfx@lists.freedesktop.org>, dri-devel <dri-devel@lists.freedesktop.org> Subject: Re: [PATCH 2/2] drm/amd/display: Allow current eDP link settings to override verified ones. Date: Thu, 9 Jan 2020 22:13:23 +0100 [thread overview] Message-ID: <CAEsyxygx+2p+i91bvYBLVfq-9qog-SLQ_KdHBTmSyq4Zfr09jg@mail.gmail.com> (raw) In-Reply-To: <9238371c-fc93-2a65-c3e5-df6b3d1270dd@amd.com> [-- Attachment #1.1: Type: text/plain, Size: 4481 bytes --] On Thu, Jan 9, 2020 at 7:44 PM Harry Wentland <hwentlan@amd.com> 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! 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? > 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... -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@gmail.com> > > Cc: Alex Deucher <alexander.deucher@amd.com> > > --- > > 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; > > > [-- Attachment #1.2: Type: text/html, Size: 6145 bytes --] [-- Attachment #2: Type: text/plain, Size: 154 bytes --] _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2020-01-09 21:13 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-01-09 15:20 Some eDP fixes/improvements Mario Kleiner 2020-01-09 15:20 ` Mario Kleiner 2020-01-09 15:20 ` [PATCH 1/2] drm/amd/display: Reorder detect_edp_sink_caps before link settings read Mario Kleiner 2020-01-09 15:20 ` Mario Kleiner 2020-01-09 18:40 ` Harry Wentland 2020-01-09 18:40 ` Harry Wentland 2020-01-11 0:17 ` Alex Deucher 2020-01-11 0:17 ` Alex Deucher 2020-01-09 15:20 ` [PATCH 2/2] drm/amd/display: Allow current eDP link settings to override verified ones Mario Kleiner 2020-01-09 15:20 ` Mario Kleiner 2020-01-09 18:44 ` Harry Wentland 2020-01-09 18:44 ` Harry Wentland 2020-01-09 21:13 ` Mario Kleiner [this message] 2020-01-09 21:13 ` Mario Kleiner 2020-01-09 21:26 ` Harry Wentland 2020-01-09 21:26 ` Harry Wentland 2020-02-27 19:11 ` Mario Kleiner 2020-02-27 19:11 ` Mario Kleiner 2020-02-28 21:41 ` Mario Kleiner 2020-02-28 21:41 ` Mario Kleiner 2020-01-11 11:41 ` Some eDP fixes/improvements Timo Aaltonen 2020-01-11 11:41 ` Timo Aaltonen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAEsyxygx+2p+i91bvYBLVfq-9qog-SLQ_KdHBTmSyq4Zfr09jg@mail.gmail.com \ --to=mario.kleiner.de@gmail.com \ --cc=alexander.deucher@amd.com \ --cc=amd-gfx@lists.freedesktop.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=hwentlan@amd.com \ --cc=mario.kleiner.de@gmail.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.