On Fri, Aug 18, 2023 at 12:09:29PM +0800, suijingfeng wrote:
On 2023/8/18 06:08, Bjorn Helgaas wrote:Of course. If we make the patch simple and the commit log simple by
On Wed, Aug 16, 2023 at 06:05:27AM +0800, Sui Jingfeng wrote:Yes, what you said is right in overall.
Currently, the vga_is_firmware_default() function only works on x86 andAs far as I can tell, this is basically identical to the existing
ia64, it is a no-op on ARM, ARM64, PPC, RISC-V, etc. This patch completes
the implementation for the rest of the architectures. The added code tries
to identify the PCI(e) VGA device that owns the firmware framebuffer
before PCI resource reallocation happens.
vga_is_firmware_default(), except that this patch funs that code as a
header fixup, so it happens before any PCI BAR reallocations happen.
But I think I should mention a few tiny points that make a difference.
1) My version is *less arch-dependent*
removing extraneous details, this will all be obvious.
2) My version focus on the address in ranges, weaken the size parameter.Whether it's start/size or start/end is a trivial question. We don't
Which make the code easy to read and follow the canonical convention to
express the address range. while the vga_is_firmware_default() is not.
need to waste time on it now.
3) A tiny change make a big difference.I don't want both mechanisms when only one of them is useful. PCI BAR
The original vga_is_firmware_default() only works with the assumption
that the PCI resource reallocation won't happens. While I see no clue
that why this is true even on X86 and IA64. The original patch[1] not
mention this assumption explicitly.
[1] 86fd887b7fe3 ('vgaarb: Don't default exclusively to first video device with mem+io')
That sounds like a good idea, because this is all based on theYes.
framebuffer in screen_info, and screen_info was initialized before PCI
enumeration, and it certainly doesn't account for any BAR changes done
by the PCI core.
So why would we keep vga_is_firmware_default() at all? If the headerIt need another patch to do the cleanup work, while my patch just
fixup has already identified the firmware framebuffer, it seems
pointless to look again later.
add code to solve the real problem. It focus on provide a solution
for the architectures which have a decent way set up the
screen_info. Other things except that is secondary.
reassignment is completely fine, and keeping the assumption in
vga_is_firmware_default() that we can compare reassigned BAR values to
the pre-reassignment screen_info range is a trap that we should
remove.
Bjorn