[RFC] digital::OutputPort, atomically drive several pins

[japaric]

In the v0.1.0 release of this crate we have a digital::OutputPin that represents a single digital output pin. That trait is useful for, e.g., implementing the NSS pin of a SPI interface but not enough to implement 8 or 16 bit parallel port interfaces where all the pins need to change at the same time (atomically) to meet timing requirements (e.g. LCD interfaces) -- using 16 impl OutputPin would result in them changing state at different time intervals.

I think the obvious trait to use would be the following:

/// A digital output "port"
///
/// `Width` is the size of the port; it could be `u8` for an 8-bit parallel
/// port, `u16` for a 16-bit one, etc.
///
/// **NOTE** The "port" doesn't necessarily has to match a hardware GPIO port;
/// it could for instance be a 4-bit ports made up of non contiguous pins, say
/// `PA0`, `PA3`, `PA10` and `PA13`.
pub trait OutputPort<Width> {
    /// Outputs `word` on the port pins
    ///
    /// # Contract
    ///
    /// The state of all the port pins will change atomically ("at the same time"). This usually
    /// means that state of all the pins will be changed in a single register operation.
    fn output(&mut self, word: Width);
}

cc @kunerd

[kunerd]

Never thought about this, but you are right, without an atomic set the communication timing could become broken by interrupts. But, wouldn't we than need an InputPort and an InputOutputPort, too?

[japaric]

But, wouldn't we than need an InputPort and an InputOutputPort, too?

Possibly. You let me know what you need for your driver. :-)

[idubrov]

I like the idea of being able to drive multiple pins separately as a single atomic thing -- for the timing reasons you've mentioned.

However, for the purpose of HD44780 LCD, I think, timings on data output are less important than being able to use pins from different GPIO ports (like GPIOA/GPIOB).

Even if bit operation is interrupted by, well, interrupt, it does not matter for HD44780 as data is transferred via dedicated "pulse" on EN bit after all data pins are set.

Given that flexibility of LCD, it would be tempting to assign pins to it which are "left" from the more demanding devices (like those requiring timer PWM, atomic operations on across multiple pins, etc), which could lead to the situation when all pins are taken from different ports.

Theoretically, you can design this trait to be able to span multiple ports and make it "do its best" to do as little operations as possible...

[japaric]

@idubrov in that case I think we can split the atomic property into a marker trait so that where T: digital::OutputPort + Atomic will toggle all the pins atomically whereas where T: digital::Output may or may not change the state of the pins atomically. A driver for the HD44780 could make use of the latter bound.

[kunerd]

I have done some quick testing and @idubrov seems to be right, for the HD44780 LCD driver the timing doesn't matter. The only situation where the output can become corrupted is an Interrupt that changes the state of the display Pins. Now, the question is, should preventing that be a part of the embedded-hal crate? If not I will go by with the InputPin, OutputPin and IoPin traits.

[idubrov]

Well, in an ideal world, where you can have a perfect cut & slice on your GPIO (like all pins of GPIOA+GPIOB+GPIOC => Slice1 + Slice2 + Slice3), your interrupt should not have "mutable"/"exclusive" access to the same slice as your non-interrupt code writing to the display.

Regarding Atomic, my concern (could be unfounded, I haven't really done a lot of thinking), is that if you go for that "perfect solution" so to speak, with all these Atomic traits, ability to mix and match multiple pins, etc, wouldn't the complexity make it hard to use/implement?

[idubrov]

Though, if you can have it, I would certainly love the solution that allows me to:

1. Mix multiple GPIO's
2. Split them into individual "slices" of pins/ranges of pins (like one slice could be "GPIOA1/GPIOA2/GPIOB3" without Atomic trait and another "GPIOC1/GPIOC2" with Atomic trait)
3. Have these slices moveable, so I can pass them as a token of "exclusive" access
4. Have them re-configurable, so I can reconfigure each individual slice as needed, without requiring any exclusive token for configuration registers (if memory bit-banding is available) and with requiring token (ownership of CRL/CRH, for example) if memory bit-banding is not-available (although, personally, I don't care about this case -- but it's probably important for those working with different devices).
5. Maybe atomic reconfiguration, which would require configuration registers ownership and would only work for ranges with pins from the same register. Or even same half-register (as there are usually two different configuration registers for "high" pins and "low" pins.
6. Don't know about integration with other peripherals. I think, as you start thinking of other peripherals, like timers, it becomes way too complicated (not all pins could be used as PWM outputs, but you can use alternative pin, or maybe inverted pin, or maybe pin from a different channel, etc).
7. Reasonable compilation time/IDE ergonomics

@kunerd @japaric

[therealprof]

I see you guys are all very ST centric. ;) Are few things are more awkward in the ST world (like having different GPIO banks and sometimes high/low registers) while others are quite a bit more comfortable, like not having to use 10 different registers to send/receive data on an I2C port or having to manually reset events after them being triggered.

However one thing that pretty much all devices have in common is that often registers have to be split into individual bits or group of bits and ideally the control over the bits should be movable into abstracted types which cannot easily be done at the moment.

This should be abstracted in a way that allows a type to exercise control over exactly the required bits but nothing more which in some cases may mean it be done atomically (either due to the availability of bitbanding or separate set/clear registers) while in other cases it will mean that RMW is required.

[japaric]

@idubrov all those sound like configuration details that both the trait and the generic driver author don't have to concern themselves about. Do you think the proposed trait would get in the way of implementing any / all of that configurability? I don't think so but won't know for sure until I sit down and try to implement it. I don't have a use case that requires implementing something so elaborated though so don't count on me implementing that.

requiring token (ownership of CRL/CRH, for example) if memory bit-banding is not-available

if you don't have bit banding you can do the RMW operation in a critical section and not require the token.

[sajattack]

Bump. Parallel writes to output pins are a necessity for many usecases, especially LCD drivers.

[eldruin]

@sajattack In the mean time, if you can add additional hardware, you can use a cheap I/O port expander, which allows you to set all 8/16 outputs at once. See pcf857x.
disclaimer: I wrote that driver.

[pitaj]

Consider the likely performance difference between consecutive and non-consecutive pins in a bus.

Say we have 32 pins controlled by a single u32 register, GPIO->DATA_OUT. Where P0 is controlled by bit 0, P1 is controlled by bit 1, etc.

If we want to write a u8 to P8:P15 or P19:P26 (in the same bit to pin order), all we need to do is left-shift that u8, mask the controlled pins out of the current register value, and logical-or the two results to get the new register value. This only requires we know:

• inverted mask of the pins in the bus
• position of the lowest index pin
• register to modify

If we want to write a u8 to P0, P2, P4, P6, P8, P10, P14, and P16 (regardless of order), it gets much more complicated and less optimizable. There are two options: (1) lookup table taking up a lot of memory or (2) bit-wise iteration. I think 1 is mostly self-explanatory, but for 2 you'd need to know:

• inverted mask for all pins
• mask for each pin
• which pin corresponds to which bit in the u8

Then you have to iterate through each bit of the u8, testing if the bit is a 1 and logical-or ing the corresponding masks together with the masked old register value.

The performance/memory implications of this increase linearly with the size of the values to write and read.

I think there should be two types of output bus, in order to accommodate these two very different cases.

[ryankurte]

hmm, interesting. afaik most processors have some bitband registers that should make that more efficient than otherwise, but it's still not always going to be efficient.

what about a single bus type (so drivers don't have to care) with a marker trait to indicate / support a requirement for pseudo-atomic / single operation implementations?

[pitaj]

My main concern is to make it abundantly clear if such a penalty will be required for a given setup.

Another confounding factor is that some micros provide a layer with which to remap certain pins to different bits on the bus. For instance, P11 could be remapped to bit 3.

[pitaj]

I think we need two traits to support discontiguous pins: OutputPort and FlexibleOutputPort. OutputPort has a the output method, and FlexibleOutputPort has a output_flexible method. Additionally, FlexibleOutputPort is implemented for every OutputPort. This makes it clear when you're paying a performance/memory cost, but if you don't care you just use output_flexible.

I've been thinking of ways to implement this in an actual hal. I like the idea of passing a range of OutputPins into a join function to get an OutputPort. The Range<OutputPin> gaurantees the pins are contiguous.

If they aren't contiguous then an array of OutputPins would be passed into join_flexible that would output a FlexibleOutputPort.

[luojia65]

Should this trait be:

pub trait OutputPort<Width> {
    type Error;
    fn output(&mut self, word: Width) -> Result<(), Self::Error>;
}

or

pub trait OutputPort {
    type Error;
    type Width;
    fn output(&mut self, word: Self::Width) -> Result<(), Self::Error>;
}

in v2 of digital input/output module?

[pitaj]

1. The first one allows implementing OutputPort for Self for multiple Width.
2. The second one, instead, enforces that for a given Self there is a single Width associated.

I think any given OutputPort should only support one width, so the second makes the most sense idiomatically in rust. If anyone can think of an instance where that wouldn't hold, please say so.

Additionally, with the associated type, you could share the width between the OutputPort and FlexibleOutputPort if it's decided to go in that direction.

[luojia65]

Maybe we should get a reference design of this feature; that could be built into stm32f30x-hal crate or somewhere else.
For example, when I split a pac::GPIOA port we get Parts and then all pins, but how should we get the ownership GPIOA itself? Implement OutputPort for it directly, or how should we wrap it as an owned struct? How to name this wrapping function? If we may change the mode of all pins at once, how should we teach it? Thanks!

[ferkulat]

I have another use case for changing the output value of several pins at once:
Driving a 7-segment display.
Rather than setting 7 pins separately for each number, I would like to use either a match expression for each number that writes a fixed value that deals with all 7 segments at once. Or some kind of lookup table.
And due to board layout issues the port pins in use are not necessarily adjacent.

[lloydjatkinson]

Has there been any progress on this? It's incredibly common to do this in C.

[burrbull]

Has there been any progress on this? It's incredibly common to do this in C.

There are several questions needed to be solved: #134 (comment)

You could discuss them on Tuesday matrix chat meeting.