From c7d73f6a8ad71f9d9f58c86981322c6e48093a4f Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 13 Dec 2012 23:38:38 +0100 Subject: drm/: reorder framebuffer init sequence With more fine-grained locking we can no longer rely on the big mode_config lock to prevent concurrent access to mode resources like framebuffers. Instead a framebuffer becomes accessible to other threads as soon as it is added to the relevant lookup structures. Hence it needs to be fully set up by the time drivers call drm_framebuffer_init. This patch here is the drivers part of that reorg. Nothing really fancy going on safe for three special cases. - exynos needs to be careful to properly unref all handles. - nouveau gets a resource leak fixed for free: one of the error cases didn't cleanup the framebuffer, which is now moot since the framebuffer is only registered once it is fully set up. - vmwgfx requires a slight reordering of operations, I'm hoping I didn't break anything (but it's refcount management only, so should be safe). v2: Split out exynos, since it's a bit more hairy than expected. v3: Drop bogus cirrus hunk noticed by Richard Wilbur. v4: Split out vmwgfx since there's a small change in return values. Reviewed-by: Rob Clark (core + omapdrm) Signed-off-by: Daniel Vetter --- drivers/gpu/drm/cirrus/cirrus_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/cirrus') diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c index 6a9b12e88d46..364474c66202 100644 --- a/drivers/gpu/drm/cirrus/cirrus_main.c +++ b/drivers/gpu/drm/cirrus/cirrus_main.c @@ -42,13 +42,13 @@ int cirrus_framebuffer_init(struct drm_device *dev, { int ret; + drm_helper_mode_fill_fb_struct(&gfb->base, mode_cmd); + gfb->obj = obj; ret = drm_framebuffer_init(dev, &gfb->base, &cirrus_fb_funcs); if (ret) { DRM_ERROR("drm_framebuffer_init failed: %d\n", ret); return ret; } - drm_helper_mode_fill_fb_struct(&gfb->base, mode_cmd); - gfb->obj = obj; return 0; } -- cgit v1.2.3 From af26ef3b3978349cfbd864163a6ebb4906b733b5 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 13 Dec 2012 23:07:50 +0100 Subject: drm/: Unified handling of unimplemented fb->create_handle Some drivers don't have real ->create_handle callbacks. - cirrus/ast/mga200: Returns either 0 or -EINVAL. - udl: Didn't even bother with a callback, leading to a nice userspace-triggerable OOPS. - vmwgfx: This driver bothered with an implementation to return 0 as the handle (which is the canonical no-obj gem handle). All have in common that ->create_handle doesn't really make too much sense for them - that ioctl is used only for seamless fb takeover in the radeon/nouveau/i915 ddx drivers. So allow drivers to not implement this and return a consistent -ENODEV. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/cirrus/cirrus_main.c | 8 -------- 1 file changed, 8 deletions(-) (limited to 'drivers/gpu/drm/cirrus') diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c index 364474c66202..35cbae827771 100644 --- a/drivers/gpu/drm/cirrus/cirrus_main.c +++ b/drivers/gpu/drm/cirrus/cirrus_main.c @@ -23,16 +23,8 @@ static void cirrus_user_framebuffer_destroy(struct drm_framebuffer *fb) kfree(fb); } -static int cirrus_user_framebuffer_create_handle(struct drm_framebuffer *fb, - struct drm_file *file_priv, - unsigned int *handle) -{ - return 0; -} - static const struct drm_framebuffer_funcs cirrus_fb_funcs = { .destroy = cirrus_user_framebuffer_destroy, - .create_handle = cirrus_user_framebuffer_create_handle, }; int cirrus_framebuffer_init(struct drm_device *dev, -- cgit v1.2.3 From 362063619cf67c2c2fc2eb90951b2623cbb69a7c Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 10 Dec 2012 20:42:17 +0100 Subject: drm: revamp framebuffer cleanup interfaces We have two classes of framebuffer - Created by the driver (atm only for fbdev), and the driver holds onto the last reference count until destruction. - Created by userspace and associated with a given fd. These framebuffers will be reaped when their assoiciated fb is closed. Now these two cases are set up differently, the framebuffers are on different lists and hence destruction needs to clean up different things. Also, for userspace framebuffers we remove them from any current usage, whereas for internal framebuffers it is assumed that the driver has done this already. Long story short, we need two different ways to cleanup such drivers. Three functions are involved in total: - drm_framebuffer_remove: Convenience function which removes the fb from all active usage and then drops the passed-in reference. - drm_framebuffer_unregister_private: Will remove driver-private framebuffers from relevant lists and drop the corresponding references. Should be called for driver-private framebuffers before dropping the last reference (or like for a lot of the drivers where the fbdev is embedded someplace else, before doing the cleanup manually). - drm_framebuffer_cleanup: Final cleanup for both classes of fbs, should be called by the driver's ->destroy callback once the last reference is gone. This patch just rolls out the new interfaces and updates all drivers (by adding calls to drm_framebuffer_unregister_private at all the right places)- no functional changes yet. Follow-on patches will move drm core code around and update the lifetime management for framebuffers, so that we are no longer required to keep framebuffers alive by locking mode_config.mutex. I've also updated the kerneldoc already. vmwgfx seems to again be a bit special, at least I haven't figured out how the fbdev support in that driver works. It smells like it's external though. v2: The i915 driver creates another private framebuffer in the load-detect code. Adjust its cleanup code, too. Reviewed-by: Rob Clark Signed-off-by: Daniel Vetter --- drivers/gpu/drm/cirrus/cirrus_fbdev.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/gpu/drm/cirrus') diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c index 6c6b4c87d309..3daea0f638c3 100644 --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c @@ -258,6 +258,7 @@ static int cirrus_fbdev_destroy(struct drm_device *dev, vfree(gfbdev->sysram); drm_fb_helper_fini(&gfbdev->helper); + drm_framebuffer_unregister_private(&gfb->base); drm_framebuffer_cleanup(&gfb->base); return 0; -- cgit v1.2.3 From 76a39dbfb2d1bc45219839e5a95d4ceaf6ca114f Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sun, 20 Jan 2013 23:12:54 +0100 Subject: drm/fb-helper: don't disable everything in initial_config This should be done in the drivers for two reasons: - it gets in the way of fastboot efforts - it links the fb helpers with the crtc helpers instead of going through the real interface vfuncs, forcing i915 to fake all the ->disable callbacks used by the crtc helper to avoid ugly Oopsen v2: Resolve conflicts since drivers still call drm_fb_helper_single_add_all_connectors. Reviewed-by: Rob Clark Signed-off-by: Daniel Vetter --- drivers/gpu/drm/cirrus/cirrus_fbdev.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/gpu/drm/cirrus') diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c index 3daea0f638c3..b96605c6e1b6 100644 --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c @@ -11,6 +11,7 @@ #include #include #include +#include #include @@ -291,6 +292,9 @@ int cirrus_fbdev_init(struct cirrus_device *cdev) return ret; } drm_fb_helper_single_add_all_connectors(&gfbdev->helper); + + /* disable all the possible outputs/crtcs before entering KMS mode */ + drm_helper_disable_unused_functions(cdev->dev); drm_fb_helper_initial_config(&gfbdev->helper, bpp_sel); return 0; -- cgit v1.2.3 From cd5428a5447cc6ca77ec6547d6f86834b205eac7 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 21 Jan 2013 23:42:49 +0100 Subject: drm/: simplify ->fb_probe callback The fb helper lost its support for reallocating an fb completely, so no need to return special success values any more. Reviewed-by: Rob Clark Signed-off-by: Daniel Vetter --- drivers/gpu/drm/cirrus/cirrus_fbdev.c | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) (limited to 'drivers/gpu/drm/cirrus') diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c index b96605c6e1b6..e25afccaf85b 100644 --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c @@ -121,9 +121,10 @@ static int cirrusfb_create_object(struct cirrus_fbdev *afbdev, return ret; } -static int cirrusfb_create(struct cirrus_fbdev *gfbdev, +static int cirrusfb_create(struct drm_fb_helper *helper, struct drm_fb_helper_surface_size *sizes) { + struct cirrus_fbdev *gfbdev = (struct cirrus_fbdev *)helper; struct drm_device *dev = gfbdev->helper.dev; struct cirrus_device *cdev = gfbdev->helper.dev->dev_private; struct fb_info *info; @@ -220,23 +221,6 @@ out_iounmap: return ret; } -static int cirrus_fb_find_or_create_single(struct drm_fb_helper *helper, - struct drm_fb_helper_surface_size - *sizes) -{ - struct cirrus_fbdev *gfbdev = (struct cirrus_fbdev *)helper; - int new_fb = 0; - int ret; - - if (!helper->fb) { - ret = cirrusfb_create(gfbdev, sizes); - if (ret) - return ret; - new_fb = 1; - } - return new_fb; -} - static int cirrus_fbdev_destroy(struct drm_device *dev, struct cirrus_fbdev *gfbdev) { @@ -268,7 +252,7 @@ static int cirrus_fbdev_destroy(struct drm_device *dev, static struct drm_fb_helper_funcs cirrus_fb_helper_funcs = { .gamma_set = cirrus_crtc_fb_gamma_set, .gamma_get = cirrus_crtc_fb_gamma_get, - .fb_probe = cirrus_fb_find_or_create_single, + .fb_probe = cirrusfb_create, }; int cirrus_fbdev_init(struct cirrus_device *cdev) -- cgit v1.2.3