bochs: convert to drm_fb_helper_fbdev_setup/teardown
diff mbox series

Message ID 20180905144127.5690-1-peter@lekensteyn.nl
State New
Headers show
Series
  • bochs: convert to drm_fb_helper_fbdev_setup/teardown
Related show

Commit Message

Peter Wu Sept. 5, 2018, 2:41 p.m. UTC
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

Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
 drivers/gpu/drm/bochs/bochs.h       | 19 ++-----
 drivers/gpu/drm/bochs/bochs_fbdev.c | 79 +++++++----------------------
 drivers/gpu/drm/bochs/bochs_kms.c   |  7 +--
 drivers/gpu/drm/bochs/bochs_mm.c    | 74 ---------------------------
 4 files changed, 22 insertions(+), 157 deletions(-)

Comments

Daniel Vetter Sept. 5, 2018, 2:46 p.m. UTC | #1
On Wed, Sep 05, 2018 at 04:41:27PM +0200, Peter Wu wrote:
> 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

You can probably get rid of this one if you're refactoring even more. The
generic fb_probe implementation (already merged) plus gem-shmem support
for it (still in flight) from Noralf should be able to pull that off. That
gives you the fb_mmap implementation, but with 100% generic code instead
of a driver specific hack like Max did.
> 
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>

lgtm. Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I'll leave merging to Gerd.
-Daniel

> ---
>  drivers/gpu/drm/bochs/bochs.h       | 19 ++-----
>  drivers/gpu/drm/bochs/bochs_fbdev.c | 79 +++++++----------------------
>  drivers/gpu/drm/bochs/bochs_kms.c   |  7 +--
>  drivers/gpu/drm/bochs/bochs_mm.c    | 74 ---------------------------
>  4 files changed, 22 insertions(+), 157 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
> index 375bf92cd04f..8514a84fbdbe 100644
> --- a/drivers/gpu/drm/bochs/bochs.h
> +++ b/drivers/gpu/drm/bochs/bochs.h
> @@ -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;
> diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c b/drivers/gpu/drm/bochs/bochs_fbdev.c
> index 14eb8d0d5a00..8f4d6c052f7b 100644
> --- a/drivers/gpu/drm/bochs/bochs_fbdev.c
> +++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
> @@ -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);
>  }
> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
> index 233980a78591..c7e575511d2f 100644
> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> @@ -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;
> diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
> index 39cd08416773..cdbc3ca3ec51 100644
> --- a/drivers/gpu/drm/bochs/bochs_mm.c
> +++ b/drivers/gpu/drm/bochs/bochs_mm.c
> @@ -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,
> -};
> -- 
> 2.18.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Peter Wu Sept. 5, 2018, 3:06 p.m. UTC | #2
On Wed, Sep 05, 2018 at 04:46:29PM +0200, Daniel Vetter wrote:
> On Wed, Sep 05, 2018 at 04:41:27PM +0200, Peter Wu wrote:
> > 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
> 
> You can probably get rid of this one if you're refactoring even more. The
> generic fb_probe implementation (already merged) plus gem-shmem support
> for it (still in flight) from Noralf should be able to pull that off. That
> gives you the fb_mmap implementation, but with 100% generic code instead
> of a driver specific hack like Max did.

Aside from the warning, I have not observed actual issues. This patch
was prepared on top of v4.18.1 but the new drm_fb_helper_generic_probe
helper is in master (future 4.19). I suppose that it can be done as a
future cleanup. Nice work Noralf on reducing duplication!
.
> > 
> > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> 
> lgtm. Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> I'll leave merging to Gerd.
> -Daniel

Thanks, I somehow missed a patch. This one does not compile due to
"fb.initialized" still being used in bochs_drv.c. Removal is trivial,
I'll wait for some more feedback and then send a v2 with another patch
prepended.

Kind regards,
Peter

> > ---
> >  drivers/gpu/drm/bochs/bochs.h       | 19 ++-----
> >  drivers/gpu/drm/bochs/bochs_fbdev.c | 79 +++++++----------------------
> >  drivers/gpu/drm/bochs/bochs_kms.c   |  7 +--
> >  drivers/gpu/drm/bochs/bochs_mm.c    | 74 ---------------------------
> >  4 files changed, 22 insertions(+), 157 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
> > index 375bf92cd04f..8514a84fbdbe 100644
> > --- a/drivers/gpu/drm/bochs/bochs.h
> > +++ b/drivers/gpu/drm/bochs/bochs.h
> > @@ -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;
> > diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c b/drivers/gpu/drm/bochs/bochs_fbdev.c
> > index 14eb8d0d5a00..8f4d6c052f7b 100644
> > --- a/drivers/gpu/drm/bochs/bochs_fbdev.c
> > +++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
> > @@ -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);
> >  }
> > diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
> > index 233980a78591..c7e575511d2f 100644
> > --- a/drivers/gpu/drm/bochs/bochs_kms.c
> > +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> > @@ -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;
> > diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
> > index 39cd08416773..cdbc3ca3ec51 100644
> > --- a/drivers/gpu/drm/bochs/bochs_mm.c
> > +++ b/drivers/gpu/drm/bochs/bochs_mm.c
> > @@ -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,
> > -};
> > -- 
> > 2.18.0
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Gerd Hoffmann Sept. 6, 2018, 7:23 a.m. UTC | #3
Hi,

> > You can probably get rid of this one if you're refactoring even more. The
> > generic fb_probe implementation (already merged) plus gem-shmem support
> > for it (still in flight) from Noralf should be able to pull that off. That
> > gives you the fb_mmap implementation, but with 100% generic code instead
> > of a driver specific hack like Max did.
> 
> Aside from the warning, I have not observed actual issues. This patch
> was prepared on top of v4.18.1 but the new drm_fb_helper_generic_probe
> helper is in master (future 4.19). I suppose that it can be done as a
> future cleanup. Nice work Noralf on reducing duplication!

FYI: qemu kms driver patches go through drm-misc-next, so you can also
work against that branch.

> > I'll leave merging to Gerd.
> 
> Thanks, I somehow missed a patch. This one does not compile due to
> "fb.initialized" still being used in bochs_drv.c. Removal is trivial,
> I'll wait for some more feedback and then send a v2 with another patch
> prepended.

Patch looks good to me (except for the build failure which you've
noticed already).

cheers,
  Gerd
kbuild test robot Sept. 7, 2018, 8:10 a.m. UTC | #4
Hi Peter,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.19-rc2 next-20180906]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Peter-Wu/bochs-convert-to-drm_fb_helper_fbdev_setup-teardown/20180907-154819
config: i386-randconfig-x006-201835 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/asm-generic/bug.h:5:0,
                    from arch/x86/include/asm/bug.h:83,
                    from include/linux/bug.h:5,
                    from include/linux/mmdebug.h:5,
                    from include/linux/mm.h:9,
                    from drivers/gpu/drm/bochs/bochs_drv.c:8:
   drivers/gpu/drm/bochs/bochs_drv.c: In function 'bochs_pm_suspend':
   drivers/gpu/drm/bochs/bochs_drv.c:110:15: error: 'struct <anonymous>' has no member named 'initialized'
     if (bochs->fb.initialized) {
                  ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/gpu/drm/bochs/bochs_drv.c:110:2: note: in expansion of macro 'if'
     if (bochs->fb.initialized) {
     ^~
   drivers/gpu/drm/bochs/bochs_drv.c:110:15: error: 'struct <anonymous>' has no member named 'initialized'
     if (bochs->fb.initialized) {
                  ^
   include/linux/compiler.h:58:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
>> drivers/gpu/drm/bochs/bochs_drv.c:110:2: note: in expansion of macro 'if'
     if (bochs->fb.initialized) {
     ^~
   drivers/gpu/drm/bochs/bochs_drv.c:110:15: error: 'struct <anonymous>' has no member named 'initialized'
     if (bochs->fb.initialized) {
                  ^
   include/linux/compiler.h:69:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
>> drivers/gpu/drm/bochs/bochs_drv.c:110:2: note: in expansion of macro 'if'
     if (bochs->fb.initialized) {
     ^~
   drivers/gpu/drm/bochs/bochs_drv.c: In function 'bochs_pm_resume':
   drivers/gpu/drm/bochs/bochs_drv.c:127:15: error: 'struct <anonymous>' has no member named 'initialized'
     if (bochs->fb.initialized) {
                  ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
   drivers/gpu/drm/bochs/bochs_drv.c:127:2: note: in expansion of macro 'if'
     if (bochs->fb.initialized) {
     ^~
   drivers/gpu/drm/bochs/bochs_drv.c:127:15: error: 'struct <anonymous>' has no member named 'initialized'
     if (bochs->fb.initialized) {
                  ^
   include/linux/compiler.h:58:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
   drivers/gpu/drm/bochs/bochs_drv.c:127:2: note: in expansion of macro 'if'
     if (bochs->fb.initialized) {
     ^~
   drivers/gpu/drm/bochs/bochs_drv.c:127:15: error: 'struct <anonymous>' has no member named 'initialized'
     if (bochs->fb.initialized) {
                  ^
   include/linux/compiler.h:69:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
   drivers/gpu/drm/bochs/bochs_drv.c:127:2: note: in expansion of macro 'if'
     if (bochs->fb.initialized) {
     ^~

vim +/if +110 drivers/gpu/drm/bochs/bochs_drv.c

0a6659bd Gerd Hoffmann           2013-12-17   @8  #include <linux/mm.h>
0a6659bd Gerd Hoffmann           2013-12-17    9  #include <linux/module.h>
0a6659bd Gerd Hoffmann           2013-12-17   10  #include <linux/slab.h>
44adece5 Daniel Vetter           2016-08-10   11  #include <drm/drm_fb_helper.h>
0a6659bd Gerd Hoffmann           2013-12-17   12  
0a6659bd Gerd Hoffmann           2013-12-17   13  #include "bochs.h"
0a6659bd Gerd Hoffmann           2013-12-17   14  
1acf5661 Max Staudt              2017-01-18   15  static int bochs_modeset = -1;
1acf5661 Max Staudt              2017-01-18   16  module_param_named(modeset, bochs_modeset, int, 0444);
1acf5661 Max Staudt              2017-01-18   17  MODULE_PARM_DESC(modeset, "enable/disable kernel modesetting");
1acf5661 Max Staudt              2017-01-18   18  
0a6659bd Gerd Hoffmann           2013-12-17   19  static bool enable_fbdev = true;
0a6659bd Gerd Hoffmann           2013-12-17   20  module_param_named(fbdev, enable_fbdev, bool, 0444);
0a6659bd Gerd Hoffmann           2013-12-17   21  MODULE_PARM_DESC(fbdev, "register fbdev device");
0a6659bd Gerd Hoffmann           2013-12-17   22  
0a6659bd Gerd Hoffmann           2013-12-17   23  /* ---------------------------------------------------------------------- */
0a6659bd Gerd Hoffmann           2013-12-17   24  /* drm interface                                                          */
0a6659bd Gerd Hoffmann           2013-12-17   25  
11b3c20b Gabriel Krisman Bertazi 2017-01-06   26  static void bochs_unload(struct drm_device *dev)
0a6659bd Gerd Hoffmann           2013-12-17   27  {
0a6659bd Gerd Hoffmann           2013-12-17   28  	struct bochs_device *bochs = dev->dev_private;
0a6659bd Gerd Hoffmann           2013-12-17   29  
0a6659bd Gerd Hoffmann           2013-12-17   30  	bochs_fbdev_fini(bochs);
0a6659bd Gerd Hoffmann           2013-12-17   31  	bochs_kms_fini(bochs);
0a6659bd Gerd Hoffmann           2013-12-17   32  	bochs_mm_fini(bochs);
0a6659bd Gerd Hoffmann           2013-12-17   33  	bochs_hw_fini(dev);
0a6659bd Gerd Hoffmann           2013-12-17   34  	kfree(bochs);
0a6659bd Gerd Hoffmann           2013-12-17   35  	dev->dev_private = NULL;
0a6659bd Gerd Hoffmann           2013-12-17   36  }
0a6659bd Gerd Hoffmann           2013-12-17   37  
0a6659bd Gerd Hoffmann           2013-12-17   38  static int bochs_load(struct drm_device *dev, unsigned long flags)
0a6659bd Gerd Hoffmann           2013-12-17   39  {
0a6659bd Gerd Hoffmann           2013-12-17   40  	struct bochs_device *bochs;
0a6659bd Gerd Hoffmann           2013-12-17   41  	int ret;
0a6659bd Gerd Hoffmann           2013-12-17   42  
0a6659bd Gerd Hoffmann           2013-12-17   43  	bochs = kzalloc(sizeof(*bochs), GFP_KERNEL);
0a6659bd Gerd Hoffmann           2013-12-17   44  	if (bochs == NULL)
0a6659bd Gerd Hoffmann           2013-12-17   45  		return -ENOMEM;
0a6659bd Gerd Hoffmann           2013-12-17   46  	dev->dev_private = bochs;
0a6659bd Gerd Hoffmann           2013-12-17   47  	bochs->dev = dev;
0a6659bd Gerd Hoffmann           2013-12-17   48  
0a6659bd Gerd Hoffmann           2013-12-17   49  	ret = bochs_hw_init(dev, flags);
0a6659bd Gerd Hoffmann           2013-12-17   50  	if (ret)
0a6659bd Gerd Hoffmann           2013-12-17   51  		goto err;
0a6659bd Gerd Hoffmann           2013-12-17   52  
0a6659bd Gerd Hoffmann           2013-12-17   53  	ret = bochs_mm_init(bochs);
0a6659bd Gerd Hoffmann           2013-12-17   54  	if (ret)
0a6659bd Gerd Hoffmann           2013-12-17   55  		goto err;
0a6659bd Gerd Hoffmann           2013-12-17   56  
0a6659bd Gerd Hoffmann           2013-12-17   57  	ret = bochs_kms_init(bochs);
0a6659bd Gerd Hoffmann           2013-12-17   58  	if (ret)
0a6659bd Gerd Hoffmann           2013-12-17   59  		goto err;
0a6659bd Gerd Hoffmann           2013-12-17   60  
0a6659bd Gerd Hoffmann           2013-12-17   61  	if (enable_fbdev)
0a6659bd Gerd Hoffmann           2013-12-17   62  		bochs_fbdev_init(bochs);
0a6659bd Gerd Hoffmann           2013-12-17   63  
0a6659bd Gerd Hoffmann           2013-12-17   64  	return 0;
0a6659bd Gerd Hoffmann           2013-12-17   65  
0a6659bd Gerd Hoffmann           2013-12-17   66  err:
0a6659bd Gerd Hoffmann           2013-12-17   67  	bochs_unload(dev);
0a6659bd Gerd Hoffmann           2013-12-17   68  	return ret;
0a6659bd Gerd Hoffmann           2013-12-17   69  }
0a6659bd Gerd Hoffmann           2013-12-17   70  
0a6659bd Gerd Hoffmann           2013-12-17   71  static const struct file_operations bochs_fops = {
0a6659bd Gerd Hoffmann           2013-12-17   72  	.owner		= THIS_MODULE,
0a6659bd Gerd Hoffmann           2013-12-17   73  	.open		= drm_open,
0a6659bd Gerd Hoffmann           2013-12-17   74  	.release	= drm_release,
0a6659bd Gerd Hoffmann           2013-12-17   75  	.unlocked_ioctl	= drm_ioctl,
0a6659bd Gerd Hoffmann           2013-12-17   76  	.compat_ioctl	= drm_compat_ioctl,
0a6659bd Gerd Hoffmann           2013-12-17   77  	.poll		= drm_poll,
0a6659bd Gerd Hoffmann           2013-12-17   78  	.read		= drm_read,
0a6659bd Gerd Hoffmann           2013-12-17   79  	.llseek		= no_llseek,
0a6659bd Gerd Hoffmann           2013-12-17   80  	.mmap           = bochs_mmap,
0a6659bd Gerd Hoffmann           2013-12-17   81  };
0a6659bd Gerd Hoffmann           2013-12-17   82  
0a6659bd Gerd Hoffmann           2013-12-17   83  static struct drm_driver bochs_driver = {
0a6659bd Gerd Hoffmann           2013-12-17   84  	.driver_features	= DRIVER_GEM | DRIVER_MODESET,
0a6659bd Gerd Hoffmann           2013-12-17   85  	.load			= bochs_load,
0a6659bd Gerd Hoffmann           2013-12-17   86  	.unload			= bochs_unload,
0a6659bd Gerd Hoffmann           2013-12-17   87  	.fops			= &bochs_fops,
0a6659bd Gerd Hoffmann           2013-12-17   88  	.name			= "bochs-drm",
0a6659bd Gerd Hoffmann           2013-12-17   89  	.desc			= "bochs dispi vga interface (qemu stdvga)",
0a6659bd Gerd Hoffmann           2013-12-17   90  	.date			= "20130925",
0a6659bd Gerd Hoffmann           2013-12-17   91  	.major			= 1,
0a6659bd Gerd Hoffmann           2013-12-17   92  	.minor			= 0,
6f2c1c15 Daniel Vetter           2016-05-30   93  	.gem_free_object_unlocked = bochs_gem_free_object,
0a6659bd Gerd Hoffmann           2013-12-17   94  	.dumb_create            = bochs_dumb_create,
0a6659bd Gerd Hoffmann           2013-12-17   95  	.dumb_map_offset        = bochs_dumb_mmap_offset,
0a6659bd Gerd Hoffmann           2013-12-17   96  };
0a6659bd Gerd Hoffmann           2013-12-17   97  
0a6659bd Gerd Hoffmann           2013-12-17   98  /* ---------------------------------------------------------------------- */
b8ccd70f Gerd Hoffmann           2014-04-14   99  /* pm interface                                                           */
b8ccd70f Gerd Hoffmann           2014-04-14  100  
150cee9c Russell King            2014-07-12  101  #ifdef CONFIG_PM_SLEEP
b8ccd70f Gerd Hoffmann           2014-04-14  102  static int bochs_pm_suspend(struct device *dev)
b8ccd70f Gerd Hoffmann           2014-04-14  103  {
b8ccd70f Gerd Hoffmann           2014-04-14  104  	struct pci_dev *pdev = to_pci_dev(dev);
b8ccd70f Gerd Hoffmann           2014-04-14  105  	struct drm_device *drm_dev = pci_get_drvdata(pdev);
b8ccd70f Gerd Hoffmann           2014-04-14  106  	struct bochs_device *bochs = drm_dev->dev_private;
b8ccd70f Gerd Hoffmann           2014-04-14  107  
b8ccd70f Gerd Hoffmann           2014-04-14  108  	drm_kms_helper_poll_disable(drm_dev);
b8ccd70f Gerd Hoffmann           2014-04-14  109  
b8ccd70f Gerd Hoffmann           2014-04-14 @110  	if (bochs->fb.initialized) {
b8ccd70f Gerd Hoffmann           2014-04-14  111  		console_lock();
6a752972 Archit Taneja           2015-07-31  112  		drm_fb_helper_set_suspend(&bochs->fb.helper, 1);
b8ccd70f Gerd Hoffmann           2014-04-14  113  		console_unlock();
b8ccd70f Gerd Hoffmann           2014-04-14  114  	}
b8ccd70f Gerd Hoffmann           2014-04-14  115  
b8ccd70f Gerd Hoffmann           2014-04-14  116  	return 0;
b8ccd70f Gerd Hoffmann           2014-04-14  117  }
b8ccd70f Gerd Hoffmann           2014-04-14  118  

:::::: The code at line 110 was first introduced by commit
:::::: b8ccd70f1363f7d4e49219dbc46ec973a14f49cd drm: bochs: add power management support

:::::: TO: Gerd Hoffmann <kraxel@redhat.com>
:::::: CC: Dave Airlie <airlied@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch
diff mbox series

diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
index 375bf92cd04f..8514a84fbdbe 100644
--- a/drivers/gpu/drm/bochs/bochs.h
+++ b/drivers/gpu/drm/bochs/bochs.h
@@ -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;
diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c b/drivers/gpu/drm/bochs/bochs_fbdev.c
index 14eb8d0d5a00..8f4d6c052f7b 100644
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -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);
 }
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
index 233980a78591..c7e575511d2f 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -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;
diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
index 39cd08416773..cdbc3ca3ec51 100644
--- a/drivers/gpu/drm/bochs/bochs_mm.c
+++ b/drivers/gpu/drm/bochs/bochs_mm.c
@@ -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,
-};