Re: [PATCH 1/4] drm/vmwgfx: Make console emulation depend on DRM_FBDEV_EMULATION

Hi

Am 15.04.21 um 20:21 schrieb Zack Rusin:

On 4/15/21 7:00 AM, Thomas Zimmermann wrote:

quoted

Respect DRM's kconfig setting for fbdev console emulation. If enabled,
it will select all required config options. So remove them from vmwgfx's
Kconfig file.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
  drivers/gpu/drm/vmwgfx/Kconfig      |  7 +------
  drivers/gpu/drm/vmwgfx/Makefile     |  

4 +++-

quoted

  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 19 +++++++++++++++++++
  3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/Kconfig 

b/drivers/gpu/drm/vmwgfx/Kconfig
index 15acdf2a7c0f..b3a34196935b 100644

--- a/drivers/gpu/drm/vmwgfx/Kconfig
+++ b/drivers/gpu/drm/vmwgfx/Kconfig

@@ -2,12 +2,7 @@

  config DRM_VMWGFX
      tristate "DRM driver for VMware Virtual 

GPU"

quoted

      depends on DRM && PCI && X86 && MMU
-    select FB_DEFERRED_IO
-    select FB_CFB_FILLRECT
-    select FB_CFB_COPYAREA
-    select FB_CFB_IMAGEBLIT
      select DRM_TTM
-    select FB
      select MAPPING_DIRTY_HELPERS
      # Only needed for the transitional use of drm_crtc_init - can be 
removed
      # again once vmwgfx sets up the primary 

plane itself.

quoted

@@ -20,7 +15,7 @@ config DRM_VMWGFX

        The compiled module will be 

called "vmwgfx.ko".

quoted

  config DRM_VMWGFX_FBCON
-    depends on DRM_VMWGFX && FB
+    depends on DRM_VMWGFX && DRM_FBDEV_EMULATION
      bool "Enable framebuffer console under vmwgfx by default"
      help
         Choose this option if 

you are shipping a new vmwgfx

quoted

diff --git a/drivers/gpu/drm/vmwgfx/Makefile 

b/drivers/gpu/drm/vmwgfx/Makefile
index 8c02fa5852e7..9f5743013cbb 100644

--- a/drivers/gpu/drm/vmwgfx/Makefile
+++ b/drivers/gpu/drm/vmwgfx/Makefile

@@ -1,6 +1,6 @@

  # SPDX-License-Identifier: GPL-2.0
  vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
-        vmwgfx_fb.o vmwgfx_ioctl.o 

vmwgfx_resource.o 

quoted

vmwgfx_ttm_buffer.o \
+        vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
          vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o vmwgfx_ttm_glue.o \
          vmwgfx_overlay.o vmwgfx_gmrid_manager.o vmwgfx_fence.o \
          vmwgfx_bo.o vmwgfx_scrn.o vmwgfx_context.o \

@@ -11,5 +11,7 @@ vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o 

vmwgfx_kms.o vmwgfx_drv.o \
          vmwgfx_validation.o vmwgfx_page_dirty.o vmwgfx_streamoutput.o \
          ttm_object.o ttm_lock.o ttm_memory.o
+vmwgfx-$(CONFIG_DRM_FBDEV_EMULATION) += vmwgfx_fb.o
  vmwgfx-$(CONFIG_TRANSPARENT_HUGEPAGE) += vmwgfx_thp.o
+
  obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h 

b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 7e6518709e14..e7836da190c4 100644

--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h

@@ -1116,10 +1116,29 @@ extern void vmw_generic_waiter_remove(struct 

vmw_private *dev_priv,
   * Kernel framebuffer - vmwgfx_fb.c
   */
+#ifdef CONFIG_DRM_FBDEV_EMULATION
  int vmw_fb_init(struct vmw_private *vmw_priv);
  int vmw_fb_close(struct vmw_private *dev_priv);
  int vmw_fb_off(struct vmw_private *vmw_priv);
  int vmw_fb_on(struct vmw_private *vmw_priv);
+#else
+static inline int vmw_fb_init(struct vmw_private *vmw_priv)
+{
+    return 0;
+}
+static inline int vmw_fb_close(struct vmw_private *dev_priv)
+{
+    return 0;
+}
+static inline int vmw_fb_off(struct vmw_private *vmw_priv)
+{
+    return 0;
+}
+static inline int vmw_fb_on(struct vmw_private *vmw_priv)
+{
+    return 0;
+}
+#endif
  /**
   * Kernel modesetting - vmwgfx_kms.c

This changes the behavior a bit, I guess DRM_VMWGFX (or at least 
DRM_VMWGFX_FBCON) would need to select DRM_FBDEV_EMULATION to preserve 
the old behavior, but that's largely due to the fact that given how 
those options were setup we never run without FB set. In general it 
should be ok and looks more reasonable than the current setup. I'll try 

it out on Monday just in case, but for now:

Reviewed-by: Zack Rusin <redacted>

All other drivers use DRM_FBDEV_EMULATION, so vmwgfx would follow common 
conventions.

AFAICT DRM_VMWGFX_FBCON is just the default on/off setting. How about 
making DRM_VMWGFX_FBCON depend on DRM_FBDEV_EMULATION? Users would be 
able to enable fbdev emulation in general and, if needed, still pick a 
separate default on/off for vmwgfx.

Best regards
Thomas

z

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer