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

Fix mismatch screen width and height and buffer synchronization when using Linux framebuffer backend #62

Open
a1091150 opened this issue Oct 9, 2024 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@a1091150
Copy link
Contributor

a1091150 commented Oct 9, 2024

The commit 980bd4c supports Linux framebuffer backend, but the behavior is weird on my machine.

Typical laptop with an intel processor.
Architecture:             x86_64
  CPU op-mode(s):         32-bit, 64-bit
  Address sizes:          39 bits physical, 48 bits virtual
  Byte Order:             Little Endian
CPU(s):                   8
  On-line CPU(s) list:    0-7
Vendor ID:                GenuineIntel
  Model name:             12th Gen Intel(R) Core(TM) i3-1215U

The root case is that some hardware require a cache synchronization when using the framebuffer.

Problem:

  1. Mismatch width and height and 3 windows are drawn(in the screenshot).
  2. Some updates (use twin_screen_damage to mark the field to be updated) are not apply immediately on computer screen.

screen

One possible fix is in a1091150/mado@fbe9e00, the code is copied from FOSDEM 2020 - Back to the Linux Framebuffer! and sync_cache.c

After the below commit, another problem appeared is that the whole screen is not rendered when startup. To fix this, simply use twin_screen_damage to mark the whole scrreen to be damaged, shown in the following code(a1091150/mado@977463a).

static bool twin_fbdev_work(void *closure)
{
    twin_screen_t *screen = SCREEN(closure);
    static bool run_once = true;
    if (run_once) {
        run_once = false;
        twin_screen_damage(screen, 0, 0, screen->width, screen->height);
    }

    if (twin_screen_damaged(screen))
        twin_screen_update(screen);
//...
}
@shengwen-tw
Copy link
Collaborator

shengwen-tw commented Oct 10, 2024

@a1091150 :

Hi, I can help fixing the problem but it is a little harder for me to duplicate the problem.
Are you interested in solving this problem? Otherwise I can take over it.

@a1091150
Copy link
Contributor Author

a1091150 commented Oct 10, 2024

@a1091150 :

Hi, I can help fixing the problem but it is a little harder for me to duplicate the problem. Are you interested in solving this problem? Otherwise I can take over it.

Yes, I am interested in this problem, but I am not familiar with Linux framebuffer. This will take a while.
Currently I tested it on virtual machine (VirtualBox) and it is more broken now.

@jserv jserv added the bug Something isn't working label Oct 10, 2024
@jserv
Copy link
Contributor

jserv commented Oct 10, 2024

Consider to call twin_screen_register_damaged after the call of twin_set_work. That is,

    twin_set_work(twin_fbdev_work, TWIN_WORK_REDISPLAY, ctx);

    /* Enable immediate refresh */
    twin_screen_register_damaged(ctx->screen, twin_fbdev_damaged, ctx);
    ...

And the callback twin_fbdev_damaged is shown below:

static void twin_fbdev_damaged(void *closure)                                                                                                                 
{
    twin_screen_t *screen = SCREEN(closure);
    twin_fbdev_t *tx = PRIV(closure);

    if (tx->vt_active /* VT switch is ready */ && twin_screen_damaged(screen))
        twin_screen_update(screen);
}

However, the current fbdev backend lacks of VT switch operations. That is, consider the following:

static void twin_fbdev_switch(twin_fbdev_t *tf, int activate)
{
    tx->vt_active = activate;
    
    /* Upon activation */
    if (activate) {
        /* Switch complete */
        ioctl(tx->vt_fd, VT_RELDISP, VT_ACKACQ);

        /* Restore fbdev settings */
        if (twin_fbdev_apply_config(tf)) {
            tx->vt_active = true;

            /* Mark entire screen for refresh */
            if (tx->screen)
                twin_screen_damage(tx->screen, 0, 0, tx->screen->width,
                                   tx->screen->height);
        }
    } else {
        /* FIXME: should expose some option to disallow them */
        ioctl(tx->vt_fd, VT_RELDISP, 1);

        tx->vt_active = 0;

        if (tx->fb_base != MAP_FAILED)
            munmap(tx->fb_base, tx->fb_len);
        tx->fb_base = MAP_FAILED;
    }
}

static bool vt_switch_pending;

static bool twin_fbdev_work(void *closure)
{
    twin_fbdev_t *tx = PRIV(closure);

    if (vt_switch_pending) {
        twin_fbdev_switch(tf, !tx->vt_active);
        vt_switch_pending = false;
    }

    return true;
}

static void twin_fbdev_vtswitch(int sig)
{
    signal(sig, twin_fbdev_vtswitch);
    vt_switch_pending = true;
}

static bool twin_fbdev_setup_vt(twin_fbdev_t *tx, int switch_sig)
{
    struct vt_mode vtm;
    struct termios tio;

    if (ioctl(tx->vt_fd, VT_GETMODE, &vtm) < 0) {
        SERROR("can't get VT mode");
        return 0;
    }
    vtm.mode = VT_PROCESS;
    vtm.relsig = switch_sig;
    vtm.acqsig = switch_sig;

    signal(switch_sig, twin_fbdev_vtswitch);
    tx->vt_swsig = switch_sig;

    if (ioctl(tx->vt_fd, VT_SETMODE, &vtm) < 0) {
        SERROR("can't set VT mode");
        signal(switch_sig, SIG_IGN);
        return 0;
    }

    tcgetattr(tx->vt_fd, &tx->old_tio);

    ioctl(tx->vt_fd, KDGKBMODE, &tx->old_kbmode);
    ioctl(tx->vt_fd, KDSKBMODE, K_MEDIUMRAW);

    tio = tx->old_tio;
    tio.c_iflag = (IGNPAR | IGNBRK) & (~PARMRK) & (~ISTRIP);
    tio.c_oflag = 0;
    tio.c_cflag = CREAD | CS8;
    tio.c_lflag = 0;
    tio.c_cc[VTIME] = 0;
    tio.c_cc[VMIN] = 1;
    cfsetispeed(&tio, 9600);
    cfsetospeed(&tio, 9600);
    tcsetattr(tx->vt_fd, TCSANOW, &tio);

    ioctl(tx->vx_fd, KDSETMODE, KD_GRAPHICS);

    return true;
}

Check the following implementations:

@a1091150
Copy link
Contributor Author

a1091150 commented Oct 13, 2024

@a1091150 :
Hi, I can help fixing the problem but it is a little harder for me to duplicate the problem. Are you interested in solving this problem? Otherwise I can take over it.

Yes, I am interested in this problem, but I am not familiar with Linux framebuffer. This will take a while. Currently I tested it on virtual machine (VirtualBox) and it is more broken now.

Another problem I found is visible resolution and virtual resolution are different on Virtualbox, and miscalculates the memory region to be rendered. Use line_length in struct fb_fix_screeninfo to replace these resolution and solve the miscalculation.

Run sudo fbset - i gives the following information, it reads and prints out from struct fb_fix_screeninfo:

mode "800x600"
    geometry 800 600 2048 2048 32
endmode

Frame buffer device information:
    ......
    LineLength  : 8192
    Accelerator : No

On my laptop is gives:

mode "1920x1080"
    geometry 1920 1080 1920 1080 32
    ....
endmode
Frame buffer device information:
    ....
    LineLength  : 7680
    Accelerator : No

Solution:
Use tx->fb_fix.line_length to replace screen->width.

static void _twin_fbdev_put_span(twin_coord_t left,
                                 twin_coord_t top,
                                 twin_coord_t right,
                                 twin_argb32_t *pixels,
                                 void *closure)
{
    twin_screen_t *screen = SCREEN(closure);
    twin_fbdev_t *tx = PRIV(closure);

    if (tx->fb_base == MAP_FAILED)
        return;

    twin_coord_t width = right - left;
-    off_t off = top * screen->width + left;
-    uint32_t *dest =
-        (uint32_t *) ((uintptr_t) tx->fb_base + (off * sizeof(uint32_t)));
+    unsigned char *dest =
+        (unsigned char *) tx->fb_base + tx->fb_fix.line_length * top + 4 * left;
    memcpy(dest, pixels, width * sizeof(uint32_t));
}

@jserv
Copy link
Contributor

jserv commented Oct 25, 2024

Another problem I found is visible resolution and virtual resolution are different on Virtualbox, and miscalculates the memory region to be rendered. Use line_length in struct fb_fix_screeninfo to replace these resolution and solve the miscalculation.
[..]
Solution: Use tx->fb_fix.line_length to replace screen->width.

Can you submit a pull request for testing and reviewing purpose?

@a1091150
Copy link
Contributor Author

a1091150 commented Oct 26, 2024

Can you submit a pull request for testing and reviewing purpose?

There are two test cases and are being tested on my Ubuntu laptop and VirtualBox.

  1. Start demo-fbdev in a pseudoterminal, and switch to another virtual terminal via ctrl+alt+F3 to tty3.
  2. First switch to tty3, and start demo-fbdev.

Case 2 works fine.
Case 1 will give the result as below screenshot. Some render is overwritten on the display, and cursor's movement looks lagging. Note that I can still type some words in tty3.

screen

In case 1, I find out another way to make it not lagging is updating the cache, the code below is from sys_cache.c, but I am not sure whether it is an appropriate way.

#define FBIO_CACHE_SYNC 0x4630
unsigned int args[2];
args[0] = tx->fb_base;
args[1] = tx->fb_base + tx->fb_var.yres_virtual * tx->fb_fix.line_length;
ioctl(tx->fb_fd, FBIO_CACHE_SYNC, args);

mado with framebuffer can start from different terminals like tty and pts (pseudoterminal), and the code 980bd4c assumes the terminal and the target framebuffer is on the same display, #62 (comment) can apply onto the case 2. However this can not work on the case 1 that framebuffer and terminal are not on the same display.

The pull request: #65
If the origin design aims to case 2, then the pull request will only fix miscalculated memory region in _twin_fbdev_put_span , and cache updating code will not be pulled.

static void _twin_fbdev_put_span(twin_coord_t left,
                                 twin_coord_t top,
                                 twin_coord_t right,
                                 twin_argb32_t *pixels,
                                 void *closure)
{
    twin_screen_t *screen = SCREEN(closure);
    twin_fbdev_t *tx = PRIV(closure);

    if (tx->fb_base == MAP_FAILED)
        return;

    twin_coord_t width = right - left;
-    off_t off = top * screen->width + left;
-    uint32_t *dest =
-        (uint32_t *) ((uintptr_t) tx->fb_base + (off * sizeof(uint32_t)));
+    off_t off = tx->fb_fix.line_length * top + 4 * left;
+    unsigned char *dest = (unsigned char *) tx->fb_base + off;
    memcpy(dest, pixels, width * sizeof(uint32_t));
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants