drm/compositor: allow to explicitly enable/disable explicit sync #1697
Conversation
this moves the private decision for enabling explicit sync to a public capability enum, allowing to override it.
| // `IN_FENCE_FD` makes commit fail on Nvidia driver | ||
| // https://github.com/NVIDIA/open-gpu-kernel-modules/issues/622 | ||
| let is_nvidia = driver.name().to_string_lossy().to_lowercase().contains("nvidia") | ||
| || driver | ||
| .description() | ||
| .to_string_lossy() | ||
| .to_lowercase() | ||
| .contains("nvidia"); |
There was a problem hiding this comment.
why not make this a debug flag or environment variable instead, like SMITHAY_DEBUG_DISABLE_EXPLICIT_SYNC? I don't think it should be on the compositor to deal with misbehaving drivers in this case.
There was a problem hiding this comment.
I find env vars have their own ux issues since many people don't know of the probably dozen that exist which you could tune.
Also an explicit type (with a little more code) means you could switch at runtime between modes.
I could see the env var make sense for configuring the default value.
There was a problem hiding this comment.
Both sounds good to me, if it can be shimmied in here.
There was a problem hiding this comment.
I am not a big fan of environment variables when they are avoidable. Giving explicit control allows projects like niri to implement thst config option in a way more visible way.
The reason for keeping the nvidia check is simply to not break existing code.
But I am not opposed to adding an environment variable that overrides the default.
this moves the private decision for enabling explicit sync to a public capability enum, allowing to override it.
related to #1695