s3switch segfaults on the thinkpad [patch]

when setting the tvout to pal the application segfaults. there exists the following patch but it was never really submitted anywhere except in fedora.
#### Begin: s3switch-thinkpad-t23.patch
Fix segfaults seen on IBM ThinkPad T23 when changing TV signal format.
Thanks to the upstream author, Tim Roberts, for help in creating this patch.

--- s3switch.c.orig 2002-09-12 01:25:33.000000000 +0300
+++ s3switch.c 2004-07-22 13:28:32.000000000 +0300
@@ -100,4 +100,8 @@
     ioperm( 0x40, 4, enable );
+ ioperm( 0x4f, 1, enable );
     ioperm( 0x61, 1, enable );
+ ioperm( 0x70, 1, enable );
+ ioperm( 0x71, 1, enable );
     ioperm( 0x80, 1, enable );
+ ioperm( 0xb2, 1, enable );
     ioperm( 0x3b0, 0x30, enable );
#### End: s3switch-thinkpad-t23.patch

For me that patch seemed to fix the problem, though s3switch does not show current state on the command line correctly.
So when there is a patch for this problem for over a year now, why does it not go into the package?

I see the same behaviour on a Thinkpad T21.

Thanks, Michael for your report. From looking at the release version of s3switch, I guess this is ancient software that is not cared for too well. There is no Maintainer for it, for example. Assigning to MOTU to either apply or reject the patch.

Sebastian, can you get Debian to include the patch?

> Sebastian, can you get Debian to include the patch?

Well, Debian is already aware of the problem -> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=310984

I'll try and let Guido know about the patch.

When I try built package with that patch I get a FTBFS:

s3switch.c: In function 'print_current_state':
s3switch.c:301: warning: format not a string literal and no format arguments
s3switch.c:313: warning: format not a string literal and no format arguments
gcc -Wall -g -O2 -c -o lrmi.o lrmi.c
lrmi.c: In function 'get_int_seg':
lrmi.c:197: warning: cast to pointer from integer of different size
lrmi.c: In function 'get_int_off':
lrmi.c:204: warning: cast to pointer from integer of different size
lrmi.c: In function 'LRMI_init':
lrmi.c:267: warning: cast from pointer to integer of different size
lrmi.c:275: warning: cast from pointer to integer of different size
lrmi.c:276: warning: cast from pointer to integer of different size
lrmi.c: In function 'set_regs':
lrmi.c:305: error: 'IF_MASK' undeclared (first use in this function)
lrmi.c:305: error: (Each undeclared identifier is reported only once
lrmi.c:305: error: for each function it appears in.)
lrmi.c:305: error: 'IOPL_MASK' undeclared (first use in this function)
lrmi.c: In function 'emulate':
lrmi.c:571: warning: cast to pointer from integer of different size
lrmi.c: In function 'run_vm86':
lrmi.c:784: error: 'VIF_MASK' undeclared (first use in this function)
lrmi.c:784: error: 'TF_MASK' undeclared (first use in this function)
lrmi.c: In function 'LRMI_int':
lrmi.c:873: error: 'IF_MASK' undeclared (first use in this function)
lrmi.c:873: error: 'IOPL_MASK' und

confirming FTBFS

The "fix released" in Debian is a sad removal there... I would prefer to keep it alive and do some testing. Currently I am also working on the savage driver to which this tool corresponds, therefore I can offer to test it and maybe even add further fixes. The hardware I have available contains Savage MX/IX chips on which I can test.

After some testing I could now start to provide patches for this tool and make it work with current kernels again - because of modifications to the "vm86" IO macros in the kernel, the lrmi files needed some updates also.

Are any maintainers listening who could help me integrating the patches correctly for Debian and Ubuntu? I'd also like to re-add this package to Debian again, helping users of the S3 savage chips to gan more comfort and usability again.

Additionally I have prepared some patches to the savage driver which allow to acommodate s3switch's changes better and, for example, keep the selected devices also in case of a mode change, where the current driver re-uses the previous information and reverts the selections done by s3switch.

Hans-Juergen, thank you for your work. You are aware that upstream for this is as dead as dead can be (no new releases for 8 years!). Have a look at http://bugs.debian.org/608180. This will be an up-hill struggle. Are you sure it's worth it? Even if so, there are no guarantees at all on this one.

Hello Rolf,

thanks for replying. Well, this piece of software is not of any kind which must be updated regularly just for updating's sake. It is a real-mode VGA register accessing tool for a graphics hardware that is about 10 years old, but very common in a lot of laptops built around 2000 to 2004. So updates are only really necessary if some constraints change, like just recently the VM86 macros and some header files. The original programmer of this tool seems to have stopped his efforts around about 2003.

In fact, this tool should even be included in the xserver-savage package as it is the simplest way for using the TV output which usually cannot be accessed by pressing the Fn key combination for LCD/CRT switching - this virtually never allows switching the TV output on.

I think it is possible to manage at least a rudimentary maintenance of this package. I have three laptops using this hardware, so I just need it for myself and spend the time improving it independently of any public demand - but I'd like to give the results back to the community.

As mentioned before, for optimised work for this tool, changes in the savage driver are also useful or even necessary, as weird behaviour might result otherwise. For example, the s3switch tool writes its changes directly to the VGA registers, but the savage driver does not notice this in its current version. In case of a resolution change or switch to VT, it reverts to the settings taken during start-up of the X server as it thinks it has to "restore" the settings.

I have a prototype version running here which collaborates quite well and even manages to get the video scaling correct for a single output. As overlaying is quite different in this driver from regular graphics and laptop LCD expansion has to be taken into account, there are a lot of details to obey.

I'm also hoping for Tormod Volden who already encouraged me towards improving the savage driver, which in turn just longs for matching the s3switch tool...

Thanks and best regards,

Hans-Juergen

Hans-Jürgen, your work in this area is very welcome. Just be prepared to encounter some hesitations because this is old soft- and hardware. I am absolutely with you in trying to maintain support for older hardware. Feel free to contact me anytime if you get stuck with the packaging and I'll see if I have any suggestions how to move onwards. Seeing you are a programmer yourself, here's what I suggest:

* consider taking over the upstream source, you're not getting this back into Debian without an active upstream
* talk to Tormod about getting this into the xserver-xorg-video-savage package in Debian
* make the upstream source for the xserver-xorg-video-savage package the new upstream home

Thank you for your work. There'll be more before you'll actually reach your goal of reviving this but with some persistence I think you have what it takes to get there. Regards, Rolf

Any work on this would be welcome. I would not mind adding the s3switch utility to the savage upstream code as long as there is no license issues. It is basically one file to make one binary. Now looking at the file, almost every part of it is already in the savage code. Which leads me to think it should be relatively easy to bake it into the savage driver. In order to control the outputs, the xrandr interface would have to be used. I don't know how much work it would be to add xrandr output control to the savage driver, but it seems like the best solution technically and in terms of future support. Would you be willing to look into this? I have not much time for this, and not much xrandr insight, but I will assist in testing (on my Twister card) and code review and make sure it gets upstream.

Hans-Jürgen, any news on this?

Hello Rolf,

some of my patches to xorg-savage were recently added by Tormod Volden.
The path direction for s3switch is directly linked to that X server, as
they are interoperating closely (and need to do this, which will be one
of the targets of my work).

On my systems (HP XE3), I am testing all my patches in all-day use and
have not stumbled over any problems yet, but it seems that we need
someone with that special kind of Thinkpad mentioned in the bug report
to verify.

There are several ways how to deal with PAL/NTSC switching, as well as
TV swichting, in the future.

I think for the rest of the year, time will be a limit on my work, too,
as my "regular" work is quite exhaustive at least up to the end of November.

In the following, I have an excerpt of ideas on this topic which I sent
to Tormod some weeks ago.

It would be great if we could either make a common decision OR just
allow a solution to be established which is achieved first. Tormod
suggested to get into XRandR to migrate the s3switch features to this
logic, but that would result in a solution not really quick to implement
as it is a completely new topic.

-------------------------------

What I did in the meantime you can see in the attached patches. It was
all about inconsistencies related to output selection, s3switch
accomodation and video playback. The following goals are reached:

- before playing a video, the driver reads in the output config which
could have been modified due to Fn+Output key or s3switch. If TV or CRT
is on, the LCD expansion is avoided. Before this, video image on TV was
cropped due to false expansion.

- when the LCD is off, DPMS for the LCD is blocked. Before this, the
display would get corrupted on external (esp. TV) screen and the LCD got
into an undesired state of turning white. This was especially annoying
as e.g. the VLC player makes calls to DPMS before playing a video! I
think this was well-meant, but was destructive in case of our savage
driver. Now the driver is protected.

- at each mode switch, the driver reads in its current output
configuration and updates its list and flags. Up to now, a resolution
switch after changing output by key or s3switch caused the outputs to be
changed back to their previous state.

- when switching to text mode, the driver updates its config also and
restores the initial setting as intended.

HERE I HAVE AN IDEA FOR THE FUTURE: a new xorg.conf option which allows
enforcing to stay in the new output configuration also in text mode.

- the expansion code has now a condition for adding the famous "+7"
offset in case of a thinkpad 1400x1050 display. WE NEED A TESTER WITH A
THINKPAD T22 OR T23 EQUIPPED WITH THIS DISPLAY, not just because of this...

- the driver now prefers the BIOS configuration for selecting the TV
format, as most savage machines allow this.

IDEA: we should add an xorg.conf option allowing to prefer BIOS or xorg.conf

IDEA: we should decouple setting the TV format (option "PAL") from
enabling TV out. Up to now it is a silly mix that PAL also enables TV.

- the driver does not overwrite the TV format setting unnecessarily
anymore. On my system this seems to cau...

Thank you for your explanation and the work you've done. Feel free to adhust this ticket's status as appropriate.

s3switch was revived in Fedora, based on the patches from the now-dead Debian package plus many more. I will take a look at giving it an upstream home at freedesktop.org and apply all these patches for everybody.

Secondly I would like to make the xorg driver more compatible with it, using stuff from Hans-Juergen's patches. But we need to clean up these patches, for now it is one big patch changing all kind of things. A patch should be a minimal change fixing one issue at a time, otherwise it can not be reviewed and tested properly, and the development history would be unreadable.

Can you please try out:
 git clone git://people.freedesktop.org/~tormod/s3switch
 cd s3switch
 make
 sudo ./s3switch ...

Can you please try the .deb from https://launchpad.net/~tormodvolden/+archive/ppa/+build/2939311 ?

Hello Tormod,

is this the same version as the git source? I'm going to test one, but I
think I will prefer the source as I can try to modify it directly in
case of new experiences.

Maybe I can check tomorrow - I'll post my results from the HP XE3-GC here.

Tormod Volden schrieb:
> Can you please try the .deb from
> https://launchpad.net/~tormodvolden/+archive/ppa/+build/2939311 ?
>

It is the same code as a fresh pull from comment 21, only Debian/Ubuntu packaging has been added. Thanks for testing!

Hello Tormod,

here are my first test results, a little bit surprising:

1. the common results:

- on my systems with the modified savage driver (the "big patch" which I
want to divide up soon...) and the Savage IX/MX chipset, BOTH versions
of s3switch, the well-known old one from the Ubuntu repository as well
as your modified one, are actually able to modify all settings including
the TV format

- the requested TV output format gets valid at the first output switch
to TV after the setting has been changed - meaning that changing the TV
format while the output is set to TV is saved, but not changed
immediately (that is how I would expect it as an "online" change could
cause other kinds of trouble)

2. the differences:

- your modified version gives a segmentation fault on my system when it
is called with "ntsc" or "pal" - but as mentioned above, the correct
format is activated (but not printed on the screen, I hope the
segmentation fault is not caused by the printf?)

- the repository version gives me a detailed error message about a vm86
error when called with "ntsc" or "pal", with register dumps complaining
that it was not able to write the setting, but after this error message
the new setting is printed to the screen

3. hardware

- the BIOS of my system never got corrupted by s3switch, no matter how
often I switched back and forth between outputs and TV formats

If and when time allows, I will also make further tests and alsso take a
look at the source code.

Hope these first results may help you a little.

Can you please provide a gdb backtrace from the segfault?

hansi@hplaptop3:~$ sudo gdb ./s3switch
GNU gdb (GDB) 7.2-ubuntu
Copyright (C) 2010 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /home/hansi/s3switch...done.
(gdb) run pal
Starting program: /home/hansi/s3switch pal

Program received signal SIGSEGV, Segmentation fault.
0x0804a073 in em_outbl (literal=237 '\355') at lrmi.c:585
585 asm volatile ("outb %b0, %w1"
(gdb) bt
#0 0x0804a073 in em_outbl (literal=237 '\355') at lrmi.c:585
#1 0x0804a3fc in emulate () at lrmi.c:702
#2 0x0804a5af in run_vm86 () at lrmi.c:840
#3 0x0804a7a9 in LRMI_int (i=16, r=0xbff32f28) at lrmi.c:1063
#4 0x08048d5f in set_tv_state (state=8) at s3switch.c:289
#5 0x08049129 in set_new_tvstate (tvstate=8) at s3switch.c:407
#6 0x080493d3 in main (argc=2, argv=0xbff3306c) at s3switch.c:473
(gdb)

Hello Tormod,

are you stil here? Was the debugger output not helpful?

I have been very busy the last months, but I am looking forward for having some time to take care about s3switch and to disentangle the "big savage patch".

I have not lost track of the bugs, on all my four Savage-equipped machines in full daily usage everything works fine.

Best regards,

Hans-Juergen

Hi Hans-Juergen, the backtrace did not give me any clue. Can you do a "backtrace full" and "info regs" also? This can be a bug in LRMI for what I know. If this is a regression, can you try git-bisecting it? And if it is not, maybe more ioperm's has to be sprinkled, like in the original bug description.

glad to see this has sprung back into life. Debian bug number is marked as fixed. Should we close this ticket as well? What is the status? Sebastian, Hans-Jürgen?