From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andreas Noever Subject: Re: [PATCH 1/2 v2] vgaarb: Don't default exclusively to first video device with mem+io Date: Tue, 26 Aug 2014 17:32:24 +0200 Message-ID: References: <20140514224339.7f8be3a9@neptune.home> <20140625005501.7ff7e982@neptune.home> <20140705171503.GC6247@google.com> <20140810112654.1bf684d6@neptune.home> <20140810183411.19370721@neptune.home> <20140816192135.34260115@neptune.home> <20140820075508.74f5b622@pluto> <20140820091152.76cd4e1a@pluto> <20140821233435.19a9cffa@neptune.home> <20140822082324.12cb6e93@pluto> <20140823130624.06ecc057@neptune.home> <20140824230953.7c04890f@neptune.home> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <20140824230953.7c04890f@neptune.home> Sender: linux-pci-owner@vger.kernel.org To: =?UTF-8?Q?Bruno_Pr=C3=A9mont?= Cc: DRI mailing list , Matthew Garrett , Linux PCI , Greg Kroah-Hartman , Bjorn Helgaas List-Id: dri-devel@lists.freedesktop.org On Sun, Aug 24, 2014 at 11:09 PM, Bruno Pr=C3=A9mont wrote: > With commit 20cde694027e boot video device detection was moved from > efifb to x86 and ia64 pci/fixup.c. > > For dual-GPU Apple computers above change represents a regression as > code in efifb did forcefully override vga_default_device while the > merge did not (vgaarb happens prior to PCI fixup). > > To improve on initial device selection by vgaarb (it cannot know if > PCI device not behind bridges see/decode legacy VGA I/O or not), move > the screen_info based check from pci_video_fixup to vgaarb's init > function and use it to refine/override decision taken while adding > the individual PCI VGA devices. > This way PCI fixup has no reason to adjust vga_default_device > anymore but can depend on its value for flagging shadowed VBIOS. > > This has the nice benefit of removing duplicated code but does > introduce a #if defined() block in vgaarb. > Not all architectures have screen_info and would cause compile to > fail without it. > > Reported-By: Andreas Noever > CC: Matthew Garrett > CC: stable@vger.kernel.org # v3.5+ > Signed-off-by: Bruno Pr=C3=A9mont > --- > Andreas, does this work properly for you, including the improvement > on i915 complaint about VBIOS going from KERN_ERR to KERN_INFO? Yep, thanks! > > Other arches using PCI and vgaarb that have screen_info may want > to be added to the #if defined() block or even introduce a new > CONFIG_HAVE_SCREEN_INFO or similar... > > > arch/ia64/pci/fixup.c | 24 +----------------------- > arch/x86/pci/fixup.c | 24 +----------------------- > drivers/gpu/vga/vgaarb.c | 38 +++++++++++++++++++++++++++++++++++++- > 3 files changed, 39 insertions(+), 47 deletions(-) > > diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c > index ec73b2c..fc505d5 100644 > --- a/arch/ia64/pci/fixup.c > +++ b/arch/ia64/pci/fixup.c > @@ -38,27 +38,6 @@ static void pci_fixup_video(struct pci_dev *pdev) > return; > /* Maybe, this machine supports legacy memory map. */ > > - if (!vga_default_device()) { > - resource_size_t start, end; > - int i; > - > - /* Does firmware framebuffer belong to us? */ > - for (i =3D 0; i < DEVICE_COUNT_RESOURCE; i++) { > - if (!(pci_resource_flags(pdev, i) & IORESOURC= E_MEM)) > - continue; > - > - start =3D pci_resource_start(pdev, i); > - end =3D pci_resource_end(pdev, i); > - > - if (!start || !end) > - continue; > - > - if (screen_info.lfb_base >=3D start && > - (screen_info.lfb_base + screen_info.lfb_s= ize) < end) > - vga_set_default_device(pdev); > - } > - } > - > /* Is VGA routed to us? */ > bus =3D pdev->bus; > while (bus) { > @@ -83,8 +62,7 @@ static void pci_fixup_video(struct pci_dev *pdev) > pci_read_config_word(pdev, PCI_COMMAND, &config); > if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) { > pdev->resource[PCI_ROM_RESOURCE].flags |=3D I= ORESOURCE_ROM_SHADOW; > - dev_printk(KERN_DEBUG, &pdev->dev, "Boot vide= o device\n"); > - vga_set_default_device(pdev); > + dev_printk(KERN_DEBUG, &pdev->dev, "Video dev= ice with shadowed ROM\n"); > } > } > } > diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c > index c61ea57..9a2b710 100644 > --- a/arch/x86/pci/fixup.c > +++ b/arch/x86/pci/fixup.c > @@ -326,27 +326,6 @@ static void pci_fixup_video(struct pci_dev *pdev= ) > struct pci_bus *bus; > u16 config; > > - if (!vga_default_device()) { > - resource_size_t start, end; > - int i; > - > - /* Does firmware framebuffer belong to us? */ > - for (i =3D 0; i < DEVICE_COUNT_RESOURCE; i++) { > - if (!(pci_resource_flags(pdev, i) & IORESOURC= E_MEM)) > - continue; > - > - start =3D pci_resource_start(pdev, i); > - end =3D pci_resource_end(pdev, i); > - > - if (!start || !end) > - continue; > - > - if (screen_info.lfb_base >=3D start && > - (screen_info.lfb_base + screen_info.lfb_s= ize) < end) > - vga_set_default_device(pdev); > - } > - } > - > /* Is VGA routed to us? */ > bus =3D pdev->bus; > while (bus) { > @@ -371,8 +350,7 @@ static void pci_fixup_video(struct pci_dev *pdev) > pci_read_config_word(pdev, PCI_COMMAND, &config); > if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) { > pdev->resource[PCI_ROM_RESOURCE].flags |=3D I= ORESOURCE_ROM_SHADOW; > - dev_printk(KERN_DEBUG, &pdev->dev, "Boot vide= o device\n"); > - vga_set_default_device(pdev); > + dev_printk(KERN_DEBUG, &pdev->dev, "Video dev= ice with shadowed ROM\n"); > } > } > } > diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c > index d2077f0..cffdff9 100644 > --- a/drivers/gpu/vga/vgaarb.c > +++ b/drivers/gpu/vga/vgaarb.c > @@ -41,6 +41,7 @@ > #include > #include > #include > +#include > > #include > > @@ -585,8 +586,11 @@ static bool vga_arbiter_add_pci_device(struct pc= i_dev *pdev) > */ > #ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE > if (vga_default =3D=3D NULL && > - ((vgadev->owns & VGA_RSRC_LEGACY_MASK) =3D=3D VGA_RSRC_LE= GACY_MASK)) > + ((vgadev->owns & VGA_RSRC_LEGACY_MASK) =3D=3D VGA_RSRC_LE= GACY_MASK)) { > + pr_info("vgaarb: setting as boot device: PCI:%s\n", > + pci_name(pdev)); > vga_set_default_device(pdev); > + } > #endif > > vga_arbiter_check_bridge_sharing(vgadev); > @@ -1320,6 +1324,38 @@ static int __init vga_arb_device_init(void) > pr_info("vgaarb: loaded\n"); > > list_for_each_entry(vgadev, &vga_list, list) { > +#if defined(CONFIG_X86) || defined(CONFIG_IA64) > + /* Override I/O based detection done by vga_arbiter_a= dd_pci_device() > + * as it may take the wrong device (e.g. on Apple sys= tem under EFI). > + * > + * Select the device owning the boot framebuffer if t= here is one. > + */ > + resource_size_t start, end; > + int i; > + > + /* Does firmware framebuffer belong to us? */ > + for (i =3D 0; i < DEVICE_COUNT_RESOURCE; i++) { > + if (!(pci_resource_flags(vgadev->pdev, i) & I= ORESOURCE_MEM)) > + continue; > + > + start =3D pci_resource_start(vgadev->pdev, i)= ; > + end =3D pci_resource_end(vgadev->pdev, i); > + > + if (!start || !end) > + continue; > + > + if (screen_info.lfb_base < start || > + (screen_info.lfb_base + screen_info.lfb_s= ize) >=3D end) > + continue; > + if (!vga_default_device()) > + pr_info("vgaarb: setting as boot devi= ce: PCI:%s\n", > + pci_name(vgadev->pdev)); > + else if (vgadev->pdev !=3D vga_default_device= ()) > + pr_info("vgaarb: overriding boot devi= ce: PCI:%s\n", > + pci_name(vgadev->pdev)); > + vga_set_default_device(vgadev->pdev); > + } > +#endif > if (vgadev->bridge_has_one_vga) > pr_info("vgaarb: bridge control possible %s\n= ", pci_name(vgadev->pdev)); > else > -- > 1.8.5.5 >