From bf8744e40cd6db20dfbd231ad44943f6bc8ac311 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Mon, 10 Sep 2018 15:21:56 +0200 Subject: qxl: refactor to use drm_fb_helper_fbdev_setup Lots of code can be removed by relying on fb-helper: - "struct drm_framebuffer" moves to fb_helper.fb. - "struct drm_gem_object" moves to fb_helper.obj[0]. - "struct qxl_device" can be inferred as drm_fb_helper is embedded. - qxl_user_framebuffer_create -> drm_gem_fb_create. - qxl_user_framebuffer_destroy -> drm_gem_fb_destroy. - qxl_fbdev_destroy -> drm_fb_helper_fbdev_teardown + vfree(shadow). Remove unused code: - qxl_fbdev_qobj_is_fb, qxl_fbdev_set_suspend. - Unused fields of qxl_fbdev: delayed_ops, delayed_ops_lock, size. Misc notes: - The dirty callback is preserved as it is necessary to trigger update commands in the hw (the screen stays black otherwise). - No idea when .create_handle in drm_framebuffer_funcs is used, but use the same drm_gem_fb_create_handle to match drm_gem_fb_funcs. - I don't know why qxl_fb_find_or_create_single used to check for an existing framebuffer and removed that check to match other drivers. - Use of drm_fb_helper_fbdev_teardown also requires "info->fbdefio" to be dynamically allocated. Replace the existing defio config by drm_fb_helper_defio_init to accomodate this. Testing results: startx with fbdev, modesetting and qxl all seems to work. Tested also with CONFIG_DRM_FBDEV_EMULATION=n, fbdev obviously fails but others are fine. QEMU -spice and QEMU -spice with vdagent and multiple (resized) displays (via remote-viewer) also works. unbind vtconsole and rmmod has *not* regressed (i.e. it still trips on a use-after-free in qxl_check_idle via qxl_ttm_fini). Ideally setup/teardown is replaced by drm_fbdev_generic_setup as that would result in further code reduction, improve error handling (like not leaking shadow memory), but unfortunately QXL has no implementation for qxl_gem_prime_vmap. Signed-off-by: Peter Wu Link: http://patchwork.freedesktop.org/patch/msgid/20180910132156.23201-1-peter@lekensteyn.nl Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_display.c | 101 ++++++++------------------------------ 1 file changed, 20 insertions(+), 81 deletions(-) (limited to 'drivers/gpu/drm/qxl/qxl_display.c') diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index 01704a7f07cb..87d16a0ce01e 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -28,6 +28,7 @@ #include #include #include +#include #include "qxl_drv.h" #include "qxl_object.h" @@ -388,17 +389,6 @@ static const struct drm_crtc_funcs qxl_crtc_funcs = { .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, }; -void qxl_user_framebuffer_destroy(struct drm_framebuffer *fb) -{ - struct qxl_framebuffer *qxl_fb = to_qxl_framebuffer(fb); - struct qxl_bo *bo = gem_to_qxl_bo(qxl_fb->obj); - - WARN_ON(bo->shadow); - drm_gem_object_put_unlocked(qxl_fb->obj); - drm_framebuffer_cleanup(fb); - kfree(qxl_fb); -} - static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb, struct drm_file *file_priv, unsigned flags, unsigned color, @@ -406,15 +396,14 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb, unsigned num_clips) { /* TODO: vmwgfx where this was cribbed from had locking. Why? */ - struct qxl_framebuffer *qxl_fb = to_qxl_framebuffer(fb); - struct qxl_device *qdev = qxl_fb->base.dev->dev_private; + struct qxl_device *qdev = fb->dev->dev_private; struct drm_clip_rect norect; struct qxl_bo *qobj; int inc = 1; drm_modeset_lock_all(fb->dev); - qobj = gem_to_qxl_bo(qxl_fb->obj); + qobj = gem_to_qxl_bo(fb->obj[0]); /* if we aren't primary surface ignore this */ if (!qobj->is_primary) { drm_modeset_unlock_all(fb->dev); @@ -432,7 +421,7 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb, inc = 2; /* skip source rects */ } - qxl_draw_dirty_fb(qdev, qxl_fb, qobj, flags, color, + qxl_draw_dirty_fb(qdev, fb, qobj, flags, color, clips, num_clips, inc); drm_modeset_unlock_all(fb->dev); @@ -441,31 +430,11 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb, } static const struct drm_framebuffer_funcs qxl_fb_funcs = { - .destroy = qxl_user_framebuffer_destroy, + .destroy = drm_gem_fb_destroy, .dirty = qxl_framebuffer_surface_dirty, -/* TODO? - * .create_handle = qxl_user_framebuffer_create_handle, */ + .create_handle = drm_gem_fb_create_handle, }; -int -qxl_framebuffer_init(struct drm_device *dev, - struct qxl_framebuffer *qfb, - const struct drm_mode_fb_cmd2 *mode_cmd, - struct drm_gem_object *obj, - const struct drm_framebuffer_funcs *funcs) -{ - int ret; - - qfb->obj = obj; - drm_helper_mode_fill_fb_struct(dev, &qfb->base, mode_cmd); - ret = drm_framebuffer_init(dev, &qfb->base, funcs); - if (ret) { - qfb->obj = NULL; - return ret; - } - return 0; -} - static void qxl_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { @@ -488,14 +457,12 @@ static int qxl_primary_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) { struct qxl_device *qdev = plane->dev->dev_private; - struct qxl_framebuffer *qfb; struct qxl_bo *bo; if (!state->crtc || !state->fb) return 0; - qfb = to_qxl_framebuffer(state->fb); - bo = gem_to_qxl_bo(qfb->obj); + bo = gem_to_qxl_bo(state->fb->obj[0]); if (bo->surf.stride * bo->surf.height > qdev->vram_size) { DRM_ERROR("Mode doesn't fit in vram size (vgamem)"); @@ -556,23 +523,19 @@ static void qxl_primary_atomic_update(struct drm_plane *plane, struct drm_plane_state *old_state) { struct qxl_device *qdev = plane->dev->dev_private; - struct qxl_framebuffer *qfb = - to_qxl_framebuffer(plane->state->fb); - struct qxl_framebuffer *qfb_old; - struct qxl_bo *bo = gem_to_qxl_bo(qfb->obj); + struct qxl_bo *bo = gem_to_qxl_bo(plane->state->fb->obj[0]); struct qxl_bo *bo_old; struct drm_clip_rect norect = { .x1 = 0, .y1 = 0, - .x2 = qfb->base.width, - .y2 = qfb->base.height + .x2 = plane->state->fb->width, + .y2 = plane->state->fb->height }; int ret; bool same_shadow = false; if (old_state->fb) { - qfb_old = to_qxl_framebuffer(old_state->fb); - bo_old = gem_to_qxl_bo(qfb_old->obj); + bo_old = gem_to_qxl_bo(old_state->fb->obj[0]); } else { bo_old = NULL; } @@ -602,7 +565,7 @@ static void qxl_primary_atomic_update(struct drm_plane *plane, bo->is_primary = true; } - qxl_draw_dirty_fb(qdev, qfb, bo, 0, 0, &norect, 1, 1); + qxl_draw_dirty_fb(qdev, plane->state->fb, bo, 0, 0, &norect, 1, 1); } static void qxl_primary_atomic_disable(struct drm_plane *plane, @@ -611,9 +574,7 @@ static void qxl_primary_atomic_disable(struct drm_plane *plane, struct qxl_device *qdev = plane->dev->dev_private; if (old_state->fb) { - struct qxl_framebuffer *qfb = - to_qxl_framebuffer(old_state->fb); - struct qxl_bo *bo = gem_to_qxl_bo(qfb->obj); + struct qxl_bo *bo = gem_to_qxl_bo(old_state->fb->obj[0]); if (bo->is_primary) { qxl_io_destroy_primary(qdev); @@ -645,7 +606,7 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane, return; if (fb != old_state->fb) { - obj = to_qxl_framebuffer(fb)->obj; + obj = fb->obj[0]; user_bo = gem_to_qxl_bo(obj); /* pinning is done in the prepare/cleanup framevbuffer */ @@ -765,13 +726,13 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane, if (!new_state->fb) return 0; - obj = to_qxl_framebuffer(new_state->fb)->obj; + obj = new_state->fb->obj[0]; user_bo = gem_to_qxl_bo(obj); if (plane->type == DRM_PLANE_TYPE_PRIMARY && user_bo->is_dumb && !user_bo->shadow) { if (plane->state->fb) { - obj = to_qxl_framebuffer(plane->state->fb)->obj; + obj = plane->state->fb->obj[0]; old_bo = gem_to_qxl_bo(obj); } if (old_bo && old_bo->shadow && @@ -815,7 +776,7 @@ static void qxl_plane_cleanup_fb(struct drm_plane *plane, return; } - obj = to_qxl_framebuffer(old_state->fb)->obj; + obj = old_state->fb->obj[0]; user_bo = gem_to_qxl_bo(obj); qxl_bo_unpin(user_bo); @@ -1115,26 +1076,8 @@ qxl_user_framebuffer_create(struct drm_device *dev, struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd) { - struct drm_gem_object *obj; - struct qxl_framebuffer *qxl_fb; - int ret; - - obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[0]); - if (!obj) - return NULL; - - qxl_fb = kzalloc(sizeof(*qxl_fb), GFP_KERNEL); - if (qxl_fb == NULL) - return NULL; - - ret = qxl_framebuffer_init(dev, qxl_fb, mode_cmd, obj, &qxl_fb_funcs); - if (ret) { - kfree(qxl_fb); - drm_gem_object_put_unlocked(obj); - return NULL; - } - - return &qxl_fb->base; + return drm_gem_fb_create_with_funcs(dev, file_priv, mode_cmd, + &qxl_fb_funcs); } static const struct drm_mode_config_funcs qxl_mode_funcs = { @@ -1221,7 +1164,6 @@ int qxl_modeset_init(struct qxl_device *qdev) } qxl_display_read_client_monitors_config(qdev); - qdev->mode_info.mode_config_initialized = true; drm_mode_config_reset(&qdev->ddev); @@ -1237,8 +1179,5 @@ void qxl_modeset_fini(struct qxl_device *qdev) qxl_fbdev_fini(qdev); qxl_destroy_monitors_object(qdev); - if (qdev->mode_info.mode_config_initialized) { - drm_mode_config_cleanup(&qdev->ddev); - qdev->mode_info.mode_config_initialized = false; - } + drm_mode_config_cleanup(&qdev->ddev); } -- cgit v1.2.3