Developing I2S module for uPy

[blmorris]

I have started work on supporting I2S as a class within the pyb module. (For reference, I2S stands for Inter-IC-Sound, a protocol for transferring real-time audio data between devices like audio ADC's, DAC's and codecs.)

The I2S hardware implementation on the STM32F4xx series is based on the special configuration of the SPI engine, although the usage and setup are different enough that I don't see a straightforward way to graft that configuration onto the existing pyb.SPI class without terribly mucking up the SPI API. Instead I am taking the approach of using the existing SPI implementation as a reference to build a new I2S class from scratch.

I will have some (many!) questions as I proceed with this, and once I have something minimally functional I will post a PR on github and move the discussion there. For now, I have a question about a code convention used frequently in spi.c that I don't really understand; I'm not sure if I should follow it completely or implement it in a way that makes more sense to me.
The shortest example I could see is in the spi_deinit() function:

Code: Select all

void spi_deinit(SPI_HandleTypeDef *spi) {
    HAL_SPI_DeInit(spi);
    if (0) {
#if MICROPY_HW_ENABLE_SPI1
    } else if (spi->Instance == SPI1) {
        __SPI1_FORCE_RESET();
        __SPI1_RELEASE_RESET();
        __SPI1_CLK_DISABLE();
#endif
#if MICROPY_HW_ENABLE_SPI2
    } else if (spi->Instance == SPI2) {
        __SPI2_FORCE_RESET();
        __SPI2_RELEASE_RESET();
        __SPI2_CLK_DISABLE();
#endif
#if MICROPY_HW_ENABLE_SPI3
    } else if (spi->Instance == SPI3) {
        __SPI3_FORCE_RESET();
        __SPI3_RELEASE_RESET();
        __SPI3_CLK_DISABLE();
#endif
    }
}

Regardless of how the guard macros are evaluated, there is always an initial 'if(0) { } else if (spi->Instance == SPIx) { … }'

What is the purpose of the empty 'if(0) { }' clause, if it always evaluates as false? The same convention also appears in the spi_init() function, but that is much more complex to serve as a clear illustration here.

Thanks!
Bryan

[Damien]

The if(0) is a way of making a sequence of If statements where the first proper one may or may not exist depending on the macro definitions. It's only done that way to make the curly braces balance neatly.

[blmorris]

Thanks, I see how that works now.
I had another question regarding a possible API for I2S, but since it was more of a design RFC I went ahead and posted it as an issue on github.
-Bryan

[pythoncoder]

@blmorris I'm interested to hear how you see I2S being used on the Pyboard. I've been writing some DSP code in assembler. While the assembler code is fast enough to perform filtering or convolution at hi-fi audio sample rates, in my tests the botleneck is in the interface to Python. One test uses a timer callback to read a value from an ADC, run the filter code, and output the result on a DAC. I found that the highest rate I could run the callback was on the order of 15KHz. While this is fast enough for many applications it falls a long way short of CD quaity (44.1KHz*2 channels).

I drew the conclusion that real time processing of CD quality audio was probaly beyond the capabilities of MicroPython running on the Pyboard, unless the processing is done in C or assembler. But perhaps I'm missing something?

[blmorris]

@pythoncoder - My main use of I2S in MicroPython won't be on the Pyboard, but rather on an audio amplifier I designed which uses an STM32F405 to control an audio DSP chip (the ADAU1701) and to provide audio playback from an SD card. (Coincidently this was designed a few months before I learned about micropython.) Of course I want this module to be usable on the Pyboard as well - I'm thinking of designing a daughterboard / pyskin with an I2S codec to make it easier for others to use.
A colleague of mine has developed code for my amplifier in C, but I am looking to use uPy to give us more flexibility in developing custom applications - in particular I would like to analyze room acoustics to do adaptive equalization; your FFT / DSP code may be very useful and I'll let you know if/when I get to use it.
I don't expect to do real-time audio filtering within uPy (I have the dedicated DSP codec for that); uPy tasks will include playback, recording, analysis, possibly signal generation. I expect that I will need to use DMA to keep the processor overhead for I2S low.
-Bryan

[pythoncoder]

That sounds very interesting. Do keep us posted.

[blmorris]

Another C programming question, this time about modifying one of the type definitions.
Out of the peripheral classes implemented in the pyb module, I2S is most similar to SPI (and in fact uses the same hardware internally, although configured differently) so I am mostly using 'stmhal/spi.c' as my model.
I am now trying to do something a bit different than I have seen so far in the uPy code: many of the peripheral constructors take an instance number or name - examples would be SPI(1), I2C(2), UART(4) or even CAN("YA") or CAN("YB").
Instead of this, I would like for my I2S constructor to accept a list of pins for the I2S bus to use: for the audio processor board I designed, this will be

Code: Select all

// pin list has format [clock, word select, data tx, data rx]:
i2s = pyb.I2S(['B10','B12','C3','C2'])

I have this part working well - I can accept either a tuple or a list, and the pin names can be either processor names (A0, B4, …) or board names (X3, Y5, …) or even initialized pin objects (pyb.Pin.board.X5). The constructor (pyb_i2s_make_new()) then checks that the list describes a valid I2S bus configuration, and returns an uninitialized I2S object or throws an error. If additional arguments are provided, then pyb_i2s_make_new() dispatches the I2S object and the arguments to pyb_i2s_init_helper(), otherwise it returns the I2S object which can be initialized later on with i2s.init(args).
I am now trying to figure out how to make the validated pin list part of the I2S object structure. Currently the I2S struct is defined by this code, which was adapted almost directly from an analogous section from SPI:

Code: Select all

typedef struct _pyb_i2s_obj_t {
    mp_obj_base_t base;
    I2S_HandleTypeDef *i2s;
    // pin_obj_t pins;  // I would like to have this but haven't figured it out yet
} pyb_i2s_obj_t;

STATIC const pyb_i2s_obj_t pyb_i2s_obj[] = {
#if MICROPY_HW_ENABLE_I2S2
    {{&pyb_i2s_type}, &I2SHandle2},
#else
    {{&pyb_i2s_type}, NULL},
#endif
#if MICROPY_HW_ENABLE_I2S3
    {{&pyb_i2s_type}, &I2SHandle3},
#else
    {{&pyb_i2s_type}, NULL},
#endif
};
#define PYB_NUM_I2S MP_ARRAY_SIZE(pyb_i2s_obj)

I2S_HandleTypeDef *i2s_get_handle(mp_obj_t o) {
    if (!MP_OBJ_IS_TYPE(o, &pyb_i2s_type)) {
        nlr_raise(mp_obj_new_exception_msg(&mp_type_ValueError,
					   "expecting an I2S object"));
    }
    pyb_i2s_obj_t *self = o;
    return self->i2s;
}

Then, at the end of pyb_i2s_make_new() there is the following code (again almost directly adapted from spi.c):

Code: Select all

    // get I2S object
    const pyb_i2s_obj_t *i2s_obj = &pyb_i2s_obj[i2s_id];
    // The next line doesn't work, I have tried many iterations and I get errors about assigning values to 
    // const structure or incompatible pointer types, basically I'm not really close...
    // i2s_obj->pins = pins; 
    
    if (n_args > 1 || n_kw > 0) {
        // start the peripheral
        mp_map_t kw_args;
        mp_map_init_fixed_table(&kw_args, n_kw, args + n_args);
        pyb_i2s_init_helper(i2s_obj, n_args - 1, args + 1, &kw_args);
    }

    return (mp_obj_t)i2s_obj;
}

I'd appreciate any ideas for how to get this to work.
-Bryan

[Damien]

You have a few options here, and it's not immediately clear which is the right one.

To start with you have to decide if the I2S object can be static/const (ie live in ROM) or must be dynamic. SPI objects can live in ROM because all their state is in the hardware registers and the SPIHandle. Same with LED objects. UART objects on the other hand must live in RAM because they have state that is not fixed.

ROM objects obviously take no RAM so are more efficient in that respect and would be the first choice if possible.

If you go for a ROM object then you can model it off SPI. But that means you must have a fixed set of pins for each I2S instance. If there were many different pin configs then I wouldn't put the objects in ROM because you'd need a separate object for each config. But if there is only a few different configs then go for a ROM object, and predefine the set of pins for each on. See the LED object on how to predefine pins.

If you need to go for RAM objects then you can look how UART works. Note that it caches the UART objects in the MP_STATE_PORT(pyb_uart_obj_all) array. It does this for 2 reasons: so that the objects can be reached by the GC, and so that if you try to recreate the same UART you get a pointer to the same object (which makes sense -- you don't want 2 separate objects controlling the same underlying hardware peripheral).

Please ask further questions if you have them!

[blmorris]

Thanks, I didn't appreciate the distinction of objects being static or dynamic. I had also started to look at the CAN implementation; I didn't understand why it was so different from SPI but I see now that it a dynamic object.
If my math is correct, each I2S port has 48 possible valid pin configurations (96 if MCLK is used). This isn't an especially large number, but clearly too many to have a separate ROM object for each config. On the other hand, once the configuration is set I don't think that there is any state to maintain outside of the peripheral and GPIO registers. I don't know if there is any way to take advantage of that within a static ROM object; for now I'll study UART and CAN and pursue the RAM object approach.
I will have more questions, but it may be a few days. Thanks again!

[blmorris]

I may be asking to grossly abuse the MicroPython C libraries here, but I am wondering if it is possible to define a null or dummy pin object that can indicate a nonexistent pin in an array of pins.
Background - Unlike most peripheral class constructors which accept an integer or string name to indicate which instance of the peripheral the object will control, the I2S class constructor will accept a list of pins or pin name strings; the constructor checks that the list is valid and infers whether I2S will operate in simplex (one direction) or duplex (both directions) based on whether the list names one or two data pins.
In my initial implementations, if one of the data pin names was omitted, I set that entry in the pin array to MP_OBJ_NULL instead of &pin_C2 (or whatever the pin happens to be)

Code: Select all

mp_buffer_info_t bufinfo;
mp_obj_t *pin_names;
const pin_obj_t *pins[4];
mp_obj_get_array(args[0], &bufinfo.len, &pin_names);
if ((mp_uint_t) bufinfo.len != 4) {
	nlr_raise(mp_obj_new_exception_msg_varg(&mp_type_ValueError, "Pin list requires 4 items, %d given", bufinfo.len));
}
// Get array of pin objects; False / empty values are valid for pins[2] and pins[3]; empty values get set to MP_OBJ_NULL 
// to be checked by pin validation logic
for (int i = 0; i < 4; i++) {
	if (mp_obj_is_true(pin_names[i])) {
		pins[i] = pin_find(pin_names[i]);
	} else {
		pins[i] = MP_OBJ_NULL;
	}
// Note: this section isn't necessary as this information and more can
// be printed automagically when pyb.Pin.debug(True) is set.
	if (1) { // DEBUG - print list of pin objects
		mp_obj_print((mp_obj_t)pins[i], PRINT_STR);
		printf("\n");
	}
}

I was pretty sure that this was a nasty hack, but it worked well enough initially and I could just add some logic to check if a pin was valid.
However, now I also want to do things like this and have it work without having to check if 'pin' is really a pin:

Code: Select all

for (int i = 0; i < 3; i++) {
	print(env, "%s ", qstr_str(self->pins[i]->name));
}

Ideally a dummy pin object would look just like a valid pin, but its name would be 'None' and I could also pass it to this GPIO initialization loop (adapted from spi.c) without breaking anything - maybe it could point to an address in the GPIO configuration register space which doesn't correspond to an actual pin that the STM32F4 series implements:

Code: Select all

for (uint i = 0; i < 4; i++) {
	GPIO_InitStructure.Pin = i2s_obj->pins[i]->pin_mask;
	if (i2s_obj->pins[i] == &pin_B14 || i2s_obj->pins[i] == &pin_C2) {
		GPIO_InitStructure.Alternate = GPIO_AF6_I2S2ext;
	} else {
		GPIO_InitStructure.Alternate = GPIO_AF5_SPI2;
	}
	HAL_GPIO_Init(i2s_obj->pins[i]->gpio, &GPIO_InitStructure);
}

Thanks!
Bryan