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

SCANVIDEO fails while saving to flash from other core #26

Closed
Memotech-Bill opened this issue Sep 23, 2021 · 5 comments
Closed

SCANVIDEO fails while saving to flash from other core #26

Memotech-Bill opened this issue Sep 23, 2021 · 5 comments

Comments

@Memotech-Bill
Copy link

I have a multicore program in which core 1 generates VGA output using pico_scanvideo_dpi, while the other core may (on request) save data to Flash.

My render_loop is declared as __time_critical_func() and therefore is RAM resident (confirmed from the load map). However whenever Flash is written, video generation halts and does not resume until the Pico is reset.

Running under gdb-multiarch and doing a back-trace after the write to flash consistently shows:

`set init
target halted due to debug-request, current mode: Thread
xPSR: 0xf1000000 pc: 0x000000ee msp: 0x20041f00
target halted due to debug-request, current mode: Thread
xPSR: 0xf1000000 pc: 0x000000ee msp: 0x20041f00
(gdb) load
Loading section .boot2, size 0x100 lma 0x10000000
Loading section .text, size 0x31b30 lma 0x10000100
Loading section .rodata, size 0x6648 lma 0x10031c30
Loading section .ARM.exidx, size 0x8 lma 0x10038278
Loading section .binary_info, size 0x3c lma 0x10038280
Loading section .data, size 0x2b68 lma 0x100382bc
Start address 0x100001e8, load size 241188
Transfer rate: 77 KB/sec, 12694 bytes/write.
(gdb) c
Continuing.
^Ctarget halted due to debug-request, current mode: Handler HardFault
xPSR: 0x81000003 pc: 0x1166dfdc msp: 0x20041f58

Thread 1 received signal SIGINT, Interrupt.
0x11051d00 in ?? ()
(gdb) thread 2
[Switching to thread 2 (Thread 2)]
#0 0x1166dfdc in ?? ()
(gdb) bt
#0 0x1166dfdc in ?? ()
#1
#2 0x14000010 in ?? ()
#3 0x20000c76 in set_next_scanline_id (scanline_id=)
at /home/pi/pico/pico-extras/src/rp2_common/pico_scanvideo_dpi/scanvideo.c:669
#4 prepare_for_active_scanline_irqs_enabled ()
at /home/pi/pico/pico-extras/src/rp2_common/pico_scanvideo_dpi/scanvideo.c:787
#5 isr_irq7 () at /home/pi/pico/pico-extras/src/rp2_common/pico_scanvideo_dpi/scanvideo.c:932
#6
#7 0x20001532 in __wfe () at /home/pi/pico/pico-sdk/src/rp2_common/hardware_sync/include/hardware/sync.h:123
#8 scanvideo_begin_scanline_generation (block=block@entry=true)
at /home/pi/pico/pico-extras/src/rp2_common/pico_scanvideo_dpi/scanvideo.c:1085
#9 0x200005d8 in render_loop () at /home/pi/pico/BBCSDL_Work/mcu/pico_vga/picovdu.c:577
#10 0x1001b7d2 in setup_video () at /home/pi/pico/BBCSDL_Work/mcu/pico_vga/picovdu.c:680
#11 0x10022808 in core1_wrapper (entry=0x1001b761 <setup_video>, stack_base=)
at /home/pi/pico/pico-sdk/src/rp2_common/pico_multicore/multicore.c:89
#12 0x00000172 in ?? ()
`
Routine set_next_scanline_id () is always the first identifiable routine in the back-trace

static void set_next_scanline_id(uint32_t scanline_id) { shared_state.scanline.next_scanline_id = scanline_id; shared_state.scanline.y_repeat_target = _scanline_repeat_count_fn(scanline_id) * video_mode.yscale; }
I tried modifying the definition of this routine to:

static void __video_most_time_critical_func(set_next_scanline_id)(uint32_t scanline_id)

but this did not help. Since the function is declared as static it does not appear in the load map to confirm its location in Flash or RAM.

I can work around the problem by calling multicore_lockout_victim_init () on core 1, and wrapping the Flash write in multicore_lockout_start_blocking () and multicore_lockout_end_blocking (). That results in a noticable interruption to video generation while writing to Flash although it does at least resume once the write to Flash has completed. However if all the pico_scanvideo code is in RAM this should not be necessary.

My code is at https://github.com/Memotech-Bill/BBCSDL

@Memotech-Bill
Copy link
Author

Memotech-Bill commented Sep 24, 2021

I have found one problem.

From the back-trace:

#3 0x20000c76 in set_next_scanline_id (scanline_id=)

And from the load map:

            0x20000ad4      0x688 CMakeFiles/bbcbasic.dir/home/pi/pico/pico-extras/src/rp2_common/pico_scanvideo_dpi/scanvideo.c.obj
            0x20000ad4                isr_irq7

Taking the difference between the address in the back-trace and that from the map: 0x20000c76 - 0x20000ad4 = 0x1a2.

Running the command objdump -dSrwF scanvideo.c.obj gives:

    shared_state.scanline.y_repeat_target = _scanline_repeat_count_fn(scanline_id) * video_mode.yscale;
    19a:	4b56      	ldr	r3, [pc, #344]	; (2f4 <isr_irq7+0x2f4> (File Offset: 0x3e0))
    shared_state.scanline.next_scanline_id = scanline_id;
    19c:	61a0      	str	r0, [r4, #24]
    shared_state.scanline.y_repeat_target = _scanline_repeat_count_fn(scanline_id) * video_mode.yscale;
    19e:	681b      	ldr	r3, [r3, #0]
    1a0:	4798      	blx	r3
    shared_state.scanline.current_scanline_buffer = NULL;
    1a2:	2200      	movs	r2, #0

Since the address in the back-trace is the return address, the instruction causing the fault is probably the previous one, so the instruction causing the fault is blx r3, which looks as though it contains the address of _scanline_repeat_count_fn.

Searching pico_scanvideo_dpi/scanvideo.c for "repeat_count_fn" gives:

    static uint default_scanvideo_scanline_repeat_count_fn(uint32_t scanline_id) {
        return 1;
    }
         :
         :
    void scanvideo_set_scanline_repeat_fn(scanvideo_scanline_repeat_count_fn fn) {
        _scanline_repeat_count_fn = fn ? fn : default_scanvideo_scanline_repeat_count_fn;
    }

And from the load map:

     .text.default_scanvideo_scanline_repeat_count_fn
                    0x10022750        0x4 CMakeFiles/bbcbasic.dir/home/pi/pico/pico-extras/src/rp2_common/pico_scanvideo_dpi/scanvideo.c.obj

So default_scanvideo_scanline_repeat_count_fn is in Flash, hence the crash.

I have modified the definition of this function in scanvideo.c to put it into RAM:

    static uint __video_time_critical_func(default_scanvideo_scanline_repeat_count_fn)(uint32_t scanline_id) {
        return 1;
    }

This certainly improves the code but I am still getting occasional crashes of the video generation when saving to Flash, so there is still another issue to resolve.

@lurch
Copy link
Contributor

lurch commented Sep 24, 2021

I guess this is tangentially related to raspberrypi/pico-sdk#410 wherein the use of __not_in_flash_func means that you have to recursively find and annotate all functions that might get called, to ensure that they ALL get loaded into RAM?

@Memotech-Bill
Copy link
Author

I have now demonstrated that core 0 flushing the XIP cache corrupts VGA generation running entirely from RAM on core 1. For details please see https://www.raspberrypi.org/forums/viewtopic.php?f=144&t=320355#p1919396

@Memotech-Bill
Copy link
Author

Finally found the problem. Although the code is entirely in RAM it was referencing data stored in Flash.

@lurch
Copy link
Contributor

lurch commented Oct 2, 2021

Glad to hear that you managed to get to the bottom of it 👍

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

No branches or pull requests

2 participants