[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230821173320.GA362442@bhelgaas>
Date: Mon, 21 Aug 2023 12:33:20 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: suijingfeng <suijingfeng@...ngson.cn>
Cc: linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
dri-devel@...ts.freedesktop.org, loongson-kernel@...ts.loongnix.cn,
"Limonciello, Mario" <Mario.Limonciello@....com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v4] PCI/VGA: Make the vga_is_firmware_default() less
arch-dependent
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.
Bjorn
Powered by blists - more mailing lists