[PATCH xserver 1/5] xfree86: Remove 24bpp pixmap format support

Adam Jackson ajax at redhat.com
Wed Dec 7 20:08:39 UTC 2016


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.

The i810 driver defaults to 16bpp since (among other things) DRI only
works at that depth. This change will mean 24bpp no longer works at all
on i810; we should pull the 32->24 shadow code up from the modesetting
driver to dix so _some_ fallback path can work there, or else paste it
into the i810 driver too.

The cirrus alpine driver will also break, because it prefers 24 to 16
and doesn't support 32bpp at all. That was my fault in the first place,
I'm pretty sure; those chips should just default to 16bpp because cirrus
cards are terribly low on VRAM in general. Also, if you're using the KMS
driver for cirrus (i.e. qemu) you won't notice this, because we're
already doing the 32->24 shadow there.

All other drivers with 24bpp support also have 32bpp support, as far as
I can tell.

Signed-off-by: Adam Jackson <ajax at redhat.com>
---
 hw/xfree86/common/xf86.h        |  5 ---
 hw/xfree86/common/xf86Config.c  | 32 ----------------
 hw/xfree86/common/xf86Globals.c |  3 --
 hw/xfree86/common/xf86Helper.c  | 84 +++--------------------------------------
 hw/xfree86/common/xf86Init.c    | 69 ++-------------------------------
 hw/xfree86/common/xf86Priv.h    |  1 -
 hw/xfree86/common/xf86Privstr.h |  2 -
 hw/xfree86/common/xf86str.h     |  8 ----
 hw/xfree86/man/Xorg.man         | 15 --------
 hw/xfree86/man/xorg.conf.man    |  8 ----
 10 files changed, 9 insertions(+), 218 deletions(-)

diff --git a/hw/xfree86/common/xf86.h b/hw/xfree86/common/xf86.h
index f724688..086ab53 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 21daf1a..adfe87b 100644
--- a/hw/xfree86/common/xf86Config.c
+++ b/hw/xfree86/common/xf86Config.c
@@ -704,7 +704,6 @@ typedef enum {
     FLAG_DPMS_STANDBYTIME,
     FLAG_DPMS_SUSPENDTIME,
     FLAG_DPMS_OFFTIME,
-    FLAG_PIXMAP,
     FLAG_NOPM,
     FLAG_XINERAMA,
     FLAG_LOG,
@@ -750,8 +749,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,
@@ -791,7 +788,6 @@ configServerFlags(XF86ConfFlagsPtr flagsconf, XF86OptionPtr layoutopts)
 {
     XF86OptionPtr optp, tmp;
     int i;
-    Pix24Flags pix24 = Pix24DontCare;
     Bool value;
     MessageType from;
     const char *s;
@@ -998,34 +994,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 e962b75..ddf7a86 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 6f3a608..297196c 100644
--- a/hw/xfree86/common/xf86Helper.c
+++ b/hw/xfree86/common/xf86Helper.c
@@ -351,33 +351,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
@@ -388,12 +370,10 @@ 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;
 
@@ -477,48 +457,12 @@ xf86SetDepthBpp(ScrnInfoPtr scrp, int depth, int dummy, int fbbpp,
             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;
-                        }
-                    }
+                /* Verify that the screen can do 32bpp */
+                if (DO_PIX32(depth24flags)) {
+                    scrp->bitsPerPixel = 32;
                 }
                 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;
-                    }
+                    nomatch = TRUE;
                 }
             }
             else if (scrp->depth <= 32)
@@ -539,8 +483,7 @@ xf86SetDepthBpp(ScrnInfoPtr scrp, int depth, int dummy, int fbbpp,
         if (scrp->bitsPerPixel < 0) {
             if (nomatch)
                 xf86DrvMsg(scrp->scrnIndex, X_ERROR,
-                           "Driver can't support depth 24 pixmap format (%d)\n",
-                           PIX24TOBPP(pix24));
+                           "Driver can't support 32bpp pixmap format\n");
             else if ((depth24flags & (Support24bppFb | Support32bppFb)) ==
                      NoDepth24Support)
                 xf86DrvMsg(scrp->scrnIndex, X_ERROR,
@@ -579,7 +522,6 @@ xf86SetDepthBpp(ScrnInfoPtr scrp, int depth, int dummy, int fbbpp,
     case 4:
     case 8:
     case 16:
-    case 24:
     case 32:
         break;
     default:
@@ -595,14 +537,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.
@@ -1462,12 +1396,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 a544b65..75a27c8 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;
+        /* 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
@@ -1520,25 +1470,12 @@ 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.
-     */
-
+    /* XXX this could be simpler now? */
     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;
+            format.bitsPerPixel = 32;
             return &format;
         }
     }
diff --git a/hw/xfree86/common/xf86Priv.h b/hw/xfree86/common/xf86Priv.h
index c1f8a18..2663c6b 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 c29b3cc..bb8f3ab 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 bfcb75e..cdc800e 100644
--- a/hw/xfree86/common/xf86str.h
+++ b/hw/xfree86/common/xf86str.h
@@ -431,13 +431,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 {
@@ -608,7 +601,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 def9bfc..6867217 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 7d0c524..6edaf07 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.
-- 
2.9.3



More information about the xorg-devel mailing list