From: Mario Kleiner <mario.kleiner.de@gmail.com>
To: Harry Wentland <hwentlan@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>,
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: Fri, 28 Feb 2020 22:41:14 +0100 [thread overview]
Message-ID: <CAEsyxyjHPMM=NcShaZRLCEfbS0xE042YcL5EU+_0CcARhg+Dqw@mail.gmail.com> (raw)
In-Reply-To: <CAEsyxygQff4kcOweR_PTKSMf9wss5i+nGA=BwiUjSH+V-MXXAQ@mail.gmail.com>
On Thu, Feb 27, 2020 at 8:11 PM Mario Kleiner
<mario.kleiner.de@gmail.com> wrote:
>
> Hi Harry
>
> Ok, back from various other emergencies and deadlines, sorry for the
> late reply. I also fixed my e-mail address - it was mistyped, causing
> all these delivery failures :/
>
> On Thu, Jan 9, 2020 at 10:26 PM Harry Wentland <hwentlan@amd.com> wrote:
> >
> > On 2020-01-09 4:13 p.m., Mario Kleiner wrote:
> > > On Thu, Jan 9, 2020 at 7:44 PM Harry Wentland <hwentlan@amd.com
> > > <mailto: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.
> > > >
> ... snip ...
> > >
> > >
> > > 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
> >
>
> How would i do this? I see various options:
>
> I could rewrite my current patch, move it down inside
> dc_link_detect_helper() until after the edid was read and we have
> vendor/model id available, then say if(everything that's there now &&
> (vendor=Apple) && (model=Troublesomepanel)) { ... }
>
> Or i could add quirk code to detect_edp_sink_caps() after
> retrieve_link_cap() [or inside retrieve_link_cap] to override the
> reported_link_cap. But at that point we don't have edid yet and
> therefore no vendor/model id. Is there something inside the dpcd one
> can use to uniquely identify this display model?
>
> struct dp_device_vendor_id sink_id; queried inside retrieve_link_cap()
> sounds like it could be a unique id? I don't know about that.
>
> My intention was to actually do nothing on the AMD side here, as my
> photometer measurements suggest that the panel gives better quality
> results for >= 10 bpc output if it is operated at 8 bit and then the
> gpu's spatial dithering convincingly fakes the extra bits. Quality
> seems worse if one actually switches the panel into 10 bpc, as it
> doesn't seem to be a real 10 bit panel, just a 8 bit panel that
> accepts 10 bit and then badly dithers it to 10 bit.
>
> The situation has changed for Linux 5.6-rc, because of this recent
> commit from Roman Li, which is already in 5.6-rc:
> 4a8ca46bae8affba063aabac85a0b1401ba810a3 "drm/amd/display: Default max
> bpc to 16 for eDP"
>
> While that commit supposedly fixes some darkness on some other eDP
> panel, it now breaks my eDP panel. It leaves edid reported bpc
> unclamped, so the driver uses 10 bpc as basis for required bandwidth
> calculations and then the required bandwidth for all modes exceeds the
> link bandwidth. I end with the eDP panel having no valid modes at all
> ==> Panel goes black, game over.
>
> We either need to revert that commit for drm-fixes, or quirk it for
> the specific panels that are troublesome, or need to get some solution
> into 5.6-rc, otherwise there will be a lot of regressions for at least
> Apple MBP users.
>
> thanks,
> -mario
>
Ok, just sent out a patch with a specific dpcd quirk for this as:
[PATCH] drm/amd/display: Add link_rate quirk for Apple 15" MBP 2017
Tested against drm-next for Linux 5.6, works.
-mario
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-02-28 21:41 UTC|newest]
Thread overview: 11+ 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 ` [PATCH 1/2] drm/amd/display: Reorder detect_edp_sink_caps before link settings read Mario Kleiner
2020-01-09 18:40 ` Harry Wentland
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 18:44 ` Harry Wentland
2020-01-09 21:13 ` Mario Kleiner
2020-01-09 21:26 ` Harry Wentland
2020-02-27 19:11 ` Mario Kleiner
2020-02-28 21:41 ` Mario Kleiner [this message]
2020-01-11 11:41 ` Some eDP fixes/improvements 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='CAEsyxyjHPMM=NcShaZRLCEfbS0xE042YcL5EU+_0CcARhg+Dqw@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 \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).