Bug 79531

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.