On Fri, Jan 12, 2024 at 02:41:53PM +0100, Daniel Vetter wrote: > > + fb = plane->state->fb; > > + /* Only support linear modifier */ > > + if (fb->modifier != DRM_FORMAT_MOD_LINEAR) > > + continue; > > + > > + /* Check if color format is supported */ > > + if (!drm_panic_is_format_supported(fb->format->format)) > > + continue; > > + > > + dma_obj = drm_fb_dma_get_gem_obj(fb, 0); > > + > > + /* Buffer should be accessible from the CPU */ > > + if (dma_obj->base.import_attach) > > This might be a bit too restrictive, since some drivers import dma-buf > including a vmap. So just checking for ->vaddr might be better. But can be > changed later on. > > > + continue; > > + > > + /* Buffer should be already mapped to CPU */ > > I'd clarify this comment to state that vaddr is invariant over the > lifetime of the buffer and therefore needs no locking. Correct locking > that a) takes all the locks b) never ever stalls for one is absolutely > crucial for a panic handler that won't make the situation worse. I think this comment was made to address buffers that are accessible to the CPU but don't have a CPU mapping (ie, created with DMA_ATTR_NO_KERNEL_MAPPING for example). Maxime
Attachment:
signature.asc
Description: PGP signature
