vc_dispmanx_update_submit: callback function is execute immediately, not on completion

[vanfanel]

Hi,

vc_dispmanx_update_submit() takes a callback function that is supossed to be called on completion. So if, for example, I do:

/* Issue a page flip at the next vblank interval. */
_dispvars->update = vc_dispmanx_update_start(0);

vc_dispmanx_element_change_source(_dispvars->update, _dispvars->element, _dispvars->resources[page->numpage]);

vc_dispmanx_update_submit(_dispvars->update, (DISPMANX_CALLBACK_FUNC_T)dispmanx_update_callback(), page);

printf ("**Next call");

where the callback function is simply:

DISPMANX_CALLBACK_FUNC_T dispmanx_update_callback (){
        printf ("**Entering CB\n"); 
}

then I get this output:

**Entering CB
**Next call

...but I should get "**Next call" first, since the update won't be "completed" until the next vsync arrives. I allways get the wrong order (just in case one could think we could be VERY near to an vsync when the pageflip is issue and that could be confusing).

What am I missing here?

thanks

[popcornmix]

It may be a threading / buffering artefact of printf.
Instead of printf's can you set a (global variable) flag before the vc_dispmanx_update_submit, reset it in the callback, and assert(flag) just after vc_dispmanx_update_submit.

If the callback is really occurring immediately the assert will fail as flag will be zero.

[vanfanel]

Yes, I did just what you said, and the assert fails.
Hence the callback is executed immediately. You can try yourself: I'm using latest firmware/sdk.
I really need this to work, can it be easily fixed from your side?

[vanfanel]

It seems Chips from the Raspberry Pi forums has found a possible candidate commit that started causing this:

raspberrypi/userland@66338d3

So, has the callback function behaviour been modified somehow? How should it be used now?
There's more software failing because of this, like UAE4ALL.

[popcornmix]

Are you calling vc_dispmanx_vsync_callback?

[vanfanel]

Nope, I am not. I've seen it in /opt/vc/include/interface/vmcs_host/vc_dispmanx.h with the new firmware.
It seems to accept an vsync callback function that will be called on every vsync.
How is that related to vc_dispmanx_update_submit()? I understan an update ends with the vsync, so both vc_dispmanx_update_submit() with a callback function as an argument and vc_dispmanx_vsync_callback() seem to be doing the same thing!
Should I call vc_dispmanx_vsync_callback() every time I call vc_dispmanx_update_submit()? Why? I am already passing a callback function to vc_dispmanx_update_submit()... It makes no sense in my head. Coud you explain it, please?

[popcornmix]

No you shouldn't be calling vc_dispmanx_vsync_callback, but the commit you indicated was vc_dispmanx_vsync_callback related which was why I asked.

[vanfanel]

Ok, so is there an issue here or it's me missing some detail?

[popcornmix]

Can you post your test code?

[vanfanel]

Of course. This is the callback function:

DISPMANX_CALLBACK_FUNC_T dispmanx_update_callback (){ 
        var = 0;
        return 0;
}

And this is how I call submit sync and assert on var, which is a global variable:

var = 1;
vc_dispmanx_update_submit(_dispvars->update (DISPMANX_CALLBACK_FUNC_T)dispmanx_update_callback(_dispvars->update, data), NULL);
assert (var);

Assert fails.

[popcornmix]

Do you have a complete test program?

[vanfanel]

Yes. There it is:

http://paste.debian.net/144761/

[popcornmix]

You were passing:

    vc_dispmanx_update_submit(_dispvars->update, (DISPMANX_CALLBACK_FUNC_T)dispmanx_update_callback(), NULL);

which is the return value of void function which is evaluating as zero. Try

    vc_dispmanx_update_submit(_dispvars->update, dispmanx_update_callback, NULL);

[vanfanel]

If I do that, I get:

main.c: In function ‘dispmanx_flip’:
main.c:139:2: warning: passing argument 2 of ‘vc_dispmanx_update_submit’ from incompatible pointer type [enabled by default]
In file included from /opt/vc/include/bcm_host.h:50:0,
                 from main.c:1:
/opt/vc/include/interface/vmcs_host/vc_dispmanx.h:101:12: note: expected ‘DISPMANX_CALLBACK_FUNC_T’ but argument is of type ‘void (* (*)())(DISPMANX_UPDATE_HANDLE_T,  void *)’

and the callback is never called...
Did you try that?

[popcornmix]

The callback functions needs to look like:

void dispmanx_update_callback (DISPMANX_UPDATE_HANDLE_T u, void * arg){
    var = 0;
}

to fix the warning. There is still another bug...

[vanfanel]

Yes, doing that fixes the warning, but the callback function is never called.

[vanfanel]

@popcornmix: Do you mean there's still a bug on my test program or there is a bug on the API? This really doesn't work as expected.

[popcornmix]

Probably a firmware bug - @pelwell is just investigating.

[vanfanel]

Great. I hope to hear from him, then 👍

[pelwell]

Before the userland commit mentioned above, the callback was called correctly but the handle callback parameter would always have been zero. The commit made that error more serious, because it had the effect of interpreting all of those update notifications as vsync notifications.

I've pushed a patch to userland that fixes the immediate problem and makes the callbacks work again. There is still a problem with vsyncs multiplying with each run, but that will require a firmware change.

[pelwell]

I've now finished a companion patch for the firmware that cancels vsync callbacks when the client disconnects. That should be released soon, along with rebuilt userland libraries, provided they pass the rigorous QA process.

[popcornmix]

Please run rpi-update and test.

[vanfanel]

pelwell & popcornmix: Seems to work as expected now!