Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit df2052cc authored by Peter Wu's avatar Peter Wu Committed by Gerd Hoffmann
Browse files

bochs: convert to drm_fb_helper_fbdev_setup/teardown 

Currently unloading bochs_drm (after unbinding the vtconsole) results in
a warning about a leaked connector:

    [drm:drm_mode_config_cleanup] *ERROR* connector Virtual-3 leaked!

While investigating a potential fix I noticed that a lot of open-coded
functionality is already implemented elsewhere, so start converting it:
bochs_fbdev_init -> drm_fb_helper_fbdev_setup: trivial (similar impl).
bochs_fbdev_fini -> drm_fb_helper_fbdev_teardown: requires unembedding
"struct drm_framebuffer" from "struct bochs_framebuffer".

Unembedding drm_framebuffer is made easy using drm_gem_fbdev_fb_create
which can replace bochs_fbdev_destroy and custom routines in bochs_mm.c.
For this to work, the GEM object is moved into "drm_framebuffer". After
that, "bochs_framebuffer" is no longer needed and therefore removed.

Remove the unused "size" and "initialized" fields from fb, the latter is
not necessary as drm_fb_helper_fbdev_teardown can be called even if
bochsfb_create fails. This theory was tested by returning early and
late (just before drm_gem_fbdev_fb_create). Both scenarios fail
gracefully although the latter seems to leak the object from
bochsfb_create_object (not a regression).

Guess on the reason for the encoder leak: drm_framebuffer_cleanup was
previously used, but did not destroy much. drm_fb_helper_fbdev_teardown
is now used and calls drm_framebuffer_remove which does a bit more work.

Tested with 'echo 0 > /sys/class/vtconsole/vtcon1/bind; rmmod bochs_drm'
and also with Xorg + fbdev (startx -> xterm). The latter triggered a
warning in ttm_bo_vm_open that existed before, see
https://lkml.kernel.org/r/1464000533-13140-4-git-send-email-mstaudt@suse.de



Acked-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: default avatarPeter Wu <peter@lekensteyn.nl>
Link: http://patchwork.freedesktop.org/patch/msgid/20180906221810.20170-3-peter@lekensteyn.nl


Signed-off-by: default avatarGerd Hoffmann <kraxel@redhat.com>
parent 70c0ef7b

drivers/gpu/drm/bochs/bochs.h

+3 −16
Original line number Diff line number Diff line
@@ -51,11 +51,6 @@ enum bochs_types { 
	BOCHS_UNKNOWN,

};



struct bochs_framebuffer {

	struct drm_framebuffer base;

	struct drm_gem_object *obj;

};



struct bochs_device {

	/* hw */

	void __iomem   *mmio;
@@ -88,15 +83,11 @@ struct bochs_device { 


	/* fbdev */

	struct {

		struct bochs_framebuffer gfb;

		struct drm_framebuffer *fb;

		struct drm_fb_helper helper;

		int size;

		bool initialized;

	} fb;

};



#define to_bochs_framebuffer(x) container_of(x, struct bochs_framebuffer, base)



struct bochs_bo {

	struct ttm_buffer_object bo;

	struct ttm_placement placement;
@@ -148,15 +139,9 @@ int bochs_dumb_create(struct drm_file *file, struct drm_device *dev, 
int bochs_dumb_mmap_offset(struct drm_file *file, struct drm_device *dev,

			   uint32_t handle, uint64_t *offset);



int bochs_framebuffer_init(struct drm_device *dev,

			   struct bochs_framebuffer *gfb,

			   const struct drm_mode_fb_cmd2 *mode_cmd,

			   struct drm_gem_object *obj);

int bochs_bo_pin(struct bochs_bo *bo, u32 pl_flag, u64 *gpu_addr);

int bochs_bo_unpin(struct bochs_bo *bo);



extern const struct drm_mode_config_funcs bochs_mode_funcs;



/* bochs_kms.c */

int bochs_kms_init(struct bochs_device *bochs);

void bochs_kms_fini(struct bochs_device *bochs);
@@ -164,3 +149,5 @@ void bochs_kms_fini(struct bochs_device *bochs); 
/* bochs_fbdev.c */

int bochs_fbdev_init(struct bochs_device *bochs);

void bochs_fbdev_fini(struct bochs_device *bochs);



extern const struct drm_mode_config_funcs bochs_mode_funcs;

drivers/gpu/drm/bochs/bochs_fbdev.c

+17 −62
Original line number Diff line number Diff line
@@ -6,6 +6,7 @@ 
 */



#include "bochs.h"

#include <drm/drm_gem_framebuffer_helper.h>



/* ---------------------------------------------------------------------- */
@@ -13,9 +14,7 @@ static int bochsfb_mmap(struct fb_info *info, 
			struct vm_area_struct *vma)

{

	struct drm_fb_helper *fb_helper = info->par;

	struct bochs_device *bochs =

		container_of(fb_helper, struct bochs_device, fb.helper);

	struct bochs_bo *bo = gem_to_bochs_bo(bochs->fb.gfb.obj);

	struct bochs_bo *bo = gem_to_bochs_bo(fb_helper->fb->obj[0]);



	return ttm_fbdev_mmap(vma, &bo->bo);

}
@@ -101,19 +100,20 @@ static int bochsfb_create(struct drm_fb_helper *helper, 


	/* init fb device */

	info = drm_fb_helper_alloc_fbi(helper);

	if (IS_ERR(info))

	if (IS_ERR(info)) {

		DRM_ERROR("Failed to allocate fbi: %ld\n", PTR_ERR(info));

		return PTR_ERR(info);

	}



	info->par = &bochs->fb.helper;



	ret = bochs_framebuffer_init(bochs->dev, &bochs->fb.gfb, &mode_cmd, gobj);

	if (ret)

		return ret;



	bochs->fb.size = size;

	fb = drm_gem_fbdev_fb_create(bochs->dev, sizes, 0, gobj, NULL);

	if (IS_ERR(fb)) {

		DRM_ERROR("Failed to create framebuffer: %ld\n", PTR_ERR(fb));

		return PTR_ERR(fb);

	}



	/* setup helper */

	fb = &bochs->fb.gfb.base;

	bochs->fb.helper.fb = fb;



	strcpy(info->fix.id, "bochsdrmfb");
@@ -130,27 +130,6 @@ static int bochsfb_create(struct drm_fb_helper *helper, 
	drm_vma_offset_remove(&bo->bo.bdev->vma_manager, &bo->bo.vma_node);

	info->fix.smem_start = 0;

	info->fix.smem_len = size;



	bochs->fb.initialized = true;

	return 0;

}



static int bochs_fbdev_destroy(struct bochs_device *bochs)

{

	struct bochs_framebuffer *gfb = &bochs->fb.gfb;



	DRM_DEBUG_DRIVER("\n");



	drm_fb_helper_unregister_fbi(&bochs->fb.helper);



	if (gfb->obj) {

		drm_gem_object_unreference_unlocked(gfb->obj);

		gfb->obj = NULL;

	}



	drm_framebuffer_unregister_private(&gfb->base);

	drm_framebuffer_cleanup(&gfb->base);



	return 0;

}
@@ -158,41 +137,17 @@ static const struct drm_fb_helper_funcs bochs_fb_helper_funcs = { 
	.fb_probe = bochsfb_create,

};



const struct drm_mode_config_funcs bochs_mode_funcs = {

	.fb_create = drm_gem_fb_create,

};



int bochs_fbdev_init(struct bochs_device *bochs)

{

	int ret;



	drm_fb_helper_prepare(bochs->dev, &bochs->fb.helper,

			      &bochs_fb_helper_funcs);



	ret = drm_fb_helper_init(bochs->dev, &bochs->fb.helper, 1);

	if (ret)

		return ret;



	ret = drm_fb_helper_single_add_all_connectors(&bochs->fb.helper);

	if (ret)

		goto fini;



	drm_helper_disable_unused_functions(bochs->dev);



	ret = drm_fb_helper_initial_config(&bochs->fb.helper, 32);

	if (ret)

		goto fini;



	return 0;



fini:

	drm_fb_helper_fini(&bochs->fb.helper);

	return ret;

	return drm_fb_helper_fbdev_setup(bochs->dev, &bochs->fb.helper,

					 &bochs_fb_helper_funcs, 32, 1);

}



void bochs_fbdev_fini(struct bochs_device *bochs)

{

	if (bochs->fb.initialized)

		bochs_fbdev_destroy(bochs);



	if (bochs->fb.helper.fbdev)

		drm_fb_helper_fini(&bochs->fb.helper);



	bochs->fb.initialized = false;

	drm_fb_helper_fbdev_teardown(bochs->dev);

}

drivers/gpu/drm/bochs/bochs_kms.c

+2 −5
Original line number Diff line number Diff line
@@ -35,14 +35,12 @@ static int bochs_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, 
{

	struct bochs_device *bochs =

		container_of(crtc, struct bochs_device, crtc);

	struct bochs_framebuffer *bochs_fb;

	struct bochs_bo *bo;

	u64 gpu_addr = 0;

	int ret;



	if (old_fb) {

		bochs_fb = to_bochs_framebuffer(old_fb);

		bo = gem_to_bochs_bo(bochs_fb->obj);

		bo = gem_to_bochs_bo(old_fb->obj[0]);

		ret = ttm_bo_reserve(&bo->bo, true, false, NULL);

		if (ret) {

			DRM_ERROR("failed to reserve old_fb bo\n");
@@ -55,8 +53,7 @@ static int bochs_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, 
	if (WARN_ON(crtc->primary->fb == NULL))

		return -EINVAL;



	bochs_fb = to_bochs_framebuffer(crtc->primary->fb);

	bo = gem_to_bochs_bo(bochs_fb->obj);

	bo = gem_to_bochs_bo(crtc->primary->fb->obj[0]);

	ret = ttm_bo_reserve(&bo->bo, true, false, NULL);

	if (ret)

		return ret;

drivers/gpu/drm/bochs/bochs_mm.c

+0 −74
Original line number Diff line number Diff line
@@ -457,77 +457,3 @@ int bochs_dumb_mmap_offset(struct drm_file *file, struct drm_device *dev, 
	drm_gem_object_unreference_unlocked(obj);

	return 0;

}
 


/* ---------------------------------------------------------------------- */



static void bochs_user_framebuffer_destroy(struct drm_framebuffer *fb)

{

	struct bochs_framebuffer *bochs_fb = to_bochs_framebuffer(fb);



	drm_gem_object_unreference_unlocked(bochs_fb->obj);

	drm_framebuffer_cleanup(fb);

	kfree(fb);

}



static const struct drm_framebuffer_funcs bochs_fb_funcs = {

	.destroy = bochs_user_framebuffer_destroy,

};



int bochs_framebuffer_init(struct drm_device *dev,

			   struct bochs_framebuffer *gfb,

			   const struct drm_mode_fb_cmd2 *mode_cmd,

			   struct drm_gem_object *obj)

{

	int ret;



	drm_helper_mode_fill_fb_struct(dev, &gfb->base, mode_cmd);

	gfb->obj = obj;

	ret = drm_framebuffer_init(dev, &gfb->base, &bochs_fb_funcs);

	if (ret) {

		DRM_ERROR("drm_framebuffer_init failed: %d\n", ret);

		return ret;

	}

	return 0;

}



static struct drm_framebuffer *

bochs_user_framebuffer_create(struct drm_device *dev,

			      struct drm_file *filp,

			      const struct drm_mode_fb_cmd2 *mode_cmd)

{

	struct drm_gem_object *obj;

	struct bochs_framebuffer *bochs_fb;

	int ret;



	DRM_DEBUG_DRIVER("%dx%d, format %c%c%c%c\n",

	       mode_cmd->width, mode_cmd->height,

	       (mode_cmd->pixel_format)       & 0xff,

	       (mode_cmd->pixel_format >> 8)  & 0xff,

	       (mode_cmd->pixel_format >> 16) & 0xff,

	       (mode_cmd->pixel_format >> 24) & 0xff);



	if (mode_cmd->pixel_format != DRM_FORMAT_XRGB8888)

		return ERR_PTR(-ENOENT);



	obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]);

	if (obj == NULL)

		return ERR_PTR(-ENOENT);



	bochs_fb = kzalloc(sizeof(*bochs_fb), GFP_KERNEL);

	if (!bochs_fb) {

		drm_gem_object_unreference_unlocked(obj);

		return ERR_PTR(-ENOMEM);

	}



	ret = bochs_framebuffer_init(dev, bochs_fb, mode_cmd, obj);

	if (ret) {

		drm_gem_object_unreference_unlocked(obj);

		kfree(bochs_fb);

		return ERR_PTR(ret);

	}

	return &bochs_fb->base;

}



const struct drm_mode_config_funcs bochs_mode_funcs = {

	.fb_create = bochs_user_framebuffer_create,

};