xfree86: Remove 24bpp pixmap format support (v2)

There's really no reason to pretend to support this, apps hate it, all
we're doing is giving people a way to injure themselves. It doesn't work
anyway with any Radeon, any NVIDIA chip, or any Intel chip since i810.
Rip out all the logic for handling 24bpp pixmaps and framebuffers, and
silently ignore the old options that would ask for it.

The cirrus alpine driver has been updated to default to 16bpp, and both
it and the i810 driver can now use the 32->24 conversion code in shadow
if they want. All other drivers support 32bpp. Configurations that
explicitly request 24bpp in order to fit in VRAM will be broken now
though.

v2: Fix command line options to silently ignore 24bpp rather than fail

Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Adam Jackson <ajax@redhat.com>

10 files changed, 9 insertions, 238 deletions

diff --git a/hw/xfree86/common/xf86.h b/hw/xfree86/common/xf86.h
index 828bff1da..ee24835b0 100644
--- a/hw/xfree86/common/xf86.h
+++ b/hw/xfree86/common/xf86.h
@@ -91,9 +91,6 @@ extern _X_EXPORT Bool VTSwitchEnabled;  /* kbd driver */
 
 #define BOOLTOSTRING(b) ((b) ? "TRUE" : "FALSE")
 
-#define PIX24TOBPP(p) (((p) == Pix24Use24) ? 24 : \
-			(((p) == Pix24Use32) ? 32 : 0))
-
 /* Compatibility functions for pre-input-thread drivers */
 static inline _X_DEPRECATED int xf86BlockSIGIO(void) { input_lock(); return 0; }
 static inline _X_DEPRECATED void xf86UnblockSIGIO(int wasset) { input_unlock(); }
@@ -290,8 +287,6 @@ extern _X_EXPORT const char *
 xf86GetVisualName(int visual);
 extern _X_EXPORT int
 xf86GetVerbosity(void);
-extern _X_EXPORT Pix24Flags
-xf86GetPix24(void);
 extern _X_EXPORT int
 xf86GetDepth(void);
 extern _X_EXPORT rgb
diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
index 89861e0a7..49b898da1 100644
--- a/hw/xfree86/common/xf86Config.c
+++ b/hw/xfree86/common/xf86Config.c
@@ -628,7 +628,6 @@ typedef enum {
     FLAG_DPMS_STANDBYTIME,
     FLAG_DPMS_SUSPENDTIME,
     FLAG_DPMS_OFFTIME,
-    FLAG_PIXMAP,
     FLAG_NOPM,
     FLAG_XINERAMA,
     FLAG_LOG,
@@ -674,8 +673,6 @@ static OptionInfoRec FlagOptions[] = {
      {0}, FALSE},
     {FLAG_DPMS_OFFTIME, "OffTime", OPTV_INTEGER,
      {0}, FALSE},
-    {FLAG_PIXMAP, "Pixmap", OPTV_INTEGER,
-     {0}, FALSE},
     {FLAG_NOPM, "NoPM", OPTV_BOOLEAN,
      {0}, FALSE},
     {FLAG_XINERAMA, "Xinerama", OPTV_BOOLEAN,
@@ -715,7 +712,6 @@ configServerFlags(XF86ConfFlagsPtr flagsconf, XF86OptionPtr layoutopts)
 {
     XF86OptionPtr optp, tmp;
     int i;
-    Pix24Flags pix24 = Pix24DontCare;
     Bool value;
     MessageType from;
     const char *s;
@@ -922,34 +918,6 @@ configServerFlags(XF86ConfFlagsPtr flagsconf, XF86OptionPtr layoutopts)
                i, MAX_TIME_IN_MIN);
 #endif
 
-    i = -1;
-    xf86GetOptValInteger(FlagOptions, FLAG_PIXMAP, &i);
-    switch (i) {
-    case 24:
-        pix24 = Pix24Use24;
-        break;
-    case 32:
-        pix24 = Pix24Use32;
-        break;
-    case -1:
-        break;
-    default:
-        ErrorF("Pixmap option's value (%d) must be 24 or 32\n", i);
-        break;
-    }
-    if (xf86Pix24 != Pix24DontCare) {
-        xf86Info.pixmap24 = xf86Pix24;
-        xf86Info.pix24From = X_CMDLINE;
-    }
-    else if (pix24 != Pix24DontCare) {
-        xf86Info.pixmap24 = pix24;
-        xf86Info.pix24From = X_CONFIG;
-    }
-    else {
-        xf86Info.pixmap24 = Pix24DontCare;
-        xf86Info.pix24From = X_DEFAULT;
-    }
-
 #ifdef PANORAMIX
     from = X_DEFAULT;
     if (!noPanoramiXExtension)
diff --git a/hw/xfree86/common/xf86Globals.c b/hw/xfree86/common/xf86Globals.c
index e962b756b..ddf7a8696 100644
--- a/hw/xfree86/common/xf86Globals.c
+++ b/hw/xfree86/common/xf86Globals.c
@@ -117,8 +117,6 @@ xf86InfoRec xf86Info = {
     .vidModeAllowNonLocal = FALSE,
     .miscModInDevEnabled = TRUE,
     .miscModInDevAllowNonLocal = FALSE,
-    .pixmap24 = Pix24DontCare,
-    .pix24From = X_DEFAULT,
     .pmFlag = TRUE,
     .disableRandR = FALSE,
     .randRFrom = X_DEFAULT,
@@ -189,7 +187,6 @@ char *xf86KeyboardName = NULL;
 int xf86Verbose = DEFAULT_VERBOSE;
 int xf86LogVerbose = DEFAULT_LOG_VERBOSE;
 int xf86FbBpp = -1;
-Pix24Flags xf86Pix24 = Pix24DontCare;
 int xf86Depth = -1;
 rgb xf86Weight = { 0, 0, 0 };
 
diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c
index 7d6a3742d..b745793c7 100644
--- a/hw/xfree86/common/xf86Helper.c
+++ b/hw/xfree86/common/xf86Helper.c
@@ -344,33 +344,15 @@ xf86AddPixFormat(ScrnInfoPtr pScrn, int depth, int bpp, int pad)
  * Also find a Display subsection matching the depth/bpp found.
  *
  * Sets the following ScrnInfoRec fields:
- *     bitsPerPixel, pixmap24, depth, display, imageByteOrder,
+ *     bitsPerPixel, depth, display, imageByteOrder,
  *     bitmapScanlinePad, bitmapScanlineUnit, bitmapBitOrder, numFormats,
  *     formats, fbFormat.
  */
 
-/* Can the screen handle 24 bpp pixmaps */
-#define DO_PIX24(f) ((f & Support24bppFb) || \
-		     ((f & Support32bppFb) && (f & SupportConvert24to32)))
-
 /* Can the screen handle 32 bpp pixmaps */
 #define DO_PIX32(f) ((f & Support32bppFb) || \
 		     ((f & Support24bppFb) && (f & SupportConvert32to24)))
 
-/* Does the screen prefer 32bpp fb for 24bpp pixmaps */
-#define CHOOSE32FOR24(f) ((f & Support32bppFb) && (f & SupportConvert24to32) \
-			  && (f & PreferConvert24to32))
-
-/* Does the screen prefer 24bpp fb for 32bpp pixmaps */
-#define CHOOSE24FOR32(f) ((f & Support24bppFb) && (f & SupportConvert32to24) \
-			  && (f & PreferConvert32to24))
-
-/* Can the screen handle 32bpp pixmaps for 24bpp fb */
-#define DO_PIX32FOR24(f) ((f & Support24bppFb) && (f & SupportConvert32to24))
-
-/* Can the screen handle 24bpp pixmaps for 32bpp fb */
-#define DO_PIX24FOR32(f) ((f & Support32bppFb) && (f & SupportConvert24to32))
-
 #ifndef GLOBAL_DEFAULT_DEPTH
 #define GLOBAL_DEFAULT_DEPTH 24
 #endif
@@ -381,16 +363,15 @@ xf86SetDepthBpp(ScrnInfoPtr scrp, int depth, int dummy, int fbbpp,
 {
     int i;
     DispPtr disp;
-    Pix24Flags pix24 = xf86Info.pixmap24;
-    Bool nomatch = FALSE;
 
     scrp->bitsPerPixel = -1;
     scrp->depth = -1;
-    scrp->pixmap24 = Pix24DontCare;
     scrp->bitsPerPixelFrom = X_DEFAULT;
     scrp->depthFrom = X_DEFAULT;
 
     if (xf86FbBpp > 0) {
+        if (xf86FbBpp == 24) /* lol no */
+            xf86FbBpp = 32;
         scrp->bitsPerPixel = xf86FbBpp;
         scrp->bitsPerPixelFrom = X_CMDLINE;
     }
@@ -469,57 +450,14 @@ xf86SetDepthBpp(ScrnInfoPtr scrp, int depth, int dummy, int fbbpp,
                 scrp->bitsPerPixel = 8;
             else if (scrp->depth <= 16)
                 scrp->bitsPerPixel = 16;
-            else if (scrp->depth <= 24) {
-                /*
-                 * Figure out if a choice is possible based on the depth24
-                 * and pix24 flags.
-                 */
-                /* Check pix24 first */
-                if (pix24 != Pix24DontCare) {
-                    if (pix24 == Pix24Use32) {
-                        if (DO_PIX32(depth24flags)) {
-                            if (CHOOSE24FOR32(depth24flags))
-                                scrp->bitsPerPixel = 24;
-                            else
-                                scrp->bitsPerPixel = 32;
-                        }
-                        else {
-                            nomatch = TRUE;
-                        }
-                    }
-                    else if (pix24 == Pix24Use24) {
-                        if (DO_PIX24(depth24flags)) {
-                            if (CHOOSE32FOR24(depth24flags))
-                                scrp->bitsPerPixel = 32;
-                            else
-                                scrp->bitsPerPixel = 24;
-                        }
-                        else {
-                            nomatch = TRUE;
-                        }
-                    }
-                }
-                else {
-                    if (DO_PIX32(depth24flags)) {
-                        if (CHOOSE24FOR32(depth24flags))
-                            scrp->bitsPerPixel = 24;
-                        else
-                            scrp->bitsPerPixel = 32;
-                    }
-                    else if (DO_PIX24(depth24flags)) {
-                        if (CHOOSE32FOR24(depth24flags))
-                            scrp->bitsPerPixel = 32;
-                        else
-                            scrp->bitsPerPixel = 24;
-                    }
-                }
+            else if (scrp->depth <= 24 && DO_PIX32(depth24flags)) {
+                scrp->bitsPerPixel = 32;
             }
             else if (scrp->depth <= 32)
                 scrp->bitsPerPixel = 32;
             else {
                 xf86DrvMsg(scrp->scrnIndex, X_ERROR,
-                           "Specified depth (%d) is greater than 32\n",
-                           scrp->depth);
+                           "No bpp for depth (%d)\n", scrp->depth);
                 return FALSE;
             }
         }
@@ -530,11 +468,7 @@ xf86SetDepthBpp(ScrnInfoPtr scrp, int depth, int dummy, int fbbpp,
             return FALSE;
         }
         if (scrp->bitsPerPixel < 0) {
-            if (nomatch)
-                xf86DrvMsg(scrp->scrnIndex, X_ERROR,
-                           "Driver can't support depth 24 pixmap format (%d)\n",
-                           PIX24TOBPP(pix24));
-            else if ((depth24flags & (Support24bppFb | Support32bppFb)) ==
+            if ((depth24flags & (Support24bppFb | Support32bppFb)) ==
                      NoDepth24Support)
                 xf86DrvMsg(scrp->scrnIndex, X_ERROR,
                            "Driver can't support depth 24\n");
@@ -572,7 +506,6 @@ xf86SetDepthBpp(ScrnInfoPtr scrp, int depth, int dummy, int fbbpp,
     case 4:
     case 8:
     case 16:
-    case 24:
     case 32:
         break;
     default:
@@ -588,14 +521,6 @@ xf86SetDepthBpp(ScrnInfoPtr scrp, int depth, int dummy, int fbbpp,
         return FALSE;
     }
 
-    /* set scrp->pixmap24 if the driver isn't flexible */
-    if (scrp->bitsPerPixel == 24 && !DO_PIX32FOR24(depth24flags)) {
-        scrp->pixmap24 = Pix24Use24;
-    }
-    if (scrp->bitsPerPixel == 32 && !DO_PIX24FOR32(depth24flags)) {
-        scrp->pixmap24 = Pix24Use32;
-    }
-
     /*
      * Find the Display subsection matching the depth/fbbpp and initialise
      * scrp->display with it.
@@ -1455,12 +1380,6 @@ xf86GetVerbosity(void)
     return max(xf86Verbose, xf86LogVerbose);
 }
 
-Pix24Flags
-xf86GetPix24(void)
-{
-    return xf86Info.pixmap24;
-}
-
 int
 xf86GetDepth(void)
 {
diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index e61fe664b..45f8ae63d 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -381,9 +381,6 @@ InitOutput(ScreenInfo * pScreenInfo, int argc, char **argv)
     int i, j, k, scr_index;
     const char **modulelist;
     void **optionlist;
-    Pix24Flags screenpix24, pix24;
-    MessageType pix24From = X_DEFAULT;
-    Bool pix24Fail = FALSE;
     Bool autoconfig = FALSE;
     Bool sigio_blocked = FALSE;
     Bool want_hw_access = FALSE;
@@ -636,7 +633,6 @@ InitOutput(ScreenInfo * pScreenInfo, int argc, char **argv)
          * Collect all pixmap formats and check for conflicts at the display
          * level.  Should we die here?  Or just delete the offending screens?
          */
-        screenpix24 = Pix24DontCare;
         for (i = 0; i < xf86NumScreens; i++) {
             if (xf86Screens[i]->imageByteOrder !=
                 xf86Screens[0]->imageByteOrder)
@@ -652,38 +648,7 @@ InitOutput(ScreenInfo * pScreenInfo, int argc, char **argv)
             if (xf86Screens[i]->bitmapBitOrder !=
                 xf86Screens[0]->bitmapBitOrder)
                 FatalError("Inconsistent display bitmapBitOrder.  Exiting\n");
-
-            /* Determine the depth 24 pixmap format the screens would like */
-            if (xf86Screens[i]->pixmap24 != Pix24DontCare) {
-                if (screenpix24 == Pix24DontCare)
-                    screenpix24 = xf86Screens[i]->pixmap24;
-                else if (screenpix24 != xf86Screens[i]->pixmap24)
-                    FatalError
-                        ("Inconsistent depth 24 pixmap format.  Exiting\n");
-            }
-        }
-        /* check if screenpix24 is consistent with the config/cmdline */
-        if (xf86Info.pixmap24 != Pix24DontCare) {
-            pix24 = xf86Info.pixmap24;
-            pix24From = xf86Info.pix24From;
-            if (screenpix24 != Pix24DontCare &&
-                screenpix24 != xf86Info.pixmap24)
-                pix24Fail = TRUE;
         }
-        else if (screenpix24 != Pix24DontCare) {
-            pix24 = screenpix24;
-            pix24From = X_PROBED;
-        }
-        else
-            pix24 = Pix24Use32;
-
-        if (pix24Fail)
-            FatalError("Screen(s) can't use the required depth 24 pixmap format"
-                       " (%d).  Exiting\n", PIX24TOBPP(pix24));
-
-        /* Initialise the depth 24 format */
-        for (j = 0; j < numFormats && formats[j].depth != 24; j++);
-        formats[j].bitsPerPixel = PIX24TOBPP(pix24);
 
         /* Collect additional formats */
         for (i = 0; i < xf86NumScreens; i++) {
@@ -709,15 +674,6 @@ InitOutput(ScreenInfo * pScreenInfo, int argc, char **argv)
             }
         }
         formatsDone = TRUE;
-
-        /* If a screen uses depth 24, show what the pixmap format is */
-        for (i = 0; i < xf86NumScreens; i++) {
-            if (xf86Screens[i]->depth == 24) {
-                xf86Msg(pix24From, "Depth 24 pixmap format is %d bpp\n",
-                        PIX24TOBPP(pix24));
-                break;
-            }
-        }
     }
     else {
         /*
@@ -1244,12 +1200,8 @@ ddxProcessArgument(int argc, char **argv, int i)
         xf86sFlag = TRUE;
         return 0;
     }
-    if (!strcmp(argv[i], "-pixmap24")) {
-        xf86Pix24 = Pix24Use24;
-        return 1;
-    }
-    if (!strcmp(argv[i], "-pixmap32")) {
-        xf86Pix24 = Pix24Use32;
+    if (!strcmp(argv[i], "-pixmap32") || !strcmp(argv[i], "-pixmap24")) {
+        /* silently accept */
         return 1;
     }
     if (!strcmp(argv[i], "-fbbpp")) {
@@ -1425,8 +1377,6 @@ ddxUseMsg(void)
     ErrorF("-verbose [n]           verbose startup messages\n");
     ErrorF("-logverbose [n]        verbose log messages\n");
     ErrorF("-quiet                 minimal startup messages\n");
-    ErrorF("-pixmap24              use 24bpp pixmaps for depth 24\n");
-    ErrorF("-pixmap32              use 32bpp pixmaps for depth 24\n");
     ErrorF("-fbbpp n               set bpp for the framebuffer. Default: 8\n");
     ErrorF("-depth n               set colour depth. Default: 8\n");
     ErrorF
@@ -1518,30 +1468,6 @@ PixmapFormatPtr
 xf86GetPixFormat(ScrnInfoPtr pScrn, int depth)
 {
     int i;
-    static PixmapFormatRec format;      /* XXX not reentrant */
-
-    /*
-     * When the formats[] list initialisation isn't complete, check the
-     * depth 24 pixmap config/cmdline options and screen-specified formats.
-     */
-
-    if (!formatsDone) {
-        if (depth == 24) {
-            Pix24Flags pix24 = Pix24DontCare;
-
-            format.depth = 24;
-            format.scanlinePad = BITMAP_SCANLINE_PAD;
-            if (xf86Info.pixmap24 != Pix24DontCare)
-                pix24 = xf86Info.pixmap24;
-            else if (pScrn->pixmap24 != Pix24DontCare)
-                pix24 = pScrn->pixmap24;
-            if (pix24 == Pix24Use24)
-                format.bitsPerPixel = 24;
-            else
-                format.bitsPerPixel = 32;
-            return &format;
-        }
-    }
 
     for (i = 0; i < numFormats; i++)
         if (formats[i].depth == depth)
diff --git a/hw/xfree86/common/xf86Priv.h b/hw/xfree86/common/xf86Priv.h
index c1f8a18d7..2663c6b06 100644
--- a/hw/xfree86/common/xf86Priv.h
+++ b/hw/xfree86/common/xf86Priv.h
@@ -67,7 +67,6 @@ extern _X_EXPORT char *xf86PointerName;
 extern _X_EXPORT char *xf86KeyboardName;
 extern _X_EXPORT int xf86FbBpp;
 extern _X_EXPORT int xf86Depth;
-extern _X_EXPORT Pix24Flags xf86Pix24;
 extern _X_EXPORT rgb xf86Weight;
 extern _X_EXPORT Bool xf86FlipPixels;
 extern _X_EXPORT Gamma xf86Gamma;
diff --git a/hw/xfree86/common/xf86Privstr.h b/hw/xfree86/common/xf86Privstr.h
index c29b3ccc8..bb8f3ab68 100644
--- a/hw/xfree86/common/xf86Privstr.h
+++ b/hw/xfree86/common/xf86Privstr.h
@@ -81,8 +81,6 @@ typedef struct {
                                  * changed */
     Bool miscModInDevAllowNonLocal;
     Bool useSIGIO;              /* Use SIGIO for handling DRI1 swaps */
-    Pix24Flags pixmap24;
-    MessageType pix24From;
     Bool pmFlag;
     Bool disableRandR;
     MessageType randRFrom;
diff --git a/hw/xfree86/common/xf86str.h b/hw/xfree86/common/xf86str.h
index 01d12ab69..1da66e2dd 100644
--- a/hw/xfree86/common/xf86str.h
+++ b/hw/xfree86/common/xf86str.h
@@ -419,13 +419,6 @@ typedef struct _confdrirec {
 
 typedef void *(*funcPointer) (void);
 
-/* flags for depth 24 pixmap options */
-typedef enum {
-    Pix24DontCare = 0,
-    Pix24Use24,
-    Pix24Use32
-} Pix24Flags;
-
 /* Power management events: so far we only support APM */
 
 typedef enum {
@@ -592,7 +585,6 @@ typedef struct _ScrnInfoRec {
     PixmapFormatRec fbFormat;
 
     int bitsPerPixel;           /* fb bpp */
-    Pix24Flags pixmap24;        /* pixmap pref for depth 24 */
     int depth;                  /* depth of default visual */
     MessageType depthFrom;      /* set from config? */
     MessageType bitsPerPixelFrom;       /* set from config? */
diff --git a/hw/xfree86/man/Xorg.man b/hw/xfree86/man/Xorg.man
index def9bfc1f..68672172c 100644
--- a/hw/xfree86/man/Xorg.man
+++ b/hw/xfree86/man/Xorg.man
@@ -290,21 +290,6 @@ Disable Silken Mouse support.
 Disable the automatic switching on X server reset and shutdown to the
 VT that was active when the server started, if supported by the OS.
 .TP 8
-.B \-pixmap24
-Set the internal pixmap format for depth 24 pixmaps to 24 bits per pixel.
-The default is usually 32 bits per pixel.  There is normally little
-reason to use this option.  Some client applications don't like this
-pixmap format, even though it is a perfectly legal format.  This is
-equivalent to the
-.B Pixmap
-xorg.conf(__filemansuffix__) file option.
-.TP 8
-.B \-pixmap32
-Set the internal pixmap format for depth 24 pixmaps to 32 bits per pixel.
-This is usually the default.  This is equivalent to the
-.B Pixmap
-xorg.conf(__filemansuffix__) file option.
-.TP 8
 .BI \-pointer " pointer-name"
 Use the xorg.conf(__filemansuffix__) file
 .B InputDevice
diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man
index 25af3aa24..0c39062e6 100644
--- a/hw/xfree86/man/xorg.conf.man
+++ b/hw/xfree86/man/xorg.conf.man
@@ -616,14 +616,6 @@ option set (see the MONITOR section below).
 Set the maximum number of clients allowed to connect to the X server.
 Acceptable values are 64, 128, 256 or 512.
 .TP 7
-.BI "Option \*qPixmap\*q  \*q" bpp \*q
-This sets the pixmap format to use for depth 24.
-Allowed values for
-.I bpp
-are 24 and 32.
-Default: 32 unless driver constraints don't allow this (which is rare).
-Note: some clients don't behave well when this value is set to 24.
-.TP 7
 .BI "Option \*qNoPM\*q  \*q" boolean \*q
 Disables something to do with power management events.
 Default: PM enabled on platforms that support it.