Re: [PATCH v4] PCI/VGA: Make the vga_is_firmware_default() less arch-dependent

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,


On 2023/8/22 01:33, Bjorn Helgaas wrote:
On Fri, Aug 18, 2023 at 09:48:46AM +0800, suijingfeng wrote:
On 2023/8/18 06:08, Bjorn Helgaas wrote:
+		if (resource_type(res) != IORESOURCE_MEM)
+			continue;
+
+		if (!res->start || !res->end)
+			continue;
+
+		if (res->start <= fb_start && fb_end <= res->end) {
+			pdev_boot_vga = pdev;
+
+			vgaarb_info(&pdev->dev,
+				    "BAR %d contains firmware FB\n", i);
Print the BAR with %pR and include the framebuffer region from
screen_info in the same format.
I do remember that you already told me to do this in V3, sorry for not
replying to you at V3. Most of the time, what you tell me is right.But here,
I think I need to explain. Because doing it that way will make the code line
too long,and it will exceed 80 characters in the column if we print too
much.
I believe that the vgaarb_info() at here is already the most compact and
simplest form. Printing the BAR with %pR is not absolute necessary, because
we can get the additional information by: $ lspci | grep VGA

$ dmesg | grep 05:00.0
$ dmesg | grep 0000:03:00.0
$ dmesg | grep PCI
Fair enough.  The BAR info is already there.  But I don't think the
screen_info framebuffer data is in the dmesg log anywhere, and I think
that would be useful.

It's fine if dmesg lines or kernel printk lines exceed 80 columns.

Actually, I only add something that is absolute necessary.
Printing BAR with %pR and/or Printing the framebuffer region
is consider to only for *debugging* purpose.
I think printing the framebuffer region is important because it makes
it clear *why* we're selecting the device as the default VGA device.
It's more than just debugging; it helps make the system more
transparent and more understandable.

OK, I'm clear what to do next.
The printing will be added at the next version.

Bjorn




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux