Avoid gpu wakeup in hwdec_cuda by deferring cuInit #14028
Conversation
|
Download the artifacts for this pull request: |
|
Fixed signing and commit name |
|
I don't like how this is implemented. The whole point is to check whether the RA is GL or PL before processing, right? Can't we do something like this? typedef struct cuda_interop_fn {
bool (*check)(const struct ra_hwdec *hw);
bool (*init)(const struct ra_hwdec *hw);
};
bool check_valid = false;
for (int i = 0; interop_fns[i]; i++) {
if (interop_fns[i]->check(hw)) {
check_valid = true;
break;
}
}
if (check_valid)
ret = CHECK_CU(cu->cuInit(0));
// Do the rest of autoprobing and init here |
Yeah, there's no problem with that, I considered implementing that way too, I just wasn't sure which method would be the preferred. I can make a new PR implementing that logic instead. |
|
I'd prefer this way if others think this approach is OK. No need to open another PR. You can just rebase this one. |
|
...actually, it might be desirable to fallback to another interop if the init fails in one of them. Right now, we keep trying until one of the interop inits returns true. In your implementation, if the check returns but init doesn't, then there's no fallback. |
|
You can just check if all check functions fail. Only call |
|
So, is it fine to just return an error if the check is successful but the init isn't? I was just worried since the current behavior is to go through all valid inits. |
|
I meant you only do early exit if none of the checks are successful. Otherwise proceed as before and try initialize ALL interops regardless. |
|
Ah I see. That should work well as long as the interop init has no dependency on its check. I guess that could easily be fixed by calling check within init itself as well. |
|
Here's how it stands then:
-- I could also put it all in a single loop and initialize cuda after the first successful check. |
|
This caused crash on win32 when hwdec auto switch with gpu-context switch. cmd:
backtrace |
|
Hmm, I'm rebasing this PR later this day so it implements the |
|
Can't you just use |
|
This also makes sense for d3d11, where users obviously don't want to accidentally wake up dGPU while playing video on laptops using iGPU via d3d11, but it should also be taken into account that mpv may change gpu-context/vo/hwdec at runtime. |
|
Rebased. Initial interop checks have been split into |
|
|
Lesson learned: Do not forget to configure meson to use clang... |
|
hwdec=nvdec doesn't work after switching gpu-context at runtime.
|
|
@Andarwinux that's weird. As far as I know, this change shouldn't be changing which contexts CUDA can run in Is this something specific with d3d11? A d3d11 cuda interop wasn't implemented by this change and it didn't exist before either |
|
Neither of the check functions ever return true. Of course it doesn't work. |
|
Oh yeah... That's certainly an issue. Not sure how I missed that honestly. I'm rebasing this PR into a single commit soon. I'll include the fixed checks and move cuda_interop_fn structs out of |
|
Check functions were fixed, moved cuda_interop_fn structs into extern declarations. |
jrelvas-ipc
force-pushed
the
master
branch
2 times, most recently
from
59d1371
to
e9eaac6
Compare
`cuInit` wakes up the nvidia dgpu on nvidia laptops. This is bad news because the wake up process is blocking and takes a few seconds. It also needlessly increases power consumption. Sometimes, a VO loads several hwdecs (like `dmabuf_wayland`). When `cuda` is loaded, it calls `cuInit` before running all interop inits. However, the first checks in the interops do not require cuda initialization, so we only need to call `cuInit` after those checks. This commit splits the interop `init` function into `check` and `init`. `check` can be called without initializing the Cuda backend, so cuInit is only called *after* the first interop check. With these changes, there's no cuda initialization if no OpenGL/Vulkan backend is available. This prevents `dmabuf_wayland` and other VOs which automatically load cuda from waking up the nvidia dgpu unnecessarily, making them start faster and decreasing power consumption on laptops. Fixes: mpv-player#13668
This simplifies the code and makes it easier to read.
|
Just a small change which makes the check functions more readable :) (Should have no behavior changes) |
|
@Andarwinux let me know if your issue still happens with these new fixes |
It's still working perfectly, thanks! |
cuInitwakes up the nvidia dgpu on nvidia laptops. This is bad news because the wake up processis blocking and takes a few seconds. It also needlessly increases power consumption.
Sometimes, a VO loads several hwdecs (like
dmabuf_wayland). Whencudais loaded, it callscuInitbefore running all interop inits. However, the first checks in the interops do notrequire cuda initialization, so we only need to call
cuInitafter those checks.cuInitis handled by the newcuda_priv_initfunction. It ensurescuInitis only called once.With these changes, there's no cuda initialization if no OpenGL/Vulkan backend is available. This prevents
dmabuf_waylandand other VOs which automatically load cuda from waking up the nvidia dgpu unnecessarily,making them start faster and decreasing power consumption on laptops.
Fixes: #13668