Patchwork [4/4] qdev: add display capability

login
register
mail settings
Submitter Gerd Hoffmann
Date Aug. 11, 2009, 9:20 a.m.
Message ID <1249982427-14481-4-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/31127/
State Superseded
Headers show

Comments

Gerd Hoffmann - Aug. 11, 2009, 9:20 a.m.
... and tag devices.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/cirrus_vga.c |    1 +
 hw/qdev.c       |    1 +
 hw/qdev.h       |    2 ++
 hw/syborg_fb.c  |    1 +
 hw/tcx.c        |    1 +
 hw/vga.c        |    1 +
 hw/vmware_vga.c |    1 +
 7 files changed, 8 insertions(+), 0 deletions(-)
Anthony Liguori - Aug. 11, 2009, 1:42 p.m.
Gerd Hoffmann wrote:
> ... and tag devices.
>   

Shouldn't there be a direct relationship between a capability and the 
interfaces the device accepts?

That is, instead of explicitly marking something as having a capability 
of { "ethernet" }, wouldn't you be able to infer that from the present 
of a VLANClientState property?

Regards,

Anthony Liguori
Gerd Hoffmann - Aug. 11, 2009, 2:17 p.m.
On 08/11/09 15:42, Anthony Liguori wrote:
> Gerd Hoffmann wrote:
>> ... and tag devices.
>
> Shouldn't there be a direct relationship between a capability and the
> interfaces the device accepts?

That is the case for some devices only.

> That is, instead of explicitly marking something as having a capability
> of { "ethernet" }, wouldn't you be able to infer that from the present
> of a VLANClientState property?

Works for ethernet (well, not yet, but most likely some day ...).

What property do you use to identify sound devices?
What property do you use to identify display devices?
What property do you use to identify watchdog devices?

cheers,
   Gerd
Anthony Liguori - Aug. 11, 2009, 3:57 p.m.
Gerd Hoffmann wrote:
> On 08/11/09 15:42, Anthony Liguori wrote:
>> Gerd Hoffmann wrote:
>>> ... and tag devices.
>>
>> Shouldn't there be a direct relationship between a capability and the
>> interfaces the device accepts?
>
> That is the case for some devices only.

A device doesn't have a capability unless it implements the backend 
device interface, no?

That is, a device does not have the Display capability unless it returns 
a DisplayState.

>> That is, instead of explicitly marking something as having a capability
>> of { "ethernet" }, wouldn't you be able to infer that from the present
>> of a VLANClientState property?
>
> Works for ethernet (well, not yet, but most likely some day ...).
>
> What property do you use to identify sound devices?

A QEMUSoundCard * property (r/o).

> What property do you use to identify display devices?

A DisplayState * property (r/o).
> What property do you use to identify watchdog devices?

A WatchdogTimerModel * property (r/o).

Regards,

Anthony Liguori
> cheers,
>   Gerd
Gerd Hoffmann - Aug. 11, 2009, 4:28 p.m.
On 08/11/09 17:57, Anthony Liguori wrote:
> Gerd Hoffmann wrote:
>> That is the case for some devices only.
>
> A device doesn't have a capability unless it implements the backend
> device interface, no?

Right.  Although that doesn't imply there is a property needed for that.

>> What property do you use to identify display devices?
>
> A DisplayState * property (r/o).

Ok, that is the backend side of display devices.  It would somehow make 
sense to export that as property like we do today for DriveInfo and 
CharDriverState, although there is no need right now to have that 
accessable as property.


>> What property do you use to identify sound devices?
>
> A QEMUSoundCard * property (r/o).

That is a linked list of all sound card device instances.

>> What property do you use to identify watchdog devices?
>
> A WatchdogTimerModel * property (r/o).

That is a linked list of all watchdog device models.

Why on earth these should become device properties?
That doesn't make sense to me at all.

Oh, and the WatchdogTimerModel is the one I actually want to kill off 
some day (after converting watchdogs to qdev).  There is no need for a 
private device model list, we can use the qdev list instead and filter 
for DeviceInfo->caps & DEV_CAP_WATCHDOG.

cheers,
   Gerd

Patch

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 95d822a..65dd8a0 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -3345,6 +3345,7 @@  void pci_cirrus_vga_init(PCIBus *bus)
 static PCIDeviceInfo cirrus_vga_info = {
     .qdev.name    = "Cirrus VGA",
     .qdev.size    = sizeof(PCICirrusVGAState),
+    .qdev.caps    = DEV_CAP_DISPLAY,
     .init         = pci_cirrus_vga_initfn,
     .config_write = pci_cirrus_write_config,
 };
diff --git a/hw/qdev.c b/hw/qdev.c
index c026305..b15408d 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -110,6 +110,7 @@  static int qdev_print_devinfo(DeviceInfo *info, char *dest, int len)
     static const char *capname[] = {
         [ DEV_CAP_BIT_AUDIO       ] = "audio",
         [ DEV_CAP_BIT_ETHERNET    ] = "ethernet",
+        [ DEV_CAP_BIT_DISPLAY     ] = "display",
     };
     const char *sep;
     int pos = 0;
diff --git a/hw/qdev.h b/hw/qdev.h
index ee282f0..ea38a88 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -105,10 +105,12 @@  typedef void (*SCSIAttachFn)(DeviceState *host, BlockDriverState *bdrv,
 enum DeviceCapBits {
     DEV_CAP_BIT_AUDIO      = 0,
     DEV_CAP_BIT_ETHERNET   = 1,
+    DEV_CAP_BIT_DISPLAY    = 2,
 };
 
 #define DEV_CAP_AUDIO      (1 << DEV_CAP_BIT_AUDIO)
 #define DEV_CAP_ETHERNET   (1 << DEV_CAP_BIT_ETHERNET)
+#define DEV_CAP_DISPLAY    (1 << DEV_CAP_BIT_DISPLAY)
 
 struct DeviceInfo {
     const char *name;
diff --git a/hw/syborg_fb.c b/hw/syborg_fb.c
index efa5c0e..9d9a07e 100644
--- a/hw/syborg_fb.c
+++ b/hw/syborg_fb.c
@@ -534,6 +534,7 @@  static SysBusDeviceInfo syborg_fb_info = {
     .init = syborg_fb_init,
     .qdev.name  = "syborg,framebuffer",
     .qdev.size  = sizeof(SyborgFBState),
+    .qdev.caps  = DEV_CAP_DISPLAY,
     .qdev.props = (Property[]) {
         DEFINE_PROP_UINT32("width",  SyborgFBState, cols, 0),
         DEFINE_PROP_UINT32("height", SyborgFBState, rows, 0),
diff --git a/hw/tcx.c b/hw/tcx.c
index 68dbf02..a6f29fc 100644
--- a/hw/tcx.c
+++ b/hw/tcx.c
@@ -645,6 +645,7 @@  static SysBusDeviceInfo tcx_info = {
     .init = tcx_init1,
     .qdev.name  = "SUNW,tcx",
     .qdev.size  = sizeof(TCXState),
+    .qdev.caps  = DEV_CAP_DISPLAY,
     .qdev.props = (Property[]) {
         DEFINE_PROP_TADDR("addr",      TCXState, addr,      -1),
         DEFINE_PROP_HEX32("vram_size", TCXState, vram_size, -1),
diff --git a/hw/vga.c b/hw/vga.c
index 4a0f197..7090075 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2530,6 +2530,7 @@  int pci_vga_init(PCIBus *bus,
 static PCIDeviceInfo vga_info = {
     .qdev.name    = "VGA",
     .qdev.size    = sizeof(PCIVGAState),
+    .qdev.caps    = DEV_CAP_DISPLAY,
     .init         = pci_vga_initfn,
     .config_write = pci_vga_write_config,
     .qdev.props   = (Property[]) {
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 5ceebf1..0c7df8e 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1246,6 +1246,7 @@  void pci_vmsvga_init(PCIBus *bus)
 static PCIDeviceInfo vmsvga_info = {
     .qdev.name    = "QEMUware SVGA",
     .qdev.size    = sizeof(struct pci_vmsvga_state_s),
+    .qdev.caps    = DEV_CAP_DISPLAY,
     .init         = pci_vmsvga_initfn,
 };