[PATCH v2 00/15] drm/fb-helper: Add .last_close and .output_poll_changed helpers

[PATCH v2 00/15] drm/fb-helper: Add .last_close and .output_poll_changed helpers

[Noralf Trønnes]

This patchset adds fbdev .last_close and .output_poll_changed helpers to
reduce fbdev emulation footprint in drivers.

I don't know which drivers have their own tree or not, so if you want me
to apply your patch to drm-misc, please let me know.

I will do a separate patchset for the cma helper drivers.

Noralf.

Changes since version 1:
- drm_device.drm_fb_helper_private -> drm_device.fb_helper (Ville)

Noralf Trønnes (15):
  drm/fb-helper: Handle function NULL argument
  drm: Add drm_device->fb_helper pointer
  drm/fb-helper: Add .last_close and .output_poll_changed helpers
  drm/amdgpu: Use drm_fb_helper_lastclose() and _poll_changed()
  drm/armada: Use drm_fb_helper_lastclose() and _poll_changed()
  drm/exynos: Use drm_fb_helper_lastclose() and _poll_changed()
  drm/gma500: Use drm_fb_helper_lastclose() and _poll_changed()
  drm/i915: Use drm_fb_helper_output_poll_changed()
  drm/msm: Use drm_fb_helper_lastclose() and _poll_changed()
  drm/nouveau: Use drm_fb_helper_output_poll_changed()
  drm/omap: Use drm_fb_helper_lastclose() and _poll_changed()
  drm/radeon: Use drm_fb_helper_lastclose() and _poll_changed()
  drm/rockchip: Use drm_fb_helper_lastclose() and _poll_changed()
  drm/tegra: Use drm_fb_helper_lastclose() and _poll_changed()
  staging: vboxvideo: Use drm_fb_helper_lastclose()

 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  9 +---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c      | 27 -----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c     |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h    |  4 --
 drivers/gpu/drm/armada/armada_drm.h         |  1 -
 drivers/gpu/drm/armada/armada_drv.c         |  8 +---
 drivers/gpu/drm/armada/armada_fb.c          | 11 +----
 drivers/gpu/drm/armada/armada_fbdev.c       |  8 ----
 drivers/gpu/drm/drm_fb_helper.c             | 69 +++++++++++++++++++++++------
 drivers/gpu/drm/exynos/exynos_drm_drv.c     |  8 +---
 drivers/gpu/drm/exynos/exynos_drm_fb.c      |  2 +-
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c   | 18 --------
 drivers/gpu/drm/exynos/exynos_drm_fbdev.h   |  2 -
 drivers/gpu/drm/gma500/framebuffer.c        |  9 +---
 drivers/gpu/drm/gma500/psb_drv.c            | 15 +------
 drivers/gpu/drm/i915/intel_display.c        |  2 +-
 drivers/gpu/drm/i915/intel_drv.h            |  5 ---
 drivers/gpu/drm/i915/intel_fbdev.c          |  8 ----
 drivers/gpu/drm/msm/msm_drv.c               | 18 +-------
 drivers/gpu/drm/nouveau/nouveau_display.c   |  3 +-
 drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  8 ----
 drivers/gpu/drm/nouveau/nouveau_fbcon.h     |  2 -
 drivers/gpu/drm/nouveau/nouveau_vga.c       |  3 +-
 drivers/gpu/drm/nouveau/nv50_display.c      |  2 +-
 drivers/gpu/drm/omapdrm/omap_drv.c          | 34 +-------------
 drivers/gpu/drm/radeon/radeon_display.c     |  9 +---
 drivers/gpu/drm/radeon/radeon_fb.c          | 22 ---------
 drivers/gpu/drm/radeon/radeon_kms.c         |  5 +--
 drivers/gpu/drm/radeon/radeon_mode.h        |  3 --
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c |  9 +---
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  9 +---
 drivers/gpu/drm/tegra/drm.c                 | 13 +-----
 drivers/gpu/drm/tegra/drm.h                 |  4 --
 drivers/gpu/drm/tegra/fb.c                  | 14 ------
 drivers/staging/vboxvideo/vbox_drv.c        |  2 +-
 drivers/staging/vboxvideo/vbox_drv.h        |  1 -
 drivers/staging/vboxvideo/vbox_main.c       | 12 -----
 include/drm/drm_device.h                    |  9 ++++
 include/drm/drm_fb_helper.h                 | 11 +++++
 39 files changed, 106 insertions(+), 297 deletions(-)

-- 
2.14.2

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

[PATCH v2 01/15] drm/fb-helper: Handle function NULL argument

[Noralf Trønnes]

Make functions tolerate that the drm_fb_helper argument is NULL.
This is useful for drivers that continue probing when fbdev emulation
fails and not having to do this check themselves.
Update docs for functions that already handles this.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_fb_helper.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 116d1f1337c7..954cdd48de92 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -150,6 +150,9 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
 {
 	int err;
 
+	if (!fb_helper)
+		return 0;
+
 	mutex_lock(&fb_helper->lock);
 	err = __drm_fb_helper_add_one_connector(fb_helper, connector);
 	mutex_unlock(&fb_helper->lock);
@@ -161,7 +164,7 @@ EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
 /**
  * drm_fb_helper_single_add_all_connectors() - add all connectors to fbdev
  * 					       emulation helper
- * @fb_helper: fbdev initialized with drm_fb_helper_init
+ * @fb_helper: fbdev initialized with drm_fb_helper_init, can be NULL
  *
  * This functions adds all the available connectors for use with the given
  * fb_helper. This is a separate step to allow drivers to freely assign
@@ -179,7 +182,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
 	struct drm_connector_list_iter conn_iter;
 	int i, ret = 0;
 
-	if (!drm_fbdev_emulation)
+	if (!drm_fbdev_emulation || !fb_helper)
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
@@ -245,6 +248,9 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 {
 	int err;
 
+	if (!fb_helper)
+		return 0;
+
 	mutex_lock(&fb_helper->lock);
 	err = __drm_fb_helper_remove_one_connector(fb_helper, connector);
 	mutex_unlock(&fb_helper->lock);
@@ -484,7 +490,7 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
 
 /**
  * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
- * @fb_helper: fbcon to restore
+ * @fb_helper: driver-allocated fbdev helper, can be NULL
  *
  * This should be called from driver's drm &drm_driver.lastclose callback
  * when implementing an fbcon on top of kms using this helper. This ensures that
@@ -498,7 +504,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 	bool do_delayed;
 	int ret;
 
-	if (!drm_fbdev_emulation)
+	if (!drm_fbdev_emulation || !fb_helper)
 		return -ENODEV;
 
 	if (READ_ONCE(fb_helper->deferred_setup))
@@ -883,7 +889,7 @@ EXPORT_SYMBOL(drm_fb_helper_alloc_fbi);
 
 /**
  * drm_fb_helper_unregister_fbi - unregister fb_info framebuffer device
- * @fb_helper: driver-allocated fbdev helper
+ * @fb_helper: driver-allocated fbdev helper, can be NULL
  *
  * A wrapper around unregister_framebuffer, to release the fb_info
  * framebuffer device. This must be called before releasing all resources for
@@ -898,7 +904,7 @@ EXPORT_SYMBOL(drm_fb_helper_unregister_fbi);
 
 /**
  * drm_fb_helper_fini - finialize a &struct drm_fb_helper
- * @fb_helper: driver-allocated fbdev helper
+ * @fb_helper: driver-allocated fbdev helper, can be NULL
  *
  * This cleans up all remaining resources associated with @fb_helper. Must be
  * called after drm_fb_helper_unlink_fbi() was called.
@@ -937,7 +943,7 @@ EXPORT_SYMBOL(drm_fb_helper_fini);
 
 /**
  * drm_fb_helper_unlink_fbi - wrapper around unlink_framebuffer
- * @fb_helper: driver-allocated fbdev helper
+ * @fb_helper: driver-allocated fbdev helper, can be NULL
  *
  * A wrapper around unlink_framebuffer implemented by fbdev core
  */
@@ -1138,7 +1144,7 @@ EXPORT_SYMBOL(drm_fb_helper_cfb_imageblit);
 
 /**
  * drm_fb_helper_set_suspend - wrapper around fb_set_suspend
- * @fb_helper: driver-allocated fbdev helper
+ * @fb_helper: driver-allocated fbdev helper, can be NULL
  * @suspend: whether to suspend or resume
  *
  * A wrapper around fb_set_suspend implemented by fbdev core.
@@ -1155,7 +1161,7 @@ EXPORT_SYMBOL(drm_fb_helper_set_suspend);
 /**
  * drm_fb_helper_set_suspend_unlocked - wrapper around fb_set_suspend that also
  *                                      takes the console lock
- * @fb_helper: driver-allocated fbdev helper
+ * @fb_helper: driver-allocated fbdev helper, can be NULL
  * @suspend: whether to suspend or resume
  *
  * A wrapper around fb_set_suspend() that takes the console lock. If the lock
@@ -2568,7 +2574,7 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
 /**
  * drm_fb_helper_hotplug_event - respond to a hotplug notification by
  *                               probing all the outputs attached to the fb
- * @fb_helper: the drm_fb_helper
+ * @fb_helper: driver-allocated fbdev helper, can be NULL
  *
  * Scan the connectors attached to the fb_helper and try to put together a
  * setup after notification of a change in output configuration.
@@ -2590,7 +2596,7 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 {
 	int err = 0;
 
-	if (!drm_fbdev_emulation)
+	if (!drm_fbdev_emulation || !fb_helper)
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
-- 
2.14.2

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

[PATCH v2 02/15] drm: Add drm_device->fb_helper pointer

[Noralf Trønnes]

drm_fb_helper is *the* way of doing fbdev emulation so add a pointer to
struct drm_device. This makes it possible to add callback helpers for
.last_close and .output_poll_changed further reducing fbdev emulation
footprint in drivers. The pointer is set by drm_fb_helper_init() and
cleared by drm_fb_helper_fini().

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_fb_helper.c | 13 +++++++++++--
 include/drm/drm_device.h        |  9 +++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 954cdd48de92..19aaca9fea06 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -799,8 +799,10 @@ int drm_fb_helper_init(struct drm_device *dev,
 	struct drm_mode_config *config = &dev->mode_config;
 	int i;
 
-	if (!drm_fbdev_emulation)
+	if (!drm_fbdev_emulation) {
+		dev->fb_helper = fb_helper;
 		return 0;
+	}
 
 	if (!max_conn_count)
 		return -EINVAL;
@@ -835,6 +837,8 @@ int drm_fb_helper_init(struct drm_device *dev,
 		i++;
 	}
 
+	dev->fb_helper = fb_helper;
+
 	return 0;
 out_free:
 	drm_fb_helper_crtc_free(fb_helper);
@@ -913,7 +917,12 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
 {
 	struct fb_info *info;
 
-	if (!drm_fbdev_emulation || !fb_helper)
+	if (!fb_helper)
+		return;
+
+	fb_helper->dev->fb_helper = NULL;
+
+	if (!drm_fbdev_emulation)
 		return;
 
 	cancel_work_sync(&fb_helper->resume_work);
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index e21af87a2f3c..7c4fa32f3fc6 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -17,6 +17,7 @@ struct drm_vblank_crtc;
 struct drm_sg_mem;
 struct drm_local_map;
 struct drm_vma_offset_manager;
+struct drm_fb_helper;
 
 struct inode;
 
@@ -185,6 +186,14 @@ struct drm_device {
 	struct drm_vma_offset_manager *vma_offset_manager;
 	/*@} */
 	int switch_power_state;
+
+	/**
+	 * @fb_helper:
+	 *
+	 * Pointer to the fbdev emulation structure.
+	 * Set by drm_fb_helper_init() and cleared by drm_fb_helper_fini().
+	 */
+	struct drm_fb_helper *fb_helper;
 };
 
 #endif
-- 
2.14.2

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

[PATCH v2 03/15] drm/fb-helper: Add .last_close and .output_poll_changed helpers

[Noralf Trønnes]

This adds helpers for the drm_driver->last_close and the
drm_mode_config_funcs->output_poll_changed callbacks.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_fb_helper.c | 28 ++++++++++++++++++++++++++++
 include/drm/drm_fb_helper.h     | 11 +++++++++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 19aaca9fea06..defd6a76fef3 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2633,6 +2633,34 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 }
 EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
 
+/**
+ * drm_fb_helper_lastclose - DRM driver lastclose helper for fbdev emulation
+ * @dev: DRM device
+ *
+ * This function can be used as the &drm_driver->lastclose callback for drivers
+ * that only need to call drm_fb_helper_restore_fbdev_mode_unlocked().
+ */
+void drm_fb_helper_lastclose(struct drm_device *dev)
+{
+	drm_fb_helper_restore_fbdev_mode_unlocked(dev->fb_helper);
+}
+EXPORT_SYMBOL(drm_fb_helper_lastclose);
+
+/**
+ * drm_fb_helper_output_poll_changed - DRM mode config \.output_poll_changed
+ *                                     helper for fbdev emulation
+ * @dev: DRM device
+ *
+ * This function can be used as the
+ * &drm_mode_config_funcs.output_poll_changed callback for drivers that only
+ * need to call drm_fb_helper_hotplug_event().
+ */
+void drm_fb_helper_output_poll_changed(struct drm_device *dev)
+{
+	drm_fb_helper_hotplug_event(dev->fb_helper);
+}
+EXPORT_SYMBOL(drm_fb_helper_output_poll_changed);
+
 /* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
  * but the module doesn't depend on any fb console symbols.  At least
  * attempt to load fbcon to avoid leaving the system without a usable console.
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 33fe95927742..877e5b395c02 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -310,6 +310,9 @@ drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn);
 int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector);
 int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 				       struct drm_connector *connector);
+
+void drm_fb_helper_lastclose(struct drm_device *dev);
+void drm_fb_helper_output_poll_changed(struct drm_device *dev);
 #else
 static inline void drm_fb_helper_prepare(struct drm_device *dev,
 					struct drm_fb_helper *helper,
@@ -507,6 +510,14 @@ drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 	return 0;
 }
 
+static inline void drm_fb_helper_lastclose(struct drm_device *dev)
+{
+}
+
+static inline void drm_fb_helper_output_poll_changed(struct drm_device *dev)
+{
+}
+
 #endif
 
 static inline int
-- 
2.14.2

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

[PATCH v2 04/15] drm/amdgpu: Use drm_fb_helper_lastclose() and _poll_changed()

[Noralf Trønnes]

This driver can use drm_fb_helper_lastclose() in its .lastclose function.
It can also use drm_fb_helper_output_poll_changed() as its
.output_poll_changed callback.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  9 ++-------
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c      | 27 ---------------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c     |  4 +---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h    |  4 ----
 4 files changed, 3 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 6ad243293a78..c41262865616 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -34,6 +34,7 @@
 #include <linux/pm_runtime.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_fb_helper.h>
 
 static void amdgpu_flip_callback(struct dma_fence *f, struct dma_fence_cb *cb)
 {
@@ -556,15 +557,9 @@ amdgpu_user_framebuffer_create(struct drm_device *dev,
 	return &amdgpu_fb->base;
 }
 
-static void amdgpu_output_poll_changed(struct drm_device *dev)
-{
-	struct amdgpu_device *adev = dev->dev_private;
-	amdgpu_fb_output_poll_changed(adev);
-}
-
 const struct drm_mode_config_funcs amdgpu_mode_funcs = {
 	.fb_create = amdgpu_user_framebuffer_create,
-	.output_poll_changed = amdgpu_output_poll_changed
+	.output_poll_changed = drm_fb_helper_output_poll_changed,
 };
 
 static const struct drm_prop_enum_list amdgpu_underscan_enum_list[] =
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index 562930b17a6d..e38bf856f48b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -288,12 +288,6 @@ static int amdgpufb_create(struct drm_fb_helper *helper,
 	return ret;
 }
 
-void amdgpu_fb_output_poll_changed(struct amdgpu_device *adev)
-{
-	if (adev->mode_info.rfbdev)
-		drm_fb_helper_hotplug_event(&adev->mode_info.rfbdev->helper);
-}
-
 static int amdgpu_fbdev_destroy(struct drm_device *dev, struct amdgpu_fbdev *rfbdev)
 {
 	struct amdgpu_framebuffer *rfb = &rfbdev->rfb;
@@ -397,24 +391,3 @@ bool amdgpu_fbdev_robj_is_fb(struct amdgpu_device *adev, struct amdgpu_bo *robj)
 		return true;
 	return false;
 }
-
-void amdgpu_fbdev_restore_mode(struct amdgpu_device *adev)
-{
-	struct amdgpu_fbdev *afbdev;
-	struct drm_fb_helper *fb_helper;
-	int ret;
-
-	if (!adev)
-		return;
-
-	afbdev = adev->mode_info.rfbdev;
-
-	if (!afbdev)
-		return;
-
-	fb_helper = &afbdev->helper;
-
-	ret = drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper);
-	if (ret)
-		DRM_DEBUG("failed to restore crtc mode\n");
-}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 4fd06f8d9768..db1e320be54a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -785,9 +785,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
  */
 void amdgpu_driver_lastclose_kms(struct drm_device *dev)
 {
-	struct amdgpu_device *adev = dev->dev_private;
-
-	amdgpu_fbdev_restore_mode(adev);
+	drm_fb_helper_lastclose(dev);
 	vga_switcheroo_process_delayed_switch();
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 2af2678ddaf6..9e9cd0e1c558 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -570,10 +570,6 @@ void amdgpu_fbdev_fini(struct amdgpu_device *adev);
 void amdgpu_fbdev_set_suspend(struct amdgpu_device *adev, int state);
 int amdgpu_fbdev_total_size(struct amdgpu_device *adev);
 bool amdgpu_fbdev_robj_is_fb(struct amdgpu_device *adev, struct amdgpu_bo *robj);
-void amdgpu_fbdev_restore_mode(struct amdgpu_device *adev);
-
-void amdgpu_fb_output_poll_changed(struct amdgpu_device *adev);
-
 
 int amdgpu_align_pitch(struct amdgpu_device *adev, int width, int bpp, bool tiled);
 
-- 
2.14.2

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

[PATCH v2 05/15] drm/armada: Use drm_fb_helper_lastclose() and _poll_changed()

[Noralf Trønnes]

This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
It can also use drm_fb_helper_output_poll_changed() as its
.output_poll_changed callback.

Cc: Russell King <linux@armlinux.org.uk>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/gpu/drm/armada/armada_drm.h   |  1 -
 drivers/gpu/drm/armada/armada_drv.c   |  8 ++------
 drivers/gpu/drm/armada/armada_fb.c    | 11 +----------
 drivers/gpu/drm/armada/armada_fbdev.c |  8 --------
 4 files changed, 3 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_drm.h b/drivers/gpu/drm/armada/armada_drm.h
index b064879ecdbd..cc4c557c9f66 100644
--- a/drivers/gpu/drm/armada/armada_drm.h
+++ b/drivers/gpu/drm/armada/armada_drm.h
@@ -84,7 +84,6 @@ void armada_drm_queue_unref_work(struct drm_device *,
 extern const struct drm_mode_config_funcs armada_drm_mode_config_funcs;
 
 int armada_fbdev_init(struct drm_device *);
-void armada_fbdev_lastclose(struct drm_device *);
 void armada_fbdev_fini(struct drm_device *);
 
 int armada_overlay_plane_create(struct drm_device *, unsigned long);
diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index e857b88a9799..4b11b6b52f1d 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -10,6 +10,7 @@
 #include <linux/module.h>
 #include <linux/of_graph.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_of.h>
 #include "armada_crtc.h"
 #include "armada_drm.h"
@@ -54,15 +55,10 @@ static struct drm_ioctl_desc armada_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(ARMADA_GEM_PWRITE, armada_gem_pwrite_ioctl, 0),
 };
 
-static void armada_drm_lastclose(struct drm_device *dev)
-{
-	armada_fbdev_lastclose(dev);
-}
-
 DEFINE_DRM_GEM_FOPS(armada_drm_fops);
 
 static struct drm_driver armada_drm_driver = {
-	.lastclose		= armada_drm_lastclose,
+	.lastclose		= drm_fb_helper_lastclose,
 	.gem_free_object_unlocked = armada_gem_free_object,
 	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
 	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
diff --git a/drivers/gpu/drm/armada/armada_fb.c b/drivers/gpu/drm/armada/armada_fb.c
index a38d5a0892a9..ac92bce07ecd 100644
--- a/drivers/gpu/drm/armada/armada_fb.c
+++ b/drivers/gpu/drm/armada/armada_fb.c
@@ -154,16 +154,7 @@ static struct drm_framebuffer *armada_fb_create(struct drm_device *dev,
 	return ERR_PTR(ret);
 }
 
-static void armada_output_poll_changed(struct drm_device *dev)
-{
-	struct armada_private *priv = dev->dev_private;
-	struct drm_fb_helper *fbh = priv->fbdev;
-
-	if (fbh)
-		drm_fb_helper_hotplug_event(fbh);
-}
-
 const struct drm_mode_config_funcs armada_drm_mode_config_funcs = {
 	.fb_create		= armada_fb_create,
-	.output_poll_changed	= armada_output_poll_changed,
+	.output_poll_changed	= drm_fb_helper_output_poll_changed,
 };
diff --git a/drivers/gpu/drm/armada/armada_fbdev.c b/drivers/gpu/drm/armada/armada_fbdev.c
index a2ce83f84800..2a59db0994b2 100644
--- a/drivers/gpu/drm/armada/armada_fbdev.c
+++ b/drivers/gpu/drm/armada/armada_fbdev.c
@@ -159,14 +159,6 @@ int armada_fbdev_init(struct drm_device *dev)
 	return ret;
 }
 
-void armada_fbdev_lastclose(struct drm_device *dev)
-{
-	struct armada_private *priv = dev->dev_private;
-
-	if (priv->fbdev)
-		drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev);
-}
-
 void armada_fbdev_fini(struct drm_device *dev)
 {
 	struct armada_private *priv = dev->dev_private;
-- 
2.14.2

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

Re: [PATCH v2 05/15] drm/armada: Use drm_fb_helper_lastclose() and _poll_changed()

[Russell King - ARM Linux]

On Mon, Oct 30, 2017 at 04:39:41PM +0100, Noralf Trønnes wrote:
> -static void armada_output_poll_changed(struct drm_device *dev)
> -{
> -	struct armada_private *priv = dev->dev_private;
> -	struct drm_fb_helper *fbh = priv->fbdev;
> -
> -	if (fbh)
> -		drm_fb_helper_hotplug_event(fbh);
> -}
> -
>  const struct drm_mode_config_funcs armada_drm_mode_config_funcs = {
>  	.fb_create		= armada_fb_create,
> -	.output_poll_changed	= armada_output_poll_changed,
> +	.output_poll_changed	= drm_fb_helper_output_poll_changed,

I think this is fine, because although it gets rid of the NULL checks
for the drm_fb_helper structure, the driver will fail to initialise
unless priv->fbdev and the fb helper successfully initialises.

It would be nice to mention that in the commit message as to why
removing those NULL checks is safe.

In any case,

Acked-by: Russell King <rmk+kernel@armlinux.org.uk>

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 06/15] drm/exynos: Use drm_fb_helper_lastclose() and _poll_changed()

[Noralf Trønnes]

This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
It can also use drm_fb_helper_output_poll_changed() as its
.output_poll_changed callback.

Cc: Inki Dae <inki.dae@samsung.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.c   |  8 ++------
 drivers/gpu/drm/exynos/exynos_drm_fb.c    |  2 +-
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 18 ------------------
 drivers/gpu/drm/exynos/exynos_drm_fbdev.h |  2 --
 4 files changed, 3 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index e651a58c18cf..70f4895ac49c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -16,6 +16,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
 
 #include <linux/component.h>
 
@@ -89,11 +90,6 @@ static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
 	file->driver_priv = NULL;
 }
 
-static void exynos_drm_lastclose(struct drm_device *dev)
-{
-	exynos_drm_fbdev_restore_mode(dev);
-}
-
 static const struct vm_operations_struct exynos_drm_gem_vm_ops = {
 	.fault = exynos_drm_gem_fault,
 	.open = drm_gem_vm_open,
@@ -140,7 +136,7 @@ static struct drm_driver exynos_drm_driver = {
 	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME
 				  | DRIVER_ATOMIC | DRIVER_RENDER,
 	.open			= exynos_drm_open,
-	.lastclose		= exynos_drm_lastclose,
+	.lastclose		= drm_fb_helper_lastclose,
 	.postclose		= exynos_drm_postclose,
 	.gem_free_object_unlocked = exynos_drm_gem_free_object,
 	.gem_vm_ops		= &exynos_drm_gem_vm_ops,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index 8208df56a88f..0faaf829f5bf 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -205,7 +205,7 @@ static struct drm_mode_config_helper_funcs exynos_drm_mode_config_helpers = {
 
 static const struct drm_mode_config_funcs exynos_drm_mode_config_funcs = {
 	.fb_create = exynos_user_fb_create,
-	.output_poll_changed = exynos_drm_output_poll_changed,
+	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = exynos_atomic_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index dfb66ecf417b..132dd52d0ac7 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -270,24 +270,6 @@ void exynos_drm_fbdev_fini(struct drm_device *dev)
 	private->fb_helper = NULL;
 }
 
-void exynos_drm_fbdev_restore_mode(struct drm_device *dev)
-{
-	struct exynos_drm_private *private = dev->dev_private;
-
-	if (!private || !private->fb_helper)
-		return;
-
-	drm_fb_helper_restore_fbdev_mode_unlocked(private->fb_helper);
-}
-
-void exynos_drm_output_poll_changed(struct drm_device *dev)
-{
-	struct exynos_drm_private *private = dev->dev_private;
-	struct drm_fb_helper *fb_helper = private->fb_helper;
-
-	drm_fb_helper_hotplug_event(fb_helper);
-}
-
 void exynos_drm_fbdev_suspend(struct drm_device *dev)
 {
 	struct exynos_drm_private *private = dev->dev_private;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.h b/drivers/gpu/drm/exynos/exynos_drm_fbdev.h
index 645d1bb7f665..b33847223a85 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.h
@@ -19,8 +19,6 @@
 
 int exynos_drm_fbdev_init(struct drm_device *dev);
 void exynos_drm_fbdev_fini(struct drm_device *dev);
-void exynos_drm_fbdev_restore_mode(struct drm_device *dev);
-void exynos_drm_output_poll_changed(struct drm_device *dev);
 void exynos_drm_fbdev_suspend(struct drm_device *drm);
 void exynos_drm_fbdev_resume(struct drm_device *drm);
 
-- 
2.14.2

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

Re: [PATCH v2 06/15] drm/exynos: Use drm_fb_helper_lastclose() and _poll_changed()

[Daniel Vetter]

On Mon, Oct 30, 2017 at 04:39:42PM +0100, Noralf Trønnes wrote:
> This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
> It can also use drm_fb_helper_output_poll_changed() as its
> .output_poll_changed callback.
> 
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.c   |  8 ++------
>  drivers/gpu/drm/exynos/exynos_drm_fb.c    |  2 +-
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 18 ------------------
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.h |  2 --
>  4 files changed, 3 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index e651a58c18cf..70f4895ac49c 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -16,6 +16,7 @@
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
>  
>  #include <linux/component.h>
>  
> @@ -89,11 +90,6 @@ static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
>  	file->driver_priv = NULL;
>  }
>  
> -static void exynos_drm_lastclose(struct drm_device *dev)
> -{
> -	exynos_drm_fbdev_restore_mode(dev);
> -}
> -
>  static const struct vm_operations_struct exynos_drm_gem_vm_ops = {
>  	.fault = exynos_drm_gem_fault,
>  	.open = drm_gem_vm_open,
> @@ -140,7 +136,7 @@ static struct drm_driver exynos_drm_driver = {
>  	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME
>  				  | DRIVER_ATOMIC | DRIVER_RENDER,
>  	.open			= exynos_drm_open,
> -	.lastclose		= exynos_drm_lastclose,
> +	.lastclose		= drm_fb_helper_lastclose,
>  	.postclose		= exynos_drm_postclose,
>  	.gem_free_object_unlocked = exynos_drm_gem_free_object,
>  	.gem_vm_ops		= &exynos_drm_gem_vm_ops,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> index 8208df56a88f..0faaf829f5bf 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> @@ -205,7 +205,7 @@ static struct drm_mode_config_helper_funcs exynos_drm_mode_config_helpers = {
>  
>  static const struct drm_mode_config_funcs exynos_drm_mode_config_funcs = {
>  	.fb_create = exynos_user_fb_create,
> -	.output_poll_changed = exynos_drm_output_poll_changed,
> +	.output_poll_changed = drm_fb_helper_output_poll_changed,
>  	.atomic_check = exynos_atomic_check,
>  	.atomic_commit = drm_atomic_helper_commit,
>  };
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index dfb66ecf417b..132dd52d0ac7 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -270,24 +270,6 @@ void exynos_drm_fbdev_fini(struct drm_device *dev)
>  	private->fb_helper = NULL;
>  }
>  
> -void exynos_drm_fbdev_restore_mode(struct drm_device *dev)
> -{
> -	struct exynos_drm_private *private = dev->dev_private;
> -
> -	if (!private || !private->fb_helper)

Not sure this isn't risky, exynos has a strange load sequence ... Probably
best if we get an ack from Inki.
-Daniel
> -		return;
> -
> -	drm_fb_helper_restore_fbdev_mode_unlocked(private->fb_helper);
> -}
> -
> -void exynos_drm_output_poll_changed(struct drm_device *dev)
> -{
> -	struct exynos_drm_private *private = dev->dev_private;
> -	struct drm_fb_helper *fb_helper = private->fb_helper;
> -
> -	drm_fb_helper_hotplug_event(fb_helper);
> -}
> -
>  void exynos_drm_fbdev_suspend(struct drm_device *dev)
>  {
>  	struct exynos_drm_private *private = dev->dev_private;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.h b/drivers/gpu/drm/exynos/exynos_drm_fbdev.h
> index 645d1bb7f665..b33847223a85 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.h
> @@ -19,8 +19,6 @@
>  
>  int exynos_drm_fbdev_init(struct drm_device *dev);
>  void exynos_drm_fbdev_fini(struct drm_device *dev);
> -void exynos_drm_fbdev_restore_mode(struct drm_device *dev);
> -void exynos_drm_output_poll_changed(struct drm_device *dev);
>  void exynos_drm_fbdev_suspend(struct drm_device *drm);
>  void exynos_drm_fbdev_resume(struct drm_device *drm);
>  
> -- 
> 2.14.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 06/15] drm/exynos: Use drm_fb_helper_lastclose() and _poll_changed()

[Inki Dae]

2017년 10월 31일 19:28에 Daniel Vetter 이(가) 쓴 글:
> On Mon, Oct 30, 2017 at 04:39:42PM +0100, Noralf Trønnes wrote:
>> This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
>> It can also use drm_fb_helper_output_poll_changed() as its
>> .output_poll_changed callback.
>>
>> Cc: Inki Dae <inki.dae@samsung.com>
>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_drv.c   |  8 ++------
>>  drivers/gpu/drm/exynos/exynos_drm_fb.c    |  2 +-
>>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 18 ------------------
>>  drivers/gpu/drm/exynos/exynos_drm_fbdev.h |  2 --
>>  4 files changed, 3 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> index e651a58c18cf..70f4895ac49c 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> @@ -16,6 +16,7 @@
>>  #include <drm/drm_atomic.h>
>>  #include <drm/drm_atomic_helper.h>
>>  #include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_fb_helper.h>
>>  
>>  #include <linux/component.h>
>>  
>> @@ -89,11 +90,6 @@ static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
>>  	file->driver_priv = NULL;
>>  }
>>  
>> -static void exynos_drm_lastclose(struct drm_device *dev)
>> -{
>> -	exynos_drm_fbdev_restore_mode(dev);
>> -}
>> -
>>  static const struct vm_operations_struct exynos_drm_gem_vm_ops = {
>>  	.fault = exynos_drm_gem_fault,
>>  	.open = drm_gem_vm_open,
>> @@ -140,7 +136,7 @@ static struct drm_driver exynos_drm_driver = {
>>  	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME
>>  				  | DRIVER_ATOMIC | DRIVER_RENDER,
>>  	.open			= exynos_drm_open,
>> -	.lastclose		= exynos_drm_lastclose,
>> +	.lastclose		= drm_fb_helper_lastclose,
>>  	.postclose		= exynos_drm_postclose,
>>  	.gem_free_object_unlocked = exynos_drm_gem_free_object,
>>  	.gem_vm_ops		= &exynos_drm_gem_vm_ops,
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
>> index 8208df56a88f..0faaf829f5bf 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
>> @@ -205,7 +205,7 @@ static struct drm_mode_config_helper_funcs exynos_drm_mode_config_helpers = {
>>  
>>  static const struct drm_mode_config_funcs exynos_drm_mode_config_funcs = {
>>  	.fb_create = exynos_user_fb_create,
>> -	.output_poll_changed = exynos_drm_output_poll_changed,
>> +	.output_poll_changed = drm_fb_helper_output_poll_changed,
>>  	.atomic_check = exynos_atomic_check,
>>  	.atomic_commit = drm_atomic_helper_commit,
>>  };
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> index dfb66ecf417b..132dd52d0ac7 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> @@ -270,24 +270,6 @@ void exynos_drm_fbdev_fini(struct drm_device *dev)
>>  	private->fb_helper = NULL;
>>  }
>>  
>> -void exynos_drm_fbdev_restore_mode(struct drm_device *dev)
>> -{
>> -	struct exynos_drm_private *private = dev->dev_private;
>> -
>> -	if (!private || !private->fb_helper)
> 
> Not sure this isn't risky, exynos has a strange load sequence ... Probably
> best if we get an ack from Inki.

I didn't test this patch on real hardware due to below two issues,
1. many warning messages printed out at vblank period.
   - to finalize atomic flush drm_crtc_arm_vblank_event function is called but Exynos drm driver has no implementation of get_vblank_timestamp and get_scanout_position callbacks.
2. tranferring Panel commands to Panel device - s6e3ha2 Panel device - timed out when exynos_drm_fbdev_restore_mode is called. 

So I just looked into this patch and looks good to me.
Only a difference between old and new helper functions is above condition - checking if private and private->fb_helper are null or not.
And dev->dev_private and private->fb_helper are cleared after calling drm_dev_unregister function which calls drm_fb_helpaer_lastclose function so it must be no problem.

Acked-by: Inki Dae <inki.dae@samsung.com>

Thanks,
Inki Dae

> -Daniel
>> -		return;
>> -
>> -	drm_fb_helper_restore_fbdev_mode_unlocked(private->fb_helper);
>> -}
>> -
>> -void exynos_drm_output_poll_changed(struct drm_device *dev)
>> -{
>> -	struct exynos_drm_private *private = dev->dev_private;
>> -	struct drm_fb_helper *fb_helper = private->fb_helper;
>> -
>> -	drm_fb_helper_hotplug_event(fb_helper);
>> -}
>> -
>>  void exynos_drm_fbdev_suspend(struct drm_device *dev)
>>  {
>>  	struct exynos_drm_private *private = dev->dev_private;
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.h b/drivers/gpu/drm/exynos/exynos_drm_fbdev.h
>> index 645d1bb7f665..b33847223a85 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.h
>> @@ -19,8 +19,6 @@
>>  
>>  int exynos_drm_fbdev_init(struct drm_device *dev);
>>  void exynos_drm_fbdev_fini(struct drm_device *dev);
>> -void exynos_drm_fbdev_restore_mode(struct drm_device *dev);
>> -void exynos_drm_output_poll_changed(struct drm_device *dev);
>>  void exynos_drm_fbdev_suspend(struct drm_device *drm);
>>  void exynos_drm_fbdev_resume(struct drm_device *drm);
>>  
>> -- 
>> 2.14.2
>>
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 07/15] drm/gma500: Use drm_fb_helper_lastclose() and _poll_changed()

[Noralf Trønnes]

This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
It can also use drm_fb_helper_output_poll_changed() as its
.output_poll_changed callback.

Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/gma500/framebuffer.c |  9 +--------
 drivers/gpu/drm/gma500/psb_drv.c     | 15 +--------------
 2 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index 2570c7f647a6..cb0a2ae916e0 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -576,13 +576,6 @@ static void psb_fbdev_fini(struct drm_device *dev)
 	dev_priv->fbdev = NULL;
 }
 
-static void psbfb_output_poll_changed(struct drm_device *dev)
-{
-	struct drm_psb_private *dev_priv = dev->dev_private;
-	struct psb_fbdev *fbdev = (struct psb_fbdev *)dev_priv->fbdev;
-	drm_fb_helper_hotplug_event(&fbdev->psb_fb_helper);
-}
-
 /**
  *	psb_user_framebuffer_create_handle - add hamdle to a framebuffer
  *	@fb: framebuffer
@@ -623,7 +616,7 @@ static void psb_user_framebuffer_destroy(struct drm_framebuffer *fb)
 
 static const struct drm_mode_config_funcs psb_mode_funcs = {
 	.fb_create = psb_user_framebuffer_create,
-	.output_poll_changed = psbfb_output_poll_changed,
+	.output_poll_changed = drm_fb_helper_output_poll_changed,
 };
 
 static void psb_setup_outputs(struct drm_device *dev)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 37a3be71acd9..7ab4c532f1d4 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -107,19 +107,6 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
 static const struct drm_ioctl_desc psb_ioctls[] = {
 };
 
-static void psb_driver_lastclose(struct drm_device *dev)
-{
-	int ret;
-	struct drm_psb_private *dev_priv = dev->dev_private;
-	struct psb_fbdev *fbdev = dev_priv->fbdev;
-
-	ret = drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev->psb_fb_helper);
-	if (ret)
-		DRM_DEBUG("failed to restore crtc mode\n");
-
-	return;
-}
-
 static int psb_do_init(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = dev->dev_private;
@@ -479,7 +466,7 @@ static struct drm_driver driver = {
 			   DRIVER_MODESET | DRIVER_GEM,
 	.load = psb_driver_load,
 	.unload = psb_driver_unload,
-	.lastclose = psb_driver_lastclose,
+	.lastclose = drm_fb_helper_lastclose,
 
 	.num_ioctls = ARRAY_SIZE(psb_ioctls),
 	.irq_preinstall = psb_irq_preinstall,
-- 
2.14.2

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

Re: [PATCH v2 07/15] drm/gma500: Use drm_fb_helper_lastclose() and _poll_changed()

[Daniel Vetter]

On Mon, Oct 30, 2017 at 04:39:43PM +0100, Noralf Trønnes wrote:
> This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
> It can also use drm_fb_helper_output_poll_changed() as its
> .output_poll_changed callback.
> 
> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/gma500/framebuffer.c |  9 +--------
>  drivers/gpu/drm/gma500/psb_drv.c     | 15 +--------------
>  2 files changed, 2 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
> index 2570c7f647a6..cb0a2ae916e0 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -576,13 +576,6 @@ static void psb_fbdev_fini(struct drm_device *dev)
>  	dev_priv->fbdev = NULL;
>  }
>  
> -static void psbfb_output_poll_changed(struct drm_device *dev)
> -{
> -	struct drm_psb_private *dev_priv = dev->dev_private;
> -	struct psb_fbdev *fbdev = (struct psb_fbdev *)dev_priv->fbdev;
> -	drm_fb_helper_hotplug_event(&fbdev->psb_fb_helper);
> -}
> -
>  /**
>   *	psb_user_framebuffer_create_handle - add hamdle to a framebuffer
>   *	@fb: framebuffer
> @@ -623,7 +616,7 @@ static void psb_user_framebuffer_destroy(struct drm_framebuffer *fb)
>  
>  static const struct drm_mode_config_funcs psb_mode_funcs = {
>  	.fb_create = psb_user_framebuffer_create,
> -	.output_poll_changed = psbfb_output_poll_changed,
> +	.output_poll_changed = drm_fb_helper_output_poll_changed,
>  };
>  
>  static void psb_setup_outputs(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
> index 37a3be71acd9..7ab4c532f1d4 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.c
> +++ b/drivers/gpu/drm/gma500/psb_drv.c
> @@ -107,19 +107,6 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
>  static const struct drm_ioctl_desc psb_ioctls[] = {
>  };
>  
> -static void psb_driver_lastclose(struct drm_device *dev)
> -{
> -	int ret;
> -	struct drm_psb_private *dev_priv = dev->dev_private;
> -	struct psb_fbdev *fbdev = dev_priv->fbdev;
> -
> -	ret = drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev->psb_fb_helper);
> -	if (ret)
> -		DRM_DEBUG("failed to restore crtc mode\n");
> -
> -	return;
> -}
> -
>  static int psb_do_init(struct drm_device *dev)
>  {
>  	struct drm_psb_private *dev_priv = dev->dev_private;
> @@ -479,7 +466,7 @@ static struct drm_driver driver = {
>  			   DRIVER_MODESET | DRIVER_GEM,
>  	.load = psb_driver_load,
>  	.unload = psb_driver_unload,
> -	.lastclose = psb_driver_lastclose,
> +	.lastclose = drm_fb_helper_lastclose,
>  
>  	.num_ioctls = ARRAY_SIZE(psb_ioctls),
>  	.irq_preinstall = psb_irq_preinstall,
> -- 
> 2.14.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 08/15] drm/i915: Use drm_fb_helper_output_poll_changed()

[Noralf Trønnes]

This driver can use drm_fb_helper_output_poll_changed() as its
.output_poll_changed callback.

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 drivers/gpu/drm/i915/intel_drv.h     | 5 -----
 drivers/gpu/drm/i915/intel_fbdev.c   | 8 --------
 3 files changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f780f39e0758..b205e2c782bb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14123,7 +14123,7 @@ static void intel_atomic_state_free(struct drm_atomic_state *state)
 static const struct drm_mode_config_funcs intel_mode_funcs = {
 	.fb_create = intel_user_framebuffer_create,
 	.get_format_info = intel_get_format_info,
-	.output_poll_changed = intel_fbdev_output_poll_changed,
+	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = intel_atomic_check,
 	.atomic_commit = intel_atomic_commit,
 	.atomic_state_alloc = intel_atomic_state_alloc,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 463ed152e6b1..dfcf5ba220e8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1607,7 +1607,6 @@ extern void intel_fbdev_initial_config_async(struct drm_device *dev);
 extern void intel_fbdev_unregister(struct drm_i915_private *dev_priv);
 extern void intel_fbdev_fini(struct drm_i915_private *dev_priv);
 extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
-extern void intel_fbdev_output_poll_changed(struct drm_device *dev);
 extern void intel_fbdev_restore_mode(struct drm_device *dev);
 #else
 static inline int intel_fbdev_init(struct drm_device *dev)
@@ -1631,10 +1630,6 @@ static inline void intel_fbdev_set_suspend(struct drm_device *dev, int state, bo
 {
 }
 
-static inline void intel_fbdev_output_poll_changed(struct drm_device *dev)
-{
-}
-
 static inline void intel_fbdev_restore_mode(struct drm_device *dev)
 {
 }
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index f2bb8116227c..35babbadfc5a 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -796,14 +796,6 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
 	console_unlock();
 }
 
-void intel_fbdev_output_poll_changed(struct drm_device *dev)
-{
-	struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
-
-	if (ifbdev)
-		drm_fb_helper_hotplug_event(&ifbdev->helper);
-}
-
 void intel_fbdev_restore_mode(struct drm_device *dev)
 {
 	struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
-- 
2.14.2

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

Re: [PATCH v2 08/15] drm/i915: Use drm_fb_helper_output_poll_changed()

[Daniel Vetter]

On Mon, Oct 30, 2017 at 04:39:44PM +0100, Noralf Trønnes wrote:
> This driver can use drm_fb_helper_output_poll_changed() as its
> .output_poll_changed callback.
> 
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  drivers/gpu/drm/i915/intel_drv.h     | 5 -----
>  drivers/gpu/drm/i915/intel_fbdev.c   | 8 --------
>  3 files changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f780f39e0758..b205e2c782bb 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14123,7 +14123,7 @@ static void intel_atomic_state_free(struct drm_atomic_state *state)
>  static const struct drm_mode_config_funcs intel_mode_funcs = {
>  	.fb_create = intel_user_framebuffer_create,
>  	.get_format_info = intel_get_format_info,
> -	.output_poll_changed = intel_fbdev_output_poll_changed,
> +	.output_poll_changed = drm_fb_helper_output_poll_changed,
>  	.atomic_check = intel_atomic_check,
>  	.atomic_commit = intel_atomic_commit,
>  	.atomic_state_alloc = intel_atomic_state_alloc,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 463ed152e6b1..dfcf5ba220e8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1607,7 +1607,6 @@ extern void intel_fbdev_initial_config_async(struct drm_device *dev);
>  extern void intel_fbdev_unregister(struct drm_i915_private *dev_priv);
>  extern void intel_fbdev_fini(struct drm_i915_private *dev_priv);
>  extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
> -extern void intel_fbdev_output_poll_changed(struct drm_device *dev);
>  extern void intel_fbdev_restore_mode(struct drm_device *dev);
>  #else
>  static inline int intel_fbdev_init(struct drm_device *dev)
> @@ -1631,10 +1630,6 @@ static inline void intel_fbdev_set_suspend(struct drm_device *dev, int state, bo
>  {
>  }
>  
> -static inline void intel_fbdev_output_poll_changed(struct drm_device *dev)
> -{
> -}
> -
>  static inline void intel_fbdev_restore_mode(struct drm_device *dev)
>  {
>  }
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index f2bb8116227c..35babbadfc5a 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -796,14 +796,6 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
>  	console_unlock();
>  }
>  
> -void intel_fbdev_output_poll_changed(struct drm_device *dev)
> -{
> -	struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
> -
> -	if (ifbdev)

This removes the NULL check here, and I think we still need that one for
some slightly unclear but probably hilarious reasons.

I guess simplest if you just drop the i915 patch as too tricky.
-Daniel

> -		drm_fb_helper_hotplug_event(&ifbdev->helper);
> -}
> -
>  void intel_fbdev_restore_mode(struct drm_device *dev)
>  {
>  	struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
> -- 
> 2.14.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 08/15] drm/i915: Use drm_fb_helper_output_poll_changed()

[Noralf Trønnes]

Den 31.10.2017 11.27, skrev Daniel Vetter:
> On Mon, Oct 30, 2017 at 04:39:44PM +0100, Noralf Trønnes wrote:
>> This driver can use drm_fb_helper_output_poll_changed() as its
>> .output_poll_changed callback.
>>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 2 +-
>>   drivers/gpu/drm/i915/intel_drv.h     | 5 -----
>>   drivers/gpu/drm/i915/intel_fbdev.c   | 8 --------
>>   3 files changed, 1 insertion(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index f780f39e0758..b205e2c782bb 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -14123,7 +14123,7 @@ static void intel_atomic_state_free(struct drm_atomic_state *state)
>>   static const struct drm_mode_config_funcs intel_mode_funcs = {
>>   	.fb_create = intel_user_framebuffer_create,
>>   	.get_format_info = intel_get_format_info,
>> -	.output_poll_changed = intel_fbdev_output_poll_changed,
>> +	.output_poll_changed = drm_fb_helper_output_poll_changed,
>>   	.atomic_check = intel_atomic_check,
>>   	.atomic_commit = intel_atomic_commit,
>>   	.atomic_state_alloc = intel_atomic_state_alloc,
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 463ed152e6b1..dfcf5ba220e8 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1607,7 +1607,6 @@ extern void intel_fbdev_initial_config_async(struct drm_device *dev);
>>   extern void intel_fbdev_unregister(struct drm_i915_private *dev_priv);
>>   extern void intel_fbdev_fini(struct drm_i915_private *dev_priv);
>>   extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
>> -extern void intel_fbdev_output_poll_changed(struct drm_device *dev);
>>   extern void intel_fbdev_restore_mode(struct drm_device *dev);
>>   #else
>>   static inline int intel_fbdev_init(struct drm_device *dev)
>> @@ -1631,10 +1630,6 @@ static inline void intel_fbdev_set_suspend(struct drm_device *dev, int state, bo
>>   {
>>   }
>>   
>> -static inline void intel_fbdev_output_poll_changed(struct drm_device *dev)
>> -{
>> -}
>> -
>>   static inline void intel_fbdev_restore_mode(struct drm_device *dev)
>>   {
>>   }
>> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
>> index f2bb8116227c..35babbadfc5a 100644
>> --- a/drivers/gpu/drm/i915/intel_fbdev.c
>> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
>> @@ -796,14 +796,6 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
>>   	console_unlock();
>>   }
>>   
>> -void intel_fbdev_output_poll_changed(struct drm_device *dev)
>> -{
>> -	struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
>> -
>> -	if (ifbdev)
> This removes the NULL check here, and I think we still need that one for
> some slightly unclear but probably hilarious reasons.
>
> I guess simplest if you just drop the i915 patch as too tricky.

Ok.

Several drivers had this NULL check and I did a verification in the drivers
fbdev code that conversion was OK. But I left it to the maintainer to know
about any code paths that wasn't obvious to me by looking at the fbdev code.

On the surface i915 looks OK and here's the rationale:

dev->fb_helper is set in drm_fb_helper_init() and cleared in 
drm_fb_helper_fini().
All fbdev init error paths I've seen call drm_fb_helper_fini().
This means that dev->fb_helper is only set when fbdev is initialized.

drm_fb_helper_output_poll_changed() can handle dev->fb_helper == NULL:

void drm_fb_helper_output_poll_changed(struct drm_device *dev)
{
     drm_fb_helper_hotplug_event(dev->fb_helper);
}

int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
{
...
     if (!drm_fbdev_emulation || !fb_helper)
          return 0;
...
}

dev->fb_helper and dev_priv->fbdev are set and cleared in sync:

int intel_fbdev_init(struct drm_device *dev)
{
     struct drm_i915_private *dev_priv = to_i915(dev);
     struct intel_fbdev *ifbdev;
     int ret;

     if (WARN_ON(INTEL_INFO(dev_priv)->num_pipes == 0))
         return -ENODEV;

     ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL);
     if (ifbdev == NULL)
         return -ENOMEM;

     drm_fb_helper_prepare(dev, &ifbdev->helper, &intel_fb_helper_funcs);

     if (!intel_fbdev_init_bios(dev, ifbdev))
         ifbdev->preferred_bpp = 32;

     ret = drm_fb_helper_init(dev, &ifbdev->helper, 4);
     if (ret) {
         kfree(ifbdev);
         return ret;
     }

dev->fb_helper is now set (by drm_fb_helper_init()).

     dev_priv->fbdev = ifbdev;

dev_priv->fbdev is now set

     INIT_WORK(&dev_priv->fbdev_suspend_work, intel_fbdev_suspend_worker);

     drm_fb_helper_single_add_all_connectors(&ifbdev->helper);

     return 0;
}

void intel_fbdev_fini(struct drm_i915_private *dev_priv)
{
     struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv->fbdev);

dev_priv->fbdev is now NULL

     if (!ifbdev)
         return;

     intel_fbdev_destroy(ifbdev);
}

static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
{
     /* We rely on the object-free to release the VMA pinning for
      * the info->screen_base mmaping. Leaking the VMA is simpler than
      * trying to rectify all the possible error paths leading here.
      */

     drm_fb_helper_fini(&ifbdev->helper);

dev->fb_helper is now NULL (cleared by drm_fb_helper_fini())

     if (ifbdev->vma) {
         mutex_lock(&ifbdev->helper.dev->struct_mutex);
         intel_unpin_fb_vma(ifbdev->vma);
         mutex_unlock(&ifbdev->helper.dev->struct_mutex);
     }

     if (ifbdev->fb)
         drm_framebuffer_remove(&ifbdev->fb->base);

     kfree(ifbdev);
}


But I understand that it's better to be safe than sorry :-)

Hopefully all your CI work will make checking patches like this a breeze 
one day.

Noralf.

> -Daniel
>
>> -		drm_fb_helper_hotplug_event(&ifbdev->helper);
>> -}
>> -
>>   void intel_fbdev_restore_mode(struct drm_device *dev)
>>   {
>>   	struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
>> -- 
>> 2.14.2
>>

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

Re: [PATCH v2 08/15] drm/i915: Use drm_fb_helper_output_poll_changed()

[Daniel Vetter]

On Tue, Oct 31, 2017 at 03:26:50PM +0100, Noralf Trønnes wrote:
> 
> Den 31.10.2017 11.27, skrev Daniel Vetter:
> > On Mon, Oct 30, 2017 at 04:39:44PM +0100, Noralf Trønnes wrote:
> > > This driver can use drm_fb_helper_output_poll_changed() as its
> > > .output_poll_changed callback.
> > > 
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > > ---
> > >   drivers/gpu/drm/i915/intel_display.c | 2 +-
> > >   drivers/gpu/drm/i915/intel_drv.h     | 5 -----
> > >   drivers/gpu/drm/i915/intel_fbdev.c   | 8 --------
> > >   3 files changed, 1 insertion(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index f780f39e0758..b205e2c782bb 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -14123,7 +14123,7 @@ static void intel_atomic_state_free(struct drm_atomic_state *state)
> > >   static const struct drm_mode_config_funcs intel_mode_funcs = {
> > >   	.fb_create = intel_user_framebuffer_create,
> > >   	.get_format_info = intel_get_format_info,
> > > -	.output_poll_changed = intel_fbdev_output_poll_changed,
> > > +	.output_poll_changed = drm_fb_helper_output_poll_changed,
> > >   	.atomic_check = intel_atomic_check,
> > >   	.atomic_commit = intel_atomic_commit,
> > >   	.atomic_state_alloc = intel_atomic_state_alloc,
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 463ed152e6b1..dfcf5ba220e8 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1607,7 +1607,6 @@ extern void intel_fbdev_initial_config_async(struct drm_device *dev);
> > >   extern void intel_fbdev_unregister(struct drm_i915_private *dev_priv);
> > >   extern void intel_fbdev_fini(struct drm_i915_private *dev_priv);
> > >   extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
> > > -extern void intel_fbdev_output_poll_changed(struct drm_device *dev);
> > >   extern void intel_fbdev_restore_mode(struct drm_device *dev);
> > >   #else
> > >   static inline int intel_fbdev_init(struct drm_device *dev)
> > > @@ -1631,10 +1630,6 @@ static inline void intel_fbdev_set_suspend(struct drm_device *dev, int state, bo
> > >   {
> > >   }
> > > -static inline void intel_fbdev_output_poll_changed(struct drm_device *dev)
> > > -{
> > > -}
> > > -
> > >   static inline void intel_fbdev_restore_mode(struct drm_device *dev)
> > >   {
> > >   }
> > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > > index f2bb8116227c..35babbadfc5a 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > @@ -796,14 +796,6 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
> > >   	console_unlock();
> > >   }
> > > -void intel_fbdev_output_poll_changed(struct drm_device *dev)
> > > -{
> > > -	struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
> > > -
> > > -	if (ifbdev)
> > This removes the NULL check here, and I think we still need that one for
> > some slightly unclear but probably hilarious reasons.
> > 
> > I guess simplest if you just drop the i915 patch as too tricky.
> 
> Ok.
> 
> Several drivers had this NULL check and I did a verification in the drivers
> fbdev code that conversion was OK. But I left it to the maintainer to know
> about any code paths that wasn't obvious to me by looking at the fbdev code.
> 
> On the surface i915 looks OK and here's the rationale:
> 
> dev->fb_helper is set in drm_fb_helper_init() and cleared in
> drm_fb_helper_fini().
> All fbdev init error paths I've seen call drm_fb_helper_fini().
> This means that dev->fb_helper is only set when fbdev is initialized.
> 
> drm_fb_helper_output_poll_changed() can handle dev->fb_helper == NULL:
> 
> void drm_fb_helper_output_poll_changed(struct drm_device *dev)
> {
>     drm_fb_helper_hotplug_event(dev->fb_helper);
> }
> 
> int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> {
> ...
>     if (!drm_fbdev_emulation || !fb_helper)
>          return 0;
> ...
> }

Argh, I totally missed that your patch series is adding these checks. So
looks all good, on the driver patches (except i915):

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

But probably good if you wait another week or so before pushing those.
Core bits imo should land right away, that also gives more options to
driver maintainers to push themselves.

> dev->fb_helper and dev_priv->fbdev are set and cleared in sync:
> 
> int intel_fbdev_init(struct drm_device *dev)
> {
>     struct drm_i915_private *dev_priv = to_i915(dev);
>     struct intel_fbdev *ifbdev;
>     int ret;
> 
>     if (WARN_ON(INTEL_INFO(dev_priv)->num_pipes == 0))
>         return -ENODEV;
> 
>     ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL);
>     if (ifbdev == NULL)
>         return -ENOMEM;
> 
>     drm_fb_helper_prepare(dev, &ifbdev->helper, &intel_fb_helper_funcs);
> 
>     if (!intel_fbdev_init_bios(dev, ifbdev))
>         ifbdev->preferred_bpp = 32;
> 
>     ret = drm_fb_helper_init(dev, &ifbdev->helper, 4);
>     if (ret) {
>         kfree(ifbdev);
>         return ret;
>     }
> 
> dev->fb_helper is now set (by drm_fb_helper_init()).
> 
>     dev_priv->fbdev = ifbdev;
> 
> dev_priv->fbdev is now set
> 
>     INIT_WORK(&dev_priv->fbdev_suspend_work, intel_fbdev_suspend_worker);
> 
>     drm_fb_helper_single_add_all_connectors(&ifbdev->helper);
> 
>     return 0;
> }
> 
> void intel_fbdev_fini(struct drm_i915_private *dev_priv)
> {
>     struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv->fbdev);
> 
> dev_priv->fbdev is now NULL
> 
>     if (!ifbdev)
>         return;
> 
>     intel_fbdev_destroy(ifbdev);
> }
> 
> static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
> {
>     /* We rely on the object-free to release the VMA pinning for
>      * the info->screen_base mmaping. Leaking the VMA is simpler than
>      * trying to rectify all the possible error paths leading here.
>      */
> 
>     drm_fb_helper_fini(&ifbdev->helper);
> 
> dev->fb_helper is now NULL (cleared by drm_fb_helper_fini())
> 
>     if (ifbdev->vma) {
>         mutex_lock(&ifbdev->helper.dev->struct_mutex);
>         intel_unpin_fb_vma(ifbdev->vma);
>         mutex_unlock(&ifbdev->helper.dev->struct_mutex);
>     }
> 
>     if (ifbdev->fb)
>         drm_framebuffer_remove(&ifbdev->fb->base);
> 
>     kfree(ifbdev);
> }
> 
> 
> But I understand that it's better to be safe than sorry :-)
> 
> Hopefully all your CI work will make checking patches like this a breeze one
> day.

Already works, just submit the i915-only patch (after you've merged the
core work into drm-misc-next and drm-tip is rebuilt) to the intel-gfx
mailing list, and CI will pick it up and stress test it for you.

Once that's done the i915 patch is also ready for merge, for that one:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Please include that tag when resubmitting.

Sorry for being a bit blind, I guess coffee didn't quite work yet :-)

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 09/15] drm/msm: Use drm_fb_helper_lastclose() and _poll_changed()

[Noralf Trønnes]

This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
It can also use drm_fb_helper_output_poll_changed() as its
.output_poll_changed callback.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/msm/msm_drv.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 606df7bea97b..1bddbbc6fd3e 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -34,16 +34,9 @@
 #define MSM_VERSION_MINOR	2
 #define MSM_VERSION_PATCHLEVEL	0
 
-static void msm_fb_output_poll_changed(struct drm_device *dev)
-{
-	struct msm_drm_private *priv = dev->dev_private;
-	if (priv->fbdev)
-		drm_fb_helper_hotplug_event(priv->fbdev);
-}
-
 static const struct drm_mode_config_funcs mode_config_funcs = {
 	.fb_create = msm_framebuffer_create,
-	.output_poll_changed = msm_fb_output_poll_changed,
+	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = msm_atomic_check,
 	.atomic_commit = msm_atomic_commit,
 	.atomic_state_alloc = msm_atomic_state_alloc,
@@ -545,13 +538,6 @@ static void msm_postclose(struct drm_device *dev, struct drm_file *file)
 	kfree(ctx);
 }
 
-static void msm_lastclose(struct drm_device *dev)
-{
-	struct msm_drm_private *priv = dev->dev_private;
-	if (priv->fbdev)
-		drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev);
-}
-
 static irqreturn_t msm_irq(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
@@ -825,7 +811,7 @@ static struct drm_driver msm_driver = {
 				DRIVER_MODESET,
 	.open               = msm_open,
 	.postclose           = msm_postclose,
-	.lastclose          = msm_lastclose,
+	.lastclose          = drm_fb_helper_lastclose,
 	.irq_handler        = msm_irq,
 	.irq_preinstall     = msm_irq_preinstall,
 	.irq_postinstall    = msm_irq_postinstall,
-- 
2.14.2

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

[PATCH v2 10/15] drm/nouveau: Use drm_fb_helper_output_poll_changed()

[Noralf Trønnes]

This driver can use drm_fb_helper_output_poll_changed() instead of
its own nouveau_fbcon_output_poll_changed().

Cc: Ben Skeggs <bskeggs@redhat.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/nouveau/nouveau_display.c | 3 ++-
 drivers/gpu/drm/nouveau/nouveau_fbcon.c   | 8 --------
 drivers/gpu/drm/nouveau/nouveau_fbcon.h   | 2 --
 drivers/gpu/drm/nouveau/nouveau_vga.c     | 3 ++-
 drivers/gpu/drm/nouveau/nv50_display.c    | 2 +-
 5 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 2e7785f49e6d..009713404cc4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -29,6 +29,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
 
 #include <nvif/class.h>
 
@@ -292,7 +293,7 @@ nouveau_user_framebuffer_create(struct drm_device *dev,
 
 static const struct drm_mode_config_funcs nouveau_mode_config_funcs = {
 	.fb_create = nouveau_user_framebuffer_create,
-	.output_poll_changed = nouveau_fbcon_output_poll_changed,
+	.output_poll_changed = drm_fb_helper_output_poll_changed,
 };
 
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index f7707849bb53..60ca03c27ec4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -411,14 +411,6 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,
 	return ret;
 }
 
-void
-nouveau_fbcon_output_poll_changed(struct drm_device *dev)
-{
-	struct nouveau_drm *drm = nouveau_drm(dev);
-	if (drm->fbcon)
-		drm_fb_helper_hotplug_event(&drm->fbcon->helper);
-}
-
 static int
 nouveau_fbcon_destroy(struct drm_device *dev, struct nouveau_fbdev *fbcon)
 {
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.h b/drivers/gpu/drm/nouveau/nouveau_fbcon.h
index e2bca729721e..a6f192ea3fa6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.h
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.h
@@ -68,8 +68,6 @@ void nouveau_fbcon_set_suspend(struct drm_device *dev, int state);
 void nouveau_fbcon_accel_save_disable(struct drm_device *dev);
 void nouveau_fbcon_accel_restore(struct drm_device *dev);
 
-void nouveau_fbcon_output_poll_changed(struct drm_device *dev);
-
 extern int nouveau_nofbaccel;
 
 #endif /* __NV50_FBCON_H__ */
diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c b/drivers/gpu/drm/nouveau/nouveau_vga.c
index 48393a4f6331..ac97c30c5bbd 100644
--- a/drivers/gpu/drm/nouveau/nouveau_vga.c
+++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
@@ -3,6 +3,7 @@
 
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
 
 #include "nouveau_drv.h"
 #include "nouveau_acpi.h"
@@ -60,7 +61,7 @@ static void
 nouveau_switcheroo_reprobe(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
-	nouveau_fbcon_output_poll_changed(dev);
+	drm_fb_helper_output_poll_changed(dev);
 }
 
 static bool
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 2dbf62a2ac41..5b3db08e50e4 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -4308,7 +4308,7 @@ nv50_disp_atomic_state_alloc(struct drm_device *dev)
 static const struct drm_mode_config_funcs
 nv50_disp_func = {
 	.fb_create = nouveau_user_framebuffer_create,
-	.output_poll_changed = nouveau_fbcon_output_poll_changed,
+	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = nv50_disp_atomic_check,
 	.atomic_commit = nv50_disp_atomic_commit,
 	.atomic_state_alloc = nv50_disp_atomic_state_alloc,
-- 
2.14.2

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

[PATCH v2 11/15] drm/omap: Use drm_fb_helper_lastclose() and _poll_changed()

[Noralf Trønnes]

This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
It can also use drm_fb_helper_output_poll_changed() as its
.output_poll_changed callback.

Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 34 ++--------------------------------
 1 file changed, 2 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index cdf5b0601eba..96857c508ee0 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -46,14 +46,6 @@
  *                 devices
  */
 
-static void omap_fb_output_poll_changed(struct drm_device *dev)
-{
-	struct omap_drm_private *priv = dev->dev_private;
-	DBG("dev=%p", dev);
-	if (priv->fbdev)
-		drm_fb_helper_hotplug_event(priv->fbdev);
-}
-
 static void omap_atomic_wait_for_completion(struct drm_device *dev,
 					    struct drm_atomic_state *old_state)
 {
@@ -132,7 +124,7 @@ static const struct drm_mode_config_helper_funcs omap_mode_config_helper_funcs =
 
 static const struct drm_mode_config_funcs omap_mode_config_funcs = {
 	.fb_create = omap_framebuffer_create,
-	.output_poll_changed = omap_fb_output_poll_changed,
+	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -467,28 +459,6 @@ static int dev_open(struct drm_device *dev, struct drm_file *file)
 	return 0;
 }
 
-/**
- * lastclose - clean up after all DRM clients have exited
- * @dev: DRM device
- *
- * Take care of cleaning up after all DRM clients have exited.  In the
- * mode setting case, we want to restore the kernel's initial mode (just
- * in case the last client left us in a bad state).
- */
-static void dev_lastclose(struct drm_device *dev)
-{
-	struct omap_drm_private *priv = dev->dev_private;
-	int ret;
-
-	DBG("lastclose: dev=%p", dev);
-
-	if (priv->fbdev) {
-		ret = drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev);
-		if (ret)
-			DBG("failed to restore crtc mode");
-	}
-}
-
 static const struct vm_operations_struct omap_gem_vm_ops = {
 	.fault = omap_gem_fault,
 	.open = drm_gem_vm_open,
@@ -511,7 +481,7 @@ static struct drm_driver omap_drm_driver = {
 	.driver_features = DRIVER_MODESET | DRIVER_GEM  | DRIVER_PRIME |
 		DRIVER_ATOMIC | DRIVER_RENDER,
 	.open = dev_open,
-	.lastclose = dev_lastclose,
+	.lastclose = drm_fb_helper_lastclose,
 #ifdef CONFIG_DEBUG_FS
 	.debugfs_init = omap_debugfs_init,
 #endif
-- 
2.14.2

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

[PATCH v2 12/15] drm/radeon: Use drm_fb_helper_lastclose() and _poll_changed()

[Noralf Trønnes]

This driver can use drm_fb_helper_lastclose() in its .lastclose function.
It can also use drm_fb_helper_output_poll_changed() as its
.output_poll_changed callback.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/radeon/radeon_display.c |  9 ++-------
 drivers/gpu/drm/radeon/radeon_fb.c      | 22 ----------------------
 drivers/gpu/drm/radeon/radeon_kms.c     |  5 ++---
 drivers/gpu/drm/radeon/radeon_mode.h    |  3 ---
 4 files changed, 4 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index ddfe91efa61e..dfda5e0ed166 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -32,6 +32,7 @@
 
 #include <linux/pm_runtime.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_edid.h>
 
@@ -1362,15 +1363,9 @@ radeon_user_framebuffer_create(struct drm_device *dev,
 	return &radeon_fb->base;
 }
 
-static void radeon_output_poll_changed(struct drm_device *dev)
-{
-	struct radeon_device *rdev = dev->dev_private;
-	radeon_fb_output_poll_changed(rdev);
-}
-
 static const struct drm_mode_config_funcs radeon_mode_funcs = {
 	.fb_create = radeon_user_framebuffer_create,
-	.output_poll_changed = radeon_output_poll_changed
+	.output_poll_changed = drm_fb_helper_output_poll_changed,
 };
 
 static const struct drm_prop_enum_list radeon_tmds_pll_enum_list[] =
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 2fcf805d3a16..8a582af52073 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -307,12 +307,6 @@ static int radeonfb_create(struct drm_fb_helper *helper,
 	return ret;
 }
 
-void radeon_fb_output_poll_changed(struct radeon_device *rdev)
-{
-	if (rdev->mode_info.rfbdev)
-		drm_fb_helper_hotplug_event(&rdev->mode_info.rfbdev->helper);
-}
-
 static int radeon_fbdev_destroy(struct drm_device *dev, struct radeon_fbdev *rfbdev)
 {
 	struct radeon_framebuffer *rfb = &rfbdev->rfb;
@@ -423,19 +417,3 @@ void radeon_fb_remove_connector(struct radeon_device *rdev, struct drm_connector
 	if (rdev->mode_info.rfbdev)
 		drm_fb_helper_remove_one_connector(&rdev->mode_info.rfbdev->helper, connector);
 }
-
-void radeon_fbdev_restore_mode(struct radeon_device *rdev)
-{
-	struct radeon_fbdev *rfbdev = rdev->mode_info.rfbdev;
-	struct drm_fb_helper *fb_helper;
-	int ret;
-
-	if (!rfbdev)
-		return;
-
-	fb_helper = &rfbdev->helper;
-
-	ret = drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper);
-	if (ret)
-		DRM_DEBUG("failed to restore crtc mode\n");
-}
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index dfee8f7d94ae..e4c1bb8c21fa 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -26,6 +26,7 @@
  *          Jerome Glisse
  */
 #include <drm/drmP.h>
+#include <drm/drm_fb_helper.h>
 #include "radeon.h"
 #include <drm/radeon_drm.h>
 #include "radeon_asic.h"
@@ -636,9 +637,7 @@ static int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file
  */
 void radeon_driver_lastclose_kms(struct drm_device *dev)
 {
-	struct radeon_device *rdev = dev->dev_private;
-
-	radeon_fbdev_restore_mode(rdev);
+	drm_fb_helper_lastclose(dev);
 	vga_switcheroo_process_delayed_switch();
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index da44ac234f64..567156a7beae 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -988,9 +988,6 @@ int radeon_fbdev_init(struct radeon_device *rdev);
 void radeon_fbdev_fini(struct radeon_device *rdev);
 void radeon_fbdev_set_suspend(struct radeon_device *rdev, int state);
 bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj);
-void radeon_fbdev_restore_mode(struct radeon_device *rdev);
-
-void radeon_fb_output_poll_changed(struct radeon_device *rdev);
 
 void radeon_crtc_handle_vblank(struct radeon_device *rdev, int crtc_id);
 
-- 
2.14.2

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

[PATCH v2 13/15] drm/rockchip: Use drm_fb_helper_lastclose() and _poll_changed()

[Noralf Trønnes]

This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
It can also use drm_fb_helper_output_poll_changed() as its
.output_poll_changed callback.

Cc: Mark Yao <mark.yao@rock-chips.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 9 +--------
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c  | 9 +--------
 2 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 76d63de5921d..d85431400a0d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -207,13 +207,6 @@ static void rockchip_drm_unbind(struct device *dev)
 	drm_dev_unref(drm_dev);
 }
 
-static void rockchip_drm_lastclose(struct drm_device *dev)
-{
-	struct rockchip_drm_private *priv = dev->dev_private;
-
-	drm_fb_helper_restore_fbdev_mode_unlocked(&priv->fbdev_helper);
-}
-
 static const struct file_operations rockchip_drm_driver_fops = {
 	.owner = THIS_MODULE,
 	.open = drm_open,
@@ -228,7 +221,7 @@ static const struct file_operations rockchip_drm_driver_fops = {
 static struct drm_driver rockchip_drm_driver = {
 	.driver_features	= DRIVER_MODESET | DRIVER_GEM |
 				  DRIVER_PRIME | DRIVER_ATOMIC,
-	.lastclose		= rockchip_drm_lastclose,
+	.lastclose		= drm_fb_helper_lastclose,
 	.gem_vm_ops		= &drm_gem_cma_vm_ops,
 	.gem_free_object_unlocked = rockchip_gem_free_object,
 	.dumb_create		= rockchip_gem_dumb_create,
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index cd2ace0c3caa..e266539e04e5 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -167,20 +167,13 @@ rockchip_user_fb_create(struct drm_device *dev, struct drm_file *file_priv,
 	return ERR_PTR(ret);
 }
 
-static void rockchip_drm_output_poll_changed(struct drm_device *dev)
-{
-	struct rockchip_drm_private *private = dev->dev_private;
-
-	drm_fb_helper_hotplug_event(&private->fbdev_helper);
-}
-
 static const struct drm_mode_config_helper_funcs rockchip_mode_config_helpers = {
 	.atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
 };
 
 static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = {
 	.fb_create = rockchip_user_fb_create,
-	.output_poll_changed = rockchip_drm_output_poll_changed,
+	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
-- 
2.14.2

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

[PATCH v2 14/15] drm/tegra: Use drm_fb_helper_lastclose() and _poll_changed()

[Noralf Trønnes]

This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
It can also use drm_fb_helper_output_poll_changed() as its
.output_poll_changed callback.

Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tegra/drm.c | 13 ++-----------
 drivers/gpu/drm/tegra/drm.h |  4 ----
 drivers/gpu/drm/tegra/fb.c  | 14 --------------
 3 files changed, 2 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 597d563d636a..73bca7ac1271 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -120,7 +120,7 @@ static int tegra_atomic_commit(struct drm_device *drm,
 static const struct drm_mode_config_funcs tegra_drm_mode_funcs = {
 	.fb_create = tegra_fb_create,
 #ifdef CONFIG_DRM_FBDEV_EMULATION
-	.output_poll_changed = tegra_fb_output_poll_changed,
+	.output_poll_changed = drm_fb_helper_output_poll_changed,
 #endif
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = tegra_atomic_commit,
@@ -287,15 +287,6 @@ static void tegra_drm_context_free(struct tegra_drm_context *context)
 	kfree(context);
 }
 
-static void tegra_drm_lastclose(struct drm_device *drm)
-{
-#ifdef CONFIG_DRM_FBDEV_EMULATION
-	struct tegra_drm *tegra = drm->dev_private;
-
-	tegra_fbdev_restore_mode(tegra->fbdev);
-#endif
-}
-
 static struct host1x_bo *
 host1x_bo_lookup(struct drm_file *file, u32 handle)
 {
@@ -1100,7 +1091,7 @@ static struct drm_driver tegra_drm_driver = {
 	.unload = tegra_drm_unload,
 	.open = tegra_drm_open,
 	.postclose = tegra_drm_postclose,
-	.lastclose = tegra_drm_lastclose,
+	.lastclose = drm_fb_helper_lastclose,
 
 #if defined(CONFIG_DEBUG_FS)
 	.debugfs_init = tegra_debugfs_init,
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 063f5d397526..55b6aff25b9b 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -290,10 +290,6 @@ int tegra_drm_fb_init(struct drm_device *drm);
 void tegra_drm_fb_exit(struct drm_device *drm);
 void tegra_drm_fb_suspend(struct drm_device *drm);
 void tegra_drm_fb_resume(struct drm_device *drm);
-#ifdef CONFIG_DRM_FBDEV_EMULATION
-void tegra_fbdev_restore_mode(struct tegra_fbdev *fbdev);
-void tegra_fb_output_poll_changed(struct drm_device *drm);
-#endif
 
 extern struct platform_driver tegra_dc_driver;
 extern struct platform_driver tegra_hdmi_driver;
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index 80540c1c66dc..8dfe3c6c217e 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -361,20 +361,6 @@ static void tegra_fbdev_exit(struct tegra_fbdev *fbdev)
 	drm_fb_helper_fini(&fbdev->base);
 	tegra_fbdev_free(fbdev);
 }
-
-void tegra_fbdev_restore_mode(struct tegra_fbdev *fbdev)
-{
-	if (fbdev)
-		drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev->base);
-}
-
-void tegra_fb_output_poll_changed(struct drm_device *drm)
-{
-	struct tegra_drm *tegra = drm->dev_private;
-
-	if (tegra->fbdev)
-		drm_fb_helper_hotplug_event(&tegra->fbdev->base);
-}
 #endif
 
 int tegra_drm_fb_prepare(struct drm_device *drm)
-- 
2.14.2

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

[PATCH v2 15/15] staging: vboxvideo: Use drm_fb_helper_lastclose()

[Noralf Trønnes]

This driver can use drm_fb_helper_lastclose() as its .lastclose callback.

Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/vboxvideo/vbox_drv.c  |  2 +-
 drivers/staging/vboxvideo/vbox_drv.h  |  1 -
 drivers/staging/vboxvideo/vbox_main.c | 12 ------------
 3 files changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c
index e18642e5027e..a4d8d7898e3d 100644
--- a/drivers/staging/vboxvideo/vbox_drv.c
+++ b/drivers/staging/vboxvideo/vbox_drv.c
@@ -229,7 +229,7 @@ static struct drm_driver driver = {
 
 	.load = vbox_driver_load,
 	.unload = vbox_driver_unload,
-	.lastclose = vbox_driver_lastclose,
+	.lastclose = drm_fb_helper_lastclose,
 	.master_set = vbox_master_set,
 	.master_drop = vbox_master_drop,
 
diff --git a/drivers/staging/vboxvideo/vbox_drv.h b/drivers/staging/vboxvideo/vbox_drv.h
index 4b9302703b36..7273d7e9bc9b 100644
--- a/drivers/staging/vboxvideo/vbox_drv.h
+++ b/drivers/staging/vboxvideo/vbox_drv.h
@@ -128,7 +128,6 @@ struct vbox_private {
 
 int vbox_driver_load(struct drm_device *dev, unsigned long flags);
 void vbox_driver_unload(struct drm_device *dev);
-void vbox_driver_lastclose(struct drm_device *dev);
 
 struct vbox_gem_object;
 
diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c
index 80bd039fa08e..c3d756620fd5 100644
--- a/drivers/staging/vboxvideo/vbox_main.c
+++ b/drivers/staging/vboxvideo/vbox_main.c
@@ -421,18 +421,6 @@ void vbox_driver_unload(struct drm_device *dev)
 	vbox_hw_fini(vbox);
 }
 
-/**
- * @note this is described in the DRM framework documentation.  AST does not
- * have it, but we get an oops on driver unload if it is not present.
- */
-void vbox_driver_lastclose(struct drm_device *dev)
-{
-	struct vbox_private *vbox = dev->dev_private;
-
-	if (vbox->fbdev)
-		drm_fb_helper_restore_fbdev_mode_unlocked(&vbox->fbdev->helper);
-}
-
 int vbox_gem_create(struct drm_device *dev,
 		    u32 size, bool iskernel, struct drm_gem_object **obj)
 {
-- 
2.14.2

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

Re: [PATCH v2 15/15] staging: vboxvideo: Use drm_fb_helper_lastclose()

[Daniel Vetter]

On Mon, Oct 30, 2017 at 04:39:51PM +0100, Noralf Trønnes wrote:
> This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
> 
> Cc: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/staging/vboxvideo/vbox_drv.c  |  2 +-
>  drivers/staging/vboxvideo/vbox_drv.h  |  1 -
>  drivers/staging/vboxvideo/vbox_main.c | 12 ------------
>  3 files changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c
> index e18642e5027e..a4d8d7898e3d 100644
> --- a/drivers/staging/vboxvideo/vbox_drv.c
> +++ b/drivers/staging/vboxvideo/vbox_drv.c
> @@ -229,7 +229,7 @@ static struct drm_driver driver = {
>  
>  	.load = vbox_driver_load,
>  	.unload = vbox_driver_unload,
> -	.lastclose = vbox_driver_lastclose,
> +	.lastclose = drm_fb_helper_lastclose,
>  	.master_set = vbox_master_set,
>  	.master_drop = vbox_master_drop,
>  
> diff --git a/drivers/staging/vboxvideo/vbox_drv.h b/drivers/staging/vboxvideo/vbox_drv.h
> index 4b9302703b36..7273d7e9bc9b 100644
> --- a/drivers/staging/vboxvideo/vbox_drv.h
> +++ b/drivers/staging/vboxvideo/vbox_drv.h
> @@ -128,7 +128,6 @@ struct vbox_private {
>  
>  int vbox_driver_load(struct drm_device *dev, unsigned long flags);
>  void vbox_driver_unload(struct drm_device *dev);
> -void vbox_driver_lastclose(struct drm_device *dev);
>  
>  struct vbox_gem_object;
>  
> diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c
> index 80bd039fa08e..c3d756620fd5 100644
> --- a/drivers/staging/vboxvideo/vbox_main.c
> +++ b/drivers/staging/vboxvideo/vbox_main.c
> @@ -421,18 +421,6 @@ void vbox_driver_unload(struct drm_device *dev)
>  	vbox_hw_fini(vbox);
>  }
>  
> -/**
> - * @note this is described in the DRM framework documentation.  AST does not
> - * have it, but we get an oops on driver unload if it is not present.
> - */
> -void vbox_driver_lastclose(struct drm_device *dev)
> -{
> -	struct vbox_private *vbox = dev->dev_private;
> -
> -	if (vbox->fbdev)

Hm, except gma500 all the drivers have this NULL check here. I'm not sure
whether that's just cargo-culted or whether that's needed, but I'm wary
from slapping an Ack on the entire series as-is.

I'm pretty sure we still need it for i915, but everyone else might just
have copied too much. Or it predates the helper support for disabling the
fbdev emulation.

There's also the question that a bunch of drivers who set up fbdev forgot
to add the lastclose call it seems, but that's a different story.
-Daniel

> -		drm_fb_helper_restore_fbdev_mode_unlocked(&vbox->fbdev->helper);
> -}
> -
>  int vbox_gem_create(struct drm_device *dev,
>  		    u32 size, bool iskernel, struct drm_gem_object **obj)
>  {
> -- 
> 2.14.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 15/15] staging: vboxvideo: Use drm_fb_helper_lastclose()

[Noralf Trønnes]

Den 31.10.2017 11.32, skrev Daniel Vetter:
> On Mon, Oct 30, 2017 at 04:39:51PM +0100, Noralf Trønnes wrote:
>> This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
>>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/staging/vboxvideo/vbox_drv.c  |  2 +-
>>   drivers/staging/vboxvideo/vbox_drv.h  |  1 -
>>   drivers/staging/vboxvideo/vbox_main.c | 12 ------------
>>   3 files changed, 1 insertion(+), 14 deletions(-)
>>
>> diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c
>> index e18642e5027e..a4d8d7898e3d 100644
>> --- a/drivers/staging/vboxvideo/vbox_drv.c
>> +++ b/drivers/staging/vboxvideo/vbox_drv.c
>> @@ -229,7 +229,7 @@ static struct drm_driver driver = {
>>   
>>   	.load = vbox_driver_load,
>>   	.unload = vbox_driver_unload,
>> -	.lastclose = vbox_driver_lastclose,
>> +	.lastclose = drm_fb_helper_lastclose,
>>   	.master_set = vbox_master_set,
>>   	.master_drop = vbox_master_drop,
>>   
>> diff --git a/drivers/staging/vboxvideo/vbox_drv.h b/drivers/staging/vboxvideo/vbox_drv.h
>> index 4b9302703b36..7273d7e9bc9b 100644
>> --- a/drivers/staging/vboxvideo/vbox_drv.h
>> +++ b/drivers/staging/vboxvideo/vbox_drv.h
>> @@ -128,7 +128,6 @@ struct vbox_private {
>>   
>>   int vbox_driver_load(struct drm_device *dev, unsigned long flags);
>>   void vbox_driver_unload(struct drm_device *dev);
>> -void vbox_driver_lastclose(struct drm_device *dev);
>>   
>>   struct vbox_gem_object;
>>   
>> diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c
>> index 80bd039fa08e..c3d756620fd5 100644
>> --- a/drivers/staging/vboxvideo/vbox_main.c
>> +++ b/drivers/staging/vboxvideo/vbox_main.c
>> @@ -421,18 +421,6 @@ void vbox_driver_unload(struct drm_device *dev)
>>   	vbox_hw_fini(vbox);
>>   }
>>   
>> -/**
>> - * @note this is described in the DRM framework documentation.  AST does not
>> - * have it, but we get an oops on driver unload if it is not present.
>> - */
>> -void vbox_driver_lastclose(struct drm_device *dev)
>> -{
>> -	struct vbox_private *vbox = dev->dev_private;
>> -
>> -	if (vbox->fbdev)
> Hm, except gma500 all the drivers have this NULL check here. I'm not sure
> whether that's just cargo-culted or whether that's needed, but I'm wary
> from slapping an Ack on the entire series as-is.

I think it's cargo-culted because IIRC almost every driver bails out if
fbdev init fails. So the only time the pointer is NULL is when fbdev
emulation is compiled out. And in that case the check is not needed.

vboxvideo for instance fails probing if fbdev init fails, and if it
succeeds vbox->fbdev is always set.

But anyhow, yeah, I think it's better that the driver maintainers verify 
this.

Noralf.

> I'm pretty sure we still need it for i915, but everyone else might just
> have copied too much. Or it predates the helper support for disabling the
> fbdev emulation.
>
> There's also the question that a bunch of drivers who set up fbdev forgot
> to add the lastclose call it seems, but that's a different story.
> -Daniel
>
>> -		drm_fb_helper_restore_fbdev_mode_unlocked(&vbox->fbdev->helper);
>> -}
>> -
>>   int vbox_gem_create(struct drm_device *dev,
>>   		    u32 size, bool iskernel, struct drm_gem_object **obj)
>>   {
>> -- 
>> 2.14.2
>>

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

Re: [PATCH v2 15/15] staging: vboxvideo: Use drm_fb_helper_lastclose()

[Daniel Vetter]

On Tue, Oct 31, 2017 at 03:40:40PM +0100, Noralf Trønnes wrote:
> 
> Den 31.10.2017 11.32, skrev Daniel Vetter:
> > On Mon, Oct 30, 2017 at 04:39:51PM +0100, Noralf Trønnes wrote:
> > > This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
> > > 
> > > Cc: Hans de Goede <hdegoede@redhat.com>
> > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > > ---
> > >   drivers/staging/vboxvideo/vbox_drv.c  |  2 +-
> > >   drivers/staging/vboxvideo/vbox_drv.h  |  1 -
> > >   drivers/staging/vboxvideo/vbox_main.c | 12 ------------
> > >   3 files changed, 1 insertion(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c
> > > index e18642e5027e..a4d8d7898e3d 100644
> > > --- a/drivers/staging/vboxvideo/vbox_drv.c
> > > +++ b/drivers/staging/vboxvideo/vbox_drv.c
> > > @@ -229,7 +229,7 @@ static struct drm_driver driver = {
> > >   	.load = vbox_driver_load,
> > >   	.unload = vbox_driver_unload,
> > > -	.lastclose = vbox_driver_lastclose,
> > > +	.lastclose = drm_fb_helper_lastclose,
> > >   	.master_set = vbox_master_set,
> > >   	.master_drop = vbox_master_drop,
> > > diff --git a/drivers/staging/vboxvideo/vbox_drv.h b/drivers/staging/vboxvideo/vbox_drv.h
> > > index 4b9302703b36..7273d7e9bc9b 100644
> > > --- a/drivers/staging/vboxvideo/vbox_drv.h
> > > +++ b/drivers/staging/vboxvideo/vbox_drv.h
> > > @@ -128,7 +128,6 @@ struct vbox_private {
> > >   int vbox_driver_load(struct drm_device *dev, unsigned long flags);
> > >   void vbox_driver_unload(struct drm_device *dev);
> > > -void vbox_driver_lastclose(struct drm_device *dev);
> > >   struct vbox_gem_object;
> > > diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c
> > > index 80bd039fa08e..c3d756620fd5 100644
> > > --- a/drivers/staging/vboxvideo/vbox_main.c
> > > +++ b/drivers/staging/vboxvideo/vbox_main.c
> > > @@ -421,18 +421,6 @@ void vbox_driver_unload(struct drm_device *dev)
> > >   	vbox_hw_fini(vbox);
> > >   }
> > > -/**
> > > - * @note this is described in the DRM framework documentation.  AST does not
> > > - * have it, but we get an oops on driver unload if it is not present.
> > > - */
> > > -void vbox_driver_lastclose(struct drm_device *dev)
> > > -{
> > > -	struct vbox_private *vbox = dev->dev_private;
> > > -
> > > -	if (vbox->fbdev)
> > Hm, except gma500 all the drivers have this NULL check here. I'm not sure
> > whether that's just cargo-culted or whether that's needed, but I'm wary
> > from slapping an Ack on the entire series as-is.
> 
> I think it's cargo-culted because IIRC almost every driver bails out if
> fbdev init fails. So the only time the pointer is NULL is when fbdev
> emulation is compiled out. And in that case the check is not needed.
> 
> vboxvideo for instance fails probing if fbdev init fails, and if it
> succeeds vbox->fbdev is always set.
> 
> But anyhow, yeah, I think it's better that the driver maintainers verify
> this.

See my other mail, I was blind, I think it's all good.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✗ Fi.CI.BAT: failure for drm/fb-helper: Add .last_close and .output_poll_changed helpers (rev2)

[Patchwork]

== Series Details ==

Series: drm/fb-helper: Add .last_close and .output_poll_changed helpers (rev2)
URL   : https://patchwork.freedesktop.org/series/32332/
State : failure

== Summary ==

Series 32332v2 drm/fb-helper: Add .last_close and .output_poll_changed helpers
https://patchwork.freedesktop.org/api/1.0/series/32332/revisions/2/mbox/

Test drv_hangman:
        Subgroup error-state-basic:
                dmesg-warn -> PASS       (fi-cfl-s)
Test gem_exec_fence:
        Subgroup basic-await-default:
                pass       -> FAIL       (fi-glk-dsi)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-bdw-5557u) fdo#102473
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-n2820) fdo#101705
Test drv_module_reload:
        Subgroup basic-reload:
                pass       -> INCOMPLETE (fi-bdw-5557u)

fdo#102473 https://bugs.freedesktop.org/show_bug.cgi?id=102473
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:286  pass:264  dwarn:1   dfail:0   fail:0   skip:20 
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:449s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:377s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:536s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:266s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:509s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:498s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:498s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:481s
fi-cfl-s         total:289  pass:250  dwarn:7   dfail:0   fail:0   skip:32  time:553s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:424s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:251s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:582s
fi-glk-dsi       total:289  pass:257  dwarn:0   dfail:0   fail:2   skip:30  time:488s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:430s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:428s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:421s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:498s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:462s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:489s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:575s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:476s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:586s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:553s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:450s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:594s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:648s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:523s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:511s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:456s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:566s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:429s
fi-cnl-y failed to connect after reboot

2dd14a4ad87f449830d9fbb7b4a33f5d369dcde2 drm-tip: 2017y-10m-30d-13h-40m-22s UTC integration manifest
9cd5df5a14c1 staging: vboxvideo: Use drm_fb_helper_lastclose()
0f149672bb9a drm/tegra: Use drm_fb_helper_lastclose() and _poll_changed()
b5899a2e1bd8 drm/rockchip: Use drm_fb_helper_lastclose() and _poll_changed()
4cf9e2b4924f drm/radeon: Use drm_fb_helper_lastclose() and _poll_changed()
07a601d068ff drm/omap: Use drm_fb_helper_lastclose() and _poll_changed()
cc5935e53187 drm/nouveau: Use drm_fb_helper_output_poll_changed()
78add5d79a24 drm/msm: Use drm_fb_helper_lastclose() and _poll_changed()
2099ac609634 drm/i915: Use drm_fb_helper_output_poll_changed()
2b893b70e421 drm/gma500: Use drm_fb_helper_lastclose() and _poll_changed()
296d21877f27 drm/exynos: Use drm_fb_helper_lastclose() and _poll_changed()
dd9a3b493f8a drm/armada: Use drm_fb_helper_lastclose() and _poll_changed()
23cfc40436c0 drm/amdgpu: Use drm_fb_helper_lastclose() and _poll_changed()
9b7c24a5ebf1 drm/fb-helper: Add .last_close and .output_poll_changed helpers
3b9606bfc752 drm: Add drm_device->fb_helper pointer
e7de9b92fa6e drm/fb-helper: Handle function NULL argument

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6264/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 00/15] drm/fb-helper: Add .last_close and .output_poll_changed helpers

[Noralf Trønnes]

Den 30.10.2017 16.39, skrev Noralf Trønnes:
> This patchset adds fbdev .last_close and .output_poll_changed helpers to
> reduce fbdev emulation footprint in drivers.
>
> I don't know which drivers have their own tree or not, so if you want me
> to apply your patch to drm-misc, please let me know.
>
> I will do a separate patchset for the cma helper drivers.
>
> Noralf.
>
> Changes since version 1:
> - drm_device.drm_fb_helper_private -> drm_device.fb_helper (Ville)
>
> Noralf Trønnes (15):
>    drm/fb-helper: Handle function NULL argument
>    drm: Add drm_device->fb_helper pointer
>    drm/fb-helper: Add .last_close and .output_poll_changed helpers

Core patches 1-3 applied to drm-misc.
Thanks for reviewing!

Noralf.

>    drm/amdgpu: Use drm_fb_helper_lastclose() and _poll_changed()
>    drm/armada: Use drm_fb_helper_lastclose() and _poll_changed()
>    drm/exynos: Use drm_fb_helper_lastclose() and _poll_changed()
>    drm/gma500: Use drm_fb_helper_lastclose() and _poll_changed()
>    drm/i915: Use drm_fb_helper_output_poll_changed()
>    drm/msm: Use drm_fb_helper_lastclose() and _poll_changed()
>    drm/nouveau: Use drm_fb_helper_output_poll_changed()
>    drm/omap: Use drm_fb_helper_lastclose() and _poll_changed()
>    drm/radeon: Use drm_fb_helper_lastclose() and _poll_changed()
>    drm/rockchip: Use drm_fb_helper_lastclose() and _poll_changed()
>    drm/tegra: Use drm_fb_helper_lastclose() and _poll_changed()
>    staging: vboxvideo: Use drm_fb_helper_lastclose()
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  9 +---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c      | 27 -----------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c     |  4 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h    |  4 --
>   drivers/gpu/drm/armada/armada_drm.h         |  1 -
>   drivers/gpu/drm/armada/armada_drv.c         |  8 +---
>   drivers/gpu/drm/armada/armada_fb.c          | 11 +----
>   drivers/gpu/drm/armada/armada_fbdev.c       |  8 ----
>   drivers/gpu/drm/drm_fb_helper.c             | 69 +++++++++++++++++++++++------
>   drivers/gpu/drm/exynos/exynos_drm_drv.c     |  8 +---
>   drivers/gpu/drm/exynos/exynos_drm_fb.c      |  2 +-
>   drivers/gpu/drm/exynos/exynos_drm_fbdev.c   | 18 --------
>   drivers/gpu/drm/exynos/exynos_drm_fbdev.h   |  2 -
>   drivers/gpu/drm/gma500/framebuffer.c        |  9 +---
>   drivers/gpu/drm/gma500/psb_drv.c            | 15 +------
>   drivers/gpu/drm/i915/intel_display.c        |  2 +-
>   drivers/gpu/drm/i915/intel_drv.h            |  5 ---
>   drivers/gpu/drm/i915/intel_fbdev.c          |  8 ----
>   drivers/gpu/drm/msm/msm_drv.c               | 18 +-------
>   drivers/gpu/drm/nouveau/nouveau_display.c   |  3 +-
>   drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  8 ----
>   drivers/gpu/drm/nouveau/nouveau_fbcon.h     |  2 -
>   drivers/gpu/drm/nouveau/nouveau_vga.c       |  3 +-
>   drivers/gpu/drm/nouveau/nv50_display.c      |  2 +-
>   drivers/gpu/drm/omapdrm/omap_drv.c          | 34 +-------------
>   drivers/gpu/drm/radeon/radeon_display.c     |  9 +---
>   drivers/gpu/drm/radeon/radeon_fb.c          | 22 ---------
>   drivers/gpu/drm/radeon/radeon_kms.c         |  5 +--
>   drivers/gpu/drm/radeon/radeon_mode.h        |  3 --
>   drivers/gpu/drm/rockchip/rockchip_drm_drv.c |  9 +---
>   drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  9 +---
>   drivers/gpu/drm/tegra/drm.c                 | 13 +-----
>   drivers/gpu/drm/tegra/drm.h                 |  4 --
>   drivers/gpu/drm/tegra/fb.c                  | 14 ------
>   drivers/staging/vboxvideo/vbox_drv.c        |  2 +-
>   drivers/staging/vboxvideo/vbox_drv.h        |  1 -
>   drivers/staging/vboxvideo/vbox_main.c       | 12 -----
>   include/drm/drm_device.h                    |  9 ++++
>   include/drm/drm_fb_helper.h                 | 11 +++++
>   39 files changed, 106 insertions(+), 297 deletions(-)
>

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