dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: DRI Development <dri-devel@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH 03/35] drm/<drivers>: reorder framebuffer init sequence
Date: Thu, 10 Jan 2013 21:47:44 +0100	[thread overview]
Message-ID: <1357850897-27102-4-git-send-email-daniel.vetter@ffwll.ch> (raw)
In-Reply-To: <1357850897-27102-1-git-send-email-daniel.vetter@ffwll.ch>

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.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/ast/ast_main.c            |    4 ++--
 drivers/gpu/drm/cirrus/cirrus_main.c      |    4 ++--
 drivers/gpu/drm/drm_fb_cma_helper.c       |   10 +++++-----
 drivers/gpu/drm/gma500/framebuffer.c      |    4 ++--
 drivers/gpu/drm/i915/intel_display.c      |    5 +++--
 drivers/gpu/drm/mgag200/mgag200_main.c    |    8 +++++---
 drivers/gpu/drm/nouveau/nouveau_display.c |   10 +++++-----
 drivers/gpu/drm/radeon/radeon_display.c   |    2 +-
 drivers/gpu/drm/udl/udl_fb.c              |    2 +-
 drivers/staging/omapdrm/omap_fb.c         |   16 ++++++++--------
 10 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f668e6c..d5ba709 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -266,13 +266,13 @@ int ast_framebuffer_init(struct drm_device *dev,
 {
 	int ret;
 
+	drm_helper_mode_fill_fb_struct(&ast_fb->base, mode_cmd);
+	ast_fb->obj = obj;
 	ret = drm_framebuffer_init(dev, &ast_fb->base, &ast_fb_funcs);
 	if (ret) {
 		DRM_ERROR("framebuffer init failed %d\n", ret);
 		return ret;
 	}
-	drm_helper_mode_fill_fb_struct(&ast_fb->base, mode_cmd);
-	ast_fb->obj = obj;
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c
index 6a9b12e..364474c 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;
 }
 
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index fd9d0af..e1e0cb0 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -85,6 +85,11 @@ static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
 	if (!fb_cma)
 		return ERR_PTR(-ENOMEM);
 
+	drm_helper_mode_fill_fb_struct(&fb_cma->fb, mode_cmd);
+
+	for (i = 0; i < num_planes; i++)
+		fb_cma->obj[i] = obj[i];
+
 	ret = drm_framebuffer_init(dev, &fb_cma->fb, &drm_fb_cma_funcs);
 	if (ret) {
 		dev_err(dev->dev, "Failed to initalize framebuffer: %d\n", ret);
@@ -92,11 +97,6 @@ static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
 		return ERR_PTR(ret);
 	}
 
-	drm_helper_mode_fill_fb_struct(&fb_cma->fb, mode_cmd);
-
-	for (i = 0; i < num_planes; i++)
-		fb_cma->obj[i] = obj[i];
-
 	return fb_cma;
 }
 
diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index afded54..38e7e75 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -260,13 +260,13 @@ static int psb_framebuffer_init(struct drm_device *dev,
 	default:
 		return -EINVAL;
 	}
+	drm_helper_mode_fill_fb_struct(&fb->base, mode_cmd);
+	fb->gtt = gt;
 	ret = drm_framebuffer_init(dev, &fb->base, &psb_fb_funcs);
 	if (ret) {
 		dev_err(dev->dev, "framebuffer init failed: %d\n", ret);
 		return ret;
 	}
-	drm_helper_mode_fill_fb_struct(&fb->base, mode_cmd);
-	fb->gtt = gt;
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cf0c713..b2af790 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8387,14 +8387,15 @@ int intel_framebuffer_init(struct drm_device *dev,
 	if (mode_cmd->offsets[0] != 0)
 		return -EINVAL;
 
+	drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
+	intel_fb->obj = obj;
+
 	ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
 	if (ret) {
 		DRM_ERROR("framebuffer init failed %d\n", ret);
 		return ret;
 	}
 
-	drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
-	intel_fb->obj = obj;
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index 70dd3c5..266438a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -40,13 +40,15 @@ int mgag200_framebuffer_init(struct drm_device *dev,
 			     struct drm_mode_fb_cmd2 *mode_cmd,
 			     struct drm_gem_object *obj)
 {
-	int ret = drm_framebuffer_init(dev, &gfb->base, &mga_fb_funcs);
+	int ret;
+	
+	drm_helper_mode_fill_fb_struct(&gfb->base, mode_cmd);
+	gfb->obj = obj;
+	ret = drm_framebuffer_init(dev, &gfb->base, &mga_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;
 }
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index e4188f2..2591ff2 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -78,11 +78,6 @@ nouveau_framebuffer_init(struct drm_device *dev,
 	struct drm_framebuffer *fb = &nv_fb->base;
 	int ret;
 
-	ret = drm_framebuffer_init(dev, fb, &nouveau_framebuffer_funcs);
-	if (ret) {
-		return ret;
-	}
-
 	drm_helper_mode_fill_fb_struct(fb, mode_cmd);
 	nv_fb->nvbo = nvbo;
 
@@ -125,6 +120,11 @@ nouveau_framebuffer_init(struct drm_device *dev,
 		}
 	}
 
+	ret = drm_framebuffer_init(dev, fb, &nouveau_framebuffer_funcs);
+	if (ret) {
+		return ret;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 1da2386..12ec3ef 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -1089,12 +1089,12 @@ radeon_framebuffer_init(struct drm_device *dev,
 {
 	int ret;
 	rfb->obj = obj;
+	drm_helper_mode_fill_fb_struct(&rfb->base, mode_cmd);
 	ret = drm_framebuffer_init(dev, &rfb->base, &radeon_fb_funcs);
 	if (ret) {
 		rfb->obj = NULL;
 		return ret;
 	}
-	drm_helper_mode_fill_fb_struct(&rfb->base, mode_cmd);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index d4ab3be..829c4a7 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -435,8 +435,8 @@ udl_framebuffer_init(struct drm_device *dev,
 	int ret;
 
 	ufb->obj = obj;
-	ret = drm_framebuffer_init(dev, &ufb->base, &udlfb_funcs);
 	drm_helper_mode_fill_fb_struct(&ufb->base, mode_cmd);
+	ret = drm_framebuffer_init(dev, &ufb->base, &udlfb_funcs);
 	return ret;
 }
 
diff --git a/drivers/staging/omapdrm/omap_fb.c b/drivers/staging/omapdrm/omap_fb.c
index 09028e9..bf6421f 100644
--- a/drivers/staging/omapdrm/omap_fb.c
+++ b/drivers/staging/omapdrm/omap_fb.c
@@ -424,14 +424,6 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
 	}
 
 	fb = &omap_fb->base;
-	ret = drm_framebuffer_init(dev, fb, &omap_framebuffer_funcs);
-	if (ret) {
-		dev_err(dev->dev, "framebuffer init failed: %d\n", ret);
-		goto fail;
-	}
-
-	DBG("create: FB ID: %d (%p)", fb->base.id, fb);
-
 	omap_fb->format = format;
 
 	for (i = 0; i < n; i++) {
@@ -462,6 +454,14 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
 
 	drm_helper_mode_fill_fb_struct(fb, mode_cmd);
 
+	ret = drm_framebuffer_init(dev, fb, &omap_framebuffer_funcs);
+	if (ret) {
+		dev_err(dev->dev, "framebuffer init failed: %d\n", ret);
+		goto fail;
+	}
+
+	DBG("create: FB ID: %d (%p)", fb->base.id, fb);
+
 	return fb;
 
 fail:
-- 
1.7.10.4

  parent reply	other threads:[~2013-01-10 20:48 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-10 20:47 [PATCH 00/36] kms locking rework Daniel Vetter
2013-01-10 20:47 ` [PATCH 01/35] drm: review locking rules in drm_crtc.c Daniel Vetter
2013-01-10 20:47 ` [PATCH 02/35] drm/doc: integrate drm_crtc.c kerneldoc Daniel Vetter
2013-01-10 20:47 ` Daniel Vetter [this message]
2013-01-11 21:06   ` [PATCH 03/35] drm/<drivers>: reorder framebuffer init sequence Rob Clark
2013-01-10 20:47 ` [PATCH 04/35] drm/vmwgfx: " Daniel Vetter
2013-01-10 20:47 ` [PATCH 05/35] drm/gma500: move fbcon restore to lastclose Daniel Vetter
2013-01-10 20:47 ` [PATCH 06/35] drm/nouveau: protect evo_wait/evo_kick sections with a channel mutex Daniel Vetter
2013-01-10 20:47 ` [PATCH 07/35] drm/nouveau: try to protect nbo->pin_refcount Daniel Vetter
2013-01-10 20:47 ` [PATCH 08/35] drm/<drivers>: Unified handling of unimplemented fb->create_handle Daniel Vetter
2013-01-18 15:00   ` Thierry Reding
2013-01-18 18:32     ` Daniel Vetter
2013-01-10 20:47 ` [PATCH 09/35] drm: encapsulate crtc->set_config calls Daniel Vetter
2013-01-10 20:47 ` [PATCH 10/35] drm: add drm_modeset_lock|unlock_all Daniel Vetter
2013-01-10 20:47 ` [PATCH 11/35] drm/i915: use drm_modeset_lock_all Daniel Vetter
2013-01-10 20:47 ` [PATCH 12/35] drm/gma500: " Daniel Vetter
2013-01-10 22:36   ` Alan Cox
2013-01-11 13:25     ` Daniel Vetter
2013-01-10 20:47 ` [PATCH 13/35] drm/ast: " Daniel Vetter
2013-01-10 20:47 ` [PATCH 14/35] drm/shmobile: " Daniel Vetter
2013-01-10 20:47 ` [PATCH 15/35] drm/vmgfx: " Daniel Vetter
2013-01-10 20:47 ` [PATCH 16/35] drm: add per-crtc locks Daniel Vetter
2013-01-10 20:47 ` [PATCH 17/35] drm: only take the crtc lock for ->cursor_set Daniel Vetter
2013-01-10 20:47 ` [PATCH 18/35] drm: only take the crtc lock for ->cursor_move Daniel Vetter
2013-01-10 20:48 ` [PATCH 19/35] drm: revamp locking around fb creation/destruction Daniel Vetter
2013-01-10 20:48 ` [PATCH 20/35] drm: create drm_framebuffer_lookup Daniel Vetter
2013-01-10 20:48 ` [PATCH 21/35] drm: revamp framebuffer cleanup interfaces Daniel Vetter
2013-01-10 20:48 ` [PATCH 22/35] drm: reference framebuffers which are on the idr Daniel Vetter
2013-01-10 20:48 ` [PATCH 23/35] drm: nest modeset locks within fpriv->fbs_lock Daniel Vetter
2013-01-10 20:48 ` [PATCH 24/35] drm: push modeset_lock_all into ->fb_create driver callbacks Daniel Vetter
2013-01-10 20:48 ` [PATCH 25/35] drm: don't take modeset locks in getfb ioctl Daniel Vetter
2013-01-10 20:48 ` [PATCH 26/35] drm: fb refcounting for dirtyfb_ioctl Daniel Vetter
2013-01-10 20:48 ` [PATCH 27/35] drm: refcounting for sprite framebuffers Daniel Vetter
2013-01-10 20:48 ` [PATCH 28/35] drm: refcounting for crtc framebuffers Daniel Vetter
2013-01-10 20:48 ` [PATCH 29/35] drm/i915: dump refcount into framebuffer debugfs file Daniel Vetter
2013-01-11 22:20   ` Rob Clark
2013-01-10 20:48 ` [PATCH 30/35] drm/vmwgfx: add proper framebuffer refcounting Daniel Vetter
2013-01-10 20:48 ` [PATCH 31/35] drm: optimize drm_framebuffer_remove Daniel Vetter
2013-01-10 20:48 ` [PATCH 32/35] drm: only grab the crtc lock for pageflips Daniel Vetter
2013-01-10 20:48 ` [PATCH 33/35] drm: don't hold crtc mutexes for connector ->detect callbacks Daniel Vetter
2013-01-10 20:48 ` [PATCH 34/35] drm/doc: updates for new framebuffer lifetime rules Daniel Vetter
2013-01-10 20:48 ` [PATCH 35/35] drm/fb_helper: check whether fbcon is bound Daniel Vetter
2013-01-10 20:48 ` [PATCH 36/36] drm/i915: wake up all pageflip waiters Daniel Vetter
2013-01-10 20:50   ` Daniel Vetter
2013-01-11 23:17 ` [PATCH 00/36] kms locking rework Rob Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1357850897-27102-4-git-send-email-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).