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

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Feb 27, 2023 at 08:01:13AM -0800, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> 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@chromium.org>

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.

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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization