<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
        {font-family:PMingLiU;
        panose-1:2 1 6 1 0 1 1 1 1 1;}
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
        {font-family:"\@PMingLiU";
        panose-1:2 1 6 1 0 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0in;
        margin-bottom:.0001pt;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:#0563C1;
        text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
        {mso-style-priority:99;
        color:#954F72;
        text-decoration:underline;}
span.EmailStyle17
        {mso-style-type:personal-compose;
        font-family:"Calibri",sans-serif;
        color:windowtext;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-family:"Calibri",sans-serif;}
@page WordSection1
        {size:8.5in 11.0in;
        margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
        {page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="#0563C1" vlink="#954F72">
<div class="WordSection1">
<p class="MsoNormal">On Tue, 08 Oct 2019, Jani Nikula <jani.nikula@intel.com> wrote:<o:p></o:p></p>
<p class="MsoNormal">>On Mon, 07 Oct 2019, Adam Jackson <ajax@redhat.com> wrote:<o:p></o:p></p>
<p class="MsoNormal">>> On Mon, 2019-10-07 at 12:08 +0300, Jani Nikula wrote:<o:p></o:p></p>
<p class="MsoNormal">>><o:p> </o:p></p>
<p class="MsoNormal">>>> The problem with the EDID quirks is that exposing the quirks sticks out<o:p></o:p></p>
<p class="MsoNormal">>>> like a sore thumb. Thus far all of it has been contained in drm_edid.c<o:p></o:p></p>
<p class="MsoNormal">>>> and they affect how the EDID gets parsed, for all drivers. Obviously<o:p></o:p></p>
<p class="MsoNormal">>>> this could be changed, but is it the right thing to do?<o:p></o:p></p>
<p class="MsoNormal">>>> <o:p></o:p></p>
<p class="MsoNormal">>>> What I suggested was, check the OUI only, and if it matches, do<o:p></o:p></p>
<p class="MsoNormal">>>> more. Perhaps there's something in the 0x300 range of DPCD offsets that<o:p></o:p></p>
<p class="MsoNormal">>>> you can read? Or perhaps you need to write the source OUI first, and<o:p></o:p></p>
<p class="MsoNormal">>>> then do that.<o:p></o:p></p>
<p class="MsoNormal">>><o:p> </o:p></p>
<p class="MsoNormal">>> My issue isn't really with identifying the panel from EDID rather than<o:p></o:p></p>
<p class="MsoNormal">>> DPCD, whichever identifier is most specific is probably the best thing<o:p></o:p></p>
<p class="MsoNormal">>> to use. It's more that this quirk is identified in common code but only<o:p></o:p></p>
<p class="MsoNormal">>> applied in one driver. If this panel were ever to be attached to some<o:p></o:p></p>
<p class="MsoNormal">>> other source, they might well want to apply the same kind of fix. My<o:p></o:p></p>
<p class="MsoNormal">>> (admittedly naïve) reading of the OUI handshake process is that when<o:p></o:p></p>
<p class="MsoNormal">>> the source device writes an OUI to DP_SOURCE_OUI it is telling the sink<o:p></o:p></p>
<p class="MsoNormal">>> "I'm about to issue commands that conform to _this_ vendor's own<o:p></o:p></p>
<p class="MsoNormal">>> conventions". If that convention communicates information that is<o:p></o:p></p>
<p class="MsoNormal">>> entirely contained within AUXCH transactions (and doesn't, for example,<o:p></o:p></p>
<p class="MsoNormal">>> require looking at some other strapping pin or external device) then in<o:p></o:p></p>
<p class="MsoNormal">>> principle it doesn't matter if the source device "matches" that OUI; it<o:p></o:p></p>
<p class="MsoNormal">>> would be legal for an AMD GPU to write the same sequence and expect the<o:p></o:p></p>
<p class="MsoNormal">>> same reaction, should that panel be attached to an AMD GPU.<o:p></o:p></p>
<p class="MsoNormal">>><o:p> </o:p></p>
<p class="MsoNormal">>> So, it would be nice to know exactly what that protocol is meant to do,<o:p></o:p></p>
<p class="MsoNormal">>> if it applies only to this specific panel or anything else with the<o:p></o:p></p>
<p class="MsoNormal">>> same TCON, how one would identify such TCONs in the wild other than<o:p></o:p></p>
<p class="MsoNormal">>> EDID, if it relies on an external PWM or something, etc. And it might<o:p></o:p></p>
<p class="MsoNormal">>> make sense for now to make this a (shudder) driver-specific EDID quirk<o:p></o:p></p>
<p class="MsoNormal">>> rather than match by DPCD, at least until we know if the panel is ever<o:p></o:p></p>
<p class="MsoNormal">>> seen attached to other source devices and if the OUI convention is<o:p></o:p></p>
<p class="MsoNormal">>> self-contained.<o:p></o:p></p>
<p class="MsoNormal">><o:p> </o:p></p>
<p class="MsoNormal">>Thanks for clarifying. Pretty much agreed, unfortunately also on the<o:p></o:p></p>
<p class="MsoNormal">>"would be nice to know more" part...<o:p></o:p></p>
<p class="MsoNormal">><o:p> </o:p></p>
<p class="MsoNormal">>If this were to be an EDID quirk after all, I wonder if it would be<o:p></o:p></p>
<p class="MsoNormal">>better to store the parsed quirks to, say, struct drm_display_info, and<o:p></o:p></p>
<p class="MsoNormal">>have a drm_connector_has_quirk() function similar to drm_dp_has_quirk().<o:p></o:p></p>
<p class="MsoNormal">><o:p> </o:p></p>
<p class="MsoNormal">>This would also allow us to not return quirks from<o:p></o:p></p>
<p class="MsoNormal">>drm_add_display_info(), which would arguably clean up the interface.<o:p></o:p></p>
<p class="MsoNormal">><o:p> </o:p></p>
<p class="MsoNormal">>BR,<o:p></o:p></p>
<p class="MsoNormal">>Jani.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thanks for advice! I've already update patch V2. Driver will check sink OUI<o:p></o:p></p>
<p class="MsoNormal">and confirm TCON's capability to decide to enable this method or not.<o:p></o:p></p>
<p class="MsoNormal">It depends on TCON's feature description and does not export EDID quirk.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Best regards,<o:p></o:p></p>
<p class="MsoNormal">Shawn<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</body>
</html>