[PATCH] drm/drm_vblank: use drm_warn_once() to warn undefined mode timing

[Daniel Vetter]

On Fri, Oct 16, 2020 at 10:54 AM Shawn Guo <shawn.guo at 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 at 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.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch