Re: [PATCH] drm/i915/dp: Add current maximum eDP link rate to sink_rate array.

From: Mario Kleiner <mario.kleiner.de@gmail.com>
To: Harry Wentland <hwentlan@amd.com>
Cc: mario.kleiner.de@gmail.de, Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Maling list - DRI developers <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915/dp: Add current maximum eDP link rate to sink_rate array.
Date: Fri, 10 Jan 2020 17:02:24 +0100	[thread overview]
Message-ID: <CAEsyxyjr9MBFmVn_K_TBfq5mJO-OCLPKjwxX+xX9iRO+s7iLSQ@mail.gmail.com> (raw)
In-Reply-To: <bae132f3-73e6-5004-c9a9-adb632338268@amd.com>

[-- Attachment #1.1: Type: text/plain, Size: 3802 bytes --]

On Thu, Jan 9, 2020 at 10:26 PM Harry Wentland <hwentlan@amd.com> wrote:

>
>
> On 2020-01-09 4:04 p.m., Mario Kleiner wrote:
>
> On Thu, Jan 9, 2020 at 8:49 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
>> On Thu, Jan 9, 2020 at 11:47 AM Mario Kleiner
>> <mario.kleiner.de@gmail.com> wrote:
>> >
>> > On Thu, Jan 9, 2020 at 4:40 PM Alex Deucher <alexdeucher@gmail.com>
>> wrote:
>> >>
>> >> On Thu, Jan 9, 2020 at 10:08 AM Mario Kleiner
>> >> <mario.kleiner.de@gmail.com> wrote:
>> >> >
>> As Harry mentioned in the other thread, won't this only work if the
>> display was brought up by the vbios?  In the suspend/resume case,
>> won't we just fall back to 2.7Gbps?
>>
>> Alex
>>
>>
> Adding Harry to cc...
>
> The code is only executed for eDP. On the Intel side, it seems that
> intel_edp_init_dpcd() gets only called during driver load / modesetting
> init, so not on resume.
>
> On the AMD DC side, dc_link_detect_helper() has this early no-op return at
> the beginning:
>
> if ((link->connector_signal == SIGNAL_TYPE_LVDS ||
> 			link->connector_signal == SIGNAL_TYPE_EDP) &&
> 			link->local_sink)
> 		return true;
>
>
> So i guess if link->local_sink doesn't get NULL'ed during a suspend/resume
> cycle, then we never reach the setup code that would overwrite with non
> vbios settings?
>
> Sounds reasonable to me, given that eDP panels are usually fixed internal
> panels, nothing that gets hot(un-)plugged?
>
> I can't test, because suspend/resume with the Polaris gpu on the MBP 2017
> is totally broken atm., just as vgaswitcheroo can't do its job. Looks like
> powering down the gpu works, but powering up doesn't. And also modesetting
> at vgaswitcheroo switch time is no-go, because the DDC/AUX lines apparently
> can't be switched on that Apple gmux, and handover of that data seems to be
> not implemented in current vgaswitcheroo. At the moment switching between
> AMD only or Intel+AMD Prime setup is quite a pita...
>
>
> I haven't followed the entire discussion on the i915 thread but for the
> amdgpu dc patch I would prefer a DPCD quirk to override the reported link
> settings with the correct link rate.
>
> Harry
>
>
Ok, as you wish. How do i do that? Is there already some DP related
official mechanism, or do i just add some if-statement to

detect_edp_sink_caps
<https://elixir.bootlin.com/linux/v5.5-rc5/ident/detect_edp_sink_caps>()
that matches on a new EDID quirk to be defined for that panel in
drm_edid etc., and then

if (edit quirk for that panel)
    dpcd[DP_MAX_LINK_RATE
<https://elixir.bootlin.com/linux/v5.5-rc5/ident/DP_MAX_LINK_RATE>] =
0xc;

The other question would be if we should do it for this panel on AMD DC at
all? I see my original patch more as something to fix other odd (Apple?)
panels, than for this specific one. As mentioned above, photometer testing
on AMD DC with a Polaris on the MBP 2017 suggests that the deault 2.7 Gbps
8 bit mode + AMD's spatial dithering provides higher quality results for >=
10 bpc framebuffers than actually running the panel at 10 bit without
dithering.

As a little side-note, for squeezing out more precision than the 10 bpc
framebuffers we officially have in Mesa/OpenGL, my software Psychtoolbox
has some special hacks, playing funny tricks with resizing X-Screens,
applying bit-twiddling shaders to images and MMIO programming the gpu
"behind the back" of the driver, to get the gpu into RGBA16161616 linear
scanout mode. That gives up to 12 bpc precision on that panel according to
photometer measurements. While AMD's dithering with the panel in 8 bit + 4
bit spatial dithering gives pretty good results, panel at 10 bit + 2 bit
spatial dithering has some artifacts. And even at a normal 10 bit
framebuffer, the 8 bit panel + 2 bit dithering seems to give better results
than 10 bit panel mode.

-mario

[-- Attachment #1.2: Type: text/html, Size: 6385 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