drm/virtio: Support drm_panic with non-vmapped shmem BO

[PATCH] drm/virtio: Support drm_panic with non-vmapped shmem BO

Posted by Ryosuke Yasuoka 8 months, 1 week ago

Pass array of pages of the scanout buffer [1] to shmem BO, allowing
drm_panic to work even if the BO is not vmapped.

[1] https://lore.kernel.org/all/20250407140138.162383-3-jfalempe@redhat.com/

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
---
 drivers/gpu/drm/virtio/virtgpu_plane.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index a6f5a78f436a..2ff57d559c86 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -500,11 +500,19 @@ static int virtio_drm_get_scanout_buffer(struct drm_plane *plane,
 
 	bo = gem_to_virtio_gpu_obj(plane->state->fb->obj[0]);
 
-	/* Only support mapped shmem bo */
-	if (virtio_gpu_is_vram(bo) || bo->base.base.import_attach || !bo->base.vaddr)
+	if (virtio_gpu_is_vram(bo) || bo->base.base.import_attach)
 		return -ENODEV;
 
-	iosys_map_set_vaddr(&sb->map[0], bo->base.vaddr);
+	if (bo->base.vaddr)
+		iosys_map_set_vaddr(&sb->map[0], bo->base.vaddr);
+	else {
+		struct drm_gem_shmem_object *shmem = &bo->base;
+
+		if (!shmem->pages)
+			return -ENODEV;
+		/* map scanout buffer later */
+		sb->pages = shmem->pages;
+	}
 
 	sb->format = plane->state->fb->format;
 	sb->height = plane->state->fb->height;

base-commit: e7bb7d44c3b97aea1f0e354c6499900154ac67f2
-- 
2.49.0

Re: [PATCH] drm/virtio: Support drm_panic with non-vmapped shmem BO

Posted by Dmitry Osipenko 8 months, 1 week ago

On 4/12/25 16:20, Ryosuke Yasuoka wrote:
> Pass array of pages of the scanout buffer [1] to shmem BO, allowing
> drm_panic to work even if the BO is not vmapped.
> 
> [1] https://lore.kernel.org/all/20250407140138.162383-3-jfalempe@redhat.com/
> 
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> ---
>  drivers/gpu/drm/virtio/virtgpu_plane.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index a6f5a78f436a..2ff57d559c86 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -500,11 +500,19 @@ static int virtio_drm_get_scanout_buffer(struct drm_plane *plane,
>  
>  	bo = gem_to_virtio_gpu_obj(plane->state->fb->obj[0]);
>  
> -	/* Only support mapped shmem bo */
> -	if (virtio_gpu_is_vram(bo) || bo->base.base.import_attach || !bo->base.vaddr)
> +	if (virtio_gpu_is_vram(bo) || bo->base.base.import_attach)
>  		return -ENODEV;
>  
> -	iosys_map_set_vaddr(&sb->map[0], bo->base.vaddr);
> +	if (bo->base.vaddr)
> +		iosys_map_set_vaddr(&sb->map[0], bo->base.vaddr);
> +	else {
> +		struct drm_gem_shmem_object *shmem = &bo->base;
> +
> +		if (!shmem->pages)
> +			return -ENODEV;
> +		/* map scanout buffer later */
> +		sb->pages = shmem->pages;
> +	}
>  
>  	sb->format = plane->state->fb->format;
>  	sb->height = plane->state->fb->height;
> 
> base-commit: e7bb7d44c3b97aea1f0e354c6499900154ac67f2

Rebased on top of [1], fixed checkpatch warning about braces and applied
to misc-next. Thanks!

[1] https://lore.kernel.org/all/20250414131507.566072-1-tzimmermann@suse.de/

-- 
Best regards,
Dmitry

Re: [PATCH] drm/virtio: Support drm_panic with non-vmapped shmem BO

Posted by Ryosuke Yasuoka 8 months, 1 week ago

On Wed, Apr 16, 2025 at 4:29 PM Dmitry Osipenko
<dmitry.osipenko@collabora.com> wrote:
>
> On 4/12/25 16:20, Ryosuke Yasuoka wrote:
> > Pass array of pages of the scanout buffer [1] to shmem BO, allowing
> > drm_panic to work even if the BO is not vmapped.
> >
> > [1] https://lore.kernel.org/all/20250407140138.162383-3-jfalempe@redhat.com/
> >
> > Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> > Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> > ---
> >  drivers/gpu/drm/virtio/virtgpu_plane.c | 14 +++++++++++---
> >  1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
> > index a6f5a78f436a..2ff57d559c86 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> > @@ -500,11 +500,19 @@ static int virtio_drm_get_scanout_buffer(struct drm_plane *plane,
> >
> >       bo = gem_to_virtio_gpu_obj(plane->state->fb->obj[0]);
> >
> > -     /* Only support mapped shmem bo */
> > -     if (virtio_gpu_is_vram(bo) || bo->base.base.import_attach || !bo->base.vaddr)
> > +     if (virtio_gpu_is_vram(bo) || bo->base.base.import_attach)
> >               return -ENODEV;
> >
> > -     iosys_map_set_vaddr(&sb->map[0], bo->base.vaddr);
> > +     if (bo->base.vaddr)
> > +             iosys_map_set_vaddr(&sb->map[0], bo->base.vaddr);
> > +     else {
> > +             struct drm_gem_shmem_object *shmem = &bo->base;
> > +
> > +             if (!shmem->pages)
> > +                     return -ENODEV;
> > +             /* map scanout buffer later */
> > +             sb->pages = shmem->pages;
> > +     }
> >
> >       sb->format = plane->state->fb->format;
> >       sb->height = plane->state->fb->height;
> >
> > base-commit: e7bb7d44c3b97aea1f0e354c6499900154ac67f2
>
> Rebased on top of [1], fixed checkpatch warning about braces and applied
> to misc-next. Thanks!
>
> [1] https://lore.kernel.org/all/20250414131507.566072-1-tzimmermann@suse.de/

Thomas,
Thank you for your comment.

Dmitry,
Thank you for fixing it and applying my patch.

> --
> Best regards,
> Dmitry
>

Best regards,
Ryosuke

Re: [PATCH] drm/virtio: Support drm_panic with non-vmapped shmem BO

Posted by Thomas Zimmermann 8 months, 1 week ago

Hi

Am 12.04.25 um 15:20 schrieb Ryosuke Yasuoka:
> Pass array of pages of the scanout buffer [1] to shmem BO, allowing
> drm_panic to work even if the BO is not vmapped.
>
> [1] https://lore.kernel.org/all/20250407140138.162383-3-jfalempe@redhat.com/
>
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> ---
>   drivers/gpu/drm/virtio/virtgpu_plane.c | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index a6f5a78f436a..2ff57d559c86 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -500,11 +500,19 @@ static int virtio_drm_get_scanout_buffer(struct drm_plane *plane,
>   
>   	bo = gem_to_virtio_gpu_obj(plane->state->fb->obj[0]);
>   
> -	/* Only support mapped shmem bo */
> -	if (virtio_gpu_is_vram(bo) || bo->base.base.import_attach || !bo->base.vaddr)
> +	if (virtio_gpu_is_vram(bo) || bo->base.base.import_attach)

Please adopt drm_gem_is_imported() instead of testing import_attach 
directly. The helper is available in v6.15-rc1 and later.

Best regards
Thomas

>   		return -ENODEV;
>   
> -	iosys_map_set_vaddr(&sb->map[0], bo->base.vaddr);
> +	if (bo->base.vaddr)
> +		iosys_map_set_vaddr(&sb->map[0], bo->base.vaddr);
> +	else {
> +		struct drm_gem_shmem_object *shmem = &bo->base;
> +
> +		if (!shmem->pages)
> +			return -ENODEV;
> +		/* map scanout buffer later */
> +		sb->pages = shmem->pages;
> +	}
>   
>   	sb->format = plane->state->fb->format;
>   	sb->height = plane->state->fb->height;
>
> base-commit: e7bb7d44c3b97aea1f0e354c6499900154ac67f2

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH] drm/virtio: Support drm_panic with non-vmapped shmem BO

Posted by Dmitry Osipenko 8 months, 1 week ago

On 4/16/25 10:05, Thomas Zimmermann wrote:
> Hi
> 
> Am 12.04.25 um 15:20 schrieb Ryosuke Yasuoka:
>> Pass array of pages of the scanout buffer [1] to shmem BO, allowing
>> drm_panic to work even if the BO is not vmapped.
>>
>> [1] https://lore.kernel.org/all/20250407140138.162383-3-
>> jfalempe@redhat.com/
>>
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
>> ---
>>   drivers/gpu/drm/virtio/virtgpu_plane.c | 14 +++++++++++---
>>   1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/
>> virtio/virtgpu_plane.c
>> index a6f5a78f436a..2ff57d559c86 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
>> @@ -500,11 +500,19 @@ static int virtio_drm_get_scanout_buffer(struct
>> drm_plane *plane,
>>         bo = gem_to_virtio_gpu_obj(plane->state->fb->obj[0]);
>>   -    /* Only support mapped shmem bo */
>> -    if (virtio_gpu_is_vram(bo) || bo->base.base.import_attach || !bo-
>> >base.vaddr)
>> +    if (virtio_gpu_is_vram(bo) || bo->base.base.import_attach)
> 
> Please adopt drm_gem_is_imported() instead of testing import_attach
> directly. The helper is available in v6.15-rc1 and later.

Hi, I rebased this patch locally on top of your drm_gem_is_imported()
changes and will merge it all soon.

-- 
Best regards,
Dmitry

Re: [PATCH] drm/virtio: Support drm_panic with non-vmapped shmem BO

Posted by Dmitry Osipenko 8 months, 1 week ago

On 4/12/25 16:20, Ryosuke Yasuoka wrote:
> Pass array of pages of the scanout buffer [1] to shmem BO, allowing
> drm_panic to work even if the BO is not vmapped.
> 
> [1] https://lore.kernel.org/all/20250407140138.162383-3-jfalempe@redhat.com/
> 
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> ---
>  drivers/gpu/drm/virtio/virtgpu_plane.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index a6f5a78f436a..2ff57d559c86 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -500,11 +500,19 @@ static int virtio_drm_get_scanout_buffer(struct drm_plane *plane,
>  
>  	bo = gem_to_virtio_gpu_obj(plane->state->fb->obj[0]);
>  
> -	/* Only support mapped shmem bo */
> -	if (virtio_gpu_is_vram(bo) || bo->base.base.import_attach || !bo->base.vaddr)
> +	if (virtio_gpu_is_vram(bo) || bo->base.base.import_attach)
>  		return -ENODEV;
>  
> -	iosys_map_set_vaddr(&sb->map[0], bo->base.vaddr);
> +	if (bo->base.vaddr)
> +		iosys_map_set_vaddr(&sb->map[0], bo->base.vaddr);
> +	else {
> +		struct drm_gem_shmem_object *shmem = &bo->base;
> +
> +		if (!shmem->pages)
> +			return -ENODEV;
> +		/* map scanout buffer later */
> +		sb->pages = shmem->pages;
> +	}
>  
>  	sb->format = plane->state->fb->format;
>  	sb->height = plane->state->fb->height;
> 
> base-commit: e7bb7d44c3b97aea1f0e354c6499900154ac67f2

Works well!

Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

-- 
Best regards,
Dmitry