Page MenuHomeFreeBSD

Implement "vidcontrol -c <normal|blink|destructive>" for vt(4)
Needs RevisionPublic

Authored by smahadevan_freebsdfoundation.org on Jul 27 2017, 7:36 PM.

Details

Summary

Fixes bug 210413.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

emaste added a subscriber: ray.Jul 28 2017, 5:34 PM
emaste added inline comments.
sys/dev/vt/vt_buf.c
641–643

minor style(9) nit, convention is to in general not initialize variables in declarations. Also a blank line should follow the declarations.

sys/dev/vt/vt_core.c
1068

This fn could be simplified slightly by just naming this parameter invert, and making the TF_REVERSE check do invert = !invert;

2880

minor whitespace issue: extra blank line

ray accepted this revision.Jul 28 2017, 11:03 PM

style(9) issues still here (but in different functions), but there is a lot from very first vt project release.

This revision is now accepted and ready to land.Jul 28 2017, 11:03 PM
dumbbell requested changes to this revision.Jun 10 2018, 4:08 PM

One callout per window, plus flushing all windows twice per second is probably inefficient, on top of a vt(4) which is already slower than syscons(4).

Could you please experiment with something integrated to the vt_flush() and vt_timer() functions instead? The former could handle blinking, while the latter could schedule a new flush in hz / 2 if there are no changes reported by vt_flush(). Note that you would need to improve the vt_resume_flush_timer() and vt_suspend_flush_timer() functions to take care of scheduling a flush earlier if the currently scheduled one would fire in hz / 2. Otherwise, there will be an apparent lag for the user.

sys/dev/vt/vt_core.c
2613

Using one callout per window is overkill: only one cursor is displayed at a time.

2876

You are abusing the VBF_CURSOR flag here: it is meant to configure if the cursor should be displayed at all, whether it blinks or not. With the patch, the user can't tell anymore if the cursor should be hidden.

Also, the access to vw->vw_buf should be locked.

2877

Flushing the entire window is a bit too much just for changing the cursor state.

This revision now requires changes to proceed.Jun 10 2018, 4:08 PM