<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 9, 2020 at 10:26 PM Harry Wentland <<a href="mailto:hwentlan@amd.com">hwentlan@amd.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<br>
<br>
<div>On 2020-01-09 4:04 p.m., Mario Kleiner
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">On Thu, Jan 9, 2020 at 8:49 PM Alex Deucher <<a href="mailto:alexdeucher@gmail.com" target="_blank">alexdeucher@gmail.com</a>>
wrote:<br>
</div>
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, Jan 9, 2020 at
11:47 AM Mario Kleiner<br>
<<a href="mailto:mario.kleiner.de@gmail.com" target="_blank">mario.kleiner.de@gmail.com</a>>
wrote:<br>
><br>
> On Thu, Jan 9, 2020 at 4:40 PM Alex Deucher <<a href="mailto:alexdeucher@gmail.com" target="_blank">alexdeucher@gmail.com</a>>
wrote:<br>
>><br>
>> On Thu, Jan 9, 2020 at 10:08 AM Mario Kleiner<br>
>> <<a href="mailto:mario.kleiner.de@gmail.com" target="_blank">mario.kleiner.de@gmail.com</a>>
wrote:<br>
>> ><br>
As Harry mentioned in the other thread, won't this only work
if the<br>
display was brought up by the vbios? In the suspend/resume
case,<br>
won't we just fall back to 2.7Gbps?<br>
<br>
Alex<br>
<br>
</blockquote>
</div>
<div class="gmail_quote"><br>
</div>
<div class="gmail_quote">Adding Harry to cc...<br>
</div>
<div class="gmail_quote"><br>
</div>
<div class="gmail_quote">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.</div>
<div class="gmail_quote"><br>
</div>
<div class="gmail_quote">On the AMD DC side,
dc_link_detect_helper() has this early no-op return at the
beginning:</div>
<div class="gmail_quote"><br>
</div>
<div class="gmail_quote">
<pre>if ((link->connector_signal == SIGNAL_TYPE_LVDS ||
link->connector_signal == SIGNAL_TYPE_EDP) &&
link->local_sink)
return true;</pre>
</div>
<div class="gmail_quote"><br>
</div>
<div class="gmail_quote">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?</div>
<div class="gmail_quote"><br>
</div>
<div class="gmail_quote">Sounds reasonable to me, given that eDP
panels are usually fixed internal panels, nothing that gets
hot(un-)plugged?</div>
<div class="gmail_quote"><br>
</div>
<div class="gmail_quote">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...<br>
</div>
<div class="gmail_quote"><br>
</div>
</div>
</blockquote>
<br>
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.<br>
<br>
Harry<br>
<br></div></blockquote><div><br></div><div>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 <br><pre><span class="gmail-nf"><a href="https://elixir.bootlin.com/linux/v5.5-rc5/ident/detect_edp_sink_caps">detect_edp_sink_caps</a></span><span class="gmail-p">() that matches on a new EDID quirk to be defined for that panel in drm_edid etc., and then<br><br>if (edit quirk for that panel)<br> dpcd[<span class="gmail-cp"><a href="https://elixir.bootlin.com/linux/v5.5-rc5/ident/DP_MAX_LINK_RATE">DP_MAX_LINK_RATE</a>] = 0xc;<br><br></span></span></pre> </div><div>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.<br></div><div><br></div><div>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.</div><div><br></div><div>-mario</div><div><br></div><div> </div></div></div>