Re: [PATCH v3 0/3] Split default display handling out from VGA arbiter

From: Daniel Axtens <dja@axtens.net>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: lorenzo.pieralisi@arm.com, catalin.marinas@arm.com,
	gabriele.paoloni@huawei.com, ard.biesheuvel@linaro.org,
	linux-pci@vger.kernel.org, will.deacon@arm.com,
	dri-devel@lists.freedesktop.org, z.liuxinliang@hisilicon.com,
	alex.williamson@redhat.com, zourongrong@gmail.com,
	daniel.vetter@intel.com, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 0/3] Split default display handling out from VGA arbiter
Date: Tue, 26 Sep 2017 09:35:40 +1000	[thread overview]
Message-ID: <87zi9ixsfn.fsf@linkitivity.dja.id.au> (raw)
In-Reply-To: <20170925163948.GD15970@bhelgaas-glaptop.roam.corp.google.com>

Hi Bjorn,

> But I think trying to split the "default device" part out from the VGA
> arbiter ends up being overkill and making things more complicated
> instead of simpler.
Fair enough.

> Would something like the following work for you as well as the powerpc
> case?  On powerpc, we already use vga_set_default_device() to select a
> device that doesn't use legacy VGA resources, so maybe we can just do
> the same on ARM64?

It looks good. I'll try to get some time on the test system to test it
and I'll pester my friends at IBM to give it a go as well.

> I suppose there might be wrinkles in how the arbiter deals with
> multiple graphics devices on those systems, since I don't think it
> identifies these devices that don't use the legacy resources, but it
> seems like we live with whatever those on are powerpc and probably can
> on ARM64 as well.

I would say so, yes.

Thanks for sticking with this!

Regards,
Daniel

>
>
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 02831a396419..0ac7aa346c69 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1740,15 +1740,3 @@ static void fixup_hide_host_resource_fsl(struct pci_dev *dev)
>  }
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl);
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl);
> -
> -static void fixup_vga(struct pci_dev *pdev)
> -{
> -	u16 cmd;
> -
> -	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> -	if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
> -		vga_set_default_device(pdev);
> -
> -}
> -DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
> -			      PCI_CLASS_DISPLAY_VGA, 8, fixup_vga);
> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> index 76875f6299b8..9df4802c5f04 100644
> --- a/drivers/gpu/vga/vgaarb.c
> +++ b/drivers/gpu/vga/vgaarb.c
> @@ -1468,6 +1468,21 @@ static int __init vga_arb_device_init(void)
>  			vgaarb_info(dev, "no bridge control possible\n");
>  	}
>  
> +	if (!vga_default_device()) {
> +		list_for_each_entry(vgadev, &vga_list, list) {
> +			struct device *dev = &vgadev->pdev->dev;
> +			u16 cmd;
> +
> +			pdev = vgadev->pdev;
> +			pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> +			if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
> +				vgaarb_info(dev, "setting as boot device\n");
> +				vga_set_default_device(pdev);
> +				break;
> +			}
> +		}
> +	}
> +
>  	pr_info("loaded\n");
>  	return rc;
>  }
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel