* [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
* 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
* [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
* 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 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
* [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 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 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