[PATCH 1/6] drm/i915: read out hw state earlier v2

[PATCH 1/6] drm/i915: read out hw state earlier v2

[Jesse Barnes]

We want to do this early on before we try to fetch the plane config,
which depends on some of the pipe config state.

v2: split back out from get_plane_config change (Daniel)
    update for recent locking & reset changes (Jesse)

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0df974b..cecb2c9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11072,6 +11072,8 @@ void intel_modeset_init(struct drm_device *dev)
 
 	/* Just in case the BIOS is doing something questionable. */
 	intel_disable_fbc(dev);
+
+	intel_modeset_setup_hw_state(dev, false);
 }
 
 static void
@@ -11439,10 +11441,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
 	intel_modeset_init_hw(dev);
 
 	intel_setup_overlay(dev);
-
-	mutex_lock(&dev->mode_config.mutex);
-	intel_modeset_setup_hw_state(dev, false);
-	mutex_unlock(&dev->mode_config.mutex);
 }
 
 void intel_modeset_cleanup(struct drm_device *dev)
-- 
1.7.9.5

[PATCH 2/6] drm/i915: Pass explicit mode into mode_from_pipe_config v3

[Jesse Barnes]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

We want to reuse this in the fbdev initial config code independently
from any fastboot hacks. So allow a bit more flexibility.

v2: Forgot to git add ...
v3: make non-static (Jesse)

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |   31 ++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_drv.h     |    2 ++
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cecb2c9..e800085 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5263,25 +5263,23 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
 	pipe_config->requested_mode.hdisplay = pipe_config->pipe_src_w;
 }
 
-static void intel_crtc_mode_from_pipe_config(struct intel_crtc *intel_crtc,
-					     struct intel_crtc_config *pipe_config)
+void intel_mode_from_pipe_config(struct drm_display_mode *mode,
+				 struct intel_crtc_config *pipe_config)
 {
-	struct drm_crtc *crtc = &intel_crtc->base;
+	mode->hdisplay = pipe_config->adjusted_mode.crtc_hdisplay;
+	mode->htotal = pipe_config->adjusted_mode.crtc_htotal;
+	mode->hsync_start = pipe_config->adjusted_mode.crtc_hsync_start;
+	mode->hsync_end = pipe_config->adjusted_mode.crtc_hsync_end;
 
-	crtc->mode.hdisplay = pipe_config->adjusted_mode.crtc_hdisplay;
-	crtc->mode.htotal = pipe_config->adjusted_mode.crtc_htotal;
-	crtc->mode.hsync_start = pipe_config->adjusted_mode.crtc_hsync_start;
-	crtc->mode.hsync_end = pipe_config->adjusted_mode.crtc_hsync_end;
+	mode->vdisplay = pipe_config->adjusted_mode.crtc_vdisplay;
+	mode->vtotal = pipe_config->adjusted_mode.crtc_vtotal;
+	mode->vsync_start = pipe_config->adjusted_mode.crtc_vsync_start;
+	mode->vsync_end = pipe_config->adjusted_mode.crtc_vsync_end;
 
-	crtc->mode.vdisplay = pipe_config->adjusted_mode.crtc_vdisplay;
-	crtc->mode.vtotal = pipe_config->adjusted_mode.crtc_vtotal;
-	crtc->mode.vsync_start = pipe_config->adjusted_mode.crtc_vsync_start;
-	crtc->mode.vsync_end = pipe_config->adjusted_mode.crtc_vsync_end;
+	mode->flags = pipe_config->adjusted_mode.flags;
 
-	crtc->mode.flags = pipe_config->adjusted_mode.flags;
-
-	crtc->mode.clock = pipe_config->adjusted_mode.crtc_clock;
-	crtc->mode.flags |= pipe_config->adjusted_mode.flags;
+	mode->clock = pipe_config->adjusted_mode.crtc_clock;
+	mode->flags |= pipe_config->adjusted_mode.flags;
 }
 
 static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
@@ -11380,8 +11378,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
 			    base.head) {
 		if (crtc->active && i915.fastboot) {
-			intel_crtc_mode_from_pipe_config(crtc, &crtc->config);
-
+			intel_mode_from_pipe_config(&crtc->base.mode, &crtc->config);
 			DRM_DEBUG_KMS("[CRTC:%d] found active mode: ",
 				      crtc->base.base.id);
 			drm_mode_debug_printmodeline(&crtc->base.mode);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3599d93..bff5d0a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -726,6 +726,8 @@ void hsw_enable_ips(struct intel_crtc *crtc);
 void hsw_disable_ips(struct intel_crtc *crtc);
 void intel_display_set_init_power(struct drm_device *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);
 
 /* intel_dp.c */
 void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
-- 
1.7.9.5

[PATCH 3/6] drm/i915: allow re-use BIOS connector config for initial fbdev config v2

[Jesse Barnes]

The BIOS or boot loader will generally create an initial display
configuration for us that includes some set of active pipes and
displays.  This routine tries to figure out which pipes and connectors
are active and stuffs them into the crtcs and modes array given to us by
the drm_fb_helper code.

The overall sequence is:
  intel_fbdev_init - from driver load
    intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data
    drm_fb_helper_init - build fb helper structs
    drm_fb_helper_single_add_all_connectors - more fb helper structs
  intel_fbdev_initial_config - apply the config
    drm_fb_helper_initial_config - call ->probe then register_framebuffer()
        drm_setup_crtcs - build crtc config for fbdev
          intel_fb_initial_config - find active connectors etc
        drm_fb_helper_single_fb_probe - set up fbdev
          intelfb_create - re-use or alloc fb, build out fbdev structs

v2: use BIOS connector config unconditionally if possible (Daniel)
    check for crtc cloning and reject (Daniel)
    fix up comments (Daniel)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c |   10 ++--
 drivers/gpu/drm/i915/intel_fbdev.c   |  103 ++++++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e800085..dea995d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -51,7 +51,10 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
 
 static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
 			  int x, int y, struct drm_framebuffer *old_fb);
-
+static int intel_framebuffer_init(struct drm_device *dev,
+				  struct intel_framebuffer *ifb,
+				  struct drm_mode_fb_cmd2 *mode_cmd,
+				  struct drm_i915_gem_object *obj);
 
 typedef struct {
 	int	min, max;
@@ -7692,11 +7695,6 @@ static struct drm_display_mode load_detect_mode = {
 		 704, 832, 0, 480, 489, 491, 520, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
 };
 
-static int intel_framebuffer_init(struct drm_device *dev,
-				  struct intel_framebuffer *ifb,
-				  struct drm_mode_fb_cmd2 *mode_cmd,
-				  struct drm_i915_gem_object *obj);
-
 struct drm_framebuffer *
 __intel_framebuffer_create(struct drm_device *dev,
 			   struct drm_mode_fb_cmd2 *mode_cmd,
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index cf46273..2a0badfc 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -242,7 +242,110 @@ static void intel_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
 	*blue = intel_crtc->lut_b[regno] << 8;
 }
 
+static struct drm_fb_helper_crtc *
+intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
+{
+	int i;
+
+	for (i = 0; i < fb_helper->crtc_count; i++)
+		if (fb_helper->crtc_info[i].mode_set.crtc == crtc)
+			return &fb_helper->crtc_info[i];
+
+	return NULL;
+}
+
+/*
+ * Try to read the BIOS display configuration and use it for the initial
+ * fb configuration.
+ *
+ * The BIOS or boot loader will generally create an initial display
+ * configuration for us that includes some set of active pipes and displays.
+ * This routine tries to figure out which pipes and connectors are active
+ * and stuffs them into the crtcs and modes array given to us by the
+ * drm_fb_helper code.
+ *
+ * The overall sequence is:
+ *   intel_fbdev_init - from driver load
+ *     intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data
+ *     drm_fb_helper_init - build fb helper structs
+ *     drm_fb_helper_single_add_all_connectors - more fb helper structs
+ *   intel_fbdev_initial_config - apply the config
+ *     drm_fb_helper_initial_config - call ->probe then register_framebuffer()
+ *         drm_setup_crtcs - build crtc config for fbdev
+ *           intel_fb_initial_config - find active connectors etc
+ *         drm_fb_helper_single_fb_probe - set up fbdev
+ *           intelfb_create - re-use or alloc fb, build out fbdev structs
+ *
+ * Note that we don't make special consideration whether we could actually
+ * switch to the selected modes without a full modeset. E.g. when the display
+ * is in VGA mode we need to recalculate watermarks and set a new high-res
+ * framebuffer anyway.
+ */
+static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
+				    struct drm_fb_helper_crtc **crtcs,
+				    struct drm_display_mode **modes,
+				    bool *enabled, int width, int height)
+{
+	int i, j;
+
+	for (i = 0; i < fb_helper->connector_count; i++) {
+		struct drm_connector *connector;
+		struct drm_encoder *encoder;
+		struct drm_fb_helper_crtc *new_crtc;
+
+		connector = fb_helper->connector_info[i]->connector;
+		if (!enabled[i]) {
+			DRM_DEBUG_KMS("connector %d not enabled, skipping\n",
+				      connector->base.id);
+			continue;
+		}
+
+		encoder = connector->encoder;
+		if (!encoder || !encoder->crtc) {
+			DRM_DEBUG_KMS("connector %d has no encoder or crtc, skipping\n",
+				      connector->base.id);
+			enabled[i] = false;
+			continue;
+		}
+
+		new_crtc = intel_fb_helper_crtc(fb_helper, encoder->crtc);
+
+		/*
+		 * Make sure we're not trying to drive multiple connectors
+		 * with a single CRTC, since our cloning support may not
+		 * match the BIOS.
+		 */
+		for (j = 0; j < fb_helper->connector_count; j++) {
+			if (crtcs[j] == new_crtc)
+				return false;
+		}
+
+		/*
+		 * 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.
+		 */
+		intel_mode_from_pipe_config(&encoder->crtc->hwmode,
+					    &to_intel_crtc(encoder->crtc)->config);
+		modes[i] = &encoder->crtc->hwmode;
+		crtcs[i] = new_crtc;
+
+		DRM_DEBUG_KMS("connector %s on crtc %d: %s\n",
+			      drm_get_connector_name(connector),
+			      encoder->crtc->base.id,
+			      modes[i]->name);
+	}
+
+	return true;
+}
+
 static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
+	.initial_config = intel_fb_initial_config,
 	.gamma_set = intel_crtc_fb_gamma_set,
 	.gamma_get = intel_crtc_fb_gamma_get,
 	.fb_probe = intelfb_create,
-- 
1.7.9.5

[PATCH 4/6] drm/i915: get_plane_config for i9xx v11

[Jesse Barnes]

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)

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

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2572a95..294df4c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -397,6 +397,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;
@@ -435,6 +436,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 dea995d..425a6ae 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2047,6 +2047,27 @@ 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 int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 			     int x, int y)
 {
@@ -5517,6 +5538,96 @@ 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;
+	struct drm_i915_gem_object *obj = NULL;
+	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
+	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));
+	}
+
+	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);
+
+	/*
+	 * If the fb is shared between multiple heads, we'll just get the
+	 * first one.
+	 */
+	obj = i915_gem_object_create_stolen_for_preallocated(dev, base, base,
+							     plane_config->size);
+	if (!obj)
+		return;
+
+	mode_cmd.pixel_format = fourcc;
+	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 bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 				 struct intel_crtc_config *pipe_config)
 {
@@ -10742,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;
@@ -10749,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;
@@ -11009,6 +11122,7 @@ void intel_modeset_suspend_hw(struct drm_device *dev)
 void intel_modeset_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *crtc;
 	int i, j, ret;
 
 	drm_mode_config_init(dev);
@@ -11070,6 +11184,27 @@ void intel_modeset_init(struct drm_device *dev)
 	intel_disable_fbc(dev);
 
 	intel_modeset_setup_hw_state(dev, false);
+
+	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);
+#endif
+	}
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bff5d0a..abe576d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -210,6 +210,12 @@ 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;
+};
+
 struct intel_crtc_config {
 	/**
 	 * quirks - bitfield with hw state readout quirks
@@ -358,6 +364,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;
@@ -728,6 +735,7 @@ void intel_display_set_init_power(struct drm_device *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.7.9.5

[PATCH 5/6] drm/i915: get_plane_config support for ILK+

[Jesse Barnes]

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

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 425a6ae..a91b8c2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6571,6 +6571,96 @@ 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;
+	struct drm_i915_gem_object *obj = NULL;
+	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
+	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));
+	}
+
+	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);
+
+	/*
+	 * If the fb is shared between multiple heads, we'll just get the
+	 * first one.
+	 */
+	obj = i915_gem_object_create_stolen_for_preallocated(dev, base, base,
+							     plane_config->size);
+	if (!obj)
+		return;
+
+	mode_cmd.pixel_format = fourcc;
+	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 bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 				     struct intel_crtc_config *pipe_config)
 {
@@ -10839,6 +10929,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 +10937,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.7.9.5

[PATCH 6/6] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon v11

[Jesse Barnes]

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)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_drv.h   |    1 +
 drivers/gpu/drm/i915/intel_fbdev.c |  154 +++++++++++++++++++++++++++++++++---
 2 files changed, 146 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index abe576d..c3278d4 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 2a0badfc..0fbb5cd 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) */
@@ -371,27 +373,159 @@ static void intel_fbdev_destroy(struct drm_device *dev,
 	drm_framebuffer_unreference(&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 last_size = 0, max_size = 0;
+
+	if (!i915.fastboot)
+		return false;
+
+	/* Find how big the fb would need to be for the largest pipe */
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		unsigned int tmp, stride;
+
+		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;
+		}
+
+		stride = intel_crtc->plane_config.fb->base.pitches[0];
+		tmp = intel_crtc->config.adjusted_mode.crtc_vdisplay * stride;
+		max_size = max(max_size, tmp);
+	}
+
+	DRM_DEBUG_KMS("crtc size required in bytes: %d\n", max_size);
+
+	/* Find one that fits in the largest pipe, then free the others */
+	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;
+		}
+
+		DRM_DEBUG_KMS("checking plane %c for BIOS fb\n",
+			      pipe_name(intel_crtc->pipe));
+
+		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;
+			last_size = plane_config->size;
+			fb = plane_config->fb;
+		} else {
+			DRM_DEBUG_KMS("plane %c fb not big enough (%d vs %d)\n",
+				      pipe_name(intel_crtc->pipe),
+				      intel_crtc->plane_config.size,
+				      max_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("no active pipes found, not using BIOS config\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;
@@ -400,9 +534,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)
@@ -440,7 +575,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)
@@ -448,7 +584,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.7.9.5

Re: [PATCH 1/6] drm/i915: read out hw state earlier v2

[Daniel Vetter]

On Tue, Feb 11, 2014 at 03:28:56PM -0800, Jesse Barnes wrote:
> We want to do this early on before we try to fetch the plane config,
> which depends on some of the pipe config state.

I think the crucial step here is actually the gem setup, more specifically
the stolen setup. If we don't do the fb reconstruction before that we
can't pre-reserve the space any more. I'll amend the commit message.
-Daniel

> 
> v2: split back out from get_plane_config change (Daniel)
>     update for recent locking & reset changes (Jesse)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0df974b..cecb2c9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11072,6 +11072,8 @@ void intel_modeset_init(struct drm_device *dev)
>  
>  	/* Just in case the BIOS is doing something questionable. */
>  	intel_disable_fbc(dev);
> +
> +	intel_modeset_setup_hw_state(dev, false);
>  }
>  
>  static void
> @@ -11439,10 +11441,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
>  	intel_modeset_init_hw(dev);
>  
>  	intel_setup_overlay(dev);
> -
> -	mutex_lock(&dev->mode_config.mutex);
> -	intel_modeset_setup_hw_state(dev, false);
> -	mutex_unlock(&dev->mode_config.mutex);
>  }
>  
>  void intel_modeset_cleanup(struct drm_device *dev)
> -- 
> 1.7.9.5
> 
> _______________________________________________
> 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

Re: [PATCH 3/6] drm/i915: allow re-use BIOS connector config for initial fbdev config v2

[Daniel Vetter]

On Tue, Feb 11, 2014 at 03:28:58PM -0800, Jesse Barnes wrote:
> The BIOS or boot loader will generally create an initial display
> configuration for us that includes some set of active pipes and
> displays.  This routine tries to figure out which pipes and connectors
> are active and stuffs them into the crtcs and modes array given to us by
> the drm_fb_helper code.
> 
> The overall sequence is:
>   intel_fbdev_init - from driver load
>     intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data
>     drm_fb_helper_init - build fb helper structs
>     drm_fb_helper_single_add_all_connectors - more fb helper structs
>   intel_fbdev_initial_config - apply the config
>     drm_fb_helper_initial_config - call ->probe then register_framebuffer()
>         drm_setup_crtcs - build crtc config for fbdev
>           intel_fb_initial_config - find active connectors etc
>         drm_fb_helper_single_fb_probe - set up fbdev
>           intelfb_create - re-use or alloc fb, build out fbdev structs
> 
> v2: use BIOS connector config unconditionally if possible (Daniel)
>     check for crtc cloning and reject (Daniel)
>     fix up comments (Daniel)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   10 ++--
>  drivers/gpu/drm/i915/intel_fbdev.c   |  103 ++++++++++++++++++++++++++++++++++
>  2 files changed, 107 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e800085..dea995d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -51,7 +51,10 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
>  
>  static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
>  			  int x, int y, struct drm_framebuffer *old_fb);
> -
> +static int intel_framebuffer_init(struct drm_device *dev,
> +				  struct intel_framebuffer *ifb,
> +				  struct drm_mode_fb_cmd2 *mode_cmd,
> +				  struct drm_i915_gem_object *obj);
>  
>  typedef struct {
>  	int	min, max;
> @@ -7692,11 +7695,6 @@ static struct drm_display_mode load_detect_mode = {
>  		 704, 832, 0, 480, 489, 491, 520, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
>  };
>  
> -static int intel_framebuffer_init(struct drm_device *dev,
> -				  struct intel_framebuffer *ifb,
> -				  struct drm_mode_fb_cmd2 *mode_cmd,
> -				  struct drm_i915_gem_object *obj);
> -
>  struct drm_framebuffer *
>  __intel_framebuffer_create(struct drm_device *dev,
>  			   struct drm_mode_fb_cmd2 *mode_cmd,
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index cf46273..2a0badfc 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -242,7 +242,110 @@ static void intel_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
>  	*blue = intel_crtc->lut_b[regno] << 8;
>  }
>  
> +static struct drm_fb_helper_crtc *
> +intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
> +{
> +	int i;
> +
> +	for (i = 0; i < fb_helper->crtc_count; i++)
> +		if (fb_helper->crtc_info[i].mode_set.crtc == crtc)
> +			return &fb_helper->crtc_info[i];
> +
> +	return NULL;
> +}
> +
> +/*
> + * Try to read the BIOS display configuration and use it for the initial
> + * fb configuration.
> + *
> + * The BIOS or boot loader will generally create an initial display
> + * configuration for us that includes some set of active pipes and displays.
> + * This routine tries to figure out which pipes and connectors are active
> + * and stuffs them into the crtcs and modes array given to us by the
> + * drm_fb_helper code.
> + *
> + * The overall sequence is:
> + *   intel_fbdev_init - from driver load
> + *     intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data
> + *     drm_fb_helper_init - build fb helper structs
> + *     drm_fb_helper_single_add_all_connectors - more fb helper structs
> + *   intel_fbdev_initial_config - apply the config
> + *     drm_fb_helper_initial_config - call ->probe then register_framebuffer()
> + *         drm_setup_crtcs - build crtc config for fbdev
> + *           intel_fb_initial_config - find active connectors etc
> + *         drm_fb_helper_single_fb_probe - set up fbdev
> + *           intelfb_create - re-use or alloc fb, build out fbdev structs
> + *
> + * Note that we don't make special consideration whether we could actually
> + * switch to the selected modes without a full modeset. E.g. when the display
> + * is in VGA mode we need to recalculate watermarks and set a new high-res
> + * framebuffer anyway.
> + */
> +static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> +				    struct drm_fb_helper_crtc **crtcs,
> +				    struct drm_display_mode **modes,
> +				    bool *enabled, int width, int height)
> +{
> +	int i, j;
> +
> +	for (i = 0; i < fb_helper->connector_count; i++) {
> +		struct drm_connector *connector;
> +		struct drm_encoder *encoder;
> +		struct drm_fb_helper_crtc *new_crtc;
> +
> +		connector = fb_helper->connector_info[i]->connector;
> +		if (!enabled[i]) {
> +			DRM_DEBUG_KMS("connector %d not enabled, skipping\n",
> +				      connector->base.id);
> +			continue;
> +		}
> +
> +		encoder = connector->encoder;
> +		if (!encoder || !encoder->crtc) {

You've missed the encoder->crtc change here from my review. I've converted
it into a WARN_ON since our state sanitizer shouldn't let anything like
this get through.

Otherwise merged the first 3 patches in this series to dinq.
-Daniel

> +			DRM_DEBUG_KMS("connector %d has no encoder or crtc, skipping\n",
> +				      connector->base.id);
> +			enabled[i] = false;
> +			continue;
> +		}
> +
> +		new_crtc = intel_fb_helper_crtc(fb_helper, encoder->crtc);
> +
> +		/*
> +		 * Make sure we're not trying to drive multiple connectors
> +		 * with a single CRTC, since our cloning support may not
> +		 * match the BIOS.
> +		 */
> +		for (j = 0; j < fb_helper->connector_count; j++) {
> +			if (crtcs[j] == new_crtc)
> +				return false;
> +		}
> +
> +		/*
> +		 * 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.
> +		 */
> +		intel_mode_from_pipe_config(&encoder->crtc->hwmode,
> +					    &to_intel_crtc(encoder->crtc)->config);
> +		modes[i] = &encoder->crtc->hwmode;
> +		crtcs[i] = new_crtc;
> +
> +		DRM_DEBUG_KMS("connector %s on crtc %d: %s\n",
> +			      drm_get_connector_name(connector),
> +			      encoder->crtc->base.id,
> +			      modes[i]->name);
> +	}
> +
> +	return true;
> +}
> +
>  static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
> +	.initial_config = intel_fb_initial_config,
>  	.gamma_set = intel_crtc_fb_gamma_set,
>  	.gamma_get = intel_crtc_fb_gamma_get,
>  	.fb_probe = intelfb_create,
> -- 
> 1.7.9.5
> 
> _______________________________________________
> 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

Re: [PATCH 6/6] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon v11

[Chris Wilson]

On Tue, Feb 11, 2014 at 03:29:01PM -0800, Jesse Barnes wrote:
> +/*
> + * 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 last_size = 0, max_size = 0;
> +
> +	if (!i915.fastboot)
> +		return false;
> +
> +	/* Find how big the fb would need to be for the largest pipe */
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {

Use struct intel_crtc as your iterator:
  list_for_each_entry(crtc, &dev->mode_config.crtc_list, head.base)
The few cases where you want crtc->base.fb merit the overall reduction
in noise.

> +		unsigned int tmp, stride;
> +
> +		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;
> +		}
> +
> +		stride = intel_crtc->plane_config.fb->base.pitches[0];
> +		tmp = intel_crtc->config.adjusted_mode.crtc_vdisplay * stride;

This calculation is still flawed.

The fb you select later may have a massive stride, but this fb may be
narrow, i.e. crtc->vdisplay * preferred_fb->stride > preferred_fb->size
We don't subsequently check that the CRTC fits into the new fb.

> +		max_size = max(max_size, tmp);
> +	}


> +	/* 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);

Does sanitize_crtc do the right thing? Or are we still displaying
garbage and triggering GPU hangs?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 3/6] drm/i915: allow re-use BIOS connector config for initial fbdev config v2

[Daniel Vetter]

On Wed, Feb 12, 2014 at 10:19:39AM +0100, Daniel Vetter wrote:
> On Tue, Feb 11, 2014 at 03:28:58PM -0800, Jesse Barnes wrote:
> > The BIOS or boot loader will generally create an initial display
> > configuration for us that includes some set of active pipes and
> > displays.  This routine tries to figure out which pipes and connectors
> > are active and stuffs them into the crtcs and modes array given to us by
> > the drm_fb_helper code.
> > 
> > The overall sequence is:
> >   intel_fbdev_init - from driver load
> >     intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data
> >     drm_fb_helper_init - build fb helper structs
> >     drm_fb_helper_single_add_all_connectors - more fb helper structs
> >   intel_fbdev_initial_config - apply the config
> >     drm_fb_helper_initial_config - call ->probe then register_framebuffer()
> >         drm_setup_crtcs - build crtc config for fbdev
> >           intel_fb_initial_config - find active connectors etc
> >         drm_fb_helper_single_fb_probe - set up fbdev
> >           intelfb_create - re-use or alloc fb, build out fbdev structs
> > 
> > v2: use BIOS connector config unconditionally if possible (Daniel)
> >     check for crtc cloning and reject (Daniel)
> >     fix up comments (Daniel)
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |   10 ++--
> >  drivers/gpu/drm/i915/intel_fbdev.c   |  103 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 107 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index e800085..dea995d 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -51,7 +51,10 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> >  
> >  static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
> >  			  int x, int y, struct drm_framebuffer *old_fb);
> > -
> > +static int intel_framebuffer_init(struct drm_device *dev,
> > +				  struct intel_framebuffer *ifb,
> > +				  struct drm_mode_fb_cmd2 *mode_cmd,
> > +				  struct drm_i915_gem_object *obj);
> >  
> >  typedef struct {
> >  	int	min, max;
> > @@ -7692,11 +7695,6 @@ static struct drm_display_mode load_detect_mode = {
> >  		 704, 832, 0, 480, 489, 491, 520, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
> >  };
> >  
> > -static int intel_framebuffer_init(struct drm_device *dev,
> > -				  struct intel_framebuffer *ifb,
> > -				  struct drm_mode_fb_cmd2 *mode_cmd,
> > -				  struct drm_i915_gem_object *obj);
> > -
> >  struct drm_framebuffer *
> >  __intel_framebuffer_create(struct drm_device *dev,
> >  			   struct drm_mode_fb_cmd2 *mode_cmd,
> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > index cf46273..2a0badfc 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -242,7 +242,110 @@ static void intel_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
> >  	*blue = intel_crtc->lut_b[regno] << 8;
> >  }
> >  
> > +static struct drm_fb_helper_crtc *
> > +intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < fb_helper->crtc_count; i++)
> > +		if (fb_helper->crtc_info[i].mode_set.crtc == crtc)
> > +			return &fb_helper->crtc_info[i];
> > +
> > +	return NULL;
> > +}
> > +
> > +/*
> > + * Try to read the BIOS display configuration and use it for the initial
> > + * fb configuration.
> > + *
> > + * The BIOS or boot loader will generally create an initial display
> > + * configuration for us that includes some set of active pipes and displays.
> > + * This routine tries to figure out which pipes and connectors are active
> > + * and stuffs them into the crtcs and modes array given to us by the
> > + * drm_fb_helper code.
> > + *
> > + * The overall sequence is:
> > + *   intel_fbdev_init - from driver load
> > + *     intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data
> > + *     drm_fb_helper_init - build fb helper structs
> > + *     drm_fb_helper_single_add_all_connectors - more fb helper structs
> > + *   intel_fbdev_initial_config - apply the config
> > + *     drm_fb_helper_initial_config - call ->probe then register_framebuffer()
> > + *         drm_setup_crtcs - build crtc config for fbdev
> > + *           intel_fb_initial_config - find active connectors etc
> > + *         drm_fb_helper_single_fb_probe - set up fbdev
> > + *           intelfb_create - re-use or alloc fb, build out fbdev structs
> > + *
> > + * Note that we don't make special consideration whether we could actually
> > + * switch to the selected modes without a full modeset. E.g. when the display
> > + * is in VGA mode we need to recalculate watermarks and set a new high-res
> > + * framebuffer anyway.
> > + */
> > +static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> > +				    struct drm_fb_helper_crtc **crtcs,
> > +				    struct drm_display_mode **modes,
> > +				    bool *enabled, int width, int height)
> > +{
> > +	int i, j;
> > +
> > +	for (i = 0; i < fb_helper->connector_count; i++) {
> > +		struct drm_connector *connector;
> > +		struct drm_encoder *encoder;
> > +		struct drm_fb_helper_crtc *new_crtc;
> > +
> > +		connector = fb_helper->connector_info[i]->connector;
> > +		if (!enabled[i]) {
> > +			DRM_DEBUG_KMS("connector %d not enabled, skipping\n",
> > +				      connector->base.id);
> > +			continue;
> > +		}
> > +
> > +		encoder = connector->encoder;
> > +		if (!encoder || !encoder->crtc) {
> 
> You've missed the encoder->crtc change here from my review. I've converted
> it into a WARN_ON since our state sanitizer shouldn't let anything like
> this get through.
> 
> Otherwise merged the first 3 patches in this series to dinq.

Ville complained on irc that his box now bots into a 640x480 console. So I
think we need to add another check to compare the selected mode with the
preferred mode which fbcon would have picke to make sure that htotal and
vtotal match. Timings can obviously differe a bit.

Another regression is that now we don't parse/obey video= cmdline options
any more. This is fairly important since the enable/disable flags won't be
parsed any more so will upset Alan and his T100 again ;-) That itself is a
bit a design goof-up since with CONFIG_FBDEV=n that'll break, too. But
another issue to resolve that.

I haven't looked at the details of how to best solve this, but I guess we
could move the call to driver->initial_config down after all the fbdev
parsing has happened already. Then the driver just fixes things up. The
i915 initial_config logic will be a bit more tricky, but I think it should
work out.

I've dropped this patch for now until we figure this out.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 3/6] drm/i915: allow re-use BIOS connector config for initial fbdev config v2

[Ville Syrjälä]

On Wed, Feb 12, 2014 at 01:04:28PM +0100, Daniel Vetter wrote:
> On Wed, Feb 12, 2014 at 10:19:39AM +0100, Daniel Vetter wrote:
> > On Tue, Feb 11, 2014 at 03:28:58PM -0800, Jesse Barnes wrote:
> > > The BIOS or boot loader will generally create an initial display
> > > configuration for us that includes some set of active pipes and
> > > displays.  This routine tries to figure out which pipes and connectors
> > > are active and stuffs them into the crtcs and modes array given to us by
> > > the drm_fb_helper code.
> > > 
> > > The overall sequence is:
> > >   intel_fbdev_init - from driver load
> > >     intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data
> > >     drm_fb_helper_init - build fb helper structs
> > >     drm_fb_helper_single_add_all_connectors - more fb helper structs
> > >   intel_fbdev_initial_config - apply the config
> > >     drm_fb_helper_initial_config - call ->probe then register_framebuffer()
> > >         drm_setup_crtcs - build crtc config for fbdev
> > >           intel_fb_initial_config - find active connectors etc
> > >         drm_fb_helper_single_fb_probe - set up fbdev
> > >           intelfb_create - re-use or alloc fb, build out fbdev structs
> > > 
> > > v2: use BIOS connector config unconditionally if possible (Daniel)
> > >     check for crtc cloning and reject (Daniel)
> > >     fix up comments (Daniel)
> > > 
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c |   10 ++--
> > >  drivers/gpu/drm/i915/intel_fbdev.c   |  103 ++++++++++++++++++++++++++++++++++
> > >  2 files changed, 107 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index e800085..dea995d 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -51,7 +51,10 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> > >  
> > >  static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
> > >  			  int x, int y, struct drm_framebuffer *old_fb);
> > > -
> > > +static int intel_framebuffer_init(struct drm_device *dev,
> > > +				  struct intel_framebuffer *ifb,
> > > +				  struct drm_mode_fb_cmd2 *mode_cmd,
> > > +				  struct drm_i915_gem_object *obj);
> > >  
> > >  typedef struct {
> > >  	int	min, max;
> > > @@ -7692,11 +7695,6 @@ static struct drm_display_mode load_detect_mode = {
> > >  		 704, 832, 0, 480, 489, 491, 520, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
> > >  };
> > >  
> > > -static int intel_framebuffer_init(struct drm_device *dev,
> > > -				  struct intel_framebuffer *ifb,
> > > -				  struct drm_mode_fb_cmd2 *mode_cmd,
> > > -				  struct drm_i915_gem_object *obj);
> > > -
> > >  struct drm_framebuffer *
> > >  __intel_framebuffer_create(struct drm_device *dev,
> > >  			   struct drm_mode_fb_cmd2 *mode_cmd,
> > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > > index cf46273..2a0badfc 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > @@ -242,7 +242,110 @@ static void intel_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
> > >  	*blue = intel_crtc->lut_b[regno] << 8;
> > >  }
> > >  
> > > +static struct drm_fb_helper_crtc *
> > > +intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
> > > +{
> > > +	int i;
> > > +
> > > +	for (i = 0; i < fb_helper->crtc_count; i++)
> > > +		if (fb_helper->crtc_info[i].mode_set.crtc == crtc)
> > > +			return &fb_helper->crtc_info[i];
> > > +
> > > +	return NULL;
> > > +}
> > > +
> > > +/*
> > > + * Try to read the BIOS display configuration and use it for the initial
> > > + * fb configuration.
> > > + *
> > > + * The BIOS or boot loader will generally create an initial display
> > > + * configuration for us that includes some set of active pipes and displays.
> > > + * This routine tries to figure out which pipes and connectors are active
> > > + * and stuffs them into the crtcs and modes array given to us by the
> > > + * drm_fb_helper code.
> > > + *
> > > + * The overall sequence is:
> > > + *   intel_fbdev_init - from driver load
> > > + *     intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data
> > > + *     drm_fb_helper_init - build fb helper structs
> > > + *     drm_fb_helper_single_add_all_connectors - more fb helper structs
> > > + *   intel_fbdev_initial_config - apply the config
> > > + *     drm_fb_helper_initial_config - call ->probe then register_framebuffer()
> > > + *         drm_setup_crtcs - build crtc config for fbdev
> > > + *           intel_fb_initial_config - find active connectors etc
> > > + *         drm_fb_helper_single_fb_probe - set up fbdev
> > > + *           intelfb_create - re-use or alloc fb, build out fbdev structs
> > > + *
> > > + * Note that we don't make special consideration whether we could actually
> > > + * switch to the selected modes without a full modeset. E.g. when the display
> > > + * is in VGA mode we need to recalculate watermarks and set a new high-res
> > > + * framebuffer anyway.
> > > + */
> > > +static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> > > +				    struct drm_fb_helper_crtc **crtcs,
> > > +				    struct drm_display_mode **modes,
> > > +				    bool *enabled, int width, int height)
> > > +{
> > > +	int i, j;
> > > +
> > > +	for (i = 0; i < fb_helper->connector_count; i++) {
> > > +		struct drm_connector *connector;
> > > +		struct drm_encoder *encoder;
> > > +		struct drm_fb_helper_crtc *new_crtc;
> > > +
> > > +		connector = fb_helper->connector_info[i]->connector;
> > > +		if (!enabled[i]) {
> > > +			DRM_DEBUG_KMS("connector %d not enabled, skipping\n",
> > > +				      connector->base.id);
> > > +			continue;
> > > +		}
> > > +
> > > +		encoder = connector->encoder;
> > > +		if (!encoder || !encoder->crtc) {
> > 
> > You've missed the encoder->crtc change here from my review. I've converted
> > it into a WARN_ON since our state sanitizer shouldn't let anything like
> > this get through.
> > 
> > Otherwise merged the first 3 patches in this series to dinq.
> 
> Ville complained on irc that his box now bots into a 640x480 console. So I
> think we need to add another check to compare the selected mode with the
> preferred mode which fbcon would have picke to make sure that htotal and
> vtotal match. Timings can obviously differe a bit.
> 
> Another regression is that now we don't parse/obey video= cmdline options
> any more. This is fairly important since the enable/disable flags won't be
> parsed any more so will upset Alan and his T100 again ;-) That itself is a
> bit a design goof-up since with CONFIG_FBDEV=n that'll break, too. But
> another issue to resolve that.

BTW another funky issue I noticed about the video= option is that if you
specify a mode that's not part of the set of modes added by
drm_add_modes_noedid(), then fbcon will happily use the specified mode,
but when X starts the mode gets pruned from the mode list. It would
seem better to keep the user specified mode on the list, no matter what.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 3/6] drm/i915: allow re-use BIOS connector config for initial fbdev config v2

[Daniel Vetter]

On Wed, Feb 12, 2014 at 1:45 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> BTW another funky issue I noticed about the video= option is that if you
> specify a mode that's not part of the set of modes added by
> drm_add_modes_noedid(), then fbcon will happily use the specified mode,
> but when X starts the mode gets pruned from the mode list. It would
> seem better to keep the user specified mode on the list, no matter what.

Yeah, I think we need to move the cmdline parsing out of the fbdev
helper into crtc helpers so that this kind of stuff Just Works. Then
we could also add the cmdline mode as the first preferred one.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 3/6] drm/i915: allow re-use BIOS connector config for initial fbdev config v2

[Chris Wilson]

On Wed, Feb 12, 2014 at 02:15:18PM +0100, Daniel Vetter wrote:
> On Wed, Feb 12, 2014 at 1:45 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > BTW another funky issue I noticed about the video= option is that if you
> > specify a mode that's not part of the set of modes added by
> > drm_add_modes_noedid(), then fbcon will happily use the specified mode,
> > but when X starts the mode gets pruned from the mode list. It would
> > seem better to keep the user specified mode on the list, no matter what.
> 
> Yeah, I think we need to move the cmdline parsing out of the fbdev
> helper into crtc helpers so that this kind of stuff Just Works. Then
> we could also add the cmdline mode as the first preferred one.

fyi, https://bugs.freedesktop.org/show_bug.cgi?id=73154
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre