To be honest, I am rather green to this, but I believe I have found some
solutions to problems in the Rendition driver.  Given my relative lack of
experience, I thought I would post them here for anyone interested in
reviewing them before submitting them for inclusion.  For anyone who is
interested, I have also included a gzipped copy of the file I changed
(xc/programs/Xserver/hw/xfree86/drivers/rendition/rendition.c) for
convenience.  

At this point, I suspect it is rather late in the game to get these
changes into 4.4, so I completely understand if this has to sit on the
back burner for awhile.  Furthermore, even with these changes, the driver
still has a (previously-documented) system lock problem on switches to
text mode.  Absent hardware documentation, I have my doubts that I can fix
that one.  

Until that system lock issue is fixed, I doubt that this driver will be
ready for general use among Verite 2100 and 2200 users.  However, these
fixes might make it suitable for general use by Verite 1000 users.  



I. SEGMENTATION FAULT ON SERVER EXIT WITH HARDWARE CURSOR

The Rendition driver currently causes a segmentation fault when its
hardware cursor is enabled and the X server tries to exit (leaving the
console unusable).  

To correct this issue, I recommend that the call to
RenditionHWCursorRelease in renditionCloseScreen (see #9) be moved to
AFTER the call to the previously-wrapped close screen function (see #11). 
This change will ensure that a structure is not freed until it is no
longer needed.  

The core dump was caused by the following sequence of events (accompanied
by corresponding code snippets):  

1. During ScreenInit, the Rendition driver calls its hardware cursor
initialization function unless the software cursor has been enabled:  

    /* From renditionScreenInit function in                         */
    /* xc/programs/Xserver/hw/xfree86/drivers/rendition/rendition.c */

    if(!xf86ReturnOptValBool(pRendition->Options, OPTION_SW_CURSOR,0)&&
       !pRendition->board.rotate){
        /* Initialise HW cursor */
        if(!RenditionHWCursorInit(scrnIndex, pScreen)){
            xf86DrvMsg(pScreenInfo->scrnIndex, X_ERROR,
                       "Hardware Cursor initalization failed!!\n");
        }
    }

2.  The Rendition driver's hardware cursor initialization function
allocates a cursor information record, then saves a pointer to this record
in the driver's private structure.  

    /* From renditionHWCursorInit function in                       */
    /* xc/programs/Xserver/hw/xfree86/drivers/rendition/hwcursor.c */

    infoPtr = xf86CreateCursorInfoRec();
    if(!infoPtr) return FALSE;

    pRendition->CursorInfoRec = infoPtr;

3.  The Rendition driver designates the RENDITIONHideCursor function as
the hide cursor function in the cursor information record.  
    
    /* From renditionHWCursorInit function in                       */
    /* xc/programs/Xserver/hw/xfree86/drivers/rendition/hwcursor.c */

    infoPtr->HideCursor           = RENDITIONHideCursor;

4.  The Rendition driver calls the standard cursor initialization
function, passing the cursor information pointer.

    /* From renditionHWCursorInit function in                       */
    /* xc/programs/Xserver/hw/xfree86/drivers/rendition/hwcursor.c */

    return xf86InitCursor(pScreen, infoPtr);


5.  The standard cursor initialization function calls the standard
hardware cursor initialization function, passing the cursor information
pointer.  

    /* From xf86InitCursor function in                    */
    /* xc/programs/Xserver/hw/xfree86/ramdac/xf86Cursor.c */

    if (!xf86InitHardwareCursor(pScreen, infoPtr))
        return FALSE;

6.  The standard hardware cursor intialization function saves the
ScrnInfoPtr to the cursor information pointer.  

    /* From xf86InitHardwareCursor function in            */
    /* xc/programs/Xserver/hw/xfree86/ramdac/xf86HWCurs.c */

    infoPtr->pScrn = xf86Screens[pScreen->myNum];

7.  After the hardware cursor initialization function has returned, the
standard cursor initialization function saves the cursor information
pointer in the private cursor data for the screen.  

    /* From xf86InitCursor function in                    */
    /* xc/programs/Xserver/hw/xfree86/ramdac/xf86Cursor.c */

    ScreenPriv->CursorInfoPtr = infoPtr;

8.  At this point, the cursor information record has a copy of the
ScrnInfoPtr (see #6).  Furthermore, copies of the pointer to the cursor
information record have been saved to the private structure for the
Rendition driver (see #2) and the private cursor data for the screen (see
#7)

8.  After serving clients as appropriate, the server exits.  This exit
processing closes the screen, calling the renditionCloseScreen function in
the process.  

9.  Noticing that the hardware cursor is in use, the renditionCloseScreen
calls the RenditionHWCursorRelease function.  

    /* From renditionCloseScreen function in                         */
    /* xc/programs/Xserver/hw/xfree86/drivers/rendition/rendition.c  */

    if (prenditionPriv->board.hwcursor_used)
        RenditionHWCursorRelease(pScreenInfo);

10. The RenditionHWCursorRelease function frees the cursor information
record using the pointer saved to the Rendition driver's private structure
(see #2).  

    /* From renditionHWCursorRelease function in                   */
    /* xc/programs/Xserver/hw/xfree86/drivers/rendition/hwcursor.c */

    xf86DestroyCursorInfoRec(pRendition->CursorInfoRec);
    pRendition->CursorInfoRec=NULL;

11. After the RenditionHWCursorRelease function returns, the
renditionCloseScreen function unwraps its own close screen, then calls the
previously wrapped close screen function.  

    /* From renditionCloseScreen function in                         */
    /* xc/programs/Xserver/hw/xfree86/drivers/rendition/rendition.c  */

    if (prenditionPriv
        && (pScreen->CloseScreen = prenditionPriv->CloseScreen)) {
        prenditionPriv->CloseScreen = NULL;
        Closed = (*pScreen->CloseScreen)(scrnIndex, pScreen);
    }

12. Provided that everything works as it should, the xf86CursorCloseScreen
function should be called as various layers set up by the driver unwrap
themselves.  (See #4 for the cursor layer.)  

13. The xf86CursorCloseScreen function calls xf86SetCursor.  

    /* From xf86CursorCloseScreen function in             */
    /* xc/programs/Xserver/hw/xfree86/ramdac/xf86Cursor.c */

    if (ScreenPriv->isUp && pScrn->vtSema)
        xf86SetCursor(pScreen, NullCursor, ScreenPriv->x, ScreenPriv->y);

14. The xf86SetCursor function retrieves the cursor information pointer
stored in the private cursor data for the screen (see #7).  Note that the
memory designated by this pointer has already been freed (see #10).

    /* From xf86SetCursor function in                     */
    /* xc/programs/Xserver/hw/xfree86/ramdac/xfHW86Curs.c */

    xf86CursorInfoPtr infoPtr = ScreenPriv->CursorInfoPtr;


15. The xf86SetCursor function calls the installed hide cursor function,
passing the ScrnInfoPtr that should be stored in the cursor information
record (see #6).  Note that the hide cursor function should be
RENDITIONHideCursor (see #3).  

    /* From xf86SetCursor function in                     */
    /* xc/programs/Xserver/hw/xfree86/ramdac/xfHW86Curs.c */

    if (pCurs == NullCursor) {
        (*infoPtr->HideCursor)(infoPtr->pScrn);
        return;
    }

16. The RENDITIONHideCursor function calls the verite_enablecursor
function, passing the ScrnInfoPtr passed to it.  

    /* From renditionHideCursor function in                        */
    /* xc/programs/Xserver/hw/xfree86/drivers/rendition/hwcursor.c */

    verite_enablecursor(pScreenInfo, VERITE_NOCURSOR, 0);

17. The verite_enablecursor function attempts to use the passed
ScrnInfoPtr (see
xc/programs/Xserver/hw/xfree86/drivers/rendition/hwcursor.c).  At this
point, the pointer is likely junk, as it pulled from a structure whose
memory had already been freed (see #14 and #10).  This is where I observed
the segmentation fault, though it could have happened as early as at #14. 




II. DRIVER DOES NOT SUPPORT RESOLUTIONS SUPPORTED BY HARDWARE

The Rendition driver does not support resolutions that the Verite 1000 and
Verite 2100/2200 have been shown to support under Windows. 
Back-of-the-box listings for the Verite 1000 cited the ability to run at
1280x1024.  Similarly, the Verite 2100/2200 was advertised to run up to
1600x1200.  While I cannot personally vouch for the Verite 1000, I have
run a Diamond Stealth II S220 up to 1600x1200.  

The following code, from the renditionPreInit function, establishes the
limits against which modes are checked for the driver.  

    /* From renditionPreInit function in                             */
    /* xc/programs/Xserver/hw/xfree86/drivers/rendition/rendition.c  */

    /*
     * Validate the modes.  Note that the limits passed to
     * xf86ValidateModes() are VGA CRTC architectural limits.
     */
    pScreenInfo->maxHValue = 2080;
    pScreenInfo->maxVValue = 1025;
    nModes = xf86ValidateModes(pScreenInfo,
            pScreenInfo->monitor->Modes, pScreenInfo->display->modes,
            &renditionClockRange, NULL, 8, 2040, Rounding, 1, 1024,
            pScreenInfo->display->virtualX,
pScreenInfo->display->virtualY,
            0x10000, LOOKUP_CLOSEST_CLOCK | LOOKUP_CLKDIV2);

I recommend the following replacement code:  

    /*
     * Set the horizontal and vertical limits for the screen 
     * and then validate the modes.  
     */
    if (PCI_CHIP_V1000 == pRendition->PciInfo->chipType) {
       pScreenInfo->maxHValue = 2047;
       pScreenInfo->maxVValue = 2047;
    } else {
       pScreenInfo->maxHValue = 4095;
       pScreenInfo->maxVValue = 2047;
    }
    nModes = xf86ValidateModes( pScreenInfo,
            pScreenInfo->monitor->Modes, pScreenInfo->display->modes,
            &renditionClockRange, NULL, 8, pScreenInfo->maxHValue,
            Rounding, 1, pScreenInfo->maxVValue,
            pScreenInfo->display->virtualX,
pScreenInfo->display->virtualY,
            0x10000, LOOKUP_CLOSEST_CLOCK | LOOKUP_CLKDIV2 );

This code replaces the old horizontal and vertial limits with new ones.  I
do not know what the physical limits of the hardware, so I assumed that
the hardware limits are powers of two minus one.  Thus, I chose the lowest
number that was equal to a power of two minus one that exceeded the
requirements of the highest resolution I knew the hardware to support. 
For the Verite 1000, these assumptions lead one to to 2048 horizontal and
2048 vertical limits (1023 < 1280 < 2047 and 1023 < 1024 < 2047,
respectively).  

In the case of the Verite 2100/2200, these assumptions would lead one to
presume the same limits as the Verite 1000 (1023 < 1600 < 2047 and 1023 <
1200 < 2047, respectively).  However, I have personally tested my Verite
2100 beyond 2047 horizontally (including overdraw).  As a result, I
presume that the limit for the Verite 2100/2200 hardware is at least 4095.
 

I have made these changes to the maximums stored using the ScrnInfoPtr.  I
have also changed the code to pass the limits set using the ScrnInfoPtr
directly to the call to xf86ValidateModes.  The original code saved one
set of limits to the ScrnInfoPtr, but passed another set to the
xf86ValidateModes functions.  I may be missing something, but I'm at a
loss to explain why this was done.  



III.  COLOR CORRUPTION FROM VT-SWITCHING

I have observed color corruption from VT-switching running at an 8 bits
per pixel bit depth on my Verite 2100.  In my testing, the only way I
found to correct the colors was to start a new server generation.  This
led me to compare the screen initialization and VT entry functions for the
driver, and I was able to trace the correct colors at the start of a
server generate to a call to the xf86HandleColormaps function.  The
relevant code excerpt follows:  

    /* From renditionPreInit function in                             */
    /* xc/programs/Xserver/hw/xfree86/drivers/rendition/rendition.c  */

    /* Try the new code based on the new colormap layer */
    if (pScreenInfo->depth > 1)
        if (!xf86HandleColormaps(pScreen, 256, pScreenInfo->rgbBits,
                                 renditionLoadPalette, NULL,
                                 CMAP_LOAD_EVEN_IF_OFFSCREEN|
                                 CMAP_RELOAD_ON_MODE_SWITCH)) {
            xf86DrvMsg(pScreenInfo->scrnIndex, X_ERROR,
                       "Colormap initialization failed\n");
            return FALSE;
        }

I recommend replacing the excerpt with the following code:  

    /* Try the new code based on the new colormap layer */
    if (pScreenInfo->depth > 1) {
        (pScreenInfo->vtSema) = TRUE;
        if (!xf86HandleColormaps(pScreen, 256, pScreenInfo->rgbBits,
                                 renditionLoadPalette, NULL,
                                 CMAP_RELOAD_ON_MODE_SWITCH)) {
            xf86DrvMsg(pScreenInfo->scrnIndex, X_ERROR,
                       "Colormap initialization failed\n");
            return FALSE;
        }
        (pScreenInfo->vtSema) = FALSE;
    }

I realize that the driver should almost certainly be making more
widespread use of the vtSema flag, but my current approach is to keep
changes as small as possible in an effort to stabilize the driver without
introducing other problems.  I also realize that, given that I have only
observed the problem at an 8-bit color depth, making this change apply to
all color depths might go farther than is absolutely necessary, but I have
not seen this cause a problem in the other depths, and I do not think many
are likely to switch modes enough to make what may be an unecessary
palette reloads a real problem.  For similar reasons, I have left the
CMAP_RELOAD_ON_MODE_SWITCH flag in place, though it does not seem to be 
necessary.  

I have arrived at this change after some investigation to how the initial
code snippet affected the palette.   The portions I have decided are of
most interest are the flags CMAP_LOAD_EVEN_IF_OFFSCREEN and
CMAP_RELOAD_ON_MODE_SWITCH.  Checking the code to this function, I found
that the flags are used as follows:  

    /* From xf86HandleColormaps function in              */
    /* xc/programs/Xserver/hw/xfree86/common/xf86cmap.c  */

    if (!(flags & CMAP_LOAD_EVEN_IF_OFFSCREEN)) {
        pScrn->EnterVT = CMapEnterVT;
        if ((flags & CMAP_RELOAD_ON_MODE_SWITCH) && pScrn->SwitchMode)
            pScrn->SwitchMode = CMapSwitchMode;
    }

According to what I have read, the HandleColormaps function assures that
the palette is reloaded when upon VT entry UNLESS the
CMAP_LOAD_EVEN_IF_OFFSCREEN flag is specified, as the Rendition currently
driver does.  

Removing this flag, however, does not completely resolve the problem. 
When I first removed the CMAP_LOAD_EVEN_IF_OFFSCREEN flag, I found that
the problem had reversed:  the colors were corrupted at the start of
server generations (both first and subsequent), but could be corrected by
VT-switching or by a mode change.  

Since the palette was being reloaded after VT-switching and mode switching
(by my reading of the code), I decided to look at what was being done to
restore the palette in the functions the xf86HandeColormaps function
installed for these purposes.  I came up with the following snippets:  

    /* From CMapEnterVT function in                      */
    /* xc/programs/Xserver/hw/xfree86/common/xf86cmap.c  */

    if((*pScreenPriv->EnterVT)(index, flags)) {
        if(miInstalledMaps[index])
            CMapReinstallMap(miInstalledMaps[index]);
        return TRUE;
    }

    /* From CMapSwitchMode function in                   */
    /* xc/programs/Xserver/hw/xfree86/common/xf86cmap.c  */

    if((*pScreenPriv->SwitchMode)(index, mode, flags)) {
        if(miInstalledMaps[index])
            CMapReinstallMap(miInstalledMaps[index]);
        return TRUE;
    }

    /* From CMapReinstallMap function in                 */
    /* xc/programs/Xserver/hw/xfree86/common/xf86cmap.c  */
    /* NOTE:  clipped the surrounding conditionals for   */
    /* clarity                                           */

        (*pScrn->LoadPalette)(pScrn, cmapPriv->numColors,
                        indices, cmapPriv->colors, pmap->pVisual);

In short, the functions responsible for refreshing the palette after VT
switching or a mode change (CMapEnterVt and CMapModeSwitch, respectively)
call CMapReinstallMap, which in turns calls the registered load palette
function.  

>From here, I looked for how the palette would be loaded if the
CMAP_LOAD_EVEN_IF_OFFSCREEN flag were set.  That search uncovered the
following snippets:

    /* From xf86HandleColormaps function in              */
    /* xc/programs/Xserver/hw/xfree86/common/xf86cmap.c  */

    CMapInstallColormap(pDefMap);

    /* From CMapInstallColormap function in              */
    /* xc/programs/Xserver/hw/xfree86/common/xf86cmap.c  */

    if(LOAD_PALETTE(pmap, index))
        CMapReinstallMap(pmap);

    /* Define at the beginning of                        */
    /* xc/programs/Xserver/hw/xfree86/common/xf86cmap.c  */

#define LOAD_PALETTE(pmap, index) \
    ((pmap == miInstalledMaps[index]) && \
     ((pScreenPriv->flags & CMAP_LOAD_EVEN_IF_OFFSCREEN) || \
      xf86Screens[index]->vtSema || pScreenPriv->isDGAmode))

In short, the initial load of the palette would normally be initiated by
the CMapInstallColormap function.  This function calls CMapReinstallMap
which, in turn, calls the registered function for loading the palette. 
However, there's a catch:  CMapInstallColormap only calls CMapReinstallMap
(and thus, only loads the palette) when a number of conditions are met. 
One of these conditions is having the CMAP_LOAD_EVEN_IF_OFFSCREEN flag
set.  

Since setting the CMAP_LOAD_EVEN_IF_OFFSCREEN flag would bring the driver
back to square one, I looked into meeting the other conditions.  From what
I have seen, setting the vtSema flag seems to be the correct approach
(please see the expected replacement code).  

=====
Eric Wittry
[EMAIL PROTECTED]

__________________________________
Do you Yahoo!?
Find out what made the Top Yahoo! Searches of 2003
http://search.yahoo.com/top2003

Attachment: rendition.c.gz
Description: rendition.c.gz

Reply via email to