[Intel-gfx] [PATCH 8/8] drm/i915: remove early fb allocation dependency on CONFIG_FB v2

Jesse Barnes jbarnes at virtuousgeek.org
Fri Mar 7 17:57:55 CET 2014


By stuffing the fb allocation into the crtc, we get mode set lifetime
refcounting for free, but have to handle the initial pin & fence
slightly differently.  It also means we can move the shared fb handling
into the core rather than leaving it out in the fbdev code.

v2: null out crtc->fb on error (Daniel)
    take fbdev fb ref and remove unused error path (Daniel)

Requested-by: Daniel Vetter <daniel at ffwll.ch>
Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 145 ++++++++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_drv.h     |   1 -
 drivers/gpu/drm/i915/intel_fbdev.c   |  38 +--------
 3 files changed, 105 insertions(+), 79 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d450ab6..718cc73 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2068,7 +2068,7 @@ int intel_format_to_fourcc(int format)
 	}
 }
 
-static void intel_alloc_plane_obj(struct intel_crtc *crtc,
+static bool intel_alloc_plane_obj(struct intel_crtc *crtc,
 				  struct intel_plane_config *plane_config)
 {
 	struct drm_device *dev = crtc->base.dev;
@@ -2076,38 +2076,76 @@ static void intel_alloc_plane_obj(struct intel_crtc *crtc,
 	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
 	u32 base = plane_config->base;
 
-	if (!plane_config->fb)
-		return;
-
 	obj = i915_gem_object_create_stolen_for_preallocated(dev, base, base,
 							     plane_config->size);
 	if (!obj)
-		return;
+		return false;
 
 	if (plane_config->tiled) {
 		obj->tiling_mode = I915_TILING_X;
-		obj->stride = plane_config->fb->base.pitches[0];
+		obj->stride = crtc->base.fb->pitches[0];
 	}
 
-	mode_cmd.pixel_format = plane_config->fb->base.pixel_format;
-	mode_cmd.width = plane_config->fb->base.width;
-	mode_cmd.height = plane_config->fb->base.height;
-	mode_cmd.pitches[0] = plane_config->fb->base.pitches[0];
+	mode_cmd.pixel_format = crtc->base.fb->pixel_format;
+	mode_cmd.width = crtc->base.fb->width;
+	mode_cmd.height = crtc->base.fb->height;
+	mode_cmd.pitches[0] = crtc->base.fb->pitches[0];
 
 	mutex_lock(&dev->struct_mutex);
 
-	if (intel_framebuffer_init(dev, plane_config->fb, &mode_cmd, obj)) {
+	if (intel_framebuffer_init(dev, to_intel_framebuffer(crtc->base.fb),
+				   &mode_cmd, obj)) {
 		DRM_DEBUG_KMS("intel fb init failed\n");
 		goto out_unref_obj;
 	}
 
 	mutex_unlock(&dev->struct_mutex);
-	DRM_DEBUG_KMS("plane fb obj %p\n", plane_config->fb->obj);
-	return;
+
+	DRM_DEBUG_KMS("plane fb obj %p\n", obj);
+	return true;
 
 out_unref_obj:
 	drm_gem_object_unreference(&obj->base);
 	mutex_unlock(&dev->struct_mutex);
+	return false;
+}
+
+static void intel_find_plane_obj(struct intel_crtc *intel_crtc,
+				 struct intel_plane_config *plane_config)
+{
+	struct drm_device *dev = intel_crtc->base.dev;
+	struct drm_crtc *c;
+	struct intel_crtc *i;
+	struct intel_framebuffer *fb;
+
+	if (!intel_crtc->base.fb)
+		return;
+
+	if (intel_alloc_plane_obj(intel_crtc, plane_config))
+		return;
+
+	kfree(intel_crtc->base.fb);
+
+	/*
+	 * Failed to alloc the obj, check to see if we should share
+	 * an fb with another CRTC instead
+	 */
+	list_for_each_entry(c, &dev->mode_config.crtc_list, head) {
+		i = to_intel_crtc(c);
+
+		if (c == &intel_crtc->base)
+			continue;
+
+		if (!i->active || !c->fb)
+			continue;
+
+		fb = to_intel_framebuffer(c->fb);
+		if (i915_gem_obj_ggtt_offset(fb->obj) == plane_config->base) {
+			drm_framebuffer_reference(c->fb);
+			intel_crtc->base.fb = c->fb;
+			break;
+		}
+	}
 }
 
 static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
@@ -5636,8 +5674,8 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
 	int fourcc, pixel_format;
 	int aligned_height;
 
-	plane_config->fb = kzalloc(sizeof(*plane_config->fb), GFP_KERNEL);
-	if (!plane_config->fb) {
+	crtc->base.fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
+	if (!crtc->base.fb) {
 		DRM_DEBUG_KMS("failed to alloc fb\n");
 		return;
 	}
@@ -5650,8 +5688,8 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
 
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
 	fourcc = intel_format_to_fourcc(pixel_format);
-	plane_config->fb->base.pixel_format = fourcc;
-	plane_config->fb->base.bits_per_pixel =
+	crtc->base.fb->pixel_format = fourcc;
+	crtc->base.fb->bits_per_pixel =
 		drm_format_plane_cpp(fourcc, 0) * 8;
 
 	if (INTEL_INFO(dev)->gen >= 4) {
@@ -5666,23 +5704,23 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
 	plane_config->base = base;
 
 	val = I915_READ(PIPESRC(pipe));
-	plane_config->fb->base.width = ((val >> 16) & 0xfff) + 1;
-	plane_config->fb->base.height = ((val >> 0) & 0xfff) + 1;
+	crtc->base.fb->width = ((val >> 16) & 0xfff) + 1;
+	crtc->base.fb->height = ((val >> 0) & 0xfff) + 1;
 
 	val = I915_READ(DSPSTRIDE(pipe));
-	plane_config->fb->base.pitches[0] = val & 0xffffff80;
+	crtc->base.fb->pitches[0] = val & 0xffffff80;
 
-	aligned_height = intel_align_height(dev, plane_config->fb->base.height,
+	aligned_height = intel_align_height(dev, crtc->base.fb->height,
 					    plane_config->tiled);
 
-	plane_config->size = ALIGN(plane_config->fb->base.pitches[0] *
+	plane_config->size = ALIGN(crtc->base.fb->pitches[0] *
 				   aligned_height, PAGE_SIZE);
 
 	DRM_DEBUG_KMS("pipe/plane %d/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
-		      pipe, plane, plane_config->fb->base.width,
-		      plane_config->fb->base.height,
-		      plane_config->fb->base.bits_per_pixel, base,
-		      plane_config->fb->base.pitches[0],
+		      pipe, plane, crtc->base.fb->width,
+		      crtc->base.fb->height,
+		      crtc->base.fb->bits_per_pixel, base,
+		      crtc->base.fb->pitches[0],
 		      plane_config->size);
 
 }
@@ -6644,8 +6682,8 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 	int fourcc, pixel_format;
 	int aligned_height;
 
-	plane_config->fb = kzalloc(sizeof(*plane_config->fb), GFP_KERNEL);
-	if (!plane_config->fb) {
+	crtc->base.fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
+	if (!crtc->base.fb) {
 		DRM_DEBUG_KMS("failed to alloc fb\n");
 		return;
 	}
@@ -6658,8 +6696,8 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
 	fourcc = intel_format_to_fourcc(pixel_format);
-	plane_config->fb->base.pixel_format = fourcc;
-	plane_config->fb->base.bits_per_pixel =
+	crtc->base.fb->pixel_format = fourcc;
+	crtc->base.fb->bits_per_pixel =
 		drm_format_plane_cpp(fourcc, 0) * 8;
 
 	base = I915_READ(DSPSURF(plane)) & 0xfffff000;
@@ -6674,23 +6712,23 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 	plane_config->base = base;
 
 	val = I915_READ(PIPESRC(pipe));
-	plane_config->fb->base.width = ((val >> 16) & 0xfff) + 1;
-	plane_config->fb->base.height = ((val >> 0) & 0xfff) + 1;
+	crtc->base.fb->width = ((val >> 16) & 0xfff) + 1;
+	crtc->base.fb->height = ((val >> 0) & 0xfff) + 1;
 
 	val = I915_READ(DSPSTRIDE(pipe));
-	plane_config->fb->base.pitches[0] = val & 0xffffff80;
+	crtc->base.fb->pitches[0] = val & 0xffffff80;
 
-	aligned_height = intel_align_height(dev, plane_config->fb->base.height,
+	aligned_height = intel_align_height(dev, crtc->base.fb->height,
 					    plane_config->tiled);
 
-	plane_config->size = ALIGN(plane_config->fb->base.pitches[0] *
+	plane_config->size = ALIGN(crtc->base.fb->pitches[0] *
 				   aligned_height, PAGE_SIZE);
 
 	DRM_DEBUG_KMS("pipe/plane %d/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
-		      pipe, plane, plane_config->fb->base.width,
-		      plane_config->fb->base.height,
-		      plane_config->fb->base.bits_per_pixel, base,
-		      plane_config->fb->base.pitches[0],
+		      pipe, plane, crtc->base.fb->width,
+		      crtc->base.fb->height,
+		      crtc->base.fb->bits_per_pixel, base,
+		      crtc->base.fb->pitches[0],
 		      plane_config->size);
 }
 
@@ -11258,10 +11296,7 @@ void intel_modeset_init(struct drm_device *dev)
 		if (!crtc->active)
 			continue;
 
-#if IS_ENABLED(CONFIG_FB)
 		/*
-		 * We don't have a good way of freeing the buffer w/o the FB
-		 * layer owning it...
 		 * Note that reserving the BIOS fb up front prevents us
 		 * from stuffing other stolen allocations like the ring
 		 * on top.  This prevents some ugliness at boot time, and
@@ -11275,9 +11310,8 @@ void intel_modeset_init(struct drm_device *dev)
 			 * If the fb is shared between multiple heads, we'll
 			 * just get the first one.
 			 */
-			intel_alloc_plane_obj(crtc, &crtc->plane_config);
+			intel_find_plane_obj(crtc, &crtc->plane_config);
 		}
-#endif
 	}
 }
 
@@ -11648,9 +11682,32 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 
 void intel_modeset_gem_init(struct drm_device *dev)
 {
+	struct drm_crtc *c;
+	struct intel_framebuffer *fb;
+
 	intel_modeset_init_hw(dev);
 
 	intel_setup_overlay(dev);
+
+	/*
+	 * Make sure any fbs we allocated at startup are properly
+	 * pinned & fenced.  When we do the allocation it's too early
+	 * for this.
+	 */
+	mutex_lock(&dev->struct_mutex);
+	list_for_each_entry(c, &dev->mode_config.crtc_list, head) {
+		if (!c->fb)
+			continue;
+
+		fb = to_intel_framebuffer(c->fb);
+		if (intel_pin_and_fence_fb_obj(dev, fb->obj, NULL)) {
+			DRM_ERROR("failed to pin boot fb on pipe %d\n",
+				  to_intel_crtc(c)->pipe);
+			drm_framebuffer_unreference(c->fb);
+			c->fb = NULL;
+		}
+	}
+	mutex_unlock(&dev->struct_mutex);
 }
 
 void intel_connector_unregister(struct intel_connector *intel_connector)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3d404ab..2ed72cf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -220,7 +220,6 @@ typedef struct dpll {
 } intel_clock_t;
 
 struct intel_plane_config {
-	struct intel_framebuffer *fb; /* ends up managed by intel_fbdev.c */
 	bool tiled;
 	int size;
 	u32 base;
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 950469c..f81e3db 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -480,7 +480,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 		intel_crtc = to_intel_crtc(crtc);
 
-		if (!intel_crtc->active || !intel_crtc->plane_config.fb) {
+		if (!intel_crtc->active || !crtc->fb) {
 			DRM_DEBUG_KMS("pipe %c not active or no fb, skipping\n",
 				      pipe_name(intel_crtc->pipe));
 			continue;
@@ -490,7 +490,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
 			DRM_DEBUG_KMS("found possible fb from plane %c\n",
 				      pipe_name(intel_crtc->pipe));
 			plane_config = &intel_crtc->plane_config;
-			fb = plane_config->fb;
+			fb = to_intel_framebuffer(crtc->fb);
 			max_size = plane_config->size;
 		}
 	}
@@ -542,43 +542,15 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
 			      max_size, cur_size);
 	}
 
-	/* Free unused fbs */
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
-		struct intel_framebuffer *cur_fb;
-
-		intel_crtc = to_intel_crtc(crtc);
-		cur_fb = intel_crtc->plane_config.fb;
-
-		if (cur_fb && cur_fb != fb)
-			drm_framebuffer_unreference(&cur_fb->base);
-	}
-
 	if (!fb) {
 		DRM_DEBUG_KMS("BIOS fb not suitable for all pipes, not using\n");
 		goto out;
 	}
 
-	ifbdev->preferred_bpp = plane_config->fb->base.bits_per_pixel;
+	ifbdev->preferred_bpp = fb->base.bits_per_pixel;
 	ifbdev->fb = fb;
 
-	/* Assuming a single fb across all pipes here */
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
-		intel_crtc = to_intel_crtc(crtc);
-
-		if (!intel_crtc->active)
-			continue;
-
-		/*
-		 * This should only fail on the first one so we don't need
-		 * to cleanup any secondary crtc->fbs
-		 */
-		if (intel_pin_and_fence_fb_obj(dev, fb->obj, NULL))
-			goto out_unref_obj;
-
-		crtc->fb = &fb->base;
-		drm_gem_object_reference(&fb->obj->base);
-		drm_framebuffer_reference(&fb->base);
-	}
+	drm_framebuffer_reference(&ifbdev->fb->base);
 
 	/* Final pass to check if any active pipes don't have fbs */
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
@@ -596,8 +568,6 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
 	DRM_DEBUG_KMS("using BIOS fb for initial console\n");
 	return true;
 
-out_unref_obj:
-	drm_framebuffer_unreference(&fb->base);
 out:
 
 	return false;
-- 
1.8.4.2




More information about the Intel-gfx mailing list