* Re: [PATCH] drm/virtio: Support drm_panic with non-vmapped shmem BO
2025-04-12 13:20 [PATCH] drm/virtio: Support drm_panic with non-vmapped shmem BO Ryosuke Yasuoka
@ 2025-04-16 7:02 ` Dmitry Osipenko
2025-04-16 7:05 ` Thomas Zimmermann
2025-04-16 7:29 ` Dmitry Osipenko
2 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2025-04-16 7:02 UTC (permalink / raw)
To: Ryosuke Yasuoka, airlied, kraxel, gurchetansingh, olvaffe,
maarten.lankhorst, mripard, tzimmermann, simona, jfalempe
Cc: virtualization, linux-kernel, dri-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/virtio: Support drm_panic with non-vmapped shmem BO
2025-04-12 13:20 [PATCH] drm/virtio: Support drm_panic with non-vmapped shmem BO Ryosuke Yasuoka
2025-04-16 7:02 ` Dmitry Osipenko
@ 2025-04-16 7:05 ` Thomas Zimmermann
2025-04-16 7:08 ` Dmitry Osipenko
2025-04-16 7:29 ` Dmitry Osipenko
2 siblings, 1 reply; 6+ messages in thread
From: Thomas Zimmermann @ 2025-04-16 7:05 UTC (permalink / raw)
To: Ryosuke Yasuoka, airlied, kraxel, dmitry.osipenko,
gurchetansingh, olvaffe, maarten.lankhorst, mripard, simona,
jfalempe
Cc: virtualization, linux-kernel, dri-devel
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)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/virtio: Support drm_panic with non-vmapped shmem BO
2025-04-16 7:05 ` Thomas Zimmermann
@ 2025-04-16 7:08 ` Dmitry Osipenko
0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2025-04-16 7:08 UTC (permalink / raw)
To: Thomas Zimmermann, Ryosuke Yasuoka, airlied, kraxel,
gurchetansingh, olvaffe, maarten.lankhorst, mripard, simona,
jfalempe
Cc: virtualization, linux-kernel, dri-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/virtio: Support drm_panic with non-vmapped shmem BO
2025-04-12 13:20 [PATCH] drm/virtio: Support drm_panic with non-vmapped shmem BO Ryosuke Yasuoka
2025-04-16 7:02 ` Dmitry Osipenko
2025-04-16 7:05 ` Thomas Zimmermann
@ 2025-04-16 7:29 ` Dmitry Osipenko
2025-04-16 8:16 ` Ryosuke Yasuoka
2 siblings, 1 reply; 6+ messages in thread
From: Dmitry Osipenko @ 2025-04-16 7:29 UTC (permalink / raw)
To: Ryosuke Yasuoka, airlied, kraxel, gurchetansingh, olvaffe,
maarten.lankhorst, mripard, tzimmermann, simona, jfalempe
Cc: virtualization, linux-kernel, dri-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/virtio: Support drm_panic with non-vmapped shmem BO
2025-04-16 7:29 ` Dmitry Osipenko
@ 2025-04-16 8:16 ` Ryosuke Yasuoka
0 siblings, 0 replies; 6+ messages in thread
From: Ryosuke Yasuoka @ 2025-04-16 8:16 UTC (permalink / raw)
To: Dmitry Osipenko, tzimmermann
Cc: airlied, kraxel, gurchetansingh, olvaffe, maarten.lankhorst,
mripard, simona, jfalempe, virtualization, linux-kernel,
dri-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread