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 <hdegoede@redhat.com>
Reviewed-by: Aaron Plattner <aplattner@nvidia.com>
Remove unused arguments from ms_covering_crtc, make it static as it is
only used in vblank.c.
While at it also change its first argument from a ScrnInfoPtr to a
ScreenPtr, this makes the next patch in this patch-set cleaner.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
glamor_fd_from_pixmap() may return a tiled bo, which is not suitable
for sharing with another GPU as tiling usually is GPU specific.
Switch to glamor_shareable_fd_from_pixmap(), which always returns a
linear bo. This fixes mis-rendering when running the mode setting
driver on the master gpu in a dual-gpu setup and running an opengl
app with DRI_PRIME=1.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
>From the GLX spec:
"GLX_X_RENDERABLE is a boolean indicating whether X can be used to
render into a drawable created with the GLXFBConfig. This attribute
is True if the GLXFBConfig supports GLX windows and/or pixmaps."
Every backend was setting this to true unconditionally, and then the
core ignored that value and sent true unconditionally on its own. This
is broken for ARB_fbconfig_float and EXT_fbconfig_packed_float, which
only apply to pbuffers, which are not renderable from non-GLX APIs.
Instead compute GLX_X_RENDERABLE from the supported drawable types. The
dri backends were getting _that_ wrong too, so fix that as well.
This is not a functional change, as there are no mesa drivers that claim
to support __DRI_ATTRIB_{UNSIGNED_,}FLOAT_BIT yet.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Since xwayland's initial commit we have had a check to not process
wayland modifier events while one of our surfaces has keyboard focus
since the normal xkb event processing keeps our internal modifier
state up to date and if we use the modifiers we get from the
compositor we mess up that state.
This was slightly changed in commit
10e9116b3f to allow the xkb group to be
set from the wayland event while we have focus in case the compositor
triggers a group switch.
There's a better solution to the original problem though. Processing
queued events before overriding the xkb state with the compositor's
allows those events to be sent properly modified to X clients while
any further events will be modified with the wayland modifiers as
intended.
This allows us to fully take in the wayland modifiers, including
depressed ones, which fixes an issue where we wouldn't be aware of
already pressed modifiers on enter.
Signed-off-by: Rui Matos <tiagomatos@gmail.com>
Tested-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The common page flip handle framework can be shared with DRI2
page flip.
Signed-off-by: Qiang Yu <Qiang.Yu@amd.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
When Xorg gets started directly from a wayland-gdm the crtc still has the
wayland hw cursor set. Combine this with Xorg immediately falling back to
a sw cursor because a slave-output has a monitor attached at startup; and
we end up with the wayland hardware cursor overlay fixed in its last
position + the Xorg sw cursor resulting in 2 cursors.
This commit fixes this by hiding any left-over cursors when initializing
the crtc.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
The modesetting driver may be driving 2 screens (slave and master
gpu), which may have different behavior wrt hardware cursor support.
So stop using static variables and instead store the hw-cursor support
related data in a per screen struct. While at it actually make it per
crtc data as in theory different crtc's could have different hw-cursor
support.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
We're happily printing the error to stdout but not which module caused it...
That's in the Xorg.log but that's at least one click away.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Emitting a LeaveNotify event every time the pointer leaves an X11 window
may confuse focus follow mouse mode in window managers such as
mutter/gnome-shell.
Keep the previously found X window and compare against the new one, and
if they match then it means the pointer has left an Xwayland window for
a native Wayland surface, only in this case fake the crossing to the
root window.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Xorg -configure relies on the bus implementation, e.g.
xf86pciBus.c to call xf86AddBusDeviceToConfigure(). The new
xf86platformBus code does not have support for this.
Almost all drivers support both the xf86platformBus and xf86pciBus
nowadays, and the generic xf86Bus xf86CallDriverProbe() function
prefers the new xf86platformBus probe method when available.
Since the platformBus paths do not call xf86AddBusDeviceToConfigure()
this results in Xorg -configure failing with the following error:
"No devices to configure. Configuration failed.".
Adding support for the xf86Configure code to xf86platformBus.c
is non trivial and since we advise users to normally run without
any Xorg.conf at all not worth the trouble.
However some users still want to use Xorg -configure to generate a
template config file, this commit implements a minimal fix to make
things work again for PCI devices by skipping the platform
probe method when xf86DoConfigure is set.
This has been tested on a system with integrated intel graphics,
with both the intel and modesetting drivers and restores Xorg -configure
functionality on both cases.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Without this a change in eg Xext/ wouldn't relink Xwayland, making you
wonder why your changes didn't have any effect.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
At shutdown, the Xserver will free all its resources which includes the
RRCrtc and RROutput created.
Xwayland would do the same in its xwl_output_destroy() called from
xwl_close_screen(), leading to a double free of existing RRCrtc
RROutput:
Invalid read of size 4
at 0x4CDA10: RRCrtcDestroy (rrcrtc.c:689)
by 0x426E75: xwl_output_destroy (xwayland-output.c:301)
by 0x424144: xwl_close_screen (xwayland.c:117)
by 0x460E17: CursorCloseScreen (cursor.c:187)
by 0x4EB5A3: AnimCurCloseScreen (animcur.c:106)
by 0x4EF431: present_close_screen (present_screen.c:64)
by 0x556D40: dix_main (main.c:354)
by 0x6F0D290: (below main) (in /usr/lib/libc-2.24.so)
Address 0xbb1fc30 is 0 bytes inside a block of size 728 free'd
at 0x4C2BDB0: free (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4CCE5F: RRCrtcDestroyResource (rrcrtc.c:719)
by 0x577541: doFreeResource (resource.c:895)
by 0x5787B5: FreeClientResources (resource.c:1161)
by 0x578862: FreeAllResources (resource.c:1176)
by 0x556C54: dix_main (main.c:323)
by 0x6F0D290: (below main) (in /usr/lib/libc-2.24.so)
Block was alloc'd at
at 0x4C2CA6A: calloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4CC6DB: RRCrtcCreate (rrcrtc.c:76)
by 0x426D1C: xwl_output_create (xwayland-output.c:264)
by 0x4232EC: registry_global (xwayland.c:431)
by 0x76CB1C7: ffi_call_unix64 (in /usr/lib/libffi.so.6.0.4)
by 0x76CAC29: ffi_call (in /usr/lib/libffi.so.6.0.4)
by 0x556CEFD: wl_closure_invoke (connection.c:935)
by 0x5569CBF: dispatch_event.isra.4 (wayland-client.c:1310)
by 0x556AF13: dispatch_queue (wayland-client.c:1456)
by 0x556AF13: wl_display_dispatch_queue_pending
(wayland-client.c:1698)
by 0x556B33A: wl_display_roundtrip_queue (wayland-client.c:1121)
by 0x42371C: xwl_screen_init (xwayland.c:631)
by 0x552F60: AddScreen (dispatch.c:3864)
And:
Invalid read of size 4
at 0x522890: RROutputDestroy (rroutput.c:348)
by 0x42684E: xwl_output_destroy (xwayland-output.c:302)
by 0x423CF4: xwl_close_screen (xwayland.c:118)
by 0x4B6377: CursorCloseScreen (cursor.c:187)
by 0x539503: AnimCurCloseScreen (animcur.c:106)
by 0x53D081: present_close_screen (present_screen.c:64)
by 0x43DBF0: dix_main (main.c:354)
by 0x7068730: (below main) (libc-start.c:289)
Address 0xc403190 is 0 bytes inside a block of size 154 free'd
at 0x4C2CD5A: free (vg_replace_malloc.c:530)
by 0x521DF3: RROutputDestroyResource (rroutput.c:389)
by 0x45DA61: doFreeResource (resource.c:895)
by 0x45ECFD: FreeClientResources (resource.c:1161)
by 0x45EDC2: FreeAllResources (resource.c:1176)
by 0x43DB04: dix_main (main.c:323)
by 0x7068730: (below main) (libc-start.c:289)
Block was alloc'd at
at 0x4C2BBAD: malloc (vg_replace_malloc.c:299)
by 0x52206B: RROutputCreate (rroutput.c:84)
by 0x426763: xwl_output_create (xwayland-output.c:270)
by 0x422EDC: registry_global (xwayland.c:432)
by 0x740FC57: ffi_call_unix64 (unix64.S:76)
by 0x740F6B9: ffi_call (ffi64.c:525)
by 0x5495A9D: wl_closure_invoke (connection.c:949)
by 0x549283F: dispatch_event.isra.4 (wayland-client.c:1274)
by 0x5493A13: dispatch_queue (wayland-client.c:1420)
by 0x5493A13: wl_display_dispatch_queue_pending
(wayland-client.c:1662)
by 0x5493D2E: wl_display_roundtrip_queue (wayland-client.c:1085)
by 0x4232EC: xwl_screen_init (xwayland.c:632)
by 0x439F50: AddScreen (dispatch.c:3864)
Split xwl_output_destroy() into xwl_output_destroy() which frees the
wl_output and the xwl_output structure, and xwl_output_remove() which
does the RRCrtcDestroy() and RROutputDestroy() and call the latter only
when an output is effectively removed.
An additional benefit, on top of avoiding a double free, is to avoid
updating the screen size at shutdown.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
The frame callback set up via wl_surface_frame() needs to be freed with
wl_callback_destroy() or we'll leak memory.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97065
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
This loop was written in a buggy style, causing a NULL driver ptr to be
passed to copyScreen(). copyScreen() only uses that to generate an
identifier string, so this is mostly harmless on systems that accept
NULL for asprintf() "%s" format. (the generated identifiers are off
by one wrt the driver names and the last one contains NULL.
For systems that don't accept NULL for '%s' this would cause a
segmentation fault when this code is used (no xorg.conf, but partial
config in xorg.conf.d for instance).
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Reviewed-by: Keith Packard <keithp@keithp.com>
In function xf86VGAarbiterScrnInit when the "pEnt->bus.type" is
BUS_PLATFORM, the "pScrn->vgaDev" won't be set, so the "pScrn->vgaDev" is
equal to zero.
The variable "rsrc_decodes" in function "xf86VGAarbiterAllowDRI" is not
initialized. So it will occur error when "pScrn->vgaDev == 0", and
"vga_count > 1". For this case, as "pScrn->vgaDev == 0", the function
"pci_device_vgaarb_get_info" will only set the value of "vga_count",
but won't set the value of "rsrc_decodes", so it will has two different
return values for function "xf86VGAarbiterAllowDRI" in different
platforms. One platform will return TRUE, as the "rsrc_decodes" 's
default value is 0, but another platform will return FALSE, as the
"rsrc_decodes" 's default value is "32767", this will cause disable
direct rendering.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96937
Signed-off-by: Emily Deng <Emily.Deng@amd.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Any code called from the driver ScreenInit may want to refer to
pScrn->pScreen. As the function passed to AddScreen is the first place
the DDX sees a new screen, the generic code needs to make sure that
value is set before passing control to the video driver's
initialization code.
This was found by running a driver which didn't bother to set this
value when the initial colormap was installed; xf86RandR12LoadPalette
tried to use pScrn->pScreen and crashed.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97124
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-and-Tested-by: Michel Dänzer <michel.daenzer@amd.com>
This is a problem for the libinput driver that uses the same context across
multiple devices. The driver may be halfway through setting up an input device
(and the only way to do so is to add it to libinput) when the input thread
comes in an reads events. This then causes mayhem when data is dereferenced
that hasn't been set up yet.
In my case the cause was the call to libinput_path_remove_device() inside
preinit racing with evdev_dispatch_device() handling of ENODEV. The sequence
was:
- thread 2 gets an event and calls evdev_dispatch_device()
- thread 1 calls libinput_path_remove_device() which sets the device->source
to NULL
- thread 2 reads from the fd, gets ENODEV and now removes the device->source,
dereferencing the null-pointer
This is the one I could reproduce the most, but there are other potential
pitfalls that affect any driver that uses the same fd for multiple devices.
Avoid all this and wrap PreInit into the lock.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
If a device couldn't be enabled we left the lock hanging.
This patch also removes the leftover OsReleaseSignals() call, now unnecessary.
Note that input_unlock() is later than previously OsReleaseSignals().
RemoveDevice() manipulates the input device and its file descriptors, it's
safer to put the input_unlock() call after RemoveDevice() to avoid events
coming in while the device is being removed.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Just use the RandR gamma ramp directly.
Fixes random on-monitor colours with drivers which don't call
xf86HandleColormaps, e.g. modesetting.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97154
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Embarassingly, it looks like I introduced this dead function in
commit 13c7d53df8 a year ago.
Nothing ever used it, not even then.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
Instead of breaking the former when the driver supports the latter,
hook them up so that the hardware LUTs reflect the combination of the
current colourmap and gamma states. I.e. combine the colourmap, the
global gamma value/ramp and the RandR 1.2 per-CRTC gamma ramps into one
combined LUT per CRTC.
Fixes e.g. gamma sliders not working in games.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=27222
v2:
* Initialize palette_size and palette struct members, fixes crash on
server startup.
v3:
* Free randrp->palette in xf86RandR12CloseScreen, fixes memory leak.
v4:
* Call CMapUnwrapScreen if xf86RandR12InitGamma fails (Emil Velikov).
* Still allow xf86HandleColormaps to be called with a NULL loadPalette
parameter in the xf86_crtc_supports_gamma case.
v5:
* Clean up inner loops in xf86RandR12CrtcComputeGamma (Keith Packard)
* Move palette update out of per-CRTC loop in xf86RandR12LoadPalette
(Keith Packard)
v6:
* Handle reallocarray failure in xf86RandR12LoadPalette (Keith Packard)
Reviewed-by: Keith Packard <keithp@keithp.com>
This would normally return the same values the core RandR code passed to
xf86RandR12CrtcSetGamma before, which is rather pointless. The only
possible exception would be if a driver tried initializing
crtc->gamma_red/green/blue to reflect the hardware LUT state on startup,
but that can't work correctly if whatever set the LUT before the server
started was running at a different depth.
Even the pointless round-trip case will no longer work with the
following change.
Reviewed-by: Keith Packard <keithp@keithp.com>
RRCrtcGammaSetSize cannot be used yet in xf86InitialConfiguration,
because randr_crtc isn't allocated yet at that point, but a following
change will require RRCrtcGammaSetSize to be called from
xf86RandR12CrtcInitGamma.
v2:
* Bail from xf86RandR12CrtcInitGamma if !crtc->funcs->gamma_set (Keith
Packard)
Reviewed-by: Keith Packard <keithp@keithp.com>
This uses the wrapper in case we need to emulate poll with select
as we do on Windows.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Update for removal of fdset from Block/Wakeup handler API in 9d15912a
Signed-off-by: Jon Turney <jon.turney@dronecode.org.uk>
Reviewed-by: Keith Packard <keithp@keithp.com>
Update for removal of AddEnabledDevice in be5a513f. Use SetNotifyFd instead.
Signed-off-by: Jon Turney <jon.turney@dronecode.org.uk>
Reviewed-by: Keith Packard <keithp@keithp.com>
ATTR_KEY maps to ID_INPUT_KEY which is set for any device with keys.
ID_INPUT_KEYBOARD and thus ATTR_KEYBOARD is set for devices that are actual
keyboards (and have a set of expected keys).
Hand-written match rules may only apply ID_INPUT_KEYBOARD, so make sure we
match on that too.
Arguably we should've been matching on ATTR_KEYBOARD only all along but
changing that likely introduces regressions.
Reported-by: Marty Plummer <netz.kernel@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This error code can mean we're submitting more rects at once than the
driver can handle. If that happens, resubmit one at a time.
v2: Make the rect submit loop more error-proof (Walter Harms)
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Michael Thayer <michael.thayer@oracle.com>
This removes the last uses of fd_set from the server interfaces
outside of the OS layer itself.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
With no users of the interface needing the readmask anymore, we can
remove it from the argument passed to these functions.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Use SetNotifyFd instead, with the hope that someday someone will come
fix this to be more efficient -- right now, the wakeup handler is
doing the event reading, instead of the notify callback.
v2: no need to patch dmxsigio.c as it has been removed.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
This is a cleanup, proposed by Adam Jackson, but wasn't merged with
the original NotifyFD changes.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
This is a cleanup, proposed by Adam Jackson, but wasn't merged with
the original NotifyFD changes.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Remove code in xf86Wakeup for dealing with other input and switch to
using the new NotifyFd interface.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
This new libXfont API eliminates exposing internal X server symbols to
the font library, replacing those with a struct full of the entire API
needed to use that library.
v2: Use libXfont2 instead of libXfont_2
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
The intent here was that fallback drivers would be at the end of the
list in order, but if a fallback driver happened to be at the end of the
list already that's not what would happen. Rather than open-code
something smarter, just use qsort.
Note that qsort puts things in ascending order, so somewhat backwardsly
fallbacks are greater than native drivers, and vesa is greater than
modesetting.
v2: Use strcmp to compare non-fallback drivers so we get a predictable
result if your libc's qsort isn't stable (Keith Packard)
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Adam Jackson <ajax@redhat.com>
Otherwise if the geometry changes but the mode doesn't we end up with
the previous geometry from RR's point of view.
Fixes https://bugzilla.gnome.org/show_bug.cgi?id=768710
Reviewed-by: Jonas Ådahl <jadahl@gmail.com>
Signed-off-by: Rui Matos <tiagomatos@gmail.com>
/dev/vc/0 is a devfs thing which is long dead, so stop trying to open
/dev/vc/0, besides being a (small) code cleanup this will also fix the
"parse_vt_settings: Cannot open /dev/tty0 (%s)\n" error message to
display the actual error, rather then the -ENOENT from also trying
/dev/vc/0.
BugLink: https://patchwork.freedesktop.org/patch/8768/
Reported-by: Chad Versace <chad.versace@linux.intel.com>
Suggested-by: Julien Cristau <jcristau@debian.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Julien Cristau <jcristau@debian.org>
Reviewed-by: Chad Versace <chad.versace@intel.com>
Commit 80e64dae: "modesetting: Implement PRIME syncing as a sink" originally was
supposed to have this line, but it was dropped as part of the merge process.
Foregoing the NULL assignment causes a ton of problems with dereferencing
uninitialized memory.
Signed-off-by: Alex Goins <agoins@nvidia.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>
In newer laptops with switchable graphics, the GPU may have 0 outputs,
in this case the modesetting driver should still load if the GPU is
SourceOffload capable, so that it can be used as an offload source provider.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>