Fix memory region in framebuffer #65

a1091150 · 2024-10-26T15:34:14Z

Issue #62
The width of screen may be different from the virtual resolution.
In apps/main.c, the default value of WIDTH and HEIGHT are 640 and 480, but the visual resolution are set to 1920 and 1080 on my machine.
The different width will draw three screens on the display as the below screenshot.

Before:

After:

The reason to use ~~xres_virtual~~ line_length as the width and not to use xres in struct fb_var_screeninfo:
On VirtualBox, the visible resolution of virtual terminal, which switched by ctrl+alt+f3, is set to 800x600, but the virtual solution is set to 2048x2048. On my physical machine, Asus latop and Ubuntu system, these are the same values.

The information from sudo fbset -i on my laptop and VirtualBox.
Laptop:

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

VirtualBox:

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

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

backend/fbdev.c

shengwen-tw · 2024-10-28T14:41:16Z

backend/fbdev.c

@@ -85,6 +97,7 @@ static bool twin_fbdev_work(void *closure)

    if (twin_screen_damaged(screen))
        twin_screen_update(screen);
+    twin_fbdev_cache_sync(closure);


Could you explain why twin_fbdev_cache_sync should be called here?

After some search, this is driver-specificed code and should not be used, I will remove this code.
This code has a side-effect on my machine that it updates the smooth display but on VirtualBox is not.

jserv

Improve the git commit message to address the consideration of FBIOPAN_DISPLAY.

a1091150 · 2024-10-31T07:03:14Z

@shengwen-tw, May you test the pull request on you machine to see if it is running correctly?

shengwen-tw · 2024-10-31T07:19:12Z

@shengwen-tw, May you test the pull request on you machine to see if it is running correctly?

Okay, I'll test it soon.

shengwen-tw · 2024-10-31T15:51:32Z

@shengwen-tw, May you test the pull request on you machine to see if it is running correctly?

I have ran the modification code and have some observations:

Firstly, I want to point out that I wasn’t able to reproduce the issue you encountered, as we previously discussed. From my side, I can only confirm the functionality remains the same.

However, although the framebuffer output appears correct, I noticed that the rendering update is not smooth, with significant lag when dragging windows or even just moving the cursor.

Could you analyze any potential performance bottlenecks in your code and look into ways to address this issue?

shengwen-tw · 2024-10-31T16:00:10Z

Please check the video I recorded:
https://www.youtube.com/watch?v=1FK8XkVnP8k

jserv · 2024-11-04T10:35:43Z

Rebase the latest main branch and resolve the conflicts.

jserv

Don't use git merge. Always enforce git rebase.

backend/fbdev.c

a1091150 · 2024-11-04T15:03:25Z

Please check the video I recorded: https://www.youtube.com/watch?v=1FK8XkVnP8k

I find out that ioctl(fb_fd, FBIOPAN_DISPLAY, &var); causes your machine lagging, but my laptop and VirtualBox are not. Without it, my laptop can not show the mado's screen.
I see you are using two display, so I plug my laptop another display without the code ioctl(fb_fd, FBIOPAN_DISPLAY, &var);

mado's screen work fine on second display, subtly.

Check the video: https://youtu.be/XvhigZefDfc

The commit bbc35ff removed code ioctl(fb_fd, FBIOPAN_DISPLAY, &var);. I believe this commit is ready to pull (may be?)

jserv

Refine the statement "The same setting may not be applied on different hardware" to address the consideration and use scenario.

The size of the screen may be different to the virtual resolution. If the size is less than virtual resolution, it will draw multiple windows on display. Use `line_length` to calculate instead of the size of the screen. Changable information like virtual resoultion or setting different information on virtual terminal may not be applied on different hardware and driver. For example on VirtualBox, virtual resolution's width and height are 2048 and 2048, visible resolutions' are 800 and 600, and line length is 2048 * 4. Any setting from guest machine to these maybe ignored.

jserv

Read https://cbea.ms/git-commit/ carefully and enforce the rules.

shengwen-tw · 2024-11-07T04:41:22Z

You mentioned that

I find out that ioctl(fb_fd, FBIOPAN_DISPLAY, &var); causes your machine lagging, but my laptop and VirtualBox are not. Without it, my laptop can not show the mado's screen.

Are you planning to open a new pull request to resolve issue #62?

a1091150 · 2024-11-07T05:44:30Z

Are you planning to open a new pull request to resolve issue #62?

Nope. I notice another drm pull request #69 . The updated may followed and discussed on it.

Issue #62 mentioned some problems:

Different screen size causes multiple windwos and weird behavior(will be fixed by this pull request)
Update and render after switching back to virtual terminal.
ioctl(fb_fd, FBIOPAN_DISPLAY, &var) have different behavior on different machine, mentioned in this pull request before.

For third problem, I suspect it is just my laptop and i915drmfb (Intel DRM) will need this pan display and it is not a bug in Mado but in Intel DRM.

For second problem, I did write some switch code for switching, and I notice the proposed DRM backend (#69). The updated may followed and discussed on it.

shengwen-tw · 2024-11-07T17:17:14Z

I did various tests and found that the problem might not be caused by ioctl(fb_fd, FBIOPAN_DISPLAY, &var);, as the issue still occurs on my machine even after removing that line of code. (it also happens with the upstream code, though not consistently.)

If ioctl(fb_fd, FBIOPAN_DISPLAY, &var); resolves the issue you encountered, it would be reasonable to adopt the change you proposed earlier. (But could you clarify why the FBIOPAN_DISPLAY operation helps in this case?)

I suspect that the lag might be due to the button events being sent from the Linux input subsystem we recently integrated, as the current implementation does not ensure atomic updates for mouse events. I may open another new issue once I have a clearer understanding of this phenomenon.

jserv requested a review from shengwen-tw October 26, 2024 15:38

This comment was marked as duplicate.

Sign in to view

jserv reviewed Oct 26, 2024

View reviewed changes

backend/fbdev.c Outdated Show resolved Hide resolved

a1091150 mentioned this pull request Oct 26, 2024

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

Open

shengwen-tw reviewed Oct 28, 2024

View reviewed changes

a1091150 force-pushed the fixVideoSync2 branch from dee133c to e87ba61 Compare October 29, 2024 16:35

a1091150 changed the title ~~Fix memory region in framebuffer and cache synchronization~~ Fix memory region in framebuffer and pan display Oct 29, 2024

jserv requested changes Oct 30, 2024

View reviewed changes

a1091150 force-pushed the fixVideoSync2 branch from e87ba61 to 964d343 Compare October 31, 2024 06:58

This comment was marked as duplicate.

Sign in to view

jserv requested changes Nov 4, 2024

View reviewed changes

jserv reviewed Nov 4, 2024

View reviewed changes

backend/fbdev.c Outdated Show resolved Hide resolved

a1091150 force-pushed the fixVideoSync2 branch 2 times, most recently from acf5c4d to bbc35ff Compare November 4, 2024 14:47

jserv requested changes Nov 4, 2024

View reviewed changes

jserv changed the title ~~Fix memory region in framebuffer and pan display~~ Fix memory region in framebuffer Nov 4, 2024

jserv requested a review from shengwen-tw November 4, 2024 17:28

a1091150 force-pushed the fixVideoSync2 branch from bbc35ff to 144ff6d Compare November 7, 2024 04:33

jserv requested changes Nov 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix memory region in framebuffer #65

Fix memory region in framebuffer #65

a1091150 commented Oct 26, 2024 •

edited

Loading

This comment was marked as duplicate.

shengwen-tw Oct 28, 2024

a1091150 Oct 29, 2024

jserv left a comment

a1091150 commented Oct 31, 2024

shengwen-tw commented Oct 31, 2024

shengwen-tw commented Oct 31, 2024

shengwen-tw commented Oct 31, 2024

This comment was marked as duplicate.

jserv commented Nov 4, 2024

jserv left a comment

a1091150 commented Nov 4, 2024 •

edited

Loading

jserv left a comment

jserv left a comment

shengwen-tw commented Nov 7, 2024

a1091150 commented Nov 7, 2024 •

edited by jserv

Loading

shengwen-tw commented Nov 7, 2024

Fix memory region in framebuffer #65

Are you sure you want to change the base?

Fix memory region in framebuffer #65

Conversation

a1091150 commented Oct 26, 2024 • edited Loading

This comment was marked as duplicate.

shengwen-tw Oct 28, 2024

Choose a reason for hiding this comment

a1091150 Oct 29, 2024

Choose a reason for hiding this comment

jserv left a comment

Choose a reason for hiding this comment

a1091150 commented Oct 31, 2024

shengwen-tw commented Oct 31, 2024

shengwen-tw commented Oct 31, 2024

shengwen-tw commented Oct 31, 2024

This comment was marked as duplicate.

jserv commented Nov 4, 2024

jserv left a comment

Choose a reason for hiding this comment

a1091150 commented Nov 4, 2024 • edited Loading

jserv left a comment

Choose a reason for hiding this comment

jserv left a comment

Choose a reason for hiding this comment

shengwen-tw commented Nov 7, 2024

a1091150 commented Nov 7, 2024 • edited by jserv Loading

shengwen-tw commented Nov 7, 2024

a1091150 commented Oct 26, 2024 •

edited

Loading

a1091150 commented Nov 4, 2024 •

edited

Loading

a1091150 commented Nov 7, 2024 •

edited by jserv

Loading