Created attachment 100274 [details] [review] churn_output_names.patch Thanks to a Dave Airlie and others in irc for helping me get to the bottom of this. For cards other than "#0" (I'm still quite unclear on where exactly the numbering comes from -- I presume it's the same place as the /dev/dri/card* specials), it is still quite possible to get duplicated output names. This is because the code identified in the source as "for compatibility with UMS output names" does not run; instead, for these non-#0-cards, only the human-readable output type name (i.e.: "DVI", "HDMI", etc.) differentiates the various outputs. Unfortunately these names are not at all unique: const char *output_names[] = { "None", "VGA", "DVI", "DVI", "DVI", "Composite", "S-video", "LVDS", "CTV", "DIN", "DisplayPort", "HDMI", "HDMI", "TV", "eDP" }; The result, depending on your hardware: adding a second card results in duplicate output names in the global randr namespace. Wouldn't be so terrible, except it makes xrandr go utterly berzerk.* Rather than pile more output numbering kludges on top of the ones already in drmmode_output_init, folks in IRC seemed to agree that a better solution would be to go ahead and allow the output names to churn and replace the duplicate values in the above array with unique ones. I posted this to the mailing list some days ago and no flame-war ensued. But, I'm new in town; perhaps bugzilla is a better way to get my patch looked at? * This may represent a second bug in apps/xrandr, as I noticed several third-party consumers of libXrandr continued to work correctly in spite of the duplicate names; however, apps/xrandr always freaked out, even if I only fed it XID's.
You can't change the special case names as this will break the desktop for most people. I had a similar clean up before but it breaks X for too many people. Most desktop environments will just thow up an error saying they can't start up.
(In reply to comment #1) > You can't change the special case names as this will break the desktop for > most people. I had a similar clean up before but it breaks X for too many > people. Most desktop environments will just thow up an error saying they > can't start up. That certainly may have been the case not too long ago. And, I'm not an expert, so perhaps it still is for all I know. But just to make the case for this, consider that the victims are going to be only those whose distributions absorb the 7.4.0 driver, but are still using the Driver-level "Monitor-${OUTPUT_NAME}" trick to map their monitors. Of course there will be a many such people, in absolute terms. But I would guess that these days, a majority of folks running bleeding edge versions of this driver don't rely on xorg.conf at all. Perhaps those that do are mostly aware that this is no longer the mainstream, recommended way to set things up, and are even prepared for a certain amount of upgrade troule. Perhaps not -- but that was my line of thinking. The other argument in favor of churning the output names stems from a look at the code. To preserve the old output names and the new randr 1.4 provider functionality, and to ensure unique names going forward will likely require some sort of matrix of output naming hacks to be maintained in place of the numdvi, numhdmi, etc., variables that are in there now. Even so, this won't prevent people's output names from shuffling around if they plug things into different ports or shuffle around cards in multi-gpu setups. So, to me, it seemed that the best course of action would be to "rip the bandaid off" now. I worried I'd be contributing to a maintainability problem adding more and more hacks to the code-base. Yes some people's setups would break. Undoubtedly when this change hit certain distros there would be some nasty waves of unhappy campers showing up in various support channels with blank screens. But is the alternative better? Extending those UMS-compatibility hacks to also support multi-provider setups seemed like a recipe for continued output naming snafus, quirky corner-case behavior, and so on, which could eventually total up to a greater amount of human suffering than a one-time switch that people could at least be prepared for via release notes, etc. That stated, once could salvage the legacy behavior by creating a second array of output names; the old array would pertain to the "primary" card (however, using the type id's to determine primacy is clearly not correct
(In reply to comment #2) > That stated, once could salvage the legacy behavior by creating a second > array of output names; the old array would pertain to the "primary" card > (however, using the type id's to determine primacy is clearly not correct On second thought, maybe an approach along these lines is not so bad -- one could just hard-code the legacy-compatibility values into the case-statement and fix the array for everything else as in my patch. That should provide cake and eating too -- avoiding massive breakage for mainstream use-cases while fixing name duplication, all without making the code uglier. This does leave the question of how to properly identify the actual primary GPU, to which the legacy-output-naming-compatibility hack should apply. The "id #1 must be the primary card" assumption made, now, fails in my case (second card listed first in xorg.conf in order to make it the main one). Is it safe, by any chance, to assume that whatever drmModeConnector libdrm returns for drmmode->mode_res->connectors[0] would always be the primary one (even in the face of hotplug etc)? If that were the case, drmmode_output_init could just "make a note" (i.e., by means of a new variable passed back and forth to it by drmmode_pre_init) of the connector_type found in its first iteration, and replace the constant "1" currently used to test for primacy (and activate the output numbering hacks), with this remembered value. Or, perhaps there some more "xorg-ly correct" way to pick the primary? If I could get some feedback on that one issue, I'm quite sure I could spin up a better version of my previous patch that addresses the backward-compatibility concern while still fixing the output-name-uniqueness problem.
Created attachment 100721 [details] [review] churn_output_names_try2.patch Nobody answered my question about primary outputs, but ... if the whole point of the compatibility code is just to keep working output names from churning unnecessarily, then I suppose it doesn't actually matter what is "correct" -- only what the old code did. The old code used the constant "1", so we can just keep that as is. This revision attempts to revive the old legacy compatibility code while also fixing the duplicate output names. Somebody other than me should try running this, as I have only done so on my workstation, where the legacy output naming hacks don't kick in at all.
Created attachment 119958 [details] [review] more-output-conflict-avoidance-761.patch Rebase my patch against the latest release. Apparently I'm still the only person with more than one non-SLI radeon card in his or her system using xrandr --setprovideroutputsource. Then again, maybe that's because all the non-hackers that try give up when their /usr/bin/X segfaults, concluding that the feature does not work for dual-radeon configurations (they are more-or-less right). However, note that these folks still have duplicate output names, which must cause all sorts of havoc on their systems. Without the segfault, perhaps they can barely limp along, so long as they don't do anything too interesting with xrandr. Oh, well, tough cookies for them, I guess, since due to the non-obvious etiology of their problems, their chances of finding this bug and applying my patch are poor. As with the previous revision, this one should still preserve the legacy output names perfectly.
Please save your sarcasm and submit the patch to the xorg-driver-ati mailing list for review.
(In reply to Michel Dänzer from comment #6) > ([ed: probably in reply to my comment #5]) > > Oh, well, tough cookies for them, I guess, since due to the > > non-obvious etiology of their problems, their chances of finding > > this bug and applying my patch are poor. > > Please save your sarcasm and submit the patch to the xorg-driver-ati mailing > list for review. Sorry -- I'm not butt-hurt, just trying to make the wheel squeaky enough to get the grease. Reading back over my old comments, I wasn't sure I'd ever clearly communicated how effectively this breaks my box*. Meanwhile, my little fork has gotten a bit more difficult to comprehend every time I've revised it**, which concerns me, since it will be harder to get eyeballs on it. -gmt *Every six months or so, my X server goes mysteriously ape-kaka. I boot rescue media & scratch my head for a couple of hours... aha, a new version of the radeon driver snuck in and relapsed my duplicate-display-names-disease. Time to merge my patch, again, which, if I don't have any laptops or the like handy, I wind up doing at framebuffer-console... **By now, even I have a hard time grokking exactly how it works without some kind of side-by-side patch viewer. I probably should refactor it, at this point, into multiple patches.
oops, forgot the most important message: (In reply to Michel Dänzer from comment #6) > submit the patch to the xorg-driver-ati mailing list for review. will do.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xorg/driver/xf86-video-ati/issues/105.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.