Re: [PATCH] drm/vkms: Add a DRM render node to vkms

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jan 05, 2023 at 09:52:26PM +0900, Yi Xie wrote:
> > This doesn't sound like a good idea to me. Devices without render
> > capabilities should not fake it.
> >
> > User-space (e.g. wlroots) relies on "no render node" to enable
> > software rendering (Pixman instead of GL).
> 
> We have virtgpu driver that exports a render node even when virgl is
> not supported.
> Mesa has special code path to enable software rendering on it:
> https://gitlab.freedesktop.org/mesa/mesa/-/blob/main/src/egl/drivers/dri2/platform_device.c#L296
> We can do the same for vkms to force software rendering.

Yeah that is the old kmsro mesa issue, for every combination of kms and
gem device you need one to make this work.

> On Thu, Jan 5, 2023 at 8:36 PM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Thu, Jan 05, 2023 at 02:23:25PM +0900, Yi Xie wrote:
> > > Some libraries including Mesa and virglrenderer require a render node to
> > > fully function. By adding a render node to vkms those libraries will
> > > work properly, supporting use cases like running crosvm with virgl GPU
> > > support via llvmpipe on a headless virtual machine.
> >
> > This is what vgem exists for. More or less at least ... I'm honestly not
> > really understanding what you're trying to fix here, it sounds a bit like
> > userspace being stupid.
> > -Daniel
> The problem with vgem is that it crashes llvmpipe while working with vkms.
> Looks like it's due to the same reason as described in this thread in Mesa:
> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5830

I'm not finding any bug description in there and how/why something
crashes?

> Importing buffers allocated by vgem to vkms seems to be unexpected and
> causes the crash. If we create a render node on vkms then llvmpipe will use
> vkms to allocate buffers and it no longer crashes.

Uh importing vgem into virtio might not work because those sometimes need
special buffers iirc. But importing vgem into vkms really should work,
there's no technical reason it cannot. If it doesn't, then the right fix
would be to fix that, not paper around it.
-Daniel

> 
> > >
> > > Signed-off-by: Yi Xie <yixie@google.com>
> > > ---
> > >  drivers/gpu/drm/vkms/vkms_drv.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> > > index 293dbca50c31..8eea5d4dece8 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > > @@ -113,7 +113,7 @@ static void vkms_config_debugfs_init(struct drm_minor *minor)
> > >  }
> > >
> > >  static const struct drm_driver vkms_driver = {
> > > -     .driver_features        = DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
> > > +     .driver_features        = DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM | DRIVER_RENDER,
> > >       .release                = vkms_release,
> > >       .fops                   = &vkms_driver_fops,
> > >       DRM_GEM_SHMEM_DRIVER_OPS,
> > > --
> > > 2.39.0.314.g84b9a713c41-goog
> > >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch