All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] drm/i915: fbdev mode restoration improvements v2
@ 2013-05-31 17:07 ville.syrjala
  2013-05-31 17:07 ` [PATCH v2 1/7] drm: Add fb_helper->restore_fbdev_mode hook ville.syrjala
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: ville.syrjala @ 2013-05-31 17:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

Another attempt at restoring the fbdev mode.

Changes from v1:
- Cursors and sprites are disabled permanently (well, until someone
  explicitly re-enabls them). This was actually already the case for
  the old video overlay.
- Since the disabling is now permanent, all extra planes can be
  disabled before the crtc modeset/set_base is performed so there should
  be no extra blinking of the planes

I still left the new fbdev_restore_mode hook in place because I was too
lazy to figure out if I could just disable planes/cursors from drm_fb_helper
directly (i915 would be fine, but other drivers could have issues with it),
and even with i915 we still have to disable the old video overlay, which is
driven via some custom API, and hence can't be disabled from the common code.

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

* [PATCH v2 1/7] drm: Add fb_helper->restore_fbdev_mode hook
  2013-05-31 17:07 [PATCH 0/7] drm/i915: fbdev mode restoration improvements v2 ville.syrjala
@ 2013-05-31 17:07 ` ville.syrjala
  2013-06-03  9:50   ` Chris Wilson
  2013-05-31 17:07 ` [PATCH 2/7] drm: Add drm_plane_force_disable() ville.syrjala
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: ville.syrjala @ 2013-05-31 17:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Drivers may need to turn off overlay planes, cursors, etc. when
restoring the fbdev mode. So allow drivers to provide their own
version of drm_fb_helper_restore_fbdev_mode() that can take care
of such details.

Initially just plug in drm_fb_helper_restore_fbdev_mode for all
drivers.

v2: Add kernel-doc for the new hook

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/ast/ast_fb.c              | 1 +
 drivers/gpu/drm/cirrus/cirrus_fbdev.c     | 1 +
 drivers/gpu/drm/drm_fb_cma_helper.c       | 1 +
 drivers/gpu/drm/drm_fb_helper.c           | 2 +-
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 1 +
 drivers/gpu/drm/gma500/framebuffer.c      | 1 +
 drivers/gpu/drm/i915/intel_fb.c           | 1 +
 drivers/gpu/drm/mgag200/mgag200_fb.c      | 1 +
 drivers/gpu/drm/nouveau/nouveau_fbcon.c   | 1 +
 drivers/gpu/drm/omapdrm/omap_fbdev.c      | 1 +
 drivers/gpu/drm/qxl/qxl_fb.c              | 1 +
 drivers/gpu/drm/radeon/radeon_fb.c        | 1 +
 drivers/gpu/drm/udl/udl_fb.c              | 1 +
 include/drm/drm_fb_helper.h               | 2 ++
 14 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
index fbc0823..a9cafe2 100644
--- a/drivers/gpu/drm/ast/ast_fb.c
+++ b/drivers/gpu/drm/ast/ast_fb.c
@@ -290,6 +290,7 @@ static struct drm_fb_helper_funcs ast_fb_helper_funcs = {
 	.gamma_set = ast_fb_gamma_set,
 	.gamma_get = ast_fb_gamma_get,
 	.fb_probe = astfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static void ast_fbdev_destroy(struct drm_device *dev,
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index 3541b56..064a182 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -288,6 +288,7 @@ static struct drm_fb_helper_funcs cirrus_fb_helper_funcs = {
 	.gamma_set = cirrus_crtc_fb_gamma_set,
 	.gamma_get = cirrus_crtc_fb_gamma_get,
 	.fb_probe = cirrusfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int cirrus_fbdev_init(struct cirrus_device *cdev)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 0b5af7d..f011628 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -330,6 +330,7 @@ err_drm_gem_cma_free_object:
 
 static struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = {
 	.fb_probe = drm_fbdev_cma_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 /**
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0df0ebb..e9af615 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -318,7 +318,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
 		if (helper->dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 			continue;
 
-		ret = drm_fb_helper_restore_fbdev_mode(helper);
+		ret = helper->funcs->restore_fbdev_mode(helper);
 		if (ret)
 			error = true;
 	}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 68f0045..6ed4065 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -228,6 +228,7 @@ out:
 
 static struct drm_fb_helper_funcs exynos_drm_fb_helper_funcs = {
 	.fb_probe =	exynos_drm_fbdev_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int exynos_drm_fbdev_init(struct drm_device *dev)
diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index 1534e22..8d7f9c0 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -567,6 +567,7 @@ static struct drm_fb_helper_funcs psb_fb_helper_funcs = {
 	.gamma_set = psbfb_gamma_set,
 	.gamma_get = psbfb_gamma_get,
 	.fb_probe = psbfb_probe,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static int psb_fbdev_destroy(struct drm_device *dev, struct psb_fbdev *fbdev)
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 6b7c3ca..5fe525a 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -187,6 +187,7 @@ static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
 	.gamma_set = intel_crtc_fb_gamma_set,
 	.gamma_get = intel_crtc_fb_gamma_get,
 	.fb_probe = intelfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static void intel_fbdev_destroy(struct drm_device *dev,
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c
index 5da824c..1720316 100644
--- a/drivers/gpu/drm/mgag200/mgag200_fb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
@@ -275,6 +275,7 @@ static struct drm_fb_helper_funcs mga_fb_helper_funcs = {
 	.gamma_set = mga_crtc_fb_gamma_set,
 	.gamma_get = mga_crtc_fb_gamma_get,
 	.fb_probe = mgag200fb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int mgag200_fbdev_init(struct mga_device *mdev)
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index b035317..2a280d8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -435,6 +435,7 @@ static struct drm_fb_helper_funcs nouveau_fbcon_helper_funcs = {
 	.gamma_set = nouveau_fbcon_gamma_set,
 	.gamma_get = nouveau_fbcon_gamma_get,
 	.fb_probe = nouveau_fbcon_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 
diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
index 002988d..33f27ac 100644
--- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
+++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
@@ -283,6 +283,7 @@ fail:
 
 static struct drm_fb_helper_funcs omap_fb_helper_funcs = {
 	.fb_probe = omap_fbdev_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static struct drm_fb_helper *get_fb(struct fb_info *fbi)
diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c
index 4b955b0..a274c16 100644
--- a/drivers/gpu/drm/qxl/qxl_fb.c
+++ b/drivers/gpu/drm/qxl/qxl_fb.c
@@ -521,6 +521,7 @@ static int qxl_fbdev_destroy(struct drm_device *dev, struct qxl_fbdev *qfbdev)
 
 static struct drm_fb_helper_funcs qxl_fb_helper_funcs = {
 	.fb_probe = qxl_fb_find_or_create_single,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int qxl_fbdev_init(struct qxl_device *qdev)
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index b174674..2b1b211 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -335,6 +335,7 @@ static struct drm_fb_helper_funcs radeon_fb_helper_funcs = {
 	.gamma_set = radeon_crtc_fb_gamma_set,
 	.gamma_get = radeon_crtc_fb_gamma_get,
 	.fb_probe = radeonfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int radeon_fbdev_init(struct radeon_device *rdev)
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index 97e9d61..878c602 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -546,6 +546,7 @@ out:
 
 static struct drm_fb_helper_funcs udl_fb_helper_funcs = {
 	.fb_probe = udlfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static void udl_fbdev_destroy(struct drm_device *dev,
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 471f276..146abb6 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -58,6 +58,7 @@ struct drm_fb_helper_surface_size {
  *            structure. Futhermore it also needs to allocate the drm
  *            framebuffer used to back the fbdev.
  * @initial_config: Setup an initial fbdev display configuration
+ * @restore_fbdev_mode: Restore the fbdev display configuration (eg. to show an oops)
  *
  * Driver callbacks used by the fbdev emulation helper library.
  */
@@ -73,6 +74,7 @@ struct drm_fb_helper_funcs {
 			       struct drm_fb_helper_crtc **crtcs,
 			       struct drm_display_mode **modes,
 			       bool *enabled, int width, int height);
+	bool (*restore_fbdev_mode)(struct drm_fb_helper *fb_helper);
 };
 
 struct drm_fb_helper_connector {
-- 
1.8.1.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 2/7] drm: Add drm_plane_force_disable()
  2013-05-31 17:07 [PATCH 0/7] drm/i915: fbdev mode restoration improvements v2 ville.syrjala
  2013-05-31 17:07 ` [PATCH v2 1/7] drm: Add fb_helper->restore_fbdev_mode hook ville.syrjala
@ 2013-05-31 17:07 ` ville.syrjala
  2013-05-31 17:07 ` [PATCH v2 3/7] drm/i915: Fix fbdev sprite disable code ville.syrjala
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: ville.syrjala @ 2013-05-31 17:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

drm_plane_force_disable() will forcibly disable the plane even if user
had previously requested the plane to be enabled.

This can be used to force planes to be off when restoring the fbdev
mode.

The code was simply pulled from drm_framebuffer_remove(), which now
calls the new function as well.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 27 +++++++++++++++++----------
 include/drm/drm_crtc.h     |  1 +
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e7e9242..c65124d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -569,16 +569,8 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
 		}
 
 		list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
-			if (plane->fb == fb) {
-				/* should turn off the crtc */
-				ret = plane->funcs->disable_plane(plane);
-				if (ret)
-					DRM_ERROR("failed to disable plane with busy fb\n");
-				/* disconnect the plane from the fb and crtc: */
-				__drm_framebuffer_unreference(plane->fb);
-				plane->fb = NULL;
-				plane->crtc = NULL;
-			}
+			if (plane->fb == fb)
+				drm_plane_force_disable(plane);
 		}
 		drm_modeset_unlock_all(dev);
 	}
@@ -867,6 +859,21 @@ void drm_plane_cleanup(struct drm_plane *plane)
 }
 EXPORT_SYMBOL(drm_plane_cleanup);
 
+void drm_plane_force_disable(struct drm_plane *plane)
+{
+	int ret;
+
+	/* should turn off the crtc */
+	ret = plane->funcs->disable_plane(plane);
+	if (ret)
+		DRM_ERROR("failed to disable plane with busy fb\n");
+	/* disconnect the plane from the fb and crtc: */
+	__drm_framebuffer_unreference(plane->fb);
+	plane->fb = NULL;
+	plane->crtc = NULL;
+}
+EXPORT_SYMBOL(drm_plane_force_disable);
+
 /**
  * drm_mode_create - create a new display mode
  * @dev: DRM device
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index adb3f9b..db7a885 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -894,6 +894,7 @@ extern int drm_plane_init(struct drm_device *dev,
 			  const uint32_t *formats, uint32_t format_count,
 			  bool priv);
 extern void drm_plane_cleanup(struct drm_plane *plane);
+extern void drm_plane_force_disable(struct drm_plane *plane);
 
 extern void drm_encoder_cleanup(struct drm_encoder *encoder);
 
-- 
1.8.1.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 3/7] drm/i915: Fix fbdev sprite disable code
  2013-05-31 17:07 [PATCH 0/7] drm/i915: fbdev mode restoration improvements v2 ville.syrjala
  2013-05-31 17:07 ` [PATCH v2 1/7] drm: Add fb_helper->restore_fbdev_mode hook ville.syrjala
  2013-05-31 17:07 ` [PATCH 2/7] drm: Add drm_plane_force_disable() ville.syrjala
@ 2013-05-31 17:07 ` ville.syrjala
  2013-06-03  9:53   ` Chris Wilson
  2013-06-03 12:00   ` Daniel Vetter
  2013-05-31 17:07 ` [PATCH v2 4/7] drm/i915: Use a custom restore_fbdev_mode hook ville.syrjala
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 17+ messages in thread
From: ville.syrjala @ 2013-05-31 17:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

plane->enabled is never set, so this code didn't do anything.

Fix the code for sprites by calling the new drm_plane_force_disable()
function. That means the plane will remain off until someone explicitly
turns it back on.

And do the same for cursors and the old video overlays, since we only
want to see the primary plane for fbdev.

v2: Disable sprites/cursors until explicitly re-enabled

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_fb.c      |  9 ++-------
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 944b6d5..1d1a3fd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9842,3 +9842,19 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m,
 	}
 }
 #endif
+
+void intel_disable_cursors_and_sprites(struct drm_device *dev)
+{
+	struct drm_crtc *crtc;
+	struct drm_plane *plane;
+
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		intel_crtc_dpms_overlay(to_intel_crtc(crtc), false);
+		intel_crtc_cursor_set(crtc, NULL, 0, 0, 0);
+	}
+
+	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
+		if (plane->fb)
+			drm_plane_force_disable(plane);
+	}
+}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d0a8c1a..f8e76cf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -800,5 +800,6 @@ extern bool intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev,
 extern bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
 						 enum transcoder pch_transcoder,
 						 bool enable);
+extern void intel_disable_cursors_and_sprites(struct drm_device *dev);
 
 #endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 5fe525a..b34ccf3 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -293,22 +293,17 @@ void intel_fb_restore_mode(struct drm_device *dev)
 {
 	int ret;
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	struct drm_mode_config *config = &dev->mode_config;
-	struct drm_plane *plane;
 
 	if (INTEL_INFO(dev)->num_pipes == 0)
 		return;
 
 	drm_modeset_lock_all(dev);
 
+	intel_disable_cursors_and_sprites(dev);
+
 	ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper);
 	if (ret)
 		DRM_DEBUG("failed to restore crtc mode\n");
 
-	/* Be sure to shut off any planes that may be active */
-	list_for_each_entry(plane, &config->plane_list, head)
-		if (plane->enabled)
-			plane->funcs->disable_plane(plane);
-
 	drm_modeset_unlock_all(dev);
 }
-- 
1.8.1.5

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

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

* [PATCH v2 4/7] drm/i915: Use a custom restore_fbdev_mode hook
  2013-05-31 17:07 [PATCH 0/7] drm/i915: fbdev mode restoration improvements v2 ville.syrjala
                   ` (2 preceding siblings ...)
  2013-05-31 17:07 ` [PATCH v2 3/7] drm/i915: Fix fbdev sprite disable code ville.syrjala
@ 2013-05-31 17:07 ` ville.syrjala
  2013-06-03 10:08   ` Chris Wilson
  2013-05-31 17:07 ` [PATCH 5/7] drm/i915: Use container_of() in the fbdev code ville.syrjala
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: ville.syrjala @ 2013-05-31 17:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Disable sprite planes and cursors when restoring the fbdev mode.

Should makes oopses more readable if they're not covered by sprites and
cursors.

v2: Rebased due to changes earlier in the series

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_fb.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index b34ccf3..e8389df 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -183,11 +183,25 @@ out:
 	return ret;
 }
 
+static bool intel_fb_restore_fbdev_mode(struct drm_fb_helper *helper)
+{
+	struct drm_device *dev = helper->dev;
+	bool ret;
+
+	intel_disable_cursors_and_sprites(dev);
+
+	ret = drm_fb_helper_restore_fbdev_mode(helper);
+	if (ret)
+		DRM_DEBUG("failed to restore crtc mode\n");
+
+	return ret;
+}
+
 static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
 	.gamma_set = intel_crtc_fb_gamma_set,
 	.gamma_get = intel_crtc_fb_gamma_get,
 	.fb_probe = intelfb_create,
-	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
+	.restore_fbdev_mode = intel_fb_restore_fbdev_mode,
 };
 
 static void intel_fbdev_destroy(struct drm_device *dev,
@@ -291,7 +305,6 @@ void intel_fb_output_poll_changed(struct drm_device *dev)
 
 void intel_fb_restore_mode(struct drm_device *dev)
 {
-	int ret;
 	drm_i915_private_t *dev_priv = dev->dev_private;
 
 	if (INTEL_INFO(dev)->num_pipes == 0)
@@ -299,11 +312,7 @@ void intel_fb_restore_mode(struct drm_device *dev)
 
 	drm_modeset_lock_all(dev);
 
-	intel_disable_cursors_and_sprites(dev);
-
-	ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper);
-	if (ret)
-		DRM_DEBUG("failed to restore crtc mode\n");
+	intel_fb_restore_fbdev_mode(&dev_priv->fbdev->helper);
 
 	drm_modeset_unlock_all(dev);
 }
-- 
1.8.1.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 5/7] drm/i915: Use container_of() in the fbdev code
  2013-05-31 17:07 [PATCH 0/7] drm/i915: fbdev mode restoration improvements v2 ville.syrjala
                   ` (3 preceding siblings ...)
  2013-05-31 17:07 ` [PATCH v2 4/7] drm/i915: Use a custom restore_fbdev_mode hook ville.syrjala
@ 2013-05-31 17:07 ` ville.syrjala
  2013-06-03 10:08   ` Chris Wilson
  2013-05-31 17:07 ` [PATCH 6/7] drm/i915: s/drm_i915_private_t/struct drm_i915_private/ ville.syrjala
  2013-05-31 17:07 ` [PATCH v2 7/7] drm: Remove some unused stuff from drm_plane ville.syrjala
  6 siblings, 1 reply; 17+ messages in thread
From: ville.syrjala @ 2013-05-31 17:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use container_of() instead of a cast to get struct intel_fbdev
from struct drm_fb_helper.

Also populate the fb_info->par correctly with the drm_fb_helper pointer
instead of the intel_fbdev pointer.

There's no actual functional change since the drm_fb_helper happens to
be the first member inside intel_fbdev.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_fb.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index e8389df..60d0cf6 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -60,8 +60,9 @@ static struct fb_ops intelfb_ops = {
 static int intelfb_create(struct drm_fb_helper *helper,
 			  struct drm_fb_helper_surface_size *sizes)
 {
-	struct intel_fbdev *ifbdev = (struct intel_fbdev *)helper;
-	struct drm_device *dev = ifbdev->helper.dev;
+	struct intel_fbdev *ifbdev =
+		container_of(helper, struct intel_fbdev, helper);
+	struct drm_device *dev = helper->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct fb_info *info;
 	struct drm_framebuffer *fb;
@@ -108,7 +109,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 		goto out_unpin;
 	}
 
-	info->par = ifbdev;
+	info->par = helper;
 
 	ret = intel_framebuffer_init(dev, &ifbdev->ifb, &mode_cmd, obj);
 	if (ret)
-- 
1.8.1.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 6/7] drm/i915: s/drm_i915_private_t/struct drm_i915_private/
  2013-05-31 17:07 [PATCH 0/7] drm/i915: fbdev mode restoration improvements v2 ville.syrjala
                   ` (4 preceding siblings ...)
  2013-05-31 17:07 ` [PATCH 5/7] drm/i915: Use container_of() in the fbdev code ville.syrjala
@ 2013-05-31 17:07 ` ville.syrjala
  2013-06-03 10:09   ` Chris Wilson
  2013-05-31 17:07 ` [PATCH v2 7/7] drm: Remove some unused stuff from drm_plane ville.syrjala
  6 siblings, 1 reply; 17+ messages in thread
From: ville.syrjala @ 2013-05-31 17:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

People don't like typedefs these days. Eliminate their use from intel_fb.c.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_fb.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 60d0cf6..ae3a348 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -233,7 +233,7 @@ static void intel_fbdev_destroy(struct drm_device *dev,
 int intel_fbdev_init(struct drm_device *dev)
 {
 	struct intel_fbdev *ifbdev;
-	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
 	ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL);
@@ -258,7 +258,7 @@ int intel_fbdev_init(struct drm_device *dev)
 
 void intel_fbdev_initial_config(struct drm_device *dev)
 {
-	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	/* Due to peculiar init order wrt to hpd handling this is separate. */
 	drm_fb_helper_initial_config(&dev_priv->fbdev->helper, 32);
@@ -266,7 +266,7 @@ void intel_fbdev_initial_config(struct drm_device *dev)
 
 void intel_fbdev_fini(struct drm_device *dev)
 {
-	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	if (!dev_priv->fbdev)
 		return;
 
@@ -277,7 +277,7 @@ void intel_fbdev_fini(struct drm_device *dev)
 
 void intel_fbdev_set_suspend(struct drm_device *dev, int state)
 {
-	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_fbdev *ifbdev = dev_priv->fbdev;
 	struct fb_info *info;
 
@@ -300,13 +300,13 @@ MODULE_LICENSE("GPL and additional rights");
 
 void intel_fb_output_poll_changed(struct drm_device *dev)
 {
-	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
 }
 
 void intel_fb_restore_mode(struct drm_device *dev)
 {
-	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	if (INTEL_INFO(dev)->num_pipes == 0)
 		return;
-- 
1.8.1.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 7/7] drm: Remove some unused stuff from drm_plane
  2013-05-31 17:07 [PATCH 0/7] drm/i915: fbdev mode restoration improvements v2 ville.syrjala
                   ` (5 preceding siblings ...)
  2013-05-31 17:07 ` [PATCH 6/7] drm/i915: s/drm_i915_private_t/struct drm_i915_private/ ville.syrjala
@ 2013-05-31 17:07 ` ville.syrjala
  6 siblings, 0 replies; 17+ messages in thread
From: ville.syrjala @ 2013-05-31 17:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

There's a bunch of unused members inside drm_plane, bloating the size of
the structure needlessly. Eliminate them.

v2: Remove all of it from kernel-doc too

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_crtc.c |  2 +-
 include/drm/drm_crtc.h     | 11 -----------
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index c65124d..bbbfb3f 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1747,7 +1747,7 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
 
 	plane_resp->plane_id = plane->base.id;
 	plane_resp->possible_crtcs = plane->possible_crtcs;
-	plane_resp->gamma_size = plane->gamma_size;
+	plane_resp->gamma_size = 0;
 
 	/*
 	 * This ioctl is called twice, once to determine how much space is
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index db7a885..3c14b46 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -654,11 +654,7 @@ struct drm_plane_funcs {
  * @format_count: number of formats supported
  * @crtc: currently bound CRTC
  * @fb: currently bound fb
- * @gamma_size: size of gamma table
- * @gamma_store: gamma correction table
- * @enabled: enabled flag
  * @funcs: helper functions
- * @helper_private: storage for drver layer
  * @properties: property tracking for this plane
  */
 struct drm_plane {
@@ -674,14 +670,7 @@ struct drm_plane {
 	struct drm_crtc *crtc;
 	struct drm_framebuffer *fb;
 
-	/* CRTC gamma size for reporting to userspace */
-	uint32_t gamma_size;
-	uint16_t *gamma_store;
-
-	bool enabled;
-
 	const struct drm_plane_funcs *funcs;
-	void *helper_private;
 
 	struct drm_object_properties properties;
 };
-- 
1.8.1.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v2 1/7] drm: Add fb_helper->restore_fbdev_mode hook
  2013-05-31 17:07 ` [PATCH v2 1/7] drm: Add fb_helper->restore_fbdev_mode hook ville.syrjala
@ 2013-06-03  9:50   ` Chris Wilson
  0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2013-06-03  9:50 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, dri-devel

On Fri, May 31, 2013 at 08:07:01PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Drivers may need to turn off overlay planes, cursors, etc. when
> restoring the fbdev mode. So allow drivers to provide their own
> version of drm_fb_helper_restore_fbdev_mode() that can take care
> of such details.
> 
> Initially just plug in drm_fb_helper_restore_fbdev_mode for all
> drivers.

I really don't like the bool interface here, and it looks even more
confusing when passing through the hooks. Can we have this changed to be
a proper error code?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH v2 3/7] drm/i915: Fix fbdev sprite disable code
  2013-05-31 17:07 ` [PATCH v2 3/7] drm/i915: Fix fbdev sprite disable code ville.syrjala
@ 2013-06-03  9:53   ` Chris Wilson
  2013-06-03 12:00   ` Daniel Vetter
  1 sibling, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2013-06-03  9:53 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, dri-devel

On Fri, May 31, 2013 at 08:07:03PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> plane->enabled is never set, so this code didn't do anything.
> 
> Fix the code for sprites by calling the new drm_plane_force_disable()
> function. That means the plane will remain off until someone explicitly
> turns it back on.
> 
> And do the same for cursors and the old video overlays, since we only
> want to see the primary plane for fbdev.
> 
> v2: Disable sprites/cursors until explicitly re-enabled
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_fb.c      |  9 ++-------
>  3 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 944b6d5..1d1a3fd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9842,3 +9842,19 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m,
>  	}
>  }
>  #endif
> +
> +void intel_disable_cursors_and_sprites(struct drm_device *dev)
> +{
> +	struct drm_crtc *crtc;
> +	struct drm_plane *plane;
> +
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		intel_crtc_dpms_overlay(to_intel_crtc(crtc), false);
> +		intel_crtc_cursor_set(crtc, NULL, 0, 0, 0);
> +	}
> +
> +	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
> +		if (plane->fb)
> +			drm_plane_force_disable(plane);

This would be neater if drm_plane_force_disable did the check for
plane-fb itself.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH v2 4/7] drm/i915: Use a custom restore_fbdev_mode hook
  2013-05-31 17:07 ` [PATCH v2 4/7] drm/i915: Use a custom restore_fbdev_mode hook ville.syrjala
@ 2013-06-03 10:08   ` Chris Wilson
  0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2013-06-03 10:08 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, dri-devel

On Fri, May 31, 2013 at 08:07:04PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Disable sprite planes and cursors when restoring the fbdev mode.
> 
> Should makes oopses more readable if they're not covered by sprites and
> cursors.
> 
> v2: Rebased due to changes earlier in the series

Nice, this should fix the issue of the rogue cursor and the like if X
exits abnormally. Explicitly disabling it feels wrong, but as the cursor
is not integrated into the modesetting sequence I have no better
suggestion.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Comment inline.

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fb.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index b34ccf3..e8389df 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -183,11 +183,25 @@ out:
>  	return ret;
>  }
>  
> +static bool intel_fb_restore_fbdev_mode(struct drm_fb_helper *helper)
> +{
> +	struct drm_device *dev = helper->dev;
> +	bool ret;
> +
> +	intel_disable_cursors_and_sprites(dev);
> +
> +	ret = drm_fb_helper_restore_fbdev_mode(helper);
> +	if (ret)
> +		DRM_DEBUG("failed to restore crtc mode\n");

Just return without the DBG here, and keep the DBG up a level where we
discard the error code.

> +
> +	return ret;
> +}
> +
>  static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
>  	.gamma_set = intel_crtc_fb_gamma_set,
>  	.gamma_get = intel_crtc_fb_gamma_get,
>  	.fb_probe = intelfb_create,
> -	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
> +	.restore_fbdev_mode = intel_fb_restore_fbdev_mode,
>  };
>  
>  static void intel_fbdev_destroy(struct drm_device *dev,
> @@ -291,7 +305,6 @@ void intel_fb_output_poll_changed(struct drm_device *dev)
>  
>  void intel_fb_restore_mode(struct drm_device *dev)
>  {
> -	int ret;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
>  
>  	if (INTEL_INFO(dev)->num_pipes == 0)
> @@ -299,11 +312,7 @@ void intel_fb_restore_mode(struct drm_device *dev)
>  
>  	drm_modeset_lock_all(dev);
>  
> -	intel_disable_cursors_and_sprites(dev);
> -
> -	ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper);
> -	if (ret)
> -		DRM_DEBUG("failed to restore crtc mode\n");
As above, I think this is the better location for the DBG.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH 5/7] drm/i915: Use container_of() in the fbdev code
  2013-05-31 17:07 ` [PATCH 5/7] drm/i915: Use container_of() in the fbdev code ville.syrjala
@ 2013-06-03 10:08   ` Chris Wilson
  2013-06-03 12:01     ` Daniel Vetter
  0 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2013-06-03 10:08 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, dri-devel

On Fri, May 31, 2013 at 08:07:05PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use container_of() instead of a cast to get struct intel_fbdev
> from struct drm_fb_helper.
> 
> Also populate the fb_info->par correctly with the drm_fb_helper pointer
> instead of the intel_fbdev pointer.
> 
> There's no actual functional change since the drm_fb_helper happens to
> be the first member inside intel_fbdev.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH 6/7] drm/i915: s/drm_i915_private_t/struct drm_i915_private/
  2013-05-31 17:07 ` [PATCH 6/7] drm/i915: s/drm_i915_private_t/struct drm_i915_private/ ville.syrjala
@ 2013-06-03 10:09   ` Chris Wilson
  2013-06-03 12:01     ` [Intel-gfx] " Daniel Vetter
  0 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2013-06-03 10:09 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, dri-devel

On Fri, May 31, 2013 at 08:07:06PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> People don't like typedefs these days. Eliminate their use from intel_fb.c.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH v2 3/7] drm/i915: Fix fbdev sprite disable code
  2013-05-31 17:07 ` [PATCH v2 3/7] drm/i915: Fix fbdev sprite disable code ville.syrjala
  2013-06-03  9:53   ` Chris Wilson
@ 2013-06-03 12:00   ` Daniel Vetter
  2013-06-03 12:44     ` [Intel-gfx] " Ville Syrjälä
  1 sibling, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2013-06-03 12:00 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, dri-devel

On Fri, May 31, 2013 at 08:07:03PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> plane->enabled is never set, so this code didn't do anything.
> 
> Fix the code for sprites by calling the new drm_plane_force_disable()
> function. That means the plane will remain off until someone explicitly
> turns it back on.
> 
> And do the same for cursors and the old video overlays, since we only
> want to see the primary plane for fbdev.
> 
> v2: Disable sprites/cursors until explicitly re-enabled
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_fb.c      |  9 ++-------
>  3 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 944b6d5..1d1a3fd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9842,3 +9842,19 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m,
>  	}
>  }
>  #endif
> +
> +void intel_disable_cursors_and_sprites(struct drm_device *dev)
> +{
> +	struct drm_crtc *crtc;
> +	struct drm_plane *plane;
> +
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		intel_crtc_dpms_overlay(to_intel_crtc(crtc), false);
> +		intel_crtc_cursor_set(crtc, NULL, 0, 0, 0);
> +	}

I still think we should just move this into drm_fb_helper_restore_fbdev_mode:
- Cursors can be killed with crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0);
  Or at least we should be able to do so, I've done a quick audit of all
  drivers and none really cares about file_priv if the handle is 0. Of
  course we should put that into a drm_cursor_force_disable helper to
  prepare for a better world (atomic modeset and all ...).

  vmwgfx needs to be fixed up slightly since it derefences file_priv
  outside of a handle != 0 check, but that one can be trivially fixed for
  both cursor_set callbacks by moving the driver_private dereference into
  the if block.

- Legacy overlay stuff has been broken like that forever, and at least on
  i8xx those are used much more often. Imo whoever's offended by gen2
  being a bit broken can write a drm plane wrapper for the overlay stuff
  (or better, move the code to drm planes and convert the ioctl to be a
  shim around the real drm plane interfaces). I don't think we should add
  hacks for that.

Cheers, Daniel

> +
> +	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
> +		if (plane->fb)
> +			drm_plane_force_disable(plane);
> +	}
> +}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d0a8c1a..f8e76cf 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -800,5 +800,6 @@ extern bool intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev,
>  extern bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
>  						 enum transcoder pch_transcoder,
>  						 bool enable);
> +extern void intel_disable_cursors_and_sprites(struct drm_device *dev);
>  
>  #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index 5fe525a..b34ccf3 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -293,22 +293,17 @@ void intel_fb_restore_mode(struct drm_device *dev)
>  {
>  	int ret;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> -	struct drm_mode_config *config = &dev->mode_config;
> -	struct drm_plane *plane;
>  
>  	if (INTEL_INFO(dev)->num_pipes == 0)
>  		return;
>  
>  	drm_modeset_lock_all(dev);
>  
> +	intel_disable_cursors_and_sprites(dev);
> +
>  	ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper);
>  	if (ret)
>  		DRM_DEBUG("failed to restore crtc mode\n");
>  
> -	/* Be sure to shut off any planes that may be active */
> -	list_for_each_entry(plane, &config->plane_list, head)
> -		if (plane->enabled)
> -			plane->funcs->disable_plane(plane);
> -
>  	drm_modeset_unlock_all(dev);
>  }
> -- 
> 1.8.1.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

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

* Re: [Intel-gfx] [PATCH 6/7] drm/i915: s/drm_i915_private_t/struct drm_i915_private/
  2013-06-03 10:09   ` Chris Wilson
@ 2013-06-03 12:01     ` Daniel Vetter
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2013-06-03 12:01 UTC (permalink / raw)
  To: Chris Wilson, ville.syrjala, intel-gfx, dri-devel

On Mon, Jun 03, 2013 at 11:09:00AM +0100, Chris Wilson wrote:
> On Fri, May 31, 2013 at 08:07:06PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > People don't like typedefs these days. Eliminate their use from intel_fb.c.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

* Re: [PATCH 5/7] drm/i915: Use container_of() in the fbdev code
  2013-06-03 10:08   ` Chris Wilson
@ 2013-06-03 12:01     ` Daniel Vetter
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2013-06-03 12:01 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, dri-devel

On Mon, Jun 03, 2013 at 11:08:30AM +0100, Chris Wilson wrote:
> On Fri, May 31, 2013 at 08:07:05PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Use container_of() instead of a cast to get struct intel_fbdev
> > from struct drm_fb_helper.
> > 
> > Also populate the fb_info->par correctly with the drm_fb_helper pointer
> > instead of the intel_fbdev pointer.
> > 
> > There's no actual functional change since the drm_fb_helper happens to
> > be the first member inside intel_fbdev.
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

* Re: [Intel-gfx] [PATCH v2 3/7] drm/i915: Fix fbdev sprite disable code
  2013-06-03 12:00   ` Daniel Vetter
@ 2013-06-03 12:44     ` Ville Syrjälä
  0 siblings, 0 replies; 17+ messages in thread
From: Ville Syrjälä @ 2013-06-03 12:44 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, dri-devel

On Mon, Jun 03, 2013 at 02:00:00PM +0200, Daniel Vetter wrote:
> On Fri, May 31, 2013 at 08:07:03PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > plane->enabled is never set, so this code didn't do anything.
> > 
> > Fix the code for sprites by calling the new drm_plane_force_disable()
> > function. That means the plane will remain off until someone explicitly
> > turns it back on.
> > 
> > And do the same for cursors and the old video overlays, since we only
> > want to see the primary plane for fbdev.
> > 
> > v2: Disable sprites/cursors until explicitly re-enabled
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h     |  1 +
> >  drivers/gpu/drm/i915/intel_fb.c      |  9 ++-------
> >  3 files changed, 19 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 944b6d5..1d1a3fd 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9842,3 +9842,19 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m,
> >  	}
> >  }
> >  #endif
> > +
> > +void intel_disable_cursors_and_sprites(struct drm_device *dev)
> > +{
> > +	struct drm_crtc *crtc;
> > +	struct drm_plane *plane;
> > +
> > +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> > +		intel_crtc_dpms_overlay(to_intel_crtc(crtc), false);
> > +		intel_crtc_cursor_set(crtc, NULL, 0, 0, 0);
> > +	}
> 
> I still think we should just move this into drm_fb_helper_restore_fbdev_mode:
> - Cursors can be killed with crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0);
>   Or at least we should be able to do so, I've done a quick audit of all
>   drivers and none really cares about file_priv if the handle is 0. Of
>   course we should put that into a drm_cursor_force_disable helper to
>   prepare for a better world (atomic modeset and all ...).
> 
>   vmwgfx needs to be fixed up slightly since it derefences file_priv
>   outside of a handle != 0 check, but that one can be trivially fixed for
>   both cursor_set callbacks by moving the driver_private dereference into
>   the if block.
> 
> - Legacy overlay stuff has been broken like that forever, and at least on
>   i8xx those are used much more often. Imo whoever's offended by gen2
>   being a bit broken can write a drm plane wrapper for the overlay stuff
>   (or better, move the code to drm planes and convert the ioctl to be a
>   shim around the real drm plane interfaces). I don't think we should add
>   hacks for that.

OK I give up. I did a quick check of the private plane stuff, and it
turns out they're not on plane_list, so I think just walking the list
and disabling everything there should be OK. Since you're happy w/
leaving the video overlay out in the cold for now, I'll shovel it all
into drm_fb_helper...


-- 
Ville Syrjälä
Intel OTC

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

end of thread, other threads:[~2013-06-03 12:44 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-31 17:07 [PATCH 0/7] drm/i915: fbdev mode restoration improvements v2 ville.syrjala
2013-05-31 17:07 ` [PATCH v2 1/7] drm: Add fb_helper->restore_fbdev_mode hook ville.syrjala
2013-06-03  9:50   ` Chris Wilson
2013-05-31 17:07 ` [PATCH 2/7] drm: Add drm_plane_force_disable() ville.syrjala
2013-05-31 17:07 ` [PATCH v2 3/7] drm/i915: Fix fbdev sprite disable code ville.syrjala
2013-06-03  9:53   ` Chris Wilson
2013-06-03 12:00   ` Daniel Vetter
2013-06-03 12:44     ` [Intel-gfx] " Ville Syrjälä
2013-05-31 17:07 ` [PATCH v2 4/7] drm/i915: Use a custom restore_fbdev_mode hook ville.syrjala
2013-06-03 10:08   ` Chris Wilson
2013-05-31 17:07 ` [PATCH 5/7] drm/i915: Use container_of() in the fbdev code ville.syrjala
2013-06-03 10:08   ` Chris Wilson
2013-06-03 12:01     ` Daniel Vetter
2013-05-31 17:07 ` [PATCH 6/7] drm/i915: s/drm_i915_private_t/struct drm_i915_private/ ville.syrjala
2013-06-03 10:09   ` Chris Wilson
2013-06-03 12:01     ` [Intel-gfx] " Daniel Vetter
2013-05-31 17:07 ` [PATCH v2 7/7] drm: Remove some unused stuff from drm_plane ville.syrjala

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.