On Tue, Jan 16, 2024 at 11:54:42AM +0100, Jocelyn Falempe wrote: > On 12/01/2024 14:31, Daniel Vetter wrote: > > You need to tie these nice kerneldocs into the overall documentation tree, > > or they're not getting built. Please then also check that all the links > > and formatting works correctly. > > oh yes, I need to add it to the Documentation/drm/xx.rst. > I'm not sure where drm panic fits there. > If I move get_scanout_buffer() pointer to the drm_mode_config struct, adding > drm_panic docs to drm-kms.rst makes sense ? Yeah I think a section in the modeset docs (not the modeset helper docs, this is fairly core concept imo) makes the most sense. > > The locking here is busted. Which is why the atomic notifier chain is > > special and uses atomic semantics - I would just avoid this issue and > > directly register each drm_panic_device instead of trying to maintain a > > drm-local list and getting the locking rules wrong. > > It's a bit complex to get the drm_device pointer in the callback. > I think I can add a "struct notifier_block" in the struct drm_mode_config, > and use the container_of macro, to retrieve it. > This also makes it easy to unregister. Yeah if you register each drm_device separately you also need to embedded the notifier chain into every single one. drm_mode_config sounds ok for that to me. I didn't realize that the kmsg_dumper api doesn't have some kind of void* argument but you need to embed the notifier in the right place, in case some of my other comments confused you. -Sima -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
