tinygo icon indicating copy to clipboard operation
tinygo copied to clipboard

rp2040: basic PIO support

Open kenbell opened this issue 3 years ago • 13 comments

This implements basic PIO support using go:generate and pioasm

Related: https://github.com/kenbell/pico-sdk/tree/pioasm-go-output << changes to RPi Pico SDK pioasm tool to output TinyGo code https://github.com/kenbell/tinygo-rp2040-pio-example << Blinky example from Pico examples, converted to TinyGo

I'm not sure how much of this we want to pack into the machine package?

Open questions:

  • Do the APIs used by the generated code look good? (we need to freeze these to generate PR against Pico SDK)
  • Is the machine package the correct place for this code?
  • Do we want the code to create instructions dynamically in machine package? (should this be in a different package, or even separate repo?) [This is the code in machine_rp2040_pio_instr.go]
  • (related) do we want to make the functions in machine_rp2040_pio_instr.go public, or make private for now?

If we want to remove the dependency on pioasm or support inline PIO code in Go source files in future, we probably wouldn't want the assembler to depend on the machine package?

To try it out:

  • Get a copy of the modified pico-sdk (https://github.com/kenbell/pico-sdk/tree/pioasm-go-output), installing all prereqs
  • To build pioasm:
    • Create an directory, eg mkdir $PICO_SDK_PATH/pioasm-build
    • Inside that directory, do cmake $PICO_SDK_PATH/tools/pioasm
    • Do make
  • Add pioasm to your PATH, e.g. export PATH=$PATH:$PICO_SDK_PATH/pioasm-build
  • Get the example code (https://github.com/kenbell/tinygo-rp2040-pio-example)
  • Inside the example's directory, do go generate .
  • Build / flash using tinygo from this branch, eg tinygo build -target feather-rp2040 .

kenbell avatar Jul 03 '21 16:07 kenbell

To help identify public symbols used in generated code, I've added a comment like this:

// This function is used by code generated by pioasm, in the RP2040
// c-sdk - any changes should be backwards compatible.

kenbell avatar Jul 03 '21 16:07 kenbell

@aykevl thanks!

One big question I have is: does it make sense to support non-position independent code?

Yeah, i thought this was a bit wierd - since JMPs are relocated anyway. It looks like the reason is that it supports interleaving data and instructions in the TX FIFO (see 3.4.5.2. and 3.5.7. in https://datasheets.raspberrypi.org/rp2040/rp2040-datasheet.pdf). To do this, it's easier to pin the program, otherwise the code interleaving data and instructions also needs to support relocating it's code.

There's a directive in the PIO language to force the origin .origin <offset> - so if we don't support it, there'll be some subset of PIO code we can't support.

Also, I'm missing the PIO assembly source code in the example, is that correct? I only see the generated code. I'm interested in the source to better understand the generated code.

Oops - yeah, I forgot to include. I've added to the repo now: https://github.com/kenbell/tinygo-rp2040-pio-example/blob/main/blink.pio (this is the file in the c-sdk for comparison https://github.com/raspberrypi/pico-examples/blob/master/pio/pio_blink/blink.pio)

Move all PIO code to a separate repository, that can even be imported from regular Go code.

This is what I'm thinking, something like github.com/tinygo-org/rp2040-pio ?

Drop all dependencies from the generated code, leaving only the []uint16 slice with the instructions

Yeah, I started here. There's a few additional things to the actual program that are output by pioasm because they're specified in the .pio file as directives:

  • origin (fixed address if not relocatable)
  • wrapping config
  • sideset (every instruction can set pin state as a side-effect)
  • constants

The odd thing in the c-sdk, is they didn't consistently handle these. Origin and instructions are in a program type, but wrapping and sideset are considered 'config' and made available by a function. I'm assuming there's a reason, but I'm not seeing it.

Sideset is needed by the assembler since the instructions output depend on it (number of instruction bits allocated to sideset is variable)

If we want to support inline PIO code inside Go files in future, we might want to bundle these into the program type so a single variable can be output that encapsulates: instructions, origin, wrapping, sideset

I'll respond to the code comments inline.

kenbell avatar Jul 04 '21 16:07 kenbell

Thanks very much for creating this!

I've been coding Go professionally for the past 3 years, but I'm just starting out with TinyGo, so please forgive me if I ask a few dumb questions.

I want to use the RP2040 in a project that will need the PIO (and DMA) for precise timing and low-latency. MicroPython is almost certainly too slow and I'm afraid I've gotten spoiled by Go's great tooling, test support and sensible design to the extent I'd much prefer not to code in C unless there's no other reasonable choice.

So I guess my first question is whether PIO support is still an active effort and, if so, is it likely to be merged into tinygo master in the near future.

Michael-F-Ellis avatar Nov 11 '21 22:11 Michael-F-Ellis

The change to func (p Pin) Configure(config PinConfig) to allow for configuring a pin for use via PIO would be nice to have merged, in isolation of all the other changes. I think I'll open a PR for that.


I'm currently using this (with some of my own additions) to control an 8080 style 8-bit parallel TFT interface, as depicted here: https://forum.lvgl.io/t/raspberry-pi-pico-pio-proof-of-concept-15m-pixels-sec/5716

1155861663-4b5926eb595616fbcffae36315f28240038d7ea352872601a3d473fb8b2ef2ad-d

I ported their code from C++ to Go; it's working nicely :)

cstrahan avatar Dec 28 '21 03:12 cstrahan

There's a directive in the PIO language to force the origin .origin <offset> - so if we don't support it, there'll be some subset of PIO code we can't support.

Hmm, fair enough.

This is what I'm thinking, something like github.com/tinygo-org/rp2040-pio ?

Sounds good to me.

If we want to support inline PIO code inside Go files in future, we might want to bundle these into the program type so a single variable can be output that encapsulates: instructions, origin, wrapping, sideset

Without having spent too much time on PIO, I think that would still be possible with the bulk of it outside the machine package? The machine package could provide PIOProgram as-is and an external package could provide helper functions/structs/etc to create a PIOProgram.

aykevl avatar Dec 30 '21 16:12 aykevl

@kenbell you still working on this? I have a few improvements I'd like to push. How do I do that? fork your repo and push or some other way? No sign of the rp2040-pio repo! Is that a happening thing?

lincolngill avatar Apr 06 '22 07:04 lincolngill

Hey @lincolngill - I haven't had any time to spend on this for quite a while, unfortunately. Feel free to fork my repo and raise PRs directly to tinygo.

kenbell avatar Apr 06 '22 07:04 kenbell

Hey peeps- I've been wanting to get around to trying this out but I can't figure out how to build pioasm (I don't do that cmake thing). It'd be great to have some basic instructions on how to get this working that the average gopher could understand

soypat avatar Oct 08 '23 21:10 soypat

I've managed to get it working! I've "forked" this implementation and created a third-party package to enable using the PIO over here https://github.com/soypat/rp2040-pio.

I've renamed most of the types removing the PIO* prefix since the package name is pio and that would cause stutter.

soypat avatar Oct 22 '23 22:10 soypat

@kenbell After using it some I've got some suggestions for pioasm:

Modifications to pioasm

I think we can make pioasm library independent so that people can experiment with different ways of using PIO. Right now the implementation is hardcoded to use the machine library implementation which is not yet pushed. Proposed changes and benefits below.

Proposed changes

Remove hardcoded _pio.go generated code:

  • // +build rp2040\n\n
  • package %s\n\n
  • import \"machine\"\n\n
  • Inject the % go { include directive in .pio files at the top of the generated _pio.go file.
  • Generate the pioasm code at the end in non-struct fashion, i.e:
	const blinkWrapTarget = 2
	const blinkWrap = 7
	const blinkOrigin = -1
	var blinkInstructions = []uint64{
		0x80a0, //  0: pull   block                      
		0x6040, //  1: out    y, 32                      
		//     .wrap_target
		0xa022, //  2: mov    x, y                       
		0xe001, //  3: set    pins, 1                    
		0x0044, //  4: jmp    x--, 4                     
		0xa022, //  5: mov    x, y                       
		0xe000, //  6: set    pins, 0                    
		0x0047, //  7: jmp    x--, 7                     
		//     .wrap
	}

Benefits

  • Users can bring in external dependencies to use within the pio generated file.
  • Users can specify which PIO implementation to use
  • build tags can be specified within the pio
  • Users specify the package declaration in the pio file
  • Future proofs pioasm implementation since I'm guessing we may choose to move machine package out of the standard library sometime in the future to facilitate hardware mock/simulation testing.

pioasm external dependency problem and suggested solution

This has a glaring problem, the autogenerated *ProgramDefaultConfig needs external API that needs to be provided by user.

I suggest leaving it as-is except we switch the machine package for pio package and assume the user will provide a pio package with the functions that may be called within ProgramDefaultConfig. I'd then go ahead and rename the types and constants so as to remove their PIO prefix to avoid stutter:

  • so instead of machine.PIOStateMachineConfig -> pio.StateMachineConfig
  • machine.PIO_INSTR_NOP -> pio.INSTR_NOP

I suggest using these identifiers: https://github.com/soypat/rp2040-pio/blob/main/pio.go

Let me know what y'all think!

soypat avatar Oct 24 '23 23:10 soypat

I've modified the pioasm tool to the following:

// code above this line remains unmodified from kenbell's implementation
header(out, "Code generated by pioasm; DO NOT EDIT.");

        for (const auto &program : source.programs) {
            // todo maybe have some code blocks inside or outside here?
            for(const auto& o : program.code_blocks) {
                if (o.first == name) {
                    for(const auto &contents : o.second) {
                        fprintf(out, "%s", contents.c_str());
                    }
                }
            }

            header(out, program.name);

            std::string prefix = program.name;

            fprintf(out, "const %sWrapTarget = %d\n", prefix.c_str(), program.wrap_target);
            fprintf(out, "const %sWrap = %d\n", prefix.c_str(), program.wrap);
            fprintf(out, "\n");

            output_symbols(out, prefix, program.symbols);

            fprintf(out, "var %sInstructions = []uint16{\n", prefix.c_str());
            for (int i = 0; i < (int)program.instructions.size(); i++) {
                const auto &inst = program.instructions[i];
                if (i == program.wrap_target) {
                    fprintf(out, "\t\t//     .wrap_target\n");
                }
                fprintf(out, "\t\t0x%04x, // %2d: %s\n", inst, i,
                        disassemble(inst, program.sideset_bits_including_opt.get(), program.sideset_opt).c_str());
                if (i == program.wrap) {
                    fprintf(out, "\t\t//     .wrap\n");
                }
            }
            fprintf(out, "}\n");
            fprintf(out, "const %sOrigin = %d\n", prefix.c_str(), program.origin.get());
            
            fprintf(out, "func %sProgramDefaultConfig(offset uint8) pio.StateMachineConfig {\n", prefix.c_str());
            fprintf(out, "\tcfg := pio.DefaultStateMachineConfig()\n");
            fprintf(out, "\tcfg.SetWrap(offset+%sWrapTarget, offset+%sWrap)\n", prefix.c_str(),
                    prefix.c_str());
            if (program.sideset_bits_including_opt.is_specified()) {
                fprintf(out, "\tcfg.SetSideSet(%d, %s, %s)\n", program.sideset_bits_including_opt.get(),
                        program.sideset_opt ? "true" : "false",
                        program.sideset_pindirs ? "true" : "false");
            }
            fprintf(out, "\treturn cfg;\n");
            fprintf(out, "}\n\n");
        }
        
        output_symbols(out, "", source.global_symbols);

        if (out != stdout) { fclose(out); }
        return 0;
    }
};

static go_output::factory creator;

biohazduck avatar Oct 25 '23 17:10 biohazduck

@soypat - those changes look good to me (it's been a long while since I looked at the code).

I think the main thing is to really lock down the output of pioasm since that's in another project, so everything needs to remain backwards compatible. Your proposal solves that since there's no explicit dependency on any particular Go package.

A possible downside (fairly minor) is that we might end up with multiple implementations of Pio fragmenting the tinygo community. If we put the defacto default implementation under github.com/tinygo-org/XXXX then I think it addresses that nicely, and I assume most folks would use it by default.

kenbell avatar Oct 26 '23 15:10 kenbell

@kenbell I have created a PR against your repo with changes to pioasm. I've also taken the liberty of creating a new tinygo repo @ https://github.com/tinygo-org/pio/. I've tried to keep the API as robust as possible against possible future changes to hardware register access.

soypat avatar Oct 28 '23 03:10 soypat