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

bcm2708_fb_pan_display is not implemented #679

Closed
rst- opened this issue Sep 2, 2014 · 19 comments
Closed

bcm2708_fb_pan_display is not implemented #679

rst- opened this issue Sep 2, 2014 · 19 comments

Comments

@rst-
Copy link

rst- commented Sep 2, 2014

This would be required for implementing non-tearing animation using page flipping based on a double height virtual buffer. It appears that the mailbox interface supports this already ('Set virtual offset' https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface#set-virtual-offset ) and only the fb driver wrapper would be needed.

Discussion on the issue (the original title is something slightly different) http://www.raspberrypi.org/forums/viewtopic.php?f=67&t=19073 (example code in http://www.raspberrypi.org/forums/viewtopic.php?f=67&t=19073#p231052).

@rst-
Copy link
Author

rst- commented Sep 3, 2014

Hi,

The code in the forum post mentioned in the ticket is in fact a simple test
app for this. Good enough?

Regs,

J-P "rst-"

On 3 September 2014 11:38, popcornmix notifications@github.com wrote:

Can you provide a simple test app that makes use of this interface?


Reply to this email directly or view it on GitHub
#679 (comment).

@rst-
Copy link
Author

rst- commented Sep 3, 2014

Just in case...

On 3 September 2014 11:59, Juho-Pekka Rosti jprosti@iki.fi wrote:

Hi,

The code in the forum post mentioned in the ticket is in fact a simple
test app for this. Good enough?

Regs,

J-P "rst-"

On 3 September 2014 11:38, popcornmix notifications@github.com wrote:

Can you provide a simple test app that makes use of this interface?


Reply to this email directly or view it on GitHub
#679 (comment).

@maxnet
Copy link
Contributor

maxnet commented Sep 3, 2014

rst, you might want to try my old patch: https://raw.githubusercontent.com/maxnet/berryboot/berryboot2.0/buildroot-2012.05/bcm2708_fb-add-pan-support.patch

Not sure if the mailbox thing is the right approach though.
It does work under normal usage, but it seems kinda fragile if something on the VC side of things is wrong.
Had instances where the call to bcm_mailbox_property() never returned and hanged the system, after playing with code that accesses V3D registers directly.

@popcornmix
Copy link
Collaborator

I've got pan and wait for vsync implemented.
I've also tweaked it so FBIOPUT_VSCREENINFO doesn't clear the framebuffer when the allocated size doesn't change.
Also attempted to make the blank command disable the HDMI output (so monitor is more likely to power-save).

The test app seems to work. If I don't find any problems tomorrow, I'll push it out.

@trevd
Copy link

trevd commented Sep 3, 2014

I had a patch also which was in the same vein as @maxnet patch.
@popcornmix That sounds fantastic. any chance I could give that patch a test?

popcornmix added a commit to raspberrypi/firmware that referenced this issue Sep 4, 2014
See: raspberrypi/linux#680

kernel: I2C: Only register the I2C device for the current board revision
See: http://www.raspberrypi.org/forums/viewtopic.php?f=43&t=84000

kernel: power: Add defines for contolling DSI power through mailbox interface
kernel: vcio: Fix incorrect and add new mailbox tags
kernel: bcm2708_fb: Add pan and vsync controls
See: raspberrypi/linux#679

firmware: Improved editing of DT node
This now will create a new node if it doesn't exist, and now appends to bootargs rather than replacing.
Note: padding is still required

firmware: platform: Add support for generic DPI display
More info soon

firmware: i2c: Make sure that sda pins and scl pins are set correctly when the I2C driver is opened

firmware: arm_loader: Allow DSI power to be controlled from arm

firmware: arm_display: Power off hdmi output when framebuffer interface is blanked

firmware: arm_display: Make syncs optional on dispman updates and add a mailbox option to wait for vsync
firmware: arm_display: Avoid freeing the framebuffer when allocated size doesn't change

hello_pi: hello_fft: Update to latest version from Andrew
See: http://www.aholme.co.uk/GPU_FFT/Main.htm
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this issue Sep 4, 2014
See: raspberrypi/linux#680

kernel: I2C: Only register the I2C device for the current board revision
See: http://www.raspberrypi.org/forums/viewtopic.php?f=43&t=84000

kernel: power: Add defines for contolling DSI power through mailbox interface
kernel: vcio: Fix incorrect and add new mailbox tags
kernel: bcm2708_fb: Add pan and vsync controls
See: raspberrypi/linux#679

firmware: Improved editing of DT node
This now will create a new node if it doesn't exist, and now appends to bootargs rather than replacing.
Note: padding is still required

firmware: platform: Add support for generic DPI display
More info soon

firmware: i2c: Make sure that sda pins and scl pins are set correctly when the I2C driver is opened

firmware: arm_loader: Allow DSI power to be controlled from arm

firmware: arm_display: Power off hdmi output when framebuffer interface is blanked

firmware: arm_display: Make syncs optional on dispman updates and add a mailbox option to wait for vsync
firmware: arm_display: Avoid freeing the framebuffer when allocated size doesn't change

hello_pi: hello_fft: Update to latest version from Andrew
See: http://www.aholme.co.uk/GPU_FFT/Main.htm
@popcornmix
Copy link
Collaborator

Try running rpi-update (there is a gpu firmware change and well as the linux change) and report back on how it behaves.

@rst-
Copy link
Author

rst- commented Sep 4, 2014

Great! I will try my best to do this as soon as possible.

On 4 September 2014 17:22, popcornmix notifications@github.com wrote:

Try running rpi-update (there is a gpu firmware change and well as the
linux change) and report back on how it behaves.


Reply to this email directly or view it on GitHub
#679 (comment).

@trevd
Copy link

trevd commented Sep 4, 2014

I've added the changes to my 3.16 branch. The good news is that the blanking is now putting the HDMI into display into power saving mode. The bad news is there seems to be a fair bit of tearing when using both the pan ioctl or the VCMSG_SET_VIRTUAL_OFFSET mailbox ioctl . I'm using a modified version @rst- fbtestXIII.c . [ https://gist.github.com/trevd/0a10e86ef91264636fd6 ] for the ioctl test and @rst- original version for the mailbox.

The issue appears to be in the firmware changes as revert back to the previous version "fixes" the mailbox direct test but obviously leave the ioctl test non functional.

Hope That Helps :)

I'v not had chance to test the VSYNC yet

@popcornmix
Copy link
Collaborator

I had to remove the vsync blocking from inside the pan call, otherwise code that calls pan and vsync (which I belive is the correct way of getting tear-free updates) can only manage half the update rate (i.e. 60fps updates become impossible).

So yes, if you update and call pan without calling vsync then I believe tearing is to be expected.

@trevd
Copy link

trevd commented Sep 4, 2014

facepalm lol. of course it's going to tear if it's not vsync'd .... I'll get my coat! :)

@rst-
Copy link
Author

rst- commented Sep 4, 2014

Based on a quick test this seems to work fine now. Will do some more testing during the weekend and close then if no surprises. Thanks!

@rst-
Copy link
Author

rst- commented Sep 5, 2014

Well... yes, the PAN_DISPLAY (and PUT_VSCREENINFO/yoffset) works but I cannot seem to get the vsync working. And it appears that also the mailbox version has lost the vsync (I assume this is what @popcornmix refers to with "I had to remove the vsync blocking from inside the pan call").

The way to use these would be in pseudo code:

  1. Display page 0
  2. Draw to page 1
  3. Wait for vsync event to fire
  4. During vsync flip to display the page 1

I would assume the correct way for 3&4 with the Linux fb driver ioctls:

if (ioctl(fbfd, FBIO_WAITFORVSYNC, 0) == 0) {
    vinfo.yoffset = vinfo.yres;
        if (ioctl(fbfd, FBIOPAN_DISPLAY, &vinfo)) {
           printf("Pan failed.\n");
        }
}
else {
  printf("VSync failed.\n");
}

or alternatively if the wait for vsync is implemented inside the pan:

    vinfo.yoffset = cur_page * vinfo.yres;
    vinfo.activate = FB_ACTIVATE_VBL;
    if (ioctl(fbfd, FBIOPAN_DISPLAY, &vinfo)) {
        printf("Error panning display.\n");
    }

Not sure if the first one would return fast enough to allow the pan to happen during the vsync ...and is the vsync even happening in real life with the dot matrix displays... It seemed to work fine using the mailbox interface on previous Raspbian version.

Might have been my mistake to use too simple example (not prone to tearing) - better one would probably be https://github.com/rst-/raspberry-compote/blob/master/fb/fbtestXII.c where there is both the ioctl (commented out) and the mailbox pan (lines 170-180). In case you get a chance to look at this again.

Or should I close this one as the PAN is now implemented and open a new one for the vsync?

Thanks for the effort so far anyway!

@maxnet
Copy link
Contributor

maxnet commented Sep 5, 2014

    vinfo.yoffset = cur_page * vinfo.yres;
    vinfo.activate = FB_ACTIVATE_VBL;
    if (ioctl(fbfd, FBIOPAN_DISPLAY, &vinfo)) {
        printf("Error panning display.\n");
    }

My understanding is that by specifying the FB_ACTIVATE_VBL flag you are indicating that the change should be scheduled to happen at next vsync.
But the ioctl call itself should return straight away and not block until next vsync.
Can use FBIO_WAITFORVSYNC afterwards if you want your program to block till next vsync as well.

@popcornmix
Copy link
Collaborator

What's the situation with this issue?
Are we happy with PAN behaviour?
Are we happy with WAITFORVSYNC behaviour?

If there's a problem can someone provide example code and explain what the correct behaviour should be.

@rst-
Copy link
Author

rst- commented Jan 5, 2015

I am happy with the PAN.

Not happy with the WAITFORVSYNC - does not seem to work using either the ioctl (code snippets in my previous reply) or more importantly no longer with the mailbox iface (https://github.com/rst-/raspberry-compote/blob/master/fb/fbtestXII.c - this produced a smooth display before the changes but now produces a lot of tearing).

Let me know if you need more info on this.

@popcornmix
Copy link
Collaborator

Boreal seems happy VSYNC is working with fbtestXIII:
http://www.raspberrypi.org/forums/viewtopic.php?f=67&t=19073&start=25#p666016

@rst-
Copy link
Author

rst- commented Jan 6, 2015

Hmm, interesting - so the only combination I did not come to think of (ioctl vsync + mailbox pan)...

I am fairly sure I tried this https://github.com/rst-/raspberry-compote/blob/master/fb/fbtestXI.c changing the 'switch page' section to:

        // switch page
        vinfo.yoffset = cur_page * vinfo.yres;
        if (ioctl(fbfd, FBIO_WAITFORVSYNC, 0) == 0) {
            if (ioctl(fbfd, FBIOPAN_DISPLAY, &vinfo)) {
                printf("Error panning display.\n");
            }
        }
        else {
            printf("VSync failed.\n");
        }
        
        //usleep(1000000 / fps);

...and it did not work as expected but I may have made a mistake of course.

I will try to test this in the evening - if someone manages to find time earlier, I don't mind ;)

@maxnet
Copy link
Contributor

maxnet commented Jan 6, 2015

Hmm, interesting - so the only combination I did not come to think of (ioctl vsync + mailbox pan)...

No, reversed.
boreal is using mailbox pan first, ioctl wait_for_vsync after that.

That is pretty much the same I suggested you could use earlier on this page.
FB_PAN_DISPLAY with FB_ACTIVATE_VBL set first, FBIO_WAIT_FOR_VSYNC second.

@rst-
Copy link
Author

rst- commented Jan 6, 2015

Yes, noticed that he had it reversed - which puzzles me a bit: I would have thought you wait for the vsync (to start) and then during that (when the screen is not updated) you flip the display using the pan or alternatively using the FB_ACTIVATE_VBL flag (which in fact might be the best option in the end) with the pan force the flip to happen during the vsync.

Well, I managed to test this now myself too: both the ioctl pan and the mailbox pan with the FBIO_WAIT_FOR_VSYNC ioctl right after seem to work fine! So (comment out/in to try options):

        // switch page
        /**/
    // ioctl version
        vinfo.yoffset = cur_page * vinfo.yres;
        ioctl(fbfd, FBIOPAN_DISPLAY, &vinfo);
        ioctl(fbfd, FBIO_WAITFORVSYNC, 0);
        /**/
        /*
        // mailbox version
        vx = 0;
        vy = cur_page * vinfo.yres;
        set_fb_voffs(&vx, &vy);
        ioctl(fbfd, FBIO_WAITFORVSYNC, 0);
        */

Tested both on the upgraded 3.12.26 (Thu Sep 4) build and the very latest clean download 3.12.35 (Fri Dec 19).

I think this is good enough for closing the issue.

@rst- rst- closed this as completed Jan 6, 2015
neuschaefer pushed a commit to neuschaefer/raspi-binary-firmware that referenced this issue Feb 27, 2017
See: raspberrypi/linux#680

kernel: I2C: Only register the I2C device for the current board revision
See: http://www.raspberrypi.org/forums/viewtopic.php?f=43&t=84000

kernel: power: Add defines for contolling DSI power through mailbox interface
kernel: vcio: Fix incorrect and add new mailbox tags
kernel: bcm2708_fb: Add pan and vsync controls
See: raspberrypi/linux#679

firmware: Improved editing of DT node
This now will create a new node if it doesn't exist, and now appends to bootargs rather than replacing.
Note: padding is still required

firmware: platform: Add support for generic DPI display
More info soon

firmware: i2c: Make sure that sda pins and scl pins are set correctly when the I2C driver is opened

firmware: arm_loader: Allow DSI power to be controlled from arm

firmware: arm_display: Power off hdmi output when framebuffer interface is blanked

firmware: arm_display: Make syncs optional on dispman updates and add a mailbox option to wait for vsync
firmware: arm_display: Avoid freeing the framebuffer when allocated size doesn't change

hello_pi: hello_fft: Update to latest version from Andrew
See: http://www.aholme.co.uk/GPU_FFT/Main.htm
pfpacket pushed a commit to pfpacket/linux-rpi-rust that referenced this issue Apr 7, 2023
rust: define `LockInfo` trait that lock "type states" must implement
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

4 participants