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

A couple of useful changes to the VT handling #3

Closed
wants to merge 2 commits into from

Conversation

bluetech
Copy link
Contributor

They are not dependent so you can pick both or either.

After we turn the keyboard to raw mode, we try to set the keyboard mode
to K_OFF. This means we can avoid input events from the tty completely.

This mode was added in linux 3.0:
https://lkml.org/lkml/2011/2/1/609
We only use it if we have it.

Signed-off-by: Ran Benita <ran234@gmail.com>
Add two functions to enter/leave our VT object. This allows to
implement to expected behavior of automatically switching to the kmscon
when it is running on a new tty, and switching back to the tty we came
from when the program finishes. Presumably this behavior will be
controlled by a config variable or command line argument later on (like
Xorg's -novtswitch).

There's a bit of a subtlety in this because of VT_PROCESS. We need
permission from ourselves to switch in/out of out VT; this is done when
processing SIGUSR[12] in the eloop. We therefore must dispatch the loop
at least once after switching out. The usual case is demonstrated in
test_vt.c.

Signed-off-by: Ran Benita <ran234@gmail.com>
@dvdhrm
Copy link
Owner

dvdhrm commented Dec 11, 2011

Awesome, thx. I will pull this next week as I don't have much time right now. Anyway, they look mostly good. But I'd like to adjust the changes to test_vt. That kmscon_eloop_dispatch() looks wrong. The idea is right but I'd like to go sure we really catch the SIGUSR here and that it does not dispatch some other event. Also, we should use some timeout to avoid a deadlock if the SIGUSR never arrives (whatever reason may cause this).

Cheers
David

@dvdhrm
Copy link
Owner

dvdhrm commented Dec 14, 2011

Sorry, I have to reject the first patch. I have looked up the kernel-changes and they actually also block VT-switch events from ctrl+alt+FX. Until we have proper input handling and can catch vt-switch events ourself, I don't want this patch. It makes it impossible to switch back to X/other-VT when being on the kmscon-VT.
I don't know whether you tested this patch with a 3.1 kernel. But on my 3.1 this shows this odd behaviour.

If we have proper input handling we may reconsider this. I've added it to the TODO list.

@dvdhrm
Copy link
Owner

dvdhrm commented Dec 14, 2011

I have cherry-picked the other commit. I have changed a few things:

  • I removed the "switch" from the two new function names. It seems redundant.
  • I have added some log_warning()s and log_debug()s
  • I changed the logic in kmscon_vt_leave(). We now return -EINPROGRESS if SIGUSR needs to be handled by the application. We return 0 if no VT switch needs to be made and we return <0 on failure. This allows the test_vt application to call the eloop only if really needed and avoid hangups/delays on termination if SIGUSR never arrives.
  • kmscon_vt_leave() will trigger the VT_ACTIVATE ioctl only if our VT is currently active. I now check vts.v_active agains vt->num and not vt->saved_num. This actually makes more sense.
  • test_vt calls the eloop dispatcher with a 1second timeout to avoid hangups on termination. And it calls the dispatcher only if -EINPROGRESS is returned.

Thanks for this patch. Just for information if you planned that: I don't want this feature as default behavior in the other test cases as it is quite disturbing for debugging but it seems an important feature for the final application. We will need further investigation for the final eloop_dispatch, though. We should remove all other signals from the eloop first to avoid receiving a SIGINT or fd event before SIGUSR and the eloop terminates without handling the SIGUSR. But we can think about this later.

Cheers
David

I close this pull request now but feel free to reopen it if you have objections.

@dvdhrm dvdhrm closed this Dec 14, 2011
@bluetech
Copy link
Contributor Author

Hi,

As for the K_OFF, not pulling it now seems reasonable, especially when it's completely optional. For testing, I personally used Alt-Sysrq+r, which resets back to RAW mode. I forgot to mention it in the commit message.

BTW, I spent some time this week writing up some input support, because I wanted to learn about evdev and udev. It's in here since yesterday:
https://github.com/bluetech/kmscon/tree/input
And I've done a moderate amount of testing. I'm not sure whether to send a pull request, I figure you might want to write it yourself instead of reviewing that.

Thanks.

@dvdhrm
Copy link
Owner

dvdhrm commented Dec 14, 2011

Yeah, sysrq works but it seems like a hack.

I already looked at the input module. I will add some comments to your tree later. It seems nice but I already found some things I'd change.

Regards
David

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