[PATCH] Add configuration option for use of SIGIO handlers for input events

Peter Hutterer peter.hutterer at who-t.net
Fri Sep 11 23:12:49 PDT 2009


On Fri, Sep 11, 2009 at 04:46:37PM -0700, Alan Coopersmith wrote:
> Boolean option to enable/disable SIGIO handlers is set by the first
> of these found:
>   - UseSIGIO option is set in xorg.conf ServerFlags
>   - Default set at build time by ./configure --enable-sigio-default
>   - Platform default value: Solaris = no, all others = yes
> 
> This matches the current settings on all platforms except Solaris.
> This reverts Solaris (for now) to the settings used in Xorg 1.6, before
> SIGIO support for Solaris was added, due to some system level bugs that
> won't be resolved in time for Xorg 1.7 release, but allows us to enable
> when those are resolved (or when we need to test if they're resolved).
> See http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6879897
> 
> Signed-off-by: Alan Coopersmith <alan.coopersmith at sun.com>

Do you plan to make this a permanent option or just for the meantime?
If it is just for the meantime, I'd prefer some warnings with both the
--enable-sigio option and the xorg.conf option that this is not considered a
stable configuration and may be ignored in the future.

> ---
> 
>  configure.ac                         |   16 ++++++++++++++++
>  hw/xfree86/common/xf86Config.c       |   19 +++++++++++++++++++
>  hw/xfree86/common/xf86Helper.c       |    2 +-
>  hw/xfree86/common/xf86Privstr.h      |    2 ++
>  hw/xfree86/os-support/shared/sigio.c |    8 ++++++++
>  include/xorg-config.h.in             |    3 +++
>  6 files changed, 49 insertions(+), 1 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 6345fd9..2130612 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -379,6 +379,7 @@ AM_CONDITIONAL(PPC_VIDEO, [test "x$PPC_VIDEO" = xyes])
>  AM_CONDITIONAL(SPARC64_VIDEO, [test "x$SPARC64_VIDEO" = xyes])
>  
>  DRI=no
> +SIGIO_DEFAULT="yes"

SIGIO_DEFAULT and --enable-sigio-default vs. FLAG_USE_SIGIO and .useSIGIO.
It's probably better to be consistent with the naming in configure and add
USE_SIGIO and --enable-sigio instead.

>  dnl it would be nice to autodetect these *CONS_SUPPORTs
>  case $host_os in
>    *freebsd* | *dragonfly*)
> @@ -408,6 +409,9 @@ case $host_os in
>  	;;
>    *solaris*)
>  	PKG_CHECK_EXISTS(libdrm, DRI=yes, DRI=no)
> +	# Disable use of SIGIO by default until some system bugs are
> +	# fixed - see Sun/OpenSolaris bug id 6879897
> +	SIGIO_DEFAULT="no"
>  	;;
>    darwin*)
>  	AC_DEFINE(CSRG_BASED, 1, [System is BSD-like])
> @@ -442,6 +446,9 @@ AC_ARG_ENABLE(debug,         AS_HELP_STRING([--enable-debug],
>  AC_ARG_ENABLE(unit-tests,    AS_HELP_STRING([--enable-unit-tests],
>                                    [Enable unit-tests (default: auto)]),
>                                  [UNITTESTS=$enableval], [UNITTESTS=auto])
> +AC_ARG_ENABLE(sigio-default, AS_HELP_STRING([--enable-sigio-default]
> +	  [Enable SIGIO input handlers by default (default: $SIGIO_DEFAULT)]),
> +                                [SIGIO_DEFAULT=$enableval], [])
>  AC_ARG_WITH(int10,           AS_HELP_STRING([--with-int10=BACKEND], [int10 backend: vm86, x86emu or stub]),
>  				[INT10="$withval"],
>  				[INT10="$DEFAULT_INT10"])
> @@ -485,6 +492,7 @@ esac
>  AC_ARG_WITH(default-font-path, AS_HELP_STRING([--with-default-font-path=PATH], [Comma separated list of font dirs]),
>  				[ FONTPATH="$withval" ],
>  				[ FONTPATH="${DEFAULT_FONT_PATH}" ])
> +

whitespace only, pls remove this hunk.

>  AC_ARG_WITH(xkb-path,         AS_HELP_STRING([--with-xkb-path=PATH], [Path to XKB base dir (default: ${datadir}/X11/xkb)]),
>  				[ XKBPATH="$withval" ],
>  				[ XKBPATH="${datadir}/X11/xkb" ])
> @@ -756,6 +764,14 @@ fi
>  AM_CONDITIONAL(CONFIG_NEED_DBUS, [test "x$CONFIG_NEED_DBUS" = xyes])
>  CONFIG_LIB='$(top_builddir)/config/libconfig.la'
>  
> +if test "x$SIGIO_DEFAULT" = xyes; then
> +	SIGIO_DEFAULT_VALUE=TRUE
> +else
> +	SIGIO_DEFAULT_VALUE=FALSE
> +fi
> +AC_DEFINE_UNQUOTED([SIGIO_DEFAULT], [$SIGIO_DEFAULT_VALUE],
> +		   [Use SIGIO handlers for input device events by default])
> +
>  AC_MSG_CHECKING([for glibc...])
>  AC_PREPROC_IFELSE([
>  #include <features.h>
> diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
> index e81eb0f..a5f2e6c 100644
> --- a/hw/xfree86/common/xf86Config.c
> +++ b/hw/xfree86/common/xf86Config.c
> @@ -724,6 +724,7 @@ typedef enum {
>      FLAG_AUTO_ENABLE_DEVICES,
>      FLAG_GLX_VISUALS,
>      FLAG_DRI2,
> +    FLAG_USE_SIGIO
>  } FlagValues;
>     
>  static OptionInfoRec FlagOptions[] = {
> @@ -781,6 +782,8 @@ static OptionInfoRec FlagOptions[] = {
>          {0}, FALSE },
>    { FLAG_DRI2,			"DRI2",				OPTV_BOOLEAN,
>  	{0}, FALSE },
> +  { FLAG_USE_SIGIO,		"UseSIGIO",			OPTV_BOOLEAN,
> +	{0}, SIGIO_DEFAULT },
>    { -1,				NULL,				OPTV_NONE,
>  	{0}, FALSE },
>  };
> @@ -848,6 +851,22 @@ configServerFlags(XF86ConfFlagsPtr flagsconf, XF86OptionPtr layoutopts)
>  	    xf86Msg(X_CONFIG, "Ignoring ABI Version\n");
>      }
>  
> +    if (xf86SIGIOSupported()) {
> +	xf86GetOptValBool(FlagOptions, FLAG_USE_SIGIO, &xf86Info.useSIGIO);
> +	if (xf86IsOptionSet(FlagOptions, FLAG_USE_SIGIO)) {
> +	    from = X_CONFIG;
> +	} else {
> +	    from = X_DEFAULT;
> +	}
> +	if (!xf86Info.useSIGIO) {
> +	    xf86Msg(from, "Disabling SIGIO handlers for input devices\n");
> +	} else if (from == X_CONFIG) {
> +	    xf86Msg(from, "Enabling SIGIO handlers for input devices\n");
> +	}
> +    } else {
> +	xf86Info.useSIGIO = FALSE;
> +    }
> +
>      if (xf86IsOptionSet(FlagOptions, FLAG_AUTO_ADD_DEVICES)) {
>          xf86GetOptValBool(FlagOptions, FLAG_AUTO_ADD_DEVICES,
>                            &xf86Info.autoAddDevices);
> diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c
> index 9a2468d..56ab266 100644
> --- a/hw/xfree86/common/xf86Helper.c
> +++ b/hw/xfree86/common/xf86Helper.c
> @@ -2312,7 +2312,7 @@ xf86SetSilkenMouse (ScreenPtr pScreen)
>       * yet.  Should handle this differently so that alternate async methods
>       * work correctly with this too.
>       */
> -    pScrn->silkenMouse = useSM && xf86SIGIOSupported();
> +    pScrn->silkenMouse = useSM && xf86Info.useSIGIO && xf86SIGIOSupported();
>      if (serverGeneration == 1)
>  	xf86DrvMsg(pScreen->myNum, from, "Silken mouse %s\n",
>  		   pScrn->silkenMouse ? "enabled" : "disabled");
> diff --git a/hw/xfree86/common/xf86Privstr.h b/hw/xfree86/common/xf86Privstr.h
> index 26f822d..9982601 100644
> --- a/hw/xfree86/common/xf86Privstr.h
> +++ b/hw/xfree86/common/xf86Privstr.h
> @@ -87,6 +87,8 @@ typedef struct {
>      Bool		miscModInDevEnabled;	/* Allow input devices to be
>  						 * changed */
>      Bool		miscModInDevAllowNonLocal;
> +    Bool		useSIGIO;		/* Use SIGIO for handling
> +						   input device events */
>      Pix24Flags		pixmap24;
>      MessageType		pix24From;
>  #ifdef __i386__
> diff --git a/hw/xfree86/os-support/shared/sigio.c b/hw/xfree86/os-support/shared/sigio.c
> index 44136cc..f9322ca 100644
> --- a/hw/xfree86/os-support/shared/sigio.c
> +++ b/hw/xfree86/os-support/shared/sigio.c
> @@ -145,6 +145,10 @@ xf86InstallSIGIOHandler(int fd, void (*f)(int, void *), void *closure)
>      int blocked;
>      int installed = FALSE;
>  
> +    if (!xf86Info.useSIGIO) {
> +	return 0;
> +    }
> +
>      for (i = 0; i < MAX_FUNCS; i++)
>      {
>  	if (!xf86SigIOFuncs[i].f)
> @@ -216,6 +220,10 @@ xf86RemoveSIGIOHandler(int fd)
>      int maxfd;
>      int ret;
>  
> +    if (!xf86Info.useSIGIO) {
> +	return 0;
> +    }
> +
>      max = 0;
>      maxfd = -1;
>      ret = 0;
> diff --git a/include/xorg-config.h.in b/include/xorg-config.h.in
> index d159420..5f3e4fd 100644
> --- a/include/xorg-config.h.in
> +++ b/include/xorg-config.h.in
> @@ -130,4 +130,7 @@
>  /* Path to text files containing PCI IDs */
>  #undef PCI_TXT_IDS_PATH
>  
> +/* Use SIGIO handlers for input device events by default */
> +#undef SIGIO_DEFAULT
> +
>  #endif /* _XORG_CONFIG_H_ */
> -- 
> 1.5.6.5

Bar the comments above, I'm fine with the patch.

Cheers,
  Peter


More information about the xorg-devel mailing list