On Mon, May 13, 2019 at 12:14:29PM +0200, Michel Dänzer wrote: > 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. Hm I missed that this is about 2 external pci slots. Agreeing with you reasoning here now, retracting my r-b. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
