* [PATCH v4 1/5] hw/display : Add device DM163
2024-04-14 13:05 [PATCH v4 0/5] Add device DM163 (led driver, matrix colors shield & display) Inès Varhol
@ 2024-04-14 13:05 ` Inès Varhol
2024-04-15 10:00 ` Philippe Mathieu-Daudé
2024-04-14 13:05 ` [PATCH v4 2/5] hw/arm : Pass STM32L4x5 SYSCFG gpios to STM32L4x5 SoC Inès Varhol
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Inès Varhol @ 2024-04-14 13:05 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé, Paolo Bonzini, Thomas Huth,
Inès Varhol, Marc-André Lureau, qemu-arm,
Laurent Vivier, Samuel Tardieu, Arnaud Minier, Peter Maydell,
Alistair Francis
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.
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
docs/system/arm/b-l475e-iot01a.rst | 3 +-
include/hw/display/dm163.h | 58 +++++
hw/display/dm163.c | 333 +++++++++++++++++++++++++++++
hw/display/Kconfig | 3 +
hw/display/meson.build | 1 +
hw/display/trace-events | 14 ++
6 files changed, 411 insertions(+), 1 deletion(-)
create mode 100644 include/hw/display/dm163.h
create mode 100644 hw/display/dm163.c
diff --git a/docs/system/arm/b-l475e-iot01a.rst b/docs/system/arm/b-l475e-iot01a.rst
index 0afef8e4f4..91de5e82fc 100644
--- a/docs/system/arm/b-l475e-iot01a.rst
+++ b/docs/system/arm/b-l475e-iot01a.rst
@@ -12,13 +12,14 @@ USART, I2C, SPI, CAN and USB OTG, as well as a variety of sensors.
Supported devices
"""""""""""""""""
-Currently B-L475E-IOT01A machine's only supports the following devices:
+Currently B-L475E-IOT01A machines support the following devices:
- Cortex-M4F based STM32L4x5 SoC
- STM32L4x5 EXTI (Extended interrupts and events controller)
- STM32L4x5 SYSCFG (System configuration controller)
- STM32L4x5 RCC (Reset and clock control)
- STM32L4x5 GPIOs (General-purpose I/Os)
+- optional 8x8 led display (based on DM163 driver)
Missing devices
"""""""""""""""
diff --git a/include/hw/display/dm163.h b/include/hw/display/dm163.h
new file mode 100644
index 0000000000..00d0504640
--- /dev/null
+++ b/include/hw/display/dm163.h
@@ -0,0 +1,58 @@
+/*
+ * 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 sin;
+ uint8_t dck;
+ uint8_t rst_b;
+ uint8_t lat_b;
+ uint8_t selbk;
+ uint8_t en_b;
+
+ /* IM120417002 colors shield */
+ uint8_t activated_rows;
+
+ /* 8x8 RGB matrix */
+ QemuConsole *console;
+ uint8_t redraw;
+ /* 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 persistence of rows */
+ uint8_t age_of_row[RGB_MATRIX_NUM_ROWS];
+} DM163State;
+
+#endif /* HW_DISPLAY_DM163_H */
diff --git a/hw/display/dm163.c b/hw/display/dm163.c
new file mode 100644
index 0000000000..d4e89dd344
--- /dev/null
+++ b/hw/display/dm163.c
@@ -0,0 +1,333 @@
+/*
+ * 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_PERSISTENCE 3
+#define TURNED_OFF_ROW COLOR_BUFFER_SIZE
+
+static const VMStateDescription vmstate_dm163 = {
+ .name = TYPE_DM163,
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (const VMStateField[]) {
+ 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_UINT8(activated_rows, 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);
+
+ s->sin = 0;
+ s->dck = 0;
+ s->rst_b = 0;
+ /* Ensuring the first falling edge of lat_b isn't missed */
+ s->lat_b = 1;
+ s->selbk = 0;
+ s->en_b = 0;
+ /* Reset stops the PWM, not the shift and latched registers. */
+ memset(s->outputs, 0, sizeof(s->outputs));
+
+ s->activated_rows = 0;
+ s->redraw = 0;
+ trace_dm163_redraw(s->redraw);
+ for (unsigned i = 0; i < COLOR_BUFFER_SIZE + 1; i++) {
+ memset(s->buffer[i], 0, sizeof(s->buffer[0]));
+ }
+ s->last_buffer_idx = 0;
+ memset(s->buffer_idx_of_row, TURNED_OFF_ROW, sizeof(s->buffer_idx_of_row));
+ memset(s->age_of_row, 0, sizeof(s->age_of_row));
+}
+
+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 is high and enable is low. */
+ 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;
+ s->redraw |= (1 << row);
+ trace_dm163_redraw(s->redraw);
+ }
+ }
+}
+
+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->redraw |= (1 << line);
+ trace_dm163_redraw(s->redraw);
+ } else {
+ s->activated_rows &= ~(1 << line);
+ s->age_of_row[line] = ROW_PERSISTENCE;
+ }
+ trace_dm163_activated_rows(s->activated_rows);
+}
+
+static void dm163_invalidate_display(void *opaque)
+{
+ DM163State *s = (DM163State *)opaque;
+ s->redraw = 0xFF;
+ trace_dm163_redraw(s->redraw);
+}
+
+static void update_age_of_row(DM163State *s, unsigned row)
+{
+ if (s->age_of_row[row]) {
+ s->age_of_row[row]--;
+ } else {
+ /*
+ * If the ROW_PERSISTENCE delay is up,
+ * the row is turned off.
+ */
+ s->buffer_idx_of_row[row] = TURNED_OFF_ROW;
+ s->redraw |= (1 << row);
+ trace_dm163_redraw(s->redraw);
+ }
+}
+
+static uint32_t *update_display_of_row(DM163State *s, uint32_t *dest,
+ unsigned row)
+{
+ for (unsigned _ = 0; _ < LED_SQUARE_SIZE; _++) {
+ for (int x = RGB_MATRIX_NUM_COLS * LED_SQUARE_SIZE - 1; x >= 0; x--) {
+ /* UI layer guarantees that there's 32 bits per pixel (Mar 2024) */
+ *dest++ = s->buffer[s->buffer_idx_of_row[row]][x / LED_SQUARE_SIZE];
+ }
+ }
+
+ dpy_gfx_update(s->console, 0, LED_SQUARE_SIZE * row,
+ RGB_MATRIX_NUM_COLS * LED_SQUARE_SIZE, LED_SQUARE_SIZE);
+ s->redraw &= ~(1 << row);
+ trace_dm163_redraw(s->redraw);
+
+ return dest;
+}
+
+static void dm163_update_display(void *opaque)
+{
+ DM163State *s = (DM163State *)opaque;
+ DisplaySurface *surface = qemu_console_surface(s->console);
+ uint32_t *dest;
+
+ dest = surface_data(surface);
+ for (unsigned row = 0; row < RGB_MATRIX_NUM_ROWS; row++) {
+ update_age_of_row(s, row);
+ if (!extract8(s->redraw, row, 1)) {
+ dest += LED_SQUARE_SIZE * LED_SQUARE_SIZE * RGB_MATRIX_NUM_COLS;
+ continue;
+ }
+ dest = update_display_of_row(s, dest, row);
+ }
+}
+
+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/Kconfig b/hw/display/Kconfig
index 234c7de027..a4552c8ed7 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -140,3 +140,6 @@ config XLNX_DISPLAYPORT
bool
# defaults to "N", enabled by specific boards
depends on PIXMAN
+
+config DM163
+ bool
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..fc7cbdcd36 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -177,3 +177,17 @@ 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_redraw(uint8_t redraw) "0x%02x"
+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"
--
2.43.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 1/5] hw/display : Add device DM163
2024-04-14 13:05 ` [PATCH v4 1/5] hw/display : Add device DM163 Inès Varhol
@ 2024-04-15 10:00 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-15 10:00 UTC (permalink / raw)
To: Inès Varhol, qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Marc-André Lureau, qemu-arm,
Laurent Vivier, Samuel Tardieu, Arnaud Minier, Peter Maydell,
Alistair Francis
Hi Inès,
On 14/4/24 15:05, 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. The columns of the matrix are
> driven by the DM163 and the rows are driven externally.
>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
> docs/system/arm/b-l475e-iot01a.rst | 3 +-
> include/hw/display/dm163.h | 58 +++++
> hw/display/dm163.c | 333 +++++++++++++++++++++++++++++
> hw/display/Kconfig | 3 +
> hw/display/meson.build | 1 +
> hw/display/trace-events | 14 ++
> 6 files changed, 411 insertions(+), 1 deletion(-)
> create mode 100644 include/hw/display/dm163.h
> create mode 100644 hw/display/dm163.c
> diff --git a/include/hw/display/dm163.h b/include/hw/display/dm163.h
> new file mode 100644
> index 0000000000..00d0504640
> --- /dev/null
> +++ b/include/hw/display/dm163.h
> @@ -0,0 +1,58 @@
> +/*
> + * 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)
Maybe better as:
#define DM163_NUM_LEDS (RGB_MATRIX_NUM_COLS * RGB_MATRIX_NUM_ROWS)
> +#define COLOR_BUFFER_SIZE RGB_MATRIX_NUM_ROWS
It could ease the code to define here directly as:
/* The last row is filled with 0 (turned off row) */
#define COLOR_BUFFER_SIZE (RGB_MATRIX_NUM_ROWS + 1)
> +
> +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 sin;
> + uint8_t dck;
> + uint8_t rst_b;
> + uint8_t lat_b;
> + uint8_t selbk;
> + uint8_t en_b;
> +
> + /* IM120417002 colors shield */
> + uint8_t activated_rows;
> +
> + /* 8x8 RGB matrix */
> + QemuConsole *console;
> + uint8_t redraw;
> + /* 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 persistence of rows */
> + uint8_t age_of_row[RGB_MATRIX_NUM_ROWS];
Maybe "row_persistence_delay"?
> +} DM163State;
> +
> +#endif /* HW_DISPLAY_DM163_H */
> +static void dm163_dck_gpio_handler(void *opaque, int line, int new_state)
> +{
> + DM163State *s = DM163(opaque);
GPIO handlers are initialized in dm163_realize() where we know @dev
is already a DM163State:
static void dm163_realize(DeviceState *dev, Error **errp)
{
DM163State *s = DM163(dev);
^^^^^
qdev_init_gpio_in(dev, dm163_rows_gpio_handler, 8);
So here (and other handlers) you can avoid the QOM cast macro,
and directly use:
DM163State *s = 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_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)
No need to force the compiler to inline these methods.
> +{
> + /*
> + * 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_realize(DeviceState *dev, Error **errp)
> +{
> + DM163State *s = DM163(dev);
> +
> + qdev_init_gpio_in(dev, dm163_rows_gpio_handler, 8);
s/8/RGB_MATRIX_NUM_ROWS/
> + 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);
> +}
> diff --git a/hw/display/trace-events b/hw/display/trace-events
> index 2336a0ca15..fc7cbdcd36 100644
> --- a/hw/display/trace-events
> +++ b/hw/display/trace-events
> +# dm163.c
> +dm163_redraw(uint8_t redraw) "0x%02x"
> +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"
unsigned new_state, "%u"
> +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"
Minor comments, otherwise LGTM!
Regards,
Phil.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 2/5] hw/arm : Pass STM32L4x5 SYSCFG gpios to STM32L4x5 SoC
2024-04-14 13:05 [PATCH v4 0/5] Add device DM163 (led driver, matrix colors shield & display) Inès Varhol
2024-04-14 13:05 ` [PATCH v4 1/5] hw/display : Add device DM163 Inès Varhol
@ 2024-04-14 13:05 ` Inès Varhol
2024-04-15 9:29 ` Philippe Mathieu-Daudé
2024-04-14 13:05 ` [PATCH v4 3/5] hw/arm : Create Bl475eMachineState Inès Varhol
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Inès Varhol @ 2024-04-14 13:05 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé, Paolo Bonzini, Thomas Huth,
Inès Varhol, Marc-André Lureau, qemu-arm,
Laurent Vivier, Samuel Tardieu, Arnaud Minier, Peter Maydell
Exposing SYSCFG inputs to the SoC is practical in order to wire the SoC
to the optional DM163 display from the board code (GPIOs outputs need
to be connected to both SYSCFG inputs and DM163 inputs).
STM32L4x5 SYSCFG in-irq interception needed to be changed accordingly.
Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
hw/arm/stm32l4x5_soc.c | 6 ++++--
tests/qtest/stm32l4x5_gpio-test.c | 12 +++++++-----
tests/qtest/stm32l4x5_syscfg-test.c | 16 +++++++++-------
3 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
index 40e294f838..c4b45e6956 100644
--- a/hw/arm/stm32l4x5_soc.c
+++ b/hw/arm/stm32l4x5_soc.c
@@ -1,8 +1,8 @@
/*
* STM32L4x5 SoC family
*
- * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
- * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
+ * 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
*
@@ -221,6 +221,8 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
}
}
+ qdev_pass_gpios(DEVICE(&s->syscfg), dev_soc, NULL);
+
/* EXTI device */
busdev = SYS_BUS_DEVICE(&s->exti);
if (!sysbus_realize(busdev, errp)) {
diff --git a/tests/qtest/stm32l4x5_gpio-test.c b/tests/qtest/stm32l4x5_gpio-test.c
index 0f6bda54d3..495a6fc413 100644
--- a/tests/qtest/stm32l4x5_gpio-test.c
+++ b/tests/qtest/stm32l4x5_gpio-test.c
@@ -43,6 +43,8 @@
#define OTYPER_PUSH_PULL 0
#define OTYPER_OPEN_DRAIN 1
+#define SYSCFG "/machine/soc"
+
const uint32_t moder_reset[NUM_GPIOS] = {
0xABFFFFFF,
0xFFFFFEBF,
@@ -284,7 +286,7 @@ static void test_gpio_output_mode(const void *data)
uint32_t gpio = test_gpio_addr(data);
unsigned int gpio_id = get_gpio_id(gpio);
- qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+ qtest_irq_intercept_in(global_qtest, SYSCFG);
/* Set a bit in ODR and check nothing happens */
gpio_set_bit(gpio, ODR, pin, 1);
@@ -319,7 +321,7 @@ static void test_gpio_input_mode(const void *data)
uint32_t gpio = test_gpio_addr(data);
unsigned int gpio_id = get_gpio_id(gpio);
- qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+ qtest_irq_intercept_in(global_qtest, SYSCFG);
/* Configure a line as input, raise it, and check that the pin is high */
gpio_set_2bits(gpio, MODER, pin, MODER_INPUT);
@@ -348,7 +350,7 @@ static void test_pull_up_pull_down(const void *data)
uint32_t gpio = test_gpio_addr(data);
unsigned int gpio_id = get_gpio_id(gpio);
- qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+ qtest_irq_intercept_in(global_qtest, SYSCFG);
/* Configure a line as input with pull-up, check the line is set high */
gpio_set_2bits(gpio, MODER, pin, MODER_INPUT);
@@ -378,7 +380,7 @@ static void test_push_pull(const void *data)
uint32_t gpio = test_gpio_addr(data);
uint32_t gpio2 = GPIO_BASE_ADDR + (GPIO_H - gpio);
- qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+ qtest_irq_intercept_in(global_qtest, SYSCFG);
/* Setting a line high externally, configuring it in push-pull output */
/* And checking the pin was disconnected */
@@ -425,7 +427,7 @@ static void test_open_drain(const void *data)
uint32_t gpio = test_gpio_addr(data);
uint32_t gpio2 = GPIO_BASE_ADDR + (GPIO_H - gpio);
- qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+ qtest_irq_intercept_in(global_qtest, SYSCFG);
/* Setting a line high externally, configuring it in open-drain output */
/* And checking the pin was disconnected */
diff --git a/tests/qtest/stm32l4x5_syscfg-test.c b/tests/qtest/stm32l4x5_syscfg-test.c
index ed4801798d..eed9d5940b 100644
--- a/tests/qtest/stm32l4x5_syscfg-test.c
+++ b/tests/qtest/stm32l4x5_syscfg-test.c
@@ -1,8 +1,8 @@
/*
* QTest testcase for STM32L4x5_SYSCFG
*
- * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
- * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
+ * Copyright (c) 2024 Arnaud Minier <arnaud.minier@telecom-paris.fr>
+ * Copyright (c) 2024 Inès Varhol <ines.varhol@telecom-paris.fr>
*
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
@@ -25,6 +25,9 @@
#define SYSCFG_SWPR2 0x28
#define INVALID_ADDR 0x2C
+#define EXTI "/machine/soc/exti"
+#define SYSCFG "/machine/soc"
+
static void syscfg_writel(unsigned int offset, uint32_t value)
{
writel(SYSCFG_BASE_ADDR + offset, value);
@@ -37,8 +40,7 @@ static uint32_t syscfg_readl(unsigned int offset)
static void syscfg_set_irq(int num, int level)
{
- qtest_set_irq_in(global_qtest, "/machine/soc/syscfg",
- NULL, num, level);
+ qtest_set_irq_in(global_qtest, SYSCFG, NULL, num, level);
}
static void system_reset(void)
@@ -197,7 +199,7 @@ static void test_interrupt(void)
* Test that GPIO rising lines result in an irq
* with the right configuration
*/
- qtest_irq_intercept_in(global_qtest, "/machine/soc/exti");
+ qtest_irq_intercept_in(global_qtest, EXTI);
/* GPIOA is the default source for EXTI lines 0 to 15 */
@@ -230,7 +232,7 @@ static void test_irq_pin_multiplexer(void)
* Test that syscfg irq sets the right exti irq
*/
- qtest_irq_intercept_in(global_qtest, "/machine/soc/exti");
+ qtest_irq_intercept_in(global_qtest, EXTI);
syscfg_set_irq(0, 1);
@@ -257,7 +259,7 @@ static void test_irq_gpio_multiplexer(void)
* Test that an irq is generated only by the right GPIO
*/
- qtest_irq_intercept_in(global_qtest, "/machine/soc/exti");
+ qtest_irq_intercept_in(global_qtest, EXTI);
/* GPIOA is the default source for EXTI lines 0 to 15 */
--
2.43.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/5] hw/arm : Pass STM32L4x5 SYSCFG gpios to STM32L4x5 SoC
2024-04-14 13:05 ` [PATCH v4 2/5] hw/arm : Pass STM32L4x5 SYSCFG gpios to STM32L4x5 SoC Inès Varhol
@ 2024-04-15 9:29 ` Philippe Mathieu-Daudé
2024-04-15 9:35 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-15 9:29 UTC (permalink / raw)
To: Inès Varhol, qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Marc-André Lureau, qemu-arm,
Laurent Vivier, Samuel Tardieu, Arnaud Minier, Peter Maydell
Hi Inès,
On 14/4/24 15:05, Inès Varhol wrote:
> Exposing SYSCFG inputs to the SoC is practical in order to wire the SoC
> to the optional DM163 display from the board code (GPIOs outputs need
> to be connected to both SYSCFG inputs and DM163 inputs).
>
> STM32L4x5 SYSCFG in-irq interception needed to be changed accordingly.
>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
> hw/arm/stm32l4x5_soc.c | 6 ++++--
> tests/qtest/stm32l4x5_gpio-test.c | 12 +++++++-----
> tests/qtest/stm32l4x5_syscfg-test.c | 16 +++++++++-------
> 3 files changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
> index 40e294f838..c4b45e6956 100644
> --- a/hw/arm/stm32l4x5_soc.c
> +++ b/hw/arm/stm32l4x5_soc.c
> @@ -1,8 +1,8 @@
> /*
> * STM32L4x5 SoC family
> *
> - * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
> - * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
> + * Copyright (c) 2024 Arnaud Minier <arnaud.minier@telecom-paris.fr>
> + * Copyright (c) 2024 Inès Varhol <ines.varhol@telecom-paris.fr>
You can keep 2023-2024.
> *
> * SPDX-License-Identifier: GPL-2.0-or-later
> *
> @@ -221,6 +221,8 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
> }
> }
>
> + qdev_pass_gpios(DEVICE(&s->syscfg), dev_soc, NULL);
> +
> /* EXTI device */
> busdev = SYS_BUS_DEVICE(&s->exti);
> if (!sysbus_realize(busdev, errp)) {
> diff --git a/tests/qtest/stm32l4x5_gpio-test.c b/tests/qtest/stm32l4x5_gpio-test.c
> index 0f6bda54d3..495a6fc413 100644
> --- a/tests/qtest/stm32l4x5_gpio-test.c
> +++ b/tests/qtest/stm32l4x5_gpio-test.c
> @@ -43,6 +43,8 @@
> #define OTYPER_PUSH_PULL 0
> #define OTYPER_OPEN_DRAIN 1
>
> +#define SYSCFG "/machine/soc"
Can we have a comment such /* SoC forwards GPIOs to SysCfg */?
(Similar comments for stm32l4x5_syscfg-test.c).
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/5] hw/arm : Pass STM32L4x5 SYSCFG gpios to STM32L4x5 SoC
2024-04-15 9:29 ` Philippe Mathieu-Daudé
@ 2024-04-15 9:35 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-15 9:35 UTC (permalink / raw)
To: Inès Varhol, qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Marc-André Lureau, qemu-arm,
Laurent Vivier, Samuel Tardieu, Arnaud Minier, Peter Maydell
On 15/4/24 11:29, Philippe Mathieu-Daudé wrote:
> Hi Inès,
>
> On 14/4/24 15:05, Inès Varhol wrote:
>> Exposing SYSCFG inputs to the SoC is practical in order to wire the SoC
>> to the optional DM163 display from the board code (GPIOs outputs need
>> to be connected to both SYSCFG inputs and DM163 inputs).
>>
>> STM32L4x5 SYSCFG in-irq interception needed to be changed accordingly.
>>
>> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
>> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
>> ---
>> hw/arm/stm32l4x5_soc.c | 6 ++++--
>> tests/qtest/stm32l4x5_gpio-test.c | 12 +++++++-----
>> tests/qtest/stm32l4x5_syscfg-test.c | 16 +++++++++-------
>> 3 files changed, 20 insertions(+), 14 deletions(-)
>>
>> diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
>> index 40e294f838..c4b45e6956 100644
>> --- a/hw/arm/stm32l4x5_soc.c
>> +++ b/hw/arm/stm32l4x5_soc.c
>> @@ -1,8 +1,8 @@
>> /*
>> * STM32L4x5 SoC family
>> *
>> - * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
>> - * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
>> + * Copyright (c) 2024 Arnaud Minier <arnaud.minier@telecom-paris.fr>
>> + * Copyright (c) 2024 Inès Varhol <ines.varhol@telecom-paris.fr>
>
> You can keep 2023-2024.
>
>> *
>> * SPDX-License-Identifier: GPL-2.0-or-later
>> *
>> @@ -221,6 +221,8 @@ static void stm32l4x5_soc_realize(DeviceState
>> *dev_soc, Error **errp)
>> }
>> }
>> + qdev_pass_gpios(DEVICE(&s->syscfg), dev_soc, NULL);
>> +
>> /* EXTI device */
>> busdev = SYS_BUS_DEVICE(&s->exti);
>> if (!sysbus_realize(busdev, errp)) {
>> diff --git a/tests/qtest/stm32l4x5_gpio-test.c
>> b/tests/qtest/stm32l4x5_gpio-test.c
>> index 0f6bda54d3..495a6fc413 100644
>> --- a/tests/qtest/stm32l4x5_gpio-test.c
>> +++ b/tests/qtest/stm32l4x5_gpio-test.c
>> @@ -43,6 +43,8 @@
>> #define OTYPER_PUSH_PULL 0
>> #define OTYPER_OPEN_DRAIN 1
>> +#define SYSCFG "/machine/soc"
>
> Can we have a comment such /* SoC forwards GPIOs to SysCfg */?
>
> (Similar comments for stm32l4x5_syscfg-test.c).
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 3/5] hw/arm : Create Bl475eMachineState
2024-04-14 13:05 [PATCH v4 0/5] Add device DM163 (led driver, matrix colors shield & display) Inès Varhol
2024-04-14 13:05 ` [PATCH v4 1/5] hw/display : Add device DM163 Inès Varhol
2024-04-14 13:05 ` [PATCH v4 2/5] hw/arm : Pass STM32L4x5 SYSCFG gpios to STM32L4x5 SoC Inès Varhol
@ 2024-04-14 13:05 ` Inès Varhol
2024-04-15 9:30 ` Philippe Mathieu-Daudé
2024-04-14 13:05 ` [PATCH v4 4/5] hw/arm : Connect DM163 to B-L475E-IOT01A Inès Varhol
2024-04-14 13:05 ` [PATCH v4 5/5] tests/qtest : Add testcase for DM163 Inès Varhol
4 siblings, 1 reply; 13+ messages in thread
From: Inès Varhol @ 2024-04-14 13:05 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé, Paolo Bonzini, Thomas Huth,
Inès Varhol, Marc-André Lureau, qemu-arm,
Laurent Vivier, Samuel Tardieu, Arnaud Minier, Peter Maydell
Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
hw/arm/b-l475e-iot01a.c | 44 +++++++++++++++++++++++++++++------------
1 file changed, 31 insertions(+), 13 deletions(-)
diff --git a/hw/arm/b-l475e-iot01a.c b/hw/arm/b-l475e-iot01a.c
index d862aa43fc..2b570b3e09 100644
--- a/hw/arm/b-l475e-iot01a.c
+++ b/hw/arm/b-l475e-iot01a.c
@@ -2,8 +2,8 @@
* B-L475E-IOT01A Discovery Kit machine
* (B-L475E-IOT01A IoT Node)
*
- * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
- * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
+ * 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
*
@@ -32,33 +32,51 @@
/* B-L475E-IOT01A implementation is derived from netduinoplus2 */
-static void b_l475e_iot01a_init(MachineState *machine)
+#define TYPE_B_L475E_IOT01A MACHINE_TYPE_NAME("b-l475e-iot01a")
+OBJECT_DECLARE_SIMPLE_TYPE(Bl475eMachineState, B_L475E_IOT01A)
+
+typedef struct Bl475eMachineState {
+ MachineState parent_obj;
+
+ Stm32l4x5SocState soc;
+} Bl475eMachineState;
+
+static void bl475e_init(MachineState *machine)
{
+ Bl475eMachineState *s = B_L475E_IOT01A(machine);
const Stm32l4x5SocClass *sc;
- DeviceState *dev;
- dev = qdev_new(TYPE_STM32L4X5XG_SOC);
- object_property_add_child(OBJECT(machine), "soc", OBJECT(dev));
- sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+ object_initialize_child(OBJECT(machine), "soc", &s->soc,
+ TYPE_STM32L4X5XG_SOC);
+ sysbus_realize(SYS_BUS_DEVICE(&s->soc), &error_fatal);
- sc = STM32L4X5_SOC_GET_CLASS(dev);
+ sc = STM32L4X5_SOC_GET_CLASS(&s->soc);
armv7m_load_kernel(ARM_CPU(first_cpu),
- machine->kernel_filename,
- 0, sc->flash_size);
+ machine->kernel_filename, 0, sc->flash_size);
}
-static void b_l475e_iot01a_machine_init(MachineClass *mc)
+static void bl475e_machine_init(ObjectClass *oc, void *data)
{
+ MachineClass *mc = MACHINE_CLASS(oc);
static const char *machine_valid_cpu_types[] = {
ARM_CPU_TYPE_NAME("cortex-m4"),
NULL
};
mc->desc = "B-L475E-IOT01A Discovery Kit (Cortex-M4)";
- mc->init = b_l475e_iot01a_init;
+ mc->init = bl475e_init;
mc->valid_cpu_types = machine_valid_cpu_types;
/* SRAM pre-allocated as part of the SoC instantiation */
mc->default_ram_size = 0;
}
-DEFINE_MACHINE("b-l475e-iot01a", b_l475e_iot01a_machine_init)
+static const TypeInfo bl475e_machine_type[] = {
+ {
+ .name = TYPE_B_L475E_IOT01A,
+ .parent = TYPE_MACHINE,
+ .instance_size = sizeof(Bl475eMachineState),
+ .class_init = bl475e_machine_init,
+ }
+};
+
+DEFINE_TYPES(bl475e_machine_type)
--
2.43.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 3/5] hw/arm : Create Bl475eMachineState
2024-04-14 13:05 ` [PATCH v4 3/5] hw/arm : Create Bl475eMachineState Inès Varhol
@ 2024-04-15 9:30 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-15 9:30 UTC (permalink / raw)
To: Inès Varhol, qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Marc-André Lureau, qemu-arm,
Laurent Vivier, Samuel Tardieu, Arnaud Minier, Peter Maydell
On 14/4/24 15:05, 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/b-l475e-iot01a.c | 44 +++++++++++++++++++++++++++++------------
> 1 file changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/hw/arm/b-l475e-iot01a.c b/hw/arm/b-l475e-iot01a.c
> index d862aa43fc..2b570b3e09 100644
> --- a/hw/arm/b-l475e-iot01a.c
> +++ b/hw/arm/b-l475e-iot01a.c
> @@ -2,8 +2,8 @@
> * B-L475E-IOT01A Discovery Kit machine
> * (B-L475E-IOT01A IoT Node)
> *
> - * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
> - * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
> + * Copyright (c) 2024 Arnaud Minier <arnaud.minier@telecom-paris.fr>
> + * Copyright (c) 2024 Inès Varhol <ines.varhol@telecom-paris.fr>
2023-2024
> *
> * SPDX-License-Identifier: GPL-2.0-or-later
> *
> @@ -32,33 +32,51 @@
> +static void bl475e_init(MachineState *machine)
> {
> + Bl475eMachineState *s = B_L475E_IOT01A(machine);
> const Stm32l4x5SocClass *sc;
> - DeviceState *dev;
>
> - dev = qdev_new(TYPE_STM32L4X5XG_SOC);
> - object_property_add_child(OBJECT(machine), "soc", OBJECT(dev));
> - sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> + object_initialize_child(OBJECT(machine), "soc", &s->soc,
> + TYPE_STM32L4X5XG_SOC);
> + sysbus_realize(SYS_BUS_DEVICE(&s->soc), &error_fatal);
>
> - sc = STM32L4X5_SOC_GET_CLASS(dev);
> + sc = STM32L4X5_SOC_GET_CLASS(&s->soc);
> armv7m_load_kernel(ARM_CPU(first_cpu),
> - machine->kernel_filename,
> - 0, sc->flash_size);
> + machine->kernel_filename, 0, sc->flash_size);
Spurious line change.
> }
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 4/5] hw/arm : Connect DM163 to B-L475E-IOT01A
2024-04-14 13:05 [PATCH v4 0/5] Add device DM163 (led driver, matrix colors shield & display) Inès Varhol
` (2 preceding siblings ...)
2024-04-14 13:05 ` [PATCH v4 3/5] hw/arm : Create Bl475eMachineState Inès Varhol
@ 2024-04-14 13:05 ` Inès Varhol
2024-04-15 9:41 ` Philippe Mathieu-Daudé
2024-04-14 13:05 ` [PATCH v4 5/5] tests/qtest : Add testcase for DM163 Inès Varhol
4 siblings, 1 reply; 13+ messages in thread
From: Inès Varhol @ 2024-04-14 13:05 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé, Paolo Bonzini, Thomas Huth,
Inès Varhol, Marc-André Lureau, qemu-arm,
Laurent Vivier, Samuel Tardieu, Arnaud Minier, Peter Maydell
Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
hw/arm/b-l475e-iot01a.c | 59 +++++++++++++++++++++++++++++++++++++++--
hw/arm/Kconfig | 1 +
2 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/hw/arm/b-l475e-iot01a.c b/hw/arm/b-l475e-iot01a.c
index 2b570b3e09..6f0bf68ca6 100644
--- a/hw/arm/b-l475e-iot01a.c
+++ b/hw/arm/b-l475e-iot01a.c
@@ -27,10 +27,37 @@
#include "hw/boards.h"
#include "hw/qdev-properties.h"
#include "qemu/error-report.h"
-#include "hw/arm/stm32l4x5_soc.h"
#include "hw/arm/boot.h"
+#include "hw/core/split-irq.h"
+#include "hw/arm/stm32l4x5_soc.h"
+#include "hw/gpio/stm32l4x5_gpio.h"
+#include "hw/display/dm163.h"
+
+/* B-L475E-IOT01A implementation is inspired from netduinoplus2 and arduino */
-/* B-L475E-IOT01A implementation is derived from netduinoplus2 */
+/*
+ * 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 * GPIO_NUM_PINS + 2, /* ROW0 PB2 */
+ 0 * GPIO_NUM_PINS + 15, /* ROW1 PA15 */
+ 0 * GPIO_NUM_PINS + 2, /* ROW2 PA2 */
+ 0 * GPIO_NUM_PINS + 7, /* ROW3 PA7 */
+ 0 * GPIO_NUM_PINS + 6, /* ROW4 PA6 */
+ 0 * GPIO_NUM_PINS + 5, /* ROW5 PA5 */
+ 1 * GPIO_NUM_PINS + 0, /* ROW6 PB0 */
+ 0 * GPIO_NUM_PINS + 3, /* ROW7 PA3 */
+ 0 * GPIO_NUM_PINS + 4, /* SIN (SDA) PA4 */
+ 1 * GPIO_NUM_PINS + 1, /* DCK (SCK) PB1 */
+ 2 * GPIO_NUM_PINS + 3, /* RST_B (RST) PC3 */
+ 2 * GPIO_NUM_PINS + 4, /* LAT_B (LAT) PC4 */
+ 2 * GPIO_NUM_PINS + 5, /* SELBK (SB) PC5 */
+};
#define TYPE_B_L475E_IOT01A MACHINE_TYPE_NAME("b-l475e-iot01a")
OBJECT_DECLARE_SIMPLE_TYPE(Bl475eMachineState, B_L475E_IOT01A)
@@ -39,12 +66,16 @@ typedef struct Bl475eMachineState {
MachineState parent_obj;
Stm32l4x5SocState soc;
+ SplitIRQ gpio_splitters[NUM_DM163_INPUTS];
+ DM163State dm163;
} Bl475eMachineState;
static void bl475e_init(MachineState *machine)
{
Bl475eMachineState *s = B_L475E_IOT01A(machine);
const Stm32l4x5SocClass *sc;
+ DeviceState *dev, *gpio_out_splitter;
+ int gpio, pin;
object_initialize_child(OBJECT(machine), "soc", &s->soc,
TYPE_STM32L4X5XG_SOC);
@@ -53,6 +84,30 @@ static void bl475e_init(MachineState *machine)
sc = STM32L4X5_SOC_GET_CLASS(&s->soc);
armv7m_load_kernel(ARM_CPU(first_cpu),
machine->kernel_filename, 0, sc->flash_size);
+
+ if (object_class_by_name("dm163")) {
+ object_initialize_child(OBJECT(machine), "dm163",
+ &s->dm163, TYPE_DM163);
+ dev = DEVICE(&s->dm163);
+ qdev_realize(dev, NULL, &error_abort);
+
+ for (unsigned i = 0; i < NUM_DM163_INPUTS; i++) {
+ object_initialize_child(OBJECT(machine), "gpio-out-splitters[*]",
+ &s->gpio_splitters[i], TYPE_SPLIT_IRQ);
+ gpio_out_splitter = DEVICE(&s->gpio_splitters[i]);
+ qdev_prop_set_uint32(gpio_out_splitter, "num-lines", 2);
+ qdev_realize(gpio_out_splitter, NULL, &error_fatal);
+
+ qdev_connect_gpio_out(gpio_out_splitter, 0,
+ qdev_get_gpio_in(DEVICE(&s->soc), dm163_input[i]));
+ qdev_connect_gpio_out(gpio_out_splitter, 1,
+ qdev_get_gpio_in(dev, i));
+ gpio = dm163_input[i] / GPIO_NUM_PINS;
+ pin = dm163_input[i] % GPIO_NUM_PINS;
+ qdev_connect_gpio_out(DEVICE(&s->soc.gpio[gpio]), pin,
+ qdev_get_gpio_in(DEVICE(gpio_out_splitter), 0));
+ }
+ }
}
static void bl475e_machine_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 893a7bff66..8431384402 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -468,6 +468,7 @@ config B_L475E_IOT01A
default y
depends on TCG && ARM
select STM32L4X5_SOC
+ imply DM163
config STM32L4X5_SOC
bool
--
2.43.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 4/5] hw/arm : Connect DM163 to B-L475E-IOT01A
2024-04-14 13:05 ` [PATCH v4 4/5] hw/arm : Connect DM163 to B-L475E-IOT01A Inès Varhol
@ 2024-04-15 9:41 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-15 9:41 UTC (permalink / raw)
To: Inès Varhol, qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Marc-André Lureau, qemu-arm,
Laurent Vivier, Samuel Tardieu, Arnaud Minier, Peter Maydell
On 14/4/24 15:05, 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/b-l475e-iot01a.c | 59 +++++++++++++++++++++++++++++++++++++++--
> hw/arm/Kconfig | 1 +
> 2 files changed, 58 insertions(+), 2 deletions(-)
> +/*
> + * 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] = {
s/int/unsigned/
> + 1 * GPIO_NUM_PINS + 2, /* ROW0 PB2 */
> + 0 * GPIO_NUM_PINS + 15, /* ROW1 PA15 */
> + 0 * GPIO_NUM_PINS + 2, /* ROW2 PA2 */
> + 0 * GPIO_NUM_PINS + 7, /* ROW3 PA7 */
> + 0 * GPIO_NUM_PINS + 6, /* ROW4 PA6 */
> + 0 * GPIO_NUM_PINS + 5, /* ROW5 PA5 */
> + 1 * GPIO_NUM_PINS + 0, /* ROW6 PB0 */
> + 0 * GPIO_NUM_PINS + 3, /* ROW7 PA3 */
> + 0 * GPIO_NUM_PINS + 4, /* SIN (SDA) PA4 */
> + 1 * GPIO_NUM_PINS + 1, /* DCK (SCK) PB1 */
> + 2 * GPIO_NUM_PINS + 3, /* RST_B (RST) PC3 */
> + 2 * GPIO_NUM_PINS + 4, /* LAT_B (LAT) PC4 */
> + 2 * GPIO_NUM_PINS + 5, /* SELBK (SB) PC5 */
> +};
>
> #define TYPE_B_L475E_IOT01A MACHINE_TYPE_NAME("b-l475e-iot01a")
> OBJECT_DECLARE_SIMPLE_TYPE(Bl475eMachineState, B_L475E_IOT01A)
> @@ -39,12 +66,16 @@ typedef struct Bl475eMachineState {
> MachineState parent_obj;
>
> Stm32l4x5SocState soc;
> + SplitIRQ gpio_splitters[NUM_DM163_INPUTS];
> + DM163State dm163;
> } Bl475eMachineState;
>
> static void bl475e_init(MachineState *machine)
> {
> Bl475eMachineState *s = B_L475E_IOT01A(machine);
> const Stm32l4x5SocClass *sc;
> + DeviceState *dev, *gpio_out_splitter;
> + int gpio, pin;
unsigned.
>
> object_initialize_child(OBJECT(machine), "soc", &s->soc,
> TYPE_STM32L4X5XG_SOC);
> @@ -53,6 +84,30 @@ static void bl475e_init(MachineState *machine)
> sc = STM32L4X5_SOC_GET_CLASS(&s->soc);
> armv7m_load_kernel(ARM_CPU(first_cpu),
> machine->kernel_filename, 0, sc->flash_size);
> +
> + if (object_class_by_name("dm163")) {
TYPE_DM163
> + object_initialize_child(OBJECT(machine), "dm163",
> + &s->dm163, TYPE_DM163);
> + dev = DEVICE(&s->dm163);
> + qdev_realize(dev, NULL, &error_abort);
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 5/5] tests/qtest : Add testcase for DM163
2024-04-14 13:05 [PATCH v4 0/5] Add device DM163 (led driver, matrix colors shield & display) Inès Varhol
` (3 preceding siblings ...)
2024-04-14 13:05 ` [PATCH v4 4/5] hw/arm : Connect DM163 to B-L475E-IOT01A Inès Varhol
@ 2024-04-14 13:05 ` Inès Varhol
2024-04-15 5:11 ` Thomas Huth
2024-04-15 9:37 ` Philippe Mathieu-Daudé
4 siblings, 2 replies; 13+ messages in thread
From: Inès Varhol @ 2024-04-14 13:05 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé, Paolo Bonzini, Thomas Huth,
Inès Varhol, Marc-André Lureau, qemu-arm,
Laurent Vivier, Samuel Tardieu, Arnaud Minier, Peter Maydell
`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 propagated 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 | 5 +
2 files changed, 197 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..6f88ceef44
--- /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/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, DEVICE_NAME);
+
+ 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 36c5c13a7b..2c6fd4ebbb 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -207,6 +207,9 @@ qtests_stm32l4x5 = \
'stm32l4x5_rcc-test',
'stm32l4x5_gpio-test']
+qtests_dm163 = \
+ ['dm163-test']
+
qtests_arm = \
(config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : []) + \
(config_all_devices.has_key('CONFIG_CMSDK_APB_DUALTIMER') ? ['cmsdk-apb-dualtimer-test'] : []) + \
@@ -222,6 +225,8 @@ qtests_arm = \
(config_all_devices.has_key('CONFIG_MICROBIT') ? ['microbit-test'] : []) + \
(config_all_devices.has_key('CONFIG_STM32L4X5_SOC') ? qtests_stm32l4x5 : []) + \
(config_all_devices.has_key('CONFIG_FSI_APB2OPB_ASPEED') ? ['aspeed_fsi-test'] : []) + \
+ (config_all_devices.has_key('CONFIG_STM32L4X5_SOC') and
+ config_all_devices.has_key('CONFIG_DM163')? qtests_dm163 : []) + \
['arm-cpu-features',
'boot-serial-test']
--
2.43.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 5/5] tests/qtest : Add testcase for DM163
2024-04-14 13:05 ` [PATCH v4 5/5] tests/qtest : Add testcase for DM163 Inès Varhol
@ 2024-04-15 5:11 ` Thomas Huth
2024-04-15 9:37 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2024-04-15 5:11 UTC (permalink / raw)
To: Inès Varhol, qemu-devel
Cc: Philippe Mathieu-Daudé, Paolo Bonzini,
Marc-André Lureau, qemu-arm, Laurent Vivier, Samuel Tardieu,
Arnaud Minier, Peter Maydell
On 14/04/2024 15.05, 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 propagated 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 | 5 +
> 2 files changed, 197 insertions(+)
> create mode 100644 tests/qtest/dm163-test.c
...
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 36c5c13a7b..2c6fd4ebbb 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -207,6 +207,9 @@ qtests_stm32l4x5 = \
> 'stm32l4x5_rcc-test',
> 'stm32l4x5_gpio-test']
>
> +qtests_dm163 = \
> + ['dm163-test']
Do you plan to add more tests to qtests_dm163 later? If no, it might be more
straight forward to inline ['dm163-test'] in the hunk below instead.
> qtests_arm = \
> (config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : []) + \
> (config_all_devices.has_key('CONFIG_CMSDK_APB_DUALTIMER') ? ['cmsdk-apb-dualtimer-test'] : []) + \
> @@ -222,6 +225,8 @@ qtests_arm = \
> (config_all_devices.has_key('CONFIG_MICROBIT') ? ['microbit-test'] : []) + \
> (config_all_devices.has_key('CONFIG_STM32L4X5_SOC') ? qtests_stm32l4x5 : []) + \
> (config_all_devices.has_key('CONFIG_FSI_APB2OPB_ASPEED') ? ['aspeed_fsi-test'] : []) + \
> + (config_all_devices.has_key('CONFIG_STM32L4X5_SOC') and
> + config_all_devices.has_key('CONFIG_DM163')? qtests_dm163 : []) + \
With qtests_dm163 preferably replaced by ['dm163-test'] :
Acked-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 5/5] tests/qtest : Add testcase for DM163
2024-04-14 13:05 ` [PATCH v4 5/5] tests/qtest : Add testcase for DM163 Inès Varhol
2024-04-15 5:11 ` Thomas Huth
@ 2024-04-15 9:37 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-15 9:37 UTC (permalink / raw)
To: Inès Varhol, qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Marc-André Lureau, qemu-arm,
Laurent Vivier, Samuel Tardieu, Arnaud Minier, Peter Maydell
On 14/4/24 15:05, 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 propagated 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 | 5 +
> 2 files changed, 197 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..6f88ceef44
> --- /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
(Preferably enum).
> +static void test_dm163_bank(const void *opaque)
> +{
> + const long bank = (uintptr_t) opaque;
s/long/unsigned/ is enough.
> + 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);
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 13+ messages in thread