Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Message-ID: <59d609eb-e52c-7e81-073c-3e2f61e04583@loongson.cn>
Date:   Wed, 4 Oct 2023 21:38:01 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH pci-next v6 1/2] PCI/VGA: Make the
 vga_is_firmware_default() less arch-dependent
Content-Language: en-US
To:     Bjorn Helgaas <helgaas@kernel.org>
Cc:     Bjorn Helgaas <bhelgaas@google.com>, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20231003155420.GA674372@bhelgaas>
From:   Sui Jingfeng <suijingfeng@loongson.cn>
In-Reply-To: <20231003155420.GA674372@bhelgaas>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,


On 2023/10/3 23:54, Bjorn Helgaas wrote:
> PCI/VGA: Match firmware framebuffer before BAR reassignment
>
> vga_is_firmware_default() decides a device is the firmware default VGA
> device if it has a BAR that contains the framebuffer described by
> screen_info.
>
> Previously this was unreliable because the screen_info framebuffer address
> comes from firmware, and the Linux PCI core may reassign device BARs before
> vga_is_firmware_default() runs.  This reassignment means the BAR may not
> match the screen_info values, but we still want to select the device as the
> firmware default.
>
> Make vga_is_firmware_default() more reliable by running it as a quirk so it
> happens before any BAR reassignment.


On a specific architecture, the kernel have its own copy of screen_info.
We may choose to rely on the global screen_info, but I don't hope vgaarb
should completely depend on the firmware which without any flexibility.
After all, there are always have the old BIOS with the newer Kernel or
new graphics card combinations. That is to say that the BIOS(firmware)
is released first, then the new graphics comes after. So, we can't rely
on the BIOS know all of the graphics card in the world at the time of
BIOS release. If a BIOS don't know a specific video card, then then BIOS
is unlikely initialize a firmware framebuffer(UEFI GOP or something like
that) on it successfully.

I hope vgaarb can work without fully rely on the firmware, don't mention
the word 'firmware' as less as possible. Because, we can even modify the
kernel's screen_info by hardcode or fill it by parsing DT. We are only
rely on the global screen_info here. while how does the global screen_info
being filled is a kernel side and arch specific thing. The global screen_info
may rely on the BIOS, but this is a thing of the kernel side, vgaarb belong
to drivers directory.

Beside, the word 'firmware' have multiple meanings, for the patch at here,
it refer to the UEFI or uboot or something similar. But for the GPU domain,
it may refer to any binary executable on the macro-controller embedded in
the GPU IP. So I think the names 'is_firmware_default' and 'vga_is_firmware_default'
are putting *too much emphasis* the firmware. So I want to remove it.

vgaarb already have been exposed to userspace via sysfs interface (/sys/class/misc/vga_arbiter).
So the original spirit is actually allow user to tune or control. I hope vgaarb become more
standalone and more flexible so that there are some ways to rescue even in the worse case.
If someone (who have a better background or have better understanding toward a specific case only)
don't see this as a problem, then it is not my problem. I'm not good at debating with English.