Add pio_sm_set_consecutive_pins() #2692

New issue

Open

opened 2025-10-10 22:00:35 -05:00 by magy00 · 2 comments

magy00 commented

2025-10-10 22:00:35 -05:00

(Migrated from github.com)

A function pio_sm_set_consecutive_pins(), complementary to pio_sm_set_consecutive_pindirs(), would allow to (re-)initialize some pins to 0 (or all-ones) without having to resort to pio_sm_set_pins_with_mask64() and having to calculate a mask first. It would use a count parameter like used for pio_sm_set_*_pins(), sm_config_set_*_pin_count() or sm_config_set_*_pins() already.

It could probably be implemented by passing an additional parameter with pio_pins or pio_pindirs to a renamed pio_sm_set_consecutive_pindirs() with two wrappers.

A function `pio_sm_set_consecutive_pins()`, complementary to `pio_sm_set_consecutive_pindirs()`, would allow to (re-)initialize some pins to `0` (or all-ones) without having to resort to `pio_sm_set_pins_with_mask64()` and having to calculate a mask first. It would use a `count` parameter like used for `pio_sm_set_*_pins()`, `sm_config_set_*_pin_count()` or `sm_config_set_*_pins()` already. It could probably be implemented by passing an additional parameter with `pio_pins` or `pio_pindirs` to a renamed `pio_sm_set_consecutive_pindirs()` with two wrappers.

lurch commented

2025-10-11 03:57:12 -05:00

(Migrated from github.com)

It could probably be implemented by passing an additional parameter with pio_pins or pio_pindirs to a renamed pio_sm_set_consecutive_pindirs() with two wrappers.

I guess you're suggesting something like:

int pio_sm_set_consecutive_internal(PIO pio, uint sm, uint pin, uint count, bool is_out, bool set_pindirs) {
    check_pio_param(pio);
    check_sm_param(sm);
    pin -= pio_get_gpio_base(pio);
    invalid_params_if_and_return(PIO, pin >= 32u, PICO_ERROR_INVALID_ARG);
    uint32_t pinctrl_saved = pio->sm[sm].pinctrl;
    uint32_t execctrl_saved = pio->sm[sm].execctrl;
    hw_clear_bits(&pio->sm[sm].execctrl, 1u << PIO_SM0_EXECCTRL_OUT_STICKY_LSB);
    uint pindir_val = is_out ? 0x1f : 0;
    while (count > 5) {
        pio->sm[sm].pinctrl = (5u << PIO_SM0_PINCTRL_SET_COUNT_LSB) | (pin << PIO_SM0_PINCTRL_SET_BASE_LSB);
        pio_sm_exec(pio, sm, pio_encode_set((set_pindirs ? pio_pindirs : pio_pins), pindir_val));
        count -= 5;
        pin = (pin + 5) & 0x1f;
    }
    pio->sm[sm].pinctrl = (count << PIO_SM0_PINCTRL_SET_COUNT_LSB) | (pin << PIO_SM0_PINCTRL_SET_BASE_LSB);
    pio_sm_exec(pio, sm, pio_encode_set((set_pindirs ? pio_pindirs : pio_pins), pindir_val));
    pio->sm[sm].pinctrl = pinctrl_saved;
    pio->sm[sm].execctrl = execctrl_saved;
    return PICO_OK;
}

int pio_sm_set_consecutive_pindirs(PIO pio, uint sm, uint pin, uint count, bool is_out) {
    return pio_sm_set_consecutive_internal(pio, sm, pin, count, is_out, true);
}

int pio_sm_set_consecutive_pins(PIO pio, uint sm, uint pin, uint count, bool is_out) {
    return pio_sm_set_consecutive_internal(pio, sm, pin, count, is_out, false);
}

(note: this is entirely untested, and may or may not work!)

> It could probably be implemented by passing an additional parameter with pio_pins or pio_pindirs to a renamed pio_sm_set_consecutive_pindirs() with two wrappers. I guess you're suggesting _something_ like: ```c int pio_sm_set_consecutive_internal(PIO pio, uint sm, uint pin, uint count, bool is_out, bool set_pindirs) { check_pio_param(pio); check_sm_param(sm); pin -= pio_get_gpio_base(pio); invalid_params_if_and_return(PIO, pin >= 32u, PICO_ERROR_INVALID_ARG); uint32_t pinctrl_saved = pio->sm[sm].pinctrl; uint32_t execctrl_saved = pio->sm[sm].execctrl; hw_clear_bits(&pio->sm[sm].execctrl, 1u << PIO_SM0_EXECCTRL_OUT_STICKY_LSB); uint pindir_val = is_out ? 0x1f : 0; while (count > 5) { pio->sm[sm].pinctrl = (5u << PIO_SM0_PINCTRL_SET_COUNT_LSB) | (pin << PIO_SM0_PINCTRL_SET_BASE_LSB); pio_sm_exec(pio, sm, pio_encode_set((set_pindirs ? pio_pindirs : pio_pins), pindir_val)); count -= 5; pin = (pin + 5) & 0x1f; } pio->sm[sm].pinctrl = (count << PIO_SM0_PINCTRL_SET_COUNT_LSB) | (pin << PIO_SM0_PINCTRL_SET_BASE_LSB); pio_sm_exec(pio, sm, pio_encode_set((set_pindirs ? pio_pindirs : pio_pins), pindir_val)); pio->sm[sm].pinctrl = pinctrl_saved; pio->sm[sm].execctrl = execctrl_saved; return PICO_OK; } int pio_sm_set_consecutive_pindirs(PIO pio, uint sm, uint pin, uint count, bool is_out) { return pio_sm_set_consecutive_internal(pio, sm, pin, count, is_out, true); } int pio_sm_set_consecutive_pins(PIO pio, uint sm, uint pin, uint count, bool is_out) { return pio_sm_set_consecutive_internal(pio, sm, pin, count, is_out, false); } ``` (note: this is entirely untested, and may or may not work!)

magy00 commented

2025-10-11 08:57:02 -05:00

(Migrated from github.com)

Right. Perhaps even

diff --git src/rp2_common/hardware_pio/pio.c src/rp2_common/hardware_pio/pio.c
index 7182cff..df8991c 100644
--- src/rp2_common/hardware_pio/pio.c
+++ src/rp2_common/hardware_pio/pio.c
@@ -339,25 +339,25 @@ void pio_sm_set_pindirs_with_mask64(PIO pio, uint sm, uint64_t pindirs, uint64_t
 #endif
     pio_sm_set_pindirs_with_mask_internal(pio, sm, (uint32_t)pindirs, (uint32_t)pin_mask);
 }

-int pio_sm_set_consecutive_pindirs(PIO pio, uint sm, uint pin, uint count, bool is_out) {
+int pio_sm_set_consecutive_internal(PIO pio, uint sm, uint pin, uint count, bool set_pindirs, bool value) {
     check_pio_param(pio);
     check_sm_param(sm);
     pin -= pio_get_gpio_base(pio);
     invalid_params_if_and_return(PIO, pin >= 32u, PICO_ERROR_INVALID_ARG);
     uint32_t pinctrl_saved = pio->sm[sm].pinctrl;
     uint32_t execctrl_saved = pio->sm[sm].execctrl;
     hw_clear_bits(&pio->sm[sm].execctrl, 1u << PIO_SM0_EXECCTRL_OUT_STICKY_LSB);
-    uint pindir_val = is_out ? 0x1f : 0;
+    uint set_instr = pio_encode_set(set_pindirs ? pio_pindirs : pio_pins, value ? 0x1f : 0);
     while (count > 5) {
         pio->sm[sm].pinctrl = (5u << PIO_SM0_PINCTRL_SET_COUNT_LSB) | (pin << PIO_SM0_PINCTRL_SET_BASE_LSB);
-        pio_sm_exec(pio, sm, pio_encode_set(pio_pindirs, pindir_val));
+        pio_sm_exec(pio, sm, set_instr);
         count -= 5;
         pin = (pin + 5) & 0x1f;
     }
     pio->sm[sm].pinctrl = (count << PIO_SM0_PINCTRL_SET_COUNT_LSB) | (pin << PIO_SM0_PINCTRL_SET_BASE_LSB);
-    pio_sm_exec(pio, sm, pio_encode_set(pio_pindirs, pindir_val));
+    pio_sm_exec(pio, sm, set_instr);
     pio->sm[sm].pinctrl = pinctrl_saved;
     pio->sm[sm].execctrl = execctrl_saved;
     return PICO_OK;
 }

or just

diff --git src/rp2_common/hardware_pio/pio.c src/rp2_common/hardware_pio/pio.c
index 7182cff..a722dc1 100644
--- src/rp2_common/hardware_pio/pio.c
+++ src/rp2_common/hardware_pio/pio.c
@@ -342,3 +342,3 @@ void pio_sm_set_pindirs_with_mask64(PIO pio, uint sm, uint64_t pindirs, uint64_t

-int pio_sm_set_consecutive_pindirs(PIO pio, uint sm, uint pin, uint count, bool is_out) {
+int pio_sm_set_consecutive_internal(PIO pio, uint sm, uint pin, uint count, uint set_instr) {
     check_pio_param(pio);
@@ -350,6 +350,5 @@ int pio_sm_set_consecutive_pindirs(PIO pio, uint sm, uint pin, uint count, bool
     hw_clear_bits(&pio->sm[sm].execctrl, 1u << PIO_SM0_EXECCTRL_OUT_STICKY_LSB);
-    uint pindir_val = is_out ? 0x1f : 0;
     while (count > 5) {
         pio->sm[sm].pinctrl = (5u << PIO_SM0_PINCTRL_SET_COUNT_LSB) | (pin << PIO_SM0_PINCTRL_SET_BASE_LSB);
-        pio_sm_exec(pio, sm, pio_encode_set(pio_pindirs, pindir_val));
+        pio_sm_exec(pio, sm, set_instr);
         count -= 5;
@@ -358,3 +357,3 @@ int pio_sm_set_consecutive_pindirs(PIO pio, uint sm, uint pin, uint count, bool
     pio->sm[sm].pinctrl = (count << PIO_SM0_PINCTRL_SET_COUNT_LSB) | (pin << PIO_SM0_PINCTRL_SET_BASE_LSB);
-    pio_sm_exec(pio, sm, pio_encode_set(pio_pindirs, pindir_val));
+    pio_sm_exec(pio, sm, set_instr);
     pio->sm[sm].pinctrl = pinctrl_saved;

with the 2 wrappers as static inline in the header file (set_instr will likely be optimized to a constant).

Right. Perhaps even ```diff diff --git src/rp2_common/hardware_pio/pio.c src/rp2_common/hardware_pio/pio.c index 7182cff..df8991c 100644 --- src/rp2_common/hardware_pio/pio.c +++ src/rp2_common/hardware_pio/pio.c @@ -339,25 +339,25 @@ void pio_sm_set_pindirs_with_mask64(PIO pio, uint sm, uint64_t pindirs, uint64_t #endif pio_sm_set_pindirs_with_mask_internal(pio, sm, (uint32_t)pindirs, (uint32_t)pin_mask); } -int pio_sm_set_consecutive_pindirs(PIO pio, uint sm, uint pin, uint count, bool is_out) { +int pio_sm_set_consecutive_internal(PIO pio, uint sm, uint pin, uint count, bool set_pindirs, bool value) { check_pio_param(pio); check_sm_param(sm); pin -= pio_get_gpio_base(pio); invalid_params_if_and_return(PIO, pin >= 32u, PICO_ERROR_INVALID_ARG); uint32_t pinctrl_saved = pio->sm[sm].pinctrl; uint32_t execctrl_saved = pio->sm[sm].execctrl; hw_clear_bits(&pio->sm[sm].execctrl, 1u << PIO_SM0_EXECCTRL_OUT_STICKY_LSB); - uint pindir_val = is_out ? 0x1f : 0; + uint set_instr = pio_encode_set(set_pindirs ? pio_pindirs : pio_pins, value ? 0x1f : 0); while (count > 5) { pio->sm[sm].pinctrl = (5u << PIO_SM0_PINCTRL_SET_COUNT_LSB) | (pin << PIO_SM0_PINCTRL_SET_BASE_LSB); - pio_sm_exec(pio, sm, pio_encode_set(pio_pindirs, pindir_val)); + pio_sm_exec(pio, sm, set_instr); count -= 5; pin = (pin + 5) & 0x1f; } pio->sm[sm].pinctrl = (count << PIO_SM0_PINCTRL_SET_COUNT_LSB) | (pin << PIO_SM0_PINCTRL_SET_BASE_LSB); - pio_sm_exec(pio, sm, pio_encode_set(pio_pindirs, pindir_val)); + pio_sm_exec(pio, sm, set_instr); pio->sm[sm].pinctrl = pinctrl_saved; pio->sm[sm].execctrl = execctrl_saved; return PICO_OK; } ``` or just ```diff diff --git src/rp2_common/hardware_pio/pio.c src/rp2_common/hardware_pio/pio.c index 7182cff..a722dc1 100644 --- src/rp2_common/hardware_pio/pio.c +++ src/rp2_common/hardware_pio/pio.c @@ -342,3 +342,3 @@ void pio_sm_set_pindirs_with_mask64(PIO pio, uint sm, uint64_t pindirs, uint64_t -int pio_sm_set_consecutive_pindirs(PIO pio, uint sm, uint pin, uint count, bool is_out) { +int pio_sm_set_consecutive_internal(PIO pio, uint sm, uint pin, uint count, uint set_instr) { check_pio_param(pio); @@ -350,6 +350,5 @@ int pio_sm_set_consecutive_pindirs(PIO pio, uint sm, uint pin, uint count, bool hw_clear_bits(&pio->sm[sm].execctrl, 1u << PIO_SM0_EXECCTRL_OUT_STICKY_LSB); - uint pindir_val = is_out ? 0x1f : 0; while (count > 5) { pio->sm[sm].pinctrl = (5u << PIO_SM0_PINCTRL_SET_COUNT_LSB) | (pin << PIO_SM0_PINCTRL_SET_BASE_LSB); - pio_sm_exec(pio, sm, pio_encode_set(pio_pindirs, pindir_val)); + pio_sm_exec(pio, sm, set_instr); count -= 5; @@ -358,3 +357,3 @@ int pio_sm_set_consecutive_pindirs(PIO pio, uint sm, uint pin, uint count, bool pio->sm[sm].pinctrl = (count << PIO_SM0_PINCTRL_SET_COUNT_LSB) | (pin << PIO_SM0_PINCTRL_SET_BASE_LSB); - pio_sm_exec(pio, sm, pio_encode_set(pio_pindirs, pindir_val)); + pio_sm_exec(pio, sm, set_instr); pio->sm[sm].pinctrl = pinctrl_saved; ``` with the 2 wrappers as `static inline` in the header file (`set_instr` will likely be optimized to a constant).