sysprog21 / mado

A window system for resource-constrained devices
MIT License
40 stars 15 forks source link

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

Open a1091150 opened 1 month ago

a1091150 commented 1 month ago

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@fbe9e00b262e227b012645b4b8d7ba2432376841, 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@977463a387f75fa2fe0cfe4d600cbae3720b621c).

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 commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

@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 commented 4 weeks ago

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 commented 3 weeks ago

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 https://github.com/sysprog21/mado/commit/980bd4c5344f8f8ef0ad28bfa1b3e95020845390 assumes the terminal and the target framebuffer is on the same display, https://github.com/sysprog21/mado/issues/62#issuecomment-2405652662 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));
}