On Fri, Oct 16, 2020 at 11:30:04AM +0200, Daniel Vetter wrote:
On Fri, Oct 16, 2020 at 10:54 AM Shawn Guo shawn.guo@linaro.org wrote:
On Fri, Oct 16, 2020 at 09:58:46AM +0200, Daniel Vetter wrote:
On Fri, Oct 16, 2020 at 9:13 AM Shawn Guo shawn.guo@linaro.org wrote:
Commit 5caa0feafcc6 ("drm/vblank: Lock down vblank->hwmode more") added WARN_ON_ONCE() for atomic drivers to warn the case that vsync is enabled before a mode has been set on CRTC. This happens sometimes during the initial mode setting of a CRTC. It also happens on Android running HWC2 backed with drm_hwcomposer, where HWC2::SetVsyncEnabled could be called before the atomic mode setting on CRTC happens.
In this case, there is nothing really bad to happen as kernel function returns as no-op. So using WARN() version might be overkilled, considering some user space crash reporting services may treat kernel WARNINGS as crashes. Let's drop WARN_ON_ONCE() and change drm_dbg_core() to drm_warn_once() for warning undefined mode timing.
This indicates a bug in your driver. Please fix it there, not by shutting up the core code complaining about that. Either you're getting vblank timestamps when the vblank isn't set up yet (drm_crtc_vblank_on/off) or there's some other race going on in your driver code resulting in this.
Thanks for the comment, Daniel.
I'm hitting this warning on an Android running drm_hwcomposer. I'm indeed getting vblank timestamps request before drm_crtc_vblank_on() is called. I'm not sure this is a bug or race condition in the driver code, as both vblank timestamps and on/off requests are coming from user space ioctl for my case. @Sean, that means the problem is in Android drm_hwcomposer code?
vblank request when the crtc is off should be rejected. Most drivers got this wrong before I added the required drm_crtc_vblank_reset() into atomic helpers in 51f644b40b4b ("drm/atomic-helper: reset vblank on crtc reset")
Please make sure you have that, and that drm_crtc_vblank_reset is run at driver load time. If the crtc is off, vblank ioctl should be rejected. So this is definitely not a userspace bug, still a driver bug. In general, userspace is not allowed to do anything that results in dmesg spam at normal log levels. Anytime that happens it's a kernel bug. And if it's a warning in core code, it's most likely a driver bug since the core code tends to be better debugged about these things. But there's ofc exceptions.
Indeed! Adding drm_crtc_vblank_reset() into driver crtc reset hook removes the WARNING for me. Really appreciate your comments, Daniel!
Shawn