Skip to content

Conversation

@Bennctu
Copy link
Collaborator

@Bennctu Bennctu commented Nov 10, 2024

Move Linux VT into separate files to enable developers to use it in other backhands, such as Linux DRM.

@jserv jserv requested a review from shengwen-tw November 10, 2024 13:56
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the routine twin_vt_setup from the file backend/fbdev.c to the proposed backend/linux_vt.c and wrap the controls related to <linux/vt.h>.

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since most virtual terminal operations are brief, use static inline functions for the implementation. This means placing the function body directly in the header linux_vt.h.

@jserv
Copy link
Contributor

jserv commented Nov 11, 2024

The ioctl we would like to wrap in this task:

  • ioctl(ttyfd, KDSETMODE, KD_GRAPHICS)
  • ioctl(ttyfd, KDSETMODE, KD_TEXT)

int fd;

char vt_dev[30] = {0};
snprintf(vt_dev, 30, "/dev/tty%d", vt_num);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 30?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially, we used 30 as a conservative size. Using the command find /dev/ -name "tty*", we get results like /dev/ttyXX. Therefore, 11 is also a suitable size.

return fd;
}

static inline int twin_vt_set(int fd, int mode)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is confusing to have similar function names, twin_vt_set and twin_vt_setup. Can you clarify?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the clarity of this function needs to be improved. Here are a few options for this function name:

  1. twin_vt_apply_mode
  2. twin_vt_configure_mode
  3. twin_set_vt_mode

Or let me know the better choices. I will rename this function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the clarity of this function needs to be improved.

How about twin_vt_mode?

Move Linux VT into separate files to enable developers to use it in
other backends, such as Linux DRM.
@jserv jserv merged commit c7bbcc7 into sysprog21:main Nov 13, 2024
3 checks passed
@jserv
Copy link
Contributor

jserv commented Nov 13, 2024

Thank @Bennctu for contributing!

@Bennctu Bennctu deleted the vt_indep branch February 5, 2025 16:50
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

Successfully merging this pull request may close these issues.

2 participants