Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a GPIO applet, for controlling the pins directly from Python #677

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

attie
Copy link
Member

@attie attie commented Aug 24, 2024

Use either REPL or a script to directly control Glasgow's I/O pins.

  • Input: iface.read(channel)
  • Output: iface.write(channel, level, enable=True)
  • Toggle: iface.toggle(channel, enable=True)
  • Set pull-up / pull-down: iface.pull(channel, level, enable=True)
  • Set Hi-Z: iface.hiz(channel)

This uses a single byte command with an optional response (only currently used for reads).
The command consists of a 2-bit opcode, 4-bit channel ID (0-15), a level, and an enable flag.

@whitequark - as discussed, feedback appreciated when you get to it (no hurry from me)

@attie attie requested a review from whitequark as a code owner August 24, 2024 22:50
@attie
Copy link
Member Author

attie commented Aug 25, 2024

On reflection, I'm going to rework this to handle the OE signals separately (new opcode).

I'd also like to implement .set() and .clear() functions that will behave like pins |= v and pins &= ~v respectively. Perhaps also an .eq() function that can optionally accept a bitmask to selectively update the pins.

__iand__() and __ior__() might be nice to have, but I'd need to figure out how to deal with the async side of things, and there's no way to implement the "equals"... perhaps properties with a setter could work, but I think that's getting into the weeds...

My rationale behind not sending a full bitmap was to avoid the host needing to track signal states - I know that what I've done is contrary to what @whitequark is probably thinking (re: "a FIFO that grabs 16 bits and distributes them across buffers"). The worst-case is currently a 16-byte write to set all pins, but comes with a minor benefit that single pin changes require a single byte, instead of 3 (e.g: opcode + 16-bit) - and I suspect the pin-by-pin approach will be the more typical use case for this applet(?) ... Happy to discuss and change it around if needs be.

@whitequark
Copy link
Member

whitequark commented Aug 26, 2024

My rationale behind not sending a full bitmap was to avoid the host needing to track signal states

I don't see this as particularly valuable--sending one byte costs as much as sending two or four bytes, since the bus is packet based. (For the same reason I don't see much point in having TOGGLE as an opcode.) Yes, receiving and parsing them does cost a bit more, but the end to end latency is going to be dominated by USB, not parsing bytes in gateware.

I think it's useful to have the output and output enable set at the same time (atomically) to avoid glitches. Yes, those are only 20 ns long glitches, but 20 ns is long enough to trigger asynchronous signals like clocks or resets.

In general, I think the protocol should be based on bulk actions (i.e. "set all n pins to state") rather than individual actions (i.e. "set pin x to state") because the former is generic enough to do the latter but not vice versa. If you want to toggle two pins at the sam e time you can't do it with two individual actions, there's always going to be an offset of something like 20 to 80 ns depending on how you implement it.

@attie
Copy link
Member Author

attie commented Aug 26, 2024

In general, I think the protocol should be based on bulk actions (i.e. "set all n pins to state") rather than individual actions (i.e. "set pin x to state")

Sure, I talked myself around to that last night already... I realised my original stance was very tied to my original use-case, which isn't the correct one to base this stuff on.

I think it's useful to have the output and output enable set at the same time

To clarify, would you have just "Read" and "Write+OE" as operations?
I was happy enough to have Write and OE separate, and put it on the user to call them in the correct (safe) order, much like MCU registers.

I don't see much point in having TOGGLE as an opcode

Sure - as the python-side will be tracking the pin states, then that's easy enough to resolve.

@whitequark
Copy link
Member

To clarify, would you have just "Read" and "Write+OE" as operations?

I think so--it seems like there's no reason not to.

I might also make GPIOInterface allow usage both like this:

gpio.set_hi(2)
gpio.set_lo(1)

as well as like this:

with gpio.multiple():
  gpio.set_hi(2)
  gpio.set_lo(1)

(It would be prohibited to read pins from within a multiple block.) This is very optional though.

At some point we should brainstorm/bikeshed function names and stuff, the ones above are just whatever I came up with on the spot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants