| Summary: | KMSDRM: using atomic mode setting breaks GPU compatibility | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Substring <frog2wah> |
| Component: | video | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | critical | ||
| Priority: | P2 | CC: | sezeroz |
| Version: | 2.0.13 | Keywords: | target-2.0.14 |
| Hardware: | x86_64 | ||
| OS: | Linux | ||
| Attachments: |
test patch
test patch, v2 test patch, v3 |
||
|
Description
Substring
2020-12-11 21:53:19 UTC
(In reply to Substring from comment #0) > Hi, > > Seeing the call for 2.0.14 and no attention to my forum post, I'm filing a > bug. > > I was trying the KMSDRM video backend with some very simple programs that > were working ok on 2.0.12. The same code won’t work on the current dev > branch and I get: > > DEBUG: check_modesetting: probing “/dev/dri/card0” > DEBUG: /dev/dri/card0 connector, encoder and CRTC counts are: 4 5 6 > DEBUG: check_modesetting: probing “/dev/dri/card0” > DEBUG: /dev/dri/card0 connector, encoder and CRTC counts are: 4 5 6 > DEBUG: KMSDRM_VideoInit() > DEBUG: Opening device /dev/dri/card0 > DEBUG: Opened DRM FD (3) > DEBUG: no atomic modesetting support. > DEBUG: Video subsystem has not been initialized > INFO: Using SDL video driver: (null) > DEBUG: Video subsystem has not been initialized > > After carefully checking, the radeon driver doesn’t support atomic > modesetting. That’s not the only problem : the same happens with the amdgpu > driver if we disable Display Core (kernel parameter amdgpu.dc=0, which is > required to get analogue outputs working). > > This is a major regression in the KMSDRM driver. > > Using atomic mode setting is great, but having no fallback to the "standard > KMS" is bad. Hi, thanks for the report I may try to implement the legacy interface as a fallback sometime in the future, but just now I am swamped with another problem in the backend: https://bugzilla.libsdl.org/show_bug.cgi?id=5350#add_comment After finishing this, I won't have much time for some months. So I think it's ATOMIC for now. I will try to implement the fallback as soon as possible. To be honest, this surprises me (in a bad way). I went ATOMIC because I thought the legacy interface had it's days numbered, and that it was supported by all major drivers (AMDGPU, Intel, VC4, Freedreno...). Looks like RADEON went outside my radar. Really sorry! Maybe it's just a matter of pointing to the legacy functions in KMSDRM_CreateDevice() when ATOMIC is not supported. But we are too near to release date I think. I don't know what could be done, it worries me that the next release has such a compatibility regression, but there's not much time... If it's that simple of a fix, can you implement that and we can test it over the next couple of days? Okay, I don't have a KMSDRM setup to test, but I added back the old KMSDRM driver as a fallback if the new one doesn't work: https://hg.libsdl.org/SDL/rev/45c0170eec61 Can you try the latest snapshot and see if that a) builds, and b) works? http://www.libsdl.org/tmp/SDL-2.0.zip Thanks! (In reply to Sam Lantinga from comment #4) > Okay, I don't have a KMSDRM setup to test, but I added back the old KMSDRM > driver as a fallback if the new one doesn't work: > https://hg.libsdl.org/SDL/rev/45c0170eec61 Seeing this is 2.0.12 code, I pushed two post-2.0.12 fixes to it: https://hg.libsdl.org/SDL/rev/8151e3dbaeb5 Thanks! Pushed multiple build fixes to kmsdrm_legacy: https://hg.libsdl.org/SDL/rev/0b115e06e6b4 Please review the commit carefully (easy to make a mistake in that one.) (In reply to Sam Lantinga from comment #4) > Okay, I don't have a KMSDRM setup to test, but I added back the old KMSDRM > driver as a fallback if the new one doesn't work: > https://hg.libsdl.org/SDL/rev/45c0170eec61 > > Can you try the latest snapshot and see if that a) builds, and b) works? > http://www.libsdl.org/tmp/SDL-2.0.zip > > Thanks! As reported on the forum: it defaults to the KMSDRM driver and fails the same way. The only way round was to force the SDL_VIDEODRIVER to KMSDRM_LEGACY. So the workaround really close. Created attachment 4585 [details] test patch (In reply to Substring from comment #8) > As reported on the forum: it defaults to the KMSDRM driver and fails the > same way. The only way round was to force the SDL_VIDEODRIVER to > KMSDRM_LEGACY. So the workaround really close. Can we not add “Try ATOMIC compatibility” check from KMSDRM_VideoInit() to check_modesetting(), so that KMSDRM_CreateDevice() fails and then SDL_VideoInit() moves on to next bootstrap member which (the legacy one)? I.e. something like in the attached test.patch The patch is NOT even compile-tested. Must be tested to see, that: - must be correct and must compile ;) - the new atomic version works if appropriate drivers are available, - must fallback to kmsdrm_legacy, if not. Created attachment 4587 [details] test patch, v2 (In reply to Ozkan Sezer from comment #9) > (In reply to Substring from comment #8) > > As reported on the forum: it defaults to the KMSDRM driver and fails the > > same way. The only way round was to force the SDL_VIDEODRIVER to > > KMSDRM_LEGACY. So the workaround really close. > > Can we not add “Try ATOMIC compatibility” check from KMSDRM_VideoInit() > to check_modesetting(), so that KMSDRM_CreateDevice() fails and then > SDL_VideoInit() moves on to next bootstrap member which (the legacy one)? > I.e. something like in the attached test.patch Arguably better patch, this time compile-tested. Must be run-tested to see, that: - the new atomic version works if appropriate drivers are available, - must fallback to kmsdrm_legacy, if not. Created attachment 4588 [details]
test patch, v3
Revised, v3 patch. Same comments as above
tested test_patch_v3. It properly selects the right video backend between KMSDRM and KMSDRM_LEGACY. (In reply to Substring from comment #12) > tested test_patch_v3. > > It properly selects the right video backend between KMSDRM and KMSDRM_LEGACY. Thanks. Patch applied as https://hg.libsdl.org/SDL/rev/a4eb1257a330 Manuel Alfayate Corchete: Can you give it a look and try to verify nothing broke? (Remember that the patch is applied to new kms/drm code.) This seems to have been fixed. Closing. |