Improve output and TV format handling and make it compatible to s3switch

This patch contains several modifications to the handling of the list of
output devices and the control flags for the TV output format, which are
detailed in the following sections.

- new flags are defined to make reading and setting devices easier,
avoiding masking too much or too little

- at the moment of switching between text mode and graphics mode, the
output device settings are saved and updated accordingly which, for
example, means that a display corrupted in graphics mode can be restored
by switching back to text mode as we can assume that boot-up in text
mode works correctly.

- BUGFIX: previously a variable was counted up endlessly by one at each
mode switch. A boolean makes life easier and is what we want here by the
necessary logic.

- BUGFIX: previously the BIOS (hardware) settings for the TV output mode
were not recognised or used correctly which meant by setting "TvOn" you
had automatically set NTSC by force. To avoid this and allow the BIOS
setting to take effect, the configuration option "PAL" is ignored as it
even was not documented properly. Maybe this also reduces the need for
s3switch in many cases as most laptops allow the format to be set up in
the BIOS configuration correctly, but the savage driver not only
ignored, but overrode it and even fought against s3switch here.

If we need to override the TV format from the config, we would have to
define a new flag like "TvFormatOverride" or whatever you like.

- in general a more intelligent handling of VGA register writes related
to the output device set - these were quite unconditional before.

- the output device configuration is read in before video rendering is
started to allow applying the correct image scaling for the selected
device and resolution - previously there could be wrong assumptions
depending on the previous sequence of device settings before starting
the video rendering.

Signed-off-by: Hans-Juergen Mauser <hjmauser@gmx.net>

4 files changed, 200 insertions, 31 deletions

diff --git a/src/savage_driver.h b/src/savage_driver.h
index 338cfac..f64d606 100644
--- a/src/savage_driver.h
+++ b/src/savage_driver.h
@@ -619,6 +619,9 @@ void SavageSetTextMode( SavagePtr psav );
 void SavageSetVESAMode( SavagePtr psav, int n, int Refresh );
 void SavageSetPanelEnabled( SavagePtr psav, Bool active );
 void SavageFreeBIOSModeTable( SavagePtr psav, SavageModeTablePtr* ppTable );
+int SavageGetDevice( SavagePtr psav );
+int SavageGetTV( SavagePtr psav );
+int SavageCorrectDuoViewFlag(int iDevInfo, Bool bEnableActivate, Bool bEnableDeactivate);
 SavageModeTablePtr SavageGetBIOSModeTable( SavagePtr psav, int iDepth );
 ModeStatus SavageMatchBiosMode(ScrnInfoPtr pScrn,int width,int height,int refresh,
                               unsigned int *vesaMode,unsigned int *newRefresh);
diff --git a/src/savage_regs.h b/src/savage_regs.h
index 1a71c4e..0175fc1 100644
--- a/src/savage_regs.h
+++ b/src/savage_regs.h
@@ -200,6 +200,10 @@ do {                                    \
 #define CRT_ATTACHED	0x10
 #define LCD_ATTACHED	0x20
 #define TV_ATTACHED	0x40
+/* flag for DuoView, use identical naming to s3switch */
+#define DUO_ON	0x80
+/* flag to mask away anything but the active devices (x_ACTIVE) */
+#define ACTIVE_DEVICES	0x07
 
 
 /*
diff --git a/src/savage_vbe.c b/src/savage_vbe.c
index 7dba538..7dc959e 100644
--- a/src/savage_vbe.c
+++ b/src/savage_vbe.c
@@ -18,7 +18,6 @@
 #endif
 #define L_ADD(x)  (B_O32(x) & 0xffff) + ((B_O32(x) >> 12) & 0xffff00)
 
-static int SavageGetDevice( SavagePtr psav );
 /*static int SavageGetTVType( SavagePtr psav );*/
 void SavageSetVESAModeCrtc1( SavagePtr psav, int n, int Refresh );
 void SavageSetVESAModeCrtc2( SavagePtr psav, int n, int Refresh );
@@ -39,8 +38,21 @@ SavageClearVM86Regs( xf86Int10InfoPtr pInt )
 void
 SavageSetTextMode( SavagePtr psav )
 {
-    /* Restore display device if changed. */
-    if( psav->iDevInfo != psav->iDevInfoPrim ) {
+    int iDevInfo;
+
+    /* The BIOS settings currently in effect (which are retrieved by SavageGetDevice) are the reference for comparison */
+    iDevInfo = SavageGetDevice(psav);
+
+    /* Save the BIOS settings (which are in effect) as current if they are different from the last state saved as "current" */
+    if (iDevInfo != psav->iDevInfo) {
+	psav->iDevInfo = iDevInfo;
+    }
+
+    /* TODO: should the original settings also be corrected for the DuoView flag? */
+
+    /* Restore display device to original (primary) state if changed since last mode switch AWAY from text. */
+    /* avoid unnecessary changes - mask for ACTIVE_DEVICES only */
+    if( (iDevInfo & ACTIVE_DEVICES) != (psav->iDevInfoPrim & ACTIVE_DEVICES) ) {
 	SavageClearVM86Regs( psav->pVbe->pInt10 );
 	psav->pVbe->pInt10->ax = 0x4f14;
 	psav->pVbe->pInt10->bx = 0x0003;
@@ -48,10 +60,9 @@ SavageSetTextMode( SavagePtr psav )
 	xf86ExecX86int10( psav->pVbe->pInt10 );
     }
 
+    /* switch to text mode */
     SavageClearVM86Regs( psav->pVbe->pInt10 );
-
     psav->pVbe->pInt10->ax = 0x83;
-
     xf86ExecX86int10( psav->pVbe->pInt10 );
 }
 
@@ -119,7 +130,8 @@ void
 SavageSetVESAMode( SavagePtr psav, int n, int Refresh )
 {
     int iDevInfo;
-    static int iCount = 0;
+    int iTVInfo;
+    static Bool bFirstRun = TRUE;
 
     if (psav->IsSecondary) {
         SavageSetVESAModeCrtc2(psav, n, Refresh);
@@ -133,13 +145,41 @@ SavageSetVESAMode( SavagePtr psav, int n, int Refresh )
     /* Get current display device status. */
 
     iDevInfo = SavageGetDevice(psav);
-    psav->iDevInfo = iDevInfo;
-    if( !iCount++ )
-	psav->iDevInfoPrim = psav->iDevInfo;
-    if( psav->CrtOnly )
-	psav->iDevInfo = CRT_ACTIVE;
-    if( psav->TvOn )
-	psav->iDevInfo = TV_ACTIVE;
+    iTVInfo = SavageGetTV(psav);
+    /*
+     * Check if we run for the first time - if this is the case,
+     * save the original settings both as current and original.
+     * Additionally, prefer the flag settings at this point as they are the user's desire.
+     */
+    if( bFirstRun == TRUE ) {
+	bFirstRun = FALSE;
+	/* run for the first time - store settings as current and original */
+	psav->iDevInfo = iDevInfo;
+	psav->iDevInfoPrim = iDevInfo;
+	/* add features requested by the config to the current settings */
+	if( psav->TvOn ) {
+	    psav->iDevInfo |= TV_ACTIVE; /* TvOn is NOT TvOnly! */
+	}
+	if( psav->CrtOnly ) {
+	    psav->iDevInfo = CRT_ACTIVE; /* do this at last as this is exclusive and cancels other settings */
+	}
+	/* read in TV format from the BIOS, ignore what has been configured before as the handling is silly and not documented (PAL does not mean that TV should be switched on, etc.) */
+	/* TODO: check how preference of BIOS and config can be handled intelligently, this must be coordinated with savage_driver.c */
+	psav->PAL = ((iTVInfo & BIOS_TV_PAL) != 0);
+    } else { /* subsequent run */
+	/* if the current settings are identical to the original, we likely come from text mode - prefer the stored ones */
+	/* but take the current ones if settings are different as they might have been modified on-line */
+	if (psav->iDevInfoPrim != iDevInfo) {
+	    psav->iDevInfo = iDevInfo;
+	}
+	/* in both cases, update the flags according to the selected configuration */
+	psav->TvOn = ((psav->iDevInfo & TV_ACTIVE) != 0);
+	/* only set CRT flag if no other output is active */
+	psav->CrtOnly = (((psav->iDevInfo & ACTIVE_DEVICES) & ((unsigned char) ~CRT_ACTIVE)) == 0);
+    }
+
+    /* fully adjust the DuoView flag */
+    psav->iDevInfo = SavageCorrectDuoViewFlag(psav->iDevInfo, TRUE, TRUE);
 
     /* Establish the refresh rate for this mode. */
 
@@ -153,12 +193,16 @@ SavageSetVESAMode( SavagePtr psav, int n, int Refresh )
 
     /* Set TV type if TV is on. */
     if( psav->TvOn ) {
-	SavageClearVM86Regs( psav->pVbe->pInt10 );
-	psav->pVbe->pInt10->ax = 0x4f14;	/* S3 extensions */
-	psav->pVbe->pInt10->bx = 0x0007;	/* TV extensions */
-	psav->pVbe->pInt10->cx = psav->PAL ? 0x08 : 0x04;
-	psav->pVbe->pInt10->dx = 0x0c;
-	xf86ExecX86int10( psav->pVbe->pInt10 );
+	/* make this conditional to avoid unnecessary writes */
+	if (((psav->PAL) && (iTVInfo & BIOS_TV_NTSC))
+	    || ((!(psav->PAL)) && (iTVInfo & BIOS_TV_PAL))) {
+	    SavageClearVM86Regs( psav->pVbe->pInt10 );
+	    psav->pVbe->pInt10->ax = 0x4f14;	/* S3 extensions */
+	    psav->pVbe->pInt10->bx = 0x0007;	/* TV extensions */
+	    psav->pVbe->pInt10->cx = psav->PAL ? 0x08 : 0x04;
+	    psav->pVbe->pInt10->dx = 0x0c;
+	    xf86ExecX86int10( psav->pVbe->pInt10 );
+	}
     }
 
     /* Manipulate output device set. */
@@ -172,8 +216,9 @@ SavageSetVESAMode( SavagePtr psav, int n, int Refresh )
 	/* Re-fetch actual device set. */
 	psav->iDevInfo = SavageGetDevice( psav );
 	iDevInfo = psav->iDevInfo;
-	psav->CrtOnly = (iDevInfo == 1);
-	psav->TvOn = !!(iDevInfo & 4);
+	psav->TvOn = ((iDevInfo & TV_ACTIVE) != 0);
+	/* set CRT flag if no other output is active */
+	psav->CrtOnly = ((iDevInfo & ((unsigned char) ~CRT_ACTIVE)) == 0);
     }
 
     /* Now, make this mode current. */
@@ -191,33 +236,123 @@ void
 SavageSetPanelEnabled( SavagePtr psav, Bool active )
 {
     int iDevInfo;
+    Bool bDoChange = 0;
+
     if( !psav->PanelX )
 	return; /* no panel */
+
     iDevInfo = SavageGetDevice( psav );
-    if( active )
-	iDevInfo |= LCD_ACTIVE;
-    else
-	iDevInfo &= ~LCD_ACTIVE;
+    /*
+     * This function (currently) gets called ONLY by the DPMS mode switching function.
+     * Therefore, we should NOT change anything in the following cases:
+     * - if the TV output is active, as we will corrupt the screen
+     * - TODO: check conditions for CRT / dual modes
+     * Anyway, we will make our settings consistent here and only write out something
+     * if we need a change.
+     */
+    if ((iDevInfo & TV_ACTIVE) == 0) {
+	/* continue, no TV will disturb us */
+	if( active ) {
+	    /* LCD should be on afterwards */
+	    if ((iDevInfo & LCD_ACTIVE) == 0) {
+		iDevInfo |= LCD_ACTIVE;
+		/* we need a change */
+		bDoChange = 1;
+	    }
+	} else {
+	    /* LCD should be off afterwards */
+	    if ((iDevInfo & LCD_ACTIVE) != 0) {
+		iDevInfo &= ~LCD_ACTIVE;
+		/* change requested */
+		bDoChange = 1;
+	    }
+	}
+    }
+
+    /* now check if we really need to do anything */
+    if (bDoChange != 0) {
+	/* update the active device configuration */
+	SavageClearVM86Regs( psav->pVbe->pInt10 );
+	psav->pVbe->pInt10->ax = 0x4f14;	/* S3 extensions */
+	psav->pVbe->pInt10->bx = 0x0003;	/* set active devices */
+	psav->pVbe->pInt10->cx = iDevInfo;
+	xf86ExecX86int10( psav->pVbe->pInt10 );
+    }
+}
+
+/* Function to get supported device list. */
+
+int SavageGetDevice( SavagePtr psav )
+{
     SavageClearVM86Regs( psav->pVbe->pInt10 );
     psav->pVbe->pInt10->ax = 0x4f14;	/* S3 extensions */
-    psav->pVbe->pInt10->bx = 0x0003;	/* set active devices */
-    psav->pVbe->pInt10->cx = iDevInfo;
+    psav->pVbe->pInt10->bx = 0x0103;	/* get active devices */
+
     xf86ExecX86int10( psav->pVbe->pInt10 );
+
+    /* obey the "dual" bit at the MSB position by adding 0x80 to the mask */
+    return ((psav->pVbe->pInt10->cx) & 0x87); /* mask was 0xf, but active devices are only in the three lower bits */
 }
 
-/* Function to get supported device list. */
+/* Function to get TV format. */
 
-static int SavageGetDevice( SavagePtr psav )
+int SavageGetTV( SavagePtr psav )
 {
     SavageClearVM86Regs( psav->pVbe->pInt10 );
     psav->pVbe->pInt10->ax = 0x4f14;	/* S3 extensions */
-    psav->pVbe->pInt10->bx = 0x0103;	/* get active devices */
+    psav->pVbe->pInt10->bx = 0x0107;	/* get TV mode */
 
     xf86ExecX86int10( psav->pVbe->pInt10 );
 
-    return ((psav->pVbe->pInt10->cx) & 0xf);
+    return ((psav->pVbe->pInt10->cx) & 0xc); /* HJM: mask was 0xf, but TV mode bits are only in bits 2 and 3 */
 }
 
+/* Function to correct the DuoView flag in the case of a single display */
+
+int SavageCorrectDuoViewFlag(int iDevInfo, Bool bEnableActivate, Bool bEnableDeactivate)
+{
+    /* TODO: limit this function to be used only if no multi view mode is active ("real" DuoView) */
+
+    /*
+     * This function can be used to correctly set the DuoView flag in case of a single X display.
+     * It operates directly on an "iDevInfo" bit field, so the caller has the choice which one
+     * should be checked.
+     *
+     * The DuoView flag is only necessary as soon as the TV output is involved.
+     * Logic derived from the behaviour of the hardware features of HP XE3:
+     * - if LCD or CRT or both are active and TV is NOT active, DuoView NEVER needs to be set
+     * - if TV is the only active output, DuoView also does NOT need to be set
+     * - if TV is active together with LCD or CRT or both, DuoView MUST be set to allow all outputs to work
+     *
+     * The two Bool flags allow the caller to set which operations should be done on the bit field.
+     * To use the full logic mentioned above, set both flags to TRUE.
+     */
+
+    Bool bDoEnable = FALSE;
+    int iNewDevInfo = iDevInfo;
+
+    /* decision logic */
+    /* only use iDevInfo here to avoid side effects on the return value */
+    if ((iDevInfo & TV_ACTIVE) != 0) { /* TV enabled, check if either CRT or LCD is also on */
+	if (((iDevInfo & CRT_ACTIVE) != 0)
+	    || ((iDevInfo & LCD_ACTIVE) != 0)) {
+		bDoEnable = TRUE;
+	}
+    }
+
+    /* action logic */
+    if (bDoEnable == TRUE) {
+	if (bEnableActivate == TRUE) {
+	    iNewDevInfo |= DUO_ON;
+	}
+    } else { /* bDoEnable == FALSE */
+	if (bEnableDeactivate == TRUE) {
+	    iNewDevInfo &= ~DUO_ON;
+	}
+    }
+
+    return iNewDevInfo;
+}
 
 void
 SavageFreeBIOSModeTable( SavagePtr psav, SavageModeTablePtr* ppTable )
diff --git a/src/savage_video.c b/src/savage_video.c
index b5a6330..804d9fd 100644
--- a/src/savage_video.c
+++ b/src/savage_video.c
@@ -65,6 +65,8 @@ static int SavagePutImage( ScrnInfoPtr,
 static int SavageQueryImageAttributes(ScrnInfoPtr, 
 	int, unsigned short *, unsigned short *,  int *, int *);
 static void SavageFreeMemory(ScrnInfoPtr pScrn, void *mem_struct);
+/* function for online update of current devices and TV format */
+static void SavageUpdateDevices(ScrnInfoPtr pScrn);
 
 void SavageResetVideo(ScrnInfoPtr pScrn); 
 
@@ -1428,6 +1430,27 @@ SavageFreeMemory(
     }
 }
 
+/* function for online update of current devices and TV format */
+static void SavageUpdateDevices(ScrnInfoPtr pScrn)
+{
+    SavagePtr psav;
+    int iDeviceList;
+    int iTVFormatList;
+
+    psav = SAVPTR(pScrn);
+
+    iDeviceList = SavageGetDevice(psav);
+    iTVFormatList = SavageGetTV(psav);
+
+    /* start updating the devices */
+    psav->PAL = ((iTVFormatList & BIOS_TV_PAL) != 0);
+    psav->TvOn = ((iDeviceList & TV_ACTIVE) != 0);
+    /* set the CRT flag intelligently by obeying the word "only" */
+    psav->CrtOnly = ((iDeviceList & ((unsigned char) ~CRT_ACTIVE)) == 0);
+    /* update the current device list */
+    psav->iDevInfo = iDeviceList;
+}
+
 static void
 SavageSetBlend(ScrnInfoPtr pScrn, int id)
 {
@@ -1662,6 +1685,9 @@ SavageDisplayVideoNew(
 
     /* Calculate horizontal and vertical scale factors. */
 
+    /* update device list to get the correct prerequisites for the following calculations */
+    SavageUpdateDevices(pScrn);
+
     if ( S3_SAVAGE_MOBILE_SERIES(psav->Chipset) &&
 	    (psav->DisplayType == MT_LCD) &&
 	    !psav->CrtOnly &&
@@ -1910,6 +1936,7 @@ SavagePutImage(
     BoxRec dstBox;
     CARD32 tmp;
 /*    xf86ErrorFVerb(XVTRACE,"SavagePutImage\n"); */
+
     if(drw_w > 16384) drw_w = 16384;
 
     /* Clip */