public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/8] drm/i915: add plane_config fetching infrastructure v2
@ 2014-03-07 16:57 Jesse Barnes
  2014-03-07 16:57 ` [PATCH 2/8] drm/i915: get_plane_config for i9xx v13 Jesse Barnes
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Jesse Barnes @ 2014-03-07 16:57 UTC (permalink / raw)
  To: intel-gfx

Early at init time, we can try to read out the plane config structure
and try to preserve it if possible.

v2: alloc fb obj at init time after fetching plane config

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.h      |  3 ++
 drivers/gpu/drm/i915/intel_display.c | 92 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  9 ++++
 3 files changed, 104 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 29da39f..c9aad90 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -405,6 +405,7 @@ struct drm_i915_error_state {
 
 struct intel_connector;
 struct intel_crtc_config;
+struct intel_plane_config;
 struct intel_crtc;
 struct intel_limit;
 struct dpll;
@@ -443,6 +444,8 @@ struct drm_i915_display_funcs {
 	 * fills out the pipe-config with the hw state. */
 	bool (*get_pipe_config)(struct intel_crtc *,
 				struct intel_crtc_config *);
+	void (*get_plane_config)(struct intel_crtc *,
+				 struct intel_plane_config *);
 	int (*crtc_mode_set)(struct drm_crtc *crtc,
 			     int x, int y,
 			     struct drm_framebuffer *old_fb);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7e8bfd8..e793c2a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2047,6 +2047,70 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y,
 	}
 }
 
+int intel_format_to_fourcc(int format)
+{
+	switch (format) {
+	case DISPPLANE_8BPP:
+		return DRM_FORMAT_C8;
+	case DISPPLANE_BGRX555:
+		return DRM_FORMAT_XRGB1555;
+	case DISPPLANE_BGRX565:
+		return DRM_FORMAT_RGB565;
+	default:
+	case DISPPLANE_BGRX888:
+		return DRM_FORMAT_XRGB8888;
+	case DISPPLANE_RGBX888:
+		return DRM_FORMAT_XBGR8888;
+	case DISPPLANE_BGRX101010:
+		return DRM_FORMAT_XRGB2101010;
+	case DISPPLANE_RGBX101010:
+		return DRM_FORMAT_XBGR2101010;
+	}
+}
+
+static void intel_alloc_plane_obj(struct intel_crtc *crtc,
+				  struct intel_plane_config *plane_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_gem_object *obj = NULL;
+	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;
+
+	if (plane_config->tiled) {
+		obj->tiling_mode = I915_TILING_X;
+		obj->stride = plane_config->fb->base.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];
+
+	mutex_lock(&dev->struct_mutex);
+
+	if (intel_framebuffer_init(dev, plane_config->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;
+
+out_unref_obj:
+	drm_gem_object_unreference(&obj->base);
+	mutex_unlock(&dev->struct_mutex);
+
+}
+
 static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 			     int x, int y)
 {
@@ -10997,6 +11061,7 @@ void intel_modeset_init(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int sprite, ret;
 	enum pipe pipe;
+	struct intel_crtc *crtc;
 
 	drm_mode_config_init(dev);
 
@@ -11059,6 +11124,33 @@ void intel_modeset_init(struct drm_device *dev)
 	mutex_lock(&dev->mode_config.mutex);
 	intel_modeset_setup_hw_state(dev, false);
 	mutex_unlock(&dev->mode_config.mutex);
+
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
+			    base.head) {
+		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
+		 * can even allow for smooth boot transitions if the BIOS
+		 * fb is large enough for the active pipe configuration.
+		 */
+		if (dev_priv->display.get_plane_config) {
+			dev_priv->display.get_plane_config(crtc,
+							   &crtc->plane_config);
+			/*
+			 * If the fb is shared between multiple heads, we'll
+			 * just get the first one.
+			 */
+			intel_alloc_plane_obj(crtc, &crtc->plane_config);
+		}
+#endif
+	}
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6042797..95dd15d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -218,6 +218,13 @@ typedef struct dpll {
 	int	p;
 } intel_clock_t;
 
+struct intel_plane_config {
+	struct intel_framebuffer *fb; /* ends up managed by intel_fbdev.c */
+	bool tiled;
+	int size;
+	u32 base;
+};
+
 struct intel_crtc_config {
 	/**
 	 * quirks - bitfield with hw state readout quirks
@@ -366,6 +373,7 @@ struct intel_crtc {
 	int16_t cursor_width, cursor_height;
 	bool cursor_visible;
 
+	struct intel_plane_config plane_config;
 	struct intel_crtc_config config;
 	struct intel_crtc_config *new_config;
 	bool new_enabled;
@@ -736,6 +744,7 @@ void intel_display_set_init_power(struct drm_i915_private *dev, bool enable);
 int valleyview_get_vco(struct drm_i915_private *dev_priv);
 void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 				 struct intel_crtc_config *pipe_config);
+int intel_format_to_fourcc(int format);
 
 /* intel_dp.c */
 void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/8] drm/i915: get_plane_config for i9xx v13
  2014-03-07 16:57 [PATCH 1/8] drm/i915: add plane_config fetching infrastructure v2 Jesse Barnes
@ 2014-03-07 16:57 ` Jesse Barnes
  2014-03-07 16:57 ` [PATCH 3/8] drm/i915: get_plane_config support for ILK+ v3 Jesse Barnes
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Jesse Barnes @ 2014-03-07 16:57 UTC (permalink / raw)
  To: intel-gfx

Read out the current plane configuration at init time into a new
plane_config structure.  This allows us to track any existing
framebuffers attached to the plane and potentially re-use them in our
fbdev code for a smooth handoff.

v2: update for new pitch_for_width function (Jesse)
    comment how get_plane_config works with shared fbs (Jesse)
v3: s/ARGB/XRGB (Ville)
    use pipesrc width/height (Ville)
    fix fourcc comment (Bob)
    use drm_format_plane_cpp (Ville)
v4: use fb for tracking fb data object (Ville)
v5: fix up gen2 pitch limits (Ville)
v6: read out stride as well (Daniel)
v7: split out init ordering changes (Daniel)
    don't fetch config if !CONFIG_FB
v8: use proper height in get_plane_config (Chris)
v9: fix CONFIG_FB check for modular configs (Jani)
v10: add comment about stolen allocation stomping
v11: drop hw state readout hunk (Daniel)
v12: handle tiled BIOS fbs (Kristian)
     pull out common bits (Jesse)
v13: move fb obj alloc out to _init

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 63 ++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e793c2a..da7bac5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5627,6 +5627,67 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
 	pipe_config->port_clock = clock.dot / 5;
 }
 
+static void i9xx_get_plane_config(struct intel_crtc *crtc,
+				  struct intel_plane_config *plane_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val, base, offset;
+	int pipe = crtc->pipe, plane = crtc->plane;
+	int fourcc, pixel_format;
+	int aligned_height;
+
+	plane_config->fb = kzalloc(sizeof(*plane_config->fb), GFP_KERNEL);
+	if (!plane_config->fb) {
+		DRM_DEBUG_KMS("failed to alloc fb\n");
+		return;
+	}
+
+	val = I915_READ(DSPCNTR(plane));
+
+	if (INTEL_INFO(dev)->gen >= 4)
+		if (val & DISPPLANE_TILED)
+			plane_config->tiled = true;
+
+	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 =
+		drm_format_plane_cpp(fourcc, 0) * 8;
+
+	if (INTEL_INFO(dev)->gen >= 4) {
+		if (plane_config->tiled)
+			offset = I915_READ(DSPTILEOFF(plane));
+		else
+			offset = I915_READ(DSPLINOFF(plane));
+		base = I915_READ(DSPSURF(plane)) & 0xfffff000;
+	} else {
+		base = I915_READ(DSPADDR(plane));
+	}
+	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;
+
+	val = I915_READ(DSPSTRIDE(pipe));
+	plane_config->fb->base.pitches[0] = val & 0xffffff80;
+
+	aligned_height = intel_align_height(dev, plane_config->fb->base.height,
+					    plane_config->tiled);
+
+	plane_config->size = ALIGN(plane_config->fb->base.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],
+		      plane_config->size);
+
+}
+
 static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 				 struct intel_crtc_config *pipe_config)
 {
@@ -10792,6 +10853,7 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else if (IS_VALLEYVIEW(dev)) {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
+		dev_priv->display.get_plane_config = i9xx_get_plane_config;
 		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
 		dev_priv->display.crtc_enable = valleyview_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
@@ -10799,6 +10861,7 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = i9xx_update_plane;
 	} else {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
+		dev_priv->display.get_plane_config = i9xx_get_plane_config;
 		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
 		dev_priv->display.crtc_enable = i9xx_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/8] drm/i915: get_plane_config support for ILK+ v3
  2014-03-07 16:57 [PATCH 1/8] drm/i915: add plane_config fetching infrastructure v2 Jesse Barnes
  2014-03-07 16:57 ` [PATCH 2/8] drm/i915: get_plane_config for i9xx v13 Jesse Barnes
@ 2014-03-07 16:57 ` Jesse Barnes
  2014-03-07 16:57 ` [PATCH 4/8] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon v12 Jesse Barnes
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Jesse Barnes @ 2014-03-07 16:57 UTC (permalink / raw)
  To: intel-gfx

This should allow BIOS fb inheritance to work on ILK+ machines too.

v2: handle tiled BIOS fbs (Kristian)
    split out common bits (Jesse)
v3: alloc fb obj out in _init

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 62 ++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index da7bac5..b933a92 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6631,6 +6631,66 @@ static void ironlake_get_pfit_config(struct intel_crtc *crtc,
 	}
 }
 
+static void ironlake_get_plane_config(struct intel_crtc *crtc,
+				      struct intel_plane_config *plane_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val, base, offset;
+	int pipe = crtc->pipe, plane = crtc->plane;
+	int fourcc, pixel_format;
+	int aligned_height;
+
+	plane_config->fb = kzalloc(sizeof(*plane_config->fb), GFP_KERNEL);
+	if (!plane_config->fb) {
+		DRM_DEBUG_KMS("failed to alloc fb\n");
+		return;
+	}
+
+	val = I915_READ(DSPCNTR(plane));
+
+	if (INTEL_INFO(dev)->gen >= 4)
+		if (val & DISPPLANE_TILED)
+			plane_config->tiled = true;
+
+	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 =
+		drm_format_plane_cpp(fourcc, 0) * 8;
+
+	base = I915_READ(DSPSURF(plane)) & 0xfffff000;
+	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
+		offset = I915_READ(DSPOFFSET(plane));
+	} else {
+		if (plane_config->tiled)
+			offset = I915_READ(DSPTILEOFF(plane));
+		else
+			offset = I915_READ(DSPLINOFF(plane));
+	}
+	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;
+
+	val = I915_READ(DSPSTRIDE(pipe));
+	plane_config->fb->base.pitches[0] = val & 0xffffff80;
+
+	aligned_height = intel_align_height(dev, plane_config->fb->base.height,
+					    plane_config->tiled);
+
+	plane_config->size = ALIGN(plane_config->fb->base.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],
+		      plane_config->size);
+}
+
 static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 				     struct intel_crtc_config *pipe_config)
 {
@@ -10839,6 +10899,7 @@ static void intel_init_display(struct drm_device *dev)
 
 	if (HAS_DDI(dev)) {
 		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
+		dev_priv->display.get_plane_config = ironlake_get_plane_config;
 		dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
 		dev_priv->display.crtc_enable = haswell_crtc_enable;
 		dev_priv->display.crtc_disable = haswell_crtc_disable;
@@ -10846,6 +10907,7 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
+		dev_priv->display.get_plane_config = ironlake_get_plane_config;
 		dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
 		dev_priv->display.crtc_enable = ironlake_crtc_enable;
 		dev_priv->display.crtc_disable = ironlake_crtc_disable;
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/8] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon v12
  2014-03-07 16:57 [PATCH 1/8] drm/i915: add plane_config fetching infrastructure v2 Jesse Barnes
  2014-03-07 16:57 ` [PATCH 2/8] drm/i915: get_plane_config for i9xx v13 Jesse Barnes
  2014-03-07 16:57 ` [PATCH 3/8] drm/i915: get_plane_config support for ILK+ v3 Jesse Barnes
@ 2014-03-07 16:57 ` Jesse Barnes
  2014-03-07 16:57 ` [PATCH 5/8] drm/i915: preserve SSC if previously set Jesse Barnes
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Jesse Barnes @ 2014-03-07 16:57 UTC (permalink / raw)
  To: intel-gfx

Retrieve current framebuffer config info from the regs and create an fb
object for the buffer the BIOS or boot loader left us.  This should
allow for smooth transitions to userspace apps once we finish the
initial configuration construction.

v2: check for non-native modes and adjust (Jesse)
    fixup aperture and cmap frees (Imre)
    use unlocked unref if init_bios fails (Jesse)
    fix curly brace around DSPADDR check (Imre)
    comment failure path for pin_and_fence (Imre)
v3: fixup fixup of aperture frees (Chris)
v4: update to current bits (locking & pin_and_fence hack) (Jesse)
v5: move fb config fetch to display code (Jesse)
    re-order hw state readout on initial load to suit fb inherit (Jesse)
    re-add pin_and_fence in fbdev code to make sure we refcount properly (Je
v6: rename to plane_config (Daniel)
    check for valid object when initializing BIOS fb (Jesse)
    split from plane_config readout and other display changes (Jesse)
    drop use_bios_fb option (Chris)
    update comments (Jesse)
    rework fbdev_init_bios for clarity (Jesse)
    drop fb obj ref under lock (Chris)
v7: use fb object from plane_config instead (Ville)
    take ref on fb object (Jesse)
v8: put under i915_fastboot option (Jesse)
    fix fb ptr checking (Jesse)
    inform drm_fb_helper if we fail to enable a connector (Jesse)
    drop unnecessary enabled[] modifications in failure cases (Chris)
    split from BIOS connector config readout (Daniel)
    don't memset the fb buffer if preallocated (Chris)
    alloc ifbdev up front and pass to init_bios (Chris)
    check for bad ifbdev in restore_mode too (Chris)
v9: fix up !fastboot bpp setting (Jesse)
    fix up !fastboot helper alloc (Jesse)
    make sure BIOS fb is sufficient for biggest active pipe (Jesse)
v10:fix up size calculation for proposed fbs (Chris)
    go back to two pass pipe fb assignment (Chris)
    add warning for active pipes w/o fbs (Chris)
    clean up num_pipes checks in fbdev_init and fbdev_restore_mode (Chris)
    move i915.fastboot into fbdev_init (Chris)
v11:make BIOS connector config usage unconditional (Daniel)
v12:fix up fb vs pipe size checking (Chris)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c |   1 -
 drivers/gpu/drm/i915/intel_drv.h     |   1 +
 drivers/gpu/drm/i915/intel_fbdev.c   | 174 +++++++++++++++++++++++++++++++++--
 3 files changed, 166 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b933a92..479de3b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2108,7 +2108,6 @@ static void intel_alloc_plane_obj(struct intel_crtc *crtc,
 out_unref_obj:
 	drm_gem_object_unreference(&obj->base);
 	mutex_unlock(&dev->struct_mutex);
-
 }
 
 static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 95dd15d..3d404ab 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -113,6 +113,7 @@ struct intel_fbdev {
 	struct intel_framebuffer *fb;
 	struct list_head fbdev_list;
 	struct drm_display_mode *our_mode;
+	int preferred_bpp;
 };
 
 struct intel_encoder {
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 6b5beed..32a05ed 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -128,6 +128,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	struct drm_framebuffer *fb;
 	struct drm_i915_gem_object *obj;
 	int size, ret;
+	bool prealloc = false;
 
 	mutex_lock(&dev->struct_mutex);
 
@@ -139,6 +140,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 		intel_fb = ifbdev->fb;
 	} else {
 		DRM_DEBUG_KMS("re-using BIOS fb\n");
+		prealloc = true;
 		sizes->fb_width = intel_fb->base.width;
 		sizes->fb_height = intel_fb->base.height;
 	}
@@ -200,7 +202,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	 * If the object is stolen however, it will be full of whatever
 	 * garbage was left in there.
 	 */
-	if (ifbdev->fb->obj->stolen)
+	if (ifbdev->fb->obj->stolen && !prealloc)
 		memset_io(info->screen_base, 0, info->screen_size);
 
 	/* Use default scratch pixmap (info->pixmap.flags = FB_PIXMAP_SYSTEM) */
@@ -454,27 +456,179 @@ static void intel_fbdev_destroy(struct drm_device *dev,
 	drm_framebuffer_remove(&ifbdev->fb->base);
 }
 
+/*
+ * Build an intel_fbdev struct using a BIOS allocated framebuffer, if possible.
+ * The core display code will have read out the current plane configuration,
+ * so we use that to figure out if there's an object for us to use as the
+ * fb, and if so, we re-use it for the fbdev configuration.
+ *
+ * Note we only support a single fb shared across pipes for boot (mostly for
+ * fbcon), so we just find the biggest and use that.
+ */
+static bool intel_fbdev_init_bios(struct drm_device *dev,
+				 struct intel_fbdev *ifbdev)
+{
+	struct intel_framebuffer *fb = NULL;
+	struct drm_crtc *crtc;
+	struct intel_crtc *intel_crtc;
+	struct intel_plane_config *plane_config = NULL;
+	unsigned int max_size = 0;
+
+	if (!i915.fastboot)
+		return false;
+
+	/* Find the largest fb */
+	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) {
+			DRM_DEBUG_KMS("pipe %c not active or no fb, skipping\n",
+				      pipe_name(intel_crtc->pipe));
+			continue;
+		}
+
+		if (intel_crtc->plane_config.size > max_size) {
+			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;
+			max_size = plane_config->size;
+		}
+	}
+
+	if (!fb) {
+		DRM_DEBUG_KMS("no active fbs found, not using BIOS config\n");
+		goto out;
+	}
+
+	/* Now make sure all the pipes will fit into it */
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		unsigned int cur_size;
+
+		intel_crtc = to_intel_crtc(crtc);
+
+		if (!intel_crtc->active) {
+			DRM_DEBUG_KMS("pipe %c not active, skipping\n",
+				      pipe_name(intel_crtc->pipe));
+			continue;
+		}
+
+		DRM_DEBUG_KMS("checking plane %c for BIOS fb\n",
+			      pipe_name(intel_crtc->pipe));
+
+		/*
+		 * See if the plane fb we found above will fit on this
+		 * pipe.  Note we need to use the selected fb's bpp rather
+		 * than the current pipe's, since they could be different.
+		 */
+		cur_size = intel_crtc->config.adjusted_mode.crtc_hdisplay *
+			intel_crtc->config.adjusted_mode.crtc_vdisplay;
+		DRM_DEBUG_KMS("pipe %c area: %d\n", pipe_name(intel_crtc->pipe),
+			      cur_size);
+		cur_size *= fb->base.bits_per_pixel / 8;
+		DRM_DEBUG_KMS("total size %d (bpp %d)\n", cur_size,
+			      fb->base.bits_per_pixel / 8);
+
+		if (cur_size > max_size) {
+			DRM_DEBUG_KMS("fb not big enough for plane %c (%d vs %d)\n",
+				      pipe_name(intel_crtc->pipe),
+				      cur_size, max_size);
+			plane_config = NULL;
+			fb = NULL;
+			break;
+		}
+
+		DRM_DEBUG_KMS("fb big enough for plane %c (%d >= %d)\n",
+			      pipe_name(intel_crtc->pipe),
+			      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->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);
+	}
+
+	/* Final pass to check if any active pipes don't have fbs */
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		intel_crtc = to_intel_crtc(crtc);
+
+		if (!intel_crtc->active)
+			continue;
+
+		WARN(!crtc->fb,
+		     "re-used BIOS config but lost an fb on crtc %d\n",
+		     crtc->base.id);
+	}
+
+
+	DRM_DEBUG_KMS("using BIOS fb for initial console\n");
+	return true;
+
+out_unref_obj:
+	drm_framebuffer_unreference(&fb->base);
+out:
+
+	return false;
+}
+
 int intel_fbdev_init(struct drm_device *dev)
 {
 	struct intel_fbdev *ifbdev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
-	ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
-	if (!ifbdev)
+	if (WARN_ON(INTEL_INFO(dev)->num_pipes == 0))
+		return -ENODEV;
+
+	ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL);
+	if (ifbdev == NULL)
 		return -ENOMEM;
 
-	dev_priv->fbdev = ifbdev;
 	ifbdev->helper.funcs = &intel_fb_helper_funcs;
+	if (!intel_fbdev_init_bios(dev, ifbdev))
+		ifbdev->preferred_bpp = 32;
 
 	ret = drm_fb_helper_init(dev, &ifbdev->helper,
-				 INTEL_INFO(dev)->num_pipes,
-				 4);
+				 INTEL_INFO(dev)->num_pipes, 4);
 	if (ret) {
 		kfree(ifbdev);
 		return ret;
 	}
 
+	dev_priv->fbdev = ifbdev;
 	drm_fb_helper_single_add_all_connectors(&ifbdev->helper);
 
 	return 0;
@@ -483,9 +637,10 @@ int intel_fbdev_init(struct drm_device *dev)
 void intel_fbdev_initial_config(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_fbdev *ifbdev = dev_priv->fbdev;
 
 	/* Due to peculiar init order wrt to hpd handling this is separate. */
-	drm_fb_helper_initial_config(&dev_priv->fbdev->helper, 32);
+	drm_fb_helper_initial_config(&ifbdev->helper, ifbdev->preferred_bpp);
 }
 
 void intel_fbdev_fini(struct drm_device *dev)
@@ -523,7 +678,8 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state)
 void intel_fbdev_output_poll_changed(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
+	if (dev_priv->fbdev)
+		drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
 }
 
 void intel_fbdev_restore_mode(struct drm_device *dev)
@@ -531,7 +687,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
 	int ret;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (INTEL_INFO(dev)->num_pipes == 0)
+	if (!dev_priv->fbdev)
 		return;
 
 	drm_modeset_lock_all(dev);
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/8] drm/i915: preserve SSC if previously set
  2014-03-07 16:57 [PATCH 1/8] drm/i915: add plane_config fetching infrastructure v2 Jesse Barnes
                   ` (2 preceding siblings ...)
  2014-03-07 16:57 ` [PATCH 4/8] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon v12 Jesse Barnes
@ 2014-03-07 16:57 ` Jesse Barnes
  2014-03-08 10:34   ` Daniel Vetter
  2014-03-07 16:57 ` [PATCH 6/8] drm/i915: don't bother enabling swizzle bits on gen7+ Jesse Barnes
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Jesse Barnes @ 2014-03-07 16:57 UTC (permalink / raw)
  To: intel-gfx

Some machines may have a broken VBT or no VBT at all, but we still want
to use SSC there.  So check for it and keep it enabled if we see it
already on.  Based on an earlier fix from Kristian.

Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 479de3b..d450ab6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5806,6 +5806,10 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
 	 */
 	val = I915_READ(PCH_DREF_CONTROL);
 
+	/* Preserve SSC if the BIOS set it */
+	if (val & DREF_SSC1_ENABLE)
+		i915.panel_use_ssc = 1;
+
 	/* As we must carefully and slowly disable/enable each source in turn,
 	 * compute the final state we want first and check if we need to
 	 * make any changes at all.
-- 
1.8.4.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/8] drm/i915: don't bother enabling swizzle bits on gen7+
  2014-03-07 16:57 [PATCH 1/8] drm/i915: add plane_config fetching infrastructure v2 Jesse Barnes
                   ` (3 preceding siblings ...)
  2014-03-07 16:57 ` [PATCH 5/8] drm/i915: preserve SSC if previously set Jesse Barnes
@ 2014-03-07 16:57 ` Jesse Barnes
  2014-03-08 10:36   ` Daniel Vetter
  2014-03-07 16:57 ` [PATCH 7/8] drm/i915: use current mode if the size matches the preferred mode Jesse Barnes
  2014-03-07 16:57 ` [PATCH 8/8] drm/i915: remove early fb allocation dependency on CONFIG_FB v2 Jesse Barnes
  6 siblings, 1 reply; 14+ messages in thread
From: Jesse Barnes @ 2014-03-07 16:57 UTC (permalink / raw)
  To: intel-gfx

As of IVB, the memory controller does internal swizzling already, so we
shouldn't need to enable these.  Based on an earlier fix from Kristian.

Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_gem.c        | 7 +++----
 drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 18ea6bc..dcf4b01 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4296,6 +4296,9 @@ void i915_gem_init_swizzling(struct drm_device *dev)
 	    dev_priv->mm.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE)
 		return;
 
+	if (INTEL_INFO(dev)->gen >= 7)
+		return;
+
 	I915_WRITE(DISP_ARB_CTL, I915_READ(DISP_ARB_CTL) |
 				 DISP_TILE_SURFACE_SWIZZLING);
 
@@ -4305,10 +4308,6 @@ void i915_gem_init_swizzling(struct drm_device *dev)
 	I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_SWZCTL);
 	if (IS_GEN6(dev))
 		I915_WRITE(ARB_MODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_SNB));
-	else if (IS_GEN7(dev))
-		I915_WRITE(ARB_MODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_IVB));
-	else if (IS_GEN8(dev))
-		I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_BDW));
 	else
 		BUG();
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index eb99358..05c5d98 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -91,7 +91,7 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
 	uint32_t swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN;
 	uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN;
 
-	if (IS_VALLEYVIEW(dev)) {
+	if (INTEL_INFO(dev)->gen >= 7) {
 		swizzle_x = I915_BIT_6_SWIZZLE_NONE;
 		swizzle_y = I915_BIT_6_SWIZZLE_NONE;
 	} else if (INTEL_INFO(dev)->gen >= 6) {
-- 
1.8.4.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 7/8] drm/i915: use current mode if the size matches the preferred mode
  2014-03-07 16:57 [PATCH 1/8] drm/i915: add plane_config fetching infrastructure v2 Jesse Barnes
                   ` (4 preceding siblings ...)
  2014-03-07 16:57 ` [PATCH 6/8] drm/i915: don't bother enabling swizzle bits on gen7+ Jesse Barnes
@ 2014-03-07 16:57 ` Jesse Barnes
  2014-03-07 16:57 ` [PATCH 8/8] drm/i915: remove early fb allocation dependency on CONFIG_FB v2 Jesse Barnes
  6 siblings, 0 replies; 14+ messages in thread
From: Jesse Barnes @ 2014-03-07 16:57 UTC (permalink / raw)
  To: intel-gfx

From: Kristian Høgsberg <hoegsberg@gmail.com>

The BIOS may set a native mode that doesn't quite match the preferred
mode timings.  It should be ok to use however if it uses the same size,
so try to avoid a mode set in that case.

Signed-off-by: Kristian Høgsberg <hoegsberg@gmail.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/drm_modes.c        |  8 ++++++++
 drivers/gpu/drm/i915/intel_fbdev.c | 37 ++++++++++++++++++-------------------
 include/drm/drm_crtc.h             |  2 ++
 3 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index b073315..7d2dda4 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -894,6 +894,14 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
 }
 EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo);
 
+bool drm_mode_same_size(const struct drm_display_mode *mode1,
+			const struct drm_display_mode *mode2)
+{
+	return mode1->vdisplay == mode2->vdisplay &&
+		mode1->hdisplay == mode2->hdisplay;
+}
+EXPORT_SYMBOL(drm_mode_same_size);
+
 /**
  * drm_mode_validate_size - make sure modes adhere to size constraints
  * @dev: DRM device
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 32a05ed..950469c 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -369,31 +369,30 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 		/* go for command line mode first */
 		modes[i] = drm_pick_cmdline_mode(fb_conn, width, height);
 
-		/* try for preferred next */
+		/* try for preferred next or match current */
 		if (!modes[i]) {
+			struct drm_display_mode *preferred;
+
 			DRM_DEBUG_KMS("looking for preferred mode on connector %d\n",
 				      fb_conn->connector->base.id);
-			modes[i] = drm_has_preferred_mode(fb_conn, width,
-							  height);
-		}
-
-		/* last resort: use current mode */
-		if (!modes[i]) {
-			/*
-			 * IMPORTANT: We want to use the adjusted mode (i.e.
-			 * after the panel fitter upscaling) as the initial
-			 * config, not the input mode, which is what crtc->mode
-			 * usually contains. But since our current fastboot
-			 * code puts a mode derived from the post-pfit timings
-			 * into crtc->mode this works out correctly. We don't
-			 * use hwmode anywhere right now, so use it for this
-			 * since the fb helper layer wants a pointer to
-			 * something we own.
-			 */
+			preferred = drm_has_preferred_mode(fb_conn, width,
+							   height);
 			intel_mode_from_pipe_config(&encoder->crtc->hwmode,
 						    &to_intel_crtc(encoder->crtc)->config);
-			modes[i] = &encoder->crtc->hwmode;
+ 			modes[i] = &encoder->crtc->hwmode;
+
+			if (preferred &&
+			    !drm_mode_same_size(preferred, modes[i])) {
+				DRM_DEBUG_KMS("using preferred mode %s "
+					      "instead of current mode %s "
+					      "on connector %d\n",
+					      preferred->name,
+					      modes[i]->name,
+					      fb_conn->connector->base.id);
+				modes[i] = preferred;
+			}
 		}
+
 		crtcs[i] = new_crtc;
 
 		DRM_DEBUG_KMS("connector %s on crtc %d: %s\n",
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index f764654..d5ebe3b 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1020,6 +1020,8 @@ extern bool drm_mode_equal(const struct drm_display_mode *mode1, const struct dr
 extern bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2);
 extern int drm_mode_width(const struct drm_display_mode *mode);
 extern int drm_mode_height(const struct drm_display_mode *mode);
+extern bool drm_mode_same_size(const struct drm_display_mode *mode1,
+			       const struct drm_display_mode *mode2);
 
 /* for us by fb module */
 extern struct drm_display_mode *drm_mode_create(struct drm_device *dev);
-- 
1.8.4.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 8/8] drm/i915: remove early fb allocation dependency on CONFIG_FB v2
  2014-03-07 16:57 [PATCH 1/8] drm/i915: add plane_config fetching infrastructure v2 Jesse Barnes
                   ` (5 preceding siblings ...)
  2014-03-07 16:57 ` [PATCH 7/8] drm/i915: use current mode if the size matches the preferred mode Jesse Barnes
@ 2014-03-07 16:57 ` Jesse Barnes
  2014-03-08 10:33   ` Daniel Vetter
  6 siblings, 1 reply; 14+ messages in thread
From: Jesse Barnes @ 2014-03-07 16:57 UTC (permalink / raw)
  To: intel-gfx

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/8] drm/i915: remove early fb allocation dependency on CONFIG_FB v2
  2014-03-07 16:57 ` [PATCH 8/8] drm/i915: remove early fb allocation dependency on CONFIG_FB v2 Jesse Barnes
@ 2014-03-08 10:33   ` Daniel Vetter
  2014-03-09 17:33     ` Jesse Barnes
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Vetter @ 2014-03-08 10:33 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

On Fri, Mar 07, 2014 at 08:57:55AM -0800, Jesse Barnes wrote:
> 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>

Ok, I've merged patches 1-4 and this one here from the series. Not
terribly happy that you didn't squash in this fixup, since now people
might stumble over the strange lifetime rules of the previous patches. But
meh ;-)

Thanks, Daniel
> ---
>  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
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/8] drm/i915: preserve SSC if previously set
  2014-03-07 16:57 ` [PATCH 5/8] drm/i915: preserve SSC if previously set Jesse Barnes
@ 2014-03-08 10:34   ` Daniel Vetter
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2014-03-08 10:34 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

On Fri, Mar 07, 2014 at 08:57:52AM -0800, Jesse Barnes wrote:
> Some machines may have a broken VBT or no VBT at all, but we still want
> to use SSC there.  So check for it and keep it enabled if we see it
> already on.  Based on an earlier fix from Kristian.
> 
> Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 479de3b..d450ab6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5806,6 +5806,10 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
>  	 */
>  	val = I915_READ(PCH_DREF_CONTROL);
>  
> +	/* Preserve SSC if the BIOS set it */
> +	if (val & DREF_SSC1_ENABLE)
> +		i915.panel_use_ssc = 1;

This definitely needs some debug output, and we need to make sure that the
module option still overrides this.
-Daniel

> +
>  	/* As we must carefully and slowly disable/enable each source in turn,
>  	 * compute the final state we want first and check if we need to
>  	 * make any changes at all.
> -- 
> 1.8.4.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/8] drm/i915: don't bother enabling swizzle bits on gen7+
  2014-03-07 16:57 ` [PATCH 6/8] drm/i915: don't bother enabling swizzle bits on gen7+ Jesse Barnes
@ 2014-03-08 10:36   ` Daniel Vetter
  2014-03-09 17:33     ` Jesse Barnes
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Vetter @ 2014-03-08 10:36 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

On Fri, Mar 07, 2014 at 08:57:53AM -0800, Jesse Barnes wrote:
> As of IVB, the memory controller does internal swizzling already, so we
> shouldn't need to enable these.  Based on an earlier fix from Kristian.
> 
> Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Imo the right approach here is to check whether any of the
preserved/inherited framebuffers has tiling enabled, and if so we need to
preserve the swizzling mode the bios has set.

Also this should be done on gen6+ since those are the machines where
swizzling can be changed.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c        | 7 +++----
>  drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +-
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 18ea6bc..dcf4b01 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4296,6 +4296,9 @@ void i915_gem_init_swizzling(struct drm_device *dev)
>  	    dev_priv->mm.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE)
>  		return;
>  
> +	if (INTEL_INFO(dev)->gen >= 7)
> +		return;
> +
>  	I915_WRITE(DISP_ARB_CTL, I915_READ(DISP_ARB_CTL) |
>  				 DISP_TILE_SURFACE_SWIZZLING);
>  
> @@ -4305,10 +4308,6 @@ void i915_gem_init_swizzling(struct drm_device *dev)
>  	I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_SWZCTL);
>  	if (IS_GEN6(dev))
>  		I915_WRITE(ARB_MODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_SNB));
> -	else if (IS_GEN7(dev))
> -		I915_WRITE(ARB_MODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_IVB));
> -	else if (IS_GEN8(dev))
> -		I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_BDW));
>  	else
>  		BUG();
>  }
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index eb99358..05c5d98 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -91,7 +91,7 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
>  	uint32_t swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN;
>  	uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN;
>  
> -	if (IS_VALLEYVIEW(dev)) {
> +	if (INTEL_INFO(dev)->gen >= 7) {
>  		swizzle_x = I915_BIT_6_SWIZZLE_NONE;
>  		swizzle_y = I915_BIT_6_SWIZZLE_NONE;
>  	} else if (INTEL_INFO(dev)->gen >= 6) {
> -- 
> 1.8.4.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/8] drm/i915: remove early fb allocation dependency on CONFIG_FB v2
  2014-03-08 10:33   ` Daniel Vetter
@ 2014-03-09 17:33     ` Jesse Barnes
  2014-03-09 19:27       ` Daniel Vetter
  0 siblings, 1 reply; 14+ messages in thread
From: Jesse Barnes @ 2014-03-09 17:33 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Sat, 8 Mar 2014 11:33:15 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Fri, Mar 07, 2014 at 08:57:55AM -0800, Jesse Barnes wrote:
> > 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>
> 
> Ok, I've merged patches 1-4 and this one here from the series. Not
> terribly happy that you didn't squash in this fixup, since now people
> might stumble over the strange lifetime rules of the previous patches. But
> meh ;-)

Yeah even though there's a handoff from the core to the fbdev side, I
still think they're correct as far as fbs go.  The underlying obj ref
may not be correct though; I think that was papering over an earlier
bug.

Either way those should manifest as a leaked object (the stolen fb will
stick around forever) rather than a crash or something.  Whereas this
last patch is more likely to cause serious trouble I think since it's a
bit more invasive and relies on some other bits more...

Anyway thanks, looking forward to seeing the perf data on the swizzle
stuff.

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/8] drm/i915: don't bother enabling swizzle bits on gen7+
  2014-03-08 10:36   ` Daniel Vetter
@ 2014-03-09 17:33     ` Jesse Barnes
  0 siblings, 0 replies; 14+ messages in thread
From: Jesse Barnes @ 2014-03-09 17:33 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Sat, 8 Mar 2014 11:36:24 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Fri, Mar 07, 2014 at 08:57:53AM -0800, Jesse Barnes wrote:
> > As of IVB, the memory controller does internal swizzling already, so we
> > shouldn't need to enable these.  Based on an earlier fix from Kristian.
> > 
> > Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> Imo the right approach here is to check whether any of the
> preserved/inherited framebuffers has tiling enabled, and if so we need to
> preserve the swizzling mode the bios has set.
> 
> Also this should be done on gen6+ since those are the machines where
> swizzling can be changed.

Ah yeah good point... haven't checked to see if any BIOSes enable this
automatically.

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/8] drm/i915: remove early fb allocation dependency on CONFIG_FB v2
  2014-03-09 17:33     ` Jesse Barnes
@ 2014-03-09 19:27       ` Daniel Vetter
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2014-03-09 19:27 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

On Sun, Mar 9, 2014 at 6:33 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Sat, 8 Mar 2014 11:33:15 +0100
> Daniel Vetter <daniel@ffwll.ch> wrote:
>
>> On Fri, Mar 07, 2014 at 08:57:55AM -0800, Jesse Barnes wrote:
>> > 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>
>>
>> Ok, I've merged patches 1-4 and this one here from the series. Not
>> terribly happy that you didn't squash in this fixup, since now people
>> might stumble over the strange lifetime rules of the previous patches. But
>> meh ;-)
>
> Yeah even though there's a handoff from the core to the fbdev side, I
> still think they're correct as far as fbs go.  The underlying obj ref
> may not be correct though; I think that was papering over an earlier
> bug.
>
> Either way those should manifest as a leaked object (the stolen fb will
> stick around forever) rather than a crash or something.  Whereas this
> last patch is more likely to cause serious trouble I think since it's a
> bit more invasive and relies on some other bits more...

Yeah I agree that the refcounting before this patch was just a bit
leaky - otherwise I would have protested much louder ;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-03-09 19:27 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-07 16:57 [PATCH 1/8] drm/i915: add plane_config fetching infrastructure v2 Jesse Barnes
2014-03-07 16:57 ` [PATCH 2/8] drm/i915: get_plane_config for i9xx v13 Jesse Barnes
2014-03-07 16:57 ` [PATCH 3/8] drm/i915: get_plane_config support for ILK+ v3 Jesse Barnes
2014-03-07 16:57 ` [PATCH 4/8] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon v12 Jesse Barnes
2014-03-07 16:57 ` [PATCH 5/8] drm/i915: preserve SSC if previously set Jesse Barnes
2014-03-08 10:34   ` Daniel Vetter
2014-03-07 16:57 ` [PATCH 6/8] drm/i915: don't bother enabling swizzle bits on gen7+ Jesse Barnes
2014-03-08 10:36   ` Daniel Vetter
2014-03-09 17:33     ` Jesse Barnes
2014-03-07 16:57 ` [PATCH 7/8] drm/i915: use current mode if the size matches the preferred mode Jesse Barnes
2014-03-07 16:57 ` [PATCH 8/8] drm/i915: remove early fb allocation dependency on CONFIG_FB v2 Jesse Barnes
2014-03-08 10:33   ` Daniel Vetter
2014-03-09 17:33     ` Jesse Barnes
2014-03-09 19:27       ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox