[PATCH 0/3] drm/vboxvideo: Use generic fbdev and framebuffer

[PATCH 0/3] drm/vboxvideo: Use generic fbdev and framebuffer

[Thomas Zimmermann]

The vboxvideo driver provides its own implementation for fbdev emulation
and framebuffers. Both can be replaced by DRM's generic code.

All patches have been tested on VirtualBox 6.0.12.

Thomas Zimmermann (3):
  drm/vboxvideo: Switch to generic fbdev emulation
  drm/vboxvideo: Switch to drm_atomic_helper_dirty_fb()
  drm/vboxvideo: Replace struct vram_framebuffer with generic
    implemenation

 drivers/gpu/drm/vboxvideo/Makefile    |   2 +-
 drivers/gpu/drm/vboxvideo/vbox_drv.c  |  14 +--
 drivers/gpu/drm/vboxvideo/vbox_drv.h  |  25 -----
 drivers/gpu/drm/vboxvideo/vbox_fb.c   | 149 --------------------------
 drivers/gpu/drm/vboxvideo/vbox_main.c | 119 +-------------------
 drivers/gpu/drm/vboxvideo/vbox_mode.c |  86 +++++++--------
 6 files changed, 49 insertions(+), 346 deletions(-)
 delete mode 100644 drivers/gpu/drm/vboxvideo/vbox_fb.c

--
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/3] drm/vboxvideo: Switch to generic fbdev emulation

[Thomas Zimmermann]

There's nothing special about vboxvideo's fbdev emulation that is
not provided by the generic implementation. Switch over and remove
the driver's code.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/vboxvideo/Makefile    |   2 +-
 drivers/gpu/drm/vboxvideo/vbox_drv.c  |  14 +--
 drivers/gpu/drm/vboxvideo/vbox_drv.h  |   7 --
 drivers/gpu/drm/vboxvideo/vbox_fb.c   | 149 --------------------------
 drivers/gpu/drm/vboxvideo/vbox_mode.c |   3 +-
 5 files changed, 6 insertions(+), 169 deletions(-)
 delete mode 100644 drivers/gpu/drm/vboxvideo/vbox_fb.c

diff --git a/drivers/gpu/drm/vboxvideo/Makefile b/drivers/gpu/drm/vboxvideo/Makefile
index 55d798c76b21..f2e968b5ffa6 100644
--- a/drivers/gpu/drm/vboxvideo/Makefile
+++ b/drivers/gpu/drm/vboxvideo/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 vboxvideo-y :=  hgsmi_base.o modesetting.o vbva_base.o \
-		vbox_drv.o vbox_fb.o vbox_hgsmi.o vbox_irq.o vbox_main.o \
+		vbox_drv.o vbox_hgsmi.o vbox_irq.o vbox_main.o \
 		vbox_mode.o vbox_ttm.o
 
 obj-$(CONFIG_DRM_VBOXVIDEO) += vboxvideo.o
diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c
index 862db495d111..6ee308b453da 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
@@ -14,6 +14,7 @@
 
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_drv.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_file.h>
 #include <drm/drm_ioctl.h>
 
@@ -32,10 +33,6 @@ static const struct pci_device_id pciidlist[] = {
 };
 MODULE_DEVICE_TABLE(pci, pciidlist);
 
-static const struct drm_fb_helper_funcs vbox_fb_helper_funcs = {
-	.fb_probe = vboxfb_create,
-};
-
 static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct vbox_private *vbox;
@@ -79,20 +76,16 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (ret)
 		goto err_mode_fini;
 
-	ret = drm_fb_helper_fbdev_setup(&vbox->ddev, &vbox->fb_helper,
-					&vbox_fb_helper_funcs, 32,
-					vbox->num_crtcs);
+	ret = drm_fbdev_generic_setup(&vbox->ddev, 32);
 	if (ret)
 		goto err_irq_fini;
 
 	ret = drm_dev_register(&vbox->ddev, 0);
 	if (ret)
-		goto err_fbdev_fini;
+		goto err_irq_fini;
 
 	return 0;
 
-err_fbdev_fini:
-	vbox_fbdev_fini(vbox);
 err_irq_fini:
 	vbox_irq_fini(vbox);
 err_mode_fini:
@@ -113,7 +106,6 @@ static void vbox_pci_remove(struct pci_dev *pdev)
 	struct vbox_private *vbox = pci_get_drvdata(pdev);
 
 	drm_dev_unregister(&vbox->ddev);
-	vbox_fbdev_fini(vbox);
 	vbox_irq_fini(vbox);
 	vbox_mode_fini(vbox);
 	vbox_mm_fini(vbox);
diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.h b/drivers/gpu/drm/vboxvideo/vbox_drv.h
index fb436ec760ea..bb0c39fe7911 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_drv.h
+++ b/drivers/gpu/drm/vboxvideo/vbox_drv.h
@@ -16,7 +16,6 @@
 #include <linux/string.h>
 
 #include <drm/drm_encoder.h>
-#include <drm/drm_fb_helper.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_gem_vram_helper.h>
 
@@ -54,8 +53,6 @@ struct vbox_framebuffer {
 struct vbox_private {
 	/* Must be first; or we must define our own release callback */
 	struct drm_device ddev;
-	struct drm_fb_helper fb_helper;
-	struct vbox_framebuffer afb;
 
 	u8 __iomem *guest_heap;
 	u8 __iomem *vbva_buffers;
@@ -155,10 +152,6 @@ int vbox_framebuffer_init(struct vbox_private *vbox,
 			  const struct drm_mode_fb_cmd2 *mode_cmd,
 			  struct drm_gem_object *obj);
 
-int vboxfb_create(struct drm_fb_helper *helper,
-		  struct drm_fb_helper_surface_size *sizes);
-void vbox_fbdev_fini(struct vbox_private *vbox);
-
 int vbox_mm_init(struct vbox_private *vbox);
 void vbox_mm_fini(struct vbox_private *vbox);
 
diff --git a/drivers/gpu/drm/vboxvideo/vbox_fb.c b/drivers/gpu/drm/vboxvideo/vbox_fb.c
deleted file mode 100644
index 8f74bcffc034..000000000000
--- a/drivers/gpu/drm/vboxvideo/vbox_fb.c
+++ /dev/null
@@ -1,149 +0,0 @@
-// SPDX-License-Identifier: MIT
-/*
- * Copyright (C) 2013-2017 Oracle Corporation
- * This file is based on ast_fb.c
- * Copyright 2012 Red Hat Inc.
- * Authors: Dave Airlie <airlied@redhat.com>
- *          Michael Thayer <michael.thayer@oracle.com,
- */
-#include <linux/delay.h>
-#include <linux/errno.h>
-#include <linux/fb.h>
-#include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/mm.h>
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/string.h>
-#include <linux/sysrq.h>
-#include <linux/tty.h>
-
-#include <drm/drm_crtc.h>
-#include <drm/drm_crtc_helper.h>
-#include <drm/drm_fb_helper.h>
-#include <drm/drm_fourcc.h>
-
-#include "vbox_drv.h"
-#include "vboxvideo.h"
-
-#ifdef CONFIG_DRM_KMS_FB_HELPER
-static struct fb_deferred_io vbox_defio = {
-	.delay = HZ / 30,
-	.deferred_io = drm_fb_helper_deferred_io,
-};
-#endif
-
-static struct fb_ops vboxfb_ops = {
-	.owner = THIS_MODULE,
-	DRM_FB_HELPER_DEFAULT_OPS,
-	.fb_fillrect = drm_fb_helper_sys_fillrect,
-	.fb_copyarea = drm_fb_helper_sys_copyarea,
-	.fb_imageblit = drm_fb_helper_sys_imageblit,
-};
-
-int vboxfb_create(struct drm_fb_helper *helper,
-		  struct drm_fb_helper_surface_size *sizes)
-{
-	struct vbox_private *vbox =
-		container_of(helper, struct vbox_private, fb_helper);
-	struct pci_dev *pdev = vbox->ddev.pdev;
-	struct drm_mode_fb_cmd2 mode_cmd;
-	struct drm_framebuffer *fb;
-	struct fb_info *info;
-	struct drm_gem_object *gobj;
-	struct drm_gem_vram_object *gbo;
-	int size, ret;
-	s64 gpu_addr;
-	u32 pitch;
-
-	mode_cmd.width = sizes->surface_width;
-	mode_cmd.height = sizes->surface_height;
-	pitch = mode_cmd.width * ((sizes->surface_bpp + 7) / 8);
-	mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
-							  sizes->surface_depth);
-	mode_cmd.pitches[0] = pitch;
-
-	size = pitch * mode_cmd.height;
-
-	ret = vbox_gem_create(vbox, size, true, &gobj);
-	if (ret) {
-		DRM_ERROR("failed to create fbcon backing object %d\n", ret);
-		return ret;
-	}
-
-	ret = vbox_framebuffer_init(vbox, &vbox->afb, &mode_cmd, gobj);
-	if (ret)
-		return ret;
-
-	gbo = drm_gem_vram_of_gem(gobj);
-
-	ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
-	if (ret)
-		return ret;
-
-	info = drm_fb_helper_alloc_fbi(helper);
-	if (IS_ERR(info))
-		return PTR_ERR(info);
-
-	info->screen_size = size;
-	info->screen_base = (char __iomem *)drm_gem_vram_kmap(gbo, true, NULL);
-	if (IS_ERR(info->screen_base))
-		return PTR_ERR(info->screen_base);
-
-	fb = &vbox->afb.base;
-	helper->fb = fb;
-
-	info->fbops = &vboxfb_ops;
-
-	/*
-	 * This seems to be done for safety checking that the framebuffer
-	 * is not registered twice by different drivers.
-	 */
-	info->apertures->ranges[0].base = pci_resource_start(pdev, 0);
-	info->apertures->ranges[0].size = pci_resource_len(pdev, 0);
-
-	drm_fb_helper_fill_info(info, helper, sizes);
-
-	gpu_addr = drm_gem_vram_offset(gbo);
-	if (gpu_addr < 0)
-		return (int)gpu_addr;
-	info->fix.smem_start = info->apertures->ranges[0].base + gpu_addr;
-	info->fix.smem_len = vbox->available_vram_size - gpu_addr;
-
-#ifdef CONFIG_DRM_KMS_FB_HELPER
-	info->fbdefio = &vbox_defio;
-	fb_deferred_io_init(info);
-#endif
-
-	info->pixmap.flags = FB_PIXMAP_SYSTEM;
-
-	DRM_DEBUG_KMS("allocated %dx%d\n", fb->width, fb->height);
-
-	return 0;
-}
-
-void vbox_fbdev_fini(struct vbox_private *vbox)
-{
-	struct vbox_framebuffer *afb = &vbox->afb;
-
-#ifdef CONFIG_DRM_KMS_FB_HELPER
-	if (vbox->fb_helper.fbdev && vbox->fb_helper.fbdev->fbdefio)
-		fb_deferred_io_cleanup(vbox->fb_helper.fbdev);
-#endif
-
-	drm_fb_helper_unregister_fbi(&vbox->fb_helper);
-
-	if (afb->obj) {
-		struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(afb->obj);
-
-		drm_gem_vram_kunmap(gbo);
-		drm_gem_vram_unpin(gbo);
-
-		drm_gem_object_put_unlocked(afb->obj);
-		afb->obj = NULL;
-	}
-	drm_fb_helper_fini(&vbox->fb_helper);
-
-	drm_framebuffer_unregister_private(&afb->base);
-	drm_framebuffer_cleanup(&afb->base);
-}
diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
index e1e48ba919eb..dd9ad3fdd919 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
@@ -13,6 +13,7 @@
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
@@ -133,7 +134,7 @@ static bool vbox_set_up_input_mapping(struct vbox_private *vbox)
 
 		if (!fb1) {
 			fb1 = fb;
-			if (to_vbox_framebuffer(fb1) == &vbox->afb)
+			if (fb1 == vbox->ddev.fb_helper->fb)
 				break;
 		} else if (fb != fb1) {
 			single_framebuffer = false;
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/vboxvideo: Switch to drm_atomic_helper_dirty_fb()

[Thomas Zimmermann]

The vboxvideo driver provides struct drm_framebuffer_funcs.dirty_fb from
its own implementation. Switch over to drm_atomic_helper_dirty_fb() and
handle screen updates in the primary plane's atomic_update function.

With dirty_fb out of the way, we can further replace struct vbox_frammebuffer
with generic code.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/vboxvideo/vbox_drv.h  |  4 --
 drivers/gpu/drm/vboxvideo/vbox_main.c | 61 +--------------------------
 drivers/gpu/drm/vboxvideo/vbox_mode.c | 34 +++++++++++++++
 3 files changed, 36 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.h b/drivers/gpu/drm/vboxvideo/vbox_drv.h
index bb0c39fe7911..9976554b58cb 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_drv.h
+++ b/drivers/gpu/drm/vboxvideo/vbox_drv.h
@@ -143,10 +143,6 @@ void vbox_mode_fini(struct vbox_private *vbox);
 
 void vbox_report_caps(struct vbox_private *vbox);
 
-void vbox_framebuffer_dirty_rectangles(struct drm_framebuffer *fb,
-				       struct drm_clip_rect *rects,
-				       unsigned int num_rects);
-
 int vbox_framebuffer_init(struct vbox_private *vbox,
 			  struct vbox_framebuffer *vbox_fb,
 			  const struct drm_mode_fb_cmd2 *mode_cmd,
diff --git a/drivers/gpu/drm/vboxvideo/vbox_main.c b/drivers/gpu/drm/vboxvideo/vbox_main.c
index 02fa8277ff1e..ba24a9293d17 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_main.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_main.c
@@ -11,6 +11,7 @@
 #include <linux/vbox_err.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_damage_helper.h>
 
 #include "vbox_drv.h"
 #include "vboxvideo_guest.h"
@@ -38,67 +39,9 @@ void vbox_report_caps(struct vbox_private *vbox)
 	hgsmi_send_caps_info(vbox->guest_pool, caps);
 }
 
-/* Send information about dirty rectangles to VBVA. */
-void vbox_framebuffer_dirty_rectangles(struct drm_framebuffer *fb,
-				       struct drm_clip_rect *rects,
-				       unsigned int num_rects)
-{
-	struct vbox_private *vbox = fb->dev->dev_private;
-	struct drm_display_mode *mode;
-	struct drm_crtc *crtc;
-	int crtc_x, crtc_y;
-	unsigned int i;
-
-	mutex_lock(&vbox->hw_mutex);
-	list_for_each_entry(crtc, &fb->dev->mode_config.crtc_list, head) {
-		if (crtc->primary->state->fb != fb)
-			continue;
-
-		mode = &crtc->state->mode;
-		crtc_x = crtc->primary->state->src_x >> 16;
-		crtc_y = crtc->primary->state->src_y >> 16;
-
-		for (i = 0; i < num_rects; ++i) {
-			struct vbva_cmd_hdr cmd_hdr;
-			unsigned int crtc_id = to_vbox_crtc(crtc)->crtc_id;
-
-			if (rects[i].x1 > crtc_x + mode->hdisplay ||
-			    rects[i].y1 > crtc_y + mode->vdisplay ||
-			    rects[i].x2 < crtc_x ||
-			    rects[i].y2 < crtc_y)
-				continue;
-
-			cmd_hdr.x = (s16)rects[i].x1;
-			cmd_hdr.y = (s16)rects[i].y1;
-			cmd_hdr.w = (u16)rects[i].x2 - rects[i].x1;
-			cmd_hdr.h = (u16)rects[i].y2 - rects[i].y1;
-
-			if (!vbva_buffer_begin_update(&vbox->vbva_info[crtc_id],
-						      vbox->guest_pool))
-				continue;
-
-			vbva_write(&vbox->vbva_info[crtc_id], vbox->guest_pool,
-				   &cmd_hdr, sizeof(cmd_hdr));
-			vbva_buffer_end_update(&vbox->vbva_info[crtc_id]);
-		}
-	}
-	mutex_unlock(&vbox->hw_mutex);
-}
-
-static int vbox_user_framebuffer_dirty(struct drm_framebuffer *fb,
-				       struct drm_file *file_priv,
-				       unsigned int flags, unsigned int color,
-				       struct drm_clip_rect *rects,
-				       unsigned int num_rects)
-{
-	vbox_framebuffer_dirty_rectangles(fb, rects, num_rects);
-
-	return 0;
-}
-
 static const struct drm_framebuffer_funcs vbox_fb_funcs = {
 	.destroy = vbox_user_framebuffer_destroy,
-	.dirty = vbox_user_framebuffer_dirty,
+	.dirty = drm_atomic_helper_dirtyfb,
 };
 
 int vbox_framebuffer_init(struct vbox_private *vbox,
diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
index dd9ad3fdd919..70754e9a087e 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
@@ -284,10 +284,44 @@ static void vbox_primary_atomic_update(struct drm_plane *plane,
 {
 	struct drm_crtc *crtc = plane->state->crtc;
 	struct drm_framebuffer *fb = plane->state->fb;
+	struct vbox_private *vbox = fb->dev->dev_private;
+	struct drm_mode_rect *clips;
+	uint32_t num_clips, i;
 
 	vbox_crtc_set_base_and_mode(crtc, fb,
 				    plane->state->src_x >> 16,
 				    plane->state->src_y >> 16);
+
+	/* Send information about dirty rectangles to VBVA. */
+
+	clips = drm_plane_get_damage_clips(plane->state);
+	num_clips = drm_plane_get_damage_clips_count(plane->state);
+
+	if (!num_clips)
+		return;
+
+	mutex_lock(&vbox->hw_mutex);
+
+	for (i = 0; i < num_clips; ++i, ++clips) {
+
+		struct vbva_cmd_hdr cmd_hdr;
+		unsigned int crtc_id = to_vbox_crtc(crtc)->crtc_id;
+
+		cmd_hdr.x = (s16)clips->x1;
+		cmd_hdr.y = (s16)clips->y1;
+		cmd_hdr.w = (u16)clips->x2 - clips->x1;
+		cmd_hdr.h = (u16)clips->y2 - clips->y1;
+
+		if (!vbva_buffer_begin_update(&vbox->vbva_info[crtc_id],
+					      vbox->guest_pool))
+			continue;
+
+		vbva_write(&vbox->vbva_info[crtc_id], vbox->guest_pool,
+			   &cmd_hdr, sizeof(cmd_hdr));
+		vbva_buffer_end_update(&vbox->vbva_info[crtc_id]);
+	}
+
+	mutex_unlock(&vbox->hw_mutex);
 }
 
 static void vbox_primary_atomic_disable(struct drm_plane *plane,
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/3] drm/vboxvideo: Replace struct vram_framebuffer with generic implemenation

[Thomas Zimmermann]

The vboxvideo driver's struct vram_framebuffer stores a DRM framebuffer
with an assiciated GEM object. This functionality is also provided by
generic code. Switch vboxvideo over.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/vboxvideo/vbox_drv.h  | 14 -------
 drivers/gpu/drm/vboxvideo/vbox_main.c | 60 ---------------------------
 drivers/gpu/drm/vboxvideo/vbox_mode.c | 49 ++++------------------
 3 files changed, 8 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.h b/drivers/gpu/drm/vboxvideo/vbox_drv.h
index 9976554b58cb..87421903816c 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_drv.h
+++ b/drivers/gpu/drm/vboxvideo/vbox_drv.h
@@ -45,11 +45,6 @@
 				sizeof(struct hgsmi_host_flags))
 #define HOST_FLAGS_OFFSET GUEST_HEAP_USABLE_SIZE
 
-struct vbox_framebuffer {
-	struct drm_framebuffer base;
-	struct drm_gem_object *obj;
-};
-
 struct vbox_private {
 	/* Must be first; or we must define our own release callback */
 	struct drm_device ddev;
@@ -132,7 +127,6 @@ struct vbox_encoder {
 #define to_vbox_crtc(x) container_of(x, struct vbox_crtc, base)
 #define to_vbox_connector(x) container_of(x, struct vbox_connector, base)
 #define to_vbox_encoder(x) container_of(x, struct vbox_encoder, base)
-#define to_vbox_framebuffer(x) container_of(x, struct vbox_framebuffer, base)
 
 bool vbox_check_supported(u16 id);
 int vbox_hw_init(struct vbox_private *vbox);
@@ -143,17 +137,9 @@ void vbox_mode_fini(struct vbox_private *vbox);
 
 void vbox_report_caps(struct vbox_private *vbox);
 
-int vbox_framebuffer_init(struct vbox_private *vbox,
-			  struct vbox_framebuffer *vbox_fb,
-			  const struct drm_mode_fb_cmd2 *mode_cmd,
-			  struct drm_gem_object *obj);
-
 int vbox_mm_init(struct vbox_private *vbox);
 void vbox_mm_fini(struct vbox_private *vbox);
 
-int vbox_gem_create(struct vbox_private *vbox,
-		    u32 size, bool iskernel, struct drm_gem_object **obj);
-
 /* vbox_irq.c */
 int vbox_irq_init(struct vbox_private *vbox);
 void vbox_irq_fini(struct vbox_private *vbox);
diff --git a/drivers/gpu/drm/vboxvideo/vbox_main.c b/drivers/gpu/drm/vboxvideo/vbox_main.c
index ba24a9293d17..9dcab115a261 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_main.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_main.c
@@ -17,17 +17,6 @@
 #include "vboxvideo_guest.h"
 #include "vboxvideo_vbe.h"
 
-static void vbox_user_framebuffer_destroy(struct drm_framebuffer *fb)
-{
-	struct vbox_framebuffer *vbox_fb = to_vbox_framebuffer(fb);
-
-	if (vbox_fb->obj)
-		drm_gem_object_put_unlocked(vbox_fb->obj);
-
-	drm_framebuffer_cleanup(fb);
-	kfree(fb);
-}
-
 void vbox_report_caps(struct vbox_private *vbox)
 {
 	u32 caps = VBVACAPS_DISABLE_CURSOR_INTEGRATION |
@@ -39,29 +28,6 @@ void vbox_report_caps(struct vbox_private *vbox)
 	hgsmi_send_caps_info(vbox->guest_pool, caps);
 }
 
-static const struct drm_framebuffer_funcs vbox_fb_funcs = {
-	.destroy = vbox_user_framebuffer_destroy,
-	.dirty = drm_atomic_helper_dirtyfb,
-};
-
-int vbox_framebuffer_init(struct vbox_private *vbox,
-			  struct vbox_framebuffer *vbox_fb,
-			  const struct drm_mode_fb_cmd2 *mode_cmd,
-			  struct drm_gem_object *obj)
-{
-	int ret;
-
-	drm_helper_mode_fill_fb_struct(&vbox->ddev, &vbox_fb->base, mode_cmd);
-	vbox_fb->obj = obj;
-	ret = drm_framebuffer_init(&vbox->ddev, &vbox_fb->base, &vbox_fb_funcs);
-	if (ret) {
-		DRM_ERROR("framebuffer init failed %d\n", ret);
-		return ret;
-	}
-
-	return 0;
-}
-
 static int vbox_accel_init(struct vbox_private *vbox)
 {
 	struct vbva_buffer *vbva;
@@ -213,29 +179,3 @@ void vbox_hw_fini(struct vbox_private *vbox)
 	gen_pool_destroy(vbox->guest_pool);
 	pci_iounmap(vbox->ddev.pdev, vbox->guest_heap);
 }
-
-int vbox_gem_create(struct vbox_private *vbox,
-		    u32 size, bool iskernel, struct drm_gem_object **obj)
-{
-	struct drm_gem_vram_object *gbo;
-	int ret;
-
-	*obj = NULL;
-
-	size = roundup(size, PAGE_SIZE);
-	if (size == 0)
-		return -EINVAL;
-
-	gbo = drm_gem_vram_create(&vbox->ddev, &vbox->ddev.vram_mm->bdev,
-				  size, 0, false);
-	if (IS_ERR(gbo)) {
-		ret = PTR_ERR(gbo);
-		if (ret != -ERESTARTSYS)
-			DRM_ERROR("failed to allocate GEM object\n");
-		return ret;
-	}
-
-	*obj = &gbo->bo.base;
-
-	return 0;
-}
diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
index 70754e9a087e..e09f5f38fbff 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
@@ -15,6 +15,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_fourcc.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_vblank.h>
@@ -173,8 +174,7 @@ static void vbox_crtc_set_base_and_mode(struct drm_crtc *crtc,
 					struct drm_framebuffer *fb,
 					int x, int y)
 {
-	struct drm_gem_vram_object *gbo =
-		drm_gem_vram_of_gem(to_vbox_framebuffer(fb)->obj);
+	struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(fb->obj[0]);
 	struct vbox_private *vbox = crtc->dev->dev_private;
 	struct vbox_crtc *vbox_crtc = to_vbox_crtc(crtc);
 	bool needs_modeset = drm_atomic_crtc_needs_modeset(crtc->state);
@@ -344,7 +344,7 @@ static int vbox_primary_prepare_fb(struct drm_plane *plane,
 	if (!new_state->fb)
 		return 0;
 
-	gbo = drm_gem_vram_of_gem(to_vbox_framebuffer(new_state->fb)->obj);
+	gbo = drm_gem_vram_of_gem(new_state->fb->obj[0]);
 	ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
 	if (ret)
 		DRM_WARN("Error %d pinning new fb, out of video mem?\n", ret);
@@ -360,7 +360,7 @@ static void vbox_primary_cleanup_fb(struct drm_plane *plane,
 	if (!old_state->fb)
 		return;
 
-	gbo = drm_gem_vram_of_gem(to_vbox_framebuffer(old_state->fb)->obj);
+	gbo = drm_gem_vram_of_gem(old_state->fb->obj[0]);
 	drm_gem_vram_unpin(gbo);
 }
 
@@ -421,8 +421,7 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
 		container_of(plane->dev, struct vbox_private, ddev);
 	struct vbox_crtc *vbox_crtc = to_vbox_crtc(plane->state->crtc);
 	struct drm_framebuffer *fb = plane->state->fb;
-	struct drm_gem_vram_object *gbo =
-		drm_gem_vram_of_gem(to_vbox_framebuffer(fb)->obj);
+	struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(fb->obj[0]);
 	u32 width = plane->state->crtc_w;
 	u32 height = plane->state->crtc_h;
 	size_t data_size, mask_size;
@@ -502,7 +501,7 @@ static int vbox_cursor_prepare_fb(struct drm_plane *plane,
 	if (!new_state->fb)
 		return 0;
 
-	gbo = drm_gem_vram_of_gem(to_vbox_framebuffer(new_state->fb)->obj);
+	gbo = drm_gem_vram_of_gem(new_state->fb->obj[0]);
 	return drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_SYSTEM);
 }
 
@@ -514,7 +513,7 @@ static void vbox_cursor_cleanup_fb(struct drm_plane *plane,
 	if (!plane->state->fb)
 		return;
 
-	gbo = drm_gem_vram_of_gem(to_vbox_framebuffer(plane->state->fb)->obj);
+	gbo = drm_gem_vram_of_gem(plane->state->fb->obj[0]);
 	drm_gem_vram_unpin(gbo);
 }
 
@@ -891,40 +890,8 @@ static int vbox_connector_init(struct drm_device *dev,
 	return 0;
 }
 
-static struct drm_framebuffer *vbox_user_framebuffer_create(
-		struct drm_device *dev,
-		struct drm_file *filp,
-		const struct drm_mode_fb_cmd2 *mode_cmd)
-{
-	struct vbox_private *vbox =
-		container_of(dev, struct vbox_private, ddev);
-	struct drm_gem_object *obj;
-	struct vbox_framebuffer *vbox_fb;
-	int ret = -ENOMEM;
-
-	obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]);
-	if (!obj)
-		return ERR_PTR(-ENOENT);
-
-	vbox_fb = kzalloc(sizeof(*vbox_fb), GFP_KERNEL);
-	if (!vbox_fb)
-		goto err_unref_obj;
-
-	ret = vbox_framebuffer_init(vbox, vbox_fb, mode_cmd, obj);
-	if (ret)
-		goto err_free_vbox_fb;
-
-	return &vbox_fb->base;
-
-err_free_vbox_fb:
-	kfree(vbox_fb);
-err_unref_obj:
-	drm_gem_object_put_unlocked(obj);
-	return ERR_PTR(ret);
-}
-
 static const struct drm_mode_config_funcs vbox_mode_funcs = {
-	.fb_create = vbox_user_framebuffer_create,
+	.fb_create = drm_gem_fb_create,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/vboxvideo: Use generic fbdev and framebuffer

[Sam Ravnborg]

Hi Thomas.

On Fri, Oct 11, 2019 at 03:48:05PM +0200, Thomas Zimmermann wrote:
> The vboxvideo driver provides its own implementation for fbdev emulation
> and framebuffers. Both can be replaced by DRM's generic code.
> 
> All patches have been tested on VirtualBox 6.0.12.
> 
> Thomas Zimmermann (3):
>   drm/vboxvideo: Switch to generic fbdev emulation
>   drm/vboxvideo: Switch to drm_atomic_helper_dirty_fb()
>   drm/vboxvideo: Replace struct vram_framebuffer with generic
>     implemenation
> 
>  drivers/gpu/drm/vboxvideo/Makefile    |   2 +-
>  drivers/gpu/drm/vboxvideo/vbox_drv.c  |  14 +--
>  drivers/gpu/drm/vboxvideo/vbox_drv.h  |  25 -----
>  drivers/gpu/drm/vboxvideo/vbox_fb.c   | 149 --------------------------
>  drivers/gpu/drm/vboxvideo/vbox_main.c | 119 +-------------------
>  drivers/gpu/drm/vboxvideo/vbox_mode.c |  86 +++++++--------
>  6 files changed, 49 insertions(+), 346 deletions(-)
>  delete mode 100644 drivers/gpu/drm/vboxvideo/vbox_fb.c
Nice work. Browsed through the changes and enjoyed all the deleted code.

You can add my:

Acked-by: Sam Ravnborg <sam@ravnborg.org>

to the whole series.

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/vboxvideo: Use generic fbdev and framebuffer

[Thomas Zimmermann]

Hi

Am 12.10.19 um 14:36 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Fri, Oct 11, 2019 at 03:48:05PM +0200, Thomas Zimmermann wrote:
>> The vboxvideo driver provides its own implementation for fbdev emulation
>> and framebuffers. Both can be replaced by DRM's generic code.
>>
>> All patches have been tested on VirtualBox 6.0.12.
>>
>> Thomas Zimmermann (3):
>>    drm/vboxvideo: Switch to generic fbdev emulation
>>    drm/vboxvideo: Switch to drm_atomic_helper_dirty_fb()
>>    drm/vboxvideo: Replace struct vram_framebuffer with generic
>>      implemenation
>>
>>   drivers/gpu/drm/vboxvideo/Makefile    |   2 +-
>>   drivers/gpu/drm/vboxvideo/vbox_drv.c  |  14 +--
>>   drivers/gpu/drm/vboxvideo/vbox_drv.h  |  25 -----
>>   drivers/gpu/drm/vboxvideo/vbox_fb.c   | 149 --------------------------
>>   drivers/gpu/drm/vboxvideo/vbox_main.c | 119 +-------------------
>>   drivers/gpu/drm/vboxvideo/vbox_mode.c |  86 +++++++--------
>>   6 files changed, 49 insertions(+), 346 deletions(-)
>>   delete mode 100644 drivers/gpu/drm/vboxvideo/vbox_fb.c
> Nice work. Browsed through the changes and enjoyed all the deleted code.
> 
> You can add my:
> 
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> 
> to the whole series.

Thanks!

> 
> 	Sam
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/vboxvideo: Use generic fbdev and framebuffer

[Hans de Goede]

Hi Thomas,

On 11-10-2019 15:48, Thomas Zimmermann wrote:
> The vboxvideo driver provides its own implementation for fbdev emulation
> and framebuffers. Both can be replaced by DRM's generic code.
> 
> All patches have been tested on VirtualBox 6.0.12.
> 
> Thomas Zimmermann (3):
>    drm/vboxvideo: Switch to generic fbdev emulation
>    drm/vboxvideo: Switch to drm_atomic_helper_dirty_fb()
>    drm/vboxvideo: Replace struct vram_framebuffer with generic
>      implemenation

Thank you for these nice cleanups, unfortunately there is a small
bug in the last patch, you are setting:

	.fb_create = drm_gem_fb_create,

But since in the previous patch you switched to drm_atomic_helper_dirty_fb
that should be:

	.fb_create = drm_gem_fb_create_with_dirty,

The missing with_dirty is causing screenupdates under both plymouth and
gnome-shell (with llvmpipe) to gone missing. I'll send a patch fixing
this.

Regards,

Hans

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/vboxvideo: Use generic fbdev and framebuffer

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 1612 bytes --]

Hi

Am 28.10.19 um 12:26 schrieb Hans de Goede:
> Hi Thomas,
> 
> On 11-10-2019 15:48, Thomas Zimmermann wrote:
>> The vboxvideo driver provides its own implementation for fbdev emulation
>> and framebuffers. Both can be replaced by DRM's generic code.
>>
>> All patches have been tested on VirtualBox 6.0.12.
>>
>> Thomas Zimmermann (3):
>>    drm/vboxvideo: Switch to generic fbdev emulation
>>    drm/vboxvideo: Switch to drm_atomic_helper_dirty_fb()
>>    drm/vboxvideo: Replace struct vram_framebuffer with generic
>>      implemenation
> 
> Thank you for these nice cleanups, unfortunately there is a small
> bug in the last patch, you are setting:
> 
>     .fb_create = drm_gem_fb_create,
> 
> But since in the previous patch you switched to drm_atomic_helper_dirty_fb
> that should be:
> 
>     .fb_create = drm_gem_fb_create_with_dirty,
> 
> The missing with_dirty is causing screenupdates under both plymouth and
> gnome-shell (with llvmpipe) to gone missing. I'll send a patch fixing
> this.

You're right. I did test the patchset, but I can't tell why I didn't see
this bug. Anyway, thanks a lot for providing the fix.

Best regards
Thomas

> 
> Regards,
> 
> Hans
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/vboxvideo: Use generic fbdev and framebuffer

[Hans de Goede]

Hi,

On 28-10-2019 12:34, Thomas Zimmermann wrote:
> Hi
> 
> Am 28.10.19 um 12:26 schrieb Hans de Goede:
>> Hi Thomas,
>>
>> On 11-10-2019 15:48, Thomas Zimmermann wrote:
>>> The vboxvideo driver provides its own implementation for fbdev emulation
>>> and framebuffers. Both can be replaced by DRM's generic code.
>>>
>>> All patches have been tested on VirtualBox 6.0.12.
>>>
>>> Thomas Zimmermann (3):
>>>     drm/vboxvideo: Switch to generic fbdev emulation
>>>     drm/vboxvideo: Switch to drm_atomic_helper_dirty_fb()
>>>     drm/vboxvideo: Replace struct vram_framebuffer with generic
>>>       implemenation
>>
>> Thank you for these nice cleanups, unfortunately there is a small
>> bug in the last patch, you are setting:
>>
>>      .fb_create = drm_gem_fb_create,
>>
>> But since in the previous patch you switched to drm_atomic_helper_dirty_fb
>> that should be:
>>
>>      .fb_create = drm_gem_fb_create_with_dirty,
>>
>> The missing with_dirty is causing screenupdates under both plymouth and
>> gnome-shell (with llvmpipe) to gone missing. I'll send a patch fixing
>> this.
> 
> You're right. I did test the patchset, but I can't tell why I didn't see
> this bug.

I know you tested the patch-set, since you said so above :)  You probably
are aware of this already, but did you check what vga-card the vm is using?
New vbox VMs default to their new vmware-svga card emulation, in which case
vboxvideo is not used at all, vboxvideo is only used with the older VboxVGA
and VboxSVGA (*) virtual vga-cards.

> Anyway, thanks a lot for providing the fix.

You are welcome.

Regards,

Hans



*) Of course once I finally get their driver upstream they deprecate it, grumble.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/vboxvideo: Use generic fbdev and framebuffer

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 2192 bytes --]

Hi

Am 28.10.19 um 13:06 schrieb Hans de Goede:
> Hi,
> 
> On 28-10-2019 12:34, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 28.10.19 um 12:26 schrieb Hans de Goede:
>>> Hi Thomas,
>>>
>>> On 11-10-2019 15:48, Thomas Zimmermann wrote:
>>>> The vboxvideo driver provides its own implementation for fbdev
>>>> emulation
>>>> and framebuffers. Both can be replaced by DRM's generic code.
>>>>
>>>> All patches have been tested on VirtualBox 6.0.12.
>>>>
>>>> Thomas Zimmermann (3):
>>>>     drm/vboxvideo: Switch to generic fbdev emulation
>>>>     drm/vboxvideo: Switch to drm_atomic_helper_dirty_fb()
>>>>     drm/vboxvideo: Replace struct vram_framebuffer with generic
>>>>       implemenation
>>>
>>> Thank you for these nice cleanups, unfortunately there is a small
>>> bug in the last patch, you are setting:
>>>
>>>      .fb_create = drm_gem_fb_create,
>>>
>>> But since in the previous patch you switched to
>>> drm_atomic_helper_dirty_fb
>>> that should be:
>>>
>>>      .fb_create = drm_gem_fb_create_with_dirty,
>>>
>>> The missing with_dirty is causing screenupdates under both plymouth and
>>> gnome-shell (with llvmpipe) to gone missing. I'll send a patch fixing
>>> this.
>>
>> You're right. I did test the patchset, but I can't tell why I didn't see
>> this bug.
> 
> I know you tested the patch-set, since you said so above :)  You probably
> are aware of this already, but did you check what vga-card the vm is using?
> New vbox VMs default to their new vmware-svga card emulation, in which case
> vboxvideo is not used at all, vboxvideo is only used with the older VboxVGA
> and VboxSVGA (*) virtual vga-cards.

That sounds plausible at least. I'll double-check next time.

Best regards
Thomas

> 
>> Anyway, thanks a lot for providing the fix.
> 
> You are welcome.
> 
> Regards,
> 
> Hans
> 
> 
> 
> *) Of course once I finally get their driver upstream they deprecate it,
> grumble.
> 

-- 
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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel