Re: [Qemu-devel] [PATCH v3] hw/display: Add basic ATI VGA emulation

[Gerd Hoffmann]

> > Attached patch creates two separate devices.  It's just some QOM
> > boilerplate, they still share 95% of the code.
> 
> Thanks. Do you want me to merge this and submit a new version or you want to
> fix up after merging my patch or what should we do with this? (I'm not a fan
> of adding a new function for every new model of these devices so I'd
> probably get rid of the is_ati_* functions and still compare to device id
> internally maybe via a single is_ati_device(id) function though. I also find
> ati-rage128pro a bit too long, I liked the ati-vga better with selecting
> variants via a property but if you think it's better with more QOM
> boilerplate and adding every variant to the default_list[] one by one then I
> can live with that. How about ati-r128pro at least instead?

Just apply the patch to your branch and tidy up as you want.  Shorten
the rage name is fine with me.

I don't expect we'll add too many variants, getting the two work
properly will be difficult enough, so the QOM boilerplate shouldn't be
much of a problem.  I think it'll be more user-friendly to just have two
or three ati devices instead of expecting them to dig out the correct
id.

> >  With that in place
> > vgabios works just fine for both devices.
> 
> But I don't get why is the above needed for this. By the time the client
> runs (after device realize) both should look the same: your QOM subclasses
> and my ati-vga class that sets device_id based on property. Where's the
> difference that makes one work and other doesn't?

See pci_patch_ids() in hw/pci/pci.c

That gets vgabios-stdvga going with the ati cards (as long as the bochs
interface is available), by setting the vendor and device IDs so seabios
accepts and loads the rom.

That requires the device ID being set at that point though.
Specifically setting it in reset handler is too late, and I think doing
it in realize() is too late too.  With two QOM types it is already set,
thats why this gets the vgabios work with the rv100.

> > Neither radeonfb.ko nor radeon.ko (drm driver) can handle the rv100.
> > Loading radeonfb results in a kernel panic.  radeon.ko prints an
> > initialization error.  Seems at least radeonfb tries to pull some info
> > out of the bios image ...
> 
> Is there a live cd iso or something simple to test this with?

No.  Problem is modern distros tend to not build these drivers any more.

I have a script which creates a big initrd with a small distro install
plus a self-compiled kernel, for testing my kernel patches.  This is
what I have used here too.

> I think the ATI option ROMs have some mode timings that are needed to
> program PLLs for setting modes and drivers extract this from the BIOS which
> has some predefined structure so if a driver relies on this it may have a
> problem when we don't have a BIOS with the data it expects. But if it does
> not check BIOS format and just pulls junk out of unexpected BIOS image and
> puts that in clock registers that should not matter as those are ignored
> anyway. I think rv100 will probably need a separate vgabios that looks more
> like what drivers expect but I don't have plans yet to implement that.

For radeonfb this might work.
The radeon drm driver error messaga was quite unspecific.

cheers,
  Gerd