On Mon, 07 Oct 2019, Adam Jackson <ajax@xxxxxxxxxx> wrote: > On Mon, 2019-10-07 at 12:08 +0300, Jani Nikula wrote: > >> The problem with the EDID quirks is that exposing the quirks sticks out >> like a sore thumb. Thus far all of it has been contained in drm_edid.c >> and they affect how the EDID gets parsed, for all drivers. Obviously >> this could be changed, but is it the right thing to do? >> >> What I suggested was, check the OUI only, and if it matches, do >> more. Perhaps there's something in the 0x300 range of DPCD offsets that >> you can read? Or perhaps you need to write the source OUI first, and >> then do that. > > My issue isn't really with identifying the panel from EDID rather than > DPCD, whichever identifier is most specific is probably the best thing > to use. It's more that this quirk is identified in common code but only > applied in one driver. If this panel were ever to be attached to some > other source, they might well want to apply the same kind of fix. My > (admittedly naïve) reading of the OUI handshake process is that when > the source device writes an OUI to DP_SOURCE_OUI it is telling the sink > "I'm about to issue commands that conform to _this_ vendor's own > conventions". If that convention communicates information that is > entirely contained within AUXCH transactions (and doesn't, for example, > require looking at some other strapping pin or external device) then in > principle it doesn't matter if the source device "matches" that OUI; it > would be legal for an AMD GPU to write the same sequence and expect the > same reaction, should that panel be attached to an AMD GPU. > > So, it would be nice to know exactly what that protocol is meant to do, > if it applies only to this specific panel or anything else with the > same TCON, how one would identify such TCONs in the wild other than > EDID, if it relies on an external PWM or something, etc. And it might > make sense for now to make this a (shudder) driver-specific EDID quirk > rather than match by DPCD, at least until we know if the panel is ever > seen attached to other source devices and if the OUI convention is > self-contained. Thanks for clarifying. Pretty much agreed, unfortunately also on the "would be nice to know more" part... If this were to be an EDID quirk after all, I wonder if it would be better to store the parsed quirks to, say, struct drm_display_info, and have a drm_connector_has_quirk() function similar to drm_dp_has_quirk(). This would also allow us to not return quirks from drm_add_display_info(), which would arguably clean up the interface. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
