Re: [PATCH v5] drm/virtio: Add drm_panic support

[Dmitry Osipenko <dmitry.osipenko@collabora.com>]

On 12/2/24 17:29, Dmitry Osipenko wrote:
> On 11/29/24 15:24, Ryosuke Yasuoka wrote:
> ....
>> +static int virtio_drm_get_scanout_buffer(struct drm_plane *plane,
>> +					 struct drm_scanout_buffer *sb)
>> +{
>> +	struct virtio_gpu_object *bo;
>> +
>> +	if (!plane->state || !plane->state->fb || !plane->state->visible)
>> +		return -ENODEV;
>> +
>> +	bo = gem_to_virtio_gpu_obj(plane->state->fb->obj[0]);
>> +	if (virtio_gpu_is_vram(bo))
>> +		return -ENODEV;
> 
> VirtIO-GPU now supports importing external dmabufs, we should reject
> bo->base.base.import_attach here now too.
> 
>> +
>> +	/* try to vmap it if possible */
>> +	if (!bo->base.vaddr) {
>> +		int ret;
>> +
>> +		ret = drm_gem_shmem_vmap(&bo->base, &sb->map[0]);
>> +		if (ret)
>> +			return ret;
> 
> I've now noticed that drm_gem_shmem_vmap() expects BO reservation lock
> to be held and we can't take lock it at a panic time.
> 
> https://elixir.bootlin.com/linux/v6.12.1/source/drivers/gpu/drm/drm_gem_shmem_helper.c#L330
> 
> This resv warning isn't triggered because bo->base.vaddr is set for VT
> framebufffer BO when panic happens.
> 
> We actually should reject all BOs that don't have bo->base.vaddr set at
> the panic time. Please correct it in v6 and rebase on top of a recent
> drm-next tree.

Think ideally we need to have a pre-allocated and pre-mapped BO. Then
when panic happens, use that BO and tell host to display it, i.e. not to
reuse currently displayed BO. This will make panic display work in all
conditions. WDYT?

-- 
Best regards,
Dmitry