Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.38%. Comparing base (83131c1) to head (bdf7613).
⚠️ Report is 7 commits behind head on master.

@@           Coverage Diff           @@
##           master   #17951   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files         171      171           
  Lines       22301    22301           
=======================================
  Hits        21940    21940           
  Misses        361      361           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Code size report:

Reference:  pyproject.toml: Update ruff configuration to target Python 3.8. [83131c1]
Comparison: esp32/mphalport: Enable D-cache flushing on P4 for native code. [merge of bdf7613]
  mpy-cross:    +0 +0.000% 
   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

Everything is completed except the forced download start reset. @projectgus

But now there is a problem, that is, if I plug in the usb of CDC and the default JTAG at the same time, JTAG will not be able to load the storage space, and it is bound to CDC.

@projectgus Please review again.

I wanted to touch base on the I2C stuff. The reason why I am suggesting a separate PR for it is if it will work with the P4 the way it is done currently it is easier to revert the I2C changes if they are made in their own PR should there be an issue with it. When I went looking at the code in the ESP-IDF I didn't see any block put into place that stop the old style of I2C from working with the P4.

It does need to be updated without a doubt but a separate PR would be the better if it is not needed to make the P4 work.

@Vincent1-python Please check that review comments are actually resolved before pressing "Resolve conversation". If you're not sure, then it's better to leave it unresolved!

@projectgus I soon found that if I delete the LDO driven by SD card, there will be many development boards that can't use SD card. I think I should add it back, which is not unique to my development board.

There's a difference between fence (CPU internal state) and cache flush (RAM state), at least on ARM Cortex. Maybe there's an IDF function to flush/invalidate RAM cache?

I didn't find anything obvious in ESP-IDF, if not mentions to plain "fence" instances in a few isolated places.

However, on RISC-V fence covers memory too:

FENCE instructions are used to order device I/O and memory accesses as viewed by other RISC-V harts and external devices or coprocessors. Any combination of device input (I), device output (O), memory reads (R), and memory writes (W) may be ordered with respect to any combination of the same. Informally, no other RISC-V hart or external device can observe any operation in the successor set following a FENCE before any operation in the predecessor set preceding the FENCE.

(hart == CPU core in RISC-V speak)

Something else to try could be the "FENCE / FENCE RW, RW / FENCE.I" sequence. "FENCE RW, RW" explicitly forces memory flushing if the default fencing model implementation is configured to only cover device i/o or other stuff, whilst "FENCE.I" forces a further flush on the next instruction fetch if the default fencing model implementation doesn't cover the instruction cache.

If you don't feel like looking into this any further, feel free to disable the emitter for now - I'll place an order for an ESP32P4 board in that case.

Just 2c, there is an esp_cache_msync function which can help perform cache writeback (docs).

Just 2c, there is an esp_cache_msync function which can help perform cache writeback

Ah, excellent! Thanks for the hint @igrr . I added a call to esp_cache_msync() and it now works!

I pushed a commit for that change to this PR.

(Rebased and force pushed.)

Now merged!

I suspect there may be some teething issues with this new chip, let's see how it goes now that everyone can start using it with MicroPython.

I'm getting this

ESP-ROM:esp32p4-eco2-20240710
Build:Jul 10 2024
rst:0x7 (HP_SYS_HP_WDT_RESET),boot:0x30f (SPI_FAST_FLASH_BOOT)
invalid header: 0x00707372
invalid header: 0x00707372
invalid header: 0x00707372
invalid header: 0x00707372
invalid header: 0x00707372
invalid header: 0x00707372

Possibly something I am doing wrong though as the build step shows

esptool.py v4.7.0
Serial port /dev/ttyACM0
Connecting....
Chip is ESP32-P4 (revision v0.0)
Features: High-Performance MCU
Crystal is 40MHz
MAC: 30:ed:a0:e3:31:ff
Uploading stub...
Running stub...
Stub running...
Configuring flash size...
WARNING: Flasher stub doesn't fully support flash size larger than 16MB, in case of failure use --no-stub.

A fatal error occurred: micropython.bin requires chip revision in range [v0.1 - v1.99] (this chip is revision v0.0). Use --force to flash anyway.

and I used force.

@exussum12 it looks like you have an early revision P4. You'll probably need to change the ESP32P4_REV_MIN option to ESP32P4_REV_MIN_0 (it defaults to ESP32P4_REV_MIN_1).

Or, get newer hardware.

Does --force not do basically that?

The hardware only arrived this week - from https://www.amazon.co.uk/Waveshare-ESP32-P4-ETH-High-Performance-Development-Pre-Solder/dp/B0FMJD25QP

Ill have a look at re-flashing now though

To update here, Working fine since changing that. Thanks!

I noticed that the MicroPython download page has added P4-related content, but the images are displaying abnormally. Could you please check these image links?

https://micropython.org/download/?vendor=Espressif

@igrr Thanks for the tip about the new cache functions! May I ask a follow-up about this:

A fatal error occurred: micropython.bin requires chip revision in range [v0.1 - v1.99] (this chip is revision v0.0). Use --force to flash anyway.

[...]
The hardware only arrived this week - from https://www.amazon.co.uk/Waveshare-ESP32-P4-ETH-High-Performance-Development-Pre-Solder/dp/B0FMJD25QP

Is it reasonable for us to follow the ESP-IDF default of ESP32P4_REV_MIN_1, or is it likely a lot of people will find they have rev 0 P4s?

Hi @projectgus! Unfortunately I can't tell exactly how many development boards have been made and sold with P4 rev.0. There has certainly been a number of Waveshare boards as well as Espressif's "Function EV board"s made with rev.0. For IDF, setting the default to rev. 1 is a reasonable choice, since users can always switch it back to rev. 0 in menuconfig. However for Micropython it might make sense to support rev. 0 as well in the same binary. I don't know of any downside of selecting ESP32P4_REV_MIN_0, other than the slightly increased code size.

I also want to give a heads-up that P4 revision 3.0 is not backward-compatible, meaning that it won't be possible to build a binary with support for both rev<3.0 and >=3.0. Therefore I would recommend introducing variants of binaries for rev<3 and >=3, and eventually removing rev<3 binaries when most of the boards on the market are rev>=3. The detailed information on distinguishing P4 chip revisions should appear in https://github.com/espressif/esp-chip-errata soon.

@eMUQI the downloads page has now been updated, the images are all there, and the P4 now has the C5 and C6 variants.

Thanks @igrr for the details about the revisions. It sounds like we should support rev 0.0, so I've opened a PR for that at #18512.

My testing shows that it makes pretty much no difference to the firmware when supporting rev 0.0, which is good!

I also want to give a heads-up that P4 revision 3.0 is not backward-compatible, meaning that it won't be possible to build a binary with support for both rev<3.0 and >=3.0. Therefore I would recommend introducing variants of binaries for rev<3 and >=3, and eventually removing rev<3 binaries when most of the boards on the market are rev>=3.

OK, thanks for the heads up! When rev 3.0 is out we will probably make that the default variant, and then a separate variant for rev<3.