Re: [PATCH v5 2/9] video/aperture: use generic code to figure out the vga default device

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Apr 11, 2023 at 05:25:47PM +0200, Helge Deller wrote:
> On 4/11/23 16:36, Daniel Vetter wrote:
> > On Fri, Apr 07, 2023 at 10:54:00PM +0200, Helge Deller wrote:
> > > On 4/6/23 15:21, Thomas Zimmermann wrote:
> > > > From: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > 
> > > > Since vgaarb has been promoted to be a core piece of the pci subsystem
> > > > we don't have to open code random guesses anymore, we actually know
> > > > this in a platform agnostic way, and there's no need for an x86
> > > > specific hack. See also commit 1d38fe6ee6a8 ("PCI/VGA: Move vgaarb to
> > > > drivers/pci")
> > > > 
> > > > This should not result in any functional change, and the non-x86
> > > > multi-gpu pci systems are probably rare enough to not matter (I don't
> > > > know of any tbh). But it's a nice cleanup, so let's do it.
> > > > 
> > > > There's been a few questions on previous iterations on dri-devel and
> > > > irc:
> > > > 
> > > > - fb_is_primary_device() seems to be yet another implementation of
> > > >     this theme, and at least on x86 it checks for both
> > > >     vga_default_device OR rom shadowing. There shouldn't ever be a case
> > > >     where rom shadowing gives any additional hints about the boot vga
> > > >     device, but if there is then the default vga selection in vgaarb
> > > >     should probably be fixed. And not special-case checks replicated all
> > > >     over.
> > > > 
> > > > - Thomas also brought up that on most !x86 systems
> > > >     fb_is_primary_device() returns 0, except on sparc/parisc. But these
> > > >     2 special cases are about platform specific devices and not pci, so
> > > >     shouldn't have any interactions.
> > > 
> > > Nearly all graphics cards on parisc machines are actually PCI cards,
> > > but the way we handle the handover to graphics mode with STIcore doesn't
> > > conflicts with your planned aperture changes.
> > > So no problem as far as I can see for parisc...
> > 
> > Ah I thought sticore was some very special bus, if those can be pci cards
> 
> STI stands for "Standard Text Interface" [1], which is a API of ROM functions
> to output text chars on a console. It's comparable to the text output functions
> in a PC-BIOS on x86 and dependend on the ROM it drives any supported card which has
> a parisc ROM. So, STI supports cards on PCI & AGP busses, as well on older GSC busses.
> [1] https://parisc.wiki.kernel.org/images-parisc/e/e3/Sti.pdf
> 
> > underneath then I guess some cleanup eventually might be a good idea? For
> > anything with a pci bus it's rather strange when vgaarb and
> > fb_is_primary_device() aren't a match ...
> 
> There is no VGA on parisc, so there is no conflict. Cards come either with
> a parisc STI ROM to support text mode, or they will only be used as secondary
> cards only.  The graphics mode is only done in userspace by specific drivers, e.g.
> by the X11 server in HP-UX.
> Even on x86 the BIOS will only show text output if the graphics card comes
> with a VGA-compatible BIOS.

tbf after reading vt.c and fbcon.c some more I'm pretty sure my patch is
nonsense. As sure as you can be with vt/fbcon :-/

Since it sounds like it's a driver bug, maybe a patch to do a pr_warn in
register_framebuffer if there's no mode? I read a pile of code around
modedb.c right now, and there's a lot of things that silently assume that
you always have a mode. So would be good thing to check.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch