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

From: Daniel Vetter <daniel-/w4YWyX8dFk@public.gmane.org>
To: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>,
	Aaron Ma <aaron.ma-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH] Revert "vgaarb: Keep adding VGA device in queue"
Date: Fri, 10 May 2019 17:42:08 +0200	[thread overview]
Message-ID: <20190510154208.GL17751@phenom.ffwll.local> (raw)
In-Reply-To: <20190510142958.28017-1-alexander.deucher-5C7GfCeVMHo@public.gmane.org>

On Fri, May 10, 2019 at 09:29:58AM -0500, Alex Deucher wrote:
> This breaks multiple graphics cards in the Amigaone x5000
> on PPC.
> 
> This reverts commit 3d42f1ddc47a69c0ce155f9f30d764c4d689a5fa.
> 
> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=109345
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> CC: Aaron Ma <aaron.ma@canonical.com>

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:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/vga/vgaarb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> index f2f3ef8af271..8a3c45219a2a 100644
> --- a/drivers/gpu/vga/vgaarb.c
> +++ b/drivers/gpu/vga/vgaarb.c
> @@ -725,7 +725,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
>  	vga_arbiter_check_bridge_sharing(vgadev);
>  
>  	/* Add to the list */
> -	list_add_tail(&vgadev->list, &vga_list);
> +	list_add(&vgadev->list, &vga_list);
>  	vga_count++;
>  	vgaarb_info(&pdev->dev, "VGA device added: decodes=%s,owns=%s,locks=%s\n",
>  		vga_iostate_to_str(vgadev->decodes),
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx