[PATCH] Revert "vgaarb: Keep adding VGA device in queue"

[Michel Dänzer]

On 2019-05-10 8:01 p.m., Aaron Ma wrote:
> On 5/10/19 11:46 PM, Michel Dänzer wrote:
>>> Given that the bug is a bit a mess I think we need to add a bit more
>>> context here in the commit message. My understanding:
>>>
>>> Goal of the revert commit was to make the integrated boot device the
>>> primary one, if we can't detect which one is the boot device, instead of
>>> the last one. Which makes some sense.
>>>
>>> Now people have relied on the kernel picking the last one, which usually
>>> is an add-on card, and therefore simply plugging in an add-on card allows
>>> them to overwrite the default choice. Which also makes sense, and since
>>> it's the older behaviour, wins.
>>>
>>> I think it'd be good to add a comment here that this behaviour has become
>>> uapi, e.g.
>>>
>>> 	/* Add at the front so that we pick the last device as fallback
>>> 	 * default, with the usual result that plug in cards are preferred
>>> 	 * over integrated graphics. */
>>>
>>> With that (or similar) and more commit message context:
>> The bug reporter's system doesn't have integrated graphics though, just
>> two plug-in cards. It's not clear to me yet that their expectation of
>> Xorg to pick any particular one of them without configuration was justified.
> 
> 
> This code is kind of fail safe.
> 
> When in hybrid graphic mode.
> It should fallback to integrated GPU, which should always be primary fb.
> So picking 1st (minor PCI ID number) GPU should make more sense.
> 
> When with multiple d-GPUs, we can't say which card should be the right
> one for users when no Xorg conf set.
> Usually BIOS will set the 1st (minor PCI ID number) d-GPU as primary.
> So aligning with BIOS setting will be better I think.

Right. The bug description even says that the card the reporter expected
Xorg to come up on is the "secondary" card in a PCIe 1x slot (whereas
the primary is in PCIe 16x). It seems pretty clear that your change
actually made things better, and the reporter was just relying on the
previous wrong behaviour. Therefore I resolved the report as not a bug,
and this patch should be dropped.


-- 
Earthling Michel Dänzer               |              https://www.amd.com
Libre software enthusiast             |             Mesa and X developer