qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 05/14] sm501: Add emulation of chip connected


From: BALATON Zoltan
Subject: Re: [Qemu-devel] [PATCH v2 05/14] sm501: Add emulation of chip connected via PCI
Date: Fri, 3 Mar 2017 02:53:40 +0100 (CET)
User-agent: Alpine 2.20 (BSF 67 2015-01-07)

On Thu, 2 Mar 2017, Peter Maydell wrote:
On 2 March 2017 at 20:13, BALATON Zoltan <address@hidden> wrote:
On Thu, 2 Mar 2017, Peter Maydell wrote:

On 25 February 2017 at 18:31, BALATON Zoltan <address@hidden> wrote:

Only the display controller part is created automatically on PCI

Signed-off-by: BALATON Zoltan <address@hidden>
---

v2: Split off removing dependency on base address to separate patch

 hw/display/sm501.c | 52
++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 1cda127..d9219bd 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -31,6 +31,7 @@
 #include "ui/console.h"
 #include "hw/devices.h"
 #include "hw/sysbus.h"
+#include "hw/pci/pci.h"
 #include "qemu/range.h"
 #include "ui/pixel_ops.h"
 #include "exec/address-spaces.h"
@@ -1507,9 +1508,60 @@ static const TypeInfo sm501_sysbus_info = {
     .class_init    = sm501_sysbus_class_init,
 };

+#define TYPE_PCI_SM501 "sm501"
+#define PCI_SM501(obj) OBJECT_CHECK(SM501PCIState, (obj),
TYPE_PCI_SM501)
+
+typedef struct {
+    /*< private >*/
+    PCIDevice parent_obj;
+    /*< public >*/
+    SM501State state;
+    uint32_t vram_size;
+} SM501PCIState;
+
+static void sm501_realize_pci(PCIDevice *dev, Error **errp)
+{
+    SM501PCIState *s = PCI_SM501(dev);
+
+    sm501_init(&s->state, DEVICE(dev), s->vram_size);
+    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY,
+                     &s->state.local_mem_region);
+    pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY,
+                     &s->state.mmio_region);
+}
+
+static Property sm501_pci_properties[] = {
+    DEFINE_PROP_UINT32("vram-size", SM501PCIState, vram_size,
+                       64 * 1024 * 1024),


Something needs to sanity check this value, because it's
user settable in the PCI device. (In the sysbus version you
can only create it from board code, and we can assume board
code doesn't pass us silly values.)

Or you could just hardcode the PCI device to always be 64MB,
I guess :-)


Maybe a check could be added to get_local_mem_size_index(). What should it
do for invalid values?

The device realize function should fail:
   if (user settable property foo is wrong) {
       error_setg(errp, "foo must be X, Y or Z");
       return;
   }

Actually I think this cannot fail because get_local_mem_size_index() will pick a valid value that's closest to the one specified. However the mem region was not created using this adjusted value. I've fixed this in the v3 I've just sent and also added a warning when adjusment was made.

Thank you,
BALATON Zoltan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]