Patchwork [1/3] vga: make PCI devices optional

login
register
mail settings
Submitter Blue Swirl
Date Oct. 9, 2011, 10:22 a.m.
Message ID <CAAu8pHuz31G8WVzOGG8qt4xEuxrqT-Ho9EMJqvKDYc1R2AT2gA@mail.gmail.com>
Download mbox | patch
Permalink /patch/118579/
State New
Headers show

Comments

Blue Swirl - Oct. 9, 2011, 10:22 a.m.
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 hw/cirrus_vga.c |    5 -----
 hw/pc.c         |    6 +++++-
 hw/pc.h         |   26 ++++++++++++++++++++++++--
 hw/pci.c        |   18 ++++++++++++++++++
 hw/pci.h        |    4 ++++
 hw/vga-pci.c    |    6 ------
 6 files changed, 51 insertions(+), 14 deletions(-)
Jan Kiszka - Oct. 9, 2011, 10:47 a.m.
On 2011-10-09 12:22, Blue Swirl wrote:
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
>  hw/cirrus_vga.c |    5 -----
>  hw/pc.c         |    6 +++++-
>  hw/pc.h         |   26 ++++++++++++++++++++++++--
>  hw/pci.c        |   18 ++++++++++++++++++
>  hw/pci.h        |    4 ++++
>  hw/vga-pci.c    |    6 ------
>  6 files changed, 51 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
> index c7e365b..a11444c 100644
> --- a/hw/cirrus_vga.c
> +++ b/hw/cirrus_vga.c
> @@ -2955,11 +2955,6 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
>       return 0;
>  }
> 
> -void pci_cirrus_vga_init(PCIBus *bus)
> -{
> -    pci_create_simple(bus, -1, "cirrus-vga");
> -}
> -
>  static PCIDeviceInfo cirrus_vga_info = {
>      .qdev.name    = "cirrus-vga",
>      .qdev.desc    = "Cirrus CLGD 54xx VGA",
> diff --git a/hw/pc.c b/hw/pc.c
> index 203627d..97f93d4 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1068,7 +1068,11 @@ void pc_vga_init(PCIBus *pci_bus)
>  {
>      if (cirrus_vga_enabled) {
>          if (pci_bus) {
> -            pci_cirrus_vga_init(pci_bus);
> +            if (!pci_cirrus_vga_init(pci_bus)) {
> +                fprintf(stderr, "Warning: cirrus_vga not available,"
> +                        " using standard VGA instead\n");
> +                pci_vga_init(pci_bus);
> +            }

If you adjust the default vga interface in case cirrus is configured
out, you can simply fail here. Would be more user-friendly, specifically
if the user actually specified -vga cirrus.

Jan
Blue Swirl - Oct. 16, 2011, 4:45 p.m.
On Sun, Oct 9, 2011 at 10:47 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> On 2011-10-09 12:22, Blue Swirl wrote:
>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>> ---
>>  hw/cirrus_vga.c |    5 -----
>>  hw/pc.c         |    6 +++++-
>>  hw/pc.h         |   26 ++++++++++++++++++++++++--
>>  hw/pci.c        |   18 ++++++++++++++++++
>>  hw/pci.h        |    4 ++++
>>  hw/vga-pci.c    |    6 ------
>>  6 files changed, 51 insertions(+), 14 deletions(-)
>>
>> diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
>> index c7e365b..a11444c 100644
>> --- a/hw/cirrus_vga.c
>> +++ b/hw/cirrus_vga.c
>> @@ -2955,11 +2955,6 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
>>       return 0;
>>  }
>>
>> -void pci_cirrus_vga_init(PCIBus *bus)
>> -{
>> -    pci_create_simple(bus, -1, "cirrus-vga");
>> -}
>> -
>>  static PCIDeviceInfo cirrus_vga_info = {
>>      .qdev.name    = "cirrus-vga",
>>      .qdev.desc    = "Cirrus CLGD 54xx VGA",
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 203627d..97f93d4 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -1068,7 +1068,11 @@ void pc_vga_init(PCIBus *pci_bus)
>>  {
>>      if (cirrus_vga_enabled) {
>>          if (pci_bus) {
>> -            pci_cirrus_vga_init(pci_bus);
>> +            if (!pci_cirrus_vga_init(pci_bus)) {
>> +                fprintf(stderr, "Warning: cirrus_vga not available,"
>> +                        " using standard VGA instead\n");
>> +                pci_vga_init(pci_bus);
>> +            }
>
> If you adjust the default vga interface in case cirrus is configured
> out, you can simply fail here. Would be more user-friendly, specifically
> if the user actually specified -vga cirrus.

This matches how vmsvga is handled. Maybe the logic should be pushed
to vl.c where the -vga switch is handled instead.
Jan Kiszka - Oct. 16, 2011, 5:21 p.m.
On 2011-10-16 18:45, Blue Swirl wrote:
> On Sun, Oct 9, 2011 at 10:47 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> On 2011-10-09 12:22, Blue Swirl wrote:
>>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>>> ---
>>>  hw/cirrus_vga.c |    5 -----
>>>  hw/pc.c         |    6 +++++-
>>>  hw/pc.h         |   26 ++++++++++++++++++++++++--
>>>  hw/pci.c        |   18 ++++++++++++++++++
>>>  hw/pci.h        |    4 ++++
>>>  hw/vga-pci.c    |    6 ------
>>>  6 files changed, 51 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
>>> index c7e365b..a11444c 100644
>>> --- a/hw/cirrus_vga.c
>>> +++ b/hw/cirrus_vga.c
>>> @@ -2955,11 +2955,6 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
>>>       return 0;
>>>  }
>>>
>>> -void pci_cirrus_vga_init(PCIBus *bus)
>>> -{
>>> -    pci_create_simple(bus, -1, "cirrus-vga");
>>> -}
>>> -
>>>  static PCIDeviceInfo cirrus_vga_info = {
>>>      .qdev.name    = "cirrus-vga",
>>>      .qdev.desc    = "Cirrus CLGD 54xx VGA",
>>> diff --git a/hw/pc.c b/hw/pc.c
>>> index 203627d..97f93d4 100644
>>> --- a/hw/pc.c
>>> +++ b/hw/pc.c
>>> @@ -1068,7 +1068,11 @@ void pc_vga_init(PCIBus *pci_bus)
>>>  {
>>>      if (cirrus_vga_enabled) {
>>>          if (pci_bus) {
>>> -            pci_cirrus_vga_init(pci_bus);
>>> +            if (!pci_cirrus_vga_init(pci_bus)) {
>>> +                fprintf(stderr, "Warning: cirrus_vga not available,"
>>> +                        " using standard VGA instead\n");
>>> +                pci_vga_init(pci_bus);
>>> +            }
>>
>> If you adjust the default vga interface in case cirrus is configured
>> out, you can simply fail here. Would be more user-friendly, specifically
>> if the user actually specified -vga cirrus.
> 
> This matches how vmsvga is handled. Maybe the logic should be pushed
> to vl.c where the -vga switch is handled instead.

Definitely. This approach is creating too much boilerplate code.

Jan

Patch

From 10ffb6a14f287979003c8f1b520e7d25bd5b1df5 Mon Sep 17 00:00:00 2001
Message-Id: <10ffb6a14f287979003c8f1b520e7d25bd5b1df5.1318155718.git.blauwirbel@gmail.com>
From: Blue Swirl <blauwirbel@gmail.com>
Date: Tue, 27 Sep 2011 19:15:42 +0000
Subject: [PATCH 1/3] vga: make PCI devices optional

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 hw/cirrus_vga.c |    5 -----
 hw/pc.c         |    6 +++++-
 hw/pc.h         |   26 ++++++++++++++++++++++++--
 hw/pci.c        |   18 ++++++++++++++++++
 hw/pci.h        |    4 ++++
 hw/vga-pci.c    |    6 ------
 6 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index c7e365b..a11444c 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2955,11 +2955,6 @@  static int pci_cirrus_vga_initfn(PCIDevice *dev)
      return 0;
 }
 
-void pci_cirrus_vga_init(PCIBus *bus)
-{
-    pci_create_simple(bus, -1, "cirrus-vga");
-}
-
 static PCIDeviceInfo cirrus_vga_info = {
     .qdev.name    = "cirrus-vga",
     .qdev.desc    = "Cirrus CLGD 54xx VGA",
diff --git a/hw/pc.c b/hw/pc.c
index 203627d..97f93d4 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1068,7 +1068,11 @@  void pc_vga_init(PCIBus *pci_bus)
 {
     if (cirrus_vga_enabled) {
         if (pci_bus) {
-            pci_cirrus_vga_init(pci_bus);
+            if (!pci_cirrus_vga_init(pci_bus)) {
+                fprintf(stderr, "Warning: cirrus_vga not available,"
+                        " using standard VGA instead\n");
+                pci_vga_init(pci_bus);
+            }
         } else {
             isa_cirrus_vga_init(get_system_memory());
         }
diff --git a/hw/pc.h b/hw/pc.h
index 7e6ddba..90a502d 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -8,6 +8,7 @@ 
 #include "fdc.h"
 #include "net.h"
 #include "memory.h"
+#include "pci.h"
 
 /* PC-style peripherals (also used by other machines).  */
 
@@ -217,13 +218,34 @@  static inline int isa_vga_init(void)
     return 1;
 }
 
-int pci_vga_init(PCIBus *bus);
+/* vga-pci.c */
+static inline bool pci_vga_init(PCIBus *bus)
+{
+    PCIDevice *dev;
+
+    dev = pci_try_create_simple(bus, -1, "VGA");
+    if (!dev) {
+        return false;
+    }
+    return true;
+}
+
 int isa_vga_mm_init(target_phys_addr_t vram_base,
                     target_phys_addr_t ctrl_base, int it_shift,
                     MemoryRegion *address_space);
 
 /* cirrus_vga.c */
-void pci_cirrus_vga_init(PCIBus *bus);
+static inline bool pci_cirrus_vga_init(PCIBus *bus)
+{
+    PCIDevice *dev;
+
+    dev = pci_try_create_simple(bus, -1, "cirrus-vga");
+    if (!dev) {
+        return false;
+    }
+    return true;
+}
+
 void isa_cirrus_vga_init(MemoryRegion *address_space);
 
 /* ne2000.c */
diff --git a/hw/pci.c b/hw/pci.c
index 749e8d8..46c01ac 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1687,6 +1687,19 @@  PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
     return dev;
 }
 
+PCIDevice *pci_try_create_simple_multifunction(PCIBus *bus, int devfn,
+                                               bool multifunction,
+                                               const char *name)
+{
+    PCIDevice *dev = pci_try_create_multifunction(bus, devfn, multifunction,
+                                                  name);
+    if (!dev) {
+        return NULL;
+    }
+    qdev_init_nofail(&dev->qdev);
+    return dev;
+}
+
 PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name)
 {
     return pci_create_multifunction(bus, devfn, false, name);
@@ -1702,6 +1715,11 @@  PCIDevice *pci_try_create(PCIBus *bus, int devfn, const char *name)
     return pci_try_create_multifunction(bus, devfn, false, name);
 }
 
+PCIDevice *pci_try_create_simple(PCIBus *bus, int devfn, const char *name)
+{
+    return pci_try_create_simple_multifunction(bus, devfn, false, name);
+}
+
 static int pci_find_space(PCIDevice *pdev, uint8_t size)
 {
     int config_size = pci_config_size(pdev);
diff --git a/hw/pci.h b/hw/pci.h
index 86a81c8..aa2e040 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -473,9 +473,13 @@  PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
 PCIDevice *pci_try_create_multifunction(PCIBus *bus, int devfn,
                                         bool multifunction,
                                         const char *name);
+PCIDevice *pci_try_create_simple_multifunction(PCIBus *bus, int devfn,
+                                               bool multifunction,
+                                               const char *name);
 PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
 PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
 PCIDevice *pci_try_create(PCIBus *bus, int devfn, const char *name);
+PCIDevice *pci_try_create_simple(PCIBus *bus, int devfn, const char *name);
 
 static inline int pci_is_express(const PCIDevice *d)
 {
diff --git a/hw/vga-pci.c b/hw/vga-pci.c
index 3c8bcb0..f296b19 100644
--- a/hw/vga-pci.c
+++ b/hw/vga-pci.c
@@ -70,12 +70,6 @@  static int pci_vga_initfn(PCIDevice *dev)
      return 0;
 }
 
-int pci_vga_init(PCIBus *bus)
-{
-    pci_create_simple(bus, -1, "VGA");
-    return 0;
-}
-
 static PCIDeviceInfo vga_info = {
     .qdev.name    = "VGA",
     .qdev.size    = sizeof(PCIVGAState),
-- 
1.7.2.5