It's just a tuning parameter (that nobody touched for aeons) for
DoGetImage(), inside dix/dispatch.c
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
No need for carrying around our own (incomplete) implementation
of a standard libc function.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
When dmabuf_capable is enabled, ms_present_check_unflip may return
with reason PRESENT_FLIP_REASON_BUFFER_FORMAT to be reported back
to client by PresentCompleteModeSuboptimalCopy. We should not
overwrite it by page flip reasons anyway when exit.
This fix also avoid changing vblank->exec_msc in present_scmd_pixmap()
which caused by page flip reasons to prevent a present to be executed
immediatly. So that we can cancel pending vblanks when new vblanks with
same msc arrive. This happens when application just starts.
This problem can be observed at least using radeonsi OGL driver, start
xserver with dmabuf_capable enabled, no window manager, glxgears will
stuck for the first several seconds. If using a composite window manager
like Mutter, we can't observe the glxgears stuck, but the prensent
complete event still shows unexpected Copy mode instead of
PresentCompleteModeSuboptimalCopy or Skip mode.
glxgears window msc is 0 at the beginning, it will send present request
with target msc = 1, 2, 3, ... N before server send back the complete
event for target msc = 1. But server side window msc is way bigger than N,
so it will think all these present requests are outdated and just show the
Nth request at the next vblank. [1 .. N-1] requests should be skipped.
But without this fix, all [1 .. N] presents will be executed with Copy
mode which causes stuck.
Fixes: a94dd953 ("modesetting: add support for TearFree page flips")
Previously it was possible for the invalid modifier to be placed in the IN_FORMATS or IN_FORMATS_ASYNC arrays, this is of course not wanted as this can cause problems with devices that lack support for explicit modifiers.
Use the IN_FORMATS_ASYNC blob (as opposed to the normal
IN_FORMATS blob) to determine which formats/modifiers are
supported. This will allow the client to allocate buffers
which can actually be async flipped.
In order to guarantee that the most optimal modifier is
always used we also need to force a modifier renegotiation
when swicthing between sync and async flips. Otherwise eg.
sync flips might end up using a less optimal sync+async
modifier instead of a more optimal sync-only modifier.
Signed-off-by: notbabaisyou <though-went-some-simple@proton.me>
Link: https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1816
The kernel has gained another format/modifier blob to indicate
which formats/modifiers support async flips since Linux 6.16. Parse it.
Signed-off-by: notbabaisyou <though-went-some-simple@proton.me>
Enable the universal planes client cap so that we actually
get access to the primary plane's IN_FORMATS blob. We will
now start to parse the blob.
Signed-off-by: notbabaisyou <though-went-some-simple@proton.me>
We want the root pixmap to use conservative tiling modifiers in
order to make sure modeset/etc can never fail due to hardware
watermark restictions/etc.
Currenlty this is all dead code anyway because we aren't actually
parsing the IN_FORMATS blob (missing universal plane client cap).
But we want to start parsing that, so let's first make sure we
don't get any behavioural changes from doing so.
Signed-off-by: notbabaisyou <though-went-some-simple@proton.me>
Many Intel GPUs can't switch between sync and async flips
willy nilly. Sometimes that change itself will take one
extra frame. This means that constant ping-pong between
sync and async flips is only going to cause problems.
Stay in async flip mode as long as the client is requesting
it.
The present protocol spec does say:
"If 'options' contains PresentOptionAsync, and the 'target-msc'
is less than or equal to the current msc for 'window', then
the operation will be performed as soon as possible, not
necessarily waiting for the next vertical blank interval."
So there is an expectation that a future target-msc will
still be respected even when PresentOptionAsync is specified.
Staying in async flip mode won't actually change that given
that present_scmd_pixmap() takes the flip mode into account
when calculating exec_msc. So visually the flip should still
happen on the correct target_msc regardles of whether we
executed it as sync or async.
Signed-off-by: notbabaisyou <though-went-some-simple@proton.me>
We currently skip setting the window pixmap on any window
not using its parent's pixmap. That does not work correctly
in the presence of reparenting.
Consider the following scenario:
1. window A is created as child of B
2. present starts flipping and sets the whole window
tree to use pixmap X
3. window C is created (uses the screen pixmap by default)
4. window A is reparented to C
5. present stops flipping and attempts to set the
whole window tree back to the screen pixmap,
except the walk terminates at window C
since it's using an unexpected pixmap, and
window A is left with the stale pixmap X
6. pixmap X is destroyed
7. the X server segfaults on a rendering operation
on window A due the stale pixmap
I managed to hit this with mpv (doing present flips)
and crack-attack (keeps alternating between a menu
window and an actual game window):
1. start both applications
2. start a game in crack-attack
3. fullscreen mpv
4. end the game in crack attack
5. unfullscreen mpv
6. the crack-attack menu window has appeared, but
might be corrupted and doing stuff on it segfaults
the X server
I suppose the other option might be to make new windows
automatically inherit their parent's pixmap instead
of using the screen pixmap. But I've not looked into
how that would affect eg. composite.
Signed-off-by: notbabaisyou <though-went-some-simple@proton.me>
This warning doesn't matter in this case:
> ../test/simple-xinit.c: In function ‘handle_sigchld’:
> ../test/simple-xinit.c:69:5: warning: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
> 69 | write(server_displayfd, server_dead, strlen(server_dead));
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In order to allow building w/ -Werror, it should be suppressed.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
No need for patching up the request buffer anymore - just pass in the correct
value directly.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
This pointer just had been kept in at previous commit for keeping the
diff small and so easier to review. Now accessing the fields within
the local struct directly, dropping the extra pointer.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
No need for patching up the original request buffer anymore - just pass
in a modified copy.
Trying to keep this patch small for easier review. Some cleanups coming
with a follow-up.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
It's assigned a const char* value and not writing into it,
so it should be const, too (compiler correctly warning about that)
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
This variable is only used in os layer and PanoramiX, nowhere else,
and shouldn't be visible to drivers at all.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The old code tried to use a screen pointer that was uninitialized and set to NULL.
This caused it to segfault when this option was set.
Signed-off-by: stefan11111 <stefan11111@shitposting.expert>
Add matching call for xf86_cursors_init to clean memory, as during
initialization it allocates memory (depends, but is something like ~256Kb)
and it leaks when XServer resets.
Signed-off-by: Tautvis <gtautvis@gmail.com>
Headers should always be self-consistent, thus including anything they need.
Not relying on those already included before by somebody else.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
See: https://github.com/gentoo/gentoo/blob/master/x11-base/xorg-server/files/xorg-server-1.12-unloadsubmodule.patch
See: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=686152#14
Verbatim copy of https://github.com/X11Libre/xserver/issues/319#issuecomment-3033729517 ,which gives more context for this patch:
I took a closer look at that patch.
It is logically equivalent to:
```
diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index 2cdf91fd2..49785fdc8 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -885,6 +885,7 @@ RemoveChild(ModuleDescPtr child)
parent = child->parent;
if (parent->child == child) {
parent->child = child->sib;
+ child->sib = NULL;
return;
}
```
RemoveChild is a static function that is only called in UnloadSubModule:
```
void
UnloadSubModule(ModuleDescPtr mod)
{
/* Some drivers are calling us on built-in submodules, ignore them */
if (mod == (ModuleDescPtr) 1)
return;
RemoveChild(mod);
UnloadModule(mod);
}
```
Whether or not child->sib is NULL tells UnloadModule if it should recursively unload child->sib or not:
```
if (mod->child)
UnloadModule(mod->child);
if (mod->sib)
UnloadModule(mod->sib);
free(mod);
```
Looking at the source, the module loader uses some weird kind of tree-like structure,
where every node has at most one child and one sibling (but then, if foo has child bar, and bar has sibling baz, shouldn't baz also be foo's child?).
```
typedef struct module_desc {
struct module_desc *child;
struct module_desc *sib;
struct module_desc *parent;
void *handle;
ModuleSetupProc SetupProc;
ModuleTearDownProc TearDownProc;
void *TearDownData; /* returned from SetupProc */
const XF86ModuleVersionInfo *VersionInfo;
} ModuleDesc, *ModuleDescPtr;
```
All in all, this patch makes UnloadSubModule to never unload the sibling of the unloaded module, whereas
as it is now, UnloadSubModule would also unload the module's sibling if `child->parent == child->parent->child`
(master child?).
I don't see how this patch changed the behavior on ia64, or any other arch.
@metux Could you tell me what kind of data structure this is, and whether or not this patch is right?
Fixes: https://github.com/X11Libre/xserver/issues/319
Signed-off-by: stefan11111 <stefan11111@shitposting.expert>
Now that all individual swapping request handlers have been merged into the
actual ones, there's no need for a separate dispatcher anymore.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Instead of internally faking requests, factor out the actual logic
into separate function, which is getting everything it needs as
parameters, so no need to fiddle with request buffer anymore.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Since https://github.com/X11Libre/xserver/pull/1234 landed,
the user has a way to set the hw cursor size to the size they want.
The fallback probe works around driver bugs by probing very late,
so it initializes the cursor image buffer with the largest size the driver supports.
With this change, the SIZE_HINTS probe will also initialize
the cursor image buffer with the largest size it finds,
which is what @notbabaisyou 's code originally did.
Signed-off-by: stefan11111 <stefan11111@shitposting.expert>
