Re: xserver extensions/hooks: freeing client resources
On 2/1/24 7:03 AM, Enrico Weigelt, metux IT consult wrote: Hello folks, I'm currently writing an extension that's storing per-client data. Where's is the right place to free up all resources ? I guess the client state xace hook - but what's the difference between retained and gone ? I don't think you need xace for this: CloseDownClient calls CallCallbacks((), (void *) ) so you can register a callback on that. "Retained" means that the client's resources should remain allocated in the server even after the client is gone. It's not a super useful mechanic these days but you'll have to decide whether a "retained" client disappearing should free your per-client data or not. The other option is to use AddResource to allocate resources associated with a particular client's XIDs. Those are freed by FreeClientNeverRetainResources or FreeClientResources, depending on whether the client is supposed to be retained and whether the resource type in question is marked with the RC_NEVERRETAIN flag. -- Aaron thx --mtx -- --- Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren GPG/PGP-Schlüssel zu. --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering i...@metux.net -- +49-151-27565287
Re: SCM_RIGHTS: XTrans vs ssh forwarding with MIT-SHM clients
At least on Arch Linux, getpeereid() is only defined in bsd/unistd.h and Meson doesn't seem to find it. HAVE_GETPEEREID is undefined and GetLocalClientCreds() ends up in the `defined(SO_PEERCRED)` path. I agree that it's just broken on platforms that have getpeereid(). -- Aaron On 1/18/23 11:43 AM, Jeremy Huddleston Sequoia wrote: After implementing DetermineClientCmd for darwin (https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1040), we still have an issue. We still do not know the pid of the peer (which we need to pass DetermineClientCmd). We expect to determine the pid in GetLocalClientCreds(). Looking into that function, I expect it to be failing on other UNIX as well. GetLocalClientCreds() prefers getpeereid() if getpeereid() is available. getpeereid(), however, only returns uid and gid. If getpeereid() is not available, the implementation will try getpeerucred() or SO_PEERCRED (both of which *DO* return the pid in addition to uid/gid). So it looks like this should only be working if getpeereid() is NOT available and getpeerucred() or SO_PEERCRED are available. Am I missing something? How is this ssh detection working on other UNIX systems that have getpeereid right now (or is it not)?. On Jan 17, 2023, at 01:06, Jeremy Huddleston Sequoia wrote: Yep, thanks for the pointer. Looks like DetermineClientCmd() needs to be implemented for darwin. On Jan 16, 2023, at 01:14, Michel Dänzer wrote: On 1/16/23 06:31, Jeremy Huddleston Sequoia wrote: How should this work? Why hasn't this been reported as an issue on other platforms? This all seems pretty platform agnostic, so I'd expect this to be an issue on other platforms as well. Is it not? If not, why not? ComputeLocalClient attempts to detect SSH clients and treats them as non-local. Maybe this isn't working on macos for some reason? -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: Xorg build fails due to "Failed to clone lib/libxcvt"
On 5/26/22 3:00 PM, Jérémy Hervé wrote: Dear Xorg-Devel List, I am trying to build the Xorg server. I am using the instructions available here : https://wiki.x.org/wiki/Building_the_X_Window_System/ and chose the option "Build Process Based on build.sh Script". I type the following command : jdh@thinkpad:~/X$ ./util/modular/build.sh --clone ~/X_Build The process appears to be going on well until the following message appear : == == Processing: "lib/libxcvt" == configuration options: Cloning into 'lib/libxcvt'... fatal: remote error: access denied or repository not exported: /git/xorg/lib/libxcvt Failed to clone lib/libxcvt. Ignoring. ./util/modular/build.sh: 691: --prefix=/home/jdh/X_Build: not found build.sh: "lib" failed on libxcvt build.sh: error processing: "libxcvt" A quick look to the build.sh sources let me think that the script is looking to the next place http://anongit.freedesktop.org/git/xorg/lib/ For a "libxcvt" folder but fails to do it. How to the Xorg team build the server ? Am I using the right instructions ? If so, could you please help me solving this issue ? libxcvt isn't available via anongit, you have to fetch it from gitlab. I had to add an override for my repo manifest for that: https://cgit.freedesktop.org/~aplattner/xorg-manifest/commit/?id=d5e6e78399b96b80adf89b5f4da0d840c73c2e86 I think build.sh is going to need something similar to override $GITROOT based on which project it's fetching. -- Aaron
Re: [RFC] Release schedule for X server 21.1
Hi Povilas, thanks so much for driving this release! This schedule sounds fine to me. Unless you say otherwise, I'm going to consider the ABI frozen at this point and mark it as officially supported for the next NVIDIA driver release. Sincerely, Aaron On 8/9/21 4:57 PM, Povilas Kanapickas wrote: Hello, According to my recent testing the master branch is in relatively good shape. There exist known bugs, but we can start planning the release anyway. I would like to propose release schedule for Xserver 21.1. - 2021-08-16 - Driver ABI freeze - 2021-08-30 - 21.1 RC 1 - every 2 weeks after that: a new RC until the final release Note that there are no open MRs that affect the driver ABI and which make sense to merge at this time relatively near the RC phase. Thus for practical purposes the ABI freeze could as well be today. Differently than previous releases of the X server, the `server-21.1-branch` release branch will branch off just before the RC 1 release and not after the final release of 21.1. This is because we want to remove the code of xwayland DDX for the RC releases as xwayland is released separately. Accordingly, after RC 1 is released the master branch will be open again for changes targeting the next release. Regards, Povilas Kanapickas
Re: xserver release process
On 10/8/19 1:19 PM, Hans de Goede wrote: Hi, On 08-10-2019 18:28, Adam Jackson wrote: In short, releases need to happen, and we have CI, so let's just pop a release out on scheduled dates assuming CI passes. Given that the Xorg xserver has a lot of hw interaction, we are never going to catch everything with CI, so this seems a bit over-simplified. I think it would be good to have 2 things: 1. A way to track potential blocker bugs. Note I'm not advocating some process heavy approach here. The blocker bugs can just be gitlab issues with a special tag I guess. The idea is that the release-coordinator at least can get a list of known issues and then decide if those are bad enough to delay a release or not. If you have issues or merge requests that need to block a release, please tag them with the appropriate GitLab milestone: https://gitlab.freedesktop.org/xorg/xserver/-/milestones -- Aaron 2. Send out a notice that a new release will happen in say 4 weeks from date of sending with a request for testing + getting any pending *fixes* upstream asao, maybe together with some "beta" or "rc" tarbals, but that is optional. My main reason for suggesting either one is that I personally am aware of at least 2 issues (both related to secondary USB GPUs handling) which are only present in master and not in the 1.20 branch and which I really would like to see fixed before a new release. I have taking a look at these on my to do list, but not at the top of it (yet). Having 1. would help in tracking such known issues, I doubt I'm the only one who has a couple of "I need to look into this and fix it" items on their TODO, so being able to track these would be good. Having 2. would help me bump up these TODOs in priority to try and get them fixed before the release :) Regards, Hans ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: RFC - GLX Extension to control GLXVND dispatching for PRIME GPU offloading
On 4/17/19 8:51 AM, Kyle Brenneman wrote: For GPU offloading in libglvnd, where individual clients can run with an alternate GPU and client-side vendor library, we'd need some way for that alternate vendor library to communicate with its server-side counterpart. Normally, the server's GLXVND layer would dispatch any GLX requests to whichever driver is running an X screen. This is a GLX extension that allows a client to tell the server to send GLX requests to a different driver instead. The basic idea is that the server keeps a separate (screen -> GLXServerVendor) mapping for each client. The current global mapping is used as the default for each new client, but the client can send a request to change its own mapping. That way, if the client uses a different vendor library, then the client-side vendor can arrange for any GLX requests to go to the matching server-side driver. The extension uses Atoms as an ID to identify each GLXServerVendor, using a string provided by the driver. That way, the client-side driver can know which Atom it needs to use without having to define an extra query. The client can send a request with a screen number and a vendor ID to tell the server to dispatch any GLX requests for that screen to the specified vendor. A client can also send None as a vendor ID to revert to whatever GLXServerVendor would handle that screen by default. I also added a GLXVendorPrivate/GLXVendorPrivateWithReply-style request, which sends a request to a specific vendor based on a vendor ID, without having to worry about which vendor is assigned to a screen at the moment. Strictly speaking, a vendor library could get the same result by adding a regular GLXVendorPrivate request, and providing a dispatch function that always routes the request to itself, but that seems like it's more of an implementation detail of GLXVND. Also, this extension doesn't define any errors or queries to check whether a GLXServerVendor can support a given screen. These requests would be sent by a client-side vendor library (not by libglvnd or an application), so each driver would be responsible for figuring out on its own which screens it can support. Anyway, I've got a draft of the extension spec here, and I've written up a series of patches for the X server to implement it here: https://gitlab.freedesktop.org/kbrenneman/xserver/tree/GLX_EXT_server_vendor_select Hi Kyle, Have you gotten any feedback on the commits there? It might help to create an xorg/xserver merge request for them. You can use the "WIP:" prefix on the MR title if you just want to request feedback without actually getting it merged. -- Aaron Comments and questions welcome. -Kyle Brenneman Name EXT_server_vendor_select Name Strings GLX_EXT_server_vendor_select Contact Kyle Brenneman, NVIDIA, kbrenneman at nvidia.com Contributors Kyle Brenneman Status XXX - Not complete yet!!! Version Last Modified Date: April 11, 2019 Revision: 1 Number OpenGL Extension #??? Dependencies GLX version 1.2 is required. This specification is written against the wording of the GLX 1.3 Protocol Encoding Specification. Overview In multi-GPU systems, a client may decide at runtime which device and driver to use for GLX, for example to choose between a high-performance and low-power device. This extension defines a set of requests that allow a client to specify which server-side driver should handle GLX requests from the sending client for a particular screen. IP Status No known IP claims. New Procedures and Functions None New Tokens None Additions to the GLX Specification None. These requests are intended to be used by a client-side GLX implementation, not by an application. Therefore, this extension does not define any new functions or changes to the GLX specification. GLX Protocol Get a List of Server-Side Drivers Name: glXQueryServerVendorIDsEXT Description: This request fetches a list of available server-side drivers, and the current vendor ID selected for each screen. Each driver is identified by an Atom, with a string chosen by the driver. The reply contains a list of the currently selected vendors first, with one Atom for each screen. This will be the vendor selected with the glXSelectScreenServerVendorIDEXT request, or the default vendor if the client has not sent a glXSelectScreenServerVendorIDEXT request for a screen. If a screen is using the default vendor, and the vendor does not have a vendor ID, then the corresponding Atom in the reply will be None. After the currently selected vendors, the reply will contain a list of all available vendor ID's. Note that
Re: what does "+"(preferred) means in xrandr?
"preferred" generally means that the Extended Display Information Data (EDID) from the monitor indicates that that mode most closely matches the native timings of the display. All modes listed by RandR for a given monitor should work with that monitor, but the preferred mode should work best (for some definition of "best"). RandR will send events when the list of modes for an output changes. To receive those, you should use XRRSelectInput() to select for them. I *think* the mask you want it RROutputChangeNotifyMask. You can get more information about RandR resources and requests in randrproto.txt: https://gitlab.freedesktop.org/xorg/proto/randrproto/blob/master/randrproto.txt On 11/24/18 8:20 AM, pengyixiang wrote: > hello everyone! > What the “preferred" means in xrandr? Is it setted by display > hardware? Can we set it manual? If we call “XOpenDisplay” to open > default screen and then poll the returned x11_fd, will we interrupted > for “preferred" changed? Is there others docs about it? Looking forward > to your reply. > > > Cheers, > Pencc ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] meson: Add configuration of listening on tcp, unix and local
On 06/26/2018 06:12 PM, Peter Hutterer wrote: On Tue, Jun 26, 2018 at 01:07:23PM -0700, Aaron Plattner wrote: On 06/24/2018 11:45 PM, Laurent Carlier wrote: Le samedi 16 juin 2018, 13:00:01 CEST Laurent Carlier a écrit : bugzilla: https://bugs.kde.org/show_bug.cgi?id=395419 bugzilla: https://bugs.archlinux.org/task/59025 Signed-off-by: Laurent Carlier Seems to work, so Reviewed-by: Aaron Plattner This switches the defined version from #define LISTEN_TCP 1 to #define LISTEN_TCP but it's only used in an #ifndef so that's probably fine. best to change to meson's conf_data.set10() then to keep the exact functionality. No, that's worse. Then "meson -Dlisten_tcp=false" does this: #define LISTEN_TCP 0 which will enable TCP. Thanks, 1989 C preprocessor. -- Aaron Cheers, Peter --- include/meson.build | 6 +++--- meson_options.txt | 7 +++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/include/meson.build b/include/meson.build index f76f557..dfca3c3 100644 --- a/include/meson.build +++ b/include/meson.build @@ -153,10 +153,10 @@ conf_data.set('BUSFAULT', conf_data.get('HAVE_SIGACTION')) conf_data.set('_XTYPEDEF_POINTER', '1') conf_data.set('_XITYPEDEF_POINTER', '1') +conf_data.set('LISTEN_TCP', get_option('listen_tcp')) +conf_data.set('LISTEN_UNIX', get_option('listen_unix')) +conf_data.set('LISTEN_LOCAL', get_option('listen_local')) # XXX: Configurable? -conf_data.set('LISTEN_TCP', '1') -conf_data.set('LISTEN_UNIX', '1') -conf_data.set('LISTEN_LOCAL', '1') conf_data.set('XTRANS_SEND_FDS', '1') conf_data.set('TCPCONN', '1') diff --git a/meson_options.txt b/meson_options.txt index 86fca46..3453b8d 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -45,6 +45,13 @@ option('vendor_name_short', type: 'string', value: 'X.Org') option('vendor_web', type: 'string', value: 'http://wiki.x.org') option('os_vendor', type: 'string', value: '') +option('listen_tcp', type: 'boolean', value: false, + description: 'Listen on TCP by default') +option('listen_unix', type: 'boolean', value: true, + description: 'Listen on Unix by default') +option('listen_local', type: 'boolean', value: true, + description: 'Listen on local by default') + option('int10', type: 'combo', choices: ['stub', 'x86emu', 'vm86', 'auto', 'false'], value: 'auto', description: 'Xorg int10 backend (default: usually x86emu)') Humble ping ? ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] meson: Add configuration of listening on tcp, unix and local
On 06/24/2018 11:45 PM, Laurent Carlier wrote: > Le samedi 16 juin 2018, 13:00:01 CEST Laurent Carlier a écrit : >> bugzilla: https://bugs.kde.org/show_bug.cgi?id=395419 >> bugzilla: https://bugs.archlinux.org/task/59025 >> >> Signed-off-by: Laurent Carlier Seems to work, so Reviewed-by: Aaron Plattner This switches the defined version from #define LISTEN_TCP 1 to #define LISTEN_TCP but it's only used in an #ifndef so that's probably fine. >> --- >> include/meson.build | 6 +++--- >> meson_options.txt | 7 +++ >> 2 files changed, 10 insertions(+), 3 deletions(-) >> >> diff --git a/include/meson.build b/include/meson.build >> index f76f557..dfca3c3 100644 >> --- a/include/meson.build >> +++ b/include/meson.build >> @@ -153,10 +153,10 @@ conf_data.set('BUSFAULT', >> conf_data.get('HAVE_SIGACTION')) conf_data.set('_XTYPEDEF_POINTER', '1') >> conf_data.set('_XITYPEDEF_POINTER', '1') >> >> +conf_data.set('LISTEN_TCP', get_option('listen_tcp')) >> +conf_data.set('LISTEN_UNIX', get_option('listen_unix')) >> +conf_data.set('LISTEN_LOCAL', get_option('listen_local')) >> # XXX: Configurable? >> -conf_data.set('LISTEN_TCP', '1') >> -conf_data.set('LISTEN_UNIX', '1') >> -conf_data.set('LISTEN_LOCAL', '1') >> conf_data.set('XTRANS_SEND_FDS', '1') >> >> conf_data.set('TCPCONN', '1') >> diff --git a/meson_options.txt b/meson_options.txt >> index 86fca46..3453b8d 100644 >> --- a/meson_options.txt >> +++ b/meson_options.txt >> @@ -45,6 +45,13 @@ option('vendor_name_short', type: 'string', value: >> 'X.Org') option('vendor_web', type: 'string', value: 'http://wiki.x.org') >> option('os_vendor', type: 'string', value: '') >> >> +option('listen_tcp', type: 'boolean', value: false, >> + description: 'Listen on TCP by default') >> +option('listen_unix', type: 'boolean', value: true, >> + description: 'Listen on Unix by default') >> +option('listen_local', type: 'boolean', value: true, >> + description: 'Listen on local by default') >> + >> option('int10', type: 'combo', choices: ['stub', 'x86emu', 'vm86', 'auto', >> 'false'], value: 'auto', >> description: 'Xorg int10 backend (default: usually x86emu)') > > Humble ping ? > > > > ___ > xorg-devel@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: https://lists.x.org/mailman/listinfo/xorg-devel > ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] configure: Remove unused CONFIGFILE
On 06/11/2018 12:10 PM, Adam Jackson wrote: This isn't used for anything, which is just as well, because /etc/xorg.conf is not in fact a path xserver will try to use. Bugzilla: https://bugs.freedesktop.org/8890 Signed-off-by: Adam Jackson Reviewed-by: Aaron Plattner I checked and confirmed that meson.build doesn't define a CONFIGFILE anywhere. --- configure.ac | 1 - 1 file changed, 1 deletion(-) diff --git a/configure.ac b/configure.ac index ddc47faa24..4df977fa5c 100644 --- a/configure.ac +++ b/configure.ac @@ -2022,7 +2022,6 @@ if test "x$XORG" = xyes; then XF86CONFIGFILE="xorg.conf" XF86CONFIGDIR="xorg.conf.d" AC_SUBST(XF86CONFIGDIR) - CONFIGFILE="$sysconfdir/$XF86CONFIGFILE" LOGPREFIX="Xorg." XDG_DATA_HOME=".local/share" XDG_DATA_HOME_LOGDIR="xorg" ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] meson: Define DEFAULT_LIBRARY_PATH as join_paths(get_option('prefix'), get_option('libdir'))
'libdir' defaults to 'lib', so running X -showDefaultLibPath just prints 'lib' instead of '/usr/lib' or '/usr/local/lib'. Use joint_paths() to get the correct full path. Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- The NVIDIA driver installer noticed this meson vs. autoconf difference: WARNING: You appear to be using a modular X.Org release, but the X library installation path, 'lib', reported by `/usr/bin/X -showDefaultLibPath` does not exist. Please check your X.Org installation. include/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/meson.build b/include/meson.build index 01625d7b544d..f76f557beaee 100644 --- a/include/meson.build +++ b/include/meson.build @@ -316,7 +316,7 @@ xorg_data.set_quoted('DEFAULT_LOGDIR', log_dir) xorg_data.set_quoted('DEFAULT_LOGPREFIX', 'Xorg.') xorg_data.set_quoted('FALLBACK_INPUT_DRIVER', 'libinput') xorg_data.set_quoted('DEFAULT_MODULE_PATH', join_paths(get_option('prefix'), module_dir)) -xorg_data.set_quoted('DEFAULT_LIBRARY_PATH', get_option('libdir')) +xorg_data.set_quoted('DEFAULT_LIBRARY_PATH', join_paths(get_option('prefix'), get_option('libdir'))) xorg_data.set_quoted('__XSERVERNAME__', 'Xorg') xorg_data.set('XSERVER_LIBPCIACCESS', get_option('pciaccess')) xorg_data.set_quoted('PCI_TXT_IDS_PATH', '') -- 2.17.0 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] meson: Set XCONFIGFILE to 'xorg.conf' instead of '/etc/xorg.conf'
The autoconf build hard-codes XCONFIGFILE to just 'xorg.conf': XF86CONFIGFILE="xorg.conf" AC_DEFINE_DIR(XCONFIGFILE, XF86CONFIGFILE, [Name of configuration file]) Later, the X server passes that into DoSubstitution() which expands the path: DoSubstitution(template="/etc/X11/%X", ..., XConfigFile="xorg.conf") This returns "/etc/X11/xorg.conf". The Meson build, on the other hand, sets XCONFIGFILE to join_paths(get_option('sysconfdir'), 'xorg.conf'). If sysconfdir is /etc, this results in '/etc/xorg.conf', resulting in DoSubstitution returning '/etc/X11/etc/xorg.conf'. Fix this by just hard-coding XCONFIGFILE to 'xorg.conf'. Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- include/meson.build | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/meson.build b/include/meson.build index 691e9f0743af..01625d7b544d 100644 --- a/include/meson.build +++ b/include/meson.build @@ -261,7 +261,7 @@ endif conf_data.set('SVR4', cc.compiles(defines_svr4)) conf_data.set_quoted('XKB_DFLT_RULES', get_option('xkb_default_rules')) conf_data.set('XORGSERVER', build_xorg) -conf_data.set_quoted('XCONFIGFILE', join_paths(get_option('sysconfdir'), 'xorg.conf')) +conf_data.set_quoted('XCONFIGFILE', 'xorg.conf') conf_data.set_quoted('__XSERVERNAME__', 'Xorg') conf_data.set('WITH_VGAHW', build_vgahw) conf_data.set('CSRG_BASED', csrg_based) @@ -308,7 +308,7 @@ xorg_data = configuration_data() xorg_data.set_quoted('XORG_BIN_DIRECTORY', get_option('bindir')) xorg_data.set('XORG_VERSION_CURRENT', release) xorg_data.set_quoted('XF86CONFIGFILE', 'xorg.conf') -xorg_data.set_quoted('XCONFIGFILE', join_paths(get_option('sysconfdir'), 'xorg.conf')) +xorg_data.set_quoted('XCONFIGFILE', 'xorg.conf') xorg_data.set_quoted('XCONFIGDIR', 'xorg.conf.d') xorg_data.set_quoted('DEFAULT_XDG_DATA_HOME', '.local/share') xorg_data.set_quoted('DEFAULT_XDG_DATA_HOME_LOGDIR', 'xorg') -- 2.17.0 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] meson: Fix module_dir configuration (v2)
meson.build has code to set the module_dir variable to ${libdir}/xorg/modules if the module_dir option string is empty. However, this has several problems: 1. The variable is only used for an unused @moduledir@ substitution in the man page. The rule for xorg-server.pc uses option('module_dir') directly instead. 2. The 'module_dir' option has a default value of 'xorg/modules' so the above rule doesn't do anything by default. 3. The xorg-server.pc rule uses ${exec_prefix}/option('module_dir'), so the effect of #2 is that the default moduledir is different between autoconf and meson. E.g. if ${prefix} is /X, then you get autoconf: moduledir=/X/lib/xorg/modules meson:moduledir=/X/xorg/modules Fix this by using the module_dir variable when generating xorg-server.pc, and by using join_paths() to assign module_dir unconditionally. v2: Keep the 'xorg/modules' default path, but use join_paths() unconditionally (Thierry Reding) Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- meson.build | 9 +++-- meson_options.txt | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/meson.build b/meson.build index 6571d08ebd38..59f19d5127fc 100644 --- a/meson.build +++ b/meson.build @@ -269,10 +269,7 @@ if log_dir == '' log_dir = join_paths(get_option('prefix'), get_option('localstatedir'), 'log') endif -module_dir = get_option('module_dir') -if module_dir == '' -module_dir = join_paths(get_option('libdir'), 'xorg/modules') -endif +module_dir = join_paths(get_option('libdir'), get_option('module_dir')) if glamor_option == 'auto' build_glamor = build_xorg or build_xwayland @@ -534,7 +531,7 @@ manpage_config.set('XKB_DFLT_LAYOUT', get_option('xkb_default_layout')) manpage_config.set('XKB_DFLT_VARIANT', get_option('xkb_default_variant')) manpage_config.set('XKB_DFLT_OPTIONS', get_option('xkb_default_options')) manpage_config.set('bundle_id_prefix', '...') -manpage_config.set('modulepath', join_paths(get_option('prefix'), module_dir)) +manpage_config.set('modulepath', module_dir) # wtf doesn't this work # manpage_config.set('suid_wrapper_dir', join_paths(get_option('prefix'), libexecdir)) manpage_config.set('suid_wrapper_dir', join_paths(get_option('prefix'), 'libexec')) @@ -619,7 +616,7 @@ if build_xorg sdkconfig.set('libdir', join_paths('${exec_prefix}', get_option('libdir'))) sdkconfig.set('includedir', join_paths('${prefix}', get_option('includedir'))) sdkconfig.set('datarootdir', join_paths('${prefix}', get_option('datadir'))) -sdkconfig.set('moduledir', join_paths('${exec_prefix}', get_option('module_dir'))) +sdkconfig.set('moduledir', join_paths('${exec_prefix}', module_dir)) sdkconfig.set('sdkdir', join_paths('${prefix}', get_option('includedir'), 'xorg')) sdkconfig.set('sysconfigdir', join_paths('${datarootdir}', 'X11/xorg.conf.d')) diff --git a/meson_options.txt b/meson_options.txt index a296838a15ea..86fca4668441 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -22,7 +22,7 @@ option('builder_string', type: 'string', description: 'Additional builder string option('log_dir', type: 'string') option('module_dir', type: 'string', value: 'xorg/modules', - description: 'X.Org modules directory') + description: 'X.Org modules directory (absolute or relative to the directory specified by the libdir option)') option('default_font_path', type: 'string') option('glx', type: 'boolean', value: true) -- 2.17.0 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] meson: Fix module_dir configuration
On 04/06/2018 09:59 PM, Aaron Plattner wrote: > On 04/03/2018 02:27 AM, Thierry Reding wrote: >> On Mon, Apr 02, 2018 at 02:31:20PM -0700, Aaron Plattner wrote: >>> meson.build has code to set the module_dir variable to >>> ${libdir}/xorg/modules if the module_dir option string is empty. >>> However, this has several problems: >>> >>> 1. The variable is only used for an unused @moduledir@ substitution in >>> the man page. The rule for xorg-server.pc uses option('module_dir') >>> directly instead. >>> 2. The 'module_dir' option has a default value of 'xorg/modules' so the >>> above rule doesn't do anything by default. >>> 3. The xorg-server.pc rule uses ${exec_prefix}/option('module_dir'), so >>> the effect of #2 is that the default moduledir is different between >>> autoconf and meson. E.g. if ${prefix} is /X, then you get >>> >>> autoconf: moduledir=/X/lib/xorg/modules >>> meson: moduledir=/X/xorg/modules >> >> Ugh... you're right. I was setting --libexecdir ${prefix}/lib in my >> scripts, which is why I wasn't seeing the above inconsistency. >> >>> Fix this by using the module_dir variable when generating >>> xorg-server.pc, and by removing the default value for the module_dir >>> option. >>> >>> Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> >>> --- >>> meson.build | 2 +- >>> meson_options.txt | 3 +-- >>> 2 files changed, 2 insertions(+), 3 deletions(-) >>> >>> diff --git a/meson.build b/meson.build >>> index 277534093b94..3e3f808b2d7a 100644 >>> --- a/meson.build >>> +++ b/meson.build >>> @@ -595,7 +595,7 @@ if build_xorg >>> sdkconfig.set('libdir', join_paths('${exec_prefix}', >>> get_option('libdir'))) >>> sdkconfig.set('includedir', join_paths('${prefix}', >>> get_option('includedir'))) >>> sdkconfig.set('datarootdir', join_paths('${prefix}', >>> get_option('datadir'))) >>> - sdkconfig.set('moduledir', join_paths('${exec_prefix}', >>> get_option('module_dir'))) >>> + sdkconfig.set('moduledir', join_paths('${exec_prefix}', >>> module_dir)) >> >> This would still give us an inconsistent path if the user passed in some >> other, relative directory for module_dir, right? So if they passed: >> >> --module-dir foo/xorg/modules >> >> they'd get /usr/foo/xorg/modules in the pkg-config files, but the server >> would actually look for it in /usr/lib/foo/xorg/modules. >> >>> sdkconfig.set('sdkdir', join_paths('${prefix}', >>> get_option('includedir'), 'xorg')) >>> sdkconfig.set('sysconfigdir', join_paths('${datarootdir}', >>> 'X11/xorg.conf.d')) >>> diff --git a/meson_options.txt b/meson_options.txt >>> index 5c7be0e26ce5..4cf8349ba9e5 100644 >>> --- a/meson_options.txt >>> +++ b/meson_options.txt >>> @@ -19,8 +19,7 @@ option('builder_addr', type: 'string', description: >>> 'Builder address', value: 'x >>> option('builder_string', type: 'string', description: 'Additional >>> builder string') >>> option('log_dir', type: 'string') >>> -option('module_dir', type: 'string', value: 'xorg/modules', >>> - description: 'X.Org modules directory') >>> +option('module_dir', type: 'string', description: 'X.Org modules >>> directory') >> >> It seems somewhat backwards to me to avoid the feature of assigning a >> default value for an option and was totally surprising to me because I >> didn't go look for a default assignment in meson.build, so I completely >> missed it. >> >> Why don't we do the following: >> >> 1) define that module_dir is either absolute or relative to >> ${libdir} >> >> 2) keep the default in the option declaration >> >> 3) change the module_dir variable to always be made up of the >> libdir and module_dir options joined >> >> For 3), the Meson documentation specifies that if any of the arguments >> to join_paths() is an absolute path, all arguments before it are >> dropped, so it automatically deals with the case where users specify an >> absolute path. >> >> Something like the below squashed into your patch. > > Thanks Thierry. The patch below seems reasonable to me, but I'm out of > the office until Wednesday so I won't be able to test it until then. Sorry I forgot about this until now. I confirmed that I still get /usr/lib/
Re: [PATCH xserver] meson: Fix module_dir configuration
On 04/03/2018 02:27 AM, Thierry Reding wrote: On Mon, Apr 02, 2018 at 02:31:20PM -0700, Aaron Plattner wrote: meson.build has code to set the module_dir variable to ${libdir}/xorg/modules if the module_dir option string is empty. However, this has several problems: 1. The variable is only used for an unused @moduledir@ substitution in the man page. The rule for xorg-server.pc uses option('module_dir') directly instead. 2. The 'module_dir' option has a default value of 'xorg/modules' so the above rule doesn't do anything by default. 3. The xorg-server.pc rule uses ${exec_prefix}/option('module_dir'), so the effect of #2 is that the default moduledir is different between autoconf and meson. E.g. if ${prefix} is /X, then you get autoconf: moduledir=/X/lib/xorg/modules meson:moduledir=/X/xorg/modules Ugh... you're right. I was setting --libexecdir ${prefix}/lib in my scripts, which is why I wasn't seeing the above inconsistency. Fix this by using the module_dir variable when generating xorg-server.pc, and by removing the default value for the module_dir option. Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- meson.build | 2 +- meson_options.txt | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/meson.build b/meson.build index 277534093b94..3e3f808b2d7a 100644 --- a/meson.build +++ b/meson.build @@ -595,7 +595,7 @@ if build_xorg sdkconfig.set('libdir', join_paths('${exec_prefix}', get_option('libdir'))) sdkconfig.set('includedir', join_paths('${prefix}', get_option('includedir'))) sdkconfig.set('datarootdir', join_paths('${prefix}', get_option('datadir'))) -sdkconfig.set('moduledir', join_paths('${exec_prefix}', get_option('module_dir'))) +sdkconfig.set('moduledir', join_paths('${exec_prefix}', module_dir)) This would still give us an inconsistent path if the user passed in some other, relative directory for module_dir, right? So if they passed: --module-dir foo/xorg/modules they'd get /usr/foo/xorg/modules in the pkg-config files, but the server would actually look for it in /usr/lib/foo/xorg/modules. sdkconfig.set('sdkdir', join_paths('${prefix}', get_option('includedir'), 'xorg')) sdkconfig.set('sysconfigdir', join_paths('${datarootdir}', 'X11/xorg.conf.d')) diff --git a/meson_options.txt b/meson_options.txt index 5c7be0e26ce5..4cf8349ba9e5 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -19,8 +19,7 @@ option('builder_addr', type: 'string', description: 'Builder address', value: 'x option('builder_string', type: 'string', description: 'Additional builder string') option('log_dir', type: 'string') -option('module_dir', type: 'string', value: 'xorg/modules', - description: 'X.Org modules directory') +option('module_dir', type: 'string', description: 'X.Org modules directory') It seems somewhat backwards to me to avoid the feature of assigning a default value for an option and was totally surprising to me because I didn't go look for a default assignment in meson.build, so I completely missed it. Why don't we do the following: 1) define that module_dir is either absolute or relative to ${libdir} 2) keep the default in the option declaration 3) change the module_dir variable to always be made up of the libdir and module_dir options joined For 3), the Meson documentation specifies that if any of the arguments to join_paths() is an absolute path, all arguments before it are dropped, so it automatically deals with the case where users specify an absolute path. Something like the below squashed into your patch. Thanks Thierry. The patch below seems reasonable to me, but I'm out of the office until Wednesday so I won't be able to test it until then. -- Aaron Thierry --- >8 --- diff --git a/meson.build b/meson.build index 3e3f808b2d7a..33e2f6d88b1a 100644 --- a/meson.build +++ b/meson.build @@ -267,10 +267,7 @@ if log_dir == '' log_dir = join_paths(get_option('prefix'), get_option('localstatedir'), 'log') endif -module_dir = get_option('module_dir') -if module_dir == '' -module_dir = join_paths(get_option('libdir'), 'xorg/modules') -endif +module_dir = join_paths(get_option('libdir'), get_option('module_dir')) if glamor_option == 'auto' build_glamor = build_xorg or build_xwayland @@ -510,7 +507,7 @@ manpage_config.set('XKB_DFLT_LAYOUT', get_option('xkb_default_layout')) manpage_config.set('XKB_DFLT_VARIANT', get_option('xkb_default_variant')) manpage_config.set('XKB_DFLT_OPTIONS', get_option('xkb_default_options')) manpage_config.set('bundle_id_prefix', '...') -manpage_config.set('modulepath', join_paths(get_option('prefix'), module_dir)) +manpage_config.set('modulepath', module_dir) # wtf doesn't this work # manpage_config.set('suid_wrapper_dir', join_paths(get_option('prefix'), libexecdir)) manpage_config.set('suid_wrapper_dir', j
[PATCH xserver] xfree86: Restore newline before "X Protocol Version" string
The newline before the protocl version got lost in commit 6cbefc3e0a33b380c147c533914437c7798d9b93. Prior to that commit, the release date printed a newline at the end: X.Org X Server 1.19.6 Release Date: 2017-12-20 X Protocol Version 11, Revision 0 Build Operating System: Linux 4.14.12-1-ARCH x86_64 Now, that string gets run together with the version: X.Org X Server 1.19.99.903 (1.20.0 RC 3)X Protocol Version 11, Revision 0 Build Operating System: Linux Since the version string printing has a variety of #ifdefs in it, just add the newline to the begining of the protocol version string. Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- hw/xfree86/common/xf86Init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c index ea42ec946167..3c5cc7097c49 100644 --- a/hw/xfree86/common/xf86Init.c +++ b/hw/xfree86/common/xf86Init.c @@ -165,7 +165,7 @@ xf86PrintBanner(void) #ifdef XORG_CUSTOM_VERSION xf86ErrorFVerb(0, " (%s)", XORG_CUSTOM_VERSION); #endif -xf86ErrorFVerb(0, "X Protocol Version %d, Revision %d\n", +xf86ErrorFVerb(0, "\nX Protocol Version %d, Revision %d\n", X_PROTOCOL, X_PROTOCOL_REVISION); xf86ErrorFVerb(0, "Build Operating System: %s %s\n", OSNAME, OSVENDOR); #ifdef HAS_UTSNAME -- 2.16.3 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] meson: Fix module_dir configuration
meson.build has code to set the module_dir variable to ${libdir}/xorg/modules if the module_dir option string is empty. However, this has several problems: 1. The variable is only used for an unused @moduledir@ substitution in the man page. The rule for xorg-server.pc uses option('module_dir') directly instead. 2. The 'module_dir' option has a default value of 'xorg/modules' so the above rule doesn't do anything by default. 3. The xorg-server.pc rule uses ${exec_prefix}/option('module_dir'), so the effect of #2 is that the default moduledir is different between autoconf and meson. E.g. if ${prefix} is /X, then you get autoconf: moduledir=/X/lib/xorg/modules meson:moduledir=/X/xorg/modules Fix this by using the module_dir variable when generating xorg-server.pc, and by removing the default value for the module_dir option. Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- meson.build | 2 +- meson_options.txt | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/meson.build b/meson.build index 277534093b94..3e3f808b2d7a 100644 --- a/meson.build +++ b/meson.build @@ -595,7 +595,7 @@ if build_xorg sdkconfig.set('libdir', join_paths('${exec_prefix}', get_option('libdir'))) sdkconfig.set('includedir', join_paths('${prefix}', get_option('includedir'))) sdkconfig.set('datarootdir', join_paths('${prefix}', get_option('datadir'))) -sdkconfig.set('moduledir', join_paths('${exec_prefix}', get_option('module_dir'))) +sdkconfig.set('moduledir', join_paths('${exec_prefix}', module_dir)) sdkconfig.set('sdkdir', join_paths('${prefix}', get_option('includedir'), 'xorg')) sdkconfig.set('sysconfigdir', join_paths('${datarootdir}', 'X11/xorg.conf.d')) diff --git a/meson_options.txt b/meson_options.txt index 5c7be0e26ce5..4cf8349ba9e5 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -19,8 +19,7 @@ option('builder_addr', type: 'string', description: 'Builder address', value: 'x option('builder_string', type: 'string', description: 'Additional builder string') option('log_dir', type: 'string') -option('module_dir', type: 'string', value: 'xorg/modules', - description: 'X.Org modules directory') +option('module_dir', type: 'string', description: 'X.Org modules directory') option('default_font_path', type: 'string') option('glx', type: 'boolean', value: true) -- 2.16.3 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 2/3] meson: Distribute more SDK headers
Thanks, Thierry! I started working on a change to do this, but didn't get very far before you beat me to it. On 03/29/2018 04:07 AM, Thierry Reding wrote: > From: Thierry Reding> > Install missing headers to the SDK directory to allow external modules > to properly build against the SDK. After this commit, the list of files > installed in the SDK include directory is the same as the list of files > installed by the autotools-based build. > > Signed-off-by: Thierry Reding > --- > Xext/meson.build | 12 > composite/meson.build | 6 ++ > dbe/meson.build | 6 ++ > dri3/meson.build | 6 ++ > fb/meson.build| 10 ++ > glx/meson.build | 6 ++ > hw/xfree86/os-support/meson.build | 9 - > include/meson.build | 1 + > mi/meson.build| 15 +++ > miext/damage/meson.build | 7 +++ > miext/shadow/meson.build | 6 ++ > miext/sync/meson.build| 9 + > present/meson.build | 7 +++ > randr/meson.build | 7 +++ > render/meson.build| 9 + > 15 files changed, 115 insertions(+), 1 deletion(-) > > diff --git a/Xext/meson.build b/Xext/meson.build > index 9968f2a9e312..a7217371871d 100644 > --- a/Xext/meson.build > +++ b/Xext/meson.build > @@ -8,12 +8,19 @@ srcs_xext = [ > 'xtest.c', > ] > > +hdrs_xext = [ > +'geext.h', > +'geint.h', > +'syncsdk.h', > +] > + > if build_dpms > srcs_xext += 'dpms.c' > endif > > if build_mitshm > srcs_xext += 'shm.c' > +hdrs_xext += ['shmint.h'] > endif > > if build_res > @@ -26,6 +33,7 @@ endif > > if build_xace > srcs_xext += 'xace.c' > +hdrs_xext += ['xace.h', 'xacestr.h'] > endif > > if build_xf86bigfont > @@ -34,6 +42,7 @@ endif > > if build_xinerama > srcs_xext += ['panoramiX.c', 'panoramiXprocs.c', 'panoramiXSwap.c'] > +hdrs_xext += ['panoramiX.h', 'panoramiXsrv.h'] > endif > > if build_xsecurity > @@ -46,6 +55,7 @@ endif > > if build_xv > srcs_xext += ['xvmain.c', 'xvdisp.c', 'xvmc.c'] > +hdrs_xext += ['xvdix.h', 'xvmcext.h'] > endif > > libxserver_xext = static_library('libxserver_xext', > @@ -59,3 +69,5 @@ libxserver_xext_vidmode = > static_library('libxserver_xext_vidmode', > include_directories: inc, > dependencies: common_dep, > ) > + > +install_data(hdrs_xext, install_dir: xorgsdkdir) Do these new install_data() directives need to be behind an 'if build_xorg'? It looks like the other two instances of this that weren't behind the build_xorg check that guards subdir('xfree86') in hw/meson.build had their own build_xorg checks. Prior to this change, building with "meson configure -Dxorg=false" only installs xorg-server.h to $prefix/include/xorg. > diff --git a/composite/meson.build b/composite/meson.build > index 6c4a03fb80c2..7547f0e7edce 100644 > --- a/composite/meson.build > +++ b/composite/meson.build > @@ -6,8 +6,14 @@ srcs_composite = [ > 'compwindow.c', > ] > > +hdrs_composite = [ > + 'compositeext.h', > +] > + > libxserver_composite = static_library('libxserver_composite', > srcs_composite, > include_directories: inc, > dependencies: common_dep, > ) > + > +install_data(hdrs_composite, install_dir: xorgsdkdir) > diff --git a/dbe/meson.build b/dbe/meson.build > index e10bde19913d..76a2d3f85d2b 100644 > --- a/dbe/meson.build > +++ b/dbe/meson.build > @@ -3,8 +3,14 @@ srcs_dbe = [ > 'midbe.c', > ] > > +hdrs_dbe = [ > + 'dbestruct.h', > +] > + > libxserver_dbe = static_library('libxserver_dbe', > srcs_dbe, > include_directories: inc, > dependencies: common_dep, > ) > + > +install_data(hdrs_dbe, install_dir: xorgsdkdir) > diff --git a/dri3/meson.build b/dri3/meson.build > index 0deec32aafbe..48ce0d9d6aa1 100644 > --- a/dri3/meson.build > +++ b/dri3/meson.build > @@ -4,6 +4,10 @@ srcs_dri3 = [ > 'dri3_screen.c', > ] > > +hdrs_dri3 = [ > + 'dri3.h', > +] > + > libxserver_dri3 = [] > if build_dri3 > libxserver_dri3 = static_library('libxserver_dri3', > @@ -13,3 +17,5 @@ if build_dri3 > c_args: '-DHAVE_XORG_CONFIG_H' > ) > endif > + > +install_data(hdrs_dri3, install_dir: xorgsdkdir) > diff --git a/fb/meson.build b/fb/meson.build > index bf85141f980f..477ab047dfd6 100644 > --- a/fb/meson.build > +++ b/fb/meson.build > @@ -28,6 +28,14 @@ srcs_fb = [ > 'fbwindow.c', > ] > > +hdrs_fb = [ > + 'fb.h', > + 'fboverlay.h', > + 'fbpict.h', > + 'fbrop.h', > + 'wfbrename.h' > +] > + > libxserver_fb = static_library('libxserver_fb', > srcs_fb, > include_directories: inc, > @@ -45,3 +53,5 @@ libxserver_wfb = static_library('libxserver_wfb', > pic: true, > build_by_default: false, > ) > + >
Re: [PATCH xserver 7/8] dix: Remove colormap private fixup
On 01/31/2018 12:15 PM, Aaron Plattner wrote: > On 01/31/2018 07:54 AM, Adam Jackson wrote: >> The old xfree86 colormap system was the only thing that would register a >> colormap private after the colormap was created. Since that's gone now >> we can remove the special case. > > Our driver does this for some wacky overlay stuff, but I think it's just > a sequencing quirk that we could fix. I'll look into that today. It looks like reordering our init code works fine, so Acked-by: Aaron Plattner <aplatt...@nvidia.com> -- Aaron > >> Signed-off-by: Adam Jackson <a...@redhat.com> >> --- >> dix/privates.c | 21 + >> include/privates.h | 2 +- >> 2 files changed, 2 insertions(+), 21 deletions(-) >> >> diff --git a/dix/privates.c b/dix/privates.c >> index 9ca80f0b6..b28e60fe7 100644 >> --- a/dix/privates.c >> +++ b/dix/privates.c >> @@ -91,10 +91,10 @@ static const char *key_names[PRIVATE_LAST] = { >> /* These can have objects created before all of the keys are registered >> */ >> [PRIVATE_SCREEN] = "SCREEN", >> [PRIVATE_EXTENSION] = "EXTENSION", >> -[PRIVATE_COLORMAP] = "COLORMAP", >> [PRIVATE_DEVICE] = "DEVICE", >> >> /* These cannot have any objects before all relevant keys are >> registered */ >> +[PRIVATE_COLORMAP] = "COLORMAP", >> [PRIVATE_CLIENT] = "CLIENT", >> [PRIVATE_PROPERTY] = "PROPERTY", >> [PRIVATE_SELECTION] = "SELECTION", >> @@ -250,24 +250,6 @@ fixupExtensions(FixupFunc fixup, unsigned bytes) >> return TRUE; >> } >> >> -static Bool >> -fixupDefaultColormaps(FixupFunc fixup, unsigned bytes) >> -{ >> -int s; >> - >> -for (s = 0; s < screenInfo.numScreens; s++) { >> -ColormapPtr cmap; >> - >> -dixLookupResourceByType((void **) , >> -screenInfo.screens[s]->defColormap, >> RT_COLORMAP, >> -serverClient, DixCreateAccess); >> -if (cmap && >> -!fixup(>devPrivates, >> screenInfo.screens[s]->screenSpecificPrivates[PRIVATE_COLORMAP].offset, >> bytes)) >> -return FALSE; >> -} >> -return TRUE; >> -} >> - >> static Bool >> fixupDeviceList(DeviceIntPtr device, FixupFunc fixup, unsigned bytes) >> { >> @@ -290,7 +272,6 @@ static Bool (*const allocated_early[PRIVATE_LAST]) >> (FixupFunc, unsigned) = { >> [PRIVATE_SCREEN] = fixupScreens, >> [PRIVATE_CLIENT] = fixupServerClient, >> [PRIVATE_EXTENSION] = fixupExtensions, >> -[PRIVATE_COLORMAP] = fixupDefaultColormaps, >> [PRIVATE_DEVICE] = fixupDevices, >> }; >> >> diff --git a/include/privates.h b/include/privates.h >> index e89c3e440..bbd8c3355 100644 >> --- a/include/privates.h >> +++ b/include/privates.h >> @@ -32,10 +32,10 @@ typedef enum { >> /* These can have objects created before all of the keys are registered >> */ >> PRIVATE_SCREEN, >> PRIVATE_EXTENSION, >> -PRIVATE_COLORMAP, >> PRIVATE_DEVICE, >> >> /* These cannot have any objects before all relevant keys are >> registered */ >> +PRIVATE_COLORMAP, >> PRIVATE_CLIENT, >> PRIVATE_PROPERTY, >> PRIVATE_SELECTION, >> > > ___ > xorg-devel@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: https://lists.x.org/mailman/listinfo/xorg-devel > ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 1/2] animcur: Fix transitions between animated cursors
On 01/09/2018 10:08 AM, Aaron Plattner wrote: > On 01/09/2018 08:51 AM, Adam Jackson wrote: >> We weren't cancelling the old timer when changing cursors, making things >> go all crashy. Logically we could always cancel the timer first, but >> then we'd have to call TimerSet to re-arm ourselves, and GetTimeInMillis >> is potentially expensive. >> >> Reported-by: >> https://devtalk.nvidia.com/default/topic/1028172/linux/titan-v-ubuntu-16-04lts-and-387-34-driver-crashes-badly/post/5230967/#5230967 >> Signed-off-by: Adam Jackson <a...@redhat.com> >> --- >> render/animcur.c | 25 +++-- >> 1 file changed, 15 insertions(+), 10 deletions(-) >> >> diff --git a/render/animcur.c b/render/animcur.c >> index 058bc1b323..797029443c 100644 >> --- a/render/animcur.c >> +++ b/render/animcur.c >> @@ -151,11 +151,20 @@ AnimCurTimerNotify(OsTimerPtr timer, CARD32 now, void >> *arg) >> return ac->elts[elt].delay; >> } >> >> +static void >> +AnimCurCancelTimer(DeviceIntPtr pDev) >> +{ >> +CursorPtr cur = pDev->spriteInfo->anim.pCursor; >> + >> +if (IsAnimCur(cur)) >> +TimerCancel(GetAnimCur(cur)->timer); >> +} >> + >> static Bool >> AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr >> pCursor) >> { >> AnimCurScreenPtr as = GetAnimCurScreen(pScreen); >> -Bool ret; >> +Bool ret = TRUE; >> >> if (IsFloating(pDev)) >> return FALSE; >> @@ -165,8 +174,10 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr >> pScreen, CursorPtr pCursor) >> if (pCursor != pDev->spriteInfo->anim.pCursor) { >> AnimCurPtr ac = GetAnimCur(pCursor); >> >> -ret = (*pScreen->DisplayCursor) >> -(pDev, pScreen, ac->elts[0].pCursor); >> +AnimCurCancelTimer(pDev); >> +ret = (*pScreen->DisplayCursor) (pDev, pScreen, >> + ac->elts[0].pCursor); >> + > > This is a slight change in behavior if DisplayCursor fails since it will > cancel the previous timer whereas before it wouldn't. Are there any > weird cases where failing to change the cursor is expected and canceling > animation of the previous cursor would be a problem? > > Assuming not, both changes > Reviewed-by: Aaron Plattner <aplatt...@nvidia.com> > > I'll try to put together a build to test these today. I re-verified the crash and verified that this series fixes it, so you can add Tested-by: Aaron Plattner <aplatt...@nvidia.com> >> if (ret) { >> pDev->spriteInfo->anim.elt = 0; >> pDev->spriteInfo->anim.pCursor = pCursor; >> @@ -176,15 +187,9 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr >> pScreen, CursorPtr pCursor) >> AnimCurTimerNotify, pDev); >> } >> } >> -else >> -ret = TRUE; >> } >> else { >> -CursorPtr old = pDev->spriteInfo->anim.pCursor; >> - >> -if (old && IsAnimCur(old)) >> -TimerCancel(GetAnimCur(old)->timer); >> - >> +AnimCurCancelTimer(pDev); >> pDev->spriteInfo->anim.pCursor = 0; >> pDev->spriteInfo->anim.pScreen = 0; >> ret = (*pScreen->DisplayCursor) (pDev, pScreen, pCursor); >> > ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 1/2] animcur: Fix transitions between animated cursors
On 01/09/2018 08:51 AM, Adam Jackson wrote: > We weren't cancelling the old timer when changing cursors, making things > go all crashy. Logically we could always cancel the timer first, but > then we'd have to call TimerSet to re-arm ourselves, and GetTimeInMillis > is potentially expensive. > > Reported-by: > https://devtalk.nvidia.com/default/topic/1028172/linux/titan-v-ubuntu-16-04lts-and-387-34-driver-crashes-badly/post/5230967/#5230967 > Signed-off-by: Adam Jackson <a...@redhat.com> > --- > render/animcur.c | 25 +++-- > 1 file changed, 15 insertions(+), 10 deletions(-) > > diff --git a/render/animcur.c b/render/animcur.c > index 058bc1b323..797029443c 100644 > --- a/render/animcur.c > +++ b/render/animcur.c > @@ -151,11 +151,20 @@ AnimCurTimerNotify(OsTimerPtr timer, CARD32 now, void > *arg) > return ac->elts[elt].delay; > } > > +static void > +AnimCurCancelTimer(DeviceIntPtr pDev) > +{ > +CursorPtr cur = pDev->spriteInfo->anim.pCursor; > + > +if (IsAnimCur(cur)) > +TimerCancel(GetAnimCur(cur)->timer); > +} > + > static Bool > AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor) > { > AnimCurScreenPtr as = GetAnimCurScreen(pScreen); > -Bool ret; > +Bool ret = TRUE; > > if (IsFloating(pDev)) > return FALSE; > @@ -165,8 +174,10 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr > pScreen, CursorPtr pCursor) > if (pCursor != pDev->spriteInfo->anim.pCursor) { > AnimCurPtr ac = GetAnimCur(pCursor); > > -ret = (*pScreen->DisplayCursor) > -(pDev, pScreen, ac->elts[0].pCursor); > +AnimCurCancelTimer(pDev); > +ret = (*pScreen->DisplayCursor) (pDev, pScreen, > + ac->elts[0].pCursor); > + This is a slight change in behavior if DisplayCursor fails since it will cancel the previous timer whereas before it wouldn't. Are there any weird cases where failing to change the cursor is expected and canceling animation of the previous cursor would be a problem? Assuming not, both changes Reviewed-by: Aaron Plattner <aplatt...@nvidia.com> I'll try to put together a build to test these today. > if (ret) { > pDev->spriteInfo->anim.elt = 0; > pDev->spriteInfo->anim.pCursor = pCursor; > @@ -176,15 +187,9 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr > pScreen, CursorPtr pCursor) > AnimCurTimerNotify, pDev); > } > } > -else > -ret = TRUE; > } > else { > -CursorPtr old = pDev->spriteInfo->anim.pCursor; > - > -if (old && IsAnimCur(old)) > -TimerCancel(GetAnimCur(old)->timer); > - > +AnimCurCancelTimer(pDev); > pDev->spriteInfo->anim.pCursor = 0; > pDev->spriteInfo->anim.pScreen = 0; > ret = (*pScreen->DisplayCursor) (pDev, pScreen, pCursor); > ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] animcur: Move timer into pDev->spriteInfo->anim
On 01/09/2018 08:49 AM, Adam Jackson wrote: > On Mon, 2018-01-08 at 15:02 -0800, Aaron Plattner wrote: >> Commit 094a63d56fbfb9e23210cc9ac538fb198af37cee moved the timer that handles >> animated cursors from the per-screen AnimCurScreenRec into the per-cursor >> AnimCurRec. However, the timer that runs takes the DeviceIntPtr as its >> argument, >> and looks up which screen the cursor is supposed to be on from that. >> >> This leads to a problem when a device changes from one animated cursor to >> another: The timer for the first cursor is not canceled. Then, when the >> device >> transitions to a static cursor, the second timer is canceled >> pDev->spriteInfo->anim.pScreen is cleared. Finally, the first timer fires and >> AnimCurTimerNotify crashes because pScreen is NULL. >> >> Since there's only ever supposed to be one timer pending for a given device, >> move the timer into pDev->spriteInfo->anim.pTimer. This timer is canceled >> when >> transitioning to a static cursor, and re-armed when transitioning from one >> animated cursor to another. >> >> Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> >> Reported-by: >> https://devtalk.nvidia.com/default/topic/1028172/linux/titan-v-ubuntu-16-04lts-and-387-34-driver-crashes-badly/post/5230967/#5230967 >> --- >> I'm not convinced that I fully understood how these timers worked with >> multiple >> X screens before 094a63d56fbf, so hopefully this is an acceptable fix. It >> seems >> to work in my single-X-screen testing. > > I think what you've got gets multiple screens right. I had considered > just always cancelling the timer first, but that would mean re-arming > the timer constantly too, and the less we can call TimerSet (and thus > GetTimeInMillis) the better. > > There's one corner case I think this gets wrong: > >> @@ -325,13 +321,11 @@ AnimCursorCreate(CursorPtr *cursors, CARD32 *deltas, >> int ncursor, >> pCursor->id = cid; >> >> ac = GetAnimCur(pCursor); >> -ac->timer = TimerSet(NULL, 0, 0, AnimCurTimerNotify, NULL); > > This bit matters. If TimerSet's first argument is NULL it will > allocate, and that can fail. If the TimerSet in AnimCurDisplayCursor > would fail, we would need to unwind the work we just did to change the > sprite, which could itself fail. That's too hairy to want to deal with, > hence allocating up front. (Granted I didn't _check_ whether that > allocation succeeded...) Yeah, that's a good point. > But then the other problem is: > >> --- a/include/inputstr.h >> +++ b/include/inputstr.h >> @@ -514,6 +514,7 @@ typedef struct _SpriteInfoRec { >> struct { >> CursorPtr pCursor; >> ScreenPtr pScreen; >> +OsTimerPtr pTimer; >> int elt; >> } anim; >> } SpriteInfoRec, *SpriteInfoPtr; > > That's an ABI header, so I can't cherry-pick this fix to 1.19. I'd be surprised if anyone outside the server used these fields, but you're right, better safe than sorry. > I think we can just cancel the old timer when changing animcurs. > Patches to follow in a moment. Agreed. > - ajax > ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] animcur: Move timer into pDev->spriteInfo->anim
Commit 094a63d56fbfb9e23210cc9ac538fb198af37cee moved the timer that handles animated cursors from the per-screen AnimCurScreenRec into the per-cursor AnimCurRec. However, the timer that runs takes the DeviceIntPtr as its argument, and looks up which screen the cursor is supposed to be on from that. This leads to a problem when a device changes from one animated cursor to another: The timer for the first cursor is not canceled. Then, when the device transitions to a static cursor, the second timer is canceled pDev->spriteInfo->anim.pScreen is cleared. Finally, the first timer fires and AnimCurTimerNotify crashes because pScreen is NULL. Since there's only ever supposed to be one timer pending for a given device, move the timer into pDev->spriteInfo->anim.pTimer. This timer is canceled when transitioning to a static cursor, and re-armed when transitioning from one animated cursor to another. Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> Reported-by: https://devtalk.nvidia.com/default/topic/1028172/linux/titan-v-ubuntu-16-04lts-and-387-34-driver-crashes-badly/post/5230967/#5230967 --- I'm not convinced that I fully understood how these timers worked with multiple X screens before 094a63d56fbf, so hopefully this is an acceptable fix. It seems to work in my single-X-screen testing. dix/devices.c | 1 + include/inputstr.h | 1 + render/animcur.c | 14 -- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/dix/devices.c b/dix/devices.c index 4a628afb0f49..98493df58fb6 100644 --- a/dix/devices.c +++ b/dix/devices.c @@ -1014,6 +1014,7 @@ CloseDevice(DeviceIntPtr dev) free(dev->last.touches); dev->config_info = NULL; dixFreePrivates(dev->devPrivates, PRIVATE_DEVICE); +TimerFree(dev->spriteInfo->anim.pTimer); free(dev); } diff --git a/include/inputstr.h b/include/inputstr.h index 5f0026b9b733..7de1ed39874d 100644 --- a/include/inputstr.h +++ b/include/inputstr.h @@ -514,6 +514,7 @@ typedef struct _SpriteInfoRec { struct { CursorPtr pCursor; ScreenPtr pScreen; +OsTimerPtr pTimer; int elt; } anim; } SpriteInfoRec, *SpriteInfoPtr; diff --git a/render/animcur.c b/render/animcur.c index 058bc1b3237c..83cb5572c740 100644 --- a/render/animcur.c +++ b/render/animcur.c @@ -55,7 +55,6 @@ typedef struct _AnimCurElt { typedef struct _AnimCur { int nelt; /* number of elements in the elts array */ AnimCurElt *elts; /* actually allocated right after the structure */ -OsTimerPtr timer; } AnimCurRec, *AnimCurPtr; typedef struct _AnimScrPriv { @@ -171,19 +170,16 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor) pDev->spriteInfo->anim.elt = 0; pDev->spriteInfo->anim.pCursor = pCursor; pDev->spriteInfo->anim.pScreen = pScreen; - -ac->timer = TimerSet(ac->timer, 0, ac->elts[0].delay, - AnimCurTimerNotify, pDev); +pDev->spriteInfo->anim.pTimer = +TimerSet(pDev->spriteInfo->anim.pTimer, 0, + ac->elts[0].delay, AnimCurTimerNotify, pDev); } } else ret = TRUE; } else { -CursorPtr old = pDev->spriteInfo->anim.pCursor; - -if (old && IsAnimCur(old)) -TimerCancel(GetAnimCur(old)->timer); +TimerCancel(pDev->spriteInfo->anim.pTimer); pDev->spriteInfo->anim.pCursor = 0; pDev->spriteInfo->anim.pScreen = 0; @@ -325,13 +321,11 @@ AnimCursorCreate(CursorPtr *cursors, CARD32 *deltas, int ncursor, pCursor->id = cid; ac = GetAnimCur(pCursor); -ac->timer = TimerSet(NULL, 0, 0, AnimCurTimerNotify, NULL); /* security creation/labeling check */ rc = XaceHook(XACE_RESOURCE_ACCESS, client, cid, RT_CURSOR, pCursor, RT_NONE, NULL, DixCreateAccess); if (rc != Success) { -TimerFree(ac->timer); dixFiniPrivates(pCursor, PRIVATE_CURSOR); free(pCursor); return rc; -- 2.15.1 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 3/4] animcur: Run the timer from the device, not the screen
On 01/08/2018 01:07 PM, Adam Jackson wrote: On Mon, 2018-01-08 at 12:39 -0800, Aaron Plattner wrote: Nothing like deploying code in the wild to find bugs. :( Hah! So it goes. The user on the forum reported a crash after this patch and I reproduced it locally: Thread 1 "Xorg" received signal SIGSEGV, Segmentation fault. 0x55604b0b8acd in dixGetPrivateAddr (privates=0x3e8, key=0x55604b40ace0 ) at ../include/privates.h:123 123 ../include/privates.h: No such file or directory. (gdb) bt #0 0x55604b0b8acd in dixGetPrivateAddr (privates=0x3e8, key=0x55604b40ace0 ) at ../include/privates.h:123 #1 0x55604b0b8b5d in dixLookupPrivate (privates=0x3e8, key=0x55604b40ace0 ) at ../include/privates.h:165 #2 0x55604b0b8d79 in AnimCurTimerNotify (timer=0x55604d751a30, now=45483869, arg=0x55604d31cb70) at animcur.c:134 #3 0x55604b15fc70 in DoTimer () #4 0x55604b15fcd7 in DoTimers () #5 0x55604b15ffa7 in WaitForSomething () #6 0x55604af90ec1 in Dispatch () at dispatch.c:422 #7 0x55604af9e7dd in dix_main (argc=14, argv=0x7ffd9f1c4c78, envp=0x7ffd9f1c4cf0) at main.c:287 #8 0x7f64f5282f4a in __libc_start_main () at /usr/lib/libc.so.6 #9 0x55604af82a8a in _start () #2 0x55604b0b8d79 in AnimCurTimerNotify (timer=0x55604d751a30, now=45483869, arg=0x55604d31cb70) at animcur.c:134 134 in animcur.c (gdb) p pScreen $8 = (ScreenPtr) 0x0 I'm not sure how this is happening, yet. Yeah that's a puzzler. The only place that should zero out anim.pScreen is the "switch to static cursor" path in AnimCurDisplayCursor, but that path also cancels the timer so AnimCurTimerNotify shouldn't get called for that device anymore. Maybe I missed something about how multiple cursors work? Yeah, the problem seems to be when the cursor for the pDev transitions from one animated cursor to another. It doesn't cancel the first timer from the first cursor, but still schedules a new one for the second cursor. Later, when it transitions to a static cursor, it cancels the second timer and clears pDev->spriteInfo->anim.pScreen, and then the first timer fires: Dev 0x55e84a07dd70 animcursor went from (nil) to 0x55e84a294340 Dev 0x55e84a07dd70 animcursor went from (nil) to 0x55e84a3b5ad0 Dev 0x55e84a07dd70 animcursor went from (nil) to 0x55e84a4cb770, scheduled timer 0x55e84a4cbb90 Dev 0x55e84a07dd70 animcursor went from 0x55e84a4cb770 to 0x55e84a4b4570, scheduled timer 0x55e84a4b1120 Dev 0x55e84a07dd70 animcursor went from 0x55e84a4b4570 to 0x55e84a46b800 Canceled timer 0x55e84a4b1120 (EE) (EE) Backtrace: (EE) 0: /usr/lib/xorg-server/Xorg (OsSigHandler+0x3b) [0x55e847e294f0] (EE) 1: /usr/lib/libpthread.so.0 (funlockfile+0x50) [0x7fa9aba62dff] (EE) 2: /usr/lib/xorg-server/Xorg (dixGetPrivateAddr+0x3e) [0x55e847d699f3] (EE) 3: /usr/lib/xorg-server/Xorg (dixLookupPrivate+0x2e) [0x55e847d69a83] (EE) 4: /usr/lib/xorg-server/Xorg (AnimCurTimerNotify+0x48) [0x55e847d69c9f] (EE) 5: /usr/lib/xorg-server/Xorg (DoTimer+0x37) [0x55e847e20c63] (EE) 6: /usr/lib/xorg-server/Xorg (DoTimers+0x31) [0x55e847e20cc7] (EE) 7: /usr/lib/xorg-server/Xorg (check_timers+0x3d) [0x55e847e209a7] (EE) 8: /usr/lib/xorg-server/Xorg (WaitForSomething+0x7e) [0x55e847e20a53] (EE) 9: /usr/lib/xorg-server/Xorg (Dispatch+0x53) [0x55e847c2c1de] (EE) 10: /usr/lib/xorg-server/Xorg (dix_main+0x651) [0x55e847c3b55f] (EE) 11: /usr/lib/xorg-server/Xorg (main+0x28) [0x55e847c1bff2] (EE) 12: /usr/lib/libc.so.6 (__libc_start_main+0xea) [0x7fa9ab6baf4a] (EE) 13: /usr/lib/xorg-server/Xorg (_start+0x2a) [0x55e847c1beea] (EE) (EE) Segmentation fault at address 0x3e8 (EE) Fatal server error: (EE) Caught signal 11 (Segmentation fault). Server aborting Given how this timer is used, would it make sense to move the timer into pDev->spriteInfo->anim.pTimer? The downside there I guess is that CloseDevice() would have to TimerFree() it. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 3/4] animcur: Run the timer from the device, not the screen
Nothing like deploying code in the wild to find bugs. :( The user on the forum reported a crash after this patch and I reproduced it locally: Thread 1 "Xorg" received signal SIGSEGV, Segmentation fault. 0x55604b0b8acd in dixGetPrivateAddr (privates=0x3e8, key=0x55604b40ace0 ) at ../include/privates.h:123 123 ../include/privates.h: No such file or directory. (gdb) bt #0 0x55604b0b8acd in dixGetPrivateAddr (privates=0x3e8, key=0x55604b40ace0 ) at ../include/privates.h:123 #1 0x55604b0b8b5d in dixLookupPrivate (privates=0x3e8, key=0x55604b40ace0 ) at ../include/privates.h:165 #2 0x55604b0b8d79 in AnimCurTimerNotify (timer=0x55604d751a30, now=45483869, arg=0x55604d31cb70) at animcur.c:134 #3 0x55604b15fc70 in DoTimer () #4 0x55604b15fcd7 in DoTimers () #5 0x55604b15ffa7 in WaitForSomething () #6 0x55604af90ec1 in Dispatch () at dispatch.c:422 #7 0x55604af9e7dd in dix_main (argc=14, argv=0x7ffd9f1c4c78, envp=0x7ffd9f1c4cf0) at main.c:287 #8 0x7f64f5282f4a in __libc_start_main () at /usr/lib/libc.so.6 #9 0x55604af82a8a in _start () #2 0x55604b0b8d79 in AnimCurTimerNotify (timer=0x55604d751a30, now=45483869, arg=0x55604d31cb70) at animcur.c:134 134 in animcur.c (gdb) p pScreen $8 = (ScreenPtr) 0x0 I'm not sure how this is happening, yet. -- Aaron On 11/06/2017 12:19 PM, Adam Jackson wrote: > This is very slightly more efficient since the callback now doesn't need > to walk every input device, instead we know exactly which device's > cursor is being updated. AnimCurTimerNotify() gets outdented nicely as a > result. A more important side effect is that we can stop using the > TimerAbsolute mode and just pass in the relative delay. > > In AnimCurSetCursorPosition, we no longer need to rearm the timer with > the new screen; it is enough to update the device's state. In > AnimCurDisplayCursor we need to notice when we're switching from > animated cursor to regular and cancel the existing timer. > > Signed-off-by: Adam Jackson> --- > render/animcur.c | 87 > +++- > 1 file changed, 29 insertions(+), 58 deletions(-) > > diff --git a/render/animcur.c b/render/animcur.c > index 05dfc640aa..e59a7c3c40 100644 > --- a/render/animcur.c > +++ b/render/animcur.c > @@ -55,6 +55,7 @@ typedef struct _AnimCurElt { > typedef struct _AnimCur { > int nelt; /* number of elements in the elts array */ > AnimCurElt *elts; /* actually allocated right after the > structure */ > +OsTimerPtr timer; > } AnimCurRec, *AnimCurPtr; > > typedef struct _AnimScrPriv { > @@ -65,8 +66,6 @@ typedef struct _AnimScrPriv { > RealizeCursorProcPtr RealizeCursor; > UnrealizeCursorProcPtr UnrealizeCursor; > RecolorCursorProcPtr RecolorCursor; > -OsTimerPtr timer; > -Bool timer_set; > } AnimCurScreenRec, *AnimCurScreenPtr; > > static unsigned char empty[4]; > @@ -131,49 +130,27 @@ AnimCurCursorLimits(DeviceIntPtr pDev, > static CARD32 > AnimCurTimerNotify(OsTimerPtr timer, CARD32 now, void *arg) > { > -ScreenPtr pScreen = arg; > +DeviceIntPtr dev = arg; > +ScreenPtr pScreen = dev->spriteInfo->anim.pScreen; > AnimCurScreenPtr as = GetAnimCurScreen(pScreen); > -DeviceIntPtr dev; > -Bool activeDevice = FALSE; > -CARD32 soonest = ~0; /* earliest time to wakeup again */ > - > -for (dev = inputInfo.devices; dev; dev = dev->next) { > -if (IsPointerDevice(dev) && pScreen == > dev->spriteInfo->anim.pScreen) { > -if (!activeDevice) > -activeDevice = TRUE; > - > -if ((INT32) (now - dev->spriteInfo->anim.time) >= 0) { > -AnimCurPtr ac = GetAnimCur(dev->spriteInfo->anim.pCursor); > -int elt = (dev->spriteInfo->anim.elt + 1) % ac->nelt; > -DisplayCursorProcPtr DisplayCursor; > - > -/* > - * Not a simple Unwrap/Wrap as this > - * isn't called along the DisplayCursor > - * wrapper chain. > - */ > -DisplayCursor = pScreen->DisplayCursor; > -pScreen->DisplayCursor = as->DisplayCursor; > -(void) (*pScreen->DisplayCursor) (dev, > - pScreen, > - ac->elts[elt].pCursor); > -as->DisplayCursor = pScreen->DisplayCursor; > -pScreen->DisplayCursor = DisplayCursor; > - > -dev->spriteInfo->anim.elt = elt; > -dev->spriteInfo->anim.time = now + ac->elts[elt].delay; > -} > > -if (soonest > dev->spriteInfo->anim.time) > -soonest = dev->spriteInfo->anim.time; > -} > -} > +AnimCurPtr ac = GetAnimCur(dev->spriteInfo->anim.pCursor); > +int elt = (dev->spriteInfo->anim.elt
Re: [PATCH xserver 1/4] animcur: Use fixed-size screen private
On 11/12/2017 04:35 PM, Robert Morell wrote: On Mon, Nov 06, 2017 at 03:19:51PM -0500, Adam Jackson wrote: Signed-off-by: Adam Jackson--- render/animcur.c | 15 --- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/render/animcur.c b/render/animcur.c index 52e6b8b79f..0707fe7271 100644 --- a/render/animcur.c +++ b/render/animcur.c @@ -77,12 +77,9 @@ static CursorBits animCursorBits = { static DevPrivateKeyRec AnimCurScreenPrivateKeyRec; -#define AnimCurScreenPrivateKey () - #define IsAnimCur(c) ((c) && ((c)->bits == )) #define GetAnimCur(c) ((AnimCurPtr) char *)(c) + CURSOR_REC_SIZE -#define GetAnimCurScreen(s) ((AnimCurScreenPtr)dixLookupPrivate(&(s)->devPrivates, AnimCurScreenPrivateKey)) -#define SetAnimCurScreen(s,p) dixSetPrivate(&(s)->devPrivates, AnimCurScreenPrivateKey, p) +#define GetAnimCurScreen(s) ((AnimCurScreenPtr)dixLookupPrivate(&(s)->devPrivates, )) #define Wrap(as,s,elt,func) (((as)->elt = (s)->elt), (s)->elt = func) #define Unwrap(as,s,elt)((s)->elt = (as)->elt) @@ -101,7 +98,6 @@ AnimCurCloseScreen(ScreenPtr pScreen) Unwrap(as, pScreen, RealizeCursor); Unwrap(as, pScreen, UnrealizeCursor); Unwrap(as, pScreen, RecolorCursor); -SetAnimCurScreen(pScreen, 0); ret = (*pScreen->CloseScreen) (pScreen); free(as); The free() above should be removed as well, otherwise it causes a double-free on screen teardown. Other than that, Reviewed-by: Robert Morell Tested-by: Robert Morell for the series. At least two users have hit this now: https://devtalk.nvidia.com/default/topic/1028172/linux/titan-v-ubuntu-16-04lts-and-387-34-driver-crashes-badly/post/5230799/#5230799 Adam, can this series be merged? -- Aaron Thanks, Robert return ret; @@ -308,15 +304,13 @@ AnimCurInit(ScreenPtr pScreen) { AnimCurScreenPtr as; -if (!dixRegisterPrivateKey(, PRIVATE_SCREEN, 0)) +if (!dixRegisterPrivateKey(, PRIVATE_SCREEN, + sizeof(AnimCurScreenRec))) return FALSE; -as = (AnimCurScreenPtr) malloc(sizeof(AnimCurScreenRec)); -if (!as) -return FALSE; +as = GetAnimCurScreen(pScreen); as->timer = TimerSet(NULL, TimerAbsolute, 0, AnimCurTimerNotify, pScreen); if (!as->timer) { -free(as); return FALSE; } as->timer_set = FALSE; @@ -329,7 +323,6 @@ AnimCurInit(ScreenPtr pScreen) Wrap(as, pScreen, RealizeCursor, AnimCurRealizeCursor); Wrap(as, pScreen, UnrealizeCursor, AnimCurUnrealizeCursor); Wrap(as, pScreen, RecolorCursor, AnimCurRecolorCursor); -SetAnimCurScreen(pScreen, as); return TRUE; } -- 2.14.2 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Add NonDesktop output property and behaviors.
Sorry for the extremely slow reply. I started writing this and then forgot about it when I went on vacation. I've been out of the loop since I got back so I apologize if the discussion has moved on and the responses below are stale. On 10/20/2017 12:53 PM, Keith Packard wrote: Aaron Plattner <aplatt...@nvidia.com> writes: I think this makes sense. Comments below. Reviewed-by: Aaron Plattner <aplatt...@nvidia.com> Thanks, Aaron. Have you also looked at the leasing changes on the same branch? I'd like to know what you think of that plan for implementing the Vulkan acquire Xlib display extension. I've got code which uses it, but I'd like to know if that's something you might also be able to implement in your environment. With those two pieces, I'd be able to finish version 1.6. I didn't look at it in a lot of detail, but the plan sounds mostly alright to me, although I'm concerned about which pieces applications are going to try to use directly. The big issue for us is that we have to support systems that don't have DRM or this new RandR interface. We have a permission model similar to DRM leases, and use NV-GLX to pass the relevant file descriptors to the client. The Vulkan interface is layered on top of that. I'd hoped that the RandR-based protocol for getting DRM lease file descriptors would likewise be an implementation detail opaque to Vulkan clients. As long as we're treating them that way, the changes to the Vulkan API sounded okay to me, but James and Damien are the experts there. For actually supporting the DRM lease / RandR interface: Our nvidia-modeset.ko has the concept of a "modeset owner" that's pretty similar to DRM's master concept. When nvidia-drm.ko is loaded with the "modeset=1" parameter, the DRM master becomes the nvidia-modeset owner and all of DRM's normal permission model should apply. However, our X driver currently closes the DRM master fd if we get one and talks to nvidia-modeset directly. I don't know if we'll be able to handle DRM leasing and nvidia-modeset permissions at the same time with our X driver -- we would need to wrap nvidia-modeset permission tokens in DRM lease file descriptors somehow. I don't think a separate NonDesktop connection state property is needed. I agree that you can infer that from the presence of an EDID and modes. To take the other side of this argument, in reviewing the contents of an RRGetOutputInfo reply, I don't think any of those values are 100% reliable in telling whether something is connected or not. You can create and assign modes to a disconnected output, and there are no properties guaranteed to be set. Hmm, I suppose clients can create and write to the EDID property as well. But if you manually write modes and an HMD EDID property to an output, it seems reasonable to me for a desktop environment to treat it as one. That actually seems like a good way to test this infrastructure without needing to have a real HMD attached. Of course, it's rare these days for people to create their own modes and assign them. We still have many cases where monitors fail to provide EDID data, and I wonder if that won't be more common for some things that we do want to hide from the normal desktop? Hrm. Now I'm thinking that just having a property which gets set wouldn't be a terrible plan. Thoughts? How would the driver know to set NonDesktop without an EDID? Some kind of devicetree entry or platform-specific hardcode? Without a specific example of such a device this feels like overengineering to me, but I'm not opposed to adding the extra property if it's just a boolean. -- Aaron Add a period at the end of this sentence (and a couple below), maybe? Thanks, just a cut error. Giuseppe, I'm not sure I understand your suggestion about "attached" and "detached" modes here. I.e. what exactly would a driver do in response to a client writing this property? If you just want to switch between being part of the desktop and not, you can do that by attaching or detaching it from a crtc. I think the goal was to have it controlled through the X server, but not play a part of the larger desktop. The motivation here is that a client would use this output through some other non-X API. Specifically Vulkan direct-to-display for virtual reality. So X wouldn't be configured to drive this output with a crtc. Instead, it would lease the output and a crtc to the client for its direct use. That's certainly our current motivation; we need to be careful to not design-out other possible uses, while at the same time not over-designing the solution. Nothing in this proposal makes it impossible for an application to use X to drive this extra output, it's just not the motivation at present. And, I agree that there's no particular need to write this property to get the output to not appear in the desktop; the existing 'Monitor' mechanism is sufficient for this. Having it writable opens a
Re: [PATCH xserver] xfree86: fix gamma compute when palette_size > 256
On 10/30/2017 03:28 AM, Michel Dänzer wrote: On 30/10/17 07:33 AM, Qiang Yu wrote: palette_(red|green|blue)_size > crtc->gamma_size (=256) this may happen when screen has per RGB chanel > 8bit, i.e. 30bit depth screen 10bit per RGB. Is palette_size > gamma_size really useful though? Seems to me like gamma_size should always be >= palette_size. Specifically, if the gamma ramp only has 256 entries, what's the benefit of having 1024 entries in the palette? What's the exact configuration in which you hit this? It's the significant bits in the colormap that matter, and not the number of colormap entries, right? There are always 2^(depth) colormap entries, so 256 for 8 bits per component and 1024 for 10. On our hardware, colormap entries have 11 significant bits of precision, so they have a [0,2047] range. On the other end of the display pipeline, the gamma ramp in the hardware has 1024 entries. The hardware will interpolate between gamma ramp entries if your color happens to fall between them. This can happen both because the colormap entries has twice as much precision as the gamma ramp, and because the "digital vibrance" and CSC matrix stuff is applied between the colormap and the gamma ramp and can skew colors around a bit. This patch doesn't affect our driver since we don't use the xf86 RandR layer, but I thought I would point out that palette_size is not directly related to gamma_size, and the colormap precision is only loosely related to gamma_size. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Add NonDesktop output property and behaviors.
On 10/19/2017 10:52 AM, Keith Packard wrote: NonDesktop devices are those to which the normal desktop environment should not be extended. Examples are Head-mounted displays and the Apple Touch Bar. How an output device is set to NonDesktop is not part of this proposal; it is expected that the underlying operating system will provide this information and have it reflected to X applications through this extension. Signed-off-by: Keith Packard <kei...@keithp.com> I think this makes sense. Comments below. Reviewed-by: Aaron Plattner <aplatt...@nvidia.com> --- randrproto.txt | 84 +++--- 1 file changed, 81 insertions(+), 3 deletions(-) diff --git a/randrproto.txt b/randrproto.txt index 7312e0b..e717a60 100644 --- a/randrproto.txt +++ b/randrproto.txt @@ -186,14 +186,23 @@ from the CRTCs. The Monitor marked as Primary will be listed first. 1.6. Introduction to version 1.6 of the extension -Version 1.6 adds resource leasing. +Version 1.6 adds resource leasing and non desktop output management. - • A 'Lease' is a collection of crtcs and outputs which are made + • A “Lease” is a collection of crtcs and outputs which are made available to a client for direct access via kernel KMS and DRM APIs. This is done by passing a suitable file descriptor back to the client which has access to those resources. While leased, those resources aren't used by the X server. + • A “NonDesktop” output is a device which should not normally be + considered as part of the desktop environment. Head-mounted + displays and the Apple "Touch Bar" are examples of such + devices. A desktop environment should be able to discover which + outputs are connected to such devices and, by default, not present + normal desktop applications on them. This is done by having + RRGetOutputInfo report such devices as Disconnected while reporting + all other information about the device correctly. I don't think a separate NonDesktop connection state property is needed. I agree that you can infer that from the presence of an EDID and modes. 1.99 Acknowledgments Our thanks to the contributors to the design found on the xpert mailing @@ -764,6 +773,12 @@ dynamic changes in the display environment. monitor in some way; for fixed-pixel devices, this would generally indicate which modes match the resolution of the output device. + Changes in version 1.6 of the protocol: + + When a “NonDesktop” device is connected, the 'connection' + field will report Disconnected but the remaining fields will + report information about the connected device. + ┌─── RRListOutputProperties output:OUTPUT @@ -775,6 +790,12 @@ dynamic changes in the display environment. This request returns the atoms of properties currently defined on the output. + Changes in version 1.6 of the protocol: + + When a “NonDesktop” device is connected, the property list + will be correct for the device, even though RRGetOutputInfo + reports the device as disconnected Add a period at the end of this sentence (and a couple below), maybe? + ┌─── RRQueryOutputProperty output: OUTPUT @@ -806,6 +827,12 @@ dynamic changes in the display environment. changed by clients. Immutable properties are interpreted by the X server. + Changes in version 1.6 of the protocol: + + When a “NonDesktop” device is connected, the property information + will be correct for the device, even though RRGetOutputInfo + reports the device as disconnected + ┌─── RRConfigureOutputProperty output: OUTPUT @@ -924,6 +951,12 @@ dynamic changes in the display environment. is True and the bytes-after is zero, the property is also deleted from the output, and a RROutputPropertyNotify event is generated. + Changes in version 1.6 of the protocol: + + When a “NonDesktop” device is connected, the property value + will be correct for the device, even though RRGetOutputInfo + reports the device as disconnected + ┌─── RRCreateMode window: WINDOW @@ -1816,6 +1849,12 @@ factors, such as re-cabling a monitor, etc. precise change can be detected by examining the new state of the system. + Changes in version 1.6 of the protocol: + + When a “NonDesktop” device is connected, this event will be + delivered when the connection status of the output changes, + however the 'connection' value will be set to 'Disconnected'. + ┌─── RROutputPropertyNotify: window: WINDOW window requesting notification @@ -1955,6 +1994,13 @@ as long as the semantics are not altered. Clients SHOULD fall back gracefully to lower version functionality, though, if the driver doesn't handle a mandatory property correctly. +Changes in version 1.6 of
Re: [RFC] DeepColor Visual Class Extension
On 09/11/2017 09:10 AM, Adam Jackson wrote: > On Mon, 2017-08-14 at 19:17 -0700, Alex Goins wrote: > >> 2. DeepColor Visual Class >> >> The DeepColor extension defines a new visual class, DeepColor. >> >> DeepColor visuals do not make use of the red_mask, green_mask, blue_mask, or >> colormap_size fields of the XVisualInfo structure. Colormap size is defined >> to >> be 0. > > Setting things to 0 that used to be reliably not 0 still makes me a > little nervous, it tends to be a good way to provoke the client into > either dividing by zero or transforming malloc(0) into a failure. > > Or, just violating other invariants. tk has this little gem when > mapping from color name to pixel value: > > if (stressPtr->numColors == 0) { > Tcl_Panic("FindClosestColor ran out of colors"); > } > > ... where numColors is filled in from ->colormap_size in the > XVisualInfo. Granted there are probably few tk apps in the world that > would like to take advantage of HDR, but any amount of mandatory > toolkit change is going to make adoption harder. > > Some of this can probably be avoided by ensuring that the enum for > DeepColor is an even number (see comment in ), but I might > still be cautious and set this to 1 in practice and assert that it's > undefined in the spec. > >> 3. Colorspace/Encoding Window Properties >> >> Windows with DeepColor visuals must rely on window properties, as opposed to >> colormaps, to determine the relationship between pixel values and colors. >> These >> properties must specify constants that correspond to colorspace/encoding >> pairs. >> >> Possible colorspace/encoding constants are defined to be: >> >> // Undefined: >> // Undefined colorspace and encoding >> #define _DEEPCOLOR_UNDEFINED0 > > This may want a note saying this is just a sentinel value and not > something you actually want to set on your window. Certainly the > compositor is not going to know how to properly present "no > colorspace". > >> 3.1.1. Root Window Properties >> >> Servers that support the DeepColor extension MUST, when initialized, create a >> child of the root window named "DEEPCOLOR_PROPERTIES". > > Name as in the WM_NAME property I assume. If the window is identified by an XID property on the root window, it doesn't seem like it necessarily needs a WM_NAME. Alex would it make sense to just drop this from the spec, and then we can still add a WM_NAME in the implementation for convenience? I would hate for applications to get the wrong idea and try to find this window by walking the window tree instead of just looking up the root window property. >> 3.1.2. "DEEPCOLOR_PROPERTIES" Window Properties >> >> The DeepColor extension defines a window property associated with >> "DEEPCOLOR_PROPERTIES" for determining the set of colorspaces/encodings >> supported by the compositor: >> >> _DEEPCOLOR_COMPOSITOR_COLORSPACES, colorspace, score, CARDINAL[][2]/32 >> >> This is a list of 2-tuples indicating the colorspaces/encodings supported by >> the >> compositor (whether internal or external to the server), and their level of >> preference as indicated by their score, where higher is better. The server >> MUST >> initialize the property with a list of colorspaces/encodings supported by its >> internal compositor, which may be an empty list if no DeepColor capabilities >> are >> supported. As discussed in section 3.2, applications MUST choose a >> colorspace/encoding from this set, if non-empty. > > Would there be any benefit to making these a 3-tuple of { colorspace, > pixel format, score } ? Thinking there might be hardware where FP16 is > enough more painful than 10bpc that performance would be unacceptable. > Though, if that were true, that's almost certainly true regardless of > the associated colorspace, so maybe pixel formats just want a separate > priority list, or maybe there are so few pixel formats that it's > obvious what to do. > >> If an external application becomes responsible for compositing and supports >> the >> DeepColor extension, it MUST override this property with its own supported >> colorspaces/encodings prior to calling XCompositeRedirectSubwindows on the >> root >> window. The server MUST delay sending the PropertyNotify event for the >> change in >> _DEEPCOLOR_COMPOSITOR_COLORSPACES until after the root window hierarchy has >> been >> redirected. > > The only way to implement this at present is for the driver to override > ProcChangeProperty in the dispatch vector, which is... distasteful. I > really think this should be a new request that the compositor must make > before redirecting subwindows: > > DPCCOLORSPACE: [ > colorspace: CARD32 > score: CARD32 > ] > > DPCSetCompositeColorspace: > window: WINDOW > colorspace_list: LISTofDPCCOLORSPACE > > If we remember that redirection can happen anywhere in the hierarchy, > we could imagine a DEEPCOLOR_PROPERTIES window at any level. With a new > request,
Re: xf86-video-dummy patch series - was "Cleanups Redux"
On 08/05/2017 08:02 AM, Antoine Martin wrote: > On 17/12/16 02:04, Emil Velikov wrote: >> Hi Bob, >> >> On 9 December 2016 at 22:25, Bob Terekwrote: >>> >>> On 12/09/2016 03:13 AM, Emil Velikov wrote: >>> On 6 December 2016 at 22:41, Bob Terek wrote: > > Resubmitting some of Aaron Plattner's cleanup patches to > xf86-video-dummy: > >https://lists.x.org/archives/xorg-devel/2015-January/045395.html > . . . > . . . >>> >>> Afaict the patches are literally unchanged since Aaron's submit. As such changing the authorship is a bit ... bad. >>> >>> >>> Oops, sorry for the breach in protocol. Do I need to do something specific >>> to "withdraw" the patch I sent? Should I do something at Patchwork? >>> > I've restored the "author" and added Bob as "Reviewed-by" instead. > >> Updating patchwork would be very good, indeed. >> >>> I'm going to submit an alternative approach to Aaron's 6/6, and was going >>> to include the remaining cleanups, but then it was thought the cleanups >>> should be addressed separately. So then for some reason I thought they >>> needed to be submitted again, against the current version. . . >>> >> Agreed. > These patches have been reviewed and tested quite a few times now. > I've just created a git repo with all the uncontroversial pending > changes to the dummy driver, 6 so far: > https://github.com/totaam/xf86-video-dummy/commits/master > What else can I do to help move this along? I've pushed all of these except "support for 30 bit depth in dummy driver". I want to check that changing the colormap size to 1024 for depth 24 doesn't change behavior. remote: Updating patchwork state for https://patchwork.freedesktop.org/project/Xorg/list/ remote: I: patch #41059 updated using rev 12e3e2030171b7a5df074a56293eb16da40cd99b. remote: I: patch #41060 updated using rev 7c3b090e80a9b364434120262f9bef5686cd2e2e. remote: E: failed to find patch for rev 87249af5faf85c8d093e910c069faa4db0aee843. remote: I: patch #125912 updated using rev 33e68185665b2d065525ac03332f080026b18d8d. remote: I: patch #168436 updated using rev 7957ad83b53b57f376164b10742d4e35223c9dcc. remote: E: failed to find patch for rev 5e90221dc68ae0893acd5c9b12d702269202558d. remote: I: 4 patch(es) updated to state Accepted. To git.freedesktop.org:/git/xorg/driver/xf86-video-dummy af0f808922f2..5e90221dc68a master -> master > Thanks > Antoine > > > > >> >> Thanks >> Emil >> ___ >> xorg-devel@lists.x.org: X.Org development >> Archives: http://lists.x.org/archives/xorg-devel >> Info: https://lists.x.org/mailman/listinfo/xorg-devel >> > > ___ > xorg-devel@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: https://lists.x.org/mailman/listinfo/xorg-devel > ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] composite: Make compIsAlternateVisual safe even if Composite is off
On 07/27/2017 01:02 PM, Adam Jackson wrote: As of ea483af9 we're calling this unconditionally from the GLX code so the synthetic visual is in a lower select group. If Composite has been disabled then GetCompScreen() will return NULL, and this would crash. Rather than force the caller to check first, just always return FALSE if Composite is disabled (which is correct, since none of the visuals will be synthetic in that case). Signed-off-by: Adam Jackson <a...@redhat.com> --- composite/compwindow.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composite/compwindow.c b/composite/compwindow.c index 367f23eb7..f88238146 100644 --- a/composite/compwindow.c +++ b/composite/compwindow.c @@ -326,7 +326,7 @@ compIsAlternateVisual(ScreenPtr pScreen, XID visual) CompScreenPtr cs = GetCompScreen(pScreen); int i; -for (i = 0; i < cs->numAlternateVisuals; i++) +for (i = 0; cs && i < cs->numAlternateVisuals; i++) I really hope gcc is smart enough to know that cs won't change here and this check can be hoisted out of the loop. if (cs->alternateVisuals[i] == visual) return TRUE; return FALSE; Sure, can't hurt. Reviewed-by: Aaron Plattner <aplatt...@nvidia.com> -- nvpublic ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [RFC] DeepColor Visual Class Extension
On 07/18/2017 07:18 AM, Adam Jackson wrote: > On Mon, 2017-07-17 at 18:04 -0700, Alex Goins wrote: >> This is our latest iteration on the design of the visual class for use with >> HDR >> drawables, handling how colorspace/encoding and pixel format are specified >> and >> interpreted. We've been brainstorming this internally for a while, taking >> into >> consideration comments that came up in the xorg-devel thread several months >> back. We've now formalized it as an X extension. >> >> This extension is designed to be flexible as to how HDR drawables are >> actually >> implemented, leaving it up to the server / DDX drivers to decide. It should >> be >> capable of working interchangeably when an external compositor is being used >> and >> when one is not, with the server/driver being responsible for internal >> composition in the latter case. >> >> It should be capable of working with GLX, EGL, and Vulkan, keeping track of >> colorspace/encoding and pixel format in API-agnostic locations so that >> compositors need not rely on API-specific methods for querying these >> properties. >> Supported colorspaces/encodings were chosen as a superset of those supported >> by >> the requisite EGL/Vulkan extensions, with GLX relying entirely on the X >> properties for the purpose of determining colorspace/encoding. >> >> Let us know what you think! > > Apologies for the brief reply, I'm about to be on PTO for a bit so this > is a little rushed. In general this looks quite good on a first read- > through, I'll let it sink in a bit while I'm away. > >> 2. DeepColor Visual Class >> >> The DeepColor extension defines a new visual class, DeepColor. >> >> DeepColor visuals do not make use of the red_mask, green_mask, blue_mask, or >> colormap_size fields of the XVisualInfo structure. Colormap size is defined >> to >> be 0. > > This logically makes sense, but I'm slightly nervous about exposing > clients to an xVisualType they've never seen before. Would it make > sense to hide this list from the connection setup block and only return > it in DPCGetVisualInfo? I don't think we can hide it completely, since XGetWindowAttributes will return what looks like a bogus visual then. We went through something similar with XLIB_SKIP_ARGB_VISUALS -- maybe something like that would work here? > Alternatively, would it make sense to fill in those masks with dummy > (non-zero) values that look like xrgb32 so we don't trigger a divide- > by-zero in a weird place? If we're going to ignore the values anyway... > >> 3.1. Root Window Properties >> >> The DeepColor extension defines a root window property for determining the >> set >> of colorspaces/encodings supported by the X server or external compositor: >> >> _DEEPCOLOR_COMPOSITOR_COLORSPACES, ATOM[] > > I didn't like this for style reasons when I first read through. The > problem with root window properties is that every client ends up > listening for PropertyNotify on the root window, so every change to any > property wakes up every client. Which... > >> 7. Issues >> >> This spec does not address how HDR content should be downsampled to SDR >> content >> for e.g. automatic redirection, XGetImage(), or core X11 / RENDER rendering. >> Perhaps the server could handle this transparently without the need for >> explicit functionality outlined in the spec. >> >> This spec does not address what should happen to >> __DEEPCOLOR_COMPOSITOR_COLORSPACES when an HDR-unaware compositor is >> started, as >> it probably should. > > ... kind of matters here. I think what should happen is, if the > compositor thinks it's going to be HDR-aware, it sets that property > before CompositeRedirectSubwindows for the screen, but the server > delays sending that property change event until afterwards. That way if > you (the server) see RedirectSubwindows without a property change > pending, you know to clear the colorspace list. And clients never see > an inconsistent state about which colorspaces will work. Clever. > To avoid doing too much surgery to how core property changes work we > could add this as a declaration-of-intent request to DEEP-COLOR: > > DPCSendPendingColorspaces > 1CARD8 opcode > 1X DPC opcode > 2 unused > 4n number of ATOMs in list > 4n ATOM colorspaces > > But: whether that colorspace list is cleared or changed, every client > wakes up to notice. One way around this is to create a child window of > the root that lives only to hold the deep color properties, and point > to it with a root window property (that then never changes). > > This isn't exactly the first instance of this problem. I proposed > adding property notify filters to fixes a while ago, which would > address this whole category of problem. This might be a nice excuse to > get that landed so we can stop making the problem worse. Do you have any measurements of how much real-world pain
Re: [RFC] Server side glvnd
Adding Kyle to To -- he's been working on something similar. On 07/18/2017 08:43 AM, Adam Jackson wrote: > I've been thinking about how to get multiple GL stacks to coexist > within the server, along the lines of libglvnd on the client side. This > is a bit of a brain dump, the intrepid can find some work in progress > along these lines here: > > https://cgit.freedesktop.org/~ajax/xserver/log/?h=glxfe > > The basic approach I've been taking is: how little does the GLX > provider's dispatch code need to be modified? > > For QueryVersion, we can just return 1.4. For the ClientInfo requests, > we have to dispatch them to every backend. For every other request, we > need to inspect some key element of the request and use that to map to > a backend. There are only three kinds of objects for this purpose: > screen numbers (sigh), XIDs, and context tags. The screen mapping is > straightforward. For XIDs we can set up a mapping on creation: > GLXCreateWindow is dispatched to a backend (by screen number), on > success we create a shadow resource with the same XID whose value > points to the backend that created it. > > Tags are uglier because tags are ugly [1]. The code I currently have > for this simply treats tags as XIDs because in Xorg's GLX they are. But > tags are assigned by the backend, not the client: they're in the reply > to MakeCurrent. So the backend's MakeCurrent hook will need to be > different from a non-glvnded version, allocating the tag from the > frontend. > > The other quirk with MakeCurrent is, when switching between contexts on > different backends, one must first detach the old context and then > attach the new. But that's two backend calls, and MakeCurrent as a > protocol request wants to emit a reply, and libGL is absolutely not > prepared to hear two replies to a single request. So the backend vtable > probably also needs a LoseCurrent hook. (I tried to hide this detail in > the frontend via ReplyCallback, and I was not very pleased with myself > afterwards.) > > SwapBuffers has to dispatch based on the drawable, not the context tag, > because the tag may be None. Thanks SGI. > > GLX requests with opcode >= 101 are "single" requests corresponding to > GL API calls that don't fit in a Render request; they are dispatched > relative to a context tag, so the backend vfunc can simply be int > (*Single)(ClientPtr client). Requests from 1 to 35 are GLX API calls, > and have a mostly static dispatch setup. The exceptions are > VendorPrivate and VendorPrivateWithReply. For these I imagine the > backend will register a list of { vop, object, offset, error } tuples > for each request it supports. > > VendorPrivate requests are also tricky because, being GLX API, they can > also create and destroy objects. The shadow resource trick (above) > makes destruction lifetime simple, but the frontend will need to expose > methods to allow e.g. glXCreateGLXPbufferSGIX to register its XID. (In > principle GLX extensions could also create named objects on which one > should dispatch that are not XIDs. I'm not aware of any extensions like > that at the moment, and I kind of want not to support anyone making > that mistake.) > > In principle, the above is enough in a Zaphod Xorg configuration to > have heterogeneous GLX providers on different ScreenRecs. It would also > make it easy, in a single ScreenRec case, to determine the GLX provider > based on the DDX driver in use (e.g., I updated my kernel but not my > NVIDIA driver, nouveau wants Xorg's GLX not NVIDIA's). For homogeneous > Xinerama-enabled GLX this should not lose any functionality, since the > backend can simply be the same for all objects. > > Open questions: > > - What more is needed for Prime setups? For direct contexts, I don't > think much. For indirect contexts presumably you'd want a way to > communicate the desired GPU when you create the GLX drawable and have > that determine the screen you dispatch to. > > - What more is needed for Xinerama+GLX with the open driver stack? We > have some flexibility here, GLX_EXT_glvnd lets us name whatever client- > side library we want, so we can ask for a new Mesa target that knows > how to be multi-GPU aware. > > - What more is needed for credible GLX on non-xfree86 servers, in > particular Xwayland? I think, if the pig is given enough thrust, it > would be possible to build accelerated GLX atop just about arbitrary > EGL stacks; in that sense it'd be cool if Xwayland could do reasonable > amounts of GLX accel with the stock backend. But I don't think that's > going to get feature or performance parity with xfree86 on its own, if > nothing else the open stack doesn't have stereo support. > > - What else am I missing? > > [1] - The GLX spec describes context tags as _not_ being the same as > the context XID, with the explanation that the context may be destroyed > but still be current to a client. Yes, and? You can prevent the client > from reusing the XID by simply not freeing it, the
Re: twm deadlocks the server
On 07/03/2017 10:41 PM, Antoine Martin wrote: Hi, I've just come across this easy DoS with twm and X.Org X Server 1.19.3 from Fedora 26. Steps: /usr/libexec/Xorg -noreset -novtswitch -config /etc/xpra/xorg.conf :10& #verify we can access the display: DISPLAY=:10 xprop -root #start xterm so we have a window, then twm: DISPLAY=:10 xterm& DISPLAY=:10 twm& #now click on the title bar of the xterm: DISPLAY=:10 xdotool mousemove 90 10 mousedown 1 #and now the X11 server is inaccessible: DISPLAY=:10 xprop -root Isn't this just twm grabbing the server? If you can connect to the server to launch twm, you can also just call XGrabServer() and not let go. Cheers Antoine PS: the xorg.conf to use for the dummy driver can be found here: http://xpra.org/trac/browser/xpra/tags/v2.0.x/src/etc/xpra/xorg.conf ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xrandr v2 1/2] xrandr: allow a single value for --scale
On 06/22/2017 03:39 PM, Giuseppe Bilotta wrote: This allows using e.g. --scale 0.5 as a shorthand for --scale 0.5x0.5 Signed-off-by: Giuseppe Bilotta--- man/xrandr.man | 7 --- xrandr.c | 8 ++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/man/xrandr.man b/man/xrandr.man index 65ccc2a..e59abbe 100644 --- a/man/xrandr.man +++ b/man/xrandr.man @@ -34,7 +34,7 @@ xrandr \- primitive command line interface to RandR extension [\-\-current] [\-\-noprimary] [\-\-panning \fIwidth\fPx\fIheight\fP[+\fIx\fP+\fIy\fP[/\fItrack_width\fPx\fItrack_height\fP+\fItrack_x\fP+\fItrack_y\fP[/\fIborder_left\fP/\fIborder_top\fP/\fIborder_right\fP/\fIborder_bottom\fP -[\-\-scale \fIx\fPx\fIy\fP] +[\-\-scale \fIx\fP[x\fIy\fP]] [\-\-scale-from \fIw\fPx\fIh\fP] [\-\-transform \fIa\fP,\fIb\fP,\fIc\fP,\fId\fP,\fIe\fP,\fIf\fP,\fIg\fP,\fIh\fP,\fIi\fP] [\-\-primary] @@ -207,8 +207,9 @@ values are used (a unit matrix without filter). Chooses the scaling filter method to be applied when the screen is scaled or transformed. Can be either 'bilinear' or 'nearest'. -.IP "\-\-scale \fIx\fPx\fIy\fP" -Changes the dimensions of the output picture. Values superior to 1 will lead to +.IP "\-\-scale \fIx\fP[x\fIy\fP]" +Changes the dimensions of the output picture. If the \fIy\fP value is omitted, +the \fIx\fP value will be used for both dimensions. Values superior to 1 will lead to I think "greater than" or "larger than" are more common than "superior to" for numerical comparisons, and new sentences in roff format are supposed to start on their own lines. I can send a separate change to fix those if you don't feel like fixing them as part of this change. a compressed screen (screen dimension bigger than the dimension of the output mode), and values below 1 leads to a zoom in on the output. This option is actually a shortcut version of the \fI\-\-transform\fP option. diff --git a/xrandr.c b/xrandr.c index 2d4cb72..4433724 100644 --- a/xrandr.c +++ b/xrandr.c @@ -137,7 +137,7 @@ usage(void) " --below \n" " --same-as \n" " --set \n" - " --scale x\n" + " --scale [x]\n" " --scale-from x\n" " --transform \n" " --filter nearest,bilinear\n" @@ -3017,7 +3017,11 @@ main (int argc, char **argv) if (!config_output) argerr ("%s must be used after --output\n", argv[i]); if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]); if (sscanf (argv[i], "%lfx%lf", , ) != 2) - argerr ("failed to parse '%s' as a scaling factor\n", argv[i]); + { + if (sscanf (argv[i], "%lf", ) != 1) This looks like it's indented too far. Should be two tabs and no spaces, and the next line should be two tabs and four spaces. Yes, this is terrible and I hate it. ;) Otherwise, this seems fine to me. + argerr ("failed to parse '%s' as a scaling factor\n", argv[i]); + sy = sx; + } init_transform (_output->transform); config_output->transform.transform.matrix[0][0] = XDoubleToFixed (sx); config_output->transform.transform.matrix[1][1] = XDoubleToFixed (sy); ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] xfree86/modes: Use RRTransformEqual in xf86RandR12CrtcSet
On 06/19/2017 01:57 AM, Michel Dänzer wrote: On 16/06/17 03:25 PM, Aaron Plattner wrote: On 06/15/2017 07:31 PM, Michel Dänzer wrote: From: Michel Dänzer <michel.daen...@amd.com> The memcmp didn't catch when e.g. only the filter changed. Tested by alternately running xrandr --output DVI-I-0 --scale-from 3840x2160 --filter bilinear xrandr --output DVI-I-0 --scale-from 3840x2160 --filter nearest Signed-off-by: Michel Dänzer <michel.daen...@amd.com> --- hw/xfree86/modes/xf86RandR12.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c index 55d88e331..fe8770d61 100644 --- a/hw/xfree86/modes/xf86RandR12.c +++ b/hw/xfree86/modes/xf86RandR12.c @@ -1170,8 +1170,7 @@ xf86RandR12CrtcSet(ScreenPtr pScreen, if ((transform != NULL) != crtc->transformPresent) changed = TRUE; else if (transform && RRTransformEqual treats NULL transform pointers as the identity, so I think you can just drop the transform && check here. Thanks for the suggestion. Unfortunately, AFAICT xf86CrtcSetModeTransform doesn't reset crtc->transform to the identity when crtc->transformPresent = FALSE, so skipping the transform && would lead to false positives in that case and transform == NULL. Hmm, okay. In that case, Reviewed-by: Aaron Plattner <aplatt...@nvidia.com> -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] xfree86/modes: Use RRTransformEqual in xf86RandR12CrtcSet
On 06/15/2017 07:31 PM, Michel Dänzer wrote: From: Michel DänzerThe memcmp didn't catch when e.g. only the filter changed. Tested by alternately running xrandr --output DVI-I-0 --scale-from 3840x2160 --filter bilinear xrandr --output DVI-I-0 --scale-from 3840x2160 --filter nearest Signed-off-by: Michel Dänzer --- hw/xfree86/modes/xf86RandR12.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c index 55d88e331..fe8770d61 100644 --- a/hw/xfree86/modes/xf86RandR12.c +++ b/hw/xfree86/modes/xf86RandR12.c @@ -1170,8 +1170,7 @@ xf86RandR12CrtcSet(ScreenPtr pScreen, if ((transform != NULL) != crtc->transformPresent) changed = TRUE; else if (transform && RRTransformEqual treats NULL transform pointers as the identity, so I think you can just drop the transform && check here. - memcmp(>transform, >transform.transform, -sizeof(transform->transform)) != 0) + !RRTransformEqual(transform, >transform)) changed = TRUE; if (x != crtc->x || y != crtc->y) -- nvpublic ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] randr: Use RRTransformEqual in RRCrtcPendingTransform
Currently, RRCrtcPendingTransform returns false unless the transformation matrix itself is changing. This makes RRCrtcSet skip doing anything if the only thing that is changing is the transform filter. There's already a function for comparing RRTransformPtrs, so use that instead. Tested by running xrandr --output DP-1 --mode 1920x1080 --rate 144 --scale 0.5x0.5 --filter nearest follwed by xrandr --output DP-1 --mode 1920x1080 --rate 144 --scale 0.5x0.5 --filter bilinear Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- randr/rrcrtc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c index d1a51f0aa341..401a1c178b64 100644 --- a/randr/rrcrtc.c +++ b/randr/rrcrtc.c @@ -843,9 +843,8 @@ RRCrtcGetTransform(RRCrtcPtr crtc) Bool RRCrtcPendingTransform(RRCrtcPtr crtc) { -return memcmp(>client_current_transform.transform, - >client_pending_transform.transform, - sizeof(PictTransform)) != 0; +return !RRTransformEqual(>client_current_transform, + >client_pending_transform); } /* -- 2.13.1 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 3/2 xserver] travis: Enable ccache.
On 06/01/2017 01:59 PM, Eric Anholt wrote: We bind-mount the cache directory into the container. Cuts build time from about 4 minutes to 2. Signed-off-by: Eric Anholt--- .travis.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index e10cfe1e134f..0a136b23dd80 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,5 @@ language: c +cache: ccache dist: trusty services: docker @@ -6,9 +7,9 @@ before_install: - if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then docker pull anholt/xserver-travis ; fi before_script: - - if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then echo FROM anholt/xserver-travis:v3 > Dockerfile ; fi + - if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then echo FROM anholt/xserver-travis:v4 > Dockerfile ; fi - if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then echo ADD . /root >> Dockerfile ; fi - if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then docker build -t withgit . ; fi script: - - if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then docker run withgit /bin/sh -c "cd /root && ./test/scripts/travis-test.sh" ; fi + - if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then docker run --volume $HOME/.ccache:/root/.ccache withgit /bin/sh -c "cd /root && ./test/scripts/travis-test.sh" ; fi This doesn't apply cleanly because the previous patch names it build-travis-deps.sh rather than travis-test.sh. But with that fixed, and running the commands manually, this series seems to work. With ccache, this took a whopping 17s on my medium-speed non-SSD workstation. ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH app/xrandr] man: Document the new --filter option
Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- I noticed elsewhere that in PDF versions of the man pages, \- expands as U+2212 MINUS SIGN and - expands to U+002D HYPHEN-MINUS. So I think our use of - and \- is backwards. I didn't try to fix that here. man/xrandr.man | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/man/xrandr.man b/man/xrandr.man index 55ea5dd9b6f4..65ccc2a2f9c6 100644 --- a/man/xrandr.man +++ b/man/xrandr.man @@ -155,7 +155,9 @@ parameters specify the border and default to 0. A width or height set to zero disables panning on the according axis. You typically have to set the screen size with \fI--fb\fP simultaneously. .IP "\-\-transform \fIa\fP,\fIb\fP,\fIc\fP,\fId\fP,\fIe\fP,\fIf\fP,\fIg\fP,\fIh\fP,\fIi\fP" -Specifies a transformation matrix to apply on the output. Automatically a bilinear filter is selected. +Specifies a transformation matrix to apply on the output. +A bilinear filter is selected automatically unless the \-\-filter parameter is +also specified. The mathematical form corresponds to: .RS .RS @@ -201,6 +203,10 @@ As a special argument, instead of passing a matrix, one can pass the string \fInone\fP, in which case the default values are used (a unit matrix without filter). .RE +.IP "\-\-filter \fIfiltermode\fP" +Chooses the scaling filter method to be applied when the screen is scaled or +transformed. +Can be either 'bilinear' or 'nearest'. .IP "\-\-scale \fIx\fPx\fIy\fP" Changes the dimensions of the output picture. Values superior to 1 will lead to a compressed screen (screen dimension bigger than the dimension of the output -- 2.13.0 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xrandr] Adding a "--filter" flag
I fixed up a few things from this and pushed it. Can you please update https://patchwork.freedesktop.org/patch/146123/ ? remote: Updating patchwork state for https://patchwork.freedesktop.org/project/Xorg/list/ remote: E: failed to find patch for rev 6ac2afc0d7d8d51d4085767b901667393c11061b. remote: I: 0 patch(es) updated to state Accepted. To git.freedesktop.org:/git/xorg/app/xrandr 5d5db88d106a..6ac2afc0d7d8 master -> master On 03/23/2017 04:05 PM, Pablo De La Garza wrote: > From: pdelagarza> > Flag can be set to "nearest" or "bilinear" > > Signed-off-by: Pablo De La Garza > --- > xrandr.c | 35 +++ > 1 file changed, 35 insertions(+) > > diff --git a/xrandr.c b/xrandr.c > index dcfdde0..c2cc82e 100644 > --- a/xrandr.c > +++ b/xrandr.c > @@ -54,6 +54,12 @@ static Boolautomatic = False; > static Bool properties = False; > static Bool grab_server = True; > static Bool no_primary = False; > +static int filterType = -1; > + > +static const char *filterTypeList[2] = { > +"bilinear", > +"nearest"}; > + > > static const char *direction[5] = { > "normal", > @@ -118,6 +124,7 @@ usage(void) > " --fb x\n" > " --fbmm x\n" > " --dpi /\n" > + " --filter ,filterType: nearest, bilinear\n" > " --output \n" > " --auto\n" > " --mode \n" > @@ -285,6 +292,7 @@ typedef enum _changes { > changes_panning = (1 << 10), > changes_gamma = (1 << 11), > changes_primary = (1 << 12), > +changes_filter = (1 << 13), > } changes_t; > > typedef enum _name_kind { > @@ -1311,6 +1319,11 @@ set_output_info (output_t *output, RROutput xid, > XRROutputInfo *output_info) > output->transform.params = NULL; > } > } > +if (output->changes & changes_filter) > +{ > +output->transform.filter = filterTypeList[filterType]; > +} > + > > /* set primary */ > if (!(output->changes & changes_primary)) > @@ -2808,6 +2821,28 @@ main (int argc, char **argv) > action_requested = True; > continue; > } > + > +if (!strcmp("--filter", argv[i])) > +{ > +if (!config_output) argerr ("%s must be used after --output\n", > argv[i]); > +if (++i >= argc) argerr("%s requires an argument\n", argv[i-1]); > + > +for (int t=0;t < > sizeof(filterTypeList)/sizeof(filterTypeList[0]);t++) > +{ > +if (!strcmp(filterTypeList[t],argv[i])) > +{ > +filterType = t; > +break; > +} > +} > + > +if (filterType==-1) argerr("Bad argument: %s, for a filter\n", > argv[i]); > + > +config_output->changes |= changes_filter; > +action_requested = True; > +continue; > +} > + > if (!strcmp ("--crtc", argv[i])) { > if (!config_output) argerr ("%s must be used after --output\n", > argv[i]); > if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]); > ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] xfree86: fix autoConfigDevice() regression that skipped all but the first driver match
On 05/09/2017 04:51 PM, Eric Anholt wrote: Aaron Plattner <aplatt...@nvidia.com> writes: Commit 112d0d7d01b9 lost the initialization of the variable i in the loop to add secondary driver matches to the list of configs: @@ -398,8 +412,8 @@ autoConfigDevice(GDevPtr preconf_device) /* for each other driver found, copy the first screen, insert it * into the list of screens and set the driver */ -for (i = 1; i < num_matches; i++) { -if (!copyScreen(slp[0].screen, ptr, i, matches[i])) +while (i++ < md.nmatches) { +if (!copyScreen(slp[0].screen, ptr, i, md.matches[i])) return NULL; } This caused only the first match to be added, because an earlier loop sets i = md.nmatches. Fix this by reverting the while loop back to a for loop. Reported-by: Michel Dänzer <mic...@daenzer.net> Reported-by: Peter Hutterer <peter.hutte...@who-t.net> Reported-by: Eric Anholt <e...@anholt.net> Cc: Adam Jackson <a...@redhat.com> Fixes: 112d0d7d01b9 ("xfree86: Improved autoconfig drivers matching") Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- Resend: forgot to Cc the list. This is a pretty clear unintended behavior change. Tested-by: Eric Anholt <e...@anholt.net> Since we collided on me pushing the revert, shall I squash this into a re-cherry-pick of the change as a v2? Sounds good to me. -- Aaron nvpublic ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] xfree86: fix autoConfigDevice() regression that skipped all but the first driver match
Commit 112d0d7d01b9 lost the initialization of the variable i in the loop to add secondary driver matches to the list of configs: @@ -398,8 +412,8 @@ autoConfigDevice(GDevPtr preconf_device) /* for each other driver found, copy the first screen, insert it * into the list of screens and set the driver */ -for (i = 1; i < num_matches; i++) { -if (!copyScreen(slp[0].screen, ptr, i, matches[i])) +while (i++ < md.nmatches) { +if (!copyScreen(slp[0].screen, ptr, i, md.matches[i])) return NULL; } This caused only the first match to be added, because an earlier loop sets i = md.nmatches. Fix this by reverting the while loop back to a for loop. Reported-by: Michel Dänzer <mic...@daenzer.net> Reported-by: Peter Hutterer <peter.hutte...@who-t.net> Reported-by: Eric Anholt <e...@anholt.net> Cc: Adam Jackson <a...@redhat.com> Fixes: 112d0d7d01b9 ("xfree86: Improved autoconfig drivers matching") Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- Resend: forgot to Cc the list. hw/xfree86/common/xf86AutoConfig.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/xfree86/common/xf86AutoConfig.c b/hw/xfree86/common/xf86AutoConfig.c index a32c108ff2de..0f4f05de99e0 100644 --- a/hw/xfree86/common/xf86AutoConfig.c +++ b/hw/xfree86/common/xf86AutoConfig.c @@ -412,7 +412,7 @@ autoConfigDevice(GDevPtr preconf_device) /* for each other driver found, copy the first screen, insert it * into the list of screens and set the driver */ -while (i++ < md.nmatches) { +for (i = 1; i < md.nmatches; i++) { if (!copyScreen(slp[0].screen, ptr, i, md.matches[i])) return NULL; } -- 2.12.2 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Improved autoconfig drivers matching
On 05/03/2017 01:11 PM, Adam Jackson wrote: > On Mon, 2017-05-01 at 18:03 +0900, Michel Dänzer wrote: >> On 28/04/17 04:11 AM, Adam Jackson wrote: >>> Fixed up (and rebased and made meson-aware) and merged: >>> >>> To ssh://git.freedesktop.org/git/xorg/xserver >>>1549e3037..112d0d7d0 master -> master >> >> This broke things on my development box. >> >> There are two GPUs. When I want Xorg to use the secondary GPU for its >> primary screen, I use a xorg.conf with (effectively) only: >> >> Section "Device" >> Identifier "Device0" >> BusID "PCI:1:0:0" >> EndSection >> >> Before this change, this worked fine, automatically[0] using the amdgpu >> driver for that GPU. With this change, Xorg fails to start up, see the >> attached log file. > > Ugh. I don't know if I'm going to have time to investigate that in > detail, so I lean towards reverting. If the original problem was that > 20 was too small a number let's just bump it to something safely crazy > like 256. Sound reasonable? I found the problem. Karol's change lost the initialization of i here: @@ -398,8 +412,8 @@ autoConfigDevice(GDevPtr preconf_device) /* for each other driver found, copy the first screen, insert it * into the list of screens and set the driver */ -for (i = 1; i < num_matches; i++) { -if (!copyScreen(slp[0].screen, ptr, i, matches[i])) +while (i++ < md.nmatches) { +if (!copyScreen(slp[0].screen, ptr, i, md.matches[i])) return NULL; } Patch incoming. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Improved autoconfig drivers matching
On 05/03/2017 01:11 PM, Adam Jackson wrote: > On Mon, 2017-05-01 at 18:03 +0900, Michel Dänzer wrote: >> On 28/04/17 04:11 AM, Adam Jackson wrote: >>> Fixed up (and rebased and made meson-aware) and merged: >>> >>> To ssh://git.freedesktop.org/git/xorg/xserver >>>1549e3037..112d0d7d0 master -> master >> >> This broke things on my development box. >> >> There are two GPUs. When I want Xorg to use the secondary GPU for its >> primary screen, I use a xorg.conf with (effectively) only: >> >> Section "Device" >> Identifier "Device0" >> BusID "PCI:1:0:0" >> EndSection >> >> Before this change, this worked fine, automatically[0] using the amdgpu >> driver for that GPU. With this change, Xorg fails to start up, see the >> attached log file. Weird. Do you have a working log to compare against? > Ugh. I don't know if I'm going to have time to investigate that in > detail, so I lean towards reverting. If the original problem was that > 20 was too small a number let's just bump it to something safely crazy > like 256. Sound reasonable? Sad, but sure, fine with me. 16 drivers times 16 GPUs should be rare enough that it won't be a problem in practice. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 2/3] Use #ifdef instead of #if for features to make Meson easier.
On 04/26/2017 11:21 AM, Eric Anholt wrote: > Aaron Plattner <aplatt...@nvidia.com> writes: > >> We try to do exactly the opposite in our internal driver build, because >> it's too easy to accidentally do something like >> >> #ifdef GLAMOUR_HAS_GBM >> >> And mistakes like that don't always cause obvious build failures like >> this would. So we build everything with -Wundef -Werror=undef and try to >> use #if whenever possible. It's a shame that Meson makes that hard. > > Meson makes what you want easy with: > > set10(varname, boolean_value) is the same as above but the value is > either true or false and will be written as 1 or 0, respectively > > but autotools usually produces either #define 1 or #undef, unless you go > through contortions. This seemed like the easy, consistent solution > today, given that our codebase is almost all #ifdef. One alternative > would be to just disable -Wundef for now. The other alternative to this > patch would be something like: > > conf_array += ['HAVE_DBM_H', cc.has_header('dbm.h')] > > then finish with something like: > > foreach conf: conf_array > if conf[1] > conf_data.set(conf[0], '1') > endif > endforeach > > but I think I'd rather wait and consistently swap to .set10() and #if > once we've nuked autotools. Waiting until autotools is nuked sounds fine to me. I just wanted to throw the idea of using -Wundef out there. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 2/3] Use #ifdef instead of #if for features to make Meson easier.
We try to do exactly the opposite in our internal driver build, because it's too easy to accidentally do something like #ifdef GLAMOUR_HAS_GBM And mistakes like that don't always cause obvious build failures like this would. So we build everything with -Wundef -Werror=undef and try to use #if whenever possible. It's a shame that Meson makes that hard. On 04/25/2017 04:03 PM, Eric Anholt wrote: > We mostly use #ifdef throughout the tree, and this lets the generated > config.h files just be #define TOKEN instead of #define TOKEN 1. > > Signed-off-by: Eric Anholt> --- > glamor/glamor_priv.h| 4 ++-- > hw/xfree86/common/xf86.h| 2 +- > hw/xfree86/drivers/modesetting/driver.c | 16 > hw/xfree86/loader/loadmod.c | 4 ++-- > hw/xfree86/sdksyms.sh | 8 > hw/xwayland/xwayland.c | 2 +- > include/dixstruct.h | 2 +- > include/os.h| 2 +- > include/xserver_poll.h | 2 +- > os/utils.c | 16 > 10 files changed, 29 insertions(+), 29 deletions(-) > > diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h > index 7b92f35705f4..a90879a7bbfd 100644 > --- a/glamor/glamor_priv.h > +++ b/glamor/glamor_priv.h > @@ -38,7 +38,7 @@ > #endif > > #include > -#if GLAMOR_HAS_GBM > +#ifdef GLAMOR_HAS_GBM > #define MESA_EGL_NO_X11_HEADERS > #include > #endif > @@ -342,7 +342,7 @@ typedef struct glamor_pixmap_private { > GLuint pbo; > RegionRec prepare_region; > Bool prepared; > -#if GLAMOR_HAS_GBM > +#ifdef GLAMOR_HAS_GBM > EGLImageKHR image; > #endif > /** block width of this large pixmap. */ > diff --git a/hw/xfree86/common/xf86.h b/hw/xfree86/common/xf86.h > index f1a5bd6741bd..877b9e9e768e 100644 > --- a/hw/xfree86/common/xf86.h > +++ b/hw/xfree86/common/xf86.h > @@ -35,7 +35,7 @@ > #ifndef _XF86_H > #define _XF86_H > > -#if HAVE_XORG_CONFIG_H > +#ifdef HAVE_XORG_CONFIG_H > #include > #elif HAVE_DIX_CONFIG_H > #include > diff --git a/hw/xfree86/drivers/modesetting/driver.c > b/hw/xfree86/drivers/modesetting/driver.c > index d7030e5c2117..a1451fe471e8 100644 > --- a/hw/xfree86/drivers/modesetting/driver.c > +++ b/hw/xfree86/drivers/modesetting/driver.c > @@ -55,7 +55,7 @@ > #ifdef XSERVER_PLATFORM_BUS > #include "xf86platformBus.h" > #endif > -#if XSERVER_LIBPCIACCESS > +#ifdef XSERVER_LIBPCIACCESS > #include > #endif > > @@ -227,7 +227,7 @@ check_outputs(int fd, int *count) > *count = res->count_connectors; > > ret = res->count_connectors > 0; > -#if defined DRM_CAP_PRIME && GLAMOR_HAS_GBM_LINEAR > +#if defined(DRM_CAP_PRIME) && defined(GLAMOR_HAS_GBM_LINEAR) > if (ret == FALSE) { > uint64_t value = 0; > if (drmGetCap(fd, DRM_CAP_PRIME, ) == 0 && > @@ -244,7 +244,7 @@ probe_hw(const char *dev, struct xf86_platform_device > *platform_dev) > { > int fd; > > -#if XF86_PDEV_SERVER_FD > +#ifdef XF86_PDEV_SERVER_FD > if (platform_dev && (platform_dev->flags & XF86_PDEV_SERVER_FD)) { > fd = xf86_platform_device_odev_attributes(platform_dev)->fd; > if (fd == -1) > @@ -366,7 +366,7 @@ ms_setup_entity(ScrnInfoPtr scrn, int entity_num) > pPriv->ptr = xnfcalloc(sizeof(modesettingEntRec), 1); > } > > -#if XSERVER_LIBPCIACCESS > +#ifdef XSERVER_LIBPCIACCESS > static Bool > ms_pci_probe(DriverPtr driver, > int entity_num, struct pci_device *dev, intptr_t match_data) > @@ -826,7 +826,7 @@ ms_get_drm_master_fd(ScrnInfoPtr pScrn) > return TRUE; > } > > -#if XSERVER_PLATFORM_BUS > +#ifdef XSERVER_PLATFORM_BUS > if (pEnt->location.type == BUS_PLATFORM) { > #ifdef XF86_PDEV_SERVER_FD > if (pEnt->location.id.plat->flags & XF86_PDEV_SERVER_FD) > @@ -844,7 +844,7 @@ ms_get_drm_master_fd(ScrnInfoPtr pScrn) > } > else > #endif > -#if XSERVER_LIBPCIACCESS > +#ifdef XSERVER_LIBPCIACCESS > if (pEnt->location.type == BUS_PCI) { > char *BusID = NULL; > struct pci_device *PciInfo; > @@ -1018,7 +1018,7 @@ PreInit(ScrnInfoPtr pScrn, int flags) > if (ms->drmmode.glamor) > pScrn->capabilities |= RR_Capability_SinkOffload; > } > -#if GLAMOR_HAS_GBM_LINEAR > +#ifdef GLAMOR_HAS_GBM_LINEAR > if (value & DRM_PRIME_CAP_EXPORT && ms->drmmode.glamor) > pScrn->capabilities |= RR_Capability_SourceOutput | > RR_Capability_SourceOffload; > #endif > @@ -1189,7 +1189,7 @@ msEnableSharedPixmapFlipping(RRCrtcPtr crtc, PixmapPtr > front, PixmapPtr back) > if (ms->drmmode.reverse_prime_offload_mode) > return FALSE; > > -#if XSERVER_PLATFORM_BUS > +#ifdef XSERVER_PLATFORM_BUS > if (pEnt->location.type == BUS_PLATFORM) { > char *syspath = > xf86_platform_device_odev_attributes(pEnt->location.id.plat)-> > diff --git
Re: [PATCH] Improved autoconfig drivers matching
Another customer ran into this recently. Adam, can this be merged? I don't think Emil's reply was a nack. On 07/12/2016 04:31 PM, Emil Velikov wrote: On 23 July 2015 at 00:42, Karol Kosikwrote: Implementation of new drivers matching algorithm. New approach doesn't add duplicate drivers and ease drivers matching phase. Signed-off-by: Karol Kosik --- hw/xfree86/common/xf86AutoConfig.c | 124 +++ hw/xfree86/common/xf86MatchDrivers.h | 40 +++ hw/xfree86/common/xf86pciBus.c | 52 ++- hw/xfree86/common/xf86pciBus.h | 13 ++-- hw/xfree86/common/xf86platformBus.c | 31 +++-- hw/xfree86/common/xf86platformBus.h | 5 +- 6 files changed, 150 insertions(+), 115 deletions(-) create mode 100644 hw/xfree86/common/xf86MatchDrivers.h diff --git a/hw/xfree86/common/xf86AutoConfig.c b/hw/xfree86/common/xf86AutoConfig.c index 6b8d0eb..440434c 100644 --- a/hw/xfree86/common/xf86AutoConfig.c +++ b/hw/xfree86/common/xf86AutoConfig.c @@ -37,6 +37,7 @@ #include "xf86Parser.h" #include "xf86tokens.h" #include "xf86Config.h" +#include "xf86MatchDrivers.h" #include "xf86Priv.h" #include "xf86_OSlib.h" #include "xf86platformBus.h" @@ -89,7 +90,7 @@ static const char **builtinConfig = NULL; static int builtinLines = 0; -static void listPossibleVideoDrivers(char *matches[], int nmatches); +static void listPossibleVideoDrivers(XF86MatchedDrivers *md); /* * A built-in config file is stored as an array of strings, with each string @@ -140,33 +141,58 @@ AppendToConfig(const char *s) AppendToList(s, , ); } +void +xf86AddMatchedDriver(XF86MatchedDrivers *md, const char *driver) +{ +int j; +int nmatches = md->nmatches; + +for (j = 0; j < nmatches; ++j) { +if (xf86NameCmp(md->matches[j], driver) == 0) { +// Driver already in matched drivers +return; +} +} + +if (nmatches < MATCH_DRIVERS_LIMIT) { +md->matches[nmatches] = xnfstrdup(driver); +md->nmatches++; +} +else { +xf86Msg(X_WARNING, "Too many drivers registered, can't add %s\n", driver); +} +} + Bool xf86AutoConfig(void) { -char *deviceList[20]; -char **p; +XF86MatchedDrivers md; +int i; const char **cp; char buf[1024]; ConfigStatus ret; -listPossibleVideoDrivers(deviceList, 20); +listPossibleVideoDrivers(); -for (p = deviceList; *p; p++) { -snprintf(buf, sizeof(buf), BUILTIN_DEVICE_SECTION, *p, 0, *p); +for (i = 0; i < md.nmatches; i++) { +snprintf(buf, sizeof(buf), BUILTIN_DEVICE_SECTION, +md.matches[i], 0, md.matches[i]); AppendToConfig(buf); -snprintf(buf, sizeof(buf), BUILTIN_SCREEN_SECTION, *p, 0, *p, 0); +snprintf(buf, sizeof(buf), BUILTIN_SCREEN_SECTION, +md.matches[i], 0, md.matches[i], 0); AppendToConfig(buf); } AppendToConfig(BUILTIN_LAYOUT_SECTION_PRE); -for (p = deviceList; *p; p++) { -snprintf(buf, sizeof(buf), BUILTIN_LAYOUT_SCREEN_LINE, *p, 0); +for (i = 0; i < md.nmatches; i++) { +snprintf(buf, sizeof(buf), BUILTIN_LAYOUT_SCREEN_LINE, +md.matches[i], 0); AppendToConfig(buf); } AppendToConfig(BUILTIN_LAYOUT_SECTION_POST); -for (p = deviceList; *p; p++) { -free(*p); +for (i = 0; i < md.nmatches; i++) { +free(md.matches[i]); } xf86MsgVerb(X_DEFAULT, 0, @@ -190,22 +216,19 @@ xf86AutoConfig(void) } static void -listPossibleVideoDrivers(char *matches[], int nmatches) +listPossibleVideoDrivers(XF86MatchedDrivers *md) { int i; -for (i = 0; i < nmatches; i++) { -matches[i] = NULL; -} -i = 0; +md->nmatches = 0; #ifdef XSERVER_PLATFORM_BUS -i = xf86PlatformMatchDriver(matches, nmatches); +xf86PlatformMatchDriver(md); #endif #ifdef sun /* Check for driver type based on /dev/fb type and if valid, use it instead of PCI bus probe results */ -if (xf86Info.consoleFd >= 0 && (i < (nmatches - 1))) { +if (xf86Info.consoleFd >= 0) { struct vis_identifier visid; const char *cp; int iret; @@ -231,7 +254,7 @@ listPossibleVideoDrivers(char *matches[], int nmatches) /* Special case from before the general case was set */ if (strcmp(visid.name, "NVDAnvda") == 0) { -matches[i++] = xnfstrdup("nvidia"); +xf86AddMatchedDriver(md, "nvidia"); } /* General case - split into vendor name (initial all-caps @@ -241,55 +264,48 @@ listPossibleVideoDrivers(char *matches[], int nmatches) /* find end of all uppercase vendor section */ } if ((cp != visid.name) && (*cp != '\0')) { -char *driverName = xnfstrdup(cp);
[PATCH rendercheck] Skip shmblend if SHM pixmaps aren't supported
Some drivers don't support SHM pixmaps, but rendercheck doesn't care and tries to use them anyway. This causes the test to abort: Beginning SHM blend test from a8 X Error of failed request: BadImplementation (server does not implement operation) Major opcode of failed request: 130 (MIT-SHM) Minor opcode of failed request: 5 (X_ShmCreatePixmap) Serial number of failed request: 805 Current serial number in output stream: 811 X Error of failed request: BadDrawable (invalid Pixmap or Window parameter) Major opcode of failed request: 139 (RENDER) Minor opcode of failed request: 4 (RenderCreatePicture) Resource id in failed request: 0x3200215 Serial number of failed request: 806 Current serial number in output stream: 811 Fix this by skipping the shmblend tests if the extension is missing or doesn't support pixmaps. Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- On a related note, BadImplementation seems like the wrong error for a client trying to create SHM pixmaps when they're not supported. t_shmblend.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/t_shmblend.c b/t_shmblend.c index 752e17a0f8a8..c717f91d7b9b 100644 --- a/t_shmblend.c +++ b/t_shmblend.c @@ -47,9 +47,6 @@ get_x_shm_info(Display *dpy, size_t size) { XShmSegmentInfo *shm_info = calloc(1, sizeof(*shm_info)); - if (!XShmQueryExtension(dpy)) - return NULL; - shm_info->shmid = shmget(IPC_PRIVATE, size, IPC_CREAT|0777); if (shm_info->shmid < 0) { free(shm_info); @@ -225,7 +222,16 @@ static struct rendercheck_test_result test_shmblend(Display *dpy) { struct rendercheck_test_result result = {}; - int i; + int major, minor, i; + Bool pixmaps_supported; + + if (!XShmQueryExtension(dpy) || + !XShmQueryVersion(dpy, , , _supported) || + !pixmaps_supported) { + printf("SHM blend test: skipped\n"); + record_result(, true); + return result; + } for (i = 0; i < nformats; i++) { struct render_format *format = [i]; -- 2.12.1 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] dix: Move {Change, Copy, Destroy}Clip from GCFuncs to Screen
On 03/07/2017 02:44 PM, Keith Packard wrote: > Aaron Plattner <aplatt...@nvidia.com> writes: > >> They don't do anything interesting with {Change, Copy, Destroy}Clip, though. > > Yeah, can we just get rid of these and replace them with calls to the mi > versions now? For the *Clip ones, yeah definitely. ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] dix: Move {Change, Copy, Destroy}Clip from GCFuncs to Screen
On 03/02/2017 03:35 PM, Adam Jackson wrote: > On Thu, 2017-03-02 at 13:20 -0800, Keith Packard wrote: >>> Adam Jacksonwrites: >>> The "call down" convention would need to know to scan ahead to find >>> the next non-null fptr for the slot either way, but that's not really >>> different from the current model where some slots can be null at the >>> bottom. >> >> Hence my hack of filling in the pointers between the slots which are in >> use; you just call the one below 'your' entry. > > That means you have to save "your" index for every slot though. Or rely > on fptr equality I suppose, but that's not really different since you > then have to write your own detangle code to find yourself in every > slot. If your layer index is constant (until someone tries to wrap > between you, which you can get notified of) then detangling yourself > means passing your index to a common routine that nulls your column of > the arrays for you. > >>> So... seems plausible. Maybe try the experiment with GCOps first before >>> blowing up the whole Screen? >> >> Seems reasonable. The hard part to me seems to be coming up with a >> complete list of 'layers', or at least complete enough for a start. I >> don't want to have to reallocate these on the fly... > > I mean, I did grep for '\ ' for a reason. Eight is enough. We have a few of these layers. They're mostly pretty trivial, but here be dragons. They don't do anything interesting with {Change, Copy, Destroy}Clip, though. > - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libX11] _XDefaultError: set XlibDisplayIOError flag before calling exit
On 02/21/2017 08:16 AM, Arthur Huillet wrote: > With Jamey's review and Julien's questions answered, would someone > please push that change to libX11, unless there are objections to the > patch? > > Thank you $ git push upstream HEAD:master Counting objects: 4, done. Delta compression using up to 24 threads. Compressing objects: 100% (4/4), done. Writing objects: 100% (4/4), 1.62 KiB | 0 bytes/s, done. Total 4 (delta 3), reused 0 (delta 0) remote: Updating patchwork state for https://patchwork.freedesktop.org/project/Xorg/list/ remote: I: patch #136280 updated using rev 2d20890e7ffd3ee88a9ceb25cdd2ac1fe7aaceb6. remote: I: 1 patch(es) updated to state Accepted. To git.freedesktop.org:/git/xorg/lib/libX11 42f4d7af9cf6..2d20890e7ffd HEAD -> master ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 4/7] xfree86: Remove Option "BiosBase"
On 02/14/2017 12:30 PM, Adam Jackson wrote: Just no. The ddxDesign chunk removes the whole para about xf86FixPciResource, since it turns out that function doesn't exist at all anymore. i128 and mga do use this slot, but they oughtn't. Signed-off-by: Adam Jackson--- hw/xfree86/common/xf86Config.c| 1 - hw/xfree86/common/xf86Configure.c | 1 - hw/xfree86/common/xf86str.h | 2 -- hw/xfree86/doc/ddxDesign.xml | 29 - hw/xfree86/man/xorg.conf.man | 6 -- hw/xfree86/parser/Device.c| 8 hw/xfree86/parser/xf86Parser.h| 1 - hw/xfree86/parser/xf86tokens.h| 1 - 8 files changed, 49 deletions(-) diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c index f03acf3..89861e0 100644 --- a/hw/xfree86/common/xf86Config.c +++ b/hw/xfree86/common/xf86Config.c @@ -2105,7 +2105,6 @@ configDevice(GDevPtr devicep, XF86ConfDevicePtr conf_device, Bool active, Bool g devicep->driver = conf_device->dev_driver; devicep->active = active; devicep->videoRam = conf_device->dev_videoram; -devicep->BiosBase = conf_device->dev_bios_base; devicep->MemBase = conf_device->dev_mem_base; devicep->IOBase = conf_device->dev_io_base; devicep->clockchip = conf_device->dev_clockchip; diff --git a/hw/xfree86/common/xf86Configure.c b/hw/xfree86/common/xf86Configure.c index f975b98..668a551 100644 --- a/hw/xfree86/common/xf86Configure.c +++ b/hw/xfree86/common/xf86Configure.c @@ -268,7 +268,6 @@ configureDeviceSection(int screennum) for (i = 0; i < MAXDACSPEEDS; i++) ptr->dev_dacSpeeds[i] = DevToConfig[screennum].GDev.dacSpeeds[i]; ptr->dev_videoram = DevToConfig[screennum].GDev.videoRam; -ptr->dev_bios_base = DevToConfig[screennum].GDev.BiosBase; ptr->dev_mem_base = DevToConfig[screennum].GDev.MemBase; ptr->dev_io_base = DevToConfig[screennum].GDev.IOBase; ptr->dev_clockchip = DevToConfig[screennum].GDev.clockchip; diff --git a/hw/xfree86/common/xf86str.h b/hw/xfree86/common/xf86str.h index 4df10a5..b060575 100644 --- a/hw/xfree86/common/xf86str.h +++ b/hw/xfree86/common/xf86str.h @@ -303,7 +303,6 @@ typedef struct { Bool active; Bool inUse; int videoRam; -unsigned long BiosBase; /* Base address of video BIOS */ unsigned long MemBase; /* Frame buffer base address */ unsigned long IOBase; int chipID; @@ -648,7 +647,6 @@ typedef struct _ScrnInfoRec { int numClocks; /* number of clocks */ int clock[MAXCLOCKS]; /* list of clock frequencies */ int videoRam; /* amount of video ram (kb) */ -unsigned long biosBase; /* Base address of video BIOS */ unsigned long memPhysBase; /* Physical address of FB */ unsigned long fbOffset; /* Offset of FB in the above */ int memClk; /* memory clock */ diff --git a/hw/xfree86/doc/ddxDesign.xml b/hw/xfree86/doc/ddxDesign.xml index df5f64b..0a14139 100644 --- a/hw/xfree86/doc/ddxDesign.xml +++ b/hw/xfree86/doc/ddxDesign.xml @@ -1208,7 +1208,6 @@ Here is what InitOutput() does: numClocks (if not programmable) clock[] (if not programmable) videoRam - biosBase memBase memClk driverPrivate @@ -2920,34 +2919,6 @@ Two functions are provided to obtain a resource range of a given type: -Some PCI devices are broken in the sense that they return invalid size -information for a certain resource. In this case the driver can supply -the correct size and make sure that the resource range allocated for -the card is large enough to hold the address range decoded by the card. -The function xf86FixPciResource() can be used to do this: - - -Bool xf86FixPciResource(int entityIndex, unsigned int prt, -CARD32 alignment, long type); - - - This function fixes a PCI resource allocation. The - prt parameter contains the number of the PCI base - register that needs to be fixed (0-5, and - 6 for the BIOS base register). The size is - specified by the alignment. Since PCI resources need to span an - integral range of size 2n, the alignm ent also - specifies the number of addresses that will be decoded. If the - driver specifies a type mask it can override the default type for - PCI resources which is ResShared. The resource - broker needs to know that to find a matching resource range. This - function should be called before calling - xf86RegisterResources(). The return value is - TRUE when the function succeeds. - - - - Bool xf86CheckPciMemBase(pciVideoPtr pPci, memType base); diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man index 00ebf56..25af3aa 100644 ---
Re: [PATCH 0/7] ScrnInfoRec cleanup
On 02/14/2017 01:28 PM, Alex Deucher wrote: On Tue, Feb 14, 2017 at 3:30 PM, Adam Jackson <a...@redhat.com> wrote: ScrnInfoRec is xfree86's subclass of ScreenRec. It's... showing its age. Much of it is only relevant to the pre-PCI world where probing the hardware was fraught with uncertainty and configuration overrides were common. This is a first cut at cleaning it up a bit, mostly dropping unused or under-used fields. A few of these changes (noted in the commit messages) will require minor driver fixes; I'll be happy to do those before merging the series. One big change I'd like to follow this with is removing fixed dotclock support, on the way to unifying the pre- and post-RANDR-1.2 setup paths. As far as I can tell this would only affect a few ancient mach64, trident, or chips (ahem) chips. Quite possibly all of them are already unsupported because they're not PCI. Series is: Reviewed-by: Alex Deucher <alexander.deuc...@amd.com> Also Reviewed-by: Aaron Plattner <aplatt...@nvidia.com> > The sis driver does inspect this > member, but hilariously does so only so it can print the same > information as the core does. Hilarious. common/xf86Config.c|1 - common/xf86Configure.c |1 - common/xf86Mode.c | 26 ++ common/xf86str.h | 42 -- doc/ddxDesign.xml | 48 man/xorg.conf.man |6 -- parser/Device.c|8 parser/xf86Parser.h|1 - parser/xf86tokens.h|1 - 9 files changed, 10 insertions(+), 124 deletions(-) - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 11/19] xfree86: Remove DriverRec1 compat struct
On 01/24/2017 04:36 PM, Emil Velikov wrote: On 23 January 2017 at 19:32, Adam Jacksonwrote: The idea here is that the driver might have once been old enough to not have the driverFunc slot in DriverRec, with the module ABI not having changed when it was added. That was ages ago, and drivers always declare themselves with DriverRec not DriverRec1, so uninitialized slots will simply be zero. Signed-off-by: Adam Jackson --- hw/xfree86/common/xf86Helper.c | 9 + hw/xfree86/common/xf86str.h| 10 -- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c index f48af75..b464864 100644 --- a/hw/xfree86/common/xf86Helper.c +++ b/hw/xfree86/common/xf86Helper.c @@ -79,14 +79,7 @@ xf86AddDriver(DriverPtr driver, void *module, int flags) xf86DriverList = xnfreallocarray(xf86DriverList, xf86NumDrivers, sizeof(DriverPtr)); Unrelated question: This is a rather common pattern - foo++; reallocarray(.. foo...) Wondering if starting with a base (say 8 ?) and doubling as the limit is reached isn't a more common/practical solution. Or there's (has been?) memory constrained environments where that was a bad idea ? Then you need code to count and conditionally realloc. This is not a performance-critical path, so just reallocing every time seems simpler to me, especially when you can paper over error handling with the xnf* functions. xf86DriverList[xf86NumDrivers - 1] = xnfalloc(sizeof(DriverRec)); -if (flags & HaveDriverFuncs) I was purging through this as well (as mentioned in the original series) and noticed that some drivers do not use/set the HaveDriverFuncs flag. Although looking at it now, we should be safe. Reviewed-by: Emil Velikov -Emil ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 02/19] xfree86: remove unused path from the LoadModule API
On 01/23/2017 01:27 PM, Eric Anholt wrote: > Adam Jackson <a...@redhat.com> writes: > >> From: Emil Velikov <emil.l.veli...@gmail.com> >> >> Similar to its little brothre - LoadSubModule. Currently all call sites >> provide NULL anyway ;-) > > "brother". Other than that, > > Reviewed-by: Eric Anholt <e...@anholt.net> Reviewed-by: Aaron Plattner <aplatt...@nvidia.com> We don't use LoadModule in our driver, and if we do start loading extra stuff, it'll probably be with LoadSubModule anyway. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [ANNOUNCE] xorg-server 1.18.99.902
On 10/28/2016 09:50 AM, Keith Packard wrote: > Here's 1.19-RC2, which should be pretty close to 1.19. Hi Keith, Can I consider the ABI frozen at this point, so we can get the ball rolling on official driver support? -- Aaron > The only serious bug I'm aware of at this point is a crash in > FlushAllOutput: > > https://bugzilla.redhat.com/show_bug.cgi?id=1382444 > > Adam Jackson (5): > glamor: Fall back to software for CopyPlane if we need to > xephyr: Don't crash if the server advertises zero xv adaptors > test: Re-enable a couple of GetImage tests > glamor: Use eglGetPlatformDisplay{,EXT} if we can > glx/dri2: Don't include drm headers > > Alex Goins (1): > ramdac: Check sPriv != NULL in xf86CheckHWCursor() > > Carlos Garnacho (2): > xwayland: Apply "last pointer window" check only to the pointer device > xwayland: Apply touch abs axes transformation before posting events > > Daniel Martin (1): > modesetting: Consume all available udev events at once > > David CARLIER (1): > xfree86: small memory leaks fixes > > Emil Velikov (8): > glx: drisw is not accelerated IGLX, reflect that in log messages > xfree86: remove aiglx cmd/xorg.conf option > configure.ac: remove --enable-aiglx option > configure.ac: default to DRI=yes on solaris platforms > configure.ac: use $LIBDRM over libdrm when using pkg-config > configure.ac: bump the required libdrm version to 2.3.1 > xfree86/dri: remove libdrm runtime checks > glamor: don't look for non-existing EGL_KHR_platform_base > > Eric Anholt (15): > glamor: Fix some awful formatting of some fallback debug code. > glamor: Require that pixmap depths match for Render copies. > glamor: Properly handle mask formats without alpha. > ephyr: Add a mode for skipping redisplay in glamor > test: Handle srcdir != builddir in Xvfb testing > test: Add a little xinit-like program for starting servers for testing > test: Make the piglit-running script callable with an arbitrary server > test: Fix parsing of piglit results > test: Update piglit HTML even when tests all pass > test: Switch our testing X server to being spawned with simple-xinit > test: Run xts against Xephyr -glamor when present > glamor: Require GL_OES_texture_border_clamp for GLES2. > glamor: Remove many unused glamor util functions. > glamor: Remove #if 0-ed picture dumping code. > glamor: Fix link failure on GLES2. > > Eric Engestrom (1): > glamor: fix spelling mistakes > > Francois Tigeot (1): > Enable XTRANS_SEND_FDS on FreeBSD, DragonFly and OpenBSD > > Hans de Goede (8): > modesetting: Fix reverse prime partial update issues on secondary GPU > outputs > modesetting: Fix reverse prime update lagging on secondary GPU outputs > xf86RandR12: Move calculating of shift inside init_one_component > xf86RandR12: Fix XF86VidModeSetGamma triggering a BadImplementation > error > glx: Always enable EXT_texture_from_pixmap for DRI swrast glx > Xext: Fix a memory leak > XF86VidMode: Fix free() on walked pointer > xfree86: Xorg.wrap: Do not require root rights for cards with 0 outputs > > Jeremy Huddleston Sequoia (7): > dix: Make InitCoreDevices() failures more verbose. > dix: Silence TSan warnings when checking for pending input > XQuartz: Don't respond to SIGALRM on the AppKit thread > XQuartz: Remove X11ApplicationFatalError > XQuartz: pbproxy shouldn't need to wait for server initialization. > XQuartz: Adopt input_lock() and input_unlock() > XQuartz: Silence an expected TSan warning > > Jon Turney (1): > glx/dri2: Don't build DRI loader if DRI2 isn't enabled > > Jonas Ådahl (10): > dix: Add valuator_mask_set_absolute_unaccelerated > xwayland: Bind the relative pointer manager > xwayland: Split up device class init/release into functions > xwayland: Move pointer button initialization into helper > xwayland: Dispatch pointer motion events on wl_pointer.frame if possible > xwayland: Set unaccelerated pointer motion delta if available > xwayland: Put getting a xwl_window from a Window in a helper > xwayland: Bind pointer constraints global > xwayland: Translate a pointer grab with confineTo to pointer confinement > xwayland: Add pointer warp emulator > > Keith Packard (9): > os: Ready clients with pending output aren't flushed, so set > NewOutputPending > os: Clear saved poll events in listen so that edge triggering works > Require xproto 7.0.31 > xace: Don't censor window borders > fb: XYPixmap format PutImage includes all planes in depth > ephyr: Leave window unmapped for -glamor-skip-present [v2] > os: Recompute whether any clients are ready after ProcessWorkQueue() > (bug 98030) > dix: Bump
Re: [PATCH 1/1] Simplify, statement is always true.
On 09/21/2016 05:08 PM, Maya Rashish wrote: > pVideo->bus is uint8_t, always less than 256. > > Signed-off-by: Maya Rashish> --- > hw/xfree86/common/xf86pciBus.c | 6 +- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c > index 8158c2b..bed4434 100644 > --- a/hw/xfree86/common/xf86pciBus.c > +++ b/hw/xfree86/common/xf86pciBus.c > @@ -1457,11 +1457,7 @@ xf86PciConfigureNewDev(void *busData, struct > pci_device *pVideo, > > pVideo = (struct pci_device *) busData; > > -if (pVideo->bus < 256) > -snprintf(busnum, sizeof(busnum), "%d", pVideo->bus); > -else > -snprintf(busnum, sizeof(busnum), "%d@%d", > - pVideo->bus & 0x00ff, pVideo->bus >> 8); > +snprintf(busnum, sizeof(busnum), "%d", pVideo->bus); > > XNFasprintf(, "PCI:%s:%d:%d", > busnum, pVideo->dev, pVideo->func); > Presumably this was to handle the old-style encoding of PCI domain into the bus field -- we don't want to just drop the bus entirely, right? So this should probably be if (pVideo->domain > 0) snprintf(busnum, sizeof(busnum), "%d@%d", pVideo->bus, pVideo->domain); else snprintf(busnum, sizeof(busnum), "%d", pVideo->bus); -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] remove dead code in dummy driver
On 09/22/2016 04:30 PM, Bob Terek wrote: > On 09/21/2016 10:22 AM, Aaron Plattner wrote: >> On 09/20/2016 02:07 AM, Eric Engestrom wrote: >>> On Tue, Sep 20, 2016 at 01:34:40PM +0700, Antoine Martin wrote: >>>> Signed-off-by: Antoine Martin <anto...@nagafix.co.uk> >>> >>> Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com> >> >> Looks good to me too (although I'm cheating since this chunk is >> identical to part of >> https://patchwork.freedesktop.org/patch/41058/) > > Shouldn't the first 5 of Aaron's patches be applied, since they are all > cleanup items? > > https://lists.x.org/archives/xorg-devel/2015-January/045395.html I never pushed them because they were never reviewed. Would it help if I resent them? > Patch 6 supposedly caused a server crash, but the first 5 should be ok? Patch 6 was kind of controversial so I don't know if we want it anyway. > -- > Bob Terek ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xf86-video-dummy v2] Remove pointless empty functions
These functions might be useful in a real driver, but with no hardware, they're pointless. Get rid of them. v2: Rebase, get rid of pointless calls to DUMMYAdjustFrame, return TRUE from DUMMYSwitchMode. Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- src/dummy_driver.c | 44 +--- 1 file changed, 1 insertion(+), 43 deletions(-) diff --git a/src/dummy_driver.c b/src/dummy_driver.c index cf150539e10b..265660280549 100644 --- a/src/dummy_driver.c +++ b/src/dummy_driver.c @@ -65,9 +65,6 @@ static ModeStatus DUMMYValidMode(SCRN_ARG_TYPE arg, DisplayModePtr mode, static BoolDUMMYSaveScreen(ScreenPtr pScreen, int mode); /* Internally used functions */ -static Bool dummyModeInit(ScrnInfoPtr pScrn, DisplayModePtr mode); -static voiddummySave(ScrnInfoPtr pScrn); -static voiddummyRestore(ScrnInfoPtr pScrn, Bool restoreText); static BooldummyDriverFunc(ScrnInfoPtr pScrn, xorgDriverFuncOp op, pointer ptr); @@ -461,14 +458,6 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags) static Bool DUMMYEnterVT(VT_FUNC_ARGS_DECL) { -SCRN_INFO_PTR(arg); - -/* Should we re-save the text mode on each VT enter? */ -if(!dummyModeInit(pScrn, pScrn->currentMode)) - return FALSE; - -DUMMYAdjustFrame(ADJUST_FRAME_ARGS(pScrn, pScrn->frameX0, pScrn->frameY0)); - return TRUE; } @@ -476,8 +465,6 @@ DUMMYEnterVT(VT_FUNC_ARGS_DECL) static void DUMMYLeaveVT(VT_FUNC_ARGS_DECL) { -SCRN_INFO_PTR(arg); -dummyRestore(pScrn, TRUE); } static void @@ -535,15 +522,6 @@ DUMMYScreenInit(SCREEN_INIT_ARGS_DECL) if (!(dPtr->FBBase = malloc(pScrn->videoRam * 1024))) return FALSE; - -/* - * next we save the current state and setup the first mode - */ -dummySave(pScrn); - -if (!dummyModeInit(pScrn,pScrn->currentMode)) - return FALSE; -DUMMYAdjustFrame(ADJUST_FRAME_ARGS(pScrn, pScrn->frameX0, pScrn->frameY0)); /* * Reset visual list. @@ -665,8 +643,7 @@ DUMMYScreenInit(SCREEN_INIT_ARGS_DECL) Bool DUMMYSwitchMode(SWITCH_MODE_ARGS_DECL) { -SCRN_INFO_PTR(arg); -return dummyModeInit(pScrn, mode); +return TRUE; } /* Mandatory */ @@ -683,7 +660,6 @@ DUMMYCloseScreen(CLOSE_SCREEN_ARGS_DECL) DUMMYPtr dPtr = DUMMYPTR(pScrn); if(pScrn->vtSema){ - dummyRestore(pScrn, TRUE); free(dPtr->FBBase); } @@ -725,24 +701,6 @@ DUMMYValidMode(SCRN_ARG_TYPE arg, DisplayModePtr mode, Bool verbose, int flags) return(MODE_OK); } -static void -dummySave(ScrnInfoPtr pScrn) -{ -} - -static void -dummyRestore(ScrnInfoPtr pScrn, Bool restoreText) -{ -} - -static Bool -dummyModeInit(ScrnInfoPtr pScrn, DisplayModePtr mode) -{ -dummyRestore(pScrn, FALSE); - -return(TRUE); -} - Atom VFB_PROP = 0; #define VFB_PROP_NAME "VFB_IDENT" -- 2.10.0 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] remove dead code in dummy driver
On 09/20/2016 02:07 AM, Eric Engestrom wrote: > On Tue, Sep 20, 2016 at 01:34:40PM +0700, Antoine Martin wrote: >> Signed-off-by: Antoine Martin> > Reviewed-by: Eric Engestrom Looks good to me too (although I'm cheating since this chunk is identical to part of https://patchwork.freedesktop.org/patch/41058/) Pushed: remote: Updating patchwork state for https://patchwork.freedesktop.org/project/Xorg/list/ remote: I: patch #111435 updated using rev 367c778240b4266958f33cec3653d5389e283557. remote: I: 1 patch(es) updated to state Accepted. To git+ssh://git.freedesktop.org/git/xorg/driver/xf86-video-dummy 8706f60ab457..367c778240b4 HEAD -> master >> --- >> src/dummy_driver.c | 19 --- >> 1 file changed, 19 deletions(-) >> >> diff --git a/src/dummy_driver.c b/src/dummy_driver.c >> index c84000f..ec1acf3 100644 >> --- a/src/dummy_driver.c >> +++ b/src/dummy_driver.c >> @@ -700,25 +700,6 @@ DUMMYSwitchMode(SWITCH_MODE_ARGS_DECL) >> void >> DUMMYAdjustFrame(ADJUST_FRAME_ARGS_DECL) >> { >> -SCRN_INFO_PTR(arg); >> -int Base; >> - >> -Base = (y * pScrn->displayWidth + x) >> 2; >> - >> -/* Scale Base by the number of bytes per pixel. */ >> -switch (pScrn->depth) { >> -case 8 : >> -break; >> -case 15 : >> -case 16 : >> -Base *= 2; >> -break; >> -case 24 : >> -Base *= 3; >> -break; >> -default : >> -break; >> -} >> } >> >> /* Mandatory */ >> -- >> 2.7.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: Proposed X server 1.19 schedule
On 09/13/2016 10:27 PM, Timo Aaltonen wrote: On 09.09.2016 20:18, Aaron Plattner wrote: On 09/06/2016 12:27 PM, Keith Packard wrote: Adam Jackson <a...@nwnk.net> writes: ... move the non-critical bug deadline to 2016-10-01? Still leaves three weeks for critical fixes. Either way, looks plausible to me. I don't personally have any non-bug changes I want to land before 1.19. Yeah, sounds good. We don't usually get many changes during the critical bug window anyways. When do you want to freeze the ABI? It would be good to get Alex's prime sync changes in an official ABI and run through our QA test cycle before the release. that was merged on Jun 28th. Right. What I meant is that I need to flip the switch in our driver to report ABI 23 as officially supported, get a driver built, and have it run through an internal QA test cycle so we can release it around the same time as xserver 1.19. In order to do that, I need to know that the ABI isn't going to change again before the release. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH:xserver] OsSigHandler should not show rtld errors for unrelated signals
On 09/10/2016 09:08 PM, Alan Coopersmith wrote: > If RTLD_DI_SETSIGNAL is set to let us turn runtime linker/loader errors > into catchable signals, then we should only show the errors when catching > that signal, instead of tossing out red herrings to distract people with > unrelated crashes long after their last failed symbol lookup (especially > when using drivers built to support multiple API's by checking which > symbols are available before calling them). Nice. Presumably you can also SIGQUIT the server with Ctrl-\ in the terminal it was run on, but it doesn't seem worth trying to suppress the dlsym error in that case too. Reviewed-by: Aaron Plattner <aplatt...@nvidia.com> > Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> > --- > os/osinit.c | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/os/osinit.c b/os/osinit.c > index 47061bb..5b2f6b5 100644 > --- a/os/osinit.c > +++ b/os/osinit.c > @@ -114,10 +114,14 @@ OsSigHandler(int signo) > #endif > { > #ifdef RTLD_DI_SETSIGNAL > -const char *dlerr = dlerror(); > +# define SIGNAL_FOR_RTLD_ERROR SIGQUIT > +if (signo == SIGNAL_FOR_RTLD_ERROR) { > +const char *dlerr = dlerror(); > > -if (dlerr) { > -LogMessageVerbSigSafe(X_ERROR, 1, "Dynamic loader error: %s\n", > dlerr); > +if (dlerr) { > +LogMessageVerbSigSafe(X_ERROR, 1, > + "Dynamic loader error: %s\n", dlerr); > +} > } > #endif /* RTLD_DI_SETSIGNAL */ > > @@ -217,7 +221,7 @@ OsInit(void) > * after ourselves. > */ > { > -int failure_signal = SIGQUIT; > +int failure_signal = SIGNAL_FOR_RTLD_ERROR; > > dlinfo(RTLD_SELF, RTLD_DI_SETSIGNAL, _signal); > } > ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 2/3] modesetting: Remove some dead code
On 08/16/2016 02:10 AM, Hans de Goede wrote: > The "if (pixmap) ..." block this commit removes is inside an > "if (pixmap == NULL) ..." block, so it will never execute. > > Signed-off-by: Hans de Goede <hdego...@redhat.com> > --- > hw/xfree86/drivers/modesetting/dri2.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/hw/xfree86/drivers/modesetting/dri2.c > b/hw/xfree86/drivers/modesetting/dri2.c > index b810d59..9bc56c2 100644 > --- a/hw/xfree86/drivers/modesetting/dri2.c > +++ b/hw/xfree86/drivers/modesetting/dri2.c > @@ -186,8 +186,6 @@ ms_dri2_create_buffer(DrawablePtr drawable, unsigned int > attachment, >pixmap_cpp, >0); > if (pixmap == NULL) { > -if (pixmap) > -screen->DestroyPixmap(pixmap); > free(private); > free(buffer); > return NULL; > Well this one is certainly obvious. Reviewed-by: Aaron Plattner <aplatt...@nvidia.com> ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: Proposed X server 1.19 schedule
On 09/06/2016 12:27 PM, Keith Packard wrote: > Adam Jacksonwrites: > >> ... move the non-critical bug deadline to 2016-10-01? Still leaves >> three weeks for critical fixes. Either way, looks plausible to me. I >> don't personally have any non-bug changes I want to land before 1.19. > > Yeah, sounds good. We don't usually get many changes during the critical > bug window anyways. When do you want to freeze the ABI? It would be good to get Alex's prime sync changes in an official ABI and run through our QA test cycle before the release. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v6 xserver 7/7] xf86Cursor: Add hw cursor support for prime
On 09/07/2016 08:51 AM, Hans de Goede wrote: > Hi, > > On 07-09-16 17:38, Aaron Plattner wrote: >> On 09/07/2016 08:24 AM, Hans de Goede wrote: >>> Hi, >>> >>> On 07-09-16 17:02, Aaron Plattner wrote: >>>> Adding Alex. >>>> >>>> On 09/07/2016 05:26 AM, Hans de Goede wrote: >>>>> From: Dave Airlie <airl...@redhat.com> >>>>> >>>>> Currently with PRIME if we detect a secondary GPU, >>>>> we switch to using SW cursors, this isn't optimal, >>>>> esp for the intel/nvidia combinations, we have >>>>> no choice for the USB offload devices. >>>>> >>>>> This patch checks on each slave screen if hw >>>>> cursors are enabled, and also calls set cursor >>>>> and move cursor on all screens. >>>>> >>>>> Cc: Aaron Plattner <aplatt...@nvidia.com> >>>>> Signed-off-by: Dave Airlie <airl...@redhat.com> >>>>> Signed-off-by: Hans de Goede <hdego...@redhat.com> >>>>> Reviewed-by: Michel Dänzer <michel.daen...@amd.com> >>>>> --- >>>>> Changes in v6 (Hans de Goede): >>>>> -Move removal of xorg_list_is_empty(>pixmap_dirty_list) >>>>> check from xf86CursorSetCursor() back to this patch (accidentally >>>>> moved to >>>>> "xf86Cursor: Add xf86CheckHWCursor() helper function" in v5) >>>>> >>>>> Changes in v5 (Hans de Goede): >>>>> -Split out various helper functions into separate patches >>>>> -Fix cursor showing in the wrong spot on slave-outputs when rotation >>>>> is used >>>>> >>>>> Changes in v4 (Hans de Goede): >>>>> -Fix crash when xf86ScreenSetCursor() gets called on a hot-plugged GPU >>>>> >>>>> Changes in v3 (Hans de Goede): >>>>> -Re-indent xf86CursorScreenCheckHW >>>>> -Loop over slave-outputs instead of over pixmap_dirty_list, Aaron >>>>> Plattner has >>>>> indicated that the nvidia driver does not use pixmap_dirty_list and >>>>> with >>>>> the recent "randr/xf86: Add PRIME Synchronization / Double Buffer" >>>>> changes >>>>> there may be 2 pixmaps pointing to the same GPU screen on the >>>>> pixmap_dirty_list twice >>>>> -Make xf86CurrentCursor work when called on GPU screen pointers >>>>> (fixes the modesetting driver as outputslave crashing) >>>>> -Fix both hw and sw cursor showing at the same time when an usb >>>>> slave-gpu is >>>>> present at cold plug time >>>>> >>>>> Changes in v2: >>>>> -hotplugging causes the driver to try and register >>>>> the cursor private, however we can't register >>>>> these at runtime yet, we need to add support for >>>>> reallocation of cursor screen privates. However >>>>> all hotplugged devices currently don't support >>>>> HW cursors, so punt for now. This means GPUs >>>>> already in the system will get hw cursors >>>>> and USB ones won't which is all we care about >>>>> presently. >>>>> -drop list empty checks (Eric) >>>>> -fixup comments (Michel) >>>>> --- >>>>> dix/dispatch.c | 4 +++ >>>>> hw/xfree86/ramdac/xf86Cursor.c | 2 +- >>>>> hw/xfree86/ramdac/xf86HWCurs.c | 78 >>>>> +++--- >>>>> 3 files changed, 78 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/dix/dispatch.c b/dix/dispatch.c >>>>> index a3c2fbb..21c387e 100644 >>>>> --- a/dix/dispatch.c >>>>> +++ b/dix/dispatch.c >>>>> @@ -3965,6 +3965,10 @@ AddGPUScreen(Bool (*pfnInit) (ScreenPtr >>>>> /*pScreen */ , >>>>> >>>>> update_desktop_dimensions(); >>>>> >>>>> +if (!dixPrivatesCreated(PRIVATE_CURSOR)) >>>>> +dixRegisterScreenPrivateKey(, pScreen, >>>>> +PRIVATE_CURSOR, 0); >>>>> + >>>>> return i; >>>>> } >>>>> >>>>> diff --git a/hw/xfree86/ramdac/xf86Cursor.c >>>>> b/hw/xfree86/ramdac/xf86Cursor.c >>>>> index 623a65b..
Re: [PATCH v6 xserver 7/7] xf86Cursor: Add hw cursor support for prime
On 09/07/2016 08:24 AM, Hans de Goede wrote: Hi, On 07-09-16 17:02, Aaron Plattner wrote: Adding Alex. On 09/07/2016 05:26 AM, Hans de Goede wrote: From: Dave Airlie <airl...@redhat.com> Currently with PRIME if we detect a secondary GPU, we switch to using SW cursors, this isn't optimal, esp for the intel/nvidia combinations, we have no choice for the USB offload devices. This patch checks on each slave screen if hw cursors are enabled, and also calls set cursor and move cursor on all screens. Cc: Aaron Plattner <aplatt...@nvidia.com> Signed-off-by: Dave Airlie <airl...@redhat.com> Signed-off-by: Hans de Goede <hdego...@redhat.com> Reviewed-by: Michel Dänzer <michel.daen...@amd.com> --- Changes in v6 (Hans de Goede): -Move removal of xorg_list_is_empty(>pixmap_dirty_list) check from xf86CursorSetCursor() back to this patch (accidentally moved to "xf86Cursor: Add xf86CheckHWCursor() helper function" in v5) Changes in v5 (Hans de Goede): -Split out various helper functions into separate patches -Fix cursor showing in the wrong spot on slave-outputs when rotation is used Changes in v4 (Hans de Goede): -Fix crash when xf86ScreenSetCursor() gets called on a hot-plugged GPU Changes in v3 (Hans de Goede): -Re-indent xf86CursorScreenCheckHW -Loop over slave-outputs instead of over pixmap_dirty_list, Aaron Plattner has indicated that the nvidia driver does not use pixmap_dirty_list and with the recent "randr/xf86: Add PRIME Synchronization / Double Buffer" changes there may be 2 pixmaps pointing to the same GPU screen on the pixmap_dirty_list twice -Make xf86CurrentCursor work when called on GPU screen pointers (fixes the modesetting driver as outputslave crashing) -Fix both hw and sw cursor showing at the same time when an usb slave-gpu is present at cold plug time Changes in v2: -hotplugging causes the driver to try and register the cursor private, however we can't register these at runtime yet, we need to add support for reallocation of cursor screen privates. However all hotplugged devices currently don't support HW cursors, so punt for now. This means GPUs already in the system will get hw cursors and USB ones won't which is all we care about presently. -drop list empty checks (Eric) -fixup comments (Michel) --- dix/dispatch.c | 4 +++ hw/xfree86/ramdac/xf86Cursor.c | 2 +- hw/xfree86/ramdac/xf86HWCurs.c | 78 +++--- 3 files changed, 78 insertions(+), 6 deletions(-) diff --git a/dix/dispatch.c b/dix/dispatch.c index a3c2fbb..21c387e 100644 --- a/dix/dispatch.c +++ b/dix/dispatch.c @@ -3965,6 +3965,10 @@ AddGPUScreen(Bool (*pfnInit) (ScreenPtr /*pScreen */ , update_desktop_dimensions(); +if (!dixPrivatesCreated(PRIVATE_CURSOR)) +dixRegisterScreenPrivateKey(, pScreen, +PRIVATE_CURSOR, 0); + return i; } diff --git a/hw/xfree86/ramdac/xf86Cursor.c b/hw/xfree86/ramdac/xf86Cursor.c index 623a65b..afcce53 100644 --- a/hw/xfree86/ramdac/xf86Cursor.c +++ b/hw/xfree86/ramdac/xf86Cursor.c @@ -337,7 +337,7 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs, return; } -if (infoPtr->pScrn->vtSema && xorg_list_is_empty(>pixmap_dirty_list) && +if (infoPtr->pScrn->vtSema && (ScreenPriv->ForceHWCursorCount || xf86CheckHWCursor(pScreen, cursor, infoPtr))) { diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c index 0f6990a..25d9f5d 100644 --- a/hw/xfree86/ramdac/xf86HWCurs.c +++ b/hw/xfree86/ramdac/xf86HWCurs.c @@ -17,6 +17,7 @@ #include "cursorstr.h" #include "mi.h" #include "mipointer.h" +#include "randrstr.h" #include "xf86CursorPriv.h" #include "servermd.h" @@ -129,8 +130,8 @@ xf86ShowCursor(xf86CursorInfoPtr infoPtr) return TRUE; } -Bool -xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr) +static Bool +xf86ScreenCheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr) { return (cursor->bits->argb && infoPtr->UseHWCursorARGB && @@ -142,7 +143,29 @@ xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr } Bool -xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y) +xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr) +{ +ScreenPtr pSlave; + +if (!xf86ScreenCheckHWCursor(pScreen, cursor, infoPtr)) +return FALSE; + +/* ask each driver consuming a pixmap if it can support HW cursor */ +xorg_list_for_each_entry(pSlave, >slave_list, slave_head) { +xf86CursorScreenPtr sPriv; + +if (!RRHasScanoutPixmap(pSlave)) +continue; A hypothetical future version of the nvidia driver that supports be
Re: [PATCH v6 xserver 7/7] xf86Cursor: Add hw cursor support for prime
Adding Alex. On 09/07/2016 05:26 AM, Hans de Goede wrote: From: Dave Airlie <airl...@redhat.com> Currently with PRIME if we detect a secondary GPU, we switch to using SW cursors, this isn't optimal, esp for the intel/nvidia combinations, we have no choice for the USB offload devices. This patch checks on each slave screen if hw cursors are enabled, and also calls set cursor and move cursor on all screens. Cc: Aaron Plattner <aplatt...@nvidia.com> Signed-off-by: Dave Airlie <airl...@redhat.com> Signed-off-by: Hans de Goede <hdego...@redhat.com> Reviewed-by: Michel Dänzer <michel.daen...@amd.com> --- Changes in v6 (Hans de Goede): -Move removal of xorg_list_is_empty(>pixmap_dirty_list) check from xf86CursorSetCursor() back to this patch (accidentally moved to "xf86Cursor: Add xf86CheckHWCursor() helper function" in v5) Changes in v5 (Hans de Goede): -Split out various helper functions into separate patches -Fix cursor showing in the wrong spot on slave-outputs when rotation is used Changes in v4 (Hans de Goede): -Fix crash when xf86ScreenSetCursor() gets called on a hot-plugged GPU Changes in v3 (Hans de Goede): -Re-indent xf86CursorScreenCheckHW -Loop over slave-outputs instead of over pixmap_dirty_list, Aaron Plattner has indicated that the nvidia driver does not use pixmap_dirty_list and with the recent "randr/xf86: Add PRIME Synchronization / Double Buffer" changes there may be 2 pixmaps pointing to the same GPU screen on the pixmap_dirty_list twice -Make xf86CurrentCursor work when called on GPU screen pointers (fixes the modesetting driver as outputslave crashing) -Fix both hw and sw cursor showing at the same time when an usb slave-gpu is present at cold plug time Changes in v2: -hotplugging causes the driver to try and register the cursor private, however we can't register these at runtime yet, we need to add support for reallocation of cursor screen privates. However all hotplugged devices currently don't support HW cursors, so punt for now. This means GPUs already in the system will get hw cursors and USB ones won't which is all we care about presently. -drop list empty checks (Eric) -fixup comments (Michel) --- dix/dispatch.c | 4 +++ hw/xfree86/ramdac/xf86Cursor.c | 2 +- hw/xfree86/ramdac/xf86HWCurs.c | 78 +++--- 3 files changed, 78 insertions(+), 6 deletions(-) diff --git a/dix/dispatch.c b/dix/dispatch.c index a3c2fbb..21c387e 100644 --- a/dix/dispatch.c +++ b/dix/dispatch.c @@ -3965,6 +3965,10 @@ AddGPUScreen(Bool (*pfnInit) (ScreenPtr /*pScreen */ , update_desktop_dimensions(); +if (!dixPrivatesCreated(PRIVATE_CURSOR)) +dixRegisterScreenPrivateKey(, pScreen, +PRIVATE_CURSOR, 0); + return i; } diff --git a/hw/xfree86/ramdac/xf86Cursor.c b/hw/xfree86/ramdac/xf86Cursor.c index 623a65b..afcce53 100644 --- a/hw/xfree86/ramdac/xf86Cursor.c +++ b/hw/xfree86/ramdac/xf86Cursor.c @@ -337,7 +337,7 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs, return; } -if (infoPtr->pScrn->vtSema && xorg_list_is_empty(>pixmap_dirty_list) && +if (infoPtr->pScrn->vtSema && (ScreenPriv->ForceHWCursorCount || xf86CheckHWCursor(pScreen, cursor, infoPtr))) { diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c index 0f6990a..25d9f5d 100644 --- a/hw/xfree86/ramdac/xf86HWCurs.c +++ b/hw/xfree86/ramdac/xf86HWCurs.c @@ -17,6 +17,7 @@ #include "cursorstr.h" #include "mi.h" #include "mipointer.h" +#include "randrstr.h" #include "xf86CursorPriv.h" #include "servermd.h" @@ -129,8 +130,8 @@ xf86ShowCursor(xf86CursorInfoPtr infoPtr) return TRUE; } -Bool -xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr) +static Bool +xf86ScreenCheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr) { return (cursor->bits->argb && infoPtr->UseHWCursorARGB && @@ -142,7 +143,29 @@ xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr } Bool -xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y) +xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr) +{ +ScreenPtr pSlave; + +if (!xf86ScreenCheckHWCursor(pScreen, cursor, infoPtr)) +return FALSE; + +/* ask each driver consuming a pixmap if it can support HW cursor */ +xorg_list_for_each_entry(pSlave, >slave_list, slave_head) { +xf86CursorScreenPtr sPriv; + +if (!RRHasScanoutPixmap(pSlave)) +continue; A hypothetical future version of the nvidia driver that supports being loaded as a GPU screen probably wouldn't use the RR scanout pixmap stuff, but I suppo
Re: [PATCH] xace: Fix XaceCensorImage to actually censor the right part of the image
Aargh, stupid borders. I always forget about them. I guess this is why we have regression tests. On 08/18/2016 09:11 AM, Adam Jackson wrote: > On Thu, 2016-08-18 at 11:09 +0900, Michel Dänzer wrote: > >> Unfortunately, this broke two XTS tests: >> >> xts5@xlib9@xgetimage@7 >> xts5@xlib9@xgetsubimage@7 Thanks for catching this. > Low impact, fortunately, but still unpleasant. The test in question is: > > 520|0 7 00020031 1 2|Assertion XGetImage-7.(A) > 520|0 7 00020031 1 3|When the specified rectangle includes the window border, > 520|0 7 00020031 1 4|then the contents of the window border are obtained in > the > 520|0 7 00020031 1 5|XImage structure returned by a call to XGetImage. > > I think there are two issues here. One is pVisibleRegion (the region we > don't censor) is the intersection of borderClip (the exterior > dimensions of the window including the border, clipped by siblings) and > winSize (the inside-the-border region of the window). Clipping by > winSize means we'll censor the window border. I think what's actually > wanted there is borderClip also clipped by children [1]; we don't have > a function handy to compute that, but it's straightforward enough. > > The other issue is we censor the image unconditionally if the server > was built with support for any security extensions, regardless of > whether the requesting client is trusted (for XC-SECURITY) or in a > different security context than the window (for XACE). > > Patches forthcoming. And thanks Adam for fixing it. > [1] - Well kinda. You want to clip away children whose contents you > aren't authorized to see, which isn't quite the same. > > - ajax > ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH] xace: Fix XaceCensorImage to actually censor the right part of the image
The caller passes arguments into XaceCensorImage that are in window-relative coordinates. However, the pBuf that it uses to construct a temporary pixmap has its origin at (x, y) relative to the window in question. The code to convert the censor region into boxes adjusts for the Y coordinate, but leaves the X coordinate alone. The result is that if x is not zero, it censors the wrong part of the image. Fix this by just translating censorRegion into pixmap-relative coordinates and using the resulting boxes as-is. Reported-by: Fabien Lelaquais <fabien.lelaqu...@roguewave.com> Link: https://lists.x.org/archives/xorg/2016-August/058165.html Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- Xext/xace.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Xext/xace.c b/Xext/xace.c index 91c74d591267..a3a83a20c08a 100644 --- a/Xext/xace.c +++ b/Xext/xace.c @@ -245,6 +245,7 @@ XaceCensorImage(ClientPtr client, /* censorRegion = imageRegion - visibleRegion */ RegionSubtract(, , pVisibleRegion); +RegionTranslate(, -x, -y); nRects = RegionNumRects(); if (nRects > 0) { /* we have something to censor */ GCPtr pScratchGC = NULL; @@ -265,7 +266,7 @@ XaceCensorImage(ClientPtr client, } for (pBox = RegionRects(), i = 0; i < nRects; i++, pBox++) { pRects[i].x = pBox->x1; -pRects[i].y = pBox->y1 - imageBox.y1; +pRects[i].y = pBox->y1; pRects[i].width = pBox->x2 - pBox->x1; pRects[i].height = pBox->y2 - pBox->y1; } -- 2.9.0 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 2/4] xfree86: remove unused path from the LoadModule API
On 04/17/2016 01:07 PM, Emil Velikov wrote: Similar to its little brothre - LoadSubModule. Currently all call sites provide NULL anyway ;-) Cc: Aaron Plattner <aplatt...@nvidia.com> Signed-off-by: Emil Velikov <emil.l.veli...@gmail.com> --- Aaron, are you guys using the argument in the closed source driver ? We don't call LoadModule at all. -Emil hw/xfree86/common/xf86Helper.c | 2 +- hw/xfree86/common/xf86Init.c| 2 +- hw/xfree86/doc/ddxDesign.xml| 8 +--- hw/xfree86/loader/loaderProcs.h | 2 +- hw/xfree86/loader/loadmod.c | 7 +++ 5 files changed, 7 insertions(+), 14 deletions(-) diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c index 3b01a49..2c0f0d8 100644 --- a/hw/xfree86/common/xf86Helper.c +++ b/hw/xfree86/common/xf86Helper.c @@ -1626,7 +1626,7 @@ xf86LoadOneModule(const char *name, void *opt) return NULL; } -mod = LoadModule(Name, NULL, NULL, NULL, opt, NULL, , ); +mod = LoadModule(Name, NULL, NULL, opt, NULL, , ); if (!mod) LoaderErrorMsg(NULL, Name, errmaj, errmin); free(Name); diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c index de51497..9b88e29 100644 --- a/hw/xfree86/common/xf86Init.c +++ b/hw/xfree86/common/xf86Init.c @@ -1571,7 +1571,7 @@ xf86LoadModules(const char **list, void **optlist) else opt = NULL; -if (!LoadModule(name, NULL, NULL, NULL, opt, NULL, , )) { +if (!LoadModule(name, NULL, NULL, opt, NULL, , )) { LoaderErrorMsg(NULL, name, errmaj, errmin); failed = TRUE; } diff --git a/hw/xfree86/doc/ddxDesign.xml b/hw/xfree86/doc/ddxDesign.xml index a8e539c..7baf2cb 100644 --- a/hw/xfree86/doc/ddxDesign.xml +++ b/hw/xfree86/doc/ddxDesign.xml @@ -5222,7 +5222,7 @@ XFree86 common layer. -pointer LoadModule(const char *module, const char *path, +pointer LoadModule(const char *module, const char **subdirlist, const char **patternlist, pointer options, const XF86ModReqInfo * modreq, int *errmaj, int *errmin); @@ -5238,12 +5238,6 @@ XFree86 common layer. This might change. The other parameters are: - - path - - An optional comma-separated list of module search paths. - When NULL, the default search path is used. - diff --git a/hw/xfree86/loader/loaderProcs.h b/hw/xfree86/loader/loaderProcs.h index cfc4d80..8d7872f 100644 --- a/hw/xfree86/loader/loaderProcs.h +++ b/hw/xfree86/loader/loaderProcs.h @@ -74,7 +74,7 @@ void LoaderInit(void); ModuleDescPtr LoadDriver(const char *, const char *, int, void *, int *, int *); -ModuleDescPtr LoadModule(const char *, const char *, const char **, +ModuleDescPtr LoadModule(const char *, const char **, const char **, void *, const XF86ModReqInfo *, int *, int *); ModuleDescPtr DuplicateModule(ModuleDescPtr mod, ModuleDescPtr parent); diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c index 702d4e7..603ef65 100644 --- a/hw/xfree86/loader/loadmod.c +++ b/hw/xfree86/loader/loadmod.c @@ -747,7 +747,7 @@ LoadSubModule(void *_parent, const char *module, return NULL; } -submod = LoadModule(module, NULL, subdirlist, patternlist, options, +submod = LoadModule(module, subdirlist, patternlist, options, modreq, errmaj, errmin); if (submod && submod != (ModuleDescPtr) 1) { parent->child = AddSibling(parent->child, submod); @@ -817,7 +817,6 @@ static const char *compiled_in_modules[] = { * module The module name. Normally this is not a filename but the * module's "canonical name. A full pathname is, however, * also accepted. - * path A comma separated list of module directories. * subdirlist A NULL terminated list of subdirectories to search. When * NULL, the default "stdSubdirs" list is used. The default * list is also substituted for entries with value DEFAULT_LIST. @@ -849,7 +848,7 @@ static const char *compiled_in_modules[] = { * */ ModuleDescPtr -LoadModule(const char *module, const char *path, const char **subdirlist, +LoadModule(const char *module, const char **subdirlist, const char **patternlist, void *options, const XF86ModReqInfo * modreq, int *errmaj, int *errmin) { @@ -905,7 +904,7 @@ LoadModule(const char *module, const char *path, const char **subdirlist, goto LoadModule_fail; } -pathlist = InitPathList(path); +pathlist = InitPathList(NULL); if (!pathlist) { /* This could be a malloc failure too */ if (errma
Re: [PATCH xserver 1/2] xfree86/modes: Fix HW cursor clipping for crtc->driverIsPerformingTransform
On 01/29/2016 07:23 AM, Keith Packard wrote: > Michel Dänzerwrites: > >> I'm not sure that makes sense; e.g. it seems inconsistent with leaving >> the cursor image untransformed in the driverIsPerformingTransform case. >> It seems to me like the idea behind driverIsPerformingTransform was to >> leave all transformations to the driver/hardware. > > I was just thinking that we've already done the transform to perform the > clip, so forcing the driver to *also* transform seems weird to me. The idea was that for GPUs that can do scaling after compositing the cursor, you want to program the untransformed cursor position and then let the hardware transform happen, while on the Tegra hardware I wrote this for, it couldn't do that and the transform had to be applied to the cursor separately. I left it up to the driver to decide so that we could use this path in the desktop GPU X driver. That was the idea behind http://marc.info/?l=freedesktop-xorg-devel=131431532812271=2 The question of whether to transform the cursor image was a bit tricky because Tegra did scaling before compositing the cursor, but rotation after. So it wasn't just a yes or no question about whether the image needed to be transformed. For other reasons, we ended up not using the xfree86 RandR layer, so we aren't using this code in our now-unified X driver. Are there other drivers that use it? If not, I'd be fine with just ripping it out since the old Tegra X driver is defunct. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] privates: Clear screen-specific keys during CloseScreen
On 09/16/2015 07:34 PM, Keith Packard wrote: > Aaron Plattner <aplatt...@nvidia.com> writes: > >> The modesetting driver corrupts memory when used after a server regeneration >> because not enough memory is allocated for its pixmap privates. This happens >> because its call to dixRegisterScreenSpecificPrivateKey() does nothing >> because >> key->initialized is still TRUE from the first server generation. However, >> the >> key is not in the screen's linked list of screen-specific privates because >> that's freed and reallocated during the server generation loop in dix_main(). >> >> Fix this by clearing the screen-specific keys during CloseScreen, the same >> way >> other privates are cleared during dixResetPrivates(). > > Yeah, this makes sense. I think that we expected the screen-specific > privates to be allocated as a part of the screen initialization or > privates (and hence why dixFreeScreenSpecificPrivates is called before > CloseScreen when it might get freed if it were separately allocated). > > I have a slight fear that CloseScreen is going to reference these > privates, and so cleaning them before CloseScreen might turn out > badly. A lot of code gets executed in CloseScreen, after all. Yeah, that's fair. I think it should be safe to move these calls to after CloseScreen, but I'll have to think through it harder to convince myself. > There's no interface for screen-specific privates to be allocated by the > privates.c code, so we can safely assume that they do not need to be > freed themselves. Unless the caller messed with the key internals, key->allocated should be set to FALSE by dixRegisterScreenSpecificPrivateKey(). > How about we just set key->initialized = FALSE and leave the rest of the > key alone? That does assume that no screens will get hot-added during > CloseScreen, which seems entirely reasonable to me. The whole thing will > then be neatly re-set when the server comes around again. We could certainly just set key->initialized to FALSE instead, yeah. But then why does dixResetPrivates() bother setting anything else? I agree that adding new screens during the CloseScreen loop is asking for trouble. I hope that can't happen today... > Alternatively, we could assert that dixRegisterScreenSpecificPrivateKey > is only called once per key and remove the check for key->initialized, > which is really only valid for keys used across multiple objects. I think? I don't think that works because we need these keys to be in pScreen->screenSpecificPrivates[*].key, and that's lost when the screen is recycled. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH v2] privates: Clear screen-specific keys during CloseScreen
The modesetting driver corrupts memory when used after a server regeneration because not enough memory is allocated for its pixmap privates. This happens because its call to dixRegisterScreenSpecificPrivateKey() does nothing because key->initialized is still TRUE from the first server generation. However, the key is not in the screen's linked list of screen-specific privates because that's freed and reallocated during the server generation loop in dix_main(). Fix this by clearing key->initialized after CloseScreen. Move the call to dixFreeScreenSpecificPrivates() after the call to CloseScreen, in case a driver's CloseScreen needs a screen private for something. Finally, add a call to dixFreeScreenSpecificPrivates() for GPU screens. Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- Keith, I was thinking about your suggestion of ignoring key->initialized backwards yesterday. I think that would work too, but I think this is clearer and more explicit. dix/main.c | 3 ++- dix/privates.c | 9 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/dix/main.c b/dix/main.c index 09f9504b8acb..43f176f2d3b3 100644 --- a/dix/main.c +++ b/dix/main.c @@ -338,6 +338,7 @@ dix_main(int argc, char *argv[], char *envp[]) ScreenPtr pScreen = screenInfo.gpuscreens[i]; FreeScratchPixmapsForScreen(pScreen); (*pScreen->CloseScreen) (pScreen); +dixFreeScreenSpecificPrivates(pScreen); dixFreePrivates(pScreen->devPrivates, PRIVATE_SCREEN); free(pScreen); screenInfo.numGPUScreens = i; @@ -347,8 +348,8 @@ dix_main(int argc, char *argv[], char *envp[]) FreeScratchPixmapsForScreen(screenInfo.screens[i]); FreeGCperDepth(i); FreeDefaultStipple(i); -dixFreeScreenSpecificPrivates(screenInfo.screens[i]); (*screenInfo.screens[i]->CloseScreen) (screenInfo.screens[i]); +dixFreeScreenSpecificPrivates(screenInfo.screens[i]); dixFreePrivates(screenInfo.screens[i]->devPrivates, PRIVATE_SCREEN); free(screenInfo.screens[i]); screenInfo.numScreens = i; diff --git a/dix/privates.c b/dix/privates.c index e03b2255b7f3..969d0141c844 100644 --- a/dix/privates.c +++ b/dix/privates.c @@ -642,6 +642,15 @@ dixRegisterScreenSpecificPrivateKey(ScreenPtr pScreen, DevPrivateKey key, void dixFreeScreenSpecificPrivates(ScreenPtr pScreen) { +DevPrivateType t; + +for (t = PRIVATE_XSELINUX; t < PRIVATE_LAST; t++) { +DevPrivateKey key; + +for (key = pScreen->screenSpecificPrivates[t].key; key; key = key->next) { +key->initialized = FALSE; +} +} } /* Initialize screen-specific privates in AddScreen */ -- 2.5.2 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH v3] privates: Clear screen-specific keys during CloseScreen
The modesetting driver corrupts memory when used after a server regeneration because not enough memory is allocated for its pixmap privates. This happens because its call to dixRegisterScreenSpecificPrivateKey() does nothing because key->initialized is still TRUE from the first server generation. However, the key is not in the screen's linked list of screen-specific privates because that's freed and reallocated during the server generation loop in dix_main(). Fix this by clearing key->initialized before CloseScreen and add a call to dixFreeScreenSpecificPrivates() for GPU screens. v2: Just set key->initialized to FALSE and move dixFreeScreenSpecificPrivates() calls to after CloseScreen. v3: Move dixFreeScreenSpecificPrivates() calls back to just before CloseScreen. Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- Third time's the charm? dix/main.c | 1 + dix/privates.c | 9 + 2 files changed, 10 insertions(+) diff --git a/dix/main.c b/dix/main.c index 09f9504b8acb..7c6ac943f48b 100644 --- a/dix/main.c +++ b/dix/main.c @@ -337,6 +337,7 @@ dix_main(int argc, char *argv[], char *envp[]) for (i = screenInfo.numGPUScreens - 1; i >= 0; i--) { ScreenPtr pScreen = screenInfo.gpuscreens[i]; FreeScratchPixmapsForScreen(pScreen); +dixFreeScreenSpecificPrivates(pScreen); (*pScreen->CloseScreen) (pScreen); dixFreePrivates(pScreen->devPrivates, PRIVATE_SCREEN); free(pScreen); diff --git a/dix/privates.c b/dix/privates.c index e03b2255b7f3..969d0141c844 100644 --- a/dix/privates.c +++ b/dix/privates.c @@ -642,6 +642,15 @@ dixRegisterScreenSpecificPrivateKey(ScreenPtr pScreen, DevPrivateKey key, void dixFreeScreenSpecificPrivates(ScreenPtr pScreen) { +DevPrivateType t; + +for (t = PRIVATE_XSELINUX; t < PRIVATE_LAST; t++) { +DevPrivateKey key; + +for (key = pScreen->screenSpecificPrivates[t].key; key; key = key->next) { +key->initialized = FALSE; +} +} } /* Initialize screen-specific privates in AddScreen */ -- 2.5.2 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2] privates: Clear screen-specific keys during CloseScreen
On 09/17/2015 03:57 PM, Keith Packard wrote: > Aaron Plattner <aplatt...@nvidia.com> writes: > >> The modesetting driver corrupts memory when used after a server regeneration >> because not enough memory is allocated for its pixmap privates. This happens >> because its call to dixRegisterScreenSpecificPrivateKey() does nothing >> because >> key->initialized is still TRUE from the first server generation. However, >> the >> key is not in the screen's linked list of screen-specific privates because >> that's freed and reallocated during the server generation loop in dix_main(). >> >> Fix this by clearing key->initialized after CloseScreen. Move the call to >> dixFreeScreenSpecificPrivates() after the call to CloseScreen, in case a >> driver's CloseScreen needs a screen private for something. > > Oh, if you're just going to reset the key->initialized value, you should > leave the call above CloseScreen. That's because the storage for the key > may well be allocated by the driver, and would be freed by CloseScreen. Oh, duh, gotcha. Sorry for being dense. >> I was thinking about your suggestion of ignoring key->initialized backwards >> yesterday. I think that would work too, but I think this is clearer and more >> explicit. > > Cool, this does seem like the most conservative plan possible. Let's > just keep doing that before CloseScreen is called to avoid storing > through freed memory. Hopefully third time's the charm. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH] privates: Clear screen-specific keys during CloseScreen
The modesetting driver corrupts memory when used after a server regeneration because not enough memory is allocated for its pixmap privates. This happens because its call to dixRegisterScreenSpecificPrivateKey() does nothing because key->initialized is still TRUE from the first server generation. However, the key is not in the screen's linked list of screen-specific privates because that's freed and reallocated during the server generation loop in dix_main(). Fix this by clearing the screen-specific keys during CloseScreen, the same way other privates are cleared during dixResetPrivates(). Finally, add a call to dixFreeScreenSpecificPrivates() for GPU screens. Signed-off-by: Aaron Plattner <aplatt...@nvidia.com> --- dix/main.c | 1 + dix/privates.c | 34 +++--- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/dix/main.c b/dix/main.c index d7a9cdaae8e7..549567660379 100644 --- a/dix/main.c +++ b/dix/main.c @@ -339,6 +339,7 @@ dix_main(int argc, char *argv[], char *envp[]) for (i = screenInfo.numGPUScreens - 1; i >= 0; i--) { ScreenPtr pScreen = screenInfo.gpuscreens[i]; FreeScratchPixmapsForScreen(pScreen); +dixFreeScreenSpecificPrivates(pScreen); (*pScreen->CloseScreen) (pScreen); dixFreePrivates(pScreen->devPrivates, PRIVATE_SCREEN); free(pScreen); diff --git a/dix/privates.c b/dix/privates.c index e03b2255b7f3..4fd8aeda1202 100644 --- a/dix/privates.c +++ b/dix/privates.c @@ -593,6 +593,26 @@ dixLookupPrivateOffset(RESTYPE type) return -1; } +static void +FreeKeyLists(DevPrivateSetRec keys[PRIVATE_LAST]) +{ +DevPrivateType t; + +for (t = PRIVATE_XSELINUX; t < PRIVATE_LAST; t++) { +DevPrivateKey key, next; + +for (key = keys[t].key; key; key = next) { +next = key->next; +key->offset = 0; +key->initialized = FALSE; +key->size = 0; +key->type = 0; +if (key->allocated) +free(key); +} +} +} + /* * Screen-specific privates */ @@ -642,6 +662,7 @@ dixRegisterScreenSpecificPrivateKey(ScreenPtr pScreen, DevPrivateKey key, void dixFreeScreenSpecificPrivates(ScreenPtr pScreen) { +FreeKeyLists(pScreen->screenSpecificPrivates); } /* Initialize screen-specific privates in AddScreen */ @@ -751,18 +772,9 @@ dixResetPrivates(void) { DevPrivateType t; -for (t = PRIVATE_XSELINUX; t < PRIVATE_LAST; t++) { -DevPrivateKey key, next; +FreeKeyLists(global_keys); -for (key = global_keys[t].key; key; key = next) { -next = key->next; -key->offset = 0; -key->initialized = FALSE; -key->size = 0; -key->type = 0; -if (key->allocated) -free(key); -} +for (t = PRIVATE_XSELINUX; t < PRIVATE_LAST; t++) { if (global_keys[t].created) { ErrorF("%d %ss still allocated at reset\n", global_keys[t].created, key_names[t]); -- 2.5.2 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Improved autoconfig drivers matching
On 07/22/2015 04:42 PM, Karol Kosik wrote: Implementation of new drivers matching algorithm. New approach doesn't add duplicate drivers and ease drivers matching phase. Signed-off-by: Karol Kosik kko...@nvidia.com Reviewed-by: Aaron Plattner aplatt...@nvidia.com though it might be good for someone else to take a look too. In case anyone was wondering why this patch helps, the problem is that xf86PlatformMatchDriver() loops over every platform device, which means that if you have a whole bunch of GPUs, it keeps adding the same drivers over and over again, overflowing the 20-element deviceList[]. --- hw/xfree86/common/xf86AutoConfig.c | 124 +++ hw/xfree86/common/xf86MatchDrivers.h | 40 +++ hw/xfree86/common/xf86pciBus.c | 52 ++- hw/xfree86/common/xf86pciBus.h | 13 ++-- hw/xfree86/common/xf86platformBus.c | 31 +++-- hw/xfree86/common/xf86platformBus.h | 5 +- 6 files changed, 150 insertions(+), 115 deletions(-) create mode 100644 hw/xfree86/common/xf86MatchDrivers.h diff --git a/hw/xfree86/common/xf86AutoConfig.c b/hw/xfree86/common/xf86AutoConfig.c index 6b8d0eb..440434c 100644 --- a/hw/xfree86/common/xf86AutoConfig.c +++ b/hw/xfree86/common/xf86AutoConfig.c @@ -37,6 +37,7 @@ #include xf86Parser.h #include xf86tokens.h #include xf86Config.h +#include xf86MatchDrivers.h #include xf86Priv.h #include xf86_OSlib.h #include xf86platformBus.h @@ -89,7 +90,7 @@ static const char **builtinConfig = NULL; static int builtinLines = 0; -static void listPossibleVideoDrivers(char *matches[], int nmatches); +static void listPossibleVideoDrivers(XF86MatchedDrivers *md); /* * A built-in config file is stored as an array of strings, with each string @@ -140,33 +141,58 @@ AppendToConfig(const char *s) AppendToList(s, builtinConfig, builtinLines); } +void +xf86AddMatchedDriver(XF86MatchedDrivers *md, const char *driver) +{ +int j; +int nmatches = md-nmatches; + +for (j = 0; j nmatches; ++j) { +if (xf86NameCmp(md-matches[j], driver) == 0) { +// Driver already in matched drivers +return; +} +} + +if (nmatches MATCH_DRIVERS_LIMIT) { +md-matches[nmatches] = xnfstrdup(driver); +md-nmatches++; +} +else { +xf86Msg(X_WARNING, Too many drivers registered, can't add %s\n, driver); +} +} + Bool xf86AutoConfig(void) { -char *deviceList[20]; -char **p; +XF86MatchedDrivers md; +int i; const char **cp; char buf[1024]; ConfigStatus ret; -listPossibleVideoDrivers(deviceList, 20); +listPossibleVideoDrivers(md); -for (p = deviceList; *p; p++) { -snprintf(buf, sizeof(buf), BUILTIN_DEVICE_SECTION, *p, 0, *p); +for (i = 0; i md.nmatches; i++) { +snprintf(buf, sizeof(buf), BUILTIN_DEVICE_SECTION, +md.matches[i], 0, md.matches[i]); AppendToConfig(buf); -snprintf(buf, sizeof(buf), BUILTIN_SCREEN_SECTION, *p, 0, *p, 0); +snprintf(buf, sizeof(buf), BUILTIN_SCREEN_SECTION, +md.matches[i], 0, md.matches[i], 0); AppendToConfig(buf); } AppendToConfig(BUILTIN_LAYOUT_SECTION_PRE); -for (p = deviceList; *p; p++) { -snprintf(buf, sizeof(buf), BUILTIN_LAYOUT_SCREEN_LINE, *p, 0); +for (i = 0; i md.nmatches; i++) { +snprintf(buf, sizeof(buf), BUILTIN_LAYOUT_SCREEN_LINE, +md.matches[i], 0); AppendToConfig(buf); } AppendToConfig(BUILTIN_LAYOUT_SECTION_POST); -for (p = deviceList; *p; p++) { -free(*p); +for (i = 0; i md.nmatches; i++) { +free(md.matches[i]); } xf86MsgVerb(X_DEFAULT, 0, @@ -190,22 +216,19 @@ xf86AutoConfig(void) } static void -listPossibleVideoDrivers(char *matches[], int nmatches) +listPossibleVideoDrivers(XF86MatchedDrivers *md) { int i; -for (i = 0; i nmatches; i++) { -matches[i] = NULL; -} -i = 0; +md-nmatches = 0; #ifdef XSERVER_PLATFORM_BUS -i = xf86PlatformMatchDriver(matches, nmatches); +xf86PlatformMatchDriver(md); #endif #ifdef sun /* Check for driver type based on /dev/fb type and if valid, use it instead of PCI bus probe results */ -if (xf86Info.consoleFd = 0 (i (nmatches - 1))) { +if (xf86Info.consoleFd = 0) { struct vis_identifier visid; const char *cp; int iret; @@ -231,7 +254,7 @@ listPossibleVideoDrivers(char *matches[], int nmatches) /* Special case from before the general case was set */ if (strcmp(visid.name, NVDAnvda) == 0) { -matches[i++] = xnfstrdup(nvidia); +xf86AddMatchedDriver(md, nvidia); } /* General case - split into vendor name (initial all
Re: [PATCH 2/7] fb: Make rootless-agnostic
On 10/17/2013 10:30 AM, Adam Jackson wrote: Reviewed-by: Jasper St. Pierre jstpie...@mecheye.net Signed-off-by: Adam Jackson a...@redhat.com We just tracked down a bug to this ROOTLESS define, so Reviewed-by: Aaron Plattner aplatt...@nvidia.com Karol is working on verifying that this patch fixes it, so he should be able to provide a Tested-by line too. -- Aaron --- fb/fb.h | 5 - 1 file changed, 5 deletions(-) diff --git a/fb/fb.h b/fb/fb.h index 26957df..08d7e69 100644 --- a/fb/fb.h +++ b/fb/fb.h @@ -642,13 +642,8 @@ typedef struct { #define fbGetWindowPixmap(pWin) ((PixmapPtr)\ dixLookupPrivate(((WindowPtr)(pWin))-devPrivates, fbGetWinPrivateKey(pWin))) -#ifdef ROOTLESS #define __fbPixDrawableX(pPix) ((pPix)-drawable.x) #define __fbPixDrawableY(pPix) ((pPix)-drawable.y) -#else -#define __fbPixDrawableX(pPix) 0 -#define __fbPixDrawableY(pPix) 0 -#endif #ifdef COMPOSITE #define __fbPixOffXWin(pPix) (__fbPixDrawableX(pPix) - (pPix)-screen_x) ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/7] fb: Make rootless-agnostic
Heh, looks like this got merged with commit 2fcfa532532fbe4a7f668556808e6245ff4e36bc (pulling in 315661a425018a546f7bcc18ad3e5f4578473ca6) just now. So there you go. :) On 07/17/2015 10:26 AM, Karol Kosik wrote: Tested-by: Karol Kosik kko...@nvidia.com nvpublic -Original Message- From: Aaron Plattner Sent: Friday, July 17, 2015 9:44 AM To: Adam Jackson; xorg-devel@lists.x.org Cc: Karol Kosik Subject: Re: [PATCH 2/7] fb: Make rootless-agnostic On 10/17/2013 10:30 AM, Adam Jackson wrote: Reviewed-by: Jasper St. Pierre jstpie...@mecheye.net Signed-off-by: Adam Jackson a...@redhat.com We just tracked down a bug to this ROOTLESS define, so Reviewed-by: Aaron Plattner aplatt...@nvidia.com Karol is working on verifying that this patch fixes it, so he should be able to provide a Tested-by line too. -- Aaron --- fb/fb.h | 5 - 1 file changed, 5 deletions(-) diff --git a/fb/fb.h b/fb/fb.h index 26957df..08d7e69 100644 --- a/fb/fb.h +++ b/fb/fb.h @@ -642,13 +642,8 @@ typedef struct { #define fbGetWindowPixmap(pWin) ((PixmapPtr)\ dixLookupPrivate(((WindowPtr)(pWin))-devPrivates, fbGetWinPrivateKey(pWin))) -#ifdef ROOTLESS #define __fbPixDrawableX(pPix) ((pPix)-drawable.x) #define __fbPixDrawableY(pPix) ((pPix)-drawable.y) -#else -#define __fbPixDrawableX(pPix) 0 -#define __fbPixDrawableY(pPix) 0 -#endif #ifdef COMPOSITE #define __fbPixOffXWin(pPix)(__fbPixDrawableX(pPix) - (pPix)-screen_x) -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] cursor: add hw cursor support for prime (v2)
On 07/14/2015 05:15 PM, Dave Airlie wrote: Currently with PRIME if we detect a secondary GPU, we switch to using SW cursors, this isn't optimal, esp for the intel/nvidia combinations, we have no choice for the USB offload devices. This patch checks on each slave screen if hw cursors are enabled, and also calls set cursor and move cursor on all screens. v2: hotplugging causes the driver to try and register the cursor private, however we can't register these at runtime yet, we need to add support for reallocation of cursor screen privates. However all hotplugged devices currently don't support HW cursors, so punt for now. This means GPUs already in the system will get hw cursors and USB ones won't which is all we care about presently. drop list empty checks (Eric) fixup comments (Michel) Signed-off-by: Dave Airlie airl...@redhat.com --- dix/dispatch.c | 3 ++ dix/privates.c | 9 + hw/xfree86/ramdac/xf86Cursor.c | 12 +- hw/xfree86/ramdac/xf86CursorPriv.h | 1 + hw/xfree86/ramdac/xf86HWCurs.c | 83 -- include/privates.h | 3 ++ 6 files changed, 98 insertions(+), 13 deletions(-) diff --git a/dix/dispatch.c b/dix/dispatch.c index 9208582..0f4354d 100644 --- a/dix/dispatch.c +++ b/dix/dispatch.c @@ -3920,6 +3920,9 @@ AddGPUScreen(Bool (*pfnInit) (ScreenPtr /*pScreen */ , update_desktop_dimensions(); +if (!dixPrivatesCreated(PRIVATE_CURSOR)) +dixRegisterScreenPrivateKey(cursorScreenDevPriv, pScreen, PRIVATE_CURSOR, +0); return i; } diff --git a/dix/privates.c b/dix/privates.c index e03b225..b638d23 100644 --- a/dix/privates.c +++ b/dix/privates.c @@ -774,3 +774,12 @@ dixResetPrivates(void) global_keys[t].allocated = 0; } } + +Bool +dixPrivatesCreated(DevPrivateType type) +{ +if (global_keys[type].created) +return TRUE; +else +return FALSE; +} diff --git a/hw/xfree86/ramdac/xf86Cursor.c b/hw/xfree86/ramdac/xf86Cursor.c index 2a54571..175bed7 100644 --- a/hw/xfree86/ramdac/xf86Cursor.c +++ b/hw/xfree86/ramdac/xf86Cursor.c @@ -337,17 +337,9 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs, return; } -if (infoPtr-pScrn-vtSema xorg_list_is_empty(pScreen-pixmap_dirty_list) +if (infoPtr-pScrn-vtSema (ScreenPriv-ForceHWCursorCount || - (( - cursor-bits-argb - infoPtr-UseHWCursorARGB - (*infoPtr-UseHWCursorARGB)(pScreen, cursor)) || - (cursor-bits-argb == 0 - (cursor-bits-height = infoPtr-MaxHeight) - (cursor-bits-width = infoPtr-MaxWidth) - (!infoPtr-UseHWCursor || (*infoPtr-UseHWCursor) (pScreen, cursor)) { - + xf86CheckHWCursor(pScreen, cursor, infoPtr))) { if (ScreenPriv-SWCursor) /* remove the SW cursor */ (*ScreenPriv-spriteFuncs-SetCursor) (pDev, pScreen, NullCursor, x, y); diff --git a/hw/xfree86/ramdac/xf86CursorPriv.h b/hw/xfree86/ramdac/xf86CursorPriv.h index f34c1c7..397d2a1 100644 --- a/hw/xfree86/ramdac/xf86CursorPriv.h +++ b/hw/xfree86/ramdac/xf86CursorPriv.h @@ -43,6 +43,7 @@ void xf86MoveCursor(ScreenPtr pScreen, int x, int y); void xf86RecolorCursor(ScreenPtr pScreen, CursorPtr pCurs, Bool displayed); Bool xf86InitHardwareCursor(ScreenPtr pScreen, xf86CursorInfoPtr infoPtr); +Bool xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr); extern _X_EXPORT DevPrivateKeyRec xf86CursorScreenKeyRec; #define xf86CursorScreenKey (xf86CursorScreenKeyRec) diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c index 84febe0..ceddb6f 100644 --- a/hw/xfree86/ramdac/xf86HWCurs.c +++ b/hw/xfree86/ramdac/xf86HWCurs.c @@ -119,8 +119,49 @@ xf86InitHardwareCursor(ScreenPtr pScreen, xf86CursorInfoPtr infoPtr) return TRUE; } +static Bool +xf86CursorScreenCheckHW(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr) +{ +return (( + cursor-bits-argb + infoPtr-UseHWCursorARGB + (*infoPtr-UseHWCursorARGB)(pScreen, cursor)) || +(cursor-bits-argb == 0 +(cursor-bits-height = infoPtr-MaxHeight) I think this needs to be indented one more space. +(cursor-bits-width = infoPtr-MaxWidth) + (!infoPtr-UseHWCursor || (*infoPtr-UseHWCursor) (pScreen, cursor; +} + Bool -xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y) +xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr) +{ +ScreenPtr pSlave; +PixmapDirtyUpdatePtr ent; +Bool ret; +ret =
[PATCH] xfree86: Bump video driver ABI version to 20
Commit 90db5edf119187f8b1b9207c8c384d6cd7ef9edc modified the signature of StartPixmapTrackingProcPtr, so drivers implementing that need to use the updated definition. Signed-off-by: Aaron Plattner aplatt...@nvidia.com --- hw/xfree86/common/xf86Module.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/xfree86/common/xf86Module.h b/hw/xfree86/common/xf86Module.h index 66c2bb5a912f..9e5dc6dadbe5 100644 --- a/hw/xfree86/common/xf86Module.h +++ b/hw/xfree86/common/xf86Module.h @@ -80,7 +80,7 @@ typedef enum { * mask is 0x. */ #define ABI_ANSIC_VERSION SET_ABI_VERSION(0, 4) -#define ABI_VIDEODRV_VERSION SET_ABI_VERSION(19, 0) +#define ABI_VIDEODRV_VERSION SET_ABI_VERSION(20, 0) #define ABI_XINPUT_VERSION SET_ABI_VERSION(22, 1) #define ABI_EXTENSION_VERSION SET_ABI_VERSION(9, 0) #define ABI_FONT_VERSION SET_ABI_VERSION(0, 6) -- 2.4.5 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 4/6] modesetting: reverse prime support
On 06/25/2015 03:56 PM, Dave Airlie wrote: + +screenpix = screen-GetScreenPixmap(screen); +screen-width = screenpix-drawable.width = total_width; +screen-height = screenpix-drawable.height = max_height; Directly setting the width/height of a pixmap? That seems suspicious to me. Usually you're using ModifyPixmapHeader(), since you need to change other things (like the pointer/stride) at the same time. Is there an explanation for this being safe? I just pre-existing precedent for this xf86RandR12.c:xf86RandR12ScreenSetSize after it calls the driver resize hook does pScrnPix = (*pScreen-GetScreenPixmap) (pScreen); pScreen-width = pScrnPix-drawable.width = width; pScreen-height = pScrnPix-drawable.height = height; and since I'm doing pretty much the same thing resizing the screen, I used the code from there. Yeah, for things like this, ModifyPixmapHeader() doesn't really do anything useful and gets in the way when you try to do things like set devPrivate.ptr to NULL, since it just skips the pointer write in that case: if (pPixData) pPixmap-devPrivate.ptr = pPixData; Not that you *couldn't* use ModifyPixmapHeader here, though. Dave. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 01/16] cursor: drop ARGB_CURSOR
On 06/25/2015 04:51 PM, Dave Airlie wrote: From: Dave Airlie airl...@redhat.com I doubt anyone builds with this turned off or has done for a long time. It helps my eyes bleed slightly less when reading the code, I've left the define in place as some drivers use it. Reviewed-by: Alex Deucher alexander.deuc...@amd.com Signed-off-by: Dave Airlie airl...@redhat.com Yes please. Reviewed-by: Aaron Plattner aplatt...@nvidia.com FWIW, I removed the checks for these from our driver in 2008. --- dix/cursor.c | 8 hw/kdrive/ephyr/ephyrcursor.c | 4 hw/xfree86/modes/xf86Cursors.c | 8 hw/xfree86/ramdac/xf86Cursor.c | 2 -- hw/xfree86/ramdac/xf86Cursor.h | 4 hw/xfree86/ramdac/xf86HWCurs.c | 6 -- hw/xquartz/xpr/xprCursor.c | 2 -- include/cursorstr.h| 2 -- mi/midispcur.c | 20 +--- xfixes/cursor.c| 2 -- 10 files changed, 1 insertion(+), 57 deletions(-) diff --git a/dix/cursor.c b/dix/cursor.c index 1525857..e459456 100644 --- a/dix/cursor.c +++ b/dix/cursor.c @@ -80,9 +80,7 @@ FreeCursorBits(CursorBitsPtr bits) return; free(bits-source); free(bits-mask); -#ifdef ARGB_CURSOR free(bits-argb); -#endif dixFiniPrivates(bits, PRIVATE_CURSOR_BITS); if (bits-refcnt == 0) { GlyphSharePtr *prev, this; @@ -165,7 +163,6 @@ CheckForEmptyMask(CursorBitsPtr bits) while (n--) if (*(msk++) != 0) return; -#ifdef ARGB_CURSOR if (bits-argb) { CARD32 *argb = bits-argb; @@ -174,7 +171,6 @@ CheckForEmptyMask(CursorBitsPtr bits) if (*argb++ 0xff00) return; } -#endif bits-emptyMask = TRUE; } @@ -259,9 +255,7 @@ AllocARGBCursor(unsigned char *psrcbits, unsigned char *pmaskbits, dixInitPrivates(bits, bits + 1, PRIVATE_CURSOR_BITS) bits-source = psrcbits; bits-mask = pmaskbits; -#ifdef ARGB_CURSOR bits-argb = argb; -#endif bits-width = cm-width; bits-height = cm-height; bits-xhot = cm-xhot; @@ -400,9 +394,7 @@ AllocGlyphCursor(Font source, unsigned sourceChar, Font mask, unsigned maskChar, dixInitPrivates(bits, bits + 1, PRIVATE_CURSOR_BITS); bits-source = srcbits; bits-mask = mskbits; -#ifdef ARGB_CURSOR bits-argb = 0; -#endif bits-width = cm.width; bits-height = cm.height; bits-xhot = cm.xhot; diff --git a/hw/kdrive/ephyr/ephyrcursor.c b/hw/kdrive/ephyr/ephyrcursor.c index 852be33..808b3c7 100644 --- a/hw/kdrive/ephyr/ephyrcursor.c +++ b/hw/kdrive/ephyr/ephyrcursor.c @@ -100,7 +100,6 @@ ephyrRealizeCoreCursor(EphyrScrPriv *scr, CursorPtr cursor) xcb_free_pixmap(conn, mask); } -#ifdef ARGB_CURSOR static xcb_render_pictformat_t get_argb_format(void) { @@ -170,7 +169,6 @@ can_argb_cursor(void) return v-major_version == 0 v-minor_version = 5; } -#endif static Bool ephyrRealizeCursor(DeviceIntPtr dev, ScreenPtr screen, CursorPtr cursor) @@ -179,11 +177,9 @@ ephyrRealizeCursor(DeviceIntPtr dev, ScreenPtr screen, CursorPtr cursor) KdScreenInfo *kscr = pScreenPriv-screen; EphyrScrPriv *scr = kscr-driver; -#ifdef ARGB_CURSOR if (cursor-bits-argb can_argb_cursor()) ephyrRealizeARGBCursor(scr, cursor); else -#endif { ephyrRealizeCoreCursor(scr, cursor); } diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c index 379a27a..321cde7 100644 --- a/hw/xfree86/modes/xf86Cursors.c +++ b/hw/xfree86/modes/xf86Cursors.c @@ -258,9 +258,7 @@ xf86_crtc_convert_cursor_to_argb(xf86CrtcPtr crtc, unsigned char *src) CARD32 bits; const Rotation rotation = xf86_crtc_cursor_rotation(crtc); -#ifdef ARGB_CURSOR crtc-cursor_argb = FALSE; -#endif for (y = 0; y cursor_info-MaxHeight; y++) for (x = 0; x cursor_info-MaxWidth; x++) { @@ -458,9 +456,7 @@ xf86_crtc_load_cursor_image(xf86CrtcPtr crtc, CARD8 *src) CARD8 *cursor_image; const Rotation rotation = xf86_crtc_cursor_rotation(crtc); -#ifdef ARGB_CURSOR crtc-cursor_argb = FALSE; -#endif if (rotation == RR_Rotate_0) cursor_image = src; @@ -632,12 +628,10 @@ xf86_cursors_init(ScreenPtr screen, int max_width, int max_height, int flags) cursor_info-HideCursor = xf86_hide_cursors; cursor_info-ShowCursor = xf86_show_cursors; cursor_info-UseHWCursor = xf86_use_hw_cursor; -#ifdef ARGB_CURSOR if (flags HARDWARE_CURSOR_ARGB) { cursor_info-UseHWCursorARGB = xf86_use_hw_cursor_argb; cursor_info-LoadCursorARGBCheck = xf86_load_cursor_argb; } -#endif xf86_config-cursor = NULL; xf86_hide_cursors(scrn); @@ -691,11 +685,9 @@ xf86_reload_cursors(ScreenPtr screen) void *src
Re: [PATCH 02/16] xf86Rotate: remove unused macros.
On 06/25/2015 04:51 PM, Dave Airlie wrote: From: Dave Airlie airl...@redhat.com These macros aren't used anywhere. Signed-off-by: Dave Airlie airl...@redhat.com --- hw/xfree86/modes/xf86Rotate.c | 5 - 1 file changed, 5 deletions(-) diff --git a/hw/xfree86/modes/xf86Rotate.c b/hw/xfree86/modes/xf86Rotate.c index 9c00a44..01cc764 100644 --- a/hw/xfree86/modes/xf86Rotate.c +++ b/hw/xfree86/modes/xf86Rotate.c @@ -44,11 +44,6 @@ #include X11/Xatom.h /* borrowed from composite extension, move to Render and publish? */ Does this comment apply to those macros? - -#define F(x) IntToxFixed(x) - -#define toF(x) ((float) (x) / 65536.0f) - static void xf86RotateCrtcRedisplay(xf86CrtcPtr crtc, RegionPtr region) { -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: xvfb: add randr support (v2)
On 06/08/2015 03:14 PM, Siim Põder wrote: Hi This was sent to xorg-devel a few years ago. It still applies and still appears to work. I resending this because it affects me. Comments or application to the tree would be greatly appreciated :) The motivation for getting this is chrome remote desktop that runs under Xvfb and wants to use RANDR to adjust screen size according to the remote desktop client screen size. Apparently there are other use cases as well, the bug mentions gnome-settings-daemon testing. Not that this patch hurts or anything, but is there any particular reason this remote desktop thing is using Xvfb rather than Xorg + xf86-video-dummy? I thought there was an effort to kill off the redundant non-Xorg DDXes at some point. I posted a patch series a while back to upgrade the dummy driver to be able to support arbitrary resizing, including resizing to larger than you started with: http://lists.x.org/archives/xorg-devel/2015-January/045395.html Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=26391 Signed-off-by: Lambros Lambrou lambroslamb...@google.com Signed-off-by: Mike Frysinger vap...@gentoo.org Signed-off-by: Michal Srb m...@suse.com Signed-off-by: Siim Põder s...@p6drad-teel.net --- Second version, modified according to Keith suggestion. Tested by adding second mode and switching - worked correctly. diff --git a/hw/vfb/InitOutput.c b/hw/vfb/InitOutput.c index 97eccfd..bfca068 100644 --- a/hw/vfb/InitOutput.c +++ b/hw/vfb/InitOutput.c @@ -66,6 +66,7 @@ from The Open Group. #include dix.h #include miline.h #include glx_extinit.h +#include randrstr.h #define VFB_DEFAULT_WIDTH 1280 #define VFB_DEFAULT_HEIGHT 1024 @@ -785,6 +786,125 @@ vfbCloseScreen(ScreenPtr pScreen) } static Bool +vfbRROutputValidateMode(ScreenPtr pScreen, +RROutputPtr output, +RRModePtr mode) +{ +rrScrPriv(pScreen); + +if (pScrPriv-minWidth = mode-mode.width +pScrPriv-maxWidth = mode-mode.width +pScrPriv-minHeight = mode-mode.height +pScrPriv-maxHeight = mode-mode.height) +return TRUE; +else +return FALSE; +} + +static Bool +vfbRRScreenSetSize(ScreenPtr pScreen, + CARD16 width, + CARD16 height, + CARD32 mmWidth, + CARD32 mmHeight) +{ +// Prevent screen updates while we change things around +SetRootClip(pScreen, FALSE); + +pScreen-width = width; +pScreen-height = height; +pScreen-mmWidth = mmWidth; +pScreen-mmHeight = mmHeight; + +// Restore the ability to update screen, now with new dimensions +SetRootClip(pScreen, TRUE); + +RRScreenSizeNotify (pScreen); +RRTellChanged(pScreen); + +return TRUE; +} + +static Bool +vfbRRCrtcSet(ScreenPtr pScreen, + RRCrtcPtr crtc, + RRModePtr mode, + int x, + int y, + Rotation rotation, + int numOutput, + RROutputPtr *outputs) +{ + return RRCrtcNotify(crtc, mode, x, y, rotation, NULL, numOutput, outputs); +} + +static Bool +vfbRRGetInfo(ScreenPtr pScreen, Rotation *rotations) +{ +return TRUE; +} + +static Bool +vfbRandRInit(ScreenPtr pScreen) +{ +rrScrPrivPtr pScrPriv; +#if RANDR_12_INTERFACE +RRModePtr mode; +RRCrtcPtr crtc; +RROutputPtroutput; +xRRModeInfo modeInfo; +char name[64]; +#endif + +if (!RRScreenInit (pScreen)) + return FALSE; +pScrPriv = rrGetScrPriv(pScreen); +pScrPriv-rrGetInfo = vfbRRGetInfo; +#if RANDR_12_INTERFACE +pScrPriv-rrCrtcSet = vfbRRCrtcSet; +pScrPriv-rrScreenSetSize = vfbRRScreenSetSize; +pScrPriv-rrOutputSetProperty = NULL; +#if RANDR_13_INTERFACE +pScrPriv-rrOutputGetProperty = NULL; +#endif +pScrPriv-rrOutputValidateMode = vfbRROutputValidateMode; +pScrPriv-rrModeDestroy = NULL; + +RRScreenSetSizeRange (pScreen, + 1, 1, + pScreen-width, pScreen-height); + +sprintf (name, %dx%d, pScreen-width, pScreen-height); +memset (modeInfo, '\0', sizeof (modeInfo)); +modeInfo.width = pScreen-width; +modeInfo.height = pScreen-height; +modeInfo.nameLength = strlen (name); + +mode = RRModeGet (modeInfo, name); +if (!mode) + return FALSE; + +crtc = RRCrtcCreate (pScreen, NULL); +if (!crtc) + return FALSE; + +output = RROutputCreate (pScreen, screen, 6, NULL); +if (!output) + return FALSE; +if (!RROutputSetClones (output, NULL, 0)) + return FALSE; +if (!RROutputSetModes (output, mode, 1, 0)) + return FALSE; +if (!RROutputSetCrtcs (output, crtc, 1)) + return FALSE; +if (!RROutputSetConnection (output,
Re: [PATCH 1/8] dix: Unexport various implementation details
On 06/02/2015 11:14 AM, Adam Jackson wrote: Signed-off-by: Adam Jackson a...@redhat.com --- dix/colormap.c| 337 +- dix/dispatch.c| 1 + dix/dixfonts.c| 12 +- dix/enterleave.c | 2 +- dix/enterleave.h | 2 - dix/main.c| 2 + hw/xfree86/sdksyms.sh | 2 - include/Makefile.am | 4 +- include/colormap.h| 12 -- include/dixfont.h | 35 -- include/dixstruct.h | 27 ++-- include/swaprep.h | 320 +++ include/swapreq.h | 6 +- mi/miglblt.c | 1 + miext/damage/damage.c | 1 + os/utils.c| 1 + 16 files changed, 360 insertions(+), 405 deletions(-) [...] -extern _X_EXPORT int (*ProcVector[256]) (ClientPtr /*client */ ); +extern int (*ProcVector[256]) (ClientPtr /*client */ ); -extern _X_EXPORT int (*SwappedProcVector[256]) (ClientPtr /*client */ ); +extern int (*SwappedProcVector[256]) (ClientPtr /*client */ ); Sadly, we wrap a few requests for stupid reasons that I don't want to get into, so we need these. I don't think we use anything else this patch unexports. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 3/8] render: Hide/unexport some implementation details
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 06/02/2015 11:15 AM, Adam Jackson wrote: Signed-off-by: Adam Jackson a...@redhat.com I think these are all fine. Reviewed-by: Aaron Plattner aplatt...@nvidia.com - -- Aaron --- render/glyph.c | 10 +- render/glyphstr.h | 36 ++-- render/mipict.c | 16 render/mipict.h | 36 render/picture.c| 24 render/picture.h| 6 +++--- render/picturestr.h | 50 +- 7 files changed, 43 insertions(+), 135 deletions(-) diff --git a/render/glyph.c b/render/glyph.c index f3310db..ea865af 100644 --- a/render/glyph.c +++ b/render/glyph.c @@ -106,7 +106,7 @@ GlyphUninit(ScreenPtr pScreen) } } -GlyphHashSetPtr +static GlyphHashSetPtr FindGlyphHashSet(CARD32 filled) { int i; @@ -117,7 +117,7 @@ FindGlyphHashSet(CARD32 filled) return 0; } -GlyphRefPtr +static GlyphRefPtr FindGlyphRef(GlyphHashPtr hash, CARD32 signature, Bool match, unsigned char sha1[20]) { @@ -245,7 +245,7 @@ FreeGlyphPicture(GlyphPtr glyph) } } -void +static void FreeGlyph(GlyphPtr glyph, int format) { CheckDuplicates(globalGlyphs[format], FreeGlyph); @@ -383,7 +383,7 @@ AllocateGlyph(xGlyphInfo * gi, int fdepth) return 0; } -Bool +static Bool AllocateGlyphHash(GlyphHashPtr hash, GlyphHashSetPtr hashSet) { hash-table = calloc(hashSet-size, sizeof(GlyphRefRec)); @@ -394,7 +394,7 @@ AllocateGlyphHash(GlyphHashPtr hash, GlyphHashSetPtr hashSet) return TRUE; } -Bool +static Bool ResizeGlyphHash(GlyphHashPtr hash, CARD32 change, Bool global) { CARD32 tableEntries; diff --git a/render/glyphstr.h b/render/glyphstr.h index 2df055d..2f51bd2 100644 --- a/render/glyphstr.h +++ b/render/glyphstr.h @@ -90,47 +90,31 @@ typedef struct _GlyphList { PictFormatPtr format; } GlyphListRec, *GlyphListPtr; -extern _X_EXPORT void +extern void GlyphUninit(ScreenPtr pScreen); -extern _X_EXPORT GlyphHashSetPtr FindGlyphHashSet(CARD32 filled); - -extern _X_EXPORT GlyphRefPtr -FindGlyphRef(GlyphHashPtr hash, - CARD32 signature, Bool match, unsigned char sha1[20]); - -extern _X_EXPORT GlyphPtr FindGlyphByHash(unsigned char sha1[20], int format); - -extern _X_EXPORT int +extern GlyphPtr FindGlyphByHash(unsigned char sha1[20], int format); +extern int HashGlyph(xGlyphInfo * gi, CARD8 *bits, unsigned long size, unsigned char sha1[20]); -extern _X_EXPORT void - FreeGlyph(GlyphPtr glyph, int format); - -extern _X_EXPORT void +extern void AddGlyph(GlyphSetPtr glyphSet, GlyphPtr glyph, Glyph id); -extern _X_EXPORT Bool +extern Bool DeleteGlyph(GlyphSetPtr glyphSet, Glyph id); -extern _X_EXPORT GlyphPtr FindGlyph(GlyphSetPtr glyphSet, Glyph id); +extern GlyphPtr FindGlyph(GlyphSetPtr glyphSet, Glyph id); -extern _X_EXPORT GlyphPtr AllocateGlyph(xGlyphInfo * gi, int format); +extern GlyphPtr AllocateGlyph(xGlyphInfo * gi, int format); -extern _X_EXPORT Bool - AllocateGlyphHash(GlyphHashPtr hash, GlyphHashSetPtr hashSet); - -extern _X_EXPORT Bool - ResizeGlyphHash(GlyphHashPtr hash, CARD32 change, Bool global); - -extern _X_EXPORT Bool +extern Bool ResizeGlyphSet(GlyphSetPtr glyphSet, CARD32 change); -extern _X_EXPORT GlyphSetPtr AllocateGlyphSet(int fdepth, PictFormatPtr format); +extern GlyphSetPtr AllocateGlyphSet(int fdepth, PictFormatPtr format); -extern _X_EXPORT int +extern int FreeGlyphSet(void *value, XID gid); #define GLYPH_HAS_GLYPH_PICTURE_ACCESSOR 1 /* used for api compat */ diff --git a/render/mipict.c b/render/mipict.c index 2571fda..4b85512 100644 --- a/render/mipict.c +++ b/render/mipict.c @@ -46,7 +46,7 @@ miDestroyPicture(PicturePtr pPicture) RegionDestroy(pPicture-pCompositeClip); } -void +static void miDestroyPictureClip(PicturePtr pPicture) { if (pPicture-clientClip) @@ -54,7 +54,7 @@ miDestroyPictureClip(PicturePtr pPicture) pPicture-clientClip = NULL; } -int +static int miChangePictureClip(PicturePtr pPicture, int type, void *value, int n) { ScreenPtr pScreen = pPicture-pDrawable-pScreen; @@ -88,13 +88,13 @@ miChangePictureClip(PicturePtr pPicture, int type, void *value, int n) return Success; } -void +static void miChangePicture(PicturePtr pPicture, Mask mask) { return; } -void +static void miValidatePicture(PicturePtr pPicture, Mask mask) { DrawablePtr pDrawable = pPicture-pDrawable; @@ -211,13 +211,13 @@ miValidatePicture(PicturePtr pPicture, Mask mask) } } -int +static int miChangePictureTransform(PicturePtr pPicture, PictTransform * transform) { return Success; } -int +static int miChangePictureFilter(PicturePtr pPicture, int filter, xFixed * params, int nparams) { @@ -499,7 +499,7 @@ miRenderPixelToColor(PictFormatPtr format, CARD32 pixel, xRenderColor * color) } } -void +static void miTriStrip(CARD8 op, PicturePtr pSrc, PicturePtr pDst, @@ -523,7 +523,7 @@ miTriStrip(CARD8 op, free(tris
Re: [PATCH 2/8] randr: Unexport some implementation details
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 06/02/2015 11:15 AM, Adam Jackson wrote: Signed-off-by: Adam Jackson a...@redhat.com --- randr/randrstr.h | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/randr/randrstr.h b/randr/randrstr.h index 03974fd..58f3ff4 100644 --- a/randr/randrstr.h +++ b/randr/randrstr.h @@ -64,10 +64,10 @@ typedef XID RROutput; typedef XID RRCrtc; typedef XID RRProvider; -extern _X_EXPORT int RREventBase, RRErrorBase; +extern int RREventBase, RRErrorBase; -extern _X_EXPORT int (*ProcRandrVector[RRNumberRequests]) (ClientPtr); -extern _X_EXPORT int (*SProcRandrVector[RRNumberRequests]) (ClientPtr); +extern int (*ProcRandrVector[RRNumberRequests]) (ClientPtr); +extern int (*SProcRandrVector[RRNumberRequests]) (ClientPtr); /* * Modeline for a monitor. Name follows directly after this struct @@ -397,11 +397,11 @@ typedef struct _RRClient { /* RRTimesRectimes[0]; */ } RRClientRec, *RRClientPtr; -extern _X_EXPORT RESTYPE RRClientType, RREventType; /* resource types for event masks */ -extern _X_EXPORT DevPrivateKeyRec RRClientPrivateKeyRec; +extern RESTYPE RRClientType, RREventType; /* resource types for event masks */ +extern DevPrivateKeyRec RRClientPrivateKeyRec; #define RRClientPrivateKey (RRClientPrivateKeyRec) -extern _X_EXPORT RESTYPE RRCrtcType, RRModeType, RROutputType, RRProviderType; +extern RESTYPE RRCrtcType, RRModeType, RROutputType, RRProviderType; We need RRCrtcType, RRModeType, and RROutputType so we can look those things up in one of the aforementioned protocol wrappers. #define VERIFY_RR_OUTPUT(id, ptr, a)\ {\ -BEGIN PGP SIGNATURE- Version: GnuPG v2 iQIcBAEBCAAGBQJVdjkDAAoJEBvvPYQBpohhbhIP/1jULb5eTUfdFqusSxwPQ8Cb BT43+WsvB2GQCyMd6b+heXYz41vq+o62wboOxil2jyXTNWt8MlwlwJ5VGJVT3Tas P2/k0QcikTLVikQLkORVQgxoqlEda09YqhBDBkyvANJh6b0POZg095+LwMphHleO tsag+4v89uF4ysECcaAIw5OLu/u/pFMpKkHTQsUo82Cr1J1V6ikmyu97Hq4UlOj8 MYfA/sC6XTFVb0GeVWstok7LIXYmiXJFoAaddhHsojbK4ZXzEitdNSKvxiHmdAik tq4plj+ubiodShX2sDCXfa078Cb62BR9c/BPLrdGv/K0+SPJp90+JWc5IxyynFV5 a14pt6s2lf/XZIiE821LMWY8a4HSwTehgXpjtH7x5NlpLCyU4hmiBdbo1D7meRAf GoRc/iLNXnhrE/QnR6Kdd7aLCU86cwuPsLgKXkU6fVBmE6T6wvZaRX+ityCBFHe3 iVXwlWButjyKXtfulr7IUNwKC4fS9P1f2pKklUp5P8Wm2vOj52BlZP0Ps1q8HdIr L1xdkgUVKYuJESthh65DsdFLfddZZYRBvS/uLKM1eIXxyCd9plucliEE5lgO+SO/ QKhyoxv9VwIuLQuv5/CDB5w0p4qto4LCcfqFoqRe25WoBHNB4vvrnFjL2Fl77IdA 7mB/ZrmXfKCbQ6S69GUE =G8dd -END PGP SIGNATURE- ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 5/8] dga: Hide a bunch of implementation details
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 06/02/2015 11:15 AM, Adam Jackson wrote: Signed-off-by: Adam Jackson a...@redhat.com I sure wish DGA would just die already. Reviewed-by: Aaron Plattner aplatt...@nvidia.com - -- Aaron --- hw/xfree86/common/dgaproc.h | 62 - hw/xfree86/common/xf86DGA.c | 36 +- hw/xfree86/sdksyms.sh | 6 - 3 files changed, 28 insertions(+), 76 deletions(-) diff --git a/hw/xfree86/common/dgaproc.h b/hw/xfree86/common/dgaproc.h index 87e923f..7925bd4 100644 --- a/hw/xfree86/common/dgaproc.h +++ b/hw/xfree86/common/dgaproc.h @@ -54,57 +54,15 @@ typedef struct { /* DDX interface */ -extern _X_EXPORT int - DGASetMode(int Index, int num, XDGAModePtr mode, PixmapPtr *pPix); - -extern _X_EXPORT void - DGASetInputMode(int Index, Bool keyboard, Bool mouse); - -extern _X_EXPORT void - DGASelectInput(int Index, ClientPtr client, long mask); - -extern _X_EXPORT Bool DGAAvailable(int Index); -extern _X_EXPORT Bool DGAScreenAvailable(ScreenPtr pScreen); -extern _X_EXPORT Bool DGAActive(int Index); -extern _X_EXPORT void DGAShutdown(void); -extern _X_EXPORT void DGAInstallCmap(ColormapPtr cmap); -extern _X_EXPORT int DGAGetViewportStatus(int Index); -extern _X_EXPORT int DGASync(int Index); - -extern _X_EXPORT int - DGAFillRect(int Index, int x, int y, int w, int h, unsigned long color); - -extern _X_EXPORT int - DGABlitRect(int Index, int srcx, int srcy, int w, int h, int dstx, int dsty); - -extern _X_EXPORT int - -DGABlitTransRect(int Index, - int srcx, int srcy, - int w, int h, int dstx, int dsty, unsigned long color); - -extern _X_EXPORT int - DGASetViewport(int Index, int x, int y, int mode); - -extern _X_EXPORT int DGAGetModes(int Index); -extern _X_EXPORT int DGAGetOldDGAMode(int Index); - -extern _X_EXPORT int DGAGetModeInfo(int Index, XDGAModePtr mode, int num); - -extern _X_EXPORT Bool DGAVTSwitch(void); -extern _X_EXPORT Bool DGAStealButtonEvent(DeviceIntPtr dev, int Index, - int button, int is_down); -extern _X_EXPORT Bool DGAStealMotionEvent(DeviceIntPtr dev, int Index, int dx, - int dy); -extern _X_EXPORT Bool DGAStealKeyEvent(DeviceIntPtr dev, int Index, - int key_code, int is_down); - -extern _X_EXPORT Bool DGAOpenFramebuffer(int Index, char **name, - unsigned char **mem, int *size, - int *offset, int *flags); -extern _X_EXPORT void DGACloseFramebuffer(int Index); -extern _X_EXPORT Bool DGAChangePixmapMode(int Index, int *x, int *y, int mode); -extern _X_EXPORT int DGACreateColormap(int Index, ClientPtr client, int id, - int mode, int alloc); +extern Bool DGAScreenAvailable(ScreenPtr pScreen); +extern Bool DGAActive(int Index); +extern void DGAShutdown(void); + +extern Bool DGAVTSwitch(void); +extern Bool DGAStealButtonEvent(DeviceIntPtr dev, int Index, + int button, int is_down); +extern Bool DGAStealMotionEvent(DeviceIntPtr dev, int Index, int dx, int dy); +extern Bool DGAStealKeyEvent(DeviceIntPtr dev, int Index, + int key_code, int is_down); #endif /* __DGAPROC_H */ diff --git a/hw/xfree86/common/xf86DGA.c b/hw/xfree86/common/xf86DGA.c index 9533e1c..c689dcb 100644 --- a/hw/xfree86/common/xf86DGA.c +++ b/hw/xfree86/common/xf86DGA.c @@ -471,7 +471,7 @@ xf86SetDGAMode(ScrnInfoPtr pScrn, int num, DGADevicePtr devRet) /*** exported ones ***/ -void +static void DGASetInputMode(int index, Bool keyboard, Bool mouse) { ScreenPtr pScreen = screenInfo.screens[index]; @@ -488,7 +488,7 @@ DGASetInputMode(int index, Bool keyboard, Bool mouse) } } -Bool +static Bool DGAChangePixmapMode(int index, int *x, int *y, int mode) { DGAScreenPtr pScreenPriv; @@ -560,7 +560,7 @@ DGAScreenAvailable(ScreenPtr pScreen) return FALSE; } -Bool +static Bool DGAAvailable(int index) { ScreenPtr pScreen; @@ -606,7 +606,7 @@ DGAShutdown(void) /* Called by the extension to initialize a mode */ -int +static int DGASetMode(int index, int num, XDGAModePtr mode, PixmapPtr *pPix) { ScrnInfoPtr pScrn = xf86Screens[index]; @@ -626,7 +626,7 @@ DGASetMode(int index, int num, XDGAModePtr mode, PixmapPtr *pPix) /* Called from the extension to let the DDX know which events are requested */ -void +static void DGASelectInput(int index, ClientPtr client, long mask) { DGAScreenPtr pScreenPriv = DGA_GET_SCREEN_PRIV(screenInfo.screens[index]); @@ -636,7 +636,7 @@ DGASelectInput(int index, ClientPtr client, long mask) pScreenPriv-input = mask; } -int +static int DGAGetViewportStatus(int index) { DGAScreenPtr pScreenPriv = DGA_GET_SCREEN_PRIV(screenInfo.screens[index]); @@ -649,7 +649,7 @@ DGAGetViewportStatus(int index) return (*pScreenPriv-funcs-GetViewport) (pScreenPriv-pScrn); } -int +static int DGASetViewport(int index, int x, int y, int mode) { DGAScreenPtr pScreenPriv
Re: [PATCH 4/8] xfree86: Hide some pre-randr mode validation details
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 06/02/2015 11:15 AM, Adam Jackson wrote: Signed-off-by: Adam Jackson a...@redhat.com I'm happy to say that we don't use any of these, so Reviewed-by: Aaron Plattner aplatt...@nvidia.com - -- Aaron --- hw/xfree86/common/xf86.h | 11 --- hw/xfree86/common/xf86Mode.c | 11 +++ hw/xfree86/doc/ddxDesign.xml | 75 3 files changed, 5 insertions(+), 92 deletions(-) diff --git a/hw/xfree86/common/xf86.h b/hw/xfree86/common/xf86.h index 49ff35b..1cde478 100644 --- a/hw/xfree86/common/xf86.h +++ b/hw/xfree86/common/xf86.h @@ -368,22 +368,11 @@ xf86GetBppFromDepth(ScrnInfoPtr pScrn, int depth); /* xf86Mode.c */ -extern _X_EXPORT int -xf86GetNearestClock(ScrnInfoPtr scrp, int freq, Bool allowDiv2, -int DivFactor, int MulFactor, int *divider); extern _X_EXPORT const char * xf86ModeStatusToString(ModeStatus status); extern _X_EXPORT ModeStatus -xf86LookupMode(ScrnInfoPtr scrp, DisplayModePtr modep, - ClockRangePtr clockRanges, LookupModeFlags strategy); -extern _X_EXPORT ModeStatus xf86CheckModeForMonitor(DisplayModePtr mode, MonPtr monitor); extern _X_EXPORT ModeStatus -xf86InitialCheckModeForDriver(ScrnInfoPtr scrp, DisplayModePtr mode, - ClockRangePtr clockRanges, - LookupModeFlags strategy, - int maxPitch, int virtualX, int virtualY); -extern _X_EXPORT ModeStatus xf86CheckModeForDriver(ScrnInfoPtr scrp, DisplayModePtr mode, int flags); extern _X_EXPORT int xf86ValidateModes(ScrnInfoPtr scrp, DisplayModePtr availModes, diff --git a/hw/xfree86/common/xf86Mode.c b/hw/xfree86/common/xf86Mode.c index 9a5550f..3f446ae 100644 --- a/hw/xfree86/common/xf86Mode.c +++ b/hw/xfree86/common/xf86Mode.c @@ -112,11 +112,10 @@ printModeRejectMessage(int index, DisplayModePtr p, int status) } /* - * xf86GetNearestClock -- - * Find closest clock to given frequency (in kHz). This assumes the - * number of clocks is greater than zero. + * Find closest clock to given frequency (in kHz). This assumes the + * number of clocks is greater than zero. */ -int +static int xf86GetNearestClock(ScrnInfoPtr scrp, int freq, Bool allowDiv2, int DivFactor, int MulFactor, int *divider) { @@ -451,7 +450,7 @@ xf86HandleBuiltinMode(ScrnInfoPtr scrp, * reason. */ -ModeStatus +static ModeStatus xf86LookupMode(ScrnInfoPtr scrp, DisplayModePtr modep, ClockRangePtr clockRanges, LookupModeFlags strategy) { @@ -845,7 +844,7 @@ xf86CheckModeSize(ScrnInfoPtr scrp, int w, int x, int y) *maxVValuemaximum vertical timing value */ -ModeStatus +static ModeStatus xf86InitialCheckModeForDriver(ScrnInfoPtr scrp, DisplayModePtr mode, ClockRangePtr clockRanges, LookupModeFlags strategy, diff --git a/hw/xfree86/doc/ddxDesign.xml b/hw/xfree86/doc/ddxDesign.xml index 6a9de9e..aed77a9 100644 --- a/hw/xfree86/doc/ddxDesign.xml +++ b/hw/xfree86/doc/ddxDesign.xml @@ -6957,28 +6957,6 @@ use of some of these secondary mode helper functions. blockquotepara programlisting -int xf86GetNearestClock(ScrnInfoPtr scrp, int freq, Bool allowDiv2, - int *divider); -/programlisting - blockquotepara - This function returns the index of the closest clock to the - frequency parameterfreq/parameter given (in kHz). It assumes that - the number of clocks is greater than zero. It requires that the - structfieldnumClocks/structfield and structfieldclock/structfield fields of the - structnameScrnInfoRec/structname are initialised. The - structfieldallowDiv2/structfield field determines if the clocks can be - halved. The parameter*divider/parameter return value indicates - whether clock division is used when determining the clock returned. - /para - - para - This function is only for non-programmable clocks. - /para - - /blockquote/para/blockquote - - blockquotepara - programlisting const char *xf86ModeStatusToString(ModeStatus status); /programlisting blockquotepara @@ -6990,59 +6968,6 @@ use of some of these secondary mode helper functions. blockquotepara programlisting -ModeStatus xf86LookupMode(ScrnInfoPtr scrp, DisplayModePtr modep, - ClockRangePtr clockRanges, LookupModeFlags strategy); - /programlisting - blockquotepara - This function takes a pointer to a mode with the name filled in, - and looks for a mode in the structfieldmodePool/structfield list which - matches. The parameters of the matching mode are filled in to - parameter*modep/parameter. The parameterclockRanges/parameter and - parameterstrategy/parameter parameters are as for the - functionxf86ValidateModes()/function function above. - /para - - para - This function requires the structfieldmodePool/structfield, - structfieldclock[]/structfield, structfieldnumClocks/structfield and - structfieldprogClock
Re: [PATCH v2 2/2] systemd-logind: Only use systemd-logind integration together with keeptty
On 05/16/2015 01:03 AM, Hans de Goede wrote: Hi, On 15-05-15 22:21, Aaron Plattner wrote: On 04/30/2015 05:24 AM, Hans de Goede wrote: systemd-logind integration does not work when starting X on a new tty, as that detaches X from the current session and after hat systemd-logind revokes all rights any already open fds and refuses to open new fds for X. This means that currently e.g. startx -- vt7 breaks, and breaks badly, requiring ssh access to the system to kill X. The fix for this is easy, we must not use systemd-logind integration when not using KeepTty, or iow we may only use systemd-logind integration together with KeepTty. Signed-off-by: Hans de Goede hdego...@redhat.com I can confirm that this fixes VT switching for X servers started from an SSH session on Arch Linux, which I use all the time for debugging. So Tested-by: Aaron Plattner aplatt...@nvidia.com Thanks, note that testing in Fedora has found a minor issue when running headless (with the dummy driver), so a v3 of the patch-set is coming up, see: https://bugzilla.redhat.com/show_bug.cgi?id=1203780#c8 I'm waiting from feedback from the reporter before posting v3. In the mean time you can grab v3 of the patch-set here: http://cgit.freedesktop.org/~jwrdegoede/xserver/ (it is now 3 patches). Thanks! I can confirm that merging commit d7855745890d42ed56ceb97857081e9097acec12 also fixes the problem for me, so you can feel free to add my Tested-by line to those as well. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2 2/2] systemd-logind: Only use systemd-logind integration together with keeptty
On 04/30/2015 05:24 AM, Hans de Goede wrote: systemd-logind integration does not work when starting X on a new tty, as that detaches X from the current session and after hat systemd-logind revokes all rights any already open fds and refuses to open new fds for X. This means that currently e.g. startx -- vt7 breaks, and breaks badly, requiring ssh access to the system to kill X. The fix for this is easy, we must not use systemd-logind integration when not using KeepTty, or iow we may only use systemd-logind integration together with KeepTty. Signed-off-by: Hans de Goede hdego...@redhat.com I can confirm that this fixes VT switching for X servers started from an SSH session on Arch Linux, which I use all the time for debugging. So Tested-by: Aaron Plattner aplatt...@nvidia.com I was afraid not using -keeptty was going to break using a debugger, but it looks like it works these days. Not using -keeptty breaks ctrl-Z to suspend the server, but I'm less concerned about that. --- Changes in v2: -Document that -keeptty must be passed for logind integration in man page -Print an INFO message when disabling logind integration due to -keeptty not being set --- hw/xfree86/man/Xorg.man | 6 +++--- hw/xfree86/os-support/linux/systemd-logind.c | 9 + 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/hw/xfree86/man/Xorg.man b/hw/xfree86/man/Xorg.man index 3ff6aef..0864a58 100644 --- a/hw/xfree86/man/Xorg.man +++ b/hw/xfree86/man/Xorg.man @@ -271,9 +271,9 @@ is ignored if is anything other than \(oqPCI\(cq. .TP 8 .B \-keeptty -Prevent the server from detaching its initial controlling terminal. -This option is only useful when debugging the server. Not all platforms -support (or can use) this option. +Prevent the server from detaching its initial controlling terminal. If you +want to use systemd-logind integration you must specify this option. +Not all platorms support (or can use) this option. .TP 8 .BI \-keyboard keyboard-name Use the xorg.conf(__filemansuffix__) file diff --git a/hw/xfree86/os-support/linux/systemd-logind.c b/hw/xfree86/os-support/linux/systemd-logind.c index 4ad41a3..72f1ae3 100644 --- a/hw/xfree86/os-support/linux/systemd-logind.c +++ b/hw/xfree86/os-support/linux/systemd-logind.c @@ -34,6 +34,7 @@ #include os.h #include dbus-core.h +#include linux.h #include xf86.h #include xf86platformBus.h #include xf86Xinput.h @@ -596,6 +597,14 @@ static struct dbus_core_hook core_hook = { int systemd_logind_init(void) { +linux_parse_vt_settings(); +if (!linux_get_keeptty()) { +LogMessage(X_INFO, +systemd-logind: logind integration requires -keeptty and +-keeptty was not provided, disabling logind integration\n); +return 1; +} + return dbus_core_add_hook(core_hook); } ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] randrproto: clarify output XID lifetimes.
On 04/21/2015 05:59 PM, Dave Airlie wrote: From: Dave Airlie airl...@redhat.com This just makes a note that randr won't make outputs disappear dynamically. Signed-off-by: Dave Airlie airl...@redhat.com --- randrproto.txt | 8 1 file changed, 8 insertions(+) diff --git a/randrproto.txt b/randrproto.txt index 74b7c36..b2c828d 100644 --- a/randrproto.txt +++ b/randrproto.txt @@ -186,6 +186,14 @@ consider as single viewable areas. Xinerama's information now comes from the Monitors instead of directly from the CRTCs. The Monitor marked as Primary will be listed first. +1.5.2. Clarification of Output lifetimes + +With dynamic connectors being a possibility with the introduction of +MST, a lot of randr clients can't handle the XID BadMatch when a Maybe write out DisplayPort multistream here, for people unfamiliar with the MST acronym? Also, other places in this document are careful to capitalize it RandR. Otherwise, Reviewed-by: Aaron Plattner aplatt...@nvidia.com +randr output disappears. This is to clarify that going forward the +X server will not remove outputs dynamically, just mark them as +disconnected. + 1.99 Acknowledgments Our thanks to the contributors to the design found on the xpert mailing ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] composite: Don't bother copying the pixmap for ForgetGravity windows (v2)
Does this cause flickering when resizing windows and the compositor reads the new window pixmap before the app has a chance to respond to the events? On 04/23/2015 01:24 PM, Jasper St. Pierre wrote: If a window has ForgetGravity in its bitGravity, that very likely means it will repaint on the ConfigureNotify / Expose event, and thus we don't have to copy the old pixmap into the new pixmap, we can simply leave it blank. Preventing this copy is super simple to do and a big win on small devices where these blits can be expensive. A better approach would be to actually obey BitGravity correctly rather than assume NorthWestGravity always, but this is a big speedup for the common case. v2: Check all subwindows to make sure they are also ForgetGravity Cc: Adam Jackson a...@redhat.com Signed-off-by: Jasper St. Pierre jstpie...@mecheye.net --- composite/compalloc.c | 109 +- 1 file changed, 63 insertions(+), 46 deletions(-) diff --git a/composite/compalloc.c b/composite/compalloc.c index 8daded0..40bf873 100644 --- a/composite/compalloc.c +++ b/composite/compalloc.c @@ -526,6 +526,21 @@ compUnredirectOneSubwindow(WindowPtr pParent, WindowPtr pWin) return Success; } +static Bool +needsPixmapCopy(WindowPtr pWin) +{ +WindowPtr pChild; + +if (pWin-bitGravity != ForgetGravity) +return TRUE; + +for (pChild = pWin-firstChild; pChild; pChild = pChild-nextSib) +if (needsPixmapCopy(pChild)) +return TRUE; + +return FALSE; +} + static PixmapPtr compNewPixmap(WindowPtr pWin, int x, int y, int w, int h) { @@ -542,54 +557,56 @@ compNewPixmap(WindowPtr pWin, int x, int y, int w, int h) pPixmap-screen_x = x; pPixmap-screen_y = y; -if (pParent-drawable.depth == pWin-drawable.depth) { -GCPtr pGC = GetScratchGC(pWin-drawable.depth, pScreen); - -if (pGC) { -ChangeGCVal val; - -val.val = IncludeInferiors; -ChangeGC(NullClient, pGC, GCSubwindowMode, val); -ValidateGC(pPixmap-drawable, pGC); -(*pGC-ops-CopyArea) (pParent-drawable, - pPixmap-drawable, - pGC, - x - pParent-drawable.x, - y - pParent-drawable.y, w, h, 0, 0); -FreeScratchGC(pGC); +if (needsPixmapCopy(pWin)) { +if (pParent-drawable.depth == pWin-drawable.depth) { +GCPtr pGC = GetScratchGC(pWin-drawable.depth, pScreen); + +if (pGC) { +ChangeGCVal val; + +val.val = IncludeInferiors; +ChangeGC(NullClient, pGC, GCSubwindowMode, val); +ValidateGC(pPixmap-drawable, pGC); +(*pGC-ops-CopyArea) (pParent-drawable, + pPixmap-drawable, + pGC, + x - pParent-drawable.x, + y - pParent-drawable.y, w, h, 0, 0); +FreeScratchGC(pGC); +} } -} -else { -PictFormatPtr pSrcFormat = PictureWindowFormat(pParent); -PictFormatPtr pDstFormat = PictureWindowFormat(pWin); -XID inferiors = IncludeInferiors; -int error; - -PicturePtr pSrcPicture = CreatePicture(None, - pParent-drawable, - pSrcFormat, - CPSubwindowMode, - inferiors, - serverClient, error); - -PicturePtr pDstPicture = CreatePicture(None, - pPixmap-drawable, - pDstFormat, - 0, 0, - serverClient, error); - -if (pSrcPicture pDstPicture) { -CompositePicture(PictOpSrc, - pSrcPicture, - NULL, - pDstPicture, - x - pParent-drawable.x, - y - pParent-drawable.y, 0, 0, 0, 0, w, h); +else { +PictFormatPtr pSrcFormat = PictureWindowFormat(pParent); +PictFormatPtr pDstFormat = PictureWindowFormat(pWin); +XID inferiors = IncludeInferiors; +int error; + +PicturePtr pSrcPicture = CreatePicture(None, + pParent-drawable, + pSrcFormat, + CPSubwindowMode, + inferiors, +
Re: [PATCH xrandr] Split verbose mode printing into a helper function
On 04/22/2015 12:10 AM, Kenneth Graunke wrote: On Thursday, April 09, 2015 11:18:58 AM Aaron Plattner wrote: Combine the two forms of verbose mode printing into a single function. Pass the 'current' and 'preferred' flags as arguments. This fixes the code that prints unassociated modes to print the flags as well. Signed-off-by: Aaron Plattner aplatt...@nvidia.com --- xrandr.c | 62 ++ 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/xrandr.c b/xrandr.c index 8a345427a226..5be2167d724f 100644 --- a/xrandr.c +++ b/xrandr.c @@ -566,7 +566,7 @@ mode_geometry (XRRModeInfo *mode_info, Rotation rotation, /* v refresh frequency in Hz */ static double -mode_refresh (XRRModeInfo *mode_info) +mode_refresh (const XRRModeInfo *mode_info) { double rate; double vTotal = mode_info-vTotal; @@ -592,7 +592,7 @@ mode_refresh (XRRModeInfo *mode_info) /* h sync frequency in Hz */ static double -mode_hsync (XRRModeInfo *mode_info) +mode_hsync (const XRRModeInfo *mode_info) { double rate; @@ -603,6 +603,30 @@ mode_hsync (XRRModeInfo *mode_info) return rate; } +static void print_verbose_mode (const XRRModeInfo *mode, Bool current, + Bool preferred) +{ Seems like maybe static void should be on its own line, but I'm not familiar with the coding style in xrandr. Good catch. This looks correct to me, looks like a nice cleanup, and printing the flags seems sensible (though I've never read xrandr code before). Reviewed-by: Kenneth Graunke kenn...@whitecape.org Thanks! I'll get it checked in with the style thing fixed. -- Aaron ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Add amdgpu as a default driver for AMD GPUs
Is there any chance you could use the new OutputClass match rules in /usr/share/X11/xorg.conf.d rather than adding to this list? On 04/20/2015 08:08 PM, Michel Dänzer wrote: From: Michel Dänzer michel.daen...@amd.com Its Probe hook bails cleanly when it can't initialize, in which case the ati driver will be tried next. Signed-off-by: Michel Dänzer michel.daen...@amd.com --- hw/xfree86/common/xf86pciBus.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c index e86ecb9..f84bc99 100644 --- a/hw/xfree86/common/xf86pciBus.c +++ b/hw/xfree86/common/xf86pciBus.c @@ -1105,7 +1105,8 @@ xf86VideoPtrToDriverList(struct pci_device *dev, driverList[0] = ast; break; case 0x1002: -driverList[0] = ati; +driverList[0] = amdgpu; +driverList[1] = ati; break; case 0x102c: driverList[0] = chips; ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/3] modesetting: add dynamic connector hotplug support (MST) (v2)
On 04/19/2015 08:52 PM, Dave Airlie wrote: On 14 April 2015 at 13:12, Keith Packard kei...@keithp.com wrote: Dave Airlie airl...@gmail.com writes: So we hot unplug, we remove the output XID from the server, in parallel the client does an operation with the output XID it has gotten already, and still believe is valid, and it gets BadMatch, and since hardly anyone handles X errors it falls over. Yeah, I was curious if you'd found an actual race condition that couldn't be solved by correctly written applications. Sounds like it's just 'buggy' clients. Given that there just aren't that many applications which deal with RandR objects, it's pretty tempting to let them experience reality for a year or so and expect that they'll get fixed. As I said, the alternative would be to amend the spec to say that outputs may get added, but will never go away. That would enshrine their behavior as compliant, and ensure that we'd never break them in the future. I can implement that. Frankly, I think I'd probably be OK with either plan, the only one I'm not up for is supporting both modes with a configuration parameter that no-one in their right mind would enable. This is what we have today in the nvidia driver. In practice, I don't think I've ever heard of anyone enabling the option to delete outputs, but it's also not a huge amount of code so no one has felt the need to rip it out yet. I'm sort of happy to just enshrine the outputs cannot be destroyed behaviour, Me too. But I do wonder how many things would need fixing, it might just be mutter and a few things like that, though if gtk falls over then we should probably leave things alone. The big one was gnome-settings-daemon on RHEL 5, IIRC. We have customers who run old versions of GNOME with very recent drivers and hardware, so unless someone invents a time machine, I don't think that configuration can be fixed. Aaron, any ideas on how prevalent the issue is, or if nvidia care enough, since I took the option from you! I don't have a strong preference either way. We'll probably keep the option in our implementation until we have a reason to delete it, but I'd be kind of surprised if anyone ever actually needed it. If RandR starts mandating that outputs never go away, I'll probably delete the option just in the interests of spec compliance. Dave. ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xrandr] Split verbose mode printing into a helper function
Combine the two forms of verbose mode printing into a single function. Pass the 'current' and 'preferred' flags as arguments. This fixes the code that prints unassociated modes to print the flags as well. Signed-off-by: Aaron Plattner aplatt...@nvidia.com --- xrandr.c | 62 ++ 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/xrandr.c b/xrandr.c index 8a345427a226..5be2167d724f 100644 --- a/xrandr.c +++ b/xrandr.c @@ -566,7 +566,7 @@ mode_geometry (XRRModeInfo *mode_info, Rotation rotation, /* v refresh frequency in Hz */ static double -mode_refresh (XRRModeInfo *mode_info) +mode_refresh (const XRRModeInfo *mode_info) { double rate; double vTotal = mode_info-vTotal; @@ -592,7 +592,7 @@ mode_refresh (XRRModeInfo *mode_info) /* h sync frequency in Hz */ static double -mode_hsync (XRRModeInfo *mode_info) +mode_hsync (const XRRModeInfo *mode_info) { double rate; @@ -603,6 +603,30 @@ mode_hsync (XRRModeInfo *mode_info) return rate; } +static void print_verbose_mode (const XRRModeInfo *mode, Bool current, + Bool preferred) +{ +int f; + +printf ( %s (0x%x) %6.3fMHz, + mode-name, (int)mode-id, + (double)mode-dotClock / 100.0); +for (f = 0; mode_flags[f].flag; f++) + if (mode-modeFlags mode_flags[f].flag) + printf ( %s, mode_flags[f].string); +if (current) + printf ( *current); +if (preferred) + printf ( +preferred); +printf (\n); +printf (h: width %4d start %4d end %4d total %4d skew %4d clock %6.2fKHz\n, + mode-width, mode-hSyncStart, mode-hSyncEnd, + mode-hTotal, mode-hSkew, mode_hsync (mode) / 1000); +printf (v: height %4d start %4d end %4d total %4d clock %6.2fHz\n, + mode-height, mode-vSyncStart, mode-vSyncEnd, mode-vTotal, + mode_refresh (mode)); +} + static void init_name (name_t *name) { @@ -3836,25 +3860,9 @@ main (int argc, char **argv) for (j = 0; j output_info-nmode; j++) { XRRModeInfo *mode = find_mode_by_xid (output_info-modes[j]); - int f; - - printf ( %s (0x%x) %6.3fMHz, - mode-name, (int)mode-id, - (double)mode-dotClock / 100.0); - for (f = 0; mode_flags[f].flag; f++) - if (mode-modeFlags mode_flags[f].flag) - printf ( %s, mode_flags[f].string); - if (mode == output-mode_info) - printf ( *current); - if (j output_info-npreferred) - printf ( +preferred); - printf (\n); - printf (h: width %4d start %4d end %4d total %4d skew %4d clock %6.2fKHz\n, - mode-width, mode-hSyncStart, mode-hSyncEnd, - mode-hTotal, mode-hSkew, mode_hsync (mode) / 1000); - printf (v: height %4d start %4d end %4d total %4d clock %6.2fHz\n, - mode-height, mode-vSyncStart, mode-vSyncEnd, mode-vTotal, - mode_refresh (mode)); + + print_verbose_mode (mode, mode == output-mode_info, + j output_info-npreferred); mode-modeFlags |= ModeShown; } } @@ -3899,17 +3907,7 @@ main (int argc, char **argv) XRRModeInfo *mode = res-modes[m]; if (!(mode-modeFlags ModeShown)) - { - printf ( %s (0x%x) %6.3fMHz\n, - mode-name, (int)mode-id, - (double)mode-dotClock / 100.0); - printf (h: width %4d start %4d end %4d total %4d skew %4d clock %6.2fKHz\n, - mode-width, mode-hSyncStart, mode-hSyncEnd, - mode-hTotal, mode-hSkew, mode_hsync (mode) / 1000); - printf (v: height %4d start %4d end %4d total %4d clock %6.2fHz\n, - mode-height, mode-vSyncStart, mode-vSyncEnd, mode-vTotal, - mode_refresh (mode)); - } + print_verbose_mode(mode, False, False); } exit (0); } -- 2.3.5 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel