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

Jesse Barnes
2014-03-07T16:58:07Z

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@ffwll.ch>
Signed-off-by: Jesse Barnes <jbarnes@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