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

ports/esp32 add support for the ulp #3578

Closed
wants to merge 10 commits into from
Closed

Conversation

andynd
Copy link

@andynd andynd commented Jan 21, 2018

add machine.ULP() with functions load_binary, set_wakeup_period and run.
add constant machine.ULP().COPROC_RESERVE_MEM
add README.ulp.md with ulp examples

add machine.ULP() with functions load_binary, set_wakeup_period and run.
add constant machine.ULP().COPROC_RESERVE_MEM
add README.ulp.md with ulp examples
@andynd andynd changed the title add support for the esp32 ulp ports/esp32 add support for the ulp Feb 4, 2018
@andynd
Copy link
Author

andynd commented Feb 27, 2018

Any plans to merge this? Pull request was created more than a month ago.

@mattytrentini
Copy link
Sponsor Contributor

This is the PR we discussed recently that could add basic support for ULP on the ESP32 @dpgeorge. I haven't yet tested it but it's a very thin wrapper around the ESP-IDF to allow ULP binaries to be uploaded through Micropython.

*/

#ifndef MICROPY_INCLUDED_ESP32_MACINE_ULP_H
#define MICROPY_INCLUDED_ESP32_MACINE_ULP_H

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes


u = machine.ULP()
f = open('test.bin', 'rb')
b = f.read()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use "with" syntax to close the file automatically?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, didn't care for an example tbh.

@@ -1,6 +1,6 @@
#define CONFIG_TRACEMEM_RESERVE_DRAM 0x0
#define CONFIG_BT_RESERVE_DRAM 0x0
#define CONFIG_ULP_COPROC_RESERVE_MEM 0
#define CONFIG_ULP_COPROC_RESERVE_MEM 4096

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this practically mean? what are the valid limits to this value?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's 8kB of RTC SLOW memory in the ESP32, and the lower half of it is available to users. These 4kB are divided between space for variables placed into .rtc.data/.rtc.rodata by the linker (via RTC_DATA_ATTR/RTC_RODATA_ATTR attributes), and the area reserved for the ULP.

So practically you need to set this to 4096 minus the size of RTC_DATA_ATTR/RTC_RODATA_ATTR variables allocated by micropython.

# mem[R0+0] = R3
st R3, R0, 0

HALT

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could there be a more practical example (without getting too long / too complex), where the python code feeds some data to the ulp program and also fetches from data back from it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not easily. It is possible to access the slow mem from the upstream using magic values and normal memory access. We use a machine.RTC().slowmem[addr] construct internally, but it is not upstream and we do not plan to publish it. That is why i choose a very limited example.

@ThomasWaldmann
Copy link

Related: https://github.com/ThomasWaldmann/py-esp32-ulp - feedback welcome.

@andynd
Copy link
Author

andynd commented Mar 19, 2018

I see a usecase for your pure python approche as well as using a "normal" compiler:

  • sometimes I do not want to ship the code
  • C preprocessor macros are great
  • Code can be more readable if you can use includes etc
  • custom linker script
  • if you have a 20 instruction programm, it can be totaly fine not to have all of this
  • variable access from mp: you need a fixed position to be able to access a variable in memory from python

@ThomasWaldmann
Copy link

My assembler (see above) is close to being actually useful, can we merge this PR soon, so the infrastructure needed is also there?

@andynd
Copy link
Author

andynd commented Mar 25, 2018

Updated to current master, updated readme, fixed a typo... What is still needed to be able to merge this?

with open('test.bin', 'rb') as f:
b = f.read()
u.load_binary(0,b)
u.run(0)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these TABs instead of 4 spaces?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, maybe outdent the load and run, so it is not within the context manager (you already have what you need in b, so the file can be already closed before load/run).

@mattytrentini
Copy link
Sponsor Contributor

Heya @andynd and @ThomasWaldmann I'm going to try and test this over the next couple of days and, assuming it works ok, I'll lean on @dpgeorge to merge.

@andynd
Copy link
Author

andynd commented Apr 5, 2018

@mattytrentini hey, how is it going? Any more clarifications needed?

*
* The MIT License (MIT)
*
* Copyright (c) 2017 "Eric Poulsen" <eric@zyxod.com>
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should replace Eric's name with your own

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jup i should. fixed.

@mattytrentini
Copy link
Sponsor Contributor

Sorry for the lack of updates @andynd. It was easy enough to build with your changes and the code looks like a nice clean wrapper around the ESP-IDF. It's structured as I would expect ('ULP' module in machine, methods matching the ESP-IDF).

I'm now at the point of actually trying to do something useful by using the ULP and this is where I'm starting to struggle.

There's a fair bit to pull together; installing the ULP toolkit, writing some ULP assembly, compiling it and creating a python example that loads and interacts with the ULP. Although your code looks fine - and could probably be integrated as-is - I feel that we're currently setting a pretty high barrier to entry for someone to use the ULP from MicroPython, mostly due to a lack of documentation and examples.

For example, set_wakeup_period isn't documented, nor is it clear how to drive the processor into deep sleep mode - a very common use-case for ULP usage. It's also not clear how the entry point (the undocumented first parameter to load_binary and run) operate in MicroPython - most C examples refer to RTC_SLOW_MEM and an address defined in the ULP assembly, ulp_entry. Your example sets a 0 offset which is fine but doesn't seem common.

Since we're not providing much documentation about the ULP in general at this stage, we should probably at least also include a link to the ESP-IDF ULP documentation.

All that said, I'm ok with this being merged - it's exposing the ESP-IDF neatly! - but I think that we should consider this a start and build on it, particularly with examples and documentation. Of course, that doesn't have to come from you @andynd and I'm grateful you've taken the initiative to expose the ULP functionality.

@dpgeorge do you agree? Are you ok with merging this?

@ThomasWaldmann
Copy link

@mattytrentini I plan to add some examples to the py-esp32-ulp project as soon as somebody has tested that they actually work. The assembler has just been tested with a few kB long source code and was able to translate that to machine code on the ESP32 (not much more though due to memory constraints).

A first example could be just some ulp_blink.S (bonus points for having a green led blinking to show the ULP is doing something and maybe a red led reflecting main cpu being powered or not).

For actual interaction / data exchange main cpu <-> ULP I'ld like to get some feedback there what could be done with the assembler symbol table to have some comfortable way later to access stuff from micropython.

@ThomasWaldmann
Copy link

Merge this to get things started?

@andynd
Copy link
Author

andynd commented Apr 25, 2018

ping

@dpgeorge
Copy link
Member

Very sorry for the slow response on this PR. Thanks for you work on it @andynd and @ThomasWaldmann. In general it looks good but there are a few points I have. The main one being that, because the ULP is such an ESP32-specific thing, the class should go in the esp32 module.

@@ -151,6 +152,7 @@ SRC_C = \
modesp32.c \
moduhashlib.c \
espneopixel.c \
machine_ulp.c \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting the ULP class in the esp32 module would mean this file becomes esp32_ulp.c.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you, the esp module is probably a better place for this code, I will move it.

*
* The MIT License (MIT)
*
* Copyright (c) 2018 "Andreas Valder" <andreas.valder@serioese.gmbh>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check: please verify that you have the authority to release this code under MIT with your copyright (eg you didn't do it during employed hours), and that you didn't copy any of the code from somewhere else.

Copy link
Author

@andynd andynd Apr 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have done it during employed hours and we are using the code internally. Upstreaming our changes to open source projects is part of our policy and I get paid to do so. None of it is copied.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that sounds fine, thanks.

{ MP_OBJ_NEW_QSTR(MP_QSTR_set_wakeup_period), (mp_obj_t)&machine_ulp_set_wakeup_period_obj },
{ MP_OBJ_NEW_QSTR(MP_QSTR_load_binary), (mp_obj_t)&machine_ulp_load_binary_obj },
{ MP_OBJ_NEW_QSTR(MP_QSTR_run), (mp_obj_t)&machine_ulp_run_obj },
{ MP_OBJ_NEW_QSTR(MP_QSTR_COPROC_RESERVE_MEM), MP_ROM_INT(CONFIG_ULP_COPROC_RESERVE_MEM) },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this constant is needed/useful a script can check how much room is left to load a ULP binary, right?

Since the constant lives in the ULP class already it could be simplified to RESERVE_MEM.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct. I wanted to keep the same name that is used in the esp-idf but if you prefere a shorter name I can for sure change it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

COPROC_RESERVE_MEM is already a shortened version of the name so I think it's fair to shorten it further to RESERVE_MEM. It doesn't lose any of it's meaning and shorter names are easier to type, take less space in flash and in scripts, etc.

#ifndef MICROPY_INCLUDED_ESP32_MACHINE_ULP_H
#define MICROPY_INCLUDED_ESP32_MACHINE_ULP_H

#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is not needed. Any exported types/functions/etc can go in modesp32.h (assuming the class is moved to esp32.ULP).

@@ -238,6 +239,7 @@ STATIC const mp_rom_map_elem_t machine_module_globals_table[] = {
{ MP_ROM_QSTR(MP_QSTR_I2C), MP_ROM_PTR(&machine_i2c_type) },
{ MP_ROM_QSTR(MP_QSTR_PWM), MP_ROM_PTR(&machine_pwm_type) },
{ MP_ROM_QSTR(MP_QSTR_RTC), MP_ROM_PTR(&machine_rtc_type) },
{ MP_ROM_QSTR(MP_QSTR_ULP), MP_ROM_PTR(&machine_ulp_type) },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned, I think it makes more sense to put this class in the esp32 module, which is for ESP32-specific features. It's unlikey any other port would have a ULP.

@@ -15,6 +15,7 @@ extern const mp_obj_type_t machine_pin_type;
extern const mp_obj_type_t machine_touchpad_type;
extern const mp_obj_type_t machine_adc_type;
extern const mp_obj_type_t machine_dac_type;
extern const mp_obj_type_t machine_ulp_type;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would go in modesp32.h

@@ -5,7 +5,7 @@

#define CONFIG_TRACEMEM_RESERVE_DRAM 0x0
#define CONFIG_BT_RESERVE_DRAM 0x0
#define CONFIG_ULP_COPROC_RESERVE_MEM 0
#define CONFIG_ULP_COPROC_RESERVE_MEM 2040
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there a reason for the value 2040? Why not 2048? (The default value from the ESP IDF is 512.)

Also, isn't in necessary to define ULP_COPROC_ENABLED?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ULP_COPROC_ENABLED seems to be ignored by the current build system, but for consistency we should probably set it. I choose the largest posible value. 2048 is not posible because micropython it self uses a part of the rtc slow memory already. Sadly I found no way to calculate the remaining space there and had to hard code it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should set ULP_COPROC_ENABLED to 1 (it seems to be used by esp_sleep_enable_ulp_wakeup).

For the RESERVE_MEM value: ultimately I think the best solution is to overlap the ULP reserved mem with the machine.RTC().memory() feature so that the RTC class can be used to read/write into the ULP and they can talk to each other. So, for example, they'd both use the first 2048 bytes of RTC slow memory. Then it's up to the user to decide how much is used by each one. This can be done quite easily by forcing rtc_user_mem_data to be located at 0x50000000. But this can be done later. For now, 2040 is a good enough value for the RESERVE_MEM (but it may change in the future).

STATIC MP_DEFINE_CONST_FUN_OBJ_2(machine_ulp_run_obj, machine_ulp_run);

STATIC const mp_map_elem_t machine_ulp_locals_dict_table[] = {
{ MP_OBJ_NEW_QSTR(MP_QSTR_set_wakeup_period), (mp_obj_t)&machine_ulp_set_wakeup_period_obj },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please can you use the following construct for ROM tables: { MP_ROM_QSTR(MP_QSTR_set_wakeup_period), MP_ROM_PTR(&machine_ulp_set_wakeup_period_obj) },

Copy link
Author

@andynd andynd Apr 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that won't work and i don't see why:

../../py/obj.h:242:23: error: initialization discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
 #define MP_ROM_PTR(p) (p)
                       ^
esp32_ulp.c:86:47: note: in expansion of macro 'MP_ROM_PTR'
     { MP_ROM_QSTR(MP_QSTR_set_wakeup_period), MP_ROM_PTR(&esp32_ulp_set_wakeup_period_obj) },

everything else is changed

@dpgeorge
Copy link
Member

dpgeorge commented May 1, 2018

Thanks for making the updates. Now merged in 298c072

@andynd BTW, the problem with MP_ROM_PTR was that the dict needed to be mp_rom_map_elem_t, not mp_map_elem_t (I fixed this during the merged).

@dpgeorge dpgeorge closed this May 1, 2018
@ThomasWaldmann
Copy link

Yay, looks like we can get ULP stuff going now.

To all of you playing with the ULP now: we could need some hello-world / blinking led examples for py-esp32-ulp project and also more comfortable integration (so we can use the symbol table somehow instead of using hardcoded offsets into ulp memory), so please help with that.

https://github.com/ThomasWaldmann/py-esp32-ulp

@ThomasWaldmann
Copy link

Is it possible to read from ULP memory using micropython? How?

From the PR comments, it sounds like main cpu and ulp cpu can not exchange data yet?

@andynd
Copy link
Author

andynd commented May 1, 2018

They can, you can access the RTC slow memory at memory offset RTC_SLOW_MEM (0x50000000)

@ThomasWaldmann
Copy link

And how do I do that from Python?

@mattytrentini
Copy link
Sponsor Contributor

My understanding is that the CPU and ULP can access the RTC memory using the RTC module. Something like:

rtc = machine.RTC()
mem = rtc.memory() # Read from RTC memory
rtc.memory(b'Dummy') # Write to RTC memory

@dpgeorge
Copy link
Member

dpgeorge commented May 2, 2018

My understanding is that the CPU and ULP can access the RTC memory using the RTC module. Something like:

The RTC.memory() function and the ULP access different, non-overlapping areas of the slow memory. It would be good to make them overlap for easy communication between the two. But for now you can use:

machine.mem32[0x50000000 + offset] = value # write 32-bit number to ULP memory region
value = machine.mem32[0x50000000 + offset] # read ULP memory region

@ThomasWaldmann
Copy link

ThomasWaldmann commented May 2, 2018

https://github.com/ThomasWaldmann/py-esp32-ulp/tree/master/examples

yay, it works! please add more examples via PR. :)

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.

None yet

5 participants