* [PATCH] bochs: convert to drm_fb_helper_fbdev_setup/teardown
@ 2018-09-05 14:41 Peter Wu
2018-09-05 14:46 ` Daniel Vetter
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Peter Wu @ 2018-09-05 14:41 UTC (permalink / raw)
To: Gerd Hoffmann, David Airlie
Cc: virtualization, dri-devel, linux-kernel, Noralf Trønnes
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(-)
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] bochs: convert to drm_fb_helper_fbdev_setup/teardown
2018-09-05 14:41 [PATCH] bochs: convert to drm_fb_helper_fbdev_setup/teardown Peter Wu
2018-09-05 14:46 ` Daniel Vetter
@ 2018-09-05 14:46 ` Daniel Vetter
2018-09-05 15:06 ` Peter Wu
2018-09-07 8:10 ` kbuild test robot
2 siblings, 1 reply; 8+ messages in thread
From: Daniel Vetter @ 2018-09-05 14:46 UTC (permalink / raw)
To: Peter Wu
Cc: Gerd Hoffmann, David Airlie, linux-kernel, dri-devel, virtualization
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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] bochs: convert to drm_fb_helper_fbdev_setup/teardown
2018-09-05 14:41 [PATCH] bochs: convert to drm_fb_helper_fbdev_setup/teardown Peter Wu
@ 2018-09-05 14:46 ` Daniel Vetter
2018-09-05 14:46 ` Daniel Vetter
2018-09-07 8:10 ` kbuild test robot
2 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2018-09-05 14:46 UTC (permalink / raw)
To: Peter Wu; +Cc: David Airlie, virtualization, dri-devel, linux-kernel
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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] bochs: convert to drm_fb_helper_fbdev_setup/teardown
2018-09-05 14:46 ` Daniel Vetter
@ 2018-09-05 15:06 ` Peter Wu
2018-09-06 7:23 ` Gerd Hoffmann
2018-09-06 7:23 ` Gerd Hoffmann
0 siblings, 2 replies; 8+ messages in thread
From: Peter Wu @ 2018-09-05 15:06 UTC (permalink / raw)
To: Gerd Hoffmann, David Airlie, linux-kernel, dri-devel, virtualization
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
--
Kind regards,
Peter Wu
https://lekensteyn.nl
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] bochs: convert to drm_fb_helper_fbdev_setup/teardown
2018-09-05 15:06 ` Peter Wu
2018-09-06 7:23 ` Gerd Hoffmann
@ 2018-09-06 7:23 ` Gerd Hoffmann
1 sibling, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2018-09-06 7:23 UTC (permalink / raw)
To: Peter Wu; +Cc: David Airlie, linux-kernel, dri-devel, virtualization
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] bochs: convert to drm_fb_helper_fbdev_setup/teardown
2018-09-05 15:06 ` Peter Wu
@ 2018-09-06 7:23 ` Gerd Hoffmann
2018-09-06 7:23 ` Gerd Hoffmann
1 sibling, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2018-09-06 7:23 UTC (permalink / raw)
To: Peter Wu; +Cc: David Airlie, linux-kernel, dri-devel, virtualization
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] bochs: convert to drm_fb_helper_fbdev_setup/teardown
2018-09-05 14:41 [PATCH] bochs: convert to drm_fb_helper_fbdev_setup/teardown Peter Wu
@ 2018-09-07 8:10 ` kbuild test robot
2018-09-05 14:46 ` Daniel Vetter
2018-09-07 8:10 ` kbuild test robot
2 siblings, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2018-09-07 8:10 UTC (permalink / raw)
To: Peter Wu
Cc: kbuild-all, Gerd Hoffmann, David Airlie, linux-kernel, dri-devel,
virtualization
[-- Attachment #1: Type: text/plain, Size: 12327 bytes --]
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
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30665 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] bochs: convert to drm_fb_helper_fbdev_setup/teardown
@ 2018-09-07 8:10 ` kbuild test robot
0 siblings, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2018-09-07 8:10 UTC (permalink / raw)
To: Peter Wu
Cc: David Airlie, linux-kernel, dri-devel, virtualization, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 12327 bytes --]
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
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30665 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-09-07 8:10 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-05 14:41 [PATCH] bochs: convert to drm_fb_helper_fbdev_setup/teardown Peter Wu
2018-09-05 14:46 ` Daniel Vetter
2018-09-05 14:46 ` Daniel Vetter
2018-09-05 15:06 ` Peter Wu
2018-09-06 7:23 ` Gerd Hoffmann
2018-09-06 7:23 ` Gerd Hoffmann
2018-09-07 8:10 ` kbuild test robot
2018-09-07 8:10 ` kbuild test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.