| Message ID | 20181207160806.13569-3-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | macppc: minor patches for 4.0 | expand |
On Fri, Dec 07, 2018 at 04:08:05PM +0000, Mark Cave-Ayland wrote: > This is in preparation for some upcoming QEMU NDRV driver changes that pass > display information from the host to the guest. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> This looks fine by my limited knowledge of this area. I'm slightly perturbed I can't see any existing examples in the tree of setting the edid property from the machine. > --- > hw/ppc/mac_newworld.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c > index 14273a123e..df0a2f03ff 100644 > --- a/hw/ppc/mac_newworld.c > +++ b/hw/ppc/mac_newworld.c > @@ -430,7 +430,10 @@ static void ppc_core99_init(MachineState *machine) > } > } > > - pci_vga_init(pci_bus); > + dev = qdev_create(BUS(pci_bus), "VGA"); > + qdev_prop_set_int32(dev, "addr", -1); > + qdev_prop_set_bit(dev, "edid", true); > + qdev_init_nofail(dev); > > if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) { > graphic_depth = 15;
On 10/12/2018 03:46, David Gibson wrote: > On Fri, Dec 07, 2018 at 04:08:05PM +0000, Mark Cave-Ayland wrote: >> This is in preparation for some upcoming QEMU NDRV driver changes that pass >> display information from the host to the guest. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > > This looks fine by my limited knowledge of this area. I'm slightly > perturbed I can't see any existing examples in the tree of setting the > edid property from the machine. Certainly both patches work in that the corresponding MemoryRegion appears in both machines when applied, and indeed now also I have guest code that can read from it too. ATB, Mark.
On Fri, Dec 07, 2018 at 04:08:05PM +0000, Mark Cave-Ayland wrote: > This is in preparation for some upcoming QEMU NDRV driver changes that pass > display information from the host to the guest. > - pci_vga_init(pci_bus); > + dev = qdev_create(BUS(pci_bus), "VGA"); > + qdev_prop_set_int32(dev, "addr", -1); > + qdev_prop_set_bit(dev, "edid", true); > + qdev_init_nofail(dev); Hmm. IMO you should not overwrite the device defaults here. edid is off by default only because it is new and I'm conservative. I want a release (or two) with it being available for user testing. If no issues pop up flip it to default on. cheers, Gerd
On 12/12/2018 08:32, Gerd Hoffmann wrote: > On Fri, Dec 07, 2018 at 04:08:05PM +0000, Mark Cave-Ayland wrote: >> This is in preparation for some upcoming QEMU NDRV driver changes that pass >> display information from the host to the guest. > >> - pci_vga_init(pci_bus); >> + dev = qdev_create(BUS(pci_bus), "VGA"); >> + qdev_prop_set_int32(dev, "addr", -1); >> + qdev_prop_set_bit(dev, "edid", true); >> + qdev_init_nofail(dev); > > Hmm. IMO you should not overwrite the device defaults here. > > edid is off by default only because it is new and I'm conservative. > I want a release (or two) with it being available for user testing. > If no issues pop up flip it to default on. Oh, okay. I already have some unreleased guest code that makes use of this, so my questions would be: how can EDID be enabled from the command line for in-built VGA devices, and how do I detect whether EDID support is present from the guest? Otherwise a guest driver that assumes it is always present and tries to read from that area of memory will crash. ATB, Mark.
On Wed, Dec 12, 2018 at 08:54:37AM +0000, Mark Cave-Ayland wrote: > On 12/12/2018 08:32, Gerd Hoffmann wrote: > > > On Fri, Dec 07, 2018 at 04:08:05PM +0000, Mark Cave-Ayland wrote: > >> This is in preparation for some upcoming QEMU NDRV driver changes that pass > >> display information from the host to the guest. > > > >> - pci_vga_init(pci_bus); > >> + dev = qdev_create(BUS(pci_bus), "VGA"); > >> + qdev_prop_set_int32(dev, "addr", -1); > >> + qdev_prop_set_bit(dev, "edid", true); > >> + qdev_init_nofail(dev); > > > > Hmm. IMO you should not overwrite the device defaults here. > > > > edid is off by default only because it is new and I'm conservative. > > I want a release (or two) with it being available for user testing. > > If no issues pop up flip it to default on. > > Oh, okay. I already have some unreleased guest code that makes use of this, so my > questions would be: how can EDID be enabled from the command line for in-built VGA > devices, "-global VGA.edid=on" should do. > and how do I detect whether EDID support is present from the guest? Check whenever the header is present. The edid blob has a fixed 8 byte header (00 FF FF FF FF FF FF 00). The linux kernel driver is lazy and checks the first two bytes only. > Otherwise a guest driver that assumes it is always present and tries > to read from that area of memory will crash. Oh, reading should work no matter what. You just don't find valid edid data there if it is turned off. cheers, Gerd
On 12/12/2018 13:38, Gerd Hoffmann wrote: > On Wed, Dec 12, 2018 at 08:54:37AM +0000, Mark Cave-Ayland wrote: >> On 12/12/2018 08:32, Gerd Hoffmann wrote: >> >>> On Fri, Dec 07, 2018 at 04:08:05PM +0000, Mark Cave-Ayland wrote: >>>> This is in preparation for some upcoming QEMU NDRV driver changes that pass >>>> display information from the host to the guest. >>> >>>> - pci_vga_init(pci_bus); >>>> + dev = qdev_create(BUS(pci_bus), "VGA"); >>>> + qdev_prop_set_int32(dev, "addr", -1); >>>> + qdev_prop_set_bit(dev, "edid", true); >>>> + qdev_init_nofail(dev); >>> >>> Hmm. IMO you should not overwrite the device defaults here. >>> >>> edid is off by default only because it is new and I'm conservative. >>> I want a release (or two) with it being available for user testing. >>> If no issues pop up flip it to default on. >> >> Oh, okay. I already have some unreleased guest code that makes use of this, so my >> questions would be: how can EDID be enabled from the command line for in-built VGA >> devices, > > "-global VGA.edid=on" should do. > >> and how do I detect whether EDID support is present from the guest? > > Check whenever the header is present. The edid blob has a fixed 8 byte > header (00 FF FF FF FF FF FF 00). The linux kernel driver is lazy and > checks the first two bytes only. > >> Otherwise a guest driver that assumes it is always present and tries >> to read from that area of memory will crash. > > Oh, reading should work no matter what. You just don't find valid edid > data there if it is turned off. Thanks for the info. My current use case for this is passing a set of possible guest display resolutions from the host to the guest driver, rather than using an internal hard-coded list. I guess that over time QEMU could become "smarter" about building the list of supported resolutions depending upon the capabilities of the host? ATB, Mark.
Hi, > >> Otherwise a guest driver that assumes it is always present and tries > >> to read from that area of memory will crash. > > > > Oh, reading should work no matter what. You just don't find valid edid > > data there if it is turned off. > > Thanks for the info. My current use case for this is passing a set of possible guest > display resolutions from the host to the guest driver, rather than using an internal > hard-coded list. I guess that over time QEMU could become "smarter" about building > the list of supported resolutions depending upon the capabilities of the host? Yes, that is the plan. There are xres and yres properties already, which will show up as preferred resolution in the edid blob. The generator also supports to set the maximum display resolution, and the display dpi (needed when we add hidpi support some day). stdvga doesn't support to change the edid dynamically (i.e. on window resize) because the hardware doesn't support interrupts, so we can't easily signal edid updates to the guest. cheers, Gerd
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index 14273a123e..df0a2f03ff 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -430,7 +430,10 @@ static void ppc_core99_init(MachineState *machine) } } - pci_vga_init(pci_bus); + dev = qdev_create(BUS(pci_bus), "VGA"); + qdev_prop_set_int32(dev, "addr", -1); + qdev_prop_set_bit(dev, "edid", true); + qdev_init_nofail(dev); if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) { graphic_depth = 15;
This is in preparation for some upcoming QEMU NDRV driver changes that pass display information from the host to the guest. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/ppc/mac_newworld.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)