qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Add device DM163 (led driver, matrix colors shield & display)
@ 2024-01-26 19:31 Inès Varhol
  2024-01-26 19:31 ` [PATCH 1/3] hw/display : Add device DM163 Inès Varhol
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Inès Varhol @ 2024-01-26 19:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Arnaud Minier, Paolo Bonzini, qemu-arm, Samuel Tardieu,
	Peter Maydell, Alistair Francis, Philippe Mathieu-Daudé,
	Thomas Huth, Laurent Vivier, Inès Varhol

This device implements the IM120417002 colors shield v1.1 for Arduino
(which relies on the DM163 8x3-channel led driving logic) and features
a simple display of an 8x8 RGB matrix.

This color shield can be plugged on the Arduino board (or the
B-L475E-IOT01A board) to drive an 8x8 RGB led matrix.
This RGB led matrix takes advantage of retinal persistance to
seemingly display different colors in each row.

It'd be convenient to set the QEMU console's refresh rate
in order to ensure that the delay before turning off rows
(2 frames currently) isn't too short. However
`dpy_ui_info_supported(s->console)` can't be used.

I saw that Kconfig configurable components aren't visible in C files,
does that mean it's impossible to make the DM163 device optional when
using the B-L475E-IOT01A board?

Based-on: 20240123122505.516393-1-ines.varhol@telecom-paris.fr
([PATCH v3 0/3] Add device STM32L4x5 GPIO)

Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>

Inès Varhol (3):
  hw/display : Add device DM163
  hw/arm : Connect DM163 to STM32L4x5
  tests/qtest : Add testcase for DM163

 hw/arm/Kconfig                 |   1 +
 hw/arm/stm32l4x5_soc.c         |  55 +++++-
 hw/display/Kconfig             |   3 +
 hw/display/dm163.c             | 307 +++++++++++++++++++++++++++++++++
 hw/display/meson.build         |   1 +
 hw/display/trace-events        |  13 ++
 include/hw/arm/stm32l4x5_soc.h |   3 +
 include/hw/display/dm163.h     |  57 ++++++
 tests/qtest/dm163-test.c       | 192 +++++++++++++++++++++
 tests/qtest/meson.build        |   1 +
 10 files changed, 632 insertions(+), 1 deletion(-)
 create mode 100644 hw/display/dm163.c
 create mode 100644 include/hw/display/dm163.h
 create mode 100644 tests/qtest/dm163-test.c

-- 
2.43.0



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] hw/display : Add device DM163
  2024-01-26 19:31 [PATCH 0/3] Add device DM163 (led driver, matrix colors shield & display) Inès Varhol
@ 2024-01-26 19:31 ` Inès Varhol
  2024-02-05  0:09   ` Alistair Francis
  2024-01-26 19:31 ` [PATCH 2/3] hw/arm : Connect DM163 to STM32L4x5 Inès Varhol
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Inès Varhol @ 2024-01-26 19:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Arnaud Minier, Paolo Bonzini, qemu-arm, Samuel Tardieu,
	Peter Maydell, Alistair Francis, Philippe Mathieu-Daudé,
	Thomas Huth, Laurent Vivier, Inès Varhol

This device implements the IM120417002 colors shield v1.1 for Arduino
(which relies on the DM163 8x3-channel led driving logic) and features
a simple display of an 8x8 RGB matrix. The columns of the matrix are
driven by the DM163 and the rows are driven externally.

Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
 hw/display/Kconfig         |   3 +
 hw/display/dm163.c         | 307 +++++++++++++++++++++++++++++++++++++
 hw/display/meson.build     |   1 +
 hw/display/trace-events    |  13 ++
 include/hw/display/dm163.h |  57 +++++++
 5 files changed, 381 insertions(+)
 create mode 100644 hw/display/dm163.c
 create mode 100644 include/hw/display/dm163.h

diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index 1aafe1923d..4dbfc6e7af 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -139,3 +139,6 @@ config XLNX_DISPLAYPORT
     bool
     # defaults to "N", enabled by specific boards
     depends on PIXMAN
+
+config DM163
+    bool
diff --git a/hw/display/dm163.c b/hw/display/dm163.c
new file mode 100644
index 0000000000..565fc84ddf
--- /dev/null
+++ b/hw/display/dm163.c
@@ -0,0 +1,307 @@
+/*
+ * QEMU DM163 8x3-channel constant current led driver
+ * driving columns of associated 8x8 RGB matrix.
+ *
+ * Copyright (C) 2024 Samuel Tardieu <sam@rfc1149.net>
+ * Copyright (C) 2024 Arnaud Minier <arnaud.minier@telecom-paris.fr>
+ * Copyright (C) 2024 Inès Varhol <ines.varhol@telecom-paris.fr>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+/*
+ * The reference used for the DM163 is the following :
+ * http://www.siti.com.tw/product/spec/LED/DM163.pdf
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "migration/vmstate.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/display/dm163.h"
+#include "ui/console.h"
+#include "trace.h"
+
+#define LED_SQUARE_SIZE 100
+/* Number of frames a row stays visible after being turned off. */
+#define ROW_PERSISTANCE 2
+
+static const VMStateDescription vmstate_dm163 = {
+    .name = TYPE_DM163,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (const VMStateField[]) {
+        VMSTATE_UINT8(activated_rows, DM163State),
+        VMSTATE_UINT64_ARRAY(bank0_shift_register, DM163State, 3),
+        VMSTATE_UINT64_ARRAY(bank1_shift_register, DM163State, 3),
+        VMSTATE_UINT16_ARRAY(latched_outputs, DM163State, DM163_NUM_LEDS),
+        VMSTATE_UINT16_ARRAY(outputs, DM163State, DM163_NUM_LEDS),
+        VMSTATE_UINT8(dck, DM163State),
+        VMSTATE_UINT8(en_b, DM163State),
+        VMSTATE_UINT8(lat_b, DM163State),
+        VMSTATE_UINT8(rst_b, DM163State),
+        VMSTATE_UINT8(selbk, DM163State),
+        VMSTATE_UINT8(sin, DM163State),
+        VMSTATE_UINT32_2DARRAY(buffer, DM163State,
+            COLOR_BUFFER_SIZE + 1, RGB_MATRIX_NUM_COLS),
+        VMSTATE_UINT8(last_buffer_idx, DM163State),
+        VMSTATE_UINT8_ARRAY(buffer_idx_of_row, DM163State, RGB_MATRIX_NUM_ROWS),
+        VMSTATE_UINT8_ARRAY(age_of_row, DM163State, RGB_MATRIX_NUM_ROWS),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void dm163_reset_hold(Object *obj)
+{
+    DM163State *s = DM163(obj);
+
+    /* Reset only stops the PWM. */
+    memset(s->outputs, 0, sizeof(s->outputs));
+
+    /* The last row of the buffer stores a turned off row */
+    memset(s->buffer[COLOR_BUFFER_SIZE], 0, sizeof(s->buffer[0]));
+}
+
+static void dm163_dck_gpio_handler(void *opaque, int line, int new_state)
+{
+    DM163State *s = DM163(opaque);
+
+    if (new_state && !s->dck) {
+        /*
+         * On raising dck, sample selbk to get the bank to use, and
+         * sample sin for the bit to enter into the bank shift buffer.
+         */
+        uint64_t *sb =
+            s->selbk ? s->bank1_shift_register : s->bank0_shift_register;
+        /* Output the outgoing bit on sout */
+        const bool sout = (s->selbk ? sb[2] & MAKE_64BIT_MASK(63, 1) :
+                           sb[2] & MAKE_64BIT_MASK(15, 1)) != 0;
+        qemu_set_irq(s->sout, sout);
+        /* Enter sin into the shift buffer */
+        sb[2] = (sb[2] << 1) | ((sb[1] >> 63) & 1);
+        sb[1] = (sb[1] << 1) | ((sb[0] >> 63) & 1);
+        sb[0] = (sb[0] << 1) | s->sin;
+    }
+
+    s->dck = new_state;
+    trace_dm163_dck(new_state);
+}
+
+static void dm163_propagate_outputs(DM163State *s)
+{
+    s->last_buffer_idx = (s->last_buffer_idx + 1) % COLOR_BUFFER_SIZE;
+    /* Values are output when reset and enable are both high. */
+    if (s->rst_b && !s->en_b) {
+        memcpy(s->outputs, s->latched_outputs, sizeof(s->outputs));
+    } else {
+        memset(s->outputs, 0, sizeof(s->outputs));
+    }
+    for (unsigned x = 0; x < RGB_MATRIX_NUM_COLS; x++) {
+        trace_dm163_channels(3 * x, (uint8_t)(s->outputs[3 * x] >> 6));
+        trace_dm163_channels(3 * x + 1, (uint8_t)(s->outputs[3 * x + 1] >> 6));
+        trace_dm163_channels(3 * x + 2, (uint8_t)(s->outputs[3 * x + 2] >> 6));
+        s->buffer[s->last_buffer_idx][x] =
+            (s->outputs[3 * x + 2] >> 6) |
+            ((s->outputs[3 * x + 1] << 2) & 0xFF00) |
+            (((uint32_t)s->outputs[3 * x] << 10) & 0xFF0000);
+    }
+    for (unsigned row = 0; row < RGB_MATRIX_NUM_ROWS; row++) {
+        if (s->activated_rows & (1 << row)) {
+            s->buffer_idx_of_row[row] = s->last_buffer_idx;
+        }
+    }
+}
+
+static void dm163_en_b_gpio_handler(void *opaque, int line, int new_state)
+{
+    DM163State *s = DM163(opaque);
+
+    s->en_b = new_state;
+    dm163_propagate_outputs(s);
+    trace_dm163_en_b(new_state);
+}
+
+static inline uint8_t dm163_bank0(const DM163State *s, uint8_t led)
+{
+    /*
+     * Bank 1 uses 6 bits per led, so a value may be stored accross
+     * two uint64_t entries.
+     */
+    const uint8_t low_bit = 6 * led;
+    const uint8_t low_word = low_bit / 64;
+    const uint8_t high_word = (low_bit + 5) / 64;
+    const uint8_t low_shift = low_bit % 64;
+
+    if (low_word == high_word) {
+        /* Simple case: the value belongs to one entry. */
+        return (s->bank0_shift_register[low_word] &
+                MAKE_64BIT_MASK(low_shift, 6)) >> low_shift;
+    }
+
+    const uint8_t bits_in_low_word = 64 - low_shift;
+    const uint8_t bits_in_high_word = 6 - bits_in_low_word;
+    return ((s->bank0_shift_register[low_word] &
+             MAKE_64BIT_MASK(low_shift, bits_in_low_word)) >>
+            low_shift) |
+           ((s->bank0_shift_register[high_word] &
+             MAKE_64BIT_MASK(0, bits_in_high_word))
+         << bits_in_low_word);
+}
+
+static inline uint8_t dm163_bank1(const DM163State *s, uint8_t led)
+{
+    const uint64_t entry = s->bank1_shift_register[led / 8];
+    const unsigned shift = 8 * (led % 8);
+    return (entry & MAKE_64BIT_MASK(shift, 8)) >> shift;
+}
+
+static void dm163_lat_b_gpio_handler(void *opaque, int line, int new_state)
+{
+    DM163State *s = DM163(opaque);
+
+    if (s->lat_b && !new_state) {
+        for (int led = 0; led < DM163_NUM_LEDS; led++) {
+            s->latched_outputs[led] = dm163_bank0(s, led) * dm163_bank1(s, led);
+        }
+        dm163_propagate_outputs(s);
+    }
+
+    s->lat_b = new_state;
+    trace_dm163_lat_b(new_state);
+}
+
+static void dm163_rst_b_gpio_handler(void *opaque, int line, int new_state)
+{
+    DM163State *s = DM163(opaque);
+
+    s->rst_b = new_state;
+    dm163_propagate_outputs(s);
+    trace_dm163_rst_b(new_state);
+}
+
+static void dm163_selbk_gpio_handler(void *opaque, int line, int new_state)
+{
+    DM163State *s = DM163(opaque);
+
+    s->selbk = new_state;
+    trace_dm163_selbk(new_state);
+}
+
+static void dm163_sin_gpio_handler(void *opaque, int line, int new_state)
+{
+    DM163State *s = DM163(opaque);
+
+    s->sin = new_state;
+    trace_dm163_sin(new_state);
+}
+
+static void dm163_rows_gpio_handler(void *opaque, int line, int new_state)
+{
+    DM163State *s = DM163(opaque);
+
+    if (new_state) {
+        s->activated_rows |= (1 << line);
+        s->buffer_idx_of_row[line] = s->last_buffer_idx;
+        s->age_of_row[line] = 0;
+    } else {
+        s->activated_rows &= ~(1 << line);
+        s->age_of_row[line] = ROW_PERSISTANCE;
+    }
+    trace_dm163_activated_rows(s->activated_rows);
+}
+
+static void dm163_invalidate_display(void *opaque)
+{
+}
+
+static void dm163_update_display(void *opaque)
+{
+    DM163State *s = (DM163State *)opaque;
+    DisplaySurface *surface = qemu_console_surface(s->console);
+    uint32_t *dest;
+    unsigned bits_ppi = surface_bits_per_pixel(surface);
+
+    trace_dm163_bits_ppi(bits_ppi);
+    g_assert((bits_ppi == 32));
+    dest = surface_data(surface);
+    for (unsigned y = 0; y < RGB_MATRIX_NUM_ROWS; y++) {
+        for (unsigned _ = 0; _ < LED_SQUARE_SIZE; _++) {
+            for (int x = RGB_MATRIX_NUM_COLS * LED_SQUARE_SIZE - 1; x >= 0; x--) {
+                *dest++ = s->buffer[s->buffer_idx_of_row[y]][x / LED_SQUARE_SIZE];
+            }
+        }
+        if (s->age_of_row[y]) {
+            s->age_of_row[y]--;
+            if (!s->age_of_row[y]) {
+                /*
+                 * If the ROW_PERSISTANCE delay is up,
+                 * the row is turned off.
+                 * (s->buffer[COLOR_BUFFER] is filled with 0)
+                 */
+                s->buffer_idx_of_row[y] = COLOR_BUFFER_SIZE;
+            }
+        }
+    }
+    /*
+     * Ideally set the refresh rate so that the row persistance
+     * doesn't need to be changed.
+     *
+     * Currently `dpy_ui_info_supported(s->console)` returns false
+     * which makes it impossible to get or set UIInfo.
+     *
+     * if (dpy_ui_info_supported(s->console)) {
+     *     trace_dm163_refresh_rate(dpy_get_ui_info(s->console)->refresh_rate);
+     * } else {
+     *     trace_dm163_refresh_rate(0);
+     * }
+     */
+    dpy_gfx_update(s->console, 0, 0, RGB_MATRIX_NUM_COLS * LED_SQUARE_SIZE,
+                   RGB_MATRIX_NUM_ROWS * LED_SQUARE_SIZE);
+}
+
+static const GraphicHwOps dm163_ops = {
+    .invalidate  = dm163_invalidate_display,
+    .gfx_update  = dm163_update_display,
+};
+
+static void dm163_realize(DeviceState *dev, Error **errp)
+{
+    DM163State *s = DM163(dev);
+
+    qdev_init_gpio_in(dev, dm163_rows_gpio_handler, 8);
+    qdev_init_gpio_in(dev, dm163_sin_gpio_handler, 1);
+    qdev_init_gpio_in(dev, dm163_dck_gpio_handler, 1);
+    qdev_init_gpio_in(dev, dm163_rst_b_gpio_handler, 1);
+    qdev_init_gpio_in(dev, dm163_lat_b_gpio_handler, 1);
+    qdev_init_gpio_in(dev, dm163_selbk_gpio_handler, 1);
+    qdev_init_gpio_in(dev, dm163_en_b_gpio_handler, 1);
+    qdev_init_gpio_out_named(dev, &s->sout, "sout", 1);
+
+    s->console = graphic_console_init(dev, 0, &dm163_ops, s);
+    qemu_console_resize(s->console, RGB_MATRIX_NUM_COLS * LED_SQUARE_SIZE,
+                        RGB_MATRIX_NUM_ROWS * LED_SQUARE_SIZE);
+}
+
+static void dm163_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+
+    dc->desc = "DM163";
+    dc->vmsd = &vmstate_dm163;
+    dc->realize = dm163_realize;
+    rc->phases.hold = dm163_reset_hold;
+    set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
+}
+
+static const TypeInfo dm163_types[] = {
+    {
+        .name = TYPE_DM163,
+        .parent = TYPE_DEVICE,
+        .instance_size = sizeof(DM163State),
+        .class_init = dm163_class_init
+    }
+};
+
+DEFINE_TYPES(dm163_types)
diff --git a/hw/display/meson.build b/hw/display/meson.build
index f93a69f70f..71e489308e 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -38,6 +38,7 @@ system_ss.add(when: 'CONFIG_NEXTCUBE', if_true: files('next-fb.c'))
 
 system_ss.add(when: 'CONFIG_VGA', if_true: files('vga.c'))
 system_ss.add(when: 'CONFIG_VIRTIO', if_true: files('virtio-dmabuf.c'))
+system_ss.add(when: 'CONFIG_DM163', if_true: files('dm163.c'))
 
 if (config_all_devices.has_key('CONFIG_VGA_CIRRUS') or
     config_all_devices.has_key('CONFIG_VGA_PCI') or
diff --git a/hw/display/trace-events b/hw/display/trace-events
index 2336a0ca15..444b014d6e 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -177,3 +177,16 @@ macfb_ctrl_write(uint64_t addr, uint64_t value, unsigned int size) "addr 0x%"PRI
 macfb_sense_read(uint32_t value) "video sense: 0x%"PRIx32
 macfb_sense_write(uint32_t value) "video sense: 0x%"PRIx32
 macfb_update_mode(uint32_t width, uint32_t height, uint8_t depth) "setting mode to width %"PRId32 " height %"PRId32 " size %d"
+
+# dm163.c
+dm163_dck(int new_state) "dck : %d"
+dm163_en_b(int new_state) "en_b : %d"
+dm163_rst_b(int new_state) "rst_b : %d"
+dm163_lat_b(int new_state) "lat_b : %d"
+dm163_sin(int new_state) "sin : %d"
+dm163_selbk(int new_state) "selbk : %d"
+dm163_activated_rows(int new_state) "Activated rows : 0x%" PRIx32 ""
+dm163_bits_ppi(unsigned dest_width) "dest_width : %u"
+dm163_leds(int led, uint32_t value) "led %d: 0x%x"
+dm163_channels(int channel, uint8_t value) "channel %d: 0x%x"
+dm163_refresh_rate(uint32_t rr) "refresh rate %d"
diff --git a/include/hw/display/dm163.h b/include/hw/display/dm163.h
new file mode 100644
index 0000000000..aa775e51e1
--- /dev/null
+++ b/include/hw/display/dm163.h
@@ -0,0 +1,57 @@
+/*
+ * QEMU DM163 8x3-channel constant current led driver
+ * driving columns of associated 8x8 RGB matrix.
+ *
+ * Copyright (C) 2024 Samuel Tardieu <sam@rfc1149.net>
+ * Copyright (C) 2024 Arnaud Minier <arnaud.minier@telecom-paris.fr>
+ * Copyright (C) 2024 Inès Varhol <ines.varhol@telecom-paris.fr>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_DISPLAY_DM163_H
+#define HW_DISPLAY_DM163_H
+
+#include "qom/object.h"
+#include "hw/qdev-core.h"
+
+#define TYPE_DM163 "dm163"
+OBJECT_DECLARE_SIMPLE_TYPE(DM163State, DM163);
+
+#define DM163_NUM_LEDS 24
+#define RGB_MATRIX_NUM_ROWS 8
+#define RGB_MATRIX_NUM_COLS (DM163_NUM_LEDS / 3)
+#define COLOR_BUFFER_SIZE RGB_MATRIX_NUM_ROWS
+
+typedef struct DM163State {
+    DeviceState parent_obj;
+
+    /* DM163 driver */
+    uint64_t bank0_shift_register[3];
+    uint64_t bank1_shift_register[3];
+    uint16_t latched_outputs[DM163_NUM_LEDS];
+    uint16_t outputs[DM163_NUM_LEDS];
+    qemu_irq sout;
+
+    uint8_t dck;
+    uint8_t en_b;
+    uint8_t lat_b;
+    uint8_t rst_b;
+    uint8_t selbk;
+    uint8_t sin;
+
+    /* IM120417002 colors shield */
+    uint8_t activated_rows;
+
+    /* 8x8 RGB matrix */
+    QemuConsole *console;
+    /* Rows currently being displayed on the matrix. */
+    /* The last row is filled with 0 (turned off row) */
+    uint32_t buffer[COLOR_BUFFER_SIZE + 1][RGB_MATRIX_NUM_COLS];
+    uint8_t last_buffer_idx;
+    uint8_t buffer_idx_of_row[RGB_MATRIX_NUM_ROWS];
+    /* Used to simulate retinal persistance of rows */
+    uint8_t age_of_row[RGB_MATRIX_NUM_ROWS];
+} DM163State;
+
+#endif /* HW_DISPLAY_DM163_H */
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] hw/arm : Connect DM163 to STM32L4x5
  2024-01-26 19:31 [PATCH 0/3] Add device DM163 (led driver, matrix colors shield & display) Inès Varhol
  2024-01-26 19:31 ` [PATCH 1/3] hw/display : Add device DM163 Inès Varhol
@ 2024-01-26 19:31 ` Inès Varhol
  2024-02-05  0:14   ` Alistair Francis
  2024-02-05 13:46   ` Philippe Mathieu-Daudé
  2024-01-26 19:31 ` [PATCH 3/3] tests/qtest : Add testcase for DM163 Inès Varhol
  2024-02-05 14:03 ` [PATCH 0/3] Add device DM163 (led driver, matrix colors shield & display) Philippe Mathieu-Daudé
  3 siblings, 2 replies; 11+ messages in thread
From: Inès Varhol @ 2024-01-26 19:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Arnaud Minier, Paolo Bonzini, qemu-arm, Samuel Tardieu,
	Peter Maydell, Alistair Francis, Philippe Mathieu-Daudé,
	Thomas Huth, Laurent Vivier, Inès Varhol

Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
 hw/arm/Kconfig                 |  1 +
 hw/arm/stm32l4x5_soc.c         | 55 +++++++++++++++++++++++++++++++++-
 include/hw/arm/stm32l4x5_soc.h |  3 ++
 3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 3e49b913f8..818aa2f1a2 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -463,6 +463,7 @@ config STM32L4X5_SOC
     select STM32L4X5_SYSCFG
     select STM32L4X5_RCC
     select STM32L4X5_GPIO
+    select DM163
 
 config XLNX_ZYNQMP_ARM
     bool
diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
index 478c6ba056..8663546901 100644
--- a/hw/arm/stm32l4x5_soc.c
+++ b/hw/arm/stm32l4x5_soc.c
@@ -26,7 +26,9 @@
 #include "qapi/error.h"
 #include "exec/address-spaces.h"
 #include "sysemu/sysemu.h"
+#include "hw/core/split-irq.h"
 #include "hw/arm/stm32l4x5_soc.h"
+#include "hw/display/dm163.h"
 #include "hw/qdev-clock.h"
 #include "hw/misc/unimp.h"
 
@@ -78,6 +80,31 @@ static const int exti_irq[NUM_EXTI_IRQ] = {
 #define RCC_BASE_ADDRESS 0x40021000
 #define RCC_IRQ 5
 
+/*
+ * There are actually 14 input pins in the DM163 device.
+ * Here the DM163 input pin EN isn't connected to the STM32L4x5
+ * GPIOs as the IM120417002 colors shield doesn't actually use
+ * this pin to drive the RGB matrix.
+ */
+#define NUM_DM163_INPUTS 13
+
+static const int dm163_input[NUM_DM163_INPUTS] = {
+    1 * 16 + 2,  /* ROW0  PB2       */
+    0 * 16 + 15, /* ROW1  PA15      */
+    0 * 16 + 2,  /* ROW2  PA2       */
+    0 * 16 + 7,  /* ROW3  PA7       */
+    0 * 16 + 6,  /* ROW4  PA6       */
+    0 * 16 + 5,  /* ROW5  PA5       */
+    1 * 16 + 0,  /* ROW6  PB0       */
+    0 * 16 + 3,  /* ROW7  PA3       */
+    0 * 16 + 4,  /* SIN (SDA) PA4   */
+    1 * 16 + 1,  /* DCK (SCK) PB1   */
+    2 * 16 + 3,  /* RST_B (RST) PC3 */
+    2 * 16 + 4,  /* LAT_B (LAT) PC4 */
+    2 * 16 + 5,  /* SELBK (SB)  PC5 */
+};
+
+
 static const uint32_t gpio_addr[] = {
     0x48000000,
     0x48000400,
@@ -116,6 +143,8 @@ static void stm32l4x5_soc_initfn(Object *obj)
         g_autofree char *name = g_strdup_printf("gpio%c", 'a' + i);
         object_initialize_child(obj, name, &s->gpio[i], TYPE_STM32L4X5_GPIO);
     }
+
+    object_initialize_child(obj, "dm163", &s->dm163, TYPE_DM163);
 }
 
 static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
@@ -124,9 +153,10 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
     Stm32l4x5SocState *s = STM32L4X5_SOC(dev_soc);
     const Stm32l4x5SocClass *sc = STM32L4X5_SOC_GET_CLASS(dev_soc);
     MemoryRegion *system_memory = get_system_memory();
-    DeviceState *armv7m, *dev;
+    DeviceState *armv7m, *dev, *gpio_output_fork;
     SysBusDevice *busdev;
     uint32_t pin_index;
+    int gpio, pin;
 
     if (!memory_region_init_rom(&s->flash, OBJECT(dev_soc), "flash",
                                 sc->flash_size, errp)) {
@@ -166,6 +196,12 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
         return;
     }
 
+    /* DM163 */
+    dev = DEVICE(&s->dm163);
+    if (!qdev_realize(dev, NULL, errp)) {
+        return;
+    }
+
     /* GPIOs */
     for (unsigned i = 0; i < NUM_GPIOS; i++) {
         g_autofree char *name = g_strdup_printf("%c", 'A' + i);
@@ -204,6 +240,23 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
         }
     }
 
+    for (unsigned i = 0; i < NUM_DM163_INPUTS; i++) {
+        gpio_output_fork = qdev_new(TYPE_SPLIT_IRQ);
+        qdev_prop_set_uint32(gpio_output_fork, "num-lines", 2);
+        qdev_realize_and_unref(gpio_output_fork, NULL, &error_fatal);
+
+        qdev_connect_gpio_out(gpio_output_fork, 0,
+                              qdev_get_gpio_in(DEVICE(&s->syscfg),
+                                               dm163_input[i]));
+        qdev_connect_gpio_out(gpio_output_fork, 1,
+                              qdev_get_gpio_in(DEVICE(&s->dm163),
+                                               i));
+        gpio = dm163_input[i] / 16;
+        pin = dm163_input[i] % 16;
+        qdev_connect_gpio_out(DEVICE(&s->gpio[gpio]), pin,
+                              qdev_get_gpio_in(DEVICE(gpio_output_fork), 0));
+    }
+
     /* EXTI device */
     busdev = SYS_BUS_DEVICE(&s->exti);
     if (!sysbus_realize(busdev, errp)) {
diff --git a/include/hw/arm/stm32l4x5_soc.h b/include/hw/arm/stm32l4x5_soc.h
index cb4da08629..60b31d430e 100644
--- a/include/hw/arm/stm32l4x5_soc.h
+++ b/include/hw/arm/stm32l4x5_soc.h
@@ -30,6 +30,7 @@
 #include "hw/misc/stm32l4x5_exti.h"
 #include "hw/misc/stm32l4x5_rcc.h"
 #include "hw/gpio/stm32l4x5_gpio.h"
+#include "hw/display/dm163.h"
 #include "qom/object.h"
 
 #define TYPE_STM32L4X5_SOC "stm32l4x5-soc"
@@ -48,6 +49,8 @@ struct Stm32l4x5SocState {
     Stm32l4x5RccState rcc;
     Stm32l4x5GpioState gpio[NUM_GPIOS];
 
+    DM163State dm163;
+
     MemoryRegion sram1;
     MemoryRegion sram2;
     MemoryRegion flash;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] tests/qtest : Add testcase for DM163
  2024-01-26 19:31 [PATCH 0/3] Add device DM163 (led driver, matrix colors shield & display) Inès Varhol
  2024-01-26 19:31 ` [PATCH 1/3] hw/display : Add device DM163 Inès Varhol
  2024-01-26 19:31 ` [PATCH 2/3] hw/arm : Connect DM163 to STM32L4x5 Inès Varhol
@ 2024-01-26 19:31 ` Inès Varhol
  2024-02-05 13:52   ` Philippe Mathieu-Daudé
  2024-02-05 14:03 ` [PATCH 0/3] Add device DM163 (led driver, matrix colors shield & display) Philippe Mathieu-Daudé
  3 siblings, 1 reply; 11+ messages in thread
From: Inès Varhol @ 2024-01-26 19:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Arnaud Minier, Paolo Bonzini, qemu-arm, Samuel Tardieu,
	Peter Maydell, Alistair Francis, Philippe Mathieu-Daudé,
	Thomas Huth, Laurent Vivier, Inès Varhol

`test_dm163_bank()`
Checks that the pin "sout" of the DM163 led driver outputs the values
received on pin "sin" with the expected latency (depending on the bank).

`test_dm163_gpio_connection()`
Check that changes to relevant STM32L4x5 GPIO pins are prpagated to the
DM163 device.

Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
 tests/qtest/dm163-test.c | 192 +++++++++++++++++++++++++++++++++++++++
 tests/qtest/meson.build  |   1 +
 2 files changed, 193 insertions(+)
 create mode 100644 tests/qtest/dm163-test.c

diff --git a/tests/qtest/dm163-test.c b/tests/qtest/dm163-test.c
new file mode 100644
index 0000000000..7691ce1af0
--- /dev/null
+++ b/tests/qtest/dm163-test.c
@@ -0,0 +1,192 @@
+/*
+ * QTest testcase for DM163
+ *
+ * Copyright (C) 2024 Samuel Tardieu <sam@rfc1149.net>
+ * Copyright (C) 2024 Arnaud Minier <arnaud.minier@telecom-paris.fr>
+ * Copyright (C) 2024 Inès Varhol <ines.varhol@telecom-paris.fr>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+
+#define SIN 8
+#define DCK 9
+#define RST_B 10
+#define LAT_B 11
+#define SELBK 12
+#define EN_B 13
+
+#define DEVICE_NAME "/machine/soc/dm163"
+#define GPIO_OUT(name, value) qtest_set_irq_in(qts, DEVICE_NAME, NULL, name,   \
+                                               value)
+#define GPIO_PULSE(name)                                                       \
+  do {                                                                         \
+    GPIO_OUT(name, 1);                                                         \
+    GPIO_OUT(name, 0);                                                         \
+  } while (0)
+
+
+static void rise_gpio_pin_dck(QTestState *qts)
+{
+    /* Configure output mode for pin PB1 */
+    qtest_writel(qts, 0x48000400, 0xFFFFFEB7);
+    /* Write 1 in ODR for PB1 */
+    qtest_writel(qts, 0x48000414, 0x00000002);
+}
+
+static void lower_gpio_pin_dck(QTestState *qts)
+{
+    /* Configure output mode for pin PB1 */
+    qtest_writel(qts, 0x48000400, 0xFFFFFEB7);
+    /* Write 0 in ODR for PB1 */
+    qtest_writel(qts, 0x48000414, 0x00000000);
+}
+
+static void rise_gpio_pin_selbk(QTestState *qts)
+{
+    /* Configure output mode for pin PC5 */
+    qtest_writel(qts, 0x48000800, 0xFFFFF7FF);
+    /* Write 1 in ODR for PC5 */
+    qtest_writel(qts, 0x48000814, 0x00000020);
+}
+
+static void lower_gpio_pin_selbk(QTestState *qts)
+{
+    /* Configure output mode for pin PC5 */
+    qtest_writel(qts, 0x48000800, 0xFFFFF7FF);
+    /* Write 0 in ODR for PC5 */
+    qtest_writel(qts, 0x48000814, 0x00000000);
+}
+
+static void rise_gpio_pin_lat_b(QTestState *qts)
+{
+    /* Configure output mode for pin PC4 */
+    qtest_writel(qts, 0x48000800, 0xFFFFFDFF);
+    /* Write 1 in ODR for PC4 */
+    qtest_writel(qts, 0x48000814, 0x00000010);
+}
+
+static void lower_gpio_pin_lat_b(QTestState *qts)
+{
+    /* Configure output mode for pin PC4 */
+    qtest_writel(qts, 0x48000800, 0xFFFFFDFF);
+    /* Write 0 in ODR for PC4 */
+    qtest_writel(qts, 0x48000814, 0x00000000);
+}
+
+static void rise_gpio_pin_rst_b(QTestState *qts)
+{
+    /* Configure output mode for pin PC3 */
+    qtest_writel(qts, 0x48000800, 0xFFFFFF7F);
+    /* Write 1 in ODR for PC3 */
+    qtest_writel(qts, 0x48000814, 0x00000008);
+}
+
+static void lower_gpio_pin_rst_b(QTestState *qts)
+{
+    /* Configure output mode for pin PC3 */
+    qtest_writel(qts, 0x48000800, 0xFFFFFF7F);
+    /* Write 0 in ODR for PC3 */
+    qtest_writel(qts, 0x48000814, 0x00000000);
+}
+
+static void rise_gpio_pin_sin(QTestState *qts)
+{
+    /* Configure output mode for pin PA4 */
+    qtest_writel(qts, 0x48000000, 0xFFFFFDFF);
+    /* Write 1 in ODR for PA4 */
+    qtest_writel(qts, 0x48000014, 0x00000010);
+}
+
+static void lower_gpio_pin_sin(QTestState *qts)
+{
+    /* Configure output mode for pin PA4 */
+    qtest_writel(qts, 0x48000000, 0xFFFFFDFF);
+    /* Write 0 in ODR for PA4 */
+    qtest_writel(qts, 0x48000014, 0x00000000);
+}
+
+static void test_dm163_bank(const void *opaque)
+{
+    const long bank = (uintptr_t) opaque;
+    const int width = bank ? 192 : 144;
+
+    QTestState *qts = qtest_initf("-M b-l475e-iot01a");
+    qtest_irq_intercept_out_named(qts, DEVICE_NAME, "sout");
+    GPIO_OUT(RST_B, 1);
+    GPIO_OUT(EN_B, 0);
+    GPIO_OUT(DCK, 0);
+    GPIO_OUT(SELBK, bank);
+    GPIO_OUT(LAT_B, 1);
+
+    /* Fill bank with zeroes */
+    GPIO_OUT(SIN, 0);
+    for (int i = 0; i < width; i++) {
+        GPIO_PULSE(DCK);
+    }
+    /* Fill bank with ones, check that we get the previous zeroes */
+    GPIO_OUT(SIN, 1);
+    for (int i = 0; i < width; i++) {
+        GPIO_PULSE(DCK);
+        g_assert(!qtest_get_irq(qts, 0));
+    }
+
+    /* Pulse one more bit in the bank, check that we get a one */
+    GPIO_PULSE(DCK);
+    g_assert(qtest_get_irq(qts, 0));
+
+    qtest_quit(qts);
+}
+
+static void test_dm163_gpio_connection(void)
+{
+    QTestState *qts = qtest_init("-M b-l475e-iot01a");
+    qtest_irq_intercept_in(qts, "/machine/soc/dm163");
+
+    g_assert_false(qtest_get_irq(qts, SIN));
+    g_assert_false(qtest_get_irq(qts, DCK));
+    g_assert_false(qtest_get_irq(qts, RST_B));
+    g_assert_false(qtest_get_irq(qts, LAT_B));
+    g_assert_false(qtest_get_irq(qts, SELBK));
+
+    rise_gpio_pin_dck(qts);
+    g_assert_true(qtest_get_irq(qts, DCK));
+    lower_gpio_pin_dck(qts);
+    g_assert_false(qtest_get_irq(qts, DCK));
+
+    rise_gpio_pin_lat_b(qts);
+    g_assert_true(qtest_get_irq(qts, LAT_B));
+    lower_gpio_pin_lat_b(qts);
+    g_assert_false(qtest_get_irq(qts, LAT_B));
+
+    rise_gpio_pin_selbk(qts);
+    g_assert_true(qtest_get_irq(qts, SELBK));
+    lower_gpio_pin_selbk(qts);
+    g_assert_false(qtest_get_irq(qts, SELBK));
+
+    rise_gpio_pin_rst_b(qts);
+    g_assert_true(qtest_get_irq(qts, RST_B));
+    lower_gpio_pin_rst_b(qts);
+    g_assert_false(qtest_get_irq(qts, RST_B));
+
+    rise_gpio_pin_sin(qts);
+    g_assert_true(qtest_get_irq(qts, SIN));
+    lower_gpio_pin_sin(qts);
+    g_assert_false(qtest_get_irq(qts, SIN));
+
+    g_assert_false(qtest_get_irq(qts, DCK));
+    g_assert_false(qtest_get_irq(qts, LAT_B));
+    g_assert_false(qtest_get_irq(qts, SELBK));
+    g_assert_false(qtest_get_irq(qts, RST_B));
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+    qtest_add_data_func("/dm163/bank0", (void *)0, test_dm163_bank);
+    qtest_add_data_func("/dm163/bank1", (void *)1, test_dm163_bank);
+    qtest_add_func("/dm163/gpio_connection", test_dm163_gpio_connection);
+    return g_test_run();
+}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 5692da4fc1..e9f6ac46ef 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -22,6 +22,7 @@ qtests_generic = [
   'qos-test',
   'readconfig-test',
   'netdev-socket',
+  'dm163-test',
 ]
 if enable_modules
   qtests_generic += [ 'modules-test' ]
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] hw/display : Add device DM163
  2024-01-26 19:31 ` [PATCH 1/3] hw/display : Add device DM163 Inès Varhol
@ 2024-02-05  0:09   ` Alistair Francis
  0 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2024-02-05  0:09 UTC (permalink / raw)
  To: Inès Varhol
  Cc: qemu-devel, Arnaud Minier, Paolo Bonzini, qemu-arm,
	Samuel Tardieu, Peter Maydell, Alistair Francis,
	Philippe Mathieu-Daudé, Thomas Huth, Laurent Vivier

On Sat, Jan 27, 2024 at 5:38 AM Inès Varhol
<ines.varhol@telecom-paris.fr> wrote:
>
> This device implements the IM120417002 colors shield v1.1 for Arduino
> (which relies on the DM163 8x3-channel led driving logic) and features
> a simple display of an 8x8 RGB matrix. The columns of the matrix are
> driven by the DM163 and the rows are driven externally.
>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/display/Kconfig         |   3 +
>  hw/display/dm163.c         | 307 +++++++++++++++++++++++++++++++++++++
>  hw/display/meson.build     |   1 +
>  hw/display/trace-events    |  13 ++
>  include/hw/display/dm163.h |  57 +++++++
>  5 files changed, 381 insertions(+)
>  create mode 100644 hw/display/dm163.c
>  create mode 100644 include/hw/display/dm163.h
>
> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> index 1aafe1923d..4dbfc6e7af 100644
> --- a/hw/display/Kconfig
> +++ b/hw/display/Kconfig
> @@ -139,3 +139,6 @@ config XLNX_DISPLAYPORT
>      bool
>      # defaults to "N", enabled by specific boards
>      depends on PIXMAN
> +
> +config DM163
> +    bool
> diff --git a/hw/display/dm163.c b/hw/display/dm163.c
> new file mode 100644
> index 0000000000..565fc84ddf
> --- /dev/null
> +++ b/hw/display/dm163.c
> @@ -0,0 +1,307 @@
> +/*
> + * QEMU DM163 8x3-channel constant current led driver
> + * driving columns of associated 8x8 RGB matrix.
> + *
> + * Copyright (C) 2024 Samuel Tardieu <sam@rfc1149.net>
> + * Copyright (C) 2024 Arnaud Minier <arnaud.minier@telecom-paris.fr>
> + * Copyright (C) 2024 Inès Varhol <ines.varhol@telecom-paris.fr>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +/*
> + * The reference used for the DM163 is the following :
> + * http://www.siti.com.tw/product/spec/LED/DM163.pdf
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "migration/vmstate.h"
> +#include "hw/irq.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/display/dm163.h"
> +#include "ui/console.h"
> +#include "trace.h"
> +
> +#define LED_SQUARE_SIZE 100
> +/* Number of frames a row stays visible after being turned off. */
> +#define ROW_PERSISTANCE 2
> +
> +static const VMStateDescription vmstate_dm163 = {
> +    .name = TYPE_DM163,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (const VMStateField[]) {
> +        VMSTATE_UINT8(activated_rows, DM163State),
> +        VMSTATE_UINT64_ARRAY(bank0_shift_register, DM163State, 3),
> +        VMSTATE_UINT64_ARRAY(bank1_shift_register, DM163State, 3),
> +        VMSTATE_UINT16_ARRAY(latched_outputs, DM163State, DM163_NUM_LEDS),
> +        VMSTATE_UINT16_ARRAY(outputs, DM163State, DM163_NUM_LEDS),
> +        VMSTATE_UINT8(dck, DM163State),
> +        VMSTATE_UINT8(en_b, DM163State),
> +        VMSTATE_UINT8(lat_b, DM163State),
> +        VMSTATE_UINT8(rst_b, DM163State),
> +        VMSTATE_UINT8(selbk, DM163State),
> +        VMSTATE_UINT8(sin, DM163State),
> +        VMSTATE_UINT32_2DARRAY(buffer, DM163State,
> +            COLOR_BUFFER_SIZE + 1, RGB_MATRIX_NUM_COLS),
> +        VMSTATE_UINT8(last_buffer_idx, DM163State),
> +        VMSTATE_UINT8_ARRAY(buffer_idx_of_row, DM163State, RGB_MATRIX_NUM_ROWS),
> +        VMSTATE_UINT8_ARRAY(age_of_row, DM163State, RGB_MATRIX_NUM_ROWS),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void dm163_reset_hold(Object *obj)
> +{
> +    DM163State *s = DM163(obj);
> +
> +    /* Reset only stops the PWM. */
> +    memset(s->outputs, 0, sizeof(s->outputs));
> +
> +    /* The last row of the buffer stores a turned off row */
> +    memset(s->buffer[COLOR_BUFFER_SIZE], 0, sizeof(s->buffer[0]));
> +}
> +
> +static void dm163_dck_gpio_handler(void *opaque, int line, int new_state)
> +{
> +    DM163State *s = DM163(opaque);
> +
> +    if (new_state && !s->dck) {
> +        /*
> +         * On raising dck, sample selbk to get the bank to use, and
> +         * sample sin for the bit to enter into the bank shift buffer.
> +         */
> +        uint64_t *sb =
> +            s->selbk ? s->bank1_shift_register : s->bank0_shift_register;
> +        /* Output the outgoing bit on sout */
> +        const bool sout = (s->selbk ? sb[2] & MAKE_64BIT_MASK(63, 1) :
> +                           sb[2] & MAKE_64BIT_MASK(15, 1)) != 0;
> +        qemu_set_irq(s->sout, sout);
> +        /* Enter sin into the shift buffer */
> +        sb[2] = (sb[2] << 1) | ((sb[1] >> 63) & 1);
> +        sb[1] = (sb[1] << 1) | ((sb[0] >> 63) & 1);
> +        sb[0] = (sb[0] << 1) | s->sin;
> +    }
> +
> +    s->dck = new_state;
> +    trace_dm163_dck(new_state);
> +}
> +
> +static void dm163_propagate_outputs(DM163State *s)
> +{
> +    s->last_buffer_idx = (s->last_buffer_idx + 1) % COLOR_BUFFER_SIZE;
> +    /* Values are output when reset and enable are both high. */
> +    if (s->rst_b && !s->en_b) {
> +        memcpy(s->outputs, s->latched_outputs, sizeof(s->outputs));
> +    } else {
> +        memset(s->outputs, 0, sizeof(s->outputs));
> +    }
> +    for (unsigned x = 0; x < RGB_MATRIX_NUM_COLS; x++) {
> +        trace_dm163_channels(3 * x, (uint8_t)(s->outputs[3 * x] >> 6));
> +        trace_dm163_channels(3 * x + 1, (uint8_t)(s->outputs[3 * x + 1] >> 6));
> +        trace_dm163_channels(3 * x + 2, (uint8_t)(s->outputs[3 * x + 2] >> 6));
> +        s->buffer[s->last_buffer_idx][x] =
> +            (s->outputs[3 * x + 2] >> 6) |
> +            ((s->outputs[3 * x + 1] << 2) & 0xFF00) |
> +            (((uint32_t)s->outputs[3 * x] << 10) & 0xFF0000);
> +    }
> +    for (unsigned row = 0; row < RGB_MATRIX_NUM_ROWS; row++) {
> +        if (s->activated_rows & (1 << row)) {
> +            s->buffer_idx_of_row[row] = s->last_buffer_idx;
> +        }
> +    }
> +}
> +
> +static void dm163_en_b_gpio_handler(void *opaque, int line, int new_state)
> +{
> +    DM163State *s = DM163(opaque);
> +
> +    s->en_b = new_state;
> +    dm163_propagate_outputs(s);
> +    trace_dm163_en_b(new_state);
> +}
> +
> +static inline uint8_t dm163_bank0(const DM163State *s, uint8_t led)
> +{
> +    /*
> +     * Bank 1 uses 6 bits per led, so a value may be stored accross
> +     * two uint64_t entries.
> +     */
> +    const uint8_t low_bit = 6 * led;
> +    const uint8_t low_word = low_bit / 64;
> +    const uint8_t high_word = (low_bit + 5) / 64;
> +    const uint8_t low_shift = low_bit % 64;
> +
> +    if (low_word == high_word) {
> +        /* Simple case: the value belongs to one entry. */
> +        return (s->bank0_shift_register[low_word] &
> +                MAKE_64BIT_MASK(low_shift, 6)) >> low_shift;
> +    }
> +
> +    const uint8_t bits_in_low_word = 64 - low_shift;
> +    const uint8_t bits_in_high_word = 6 - bits_in_low_word;
> +    return ((s->bank0_shift_register[low_word] &
> +             MAKE_64BIT_MASK(low_shift, bits_in_low_word)) >>
> +            low_shift) |
> +           ((s->bank0_shift_register[high_word] &
> +             MAKE_64BIT_MASK(0, bits_in_high_word))
> +         << bits_in_low_word);
> +}
> +
> +static inline uint8_t dm163_bank1(const DM163State *s, uint8_t led)
> +{
> +    const uint64_t entry = s->bank1_shift_register[led / 8];
> +    const unsigned shift = 8 * (led % 8);
> +    return (entry & MAKE_64BIT_MASK(shift, 8)) >> shift;
> +}
> +
> +static void dm163_lat_b_gpio_handler(void *opaque, int line, int new_state)
> +{
> +    DM163State *s = DM163(opaque);
> +
> +    if (s->lat_b && !new_state) {
> +        for (int led = 0; led < DM163_NUM_LEDS; led++) {
> +            s->latched_outputs[led] = dm163_bank0(s, led) * dm163_bank1(s, led);
> +        }
> +        dm163_propagate_outputs(s);
> +    }
> +
> +    s->lat_b = new_state;
> +    trace_dm163_lat_b(new_state);
> +}
> +
> +static void dm163_rst_b_gpio_handler(void *opaque, int line, int new_state)
> +{
> +    DM163State *s = DM163(opaque);
> +
> +    s->rst_b = new_state;
> +    dm163_propagate_outputs(s);
> +    trace_dm163_rst_b(new_state);
> +}
> +
> +static void dm163_selbk_gpio_handler(void *opaque, int line, int new_state)
> +{
> +    DM163State *s = DM163(opaque);
> +
> +    s->selbk = new_state;
> +    trace_dm163_selbk(new_state);
> +}
> +
> +static void dm163_sin_gpio_handler(void *opaque, int line, int new_state)
> +{
> +    DM163State *s = DM163(opaque);
> +
> +    s->sin = new_state;
> +    trace_dm163_sin(new_state);
> +}
> +
> +static void dm163_rows_gpio_handler(void *opaque, int line, int new_state)
> +{
> +    DM163State *s = DM163(opaque);
> +
> +    if (new_state) {
> +        s->activated_rows |= (1 << line);
> +        s->buffer_idx_of_row[line] = s->last_buffer_idx;
> +        s->age_of_row[line] = 0;
> +    } else {
> +        s->activated_rows &= ~(1 << line);
> +        s->age_of_row[line] = ROW_PERSISTANCE;
> +    }
> +    trace_dm163_activated_rows(s->activated_rows);
> +}
> +
> +static void dm163_invalidate_display(void *opaque)
> +{
> +}
> +
> +static void dm163_update_display(void *opaque)
> +{
> +    DM163State *s = (DM163State *)opaque;
> +    DisplaySurface *surface = qemu_console_surface(s->console);
> +    uint32_t *dest;
> +    unsigned bits_ppi = surface_bits_per_pixel(surface);
> +
> +    trace_dm163_bits_ppi(bits_ppi);
> +    g_assert((bits_ppi == 32));
> +    dest = surface_data(surface);
> +    for (unsigned y = 0; y < RGB_MATRIX_NUM_ROWS; y++) {
> +        for (unsigned _ = 0; _ < LED_SQUARE_SIZE; _++) {
> +            for (int x = RGB_MATRIX_NUM_COLS * LED_SQUARE_SIZE - 1; x >= 0; x--) {
> +                *dest++ = s->buffer[s->buffer_idx_of_row[y]][x / LED_SQUARE_SIZE];
> +            }
> +        }
> +        if (s->age_of_row[y]) {
> +            s->age_of_row[y]--;
> +            if (!s->age_of_row[y]) {
> +                /*
> +                 * If the ROW_PERSISTANCE delay is up,
> +                 * the row is turned off.
> +                 * (s->buffer[COLOR_BUFFER] is filled with 0)
> +                 */
> +                s->buffer_idx_of_row[y] = COLOR_BUFFER_SIZE;
> +            }
> +        }
> +    }
> +    /*
> +     * Ideally set the refresh rate so that the row persistance
> +     * doesn't need to be changed.
> +     *
> +     * Currently `dpy_ui_info_supported(s->console)` returns false
> +     * which makes it impossible to get or set UIInfo.
> +     *
> +     * if (dpy_ui_info_supported(s->console)) {
> +     *     trace_dm163_refresh_rate(dpy_get_ui_info(s->console)->refresh_rate);
> +     * } else {
> +     *     trace_dm163_refresh_rate(0);
> +     * }
> +     */
> +    dpy_gfx_update(s->console, 0, 0, RGB_MATRIX_NUM_COLS * LED_SQUARE_SIZE,
> +                   RGB_MATRIX_NUM_ROWS * LED_SQUARE_SIZE);
> +}
> +
> +static const GraphicHwOps dm163_ops = {
> +    .invalidate  = dm163_invalidate_display,
> +    .gfx_update  = dm163_update_display,
> +};
> +
> +static void dm163_realize(DeviceState *dev, Error **errp)
> +{
> +    DM163State *s = DM163(dev);
> +
> +    qdev_init_gpio_in(dev, dm163_rows_gpio_handler, 8);
> +    qdev_init_gpio_in(dev, dm163_sin_gpio_handler, 1);
> +    qdev_init_gpio_in(dev, dm163_dck_gpio_handler, 1);
> +    qdev_init_gpio_in(dev, dm163_rst_b_gpio_handler, 1);
> +    qdev_init_gpio_in(dev, dm163_lat_b_gpio_handler, 1);
> +    qdev_init_gpio_in(dev, dm163_selbk_gpio_handler, 1);
> +    qdev_init_gpio_in(dev, dm163_en_b_gpio_handler, 1);
> +    qdev_init_gpio_out_named(dev, &s->sout, "sout", 1);
> +
> +    s->console = graphic_console_init(dev, 0, &dm163_ops, s);
> +    qemu_console_resize(s->console, RGB_MATRIX_NUM_COLS * LED_SQUARE_SIZE,
> +                        RGB_MATRIX_NUM_ROWS * LED_SQUARE_SIZE);
> +}
> +
> +static void dm163_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    ResettableClass *rc = RESETTABLE_CLASS(klass);
> +
> +    dc->desc = "DM163";
> +    dc->vmsd = &vmstate_dm163;
> +    dc->realize = dm163_realize;
> +    rc->phases.hold = dm163_reset_hold;
> +    set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
> +}
> +
> +static const TypeInfo dm163_types[] = {
> +    {
> +        .name = TYPE_DM163,
> +        .parent = TYPE_DEVICE,
> +        .instance_size = sizeof(DM163State),
> +        .class_init = dm163_class_init
> +    }
> +};
> +
> +DEFINE_TYPES(dm163_types)
> diff --git a/hw/display/meson.build b/hw/display/meson.build
> index f93a69f70f..71e489308e 100644
> --- a/hw/display/meson.build
> +++ b/hw/display/meson.build
> @@ -38,6 +38,7 @@ system_ss.add(when: 'CONFIG_NEXTCUBE', if_true: files('next-fb.c'))
>
>  system_ss.add(when: 'CONFIG_VGA', if_true: files('vga.c'))
>  system_ss.add(when: 'CONFIG_VIRTIO', if_true: files('virtio-dmabuf.c'))
> +system_ss.add(when: 'CONFIG_DM163', if_true: files('dm163.c'))
>
>  if (config_all_devices.has_key('CONFIG_VGA_CIRRUS') or
>      config_all_devices.has_key('CONFIG_VGA_PCI') or
> diff --git a/hw/display/trace-events b/hw/display/trace-events
> index 2336a0ca15..444b014d6e 100644
> --- a/hw/display/trace-events
> +++ b/hw/display/trace-events
> @@ -177,3 +177,16 @@ macfb_ctrl_write(uint64_t addr, uint64_t value, unsigned int size) "addr 0x%"PRI
>  macfb_sense_read(uint32_t value) "video sense: 0x%"PRIx32
>  macfb_sense_write(uint32_t value) "video sense: 0x%"PRIx32
>  macfb_update_mode(uint32_t width, uint32_t height, uint8_t depth) "setting mode to width %"PRId32 " height %"PRId32 " size %d"
> +
> +# dm163.c
> +dm163_dck(int new_state) "dck : %d"
> +dm163_en_b(int new_state) "en_b : %d"
> +dm163_rst_b(int new_state) "rst_b : %d"
> +dm163_lat_b(int new_state) "lat_b : %d"
> +dm163_sin(int new_state) "sin : %d"
> +dm163_selbk(int new_state) "selbk : %d"
> +dm163_activated_rows(int new_state) "Activated rows : 0x%" PRIx32 ""
> +dm163_bits_ppi(unsigned dest_width) "dest_width : %u"
> +dm163_leds(int led, uint32_t value) "led %d: 0x%x"
> +dm163_channels(int channel, uint8_t value) "channel %d: 0x%x"
> +dm163_refresh_rate(uint32_t rr) "refresh rate %d"
> diff --git a/include/hw/display/dm163.h b/include/hw/display/dm163.h
> new file mode 100644
> index 0000000000..aa775e51e1
> --- /dev/null
> +++ b/include/hw/display/dm163.h
> @@ -0,0 +1,57 @@
> +/*
> + * QEMU DM163 8x3-channel constant current led driver
> + * driving columns of associated 8x8 RGB matrix.
> + *
> + * Copyright (C) 2024 Samuel Tardieu <sam@rfc1149.net>
> + * Copyright (C) 2024 Arnaud Minier <arnaud.minier@telecom-paris.fr>
> + * Copyright (C) 2024 Inès Varhol <ines.varhol@telecom-paris.fr>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_DISPLAY_DM163_H
> +#define HW_DISPLAY_DM163_H
> +
> +#include "qom/object.h"
> +#include "hw/qdev-core.h"
> +
> +#define TYPE_DM163 "dm163"
> +OBJECT_DECLARE_SIMPLE_TYPE(DM163State, DM163);
> +
> +#define DM163_NUM_LEDS 24
> +#define RGB_MATRIX_NUM_ROWS 8
> +#define RGB_MATRIX_NUM_COLS (DM163_NUM_LEDS / 3)
> +#define COLOR_BUFFER_SIZE RGB_MATRIX_NUM_ROWS
> +
> +typedef struct DM163State {
> +    DeviceState parent_obj;
> +
> +    /* DM163 driver */
> +    uint64_t bank0_shift_register[3];
> +    uint64_t bank1_shift_register[3];
> +    uint16_t latched_outputs[DM163_NUM_LEDS];
> +    uint16_t outputs[DM163_NUM_LEDS];
> +    qemu_irq sout;
> +
> +    uint8_t dck;
> +    uint8_t en_b;
> +    uint8_t lat_b;
> +    uint8_t rst_b;
> +    uint8_t selbk;
> +    uint8_t sin;
> +
> +    /* IM120417002 colors shield */
> +    uint8_t activated_rows;
> +
> +    /* 8x8 RGB matrix */
> +    QemuConsole *console;
> +    /* Rows currently being displayed on the matrix. */
> +    /* The last row is filled with 0 (turned off row) */
> +    uint32_t buffer[COLOR_BUFFER_SIZE + 1][RGB_MATRIX_NUM_COLS];
> +    uint8_t last_buffer_idx;
> +    uint8_t buffer_idx_of_row[RGB_MATRIX_NUM_ROWS];
> +    /* Used to simulate retinal persistance of rows */
> +    uint8_t age_of_row[RGB_MATRIX_NUM_ROWS];
> +} DM163State;
> +
> +#endif /* HW_DISPLAY_DM163_H */
> --
> 2.43.0
>
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] hw/arm : Connect DM163 to STM32L4x5
  2024-01-26 19:31 ` [PATCH 2/3] hw/arm : Connect DM163 to STM32L4x5 Inès Varhol
@ 2024-02-05  0:14   ` Alistair Francis
  2024-02-05 13:46   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2024-02-05  0:14 UTC (permalink / raw)
  To: Inès Varhol
  Cc: qemu-devel, Arnaud Minier, Paolo Bonzini, qemu-arm,
	Samuel Tardieu, Peter Maydell, Alistair Francis,
	Philippe Mathieu-Daudé, Thomas Huth, Laurent Vivier

On Sat, Jan 27, 2024 at 7:09 AM Inès Varhol
<ines.varhol@telecom-paris.fr> wrote:
>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/arm/Kconfig                 |  1 +
>  hw/arm/stm32l4x5_soc.c         | 55 +++++++++++++++++++++++++++++++++-
>  include/hw/arm/stm32l4x5_soc.h |  3 ++
>  3 files changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 3e49b913f8..818aa2f1a2 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -463,6 +463,7 @@ config STM32L4X5_SOC
>      select STM32L4X5_SYSCFG
>      select STM32L4X5_RCC
>      select STM32L4X5_GPIO
> +    select DM163
>
>  config XLNX_ZYNQMP_ARM
>      bool
> diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
> index 478c6ba056..8663546901 100644
> --- a/hw/arm/stm32l4x5_soc.c
> +++ b/hw/arm/stm32l4x5_soc.c
> @@ -26,7 +26,9 @@
>  #include "qapi/error.h"
>  #include "exec/address-spaces.h"
>  #include "sysemu/sysemu.h"
> +#include "hw/core/split-irq.h"
>  #include "hw/arm/stm32l4x5_soc.h"
> +#include "hw/display/dm163.h"
>  #include "hw/qdev-clock.h"
>  #include "hw/misc/unimp.h"
>
> @@ -78,6 +80,31 @@ static const int exti_irq[NUM_EXTI_IRQ] = {
>  #define RCC_BASE_ADDRESS 0x40021000
>  #define RCC_IRQ 5
>
> +/*
> + * There are actually 14 input pins in the DM163 device.
> + * Here the DM163 input pin EN isn't connected to the STM32L4x5
> + * GPIOs as the IM120417002 colors shield doesn't actually use
> + * this pin to drive the RGB matrix.
> + */
> +#define NUM_DM163_INPUTS 13
> +
> +static const int dm163_input[NUM_DM163_INPUTS] = {
> +    1 * 16 + 2,  /* ROW0  PB2       */
> +    0 * 16 + 15, /* ROW1  PA15      */
> +    0 * 16 + 2,  /* ROW2  PA2       */
> +    0 * 16 + 7,  /* ROW3  PA7       */
> +    0 * 16 + 6,  /* ROW4  PA6       */
> +    0 * 16 + 5,  /* ROW5  PA5       */
> +    1 * 16 + 0,  /* ROW6  PB0       */
> +    0 * 16 + 3,  /* ROW7  PA3       */
> +    0 * 16 + 4,  /* SIN (SDA) PA4   */
> +    1 * 16 + 1,  /* DCK (SCK) PB1   */
> +    2 * 16 + 3,  /* RST_B (RST) PC3 */
> +    2 * 16 + 4,  /* LAT_B (LAT) PC4 */
> +    2 * 16 + 5,  /* SELBK (SB)  PC5 */
> +};
> +
> +
>  static const uint32_t gpio_addr[] = {
>      0x48000000,
>      0x48000400,
> @@ -116,6 +143,8 @@ static void stm32l4x5_soc_initfn(Object *obj)
>          g_autofree char *name = g_strdup_printf("gpio%c", 'a' + i);
>          object_initialize_child(obj, name, &s->gpio[i], TYPE_STM32L4X5_GPIO);
>      }
> +
> +    object_initialize_child(obj, "dm163", &s->dm163, TYPE_DM163);
>  }
>
>  static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
> @@ -124,9 +153,10 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
>      Stm32l4x5SocState *s = STM32L4X5_SOC(dev_soc);
>      const Stm32l4x5SocClass *sc = STM32L4X5_SOC_GET_CLASS(dev_soc);
>      MemoryRegion *system_memory = get_system_memory();
> -    DeviceState *armv7m, *dev;
> +    DeviceState *armv7m, *dev, *gpio_output_fork;
>      SysBusDevice *busdev;
>      uint32_t pin_index;
> +    int gpio, pin;
>
>      if (!memory_region_init_rom(&s->flash, OBJECT(dev_soc), "flash",
>                                  sc->flash_size, errp)) {
> @@ -166,6 +196,12 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
>          return;
>      }
>
> +    /* DM163 */
> +    dev = DEVICE(&s->dm163);
> +    if (!qdev_realize(dev, NULL, errp)) {
> +        return;
> +    }
> +
>      /* GPIOs */
>      for (unsigned i = 0; i < NUM_GPIOS; i++) {
>          g_autofree char *name = g_strdup_printf("%c", 'A' + i);
> @@ -204,6 +240,23 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
>          }
>      }
>
> +    for (unsigned i = 0; i < NUM_DM163_INPUTS; i++) {
> +        gpio_output_fork = qdev_new(TYPE_SPLIT_IRQ);
> +        qdev_prop_set_uint32(gpio_output_fork, "num-lines", 2);
> +        qdev_realize_and_unref(gpio_output_fork, NULL, &error_fatal);
> +
> +        qdev_connect_gpio_out(gpio_output_fork, 0,
> +                              qdev_get_gpio_in(DEVICE(&s->syscfg),
> +                                               dm163_input[i]));
> +        qdev_connect_gpio_out(gpio_output_fork, 1,
> +                              qdev_get_gpio_in(DEVICE(&s->dm163),
> +                                               i));
> +        gpio = dm163_input[i] / 16;
> +        pin = dm163_input[i] % 16;
> +        qdev_connect_gpio_out(DEVICE(&s->gpio[gpio]), pin,
> +                              qdev_get_gpio_in(DEVICE(gpio_output_fork), 0));
> +    }
> +
>      /* EXTI device */
>      busdev = SYS_BUS_DEVICE(&s->exti);
>      if (!sysbus_realize(busdev, errp)) {
> diff --git a/include/hw/arm/stm32l4x5_soc.h b/include/hw/arm/stm32l4x5_soc.h
> index cb4da08629..60b31d430e 100644
> --- a/include/hw/arm/stm32l4x5_soc.h
> +++ b/include/hw/arm/stm32l4x5_soc.h
> @@ -30,6 +30,7 @@
>  #include "hw/misc/stm32l4x5_exti.h"
>  #include "hw/misc/stm32l4x5_rcc.h"
>  #include "hw/gpio/stm32l4x5_gpio.h"
> +#include "hw/display/dm163.h"
>  #include "qom/object.h"
>
>  #define TYPE_STM32L4X5_SOC "stm32l4x5-soc"
> @@ -48,6 +49,8 @@ struct Stm32l4x5SocState {
>      Stm32l4x5RccState rcc;
>      Stm32l4x5GpioState gpio[NUM_GPIOS];
>
> +    DM163State dm163;
> +
>      MemoryRegion sram1;
>      MemoryRegion sram2;
>      MemoryRegion flash;
> --
> 2.43.0
>
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] hw/arm : Connect DM163 to STM32L4x5
  2024-01-26 19:31 ` [PATCH 2/3] hw/arm : Connect DM163 to STM32L4x5 Inès Varhol
  2024-02-05  0:14   ` Alistair Francis
@ 2024-02-05 13:46   ` Philippe Mathieu-Daudé
  2024-02-07 17:25     ` Inès Varhol
  1 sibling, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-05 13:46 UTC (permalink / raw)
  To: Inès Varhol, qemu-devel
  Cc: Arnaud Minier, Paolo Bonzini, qemu-arm, Samuel Tardieu,
	Peter Maydell, Alistair Francis, Thomas Huth, Laurent Vivier

Hi Inès,

On 26/1/24 20:31, Inès Varhol wrote:
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
>   hw/arm/Kconfig                 |  1 +
>   hw/arm/stm32l4x5_soc.c         | 55 +++++++++++++++++++++++++++++++++-
>   include/hw/arm/stm32l4x5_soc.h |  3 ++
>   3 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 3e49b913f8..818aa2f1a2 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -463,6 +463,7 @@ config STM32L4X5_SOC
>       select STM32L4X5_SYSCFG
>       select STM32L4X5_RCC
>       select STM32L4X5_GPIO
> +    select DM163


> +/*
> + * There are actually 14 input pins in the DM163 device.
> + * Here the DM163 input pin EN isn't connected to the STM32L4x5
> + * GPIOs as the IM120417002 colors shield doesn't actually use
> + * this pin to drive the RGB matrix.
> + */
> +#define NUM_DM163_INPUTS 13
> +
> +static const int dm163_input[NUM_DM163_INPUTS] = {
> +    1 * 16 + 2,  /* ROW0  PB2       */
> +    0 * 16 + 15, /* ROW1  PA15      */
> +    0 * 16 + 2,  /* ROW2  PA2       */
> +    0 * 16 + 7,  /* ROW3  PA7       */
> +    0 * 16 + 6,  /* ROW4  PA6       */
> +    0 * 16 + 5,  /* ROW5  PA5       */
> +    1 * 16 + 0,  /* ROW6  PB0       */
> +    0 * 16 + 3,  /* ROW7  PA3       */
> +    0 * 16 + 4,  /* SIN (SDA) PA4   */
> +    1 * 16 + 1,  /* DCK (SCK) PB1   */
> +    2 * 16 + 3,  /* RST_B (RST) PC3 */
> +    2 * 16 + 4,  /* LAT_B (LAT) PC4 */
> +    2 * 16 + 5,  /* SELBK (SB)  PC5 */
> +};
> +
> +
>   static const uint32_t gpio_addr[] = {
>       0x48000000,
>       0x48000400,
> @@ -116,6 +143,8 @@ static void stm32l4x5_soc_initfn(Object *obj)
>           g_autofree char *name = g_strdup_printf("gpio%c", 'a' + i);
>           object_initialize_child(obj, name, &s->gpio[i], TYPE_STM32L4X5_GPIO);
>       }
> +
> +    object_initialize_child(obj, "dm163", &s->dm163, TYPE_DM163);

The DM163 is another chip, not a component part of the SoC;
it belongs to the machine and should be created/wired in
b_l475e_iot01a_init(). Similarly to the IRQ splitters.

Keeping board component states in a Bl475eMachineState structure
could help organizing your model. You can find an example on how
extend the MachineState in hw/avr/arduino.c.

You might call qdev_pass_gpios() to exposes the SysCfg lines out
of the SoC.

Regards,

Phil.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] tests/qtest : Add testcase for DM163
  2024-01-26 19:31 ` [PATCH 3/3] tests/qtest : Add testcase for DM163 Inès Varhol
@ 2024-02-05 13:52   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-05 13:52 UTC (permalink / raw)
  To: Inès Varhol, qemu-devel
  Cc: Arnaud Minier, Paolo Bonzini, qemu-arm, Samuel Tardieu,
	Peter Maydell, Alistair Francis, Thomas Huth, Laurent Vivier

Hi Inès,

On 26/1/24 20:31, Inès Varhol wrote:
> `test_dm163_bank()`
> Checks that the pin "sout" of the DM163 led driver outputs the values
> received on pin "sin" with the expected latency (depending on the bank).
> 
> `test_dm163_gpio_connection()`
> Check that changes to relevant STM32L4x5 GPIO pins are prpagated to the
> DM163 device.
> 
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
>   tests/qtest/dm163-test.c | 192 +++++++++++++++++++++++++++++++++++++++
>   tests/qtest/meson.build  |   1 +
>   2 files changed, 193 insertions(+)
>   create mode 100644 tests/qtest/dm163-test.c


> +static void test_dm163_gpio_connection(void)
> +{
> +    QTestState *qts = qtest_init("-M b-l475e-iot01a");

This machine is only available in ARM binaries, ...

> +}
> +
> +int main(int argc, char **argv)
> +{
> +    g_test_init(&argc, &argv, NULL);
> +    qtest_add_data_func("/dm163/bank0", (void *)0, test_dm163_bank);
> +    qtest_add_data_func("/dm163/bank1", (void *)1, test_dm163_bank);
> +    qtest_add_func("/dm163/gpio_connection", test_dm163_gpio_connection);
> +    return g_test_run();
> +}
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 5692da4fc1..e9f6ac46ef 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -22,6 +22,7 @@ qtests_generic = [
>     'qos-test',
>     'readconfig-test',
>     'netdev-socket',
> +  'dm163-test',
... so I'd expect the test to be restricted to ARM & DM163
availability:

-- >8 --
@@ -206,2 +206,3 @@ qtests_stm32l4x5 = \
  qtests_arm = \
+  (config_all_devices.has_key('CONFIG_DM163') ? ['dm163-test'] : []) + \
    (config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : 
[]) + \
---

BTW please consider enabling scripts/git.orderfile to ease reviewing
your series :)

Regards,

Phil.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] Add device DM163 (led driver, matrix colors shield & display)
  2024-01-26 19:31 [PATCH 0/3] Add device DM163 (led driver, matrix colors shield & display) Inès Varhol
                   ` (2 preceding siblings ...)
  2024-01-26 19:31 ` [PATCH 3/3] tests/qtest : Add testcase for DM163 Inès Varhol
@ 2024-02-05 14:03 ` Philippe Mathieu-Daudé
  2024-02-07 19:27   ` Inès Varhol
  3 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-05 14:03 UTC (permalink / raw)
  To: Inès Varhol, qemu-devel
  Cc: Arnaud Minier, Paolo Bonzini, qemu-arm, Samuel Tardieu,
	Peter Maydell, Alistair Francis, Thomas Huth, Laurent Vivier,
	Marc-André Lureau

Hi Inès,

On 26/1/24 20:31, Inès Varhol wrote:
> This device implements the IM120417002 colors shield v1.1 for Arduino
> (which relies on the DM163 8x3-channel led driving logic) and features
> a simple display of an 8x8 RGB matrix.
> 
> This color shield can be plugged on the Arduino board (or the
> B-L475E-IOT01A board) to drive an 8x8 RGB led matrix.

Nice. Do you have an example? Or better, a test :)

> This RGB led matrix takes advantage of retinal persistance to
> seemingly display different colors in each row.
> 
> It'd be convenient to set the QEMU console's refresh rate
> in order to ensure that the delay before turning off rows
> (2 frames currently) isn't too short. However
> `dpy_ui_info_supported(s->console)` can't be used.

Cc'ing Marc-André Lureau.

> I saw that Kconfig configurable components aren't visible in C files,
> does that mean it's impossible to make the DM163 device optional when
> using the B-L475E-IOT01A board?

You could (but shouldn't) use for build-time:

   #include CONFIG_DEVICES

But better at run-time using [module_]object_class_by_name().

> Based-on: 20240123122505.516393-1-ines.varhol@telecom-paris.fr
> ([PATCH v3 0/3] Add device STM32L4x5 GPIO)
> 
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> 
> Inès Varhol (3):
>    hw/display : Add device DM163
>    hw/arm : Connect DM163 to STM32L4x5
>    tests/qtest : Add testcase for DM163



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] hw/arm : Connect DM163 to STM32L4x5
  2024-02-05 13:46   ` Philippe Mathieu-Daudé
@ 2024-02-07 17:25     ` Inès Varhol
  0 siblings, 0 replies; 11+ messages in thread
From: Inès Varhol @ 2024-02-07 17:25 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Arnaud Minier, Paolo Bonzini, qemu-arm,
	Samuel Tardieu, peter maydell, Alistair Francis, Thomas Huth,
	Laurent Vivier

Hello !

> De: "Philippe Mathieu-Daudé" <philmd@linaro.org>
> Envoyé: Lundi 5 Février 2024 14:46:58
>
> Hi Inès,
> 
> On 26/1/24 20:31, Inès Varhol wrote:
> > Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> > Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> > ---
> >   hw/arm/Kconfig                 |  1 +
> >   hw/arm/stm32l4x5_soc.c         | 55 +++++++++++++++++++++++++++++++++-
> >   include/hw/arm/stm32l4x5_soc.h |  3 ++
> >   3 files changed, 58 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> > index 3e49b913f8..818aa2f1a2 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -463,6 +463,7 @@ config STM32L4X5_SOC
> >       select STM32L4X5_SYSCFG
> >       select STM32L4X5_RCC
> >       select STM32L4X5_GPIO
> > +    select DM163
> 
> 
> > +/*
> > + * There are actually 14 input pins in the DM163 device.
> > + * Here the DM163 input pin EN isn't connected to the STM32L4x5
> > + * GPIOs as the IM120417002 colors shield doesn't actually use
> > + * this pin to drive the RGB matrix.
> > + */
> > +#define NUM_DM163_INPUTS 13
> > +
> > +static const int dm163_input[NUM_DM163_INPUTS] = {
> > +    1 * 16 + 2,  /* ROW0  PB2       */
> > +    0 * 16 + 15, /* ROW1  PA15      */
> > +    0 * 16 + 2,  /* ROW2  PA2       */
> > +    0 * 16 + 7,  /* ROW3  PA7       */
> > +    0 * 16 + 6,  /* ROW4  PA6       */
> > +    0 * 16 + 5,  /* ROW5  PA5       */
> > +    1 * 16 + 0,  /* ROW6  PB0       */
> > +    0 * 16 + 3,  /* ROW7  PA3       */
> > +    0 * 16 + 4,  /* SIN (SDA) PA4   */
> > +    1 * 16 + 1,  /* DCK (SCK) PB1   */
> > +    2 * 16 + 3,  /* RST_B (RST) PC3 */
> > +    2 * 16 + 4,  /* LAT_B (LAT) PC4 */
> > +    2 * 16 + 5,  /* SELBK (SB)  PC5 */
> > +};
> > +
> > +
> >   static const uint32_t gpio_addr[] = {
> >       0x48000000,
> >       0x48000400,
> > @@ -116,6 +143,8 @@ static void stm32l4x5_soc_initfn(Object *obj)
> >           g_autofree char *name = g_strdup_printf("gpio%c", 'a' + i);
> >           object_initialize_child(obj, name, &s->gpio[i], TYPE_STM32L4X5_GPIO);
> >       }
> > +
> > +    object_initialize_child(obj, "dm163", &s->dm163, TYPE_DM163);
> 
> The DM163 is another chip, not a component part of the SoC;
> it belongs to the machine and should be created/wired in
> b_l475e_iot01a_init(). Similarly to the IRQ splitters.
> 
> Keeping board component states in a Bl475eMachineState structure
> could help organizing your model. You can find an example on how
> extend the MachineState in hw/avr/arduino.c.
> 

Yes thank you ! that's done :)

> You might call qdev_pass_gpios() to exposes the SysCfg lines out
> of the SoC.

I was wondering what's the reason to expose Syscfg lines and not Gpio lines?
(Should GPIOs also be moved to the machine ?)

Best regards,

Ines


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] Add device DM163 (led driver, matrix colors shield & display)
  2024-02-05 14:03 ` [PATCH 0/3] Add device DM163 (led driver, matrix colors shield & display) Philippe Mathieu-Daudé
@ 2024-02-07 19:27   ` Inès Varhol
  0 siblings, 0 replies; 11+ messages in thread
From: Inès Varhol @ 2024-02-07 19:27 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Arnaud Minier, Paolo Bonzini, qemu-arm,
	Samuel Tardieu, peter maydell, Alistair Francis, Thomas Huth,
	Laurent Vivier, Marc-André Lureau

Hello,

> De: "Philippe Mathieu-Daudé" <philmd@linaro.org>
> Envoyé: Lundi 5 Février 2024 15:03:59
> 
> Hi Inès,
> 
> On 26/1/24 20:31, Inès Varhol wrote:
> > This device implements the IM120417002 colors shield v1.1 for Arduino
> > (which relies on the DM163 8x3-channel led driving logic) and features
> > a simple display of an 8x8 RGB matrix.
> > 
> > This color shield can be plugged on the Arduino board (or the
> > B-L475E-IOT01A board) to drive an 8x8 RGB led matrix.
> 
> Nice. Do you have an example? Or better, a test :)
> 

Actually I don't know how to test the display with QTest :/
(I've tested it by running custom executables)

I've seen that `qtest_init_internal` sets `-display none`
so I imagine there's no way to test the display visually.

It seems to me that I can't use a qdev property (to access
the DM163 buffer and check its content) either since there's
no `visit_type_*` for arrays.

I could technically access all the elements in the array 
(returning a different element each time in the getter for
example), but that seems sketchy.


In short, how can I provide a test or an example?

Best regards,

Ines


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-02-07 19:28 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-26 19:31 [PATCH 0/3] Add device DM163 (led driver, matrix colors shield & display) Inès Varhol
2024-01-26 19:31 ` [PATCH 1/3] hw/display : Add device DM163 Inès Varhol
2024-02-05  0:09   ` Alistair Francis
2024-01-26 19:31 ` [PATCH 2/3] hw/arm : Connect DM163 to STM32L4x5 Inès Varhol
2024-02-05  0:14   ` Alistair Francis
2024-02-05 13:46   ` Philippe Mathieu-Daudé
2024-02-07 17:25     ` Inès Varhol
2024-01-26 19:31 ` [PATCH 3/3] tests/qtest : Add testcase for DM163 Inès Varhol
2024-02-05 13:52   ` Philippe Mathieu-Daudé
2024-02-05 14:03 ` [PATCH 0/3] Add device DM163 (led driver, matrix colors shield & display) Philippe Mathieu-Daudé
2024-02-07 19:27   ` Inès Varhol

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).