Re: [PATCH v2] drm/virtio: Add option to disable KMS support

From: Rob Clark
Date: Mon Feb 27 2023 - 11:56:43 EST

Next message: Arnd Bergmann: "[GIT PULL 1/2] ARM: SoC drivers for 6.3"
Previous message: Paul E. McKenney: "Re: [PATCH RFC bootconfig] Allow forcing unconditional bootconfig processing"
In reply to: Daniel Vetter: "Re: [PATCH v2] drm/virtio: Add option to disable KMS support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Mon, Feb 27, 2023 at 8:16 AM Daniel Vetter <daniel@xxxxxxxx> wrote:
>
> On Mon, Feb 27, 2023 at 08:01:13AM -0800, Rob Clark wrote:
> > From: Rob Clark <robdclark@xxxxxxxxxxxx>
> >
> > Add a build option to disable modesetting support. This is useful in
> > cases where the guest only needs to use the GPU in a headless mode, or
> > (such as in the CrOS usage) window surfaces are proxied to a host
> > compositor.
> >
> > v2: Use more if (IS_ENABLED(...))
> >
> > Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx>
>
> This feels a bit much like a worksforus solution. Not objecting to landing
> this, but would some kind of feature bit on the virtio hw and
> autodetection in the guest driver side work? Especially if people ever
> want to get this to a Just Works model with standard distros.

I could probably make this also work if the host advertises zero
scanouts. But I don't expect distro's would want to disable this
option, which is why it is "If unsure, say Y". The CrOS guest kernel
already needs a special "virtio-wl" driver, so we are already
venturing outside of "guest is just a generic distro" territory.

BR,
-R

> Usually the argument for compile option is "binary size", but you're
> leaving most of the kms stuff in there so that's clearly not it :-)
> -Daniel
>
>
>
> > ---
> > drivers/gpu/drm/virtio/Kconfig | 11 +++++++++
> > drivers/gpu/drm/virtio/Makefile | 5 +++-
> > drivers/gpu/drm/virtio/virtgpu_drv.c | 6 ++++-
> > drivers/gpu/drm/virtio/virtgpu_drv.h | 10 ++++++++
> > drivers/gpu/drm/virtio/virtgpu_kms.c | 35 ++++++++++++++++------------
> > 5 files changed, 50 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig
> > index 51ec7c3240c9..ea06ff2aa4b4 100644
> > --- a/drivers/gpu/drm/virtio/Kconfig
> > +++ b/drivers/gpu/drm/virtio/Kconfig
> > @@ -11,3 +11,14 @@ config DRM_VIRTIO_GPU
> > QEMU based VMMs (like KVM or Xen).
> >
> > If unsure say M.
> > +
> > +config DRM_VIRTIO_GPU_KMS
> > + bool "Virtio GPU driver modesetting support"
> > + depends on DRM_VIRTIO_GPU
> > + default y
> > + help
> > + Enable modesetting support for virtio GPU driver. This can be
> > + disabled in cases where only "headless" usage of the GPU is
> > + required.
> > +
> > + If unsure, say Y.
> > diff --git a/drivers/gpu/drm/virtio/Makefile b/drivers/gpu/drm/virtio/Makefile
> > index b99fa4a73b68..24c7ebe87032 100644
> > --- a/drivers/gpu/drm/virtio/Makefile
> > +++ b/drivers/gpu/drm/virtio/Makefile
> > @@ -4,8 +4,11 @@
> > # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
> >
> > virtio-gpu-y := virtgpu_drv.o virtgpu_kms.o virtgpu_gem.o virtgpu_vram.o \
> > - virtgpu_display.o virtgpu_vq.o \
> > + virtgpu_vq.o \
> > virtgpu_fence.o virtgpu_object.o virtgpu_debugfs.o virtgpu_plane.o \
> > virtgpu_ioctl.o virtgpu_prime.o virtgpu_trace_points.o
> >
> > +virtio-gpu-$(CONFIG_DRM_VIRTIO_GPU_KMS) += \
> > + virtgpu_display.o
> > +
> > obj-$(CONFIG_DRM_VIRTIO_GPU) += virtio-gpu.o
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
> > index ae97b98750b6..9cb7d6dd3da6 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
> > @@ -172,7 +172,11 @@ MODULE_AUTHOR("Alon Levy");
> > DEFINE_DRM_GEM_FOPS(virtio_gpu_driver_fops);
> >
> > static const struct drm_driver driver = {
> > - .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_RENDER | DRIVER_ATOMIC,
> > + .driver_features =
> > +#if defined(CONFIG_DRM_VIRTIO_GPU_KMS)
> > + DRIVER_MODESET | DRIVER_ATOMIC |
> > +#endif
> > + DRIVER_GEM | DRIVER_RENDER,
> > .open = virtio_gpu_driver_open,
> > .postclose = virtio_gpu_driver_postclose,
> >
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> > index af6ffb696086..ffe8faf67247 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> > @@ -426,8 +426,18 @@ virtio_gpu_cmd_set_scanout_blob(struct virtio_gpu_device *vgdev,
> > uint32_t x, uint32_t y);
> >
> > /* virtgpu_display.c */
> > +#if defined(CONFIG_DRM_VIRTIO_GPU_KMS)
> > int virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev);
> > void virtio_gpu_modeset_fini(struct virtio_gpu_device *vgdev);
> > +#else
> > +static inline int virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev)
> > +{
> > + return 0;
> > +}
> > +static inline void virtio_gpu_modeset_fini(struct virtio_gpu_device *vgdev)
> > +{
> > +}
> > +#endif
> >
> > /* virtgpu_plane.c */
> > uint32_t virtio_gpu_translate_format(uint32_t drm_fourcc);
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> > index 27b7f14dae89..70d87e653d07 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> > @@ -161,7 +161,8 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
> > if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_VIRGL))
> > vgdev->has_virgl_3d = true;
> > #endif
> > - if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_EDID)) {
> > + if (IS_ENABLED(CONFIG_DRM_VIRTIO_GPU_KMS) &&
> > + virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_EDID)) {
> > vgdev->has_edid = true;
> > }
> > if (virtio_has_feature(vgdev->vdev, VIRTIO_RING_F_INDIRECT_DESC)) {
> > @@ -218,17 +219,19 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
> > goto err_vbufs;
> > }
> >
> > - /* get display info */
> > - virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
> > - num_scanouts, &num_scanouts);
> > - vgdev->num_scanouts = min_t(uint32_t, num_scanouts,
> > - VIRTIO_GPU_MAX_SCANOUTS);
> > - if (!vgdev->num_scanouts) {
> > - DRM_ERROR("num_scanouts is zero\n");
> > - ret = -EINVAL;
> > - goto err_scanouts;
> > + if (IS_ENABLED(CONFIG_DRM_VIRTIO_GPU_KMS)) {
> > + /* get display info */
> > + virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
> > + num_scanouts, &num_scanouts);
> > + vgdev->num_scanouts = min_t(uint32_t, num_scanouts,
> > + VIRTIO_GPU_MAX_SCANOUTS);
> > + if (!vgdev->num_scanouts) {
> > + DRM_ERROR("num_scanouts is zero\n");
> > + ret = -EINVAL;
> > + goto err_scanouts;
> > + }
> > + DRM_INFO("number of scanouts: %d\n", num_scanouts);
> > }
> > - DRM_INFO("number of scanouts: %d\n", num_scanouts);
> >
> > virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
> > num_capsets, &num_capsets);
> > @@ -246,10 +249,12 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
> > virtio_gpu_get_capsets(vgdev, num_capsets);
> > if (vgdev->has_edid)
> > virtio_gpu_cmd_get_edids(vgdev);
> > - virtio_gpu_cmd_get_display_info(vgdev);
> > - virtio_gpu_notify(vgdev);
> > - wait_event_timeout(vgdev->resp_wq, !vgdev->display_info_pending,
> > - 5 * HZ);
> > + if (IS_ENABLED(CONFIG_DRM_VIRTIO_GPU_KMS)) {
> > + virtio_gpu_cmd_get_display_info(vgdev);
> > + virtio_gpu_notify(vgdev);
> > + wait_event_timeout(vgdev->resp_wq, !vgdev->display_info_pending,
> > + 5 * HZ);
> > + }
> > return 0;
> >
> > err_scanouts:
> > --
> > 2.39.1
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

Next message: Arnd Bergmann: "[GIT PULL 1/2] ARM: SoC drivers for 6.3"
Previous message: Paul E. McKenney: "Re: [PATCH RFC bootconfig] Allow forcing unconditional bootconfig processing"
In reply to: Daniel Vetter: "Re: [PATCH v2] drm/virtio: Add option to disable KMS support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]