On Tue, Mar 05, 2024 at 09:20:04AM +0106, John Ogness wrote: > Hi Daniel, > > Great to see this moving forward! > > On 2024-03-01, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote: > > But for the initial cut of a drm panic printing support I don't think > > we need that, because the critical sections are extremely small and > > only happen once per display refresh. So generally just 60 tiny locked > > sections per second, which is nothing compared to a serial console > > running a 115kbaud doing really slow mmio writes for each byte. So for > > now the raw spintrylock in drm panic notifier callback should be good > > enough. > > Is there a reason you do not use the irqsave/irqrestore variants? By > leaving interrupts enabled, there is the risk that a panic from any > interrupt handler may block the drm panic handler. tbh I simply did not consider that could be useful. but yeah if we're unlucky and an interrupt happens in here and dies, the drm panic handler cannot run. And this code is definitely not hot enough to matter, the usual driver code for a plane flip does a few more irqsafe spinlocks on top. One more doesn't add anything I think, and I guess if it does we'll notice :-) Also irqsave makes drm_panic_lock/unlock a bit more widely useful to protect driver mmio access since then it also works from irq handlers. Means we have to pass irqflags around, but that sounds acceptable. So very much has my vote. -Sima -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
