[PATCH v7 5/9] drm/fb_dma: Add generic get_scanout_buffer() for drm_panic

[Maxime Ripard]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20240112/5dfd7308/attachment.sig>