[PATCH 2/2] drm/amd/display: Allow current eDP link settings to override verified ones.

[Mario Kleiner]

On Thu, Feb 27, 2020 at 8:11 PM Mario Kleiner
<mario.kleiner.de at 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 at 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 at amd.com
> > > <mailto:hwentlan at 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