[PATCH v8 0/6] drm/{i915,xe}: Convert fbdev to DRM client

[PATCH v8 0/6] drm/{i915,xe}: Convert fbdev to DRM client

[Thomas Zimmermann]

(was: drm/i915: Convert fbdev to DRM client)

Convert i915's fbdev code to struct drm_client. Replaces the current
ad-hoc integration. The conversion includes a number of cleanups. Also
update the xe driver accordingly.

As with the other drivers' fbdev emulation, fbdev in i915 is now
an in-kernel DRM client that runs after the DRM device has been
registered. This allows to remove the asynchronous initialization.

i915 and xe are the final drivers with an fbdev emulation that is not
build upon struct drm_client. Once reviewed, the patches would ideally go
into drm-misc-next, so that the old fbdev helper code can be removed.

We can also attempt to add additional in-kernel clients. A DRM-based
dmesg log or a bootsplash are commonly mentioned. DRM can then switch
easily among the existing clients if/when required.

v8:
- do setup and cleanup in intel_display_driver_{register,unregister}()
  (Jani, Jouni)
- mention xe in several commit messages (Rodrigo, Jani)
- resort patches to put driver-independent changes first
- slightly reword cover letter

v7:
- update xe driver

v6:
- reorder patches to fix build (Jouni)
- remove unnecessary handling of non-atomic commits (Jouni, Ville)
- return errors from callbacks (Jouni)
- various minor fixes

v5:
- style fixes (checkpatch)
	
v4:
<no changes>
		
v3:
- support module unloading (Jani, CI bot)
- as before, silently ignore devices without displays (CI  bot)

v2:		
- fix error handling (Jani)
- fix non-fbdev builds
- various minor fixes and cleanups

Thomas Zimmermann (6):
  drm/client: Export drm_client_dev_unregister()
  drm/i915: Move fbdev functions
  drm/i915: Initialize fbdev DRM client with callback functions
  drm/{i915,xe}: Unregister in-kernel clients
  drm/{i915,xe}: Implement fbdev client callbacks
  drm/{i915,xe}: Implement fbdev emulation as in-kernel client

 drivers/gpu/drm/drm_client.c                  |  13 +
 drivers/gpu/drm/i915/display/intel_display.c  |   1 -
 .../drm/i915/display/intel_display_driver.c   |  24 +-
 drivers/gpu/drm/i915/display/intel_fbdev.c    | 265 ++++++++++--------
 drivers/gpu/drm/i915/display/intel_fbdev.h    |  29 +-
 drivers/gpu/drm/i915/i915_driver.c            |  22 --
 drivers/gpu/drm/xe/display/xe_display.c       |  11 -
 drivers/gpu/drm/xe/xe_device.c                |   1 +
 8 files changed, 167 insertions(+), 199 deletions(-)

-- 
2.44.0

[PATCH v8 1/6] drm/client: Export drm_client_dev_unregister()

[Thomas Zimmermann]

Export drm_client_dev_unregister() for use by the i915 driver. The
driver does not use drm_dev_unregister(), so it has to clean up the
in-kernel DRM clients by itself.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_client.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 77fe217aeaf36..2803ac111bbd8 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -172,6 +172,18 @@ void drm_client_release(struct drm_client_dev *client)
 }
 EXPORT_SYMBOL(drm_client_release);
 
+/**
+ * drm_client_dev_unregister - Unregister clients
+ * @dev: DRM device
+ *
+ * This function releases all clients by calling each client's
+ * &drm_client_funcs.unregister callback. The callback function
+ * is responsibe for releaseing all resources including the client
+ * itself.
+ *
+ * The helper drm_dev_unregister() calls this function. Drivers
+ * that use it don't need to call this function themselves.
+ */
 void drm_client_dev_unregister(struct drm_device *dev)
 {
 	struct drm_client_dev *client, *tmp;
@@ -191,6 +203,7 @@ void drm_client_dev_unregister(struct drm_device *dev)
 	}
 	mutex_unlock(&dev->clientlist_mutex);
 }
+EXPORT_SYMBOL(drm_client_dev_unregister);
 
 /**
  * drm_client_dev_hotplug - Send hotplug event to clients
-- 
2.44.0

[PATCH v8 2/6] drm/i915: Move fbdev functions

[Thomas Zimmermann]

Move functions within intel_fbdev.c to simplify later updates. Minor
style fixes to make checkpatch happy, but no functional changes.

v5:
- style fixes (checkpatch)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbdev.c | 154 ++++++++++-----------
 1 file changed, 77 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 43855c6c35099..2714e12a6c44c 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -458,58 +458,6 @@ static void intel_fbdev_suspend_worker(struct work_struct *work)
 				true);
 }
 
-int intel_fbdev_init(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_fbdev *ifbdev;
-	int ret;
-
-	if (drm_WARN_ON(dev, !HAS_DISPLAY(dev_priv)))
-		return -ENODEV;
-
-	ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL);
-	if (ifbdev == NULL)
-		return -ENOMEM;
-
-	mutex_init(&ifbdev->hpd_lock);
-	drm_fb_helper_prepare(dev, &ifbdev->helper, 32, &intel_fb_helper_funcs);
-
-	if (intel_fbdev_init_bios(dev, ifbdev))
-		ifbdev->helper.preferred_bpp = ifbdev->preferred_bpp;
-	else
-		ifbdev->preferred_bpp = ifbdev->helper.preferred_bpp;
-
-	ret = drm_fb_helper_init(dev, &ifbdev->helper);
-	if (ret) {
-		kfree(ifbdev);
-		return ret;
-	}
-
-	dev_priv->display.fbdev.fbdev = ifbdev;
-	INIT_WORK(&dev_priv->display.fbdev.suspend_work, intel_fbdev_suspend_worker);
-
-	return 0;
-}
-
-static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
-{
-	struct intel_fbdev *ifbdev = data;
-
-	/* Due to peculiar init order wrt to hpd handling this is separate. */
-	if (drm_fb_helper_initial_config(&ifbdev->helper))
-		intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
-}
-
-void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv)
-{
-	struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
-
-	if (!ifbdev)
-		return;
-
-	ifbdev->cookie = async_schedule(intel_fbdev_initial_config, ifbdev);
-}
-
 static void intel_fbdev_sync(struct intel_fbdev *ifbdev)
 {
 	if (!ifbdev->cookie)
@@ -520,31 +468,6 @@ static void intel_fbdev_sync(struct intel_fbdev *ifbdev)
 	ifbdev->cookie = 0;
 }
 
-void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
-{
-	struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
-
-	if (!ifbdev)
-		return;
-
-	intel_fbdev_set_suspend(&dev_priv->drm, FBINFO_STATE_SUSPENDED, true);
-
-	if (!current_is_async())
-		intel_fbdev_sync(ifbdev);
-
-	drm_fb_helper_unregister_info(&ifbdev->helper);
-}
-
-void intel_fbdev_fini(struct drm_i915_private *dev_priv)
-{
-	struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv->display.fbdev.fbdev);
-
-	if (!ifbdev)
-		return;
-
-	intel_fbdev_destroy(ifbdev);
-}
-
 /* Suspends/resumes fbdev processing of incoming HPD events. When resuming HPD
  * processing, fbdev will perform a full connector reprobe if a hotplug event
  * was received while HPD was suspended.
@@ -661,6 +584,83 @@ void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
 		intel_fbdev_invalidate(ifbdev);
 }
 
+int intel_fbdev_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_fbdev *ifbdev;
+	int ret;
+
+	if (drm_WARN_ON(dev, !HAS_DISPLAY(dev_priv)))
+		return -ENODEV;
+
+	ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
+	if (!ifbdev)
+		return -ENOMEM;
+
+	mutex_init(&ifbdev->hpd_lock);
+	drm_fb_helper_prepare(dev, &ifbdev->helper, 32, &intel_fb_helper_funcs);
+
+	if (intel_fbdev_init_bios(dev, ifbdev))
+		ifbdev->helper.preferred_bpp = ifbdev->preferred_bpp;
+	else
+		ifbdev->preferred_bpp = ifbdev->helper.preferred_bpp;
+
+	ret = drm_fb_helper_init(dev, &ifbdev->helper);
+	if (ret) {
+		kfree(ifbdev);
+		return ret;
+	}
+
+	dev_priv->display.fbdev.fbdev = ifbdev;
+	INIT_WORK(&dev_priv->display.fbdev.suspend_work, intel_fbdev_suspend_worker);
+
+	return 0;
+}
+
+static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
+{
+	struct intel_fbdev *ifbdev = data;
+
+	/* Due to peculiar init order wrt to hpd handling this is separate. */
+	if (drm_fb_helper_initial_config(&ifbdev->helper))
+		intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
+}
+
+void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv)
+{
+	struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
+
+	if (!ifbdev)
+		return;
+
+	ifbdev->cookie = async_schedule(intel_fbdev_initial_config, ifbdev);
+}
+
+void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
+{
+	struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
+
+	if (!ifbdev)
+		return;
+
+	intel_fbdev_set_suspend(&dev_priv->drm, FBINFO_STATE_SUSPENDED, true);
+
+	if (!current_is_async())
+		intel_fbdev_sync(ifbdev);
+
+	drm_fb_helper_unregister_info(&ifbdev->helper);
+}
+
+void intel_fbdev_fini(struct drm_i915_private *dev_priv)
+{
+	struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv->display.fbdev.fbdev);
+
+	if (!ifbdev)
+		return;
+
+	intel_fbdev_destroy(ifbdev);
+}
+
 struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev)
 {
 	if (!fbdev || !fbdev->helper.fb)
-- 
2.44.0

[PATCH v8 3/6] drm/i915: Initialize fbdev DRM client with callback functions

[Thomas Zimmermann]

Initialize i915's fbdev client by giving an instance of struct
drm_client_funcs to drm_client_init(). Also clean up with
drm_client_release().

Doing this in i915 prevents fbdev helpers from initializing and
releasing the client internally (see drm_fb_helper_init()). No
functional change yet; the client callbacks will be filled later.

v6:
- rename client to "intel-fbdev" (Jouni)

v2:
- call drm_fb_helper_unprepare() in error handling (Jani)
- fix typo in commit message (Sam)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbdev.c | 43 ++++++++++++++++++++--
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 2714e12a6c44c..4d0dba6c47d3a 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -291,6 +291,7 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
 	if (ifbdev->fb)
 		drm_framebuffer_remove(&ifbdev->fb->base);
 
+	drm_client_release(&ifbdev->helper.client);
 	drm_fb_helper_unprepare(&ifbdev->helper);
 	kfree(ifbdev);
 }
@@ -584,6 +585,30 @@ void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
 		intel_fbdev_invalidate(ifbdev);
 }
 
+/*
+ * Fbdev client and struct drm_client_funcs
+ */
+
+static void intel_fbdev_client_unregister(struct drm_client_dev *client)
+{ }
+
+static int intel_fbdev_client_restore(struct drm_client_dev *client)
+{
+	return 0;
+}
+
+static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
+{
+	return 0;
+}
+
+static const struct drm_client_funcs intel_fbdev_client_funcs = {
+	.owner		= THIS_MODULE,
+	.unregister	= intel_fbdev_client_unregister,
+	.restore	= intel_fbdev_client_restore,
+	.hotplug	= intel_fbdev_client_hotplug,
+};
+
 int intel_fbdev_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -605,16 +630,26 @@ int intel_fbdev_init(struct drm_device *dev)
 	else
 		ifbdev->preferred_bpp = ifbdev->helper.preferred_bpp;
 
+	ret = drm_client_init(dev, &ifbdev->helper.client, "intel-fbdev",
+			      &intel_fbdev_client_funcs);
+	if (ret)
+		goto err_drm_fb_helper_unprepare;
+
 	ret = drm_fb_helper_init(dev, &ifbdev->helper);
-	if (ret) {
-		kfree(ifbdev);
-		return ret;
-	}
+	if (ret)
+		goto err_drm_client_release;
 
 	dev_priv->display.fbdev.fbdev = ifbdev;
 	INIT_WORK(&dev_priv->display.fbdev.suspend_work, intel_fbdev_suspend_worker);
 
 	return 0;
+
+err_drm_client_release:
+	drm_client_release(&ifbdev->helper.client);
+err_drm_fb_helper_unprepare:
+	drm_fb_helper_unprepare(&ifbdev->helper);
+	kfree(ifbdev);
+	return ret;
 }
 
 static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
-- 
2.44.0

[PATCH v8 4/6] drm/{i915,xe}: Unregister in-kernel clients

[Thomas Zimmermann]

Unregister all in-kernel clients before unloading the i915 driver. For
other drivers, drm_dev_unregister() does this automatically. As i915 and
xe do not use this helper, they have to perform the call by themselves.

Note that there are currently no in-kernel clients in i915 or xe. The
patch prepares the drivers for a related update of their fbdev support.

v8:
- unregister clients in intel_display_driver_unregister() (Jani)
- mention xe in commit message (Rodrigo, Jani)

v7:
- update xe driver

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/i915/display/intel_display_driver.c | 3 +++
 drivers/gpu/drm/xe/xe_device.c                      | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 87dd07e0d138d..b7d636980d83a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -11,6 +11,7 @@
 #include <acpi/video.h>
 #include <drm/display/drm_dp_mst_helper.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_client.h>
 #include <drm/drm_mode_config.h>
 #include <drm/drm_privacy_screen_consumer.h>
 #include <drm/drm_probe_helper.h>
@@ -638,6 +639,8 @@ void intel_display_driver_unregister(struct drm_i915_private *i915)
 	if (!HAS_DISPLAY(i915))
 		return;
 
+	drm_client_dev_unregister(&i915->drm);
+
 	intel_fbdev_unregister(i915);
 	/*
 	 * After flushing the fbdev (incl. a late async config which
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 01bd5ccf05ca6..231ab2f4cd0b9 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -9,6 +9,7 @@
 
 #include <drm/drm_aperture.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_client.h>
 #include <drm/drm_gem_ttm_helper.h>
 #include <drm/drm_ioctl.h>
 #include <drm/drm_managed.h>
-- 
2.44.0

[PATCH v8 5/6] drm/{i915,xe}: Implement fbdev client callbacks

[Thomas Zimmermann]

Move code from ad-hoc fbdev callbacks into DRM client functions
and remove the old callbacks. The functions instruct the client
to poll for changed output or restore the display.

The DRM core calls both, the old callbacks and the new client
helpers, from the same places. The new functions perform the same
operation as before, so there's no change in functionality.

Fox xe, remove xe_display_last_close(), which restored the fbdev
display. As with i915, the DRM core's drm_lastclose() performs
this operation automatically.

v8:
- mention xe in commit message

v7:
- update xe driver

v6:
- return errors from client callbacks (Jouni)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
---
 .../drm/i915/display/intel_display_driver.c   |  1 -
 drivers/gpu/drm/i915/display/intel_fbdev.c    | 33 ++++++++++++++-----
 drivers/gpu/drm/i915/display/intel_fbdev.h    |  9 -----
 drivers/gpu/drm/i915/i915_driver.c            | 22 -------------
 drivers/gpu/drm/xe/display/xe_display.c       |  9 -----
 5 files changed, 25 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
index b7d636980d83a..e5f052d4ff1cc 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -99,7 +99,6 @@ void intel_display_driver_init_hw(struct drm_i915_private *i915)
 static const struct drm_mode_config_funcs intel_mode_funcs = {
 	.fb_create = intel_user_framebuffer_create,
 	.get_format_info = intel_fb_get_format_info,
-	.output_poll_changed = intel_fbdev_output_poll_changed,
 	.mode_valid = intel_mode_valid,
 	.atomic_check = intel_atomic_check,
 	.atomic_commit = intel_atomic_commit,
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 4d0dba6c47d3a..f783de611a7f5 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -551,13 +551,13 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
 	intel_fbdev_hpd_set_suspend(dev_priv, state);
 }
 
-void intel_fbdev_output_poll_changed(struct drm_device *dev)
+static int intel_fbdev_output_poll_changed(struct drm_device *dev)
 {
 	struct intel_fbdev *ifbdev = to_i915(dev)->display.fbdev.fbdev;
 	bool send_hpd;
 
 	if (!ifbdev)
-		return;
+		return -EINVAL;
 
 	intel_fbdev_sync(ifbdev);
 
@@ -568,21 +568,29 @@ void intel_fbdev_output_poll_changed(struct drm_device *dev)
 
 	if (send_hpd && (ifbdev->vma || ifbdev->helper.deferred_setup))
 		drm_fb_helper_hotplug_event(&ifbdev->helper);
+
+	return 0;
 }
 
-void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
+static int intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
 {
 	struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
+	int ret;
 
 	if (!ifbdev)
-		return;
+		return -EINVAL;
 
 	intel_fbdev_sync(ifbdev);
 	if (!ifbdev->vma)
-		return;
+		return -ENOMEM;
 
-	if (drm_fb_helper_restore_fbdev_mode_unlocked(&ifbdev->helper) == 0)
-		intel_fbdev_invalidate(ifbdev);
+	ret = drm_fb_helper_restore_fbdev_mode_unlocked(&ifbdev->helper);
+	if (ret)
+		return ret;
+
+	intel_fbdev_invalidate(ifbdev);
+
+	return 0;
 }
 
 /*
@@ -594,12 +602,21 @@ static void intel_fbdev_client_unregister(struct drm_client_dev *client)
 
 static int intel_fbdev_client_restore(struct drm_client_dev *client)
 {
+	struct drm_i915_private *dev_priv = to_i915(client->dev);
+	int ret;
+
+	ret = intel_fbdev_restore_mode(dev_priv);
+	if (ret)
+		return ret;
+
+	vga_switcheroo_process_delayed_switch();
+
 	return 0;
 }
 
 static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
 {
-	return 0;
+	return intel_fbdev_output_poll_changed(client->dev);
 }
 
 static const struct drm_client_funcs intel_fbdev_client_funcs = {
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h b/drivers/gpu/drm/i915/display/intel_fbdev.h
index 04fd523a50232..8c953f102ba22 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.h
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.h
@@ -19,8 +19,6 @@ void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv);
 void intel_fbdev_unregister(struct drm_i915_private *dev_priv);
 void intel_fbdev_fini(struct drm_i915_private *dev_priv);
 void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
-void intel_fbdev_output_poll_changed(struct drm_device *dev);
-void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv);
 struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev);
 #else
 static inline int intel_fbdev_init(struct drm_device *dev)
@@ -44,13 +42,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_i915_private *i915)
-{
-}
 static inline struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev)
 {
 	return NULL;
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 4b9233c07a22c..6551c806e2ae0 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -920,27 +920,6 @@ static int i915_driver_open(struct drm_device *dev, struct drm_file *file)
 	return 0;
 }
 
-/**
- * i915_driver_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).
- *
- * Additionally, in the non-mode setting case, we'll tear down the GTT
- * and DMA structures, since the kernel won't be using them, and clea
- * up any GEM state.
- */
-static void i915_driver_lastclose(struct drm_device *dev)
-{
-	struct drm_i915_private *i915 = to_i915(dev);
-
-	intel_fbdev_restore_mode(i915);
-
-	vga_switcheroo_process_delayed_switch();
-}
-
 static void i915_driver_postclose(struct drm_device *dev, struct drm_file *file)
 {
 	struct drm_i915_file_private *file_priv = file->driver_priv;
@@ -1831,7 +1810,6 @@ static const struct drm_driver i915_drm_driver = {
 	    DRIVER_SYNCOBJ_TIMELINE,
 	.release = i915_driver_release,
 	.open = i915_driver_open,
-	.lastclose = i915_driver_lastclose,
 	.postclose = i915_driver_postclose,
 	.show_fdinfo = PTR_IF(IS_ENABLED(CONFIG_PROC_FS), i915_drm_client_fdinfo),
 
diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
index e4db069f0db3f..cdbc3f04c80a7 100644
--- a/drivers/gpu/drm/xe/display/xe_display.c
+++ b/drivers/gpu/drm/xe/display/xe_display.c
@@ -51,14 +51,6 @@ bool xe_display_driver_probe_defer(struct pci_dev *pdev)
 	return intel_display_driver_probe_defer(pdev);
 }
 
-static void xe_display_last_close(struct drm_device *dev)
-{
-	struct xe_device *xe = to_xe_device(dev);
-
-	if (xe->info.enable_display)
-		intel_fbdev_restore_mode(to_xe_device(dev));
-}
-
 /**
  * xe_display_driver_set_hooks - Add driver flags and hooks for display
  * @driver: DRM device driver
@@ -73,7 +65,6 @@ void xe_display_driver_set_hooks(struct drm_driver *driver)
 		return;
 
 	driver->driver_features |= DRIVER_MODESET | DRIVER_ATOMIC;
-	driver->lastclose = xe_display_last_close;
 }
 
 static void unset_display_features(struct xe_device *xe)
-- 
2.44.0

[PATCH v8 6/6] drm/{i915, xe}: Implement fbdev emulation as in-kernel client

[Thomas Zimmermann]

Replace all code that initializes or releases fbdev emulation
throughout the driver. Instead initialize the fbdev client by a
single call to intel_fbdev_setup() after i915 has registered its
DRM device. Just like similar code in other drivers, i915 fbdev
emulation now acts like a regular DRM client. Do the same for xe.

The fbdev client setup consists of the initial preparation and the
hot-plugging of the display. The latter creates the fbdev device
and sets up the fbdev framebuffer. The setup performs display
hot-plugging once. If no display can be detected, DRM probe helpers
re-run the detection on each hotplug event.

A call to drm_client_dev_unregister() releases all in-kernel clients
automatically. No further action is required within i915. If the fbdev
framebuffer has been fully set up, struct fb_ops.fb_destroy implements
the release. For partially initialized emulation, the fbdev client
reverts the initial setup. Do the same for xe and remove its call to
intel_fbdev_fini().

v8:
- setup client in intel_display_driver_register (Jouni)
- mention xe in commit message

v7:
- update xe driver
- reword commit message

v6:
- use 'i915' for i915 device (Jouni)
- remove unnecessary code for non-atomic mode setting (Jouni, Ville)
- fix function name in commit message (Jouni)

v3:
-  as before, silently ignore devices without displays

v2:
-  let drm_client_register() handle initial hotplug
-  fix driver name in error message (Jani)
-  fix non-fbdev build (kernel test robot)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/i915/display/intel_display.c  |   1 -
 .../drm/i915/display/intel_display_driver.c   |  20 +-
 drivers/gpu/drm/i915/display/intel_fbdev.c    | 177 ++++++++----------
 drivers/gpu/drm/i915/display/intel_fbdev.h    |  20 +-
 drivers/gpu/drm/xe/display/xe_display.c       |   2 -
 5 files changed, 80 insertions(+), 140 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 614e60420a29a..161a5aabf6746 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -85,7 +85,6 @@
 #include "intel_dvo.h"
 #include "intel_fb.h"
 #include "intel_fbc.h"
-#include "intel_fbdev.h"
 #include "intel_fdi.h"
 #include "intel_fifo_underrun.h"
 #include "intel_frontbuffer.h"
diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
index e5f052d4ff1cc..ed8589fa35448 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -514,10 +514,6 @@ int intel_display_driver_probe(struct drm_i915_private *i915)
 
 	intel_overlay_setup(i915);
 
-	ret = intel_fbdev_init(&i915->drm);
-	if (ret)
-		return ret;
-
 	/* Only enable hotplug handling once the fbdev is fully set up. */
 	intel_hpd_init(i915);
 
@@ -544,16 +540,6 @@ void intel_display_driver_register(struct drm_i915_private *i915)
 
 	intel_display_debugfs_register(i915);
 
-	/*
-	 * Some ports require correctly set-up hpd registers for
-	 * detection to work properly (leading to ghost connected
-	 * connector status), e.g. VGA on gm45.  Hence we can only set
-	 * up the initial fbdev config after hpd irqs are fully
-	 * enabled. We do it last so that the async config cannot run
-	 * before the connectors are registered.
-	 */
-	intel_fbdev_initial_config_async(i915);
-
 	/*
 	 * We need to coordinate the hotplugs with the asynchronous
 	 * fbdev configuration, for which we use the
@@ -562,6 +548,8 @@ void intel_display_driver_register(struct drm_i915_private *i915)
 	drm_kms_helper_poll_init(&i915->drm);
 	intel_hpd_poll_disable(i915);
 
+	intel_fbdev_setup(i915);
+
 	intel_display_device_info_print(DISPLAY_INFO(i915),
 					DISPLAY_RUNTIME_INFO(i915), &p);
 }
@@ -597,9 +585,6 @@ void intel_display_driver_remove_noirq(struct drm_i915_private *i915)
 	 */
 	intel_hpd_poll_fini(i915);
 
-	/* poll work can call into fbdev, hence clean that up afterwards */
-	intel_fbdev_fini(i915);
-
 	intel_unregister_dsm_handler();
 
 	/* flush any delayed tasks or pending work */
@@ -640,7 +625,6 @@ void intel_display_driver_unregister(struct drm_i915_private *i915)
 
 	drm_client_dev_unregister(&i915->drm);
 
-	intel_fbdev_unregister(i915);
 	/*
 	 * After flushing the fbdev (incl. a late async config which
 	 * will have delayed queuing of a hotplug event), then flush
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index f783de611a7f5..bda702c2cab8e 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -24,7 +24,6 @@
  *     David Airlie
  */
 
-#include <linux/async.h>
 #include <linux/console.h>
 #include <linux/delay.h>
 #include <linux/errno.h>
@@ -39,6 +38,7 @@
 #include <linux/vga_switcheroo.h>
 
 #include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_gem_framebuffer_helper.h>
@@ -58,7 +58,6 @@ struct intel_fbdev {
 	struct intel_framebuffer *fb;
 	struct i915_vma *vma;
 	unsigned long vma_flags;
-	async_cookie_t cookie;
 	int preferred_bpp;
 
 	/* Whether or not fbdev hpd processing is temporarily suspended */
@@ -135,6 +134,26 @@ static int intel_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
 	return i915_gem_fb_mmap(obj, vma);
 }
 
+static void intel_fbdev_fb_destroy(struct fb_info *info)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	struct intel_fbdev *ifbdev = container_of(fb_helper, struct intel_fbdev, helper);
+
+	drm_fb_helper_fini(&ifbdev->helper);
+
+	/*
+	 * 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.
+	 */
+	intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags);
+	drm_framebuffer_remove(&ifbdev->fb->base);
+
+	drm_client_release(&fb_helper->client);
+	drm_fb_helper_unprepare(&ifbdev->helper);
+	kfree(ifbdev);
+}
+
 __diag_push();
 __diag_ignore_all("-Woverride-init", "Allow field initialization overrides for fb ops");
 
@@ -147,6 +166,7 @@ static const struct fb_ops intelfb_ops = {
 	.fb_pan_display = intel_fbdev_pan_display,
 	__FB_DEFAULT_DEFERRED_OPS_DRAW(intel_fbdev),
 	.fb_mmap = intel_fbdev_mmap,
+	.fb_destroy = intel_fbdev_fb_destroy,
 };
 
 __diag_pop();
@@ -158,7 +178,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	struct intel_framebuffer *intel_fb = ifbdev->fb;
 	struct drm_device *dev = helper->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
 	const struct i915_gtt_view view = {
 		.type = I915_GTT_VIEW_NORMAL,
 	};
@@ -250,7 +269,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	ifbdev->vma_flags = flags;
 
 	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
-	vga_switcheroo_client_fb_set(pdev, info);
+
 	return 0;
 
 out_unpin:
@@ -276,26 +295,6 @@ static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
 	.fb_dirty = intelfb_dirty,
 };
 
-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);
-
-	if (ifbdev->vma)
-		intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags);
-
-	if (ifbdev->fb)
-		drm_framebuffer_remove(&ifbdev->fb->base);
-
-	drm_client_release(&ifbdev->helper.client);
-	drm_fb_helper_unprepare(&ifbdev->helper);
-	kfree(ifbdev);
-}
-
 /*
  * Build an intel_fbdev struct using a BIOS allocated framebuffer, if possible.
  * The core display code will have read out the current plane configuration,
@@ -459,16 +458,6 @@ static void intel_fbdev_suspend_worker(struct work_struct *work)
 				true);
 }
 
-static void intel_fbdev_sync(struct intel_fbdev *ifbdev)
-{
-	if (!ifbdev->cookie)
-		return;
-
-	/* Only serialises with all preceding async calls, hence +1 */
-	async_synchronize_cookie(ifbdev->cookie + 1);
-	ifbdev->cookie = 0;
-}
-
 /* Suspends/resumes fbdev processing of incoming HPD events. When resuming HPD
  * processing, fbdev will perform a full connector reprobe if a hotplug event
  * was received while HPD was suspended.
@@ -559,8 +548,6 @@ static int intel_fbdev_output_poll_changed(struct drm_device *dev)
 	if (!ifbdev)
 		return -EINVAL;
 
-	intel_fbdev_sync(ifbdev);
-
 	mutex_lock(&ifbdev->hpd_lock);
 	send_hpd = !ifbdev->hpd_suspended;
 	ifbdev->hpd_waiting = true;
@@ -580,7 +567,6 @@ static int intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
 	if (!ifbdev)
 		return -EINVAL;
 
-	intel_fbdev_sync(ifbdev);
 	if (!ifbdev->vma)
 		return -ENOMEM;
 
@@ -598,7 +584,20 @@ static int intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
  */
 
 static void intel_fbdev_client_unregister(struct drm_client_dev *client)
-{ }
+{
+	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
+	struct drm_device *dev = fb_helper->dev;
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+
+	if (fb_helper->info) {
+		vga_switcheroo_client_fb_set(pdev, NULL);
+		drm_fb_helper_unregister_info(fb_helper);
+	} else {
+		drm_fb_helper_unprepare(fb_helper);
+		drm_client_release(&fb_helper->client);
+		kfree(fb_helper);
+	}
+}
 
 static int intel_fbdev_client_restore(struct drm_client_dev *client)
 {
@@ -616,7 +615,31 @@ static int intel_fbdev_client_restore(struct drm_client_dev *client)
 
 static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
 {
-	return intel_fbdev_output_poll_changed(client->dev);
+	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
+	struct drm_device *dev = client->dev;
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	int ret;
+
+	if (dev->fb_helper)
+		return intel_fbdev_output_poll_changed(dev);
+
+	ret = drm_fb_helper_init(dev, fb_helper);
+	if (ret)
+		goto err_drm_err;
+
+	ret = drm_fb_helper_initial_config(fb_helper);
+	if (ret)
+		goto err_drm_fb_helper_fini;
+
+	vga_switcheroo_client_fb_set(pdev, fb_helper->info);
+
+	return 0;
+
+err_drm_fb_helper_fini:
+	drm_fb_helper_fini(fb_helper);
+err_drm_err:
+	drm_err(dev, "Failed to setup i915 fbdev emulation (ret=%d)\n", ret);
+	return ret;
 }
 
 static const struct drm_client_funcs intel_fbdev_client_funcs = {
@@ -626,22 +649,23 @@ static const struct drm_client_funcs intel_fbdev_client_funcs = {
 	.hotplug	= intel_fbdev_client_hotplug,
 };
 
-int intel_fbdev_init(struct drm_device *dev)
+void intel_fbdev_setup(struct drm_i915_private *i915)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_device *dev = &i915->drm;
 	struct intel_fbdev *ifbdev;
 	int ret;
 
-	if (drm_WARN_ON(dev, !HAS_DISPLAY(dev_priv)))
-		return -ENODEV;
+	if (!HAS_DISPLAY(i915))
+		return;
 
 	ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
 	if (!ifbdev)
-		return -ENOMEM;
-
-	mutex_init(&ifbdev->hpd_lock);
+		return;
 	drm_fb_helper_prepare(dev, &ifbdev->helper, 32, &intel_fb_helper_funcs);
 
+	i915->display.fbdev.fbdev = ifbdev;
+	INIT_WORK(&i915->display.fbdev.suspend_work, intel_fbdev_suspend_worker);
+	mutex_init(&ifbdev->hpd_lock);
 	if (intel_fbdev_init_bios(dev, ifbdev))
 		ifbdev->helper.preferred_bpp = ifbdev->preferred_bpp;
 	else
@@ -649,68 +673,19 @@ int intel_fbdev_init(struct drm_device *dev)
 
 	ret = drm_client_init(dev, &ifbdev->helper.client, "intel-fbdev",
 			      &intel_fbdev_client_funcs);
-	if (ret)
+	if (ret) {
+		drm_err(dev, "Failed to register client: %d\n", ret);
 		goto err_drm_fb_helper_unprepare;
+	}
 
-	ret = drm_fb_helper_init(dev, &ifbdev->helper);
-	if (ret)
-		goto err_drm_client_release;
-
-	dev_priv->display.fbdev.fbdev = ifbdev;
-	INIT_WORK(&dev_priv->display.fbdev.suspend_work, intel_fbdev_suspend_worker);
+	drm_client_register(&ifbdev->helper.client);
 
-	return 0;
+	return;
 
-err_drm_client_release:
-	drm_client_release(&ifbdev->helper.client);
 err_drm_fb_helper_unprepare:
 	drm_fb_helper_unprepare(&ifbdev->helper);
+	mutex_destroy(&ifbdev->hpd_lock);
 	kfree(ifbdev);
-	return ret;
-}
-
-static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
-{
-	struct intel_fbdev *ifbdev = data;
-
-	/* Due to peculiar init order wrt to hpd handling this is separate. */
-	if (drm_fb_helper_initial_config(&ifbdev->helper))
-		intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
-}
-
-void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv)
-{
-	struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
-
-	if (!ifbdev)
-		return;
-
-	ifbdev->cookie = async_schedule(intel_fbdev_initial_config, ifbdev);
-}
-
-void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
-{
-	struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
-
-	if (!ifbdev)
-		return;
-
-	intel_fbdev_set_suspend(&dev_priv->drm, FBINFO_STATE_SUSPENDED, true);
-
-	if (!current_is_async())
-		intel_fbdev_sync(ifbdev);
-
-	drm_fb_helper_unregister_info(&ifbdev->helper);
-}
-
-void intel_fbdev_fini(struct drm_i915_private *dev_priv)
-{
-	struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv->display.fbdev.fbdev);
-
-	if (!ifbdev)
-		return;
-
-	intel_fbdev_destroy(ifbdev);
 }
 
 struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev)
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h b/drivers/gpu/drm/i915/display/intel_fbdev.h
index 8c953f102ba22..08de2d5b34338 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.h
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.h
@@ -14,27 +14,11 @@ struct intel_fbdev;
 struct intel_framebuffer;
 
 #ifdef CONFIG_DRM_FBDEV_EMULATION
-int intel_fbdev_init(struct drm_device *dev);
-void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv);
-void intel_fbdev_unregister(struct drm_i915_private *dev_priv);
-void intel_fbdev_fini(struct drm_i915_private *dev_priv);
+void intel_fbdev_setup(struct drm_i915_private *dev_priv);
 void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
 struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev);
 #else
-static inline int intel_fbdev_init(struct drm_device *dev)
-{
-	return 0;
-}
-
-static inline void intel_fbdev_initial_config_async(struct drm_i915_private *dev_priv)
-{
-}
-
-static inline void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
-{
-}
-
-static inline void intel_fbdev_fini(struct drm_i915_private *dev_priv)
+static inline void intel_fbdev_setup(struct drm_i915_private *dev_priv)
 {
 }
 
diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
index cdbc3f04c80a7..ca5cbe1d8a03b 100644
--- a/drivers/gpu/drm/xe/display/xe_display.c
+++ b/drivers/gpu/drm/xe/display/xe_display.c
@@ -214,9 +214,7 @@ void xe_display_fini(struct xe_device *xe)
 	if (!xe->info.enable_display)
 		return;
 
-	/* poll work can call into fbdev, hence clean that up afterwards */
 	intel_hpd_poll_fini(xe);
-	intel_fbdev_fini(xe);
 
 	intel_hdcp_component_fini(xe);
 	intel_audio_deinit(xe);
-- 
2.44.0

Re: [PATCH v8 1/6] drm/client: Export drm_client_dev_unregister()

[Hogander, Jouni]

On Tue, 2024-04-09 at 10:04 +0200, Thomas Zimmermann wrote:
> Export drm_client_dev_unregister() for use by the i915 driver. The
> driver does not use drm_dev_unregister(), so it has to clean up the
> in-kernel DRM clients by itself.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Jouni Högander <jouni.hogander@intel.com>

> ---
>  drivers/gpu/drm/drm_client.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_client.c
> b/drivers/gpu/drm/drm_client.c
> index 77fe217aeaf36..2803ac111bbd8 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -172,6 +172,18 @@ void drm_client_release(struct drm_client_dev
> *client)
>  }
>  EXPORT_SYMBOL(drm_client_release);
>  
> +/**
> + * drm_client_dev_unregister - Unregister clients
> + * @dev: DRM device
> + *
> + * This function releases all clients by calling each client's
> + * &drm_client_funcs.unregister callback. The callback function
> + * is responsibe for releaseing all resources including the client
> + * itself.
> + *
> + * The helper drm_dev_unregister() calls this function. Drivers
> + * that use it don't need to call this function themselves.
> + */
>  void drm_client_dev_unregister(struct drm_device *dev)
>  {
>         struct drm_client_dev *client, *tmp;
> @@ -191,6 +203,7 @@ void drm_client_dev_unregister(struct drm_device
> *dev)
>         }
>         mutex_unlock(&dev->clientlist_mutex);
>  }
> +EXPORT_SYMBOL(drm_client_dev_unregister);
>  
>  /**
>   * drm_client_dev_hotplug - Send hotplug event to clients

Re: [PATCH v8 4/6] drm/{i915,xe}: Unregister in-kernel clients

[Hogander, Jouni]

On Tue, 2024-04-09 at 10:04 +0200, Thomas Zimmermann wrote:
> Unregister all in-kernel clients before unloading the i915 driver.
> For
> other drivers, drm_dev_unregister() does this automatically. As i915
> and
> xe do not use this helper, they have to perform the call by
> themselves.
> 
> Note that there are currently no in-kernel clients in i915 or xe. The
> patch prepares the drivers for a related update of their fbdev
> support.
> 
> v8:
> - unregister clients in intel_display_driver_unregister() (Jani)
> - mention xe in commit message (Rodrigo, Jani)
> 
> v7:
> - update xe driver
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Jouni Högander <jouni.hogander@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display_driver.c | 3 +++
>  drivers/gpu/drm/xe/xe_device.c                      | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c
> b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index 87dd07e0d138d..b7d636980d83a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> @@ -11,6 +11,7 @@
>  #include <acpi/video.h>
>  #include <drm/display/drm_dp_mst_helper.h>
>  #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_client.h>
>  #include <drm/drm_mode_config.h>
>  #include <drm/drm_privacy_screen_consumer.h>
>  #include <drm/drm_probe_helper.h>
> @@ -638,6 +639,8 @@ void intel_display_driver_unregister(struct
> drm_i915_private *i915)
>         if (!HAS_DISPLAY(i915))
>                 return;
>  
> +       drm_client_dev_unregister(&i915->drm);
> +
>         intel_fbdev_unregister(i915);
>         /*
>          * After flushing the fbdev (incl. a late async config which
> diff --git a/drivers/gpu/drm/xe/xe_device.c
> b/drivers/gpu/drm/xe/xe_device.c
> index 01bd5ccf05ca6..231ab2f4cd0b9 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -9,6 +9,7 @@
>  
>  #include <drm/drm_aperture.h>
>  #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_client.h>
>  #include <drm/drm_gem_ttm_helper.h>
>  #include <drm/drm_ioctl.h>
>  #include <drm/drm_managed.h>

Re: [PATCH v8 6/6] drm/{i915,xe}: Implement fbdev emulation as in-kernel client

[Hogander, Jouni]

On Tue, 2024-04-09 at 10:04 +0200, Thomas Zimmermann wrote:
> Replace all code that initializes or releases fbdev emulation
> throughout the driver. Instead initialize the fbdev client by a
> single call to intel_fbdev_setup() after i915 has registered its
> DRM device. Just like similar code in other drivers, i915 fbdev
> emulation now acts like a regular DRM client. Do the same for xe.
> 
> The fbdev client setup consists of the initial preparation and the
> hot-plugging of the display. The latter creates the fbdev device
> and sets up the fbdev framebuffer. The setup performs display
> hot-plugging once. If no display can be detected, DRM probe helpers
> re-run the detection on each hotplug event.
> 
> A call to drm_client_dev_unregister() releases all in-kernel clients
> automatically. No further action is required within i915. If the
> fbdev
> framebuffer has been fully set up, struct fb_ops.fb_destroy
> implements
> the release. For partially initialized emulation, the fbdev client
> reverts the initial setup. Do the same for xe and remove its call to
> intel_fbdev_fini().
> 
> v8:
> - setup client in intel_display_driver_register (Jouni)
> - mention xe in commit message
> 
> v7:
> - update xe driver
> - reword commit message
> 
> v6:
> - use 'i915' for i915 device (Jouni)
> - remove unnecessary code for non-atomic mode setting (Jouni, Ville)
> - fix function name in commit message (Jouni)
> 
> v3:
> -  as before, silently ignore devices without displays
> 
> v2:
> -  let drm_client_register() handle initial hotplug
> -  fix driver name in error message (Jani)
> -  fix non-fbdev build (kernel test robot)
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Jouni Högander <jouni.hogander@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |   1 -
>  .../drm/i915/display/intel_display_driver.c   |  20 +-
>  drivers/gpu/drm/i915/display/intel_fbdev.c    | 177 ++++++++--------
> --
>  drivers/gpu/drm/i915/display/intel_fbdev.h    |  20 +-
>  drivers/gpu/drm/xe/display/xe_display.c       |   2 -
>  5 files changed, 80 insertions(+), 140 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 614e60420a29a..161a5aabf6746 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -85,7 +85,6 @@
>  #include "intel_dvo.h"
>  #include "intel_fb.h"
>  #include "intel_fbc.h"
> -#include "intel_fbdev.h"
>  #include "intel_fdi.h"
>  #include "intel_fifo_underrun.h"
>  #include "intel_frontbuffer.h"
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c
> b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index e5f052d4ff1cc..ed8589fa35448 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> @@ -514,10 +514,6 @@ int intel_display_driver_probe(struct
> drm_i915_private *i915)
>  
>         intel_overlay_setup(i915);
>  
> -       ret = intel_fbdev_init(&i915->drm);
> -       if (ret)
> -               return ret;
> -
>         /* Only enable hotplug handling once the fbdev is fully set
> up. */
>         intel_hpd_init(i915);
>  
> @@ -544,16 +540,6 @@ void intel_display_driver_register(struct
> drm_i915_private *i915)
>  
>         intel_display_debugfs_register(i915);
>  
> -       /*
> -        * Some ports require correctly set-up hpd registers for
> -        * detection to work properly (leading to ghost connected
> -        * connector status), e.g. VGA on gm45.  Hence we can only
> set
> -        * up the initial fbdev config after hpd irqs are fully
> -        * enabled. We do it last so that the async config cannot run
> -        * before the connectors are registered.
> -        */
> -       intel_fbdev_initial_config_async(i915);
> -
>         /*
>          * We need to coordinate the hotplugs with the asynchronous
>          * fbdev configuration, for which we use the
> @@ -562,6 +548,8 @@ void intel_display_driver_register(struct
> drm_i915_private *i915)
>         drm_kms_helper_poll_init(&i915->drm);
>         intel_hpd_poll_disable(i915);
>  
> +       intel_fbdev_setup(i915);
> +
>         intel_display_device_info_print(DISPLAY_INFO(i915),
>                                         DISPLAY_RUNTIME_INFO(i915),
> &p);
>  }
> @@ -597,9 +585,6 @@ void intel_display_driver_remove_noirq(struct
> drm_i915_private *i915)
>          */
>         intel_hpd_poll_fini(i915);
>  
> -       /* poll work can call into fbdev, hence clean that up
> afterwards */
> -       intel_fbdev_fini(i915);
> -
>         intel_unregister_dsm_handler();
>  
>         /* flush any delayed tasks or pending work */
> @@ -640,7 +625,6 @@ void intel_display_driver_unregister(struct
> drm_i915_private *i915)
>  
>         drm_client_dev_unregister(&i915->drm);
>  
> -       intel_fbdev_unregister(i915);
>         /*
>          * After flushing the fbdev (incl. a late async config which
>          * will have delayed queuing of a hotplug event), then flush
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
> b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index f783de611a7f5..bda702c2cab8e 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -24,7 +24,6 @@
>   *     David Airlie
>   */
>  
> -#include <linux/async.h>
>  #include <linux/console.h>
>  #include <linux/delay.h>
>  #include <linux/errno.h>
> @@ -39,6 +38,7 @@
>  #include <linux/vga_switcheroo.h>
>  
>  #include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_gem_framebuffer_helper.h>
> @@ -58,7 +58,6 @@ struct intel_fbdev {
>         struct intel_framebuffer *fb;
>         struct i915_vma *vma;
>         unsigned long vma_flags;
> -       async_cookie_t cookie;
>         int preferred_bpp;
>  
>         /* Whether or not fbdev hpd processing is temporarily
> suspended */
> @@ -135,6 +134,26 @@ static int intel_fbdev_mmap(struct fb_info
> *info, struct vm_area_struct *vma)
>         return i915_gem_fb_mmap(obj, vma);
>  }
>  
> +static void intel_fbdev_fb_destroy(struct fb_info *info)
> +{
> +       struct drm_fb_helper *fb_helper = info->par;
> +       struct intel_fbdev *ifbdev = container_of(fb_helper, struct
> intel_fbdev, helper);
> +
> +       drm_fb_helper_fini(&ifbdev->helper);
> +
> +       /*
> +        * 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.
> +        */
> +       intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags);
> +       drm_framebuffer_remove(&ifbdev->fb->base);
> +
> +       drm_client_release(&fb_helper->client);
> +       drm_fb_helper_unprepare(&ifbdev->helper);
> +       kfree(ifbdev);
> +}
> +
>  __diag_push();
>  __diag_ignore_all("-Woverride-init", "Allow field initialization
> overrides for fb ops");
>  
> @@ -147,6 +166,7 @@ static const struct fb_ops intelfb_ops = {
>         .fb_pan_display = intel_fbdev_pan_display,
>         __FB_DEFAULT_DEFERRED_OPS_DRAW(intel_fbdev),
>         .fb_mmap = intel_fbdev_mmap,
> +       .fb_destroy = intel_fbdev_fb_destroy,
>  };
>  
>  __diag_pop();
> @@ -158,7 +178,6 @@ static int intelfb_create(struct drm_fb_helper
> *helper,
>         struct intel_framebuffer *intel_fb = ifbdev->fb;
>         struct drm_device *dev = helper->dev;
>         struct drm_i915_private *dev_priv = to_i915(dev);
> -       struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
>         const struct i915_gtt_view view = {
>                 .type = I915_GTT_VIEW_NORMAL,
>         };
> @@ -250,7 +269,7 @@ static int intelfb_create(struct drm_fb_helper
> *helper,
>         ifbdev->vma_flags = flags;
>  
>         intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
> -       vga_switcheroo_client_fb_set(pdev, info);
> +
>         return 0;
>  
>  out_unpin:
> @@ -276,26 +295,6 @@ static const struct drm_fb_helper_funcs
> intel_fb_helper_funcs = {
>         .fb_dirty = intelfb_dirty,
>  };
>  
> -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);
> -
> -       if (ifbdev->vma)
> -               intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags);
> -
> -       if (ifbdev->fb)
> -               drm_framebuffer_remove(&ifbdev->fb->base);
> -
> -       drm_client_release(&ifbdev->helper.client);
> -       drm_fb_helper_unprepare(&ifbdev->helper);
> -       kfree(ifbdev);
> -}
> -
>  /*
>   * Build an intel_fbdev struct using a BIOS allocated framebuffer,
> if possible.
>   * The core display code will have read out the current plane
> configuration,
> @@ -459,16 +458,6 @@ static void intel_fbdev_suspend_worker(struct
> work_struct *work)
>                                 true);
>  }
>  
> -static void intel_fbdev_sync(struct intel_fbdev *ifbdev)
> -{
> -       if (!ifbdev->cookie)
> -               return;
> -
> -       /* Only serialises with all preceding async calls, hence +1
> */
> -       async_synchronize_cookie(ifbdev->cookie + 1);
> -       ifbdev->cookie = 0;
> -}
> -
>  /* Suspends/resumes fbdev processing of incoming HPD events. When
> resuming HPD
>   * processing, fbdev will perform a full connector reprobe if a
> hotplug event
>   * was received while HPD was suspended.
> @@ -559,8 +548,6 @@ static int intel_fbdev_output_poll_changed(struct
> drm_device *dev)
>         if (!ifbdev)
>                 return -EINVAL;
>  
> -       intel_fbdev_sync(ifbdev);
> -
>         mutex_lock(&ifbdev->hpd_lock);
>         send_hpd = !ifbdev->hpd_suspended;
>         ifbdev->hpd_waiting = true;
> @@ -580,7 +567,6 @@ static int intel_fbdev_restore_mode(struct
> drm_i915_private *dev_priv)
>         if (!ifbdev)
>                 return -EINVAL;
>  
> -       intel_fbdev_sync(ifbdev);
>         if (!ifbdev->vma)
>                 return -ENOMEM;
>  
> @@ -598,7 +584,20 @@ static int intel_fbdev_restore_mode(struct
> drm_i915_private *dev_priv)
>   */
>  
>  static void intel_fbdev_client_unregister(struct drm_client_dev
> *client)
> -{ }
> +{
> +       struct drm_fb_helper *fb_helper =
> drm_fb_helper_from_client(client);
> +       struct drm_device *dev = fb_helper->dev;
> +       struct pci_dev *pdev = to_pci_dev(dev->dev);
> +
> +       if (fb_helper->info) {
> +               vga_switcheroo_client_fb_set(pdev, NULL);
> +               drm_fb_helper_unregister_info(fb_helper);
> +       } else {
> +               drm_fb_helper_unprepare(fb_helper);
> +               drm_client_release(&fb_helper->client);
> +               kfree(fb_helper);
> +       }
> +}
>  
>  static int intel_fbdev_client_restore(struct drm_client_dev *client)
>  {
> @@ -616,7 +615,31 @@ static int intel_fbdev_client_restore(struct
> drm_client_dev *client)
>  
>  static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
>  {
> -       return intel_fbdev_output_poll_changed(client->dev);
> +       struct drm_fb_helper *fb_helper =
> drm_fb_helper_from_client(client);
> +       struct drm_device *dev = client->dev;
> +       struct pci_dev *pdev = to_pci_dev(dev->dev);
> +       int ret;
> +
> +       if (dev->fb_helper)
> +               return intel_fbdev_output_poll_changed(dev);
> +
> +       ret = drm_fb_helper_init(dev, fb_helper);
> +       if (ret)
> +               goto err_drm_err;
> +
> +       ret = drm_fb_helper_initial_config(fb_helper);
> +       if (ret)
> +               goto err_drm_fb_helper_fini;
> +
> +       vga_switcheroo_client_fb_set(pdev, fb_helper->info);
> +
> +       return 0;
> +
> +err_drm_fb_helper_fini:
> +       drm_fb_helper_fini(fb_helper);
> +err_drm_err:
> +       drm_err(dev, "Failed to setup i915 fbdev emulation
> (ret=%d)\n", ret);
> +       return ret;
>  }
>  
>  static const struct drm_client_funcs intel_fbdev_client_funcs = {
> @@ -626,22 +649,23 @@ static const struct drm_client_funcs
> intel_fbdev_client_funcs = {
>         .hotplug        = intel_fbdev_client_hotplug,
>  };
>  
> -int intel_fbdev_init(struct drm_device *dev)
> +void intel_fbdev_setup(struct drm_i915_private *i915)
>  {
> -       struct drm_i915_private *dev_priv = to_i915(dev);
> +       struct drm_device *dev = &i915->drm;
>         struct intel_fbdev *ifbdev;
>         int ret;
>  
> -       if (drm_WARN_ON(dev, !HAS_DISPLAY(dev_priv)))
> -               return -ENODEV;
> +       if (!HAS_DISPLAY(i915))
> +               return;
>  
>         ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
>         if (!ifbdev)
> -               return -ENOMEM;
> -
> -       mutex_init(&ifbdev->hpd_lock);
> +               return;
>         drm_fb_helper_prepare(dev, &ifbdev->helper, 32,
> &intel_fb_helper_funcs);
>  
> +       i915->display.fbdev.fbdev = ifbdev;
> +       INIT_WORK(&i915->display.fbdev.suspend_work,
> intel_fbdev_suspend_worker);
> +       mutex_init(&ifbdev->hpd_lock);
>         if (intel_fbdev_init_bios(dev, ifbdev))
>                 ifbdev->helper.preferred_bpp = ifbdev->preferred_bpp;
>         else
> @@ -649,68 +673,19 @@ int intel_fbdev_init(struct drm_device *dev)
>  
>         ret = drm_client_init(dev, &ifbdev->helper.client, "intel-
> fbdev",
>                               &intel_fbdev_client_funcs);
> -       if (ret)
> +       if (ret) {
> +               drm_err(dev, "Failed to register client: %d\n", ret);
>                 goto err_drm_fb_helper_unprepare;
> +       }
>  
> -       ret = drm_fb_helper_init(dev, &ifbdev->helper);
> -       if (ret)
> -               goto err_drm_client_release;
> -
> -       dev_priv->display.fbdev.fbdev = ifbdev;
> -       INIT_WORK(&dev_priv->display.fbdev.suspend_work,
> intel_fbdev_suspend_worker);
> +       drm_client_register(&ifbdev->helper.client);
>  
> -       return 0;
> +       return;
>  
> -err_drm_client_release:
> -       drm_client_release(&ifbdev->helper.client);
>  err_drm_fb_helper_unprepare:
>         drm_fb_helper_unprepare(&ifbdev->helper);
> +       mutex_destroy(&ifbdev->hpd_lock);
>         kfree(ifbdev);
> -       return ret;
> -}
> -
> -static void intel_fbdev_initial_config(void *data, async_cookie_t
> cookie)
> -{
> -       struct intel_fbdev *ifbdev = data;
> -
> -       /* Due to peculiar init order wrt to hpd handling this is
> separate. */
> -       if (drm_fb_helper_initial_config(&ifbdev->helper))
> -               intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
> -}
> -
> -void intel_fbdev_initial_config_async(struct drm_i915_private
> *dev_priv)
> -{
> -       struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
> -
> -       if (!ifbdev)
> -               return;
> -
> -       ifbdev->cookie = async_schedule(intel_fbdev_initial_config,
> ifbdev);
> -}
> -
> -void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
> -{
> -       struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
> -
> -       if (!ifbdev)
> -               return;
> -
> -       intel_fbdev_set_suspend(&dev_priv->drm,
> FBINFO_STATE_SUSPENDED, true);
> -
> -       if (!current_is_async())
> -               intel_fbdev_sync(ifbdev);
> -
> -       drm_fb_helper_unregister_info(&ifbdev->helper);
> -}
> -
> -void intel_fbdev_fini(struct drm_i915_private *dev_priv)
> -{
> -       struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv-
> >display.fbdev.fbdev);
> -
> -       if (!ifbdev)
> -               return;
> -
> -       intel_fbdev_destroy(ifbdev);
>  }
>  
>  struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev
> *fbdev)
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h
> b/drivers/gpu/drm/i915/display/intel_fbdev.h
> index 8c953f102ba22..08de2d5b34338 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.h
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.h
> @@ -14,27 +14,11 @@ struct intel_fbdev;
>  struct intel_framebuffer;
>  
>  #ifdef CONFIG_DRM_FBDEV_EMULATION
> -int intel_fbdev_init(struct drm_device *dev);
> -void intel_fbdev_initial_config_async(struct drm_i915_private
> *dev_priv);
> -void intel_fbdev_unregister(struct drm_i915_private *dev_priv);
> -void intel_fbdev_fini(struct drm_i915_private *dev_priv);
> +void intel_fbdev_setup(struct drm_i915_private *dev_priv);
>  void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool
> synchronous);
>  struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev
> *fbdev);
>  #else
> -static inline int intel_fbdev_init(struct drm_device *dev)
> -{
> -       return 0;
> -}
> -
> -static inline void intel_fbdev_initial_config_async(struct
> drm_i915_private *dev_priv)
> -{
> -}
> -
> -static inline void intel_fbdev_unregister(struct drm_i915_private
> *dev_priv)
> -{
> -}
> -
> -static inline void intel_fbdev_fini(struct drm_i915_private
> *dev_priv)
> +static inline void intel_fbdev_setup(struct drm_i915_private
> *dev_priv)
>  {
>  }
>  
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c
> b/drivers/gpu/drm/xe/display/xe_display.c
> index cdbc3f04c80a7..ca5cbe1d8a03b 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -214,9 +214,7 @@ void xe_display_fini(struct xe_device *xe)
>         if (!xe->info.enable_display)
>                 return;
>  
> -       /* poll work can call into fbdev, hence clean that up
> afterwards */
>         intel_hpd_poll_fini(xe);
> -       intel_fbdev_fini(xe);
>  
>         intel_hdcp_component_fini(xe);
>         intel_audio_deinit(xe);

Re: [PATCH v8 6/6] drm/{i915,xe}: Implement fbdev emulation as in-kernel client

[Thomas Zimmermann]

Hi

Am 22.04.24 um 16:11 schrieb Hogander, Jouni:
> On Tue, 2024-04-09 at 10:04 +0200, Thomas Zimmermann wrote:
>> Replace all code that initializes or releases fbdev emulation
>> throughout the driver. Instead initialize the fbdev client by a
>> single call to intel_fbdev_setup() after i915 has registered its
>> DRM device. Just like similar code in other drivers, i915 fbdev
>> emulation now acts like a regular DRM client. Do the same for xe.
>>
>> The fbdev client setup consists of the initial preparation and the
>> hot-plugging of the display. The latter creates the fbdev device
>> and sets up the fbdev framebuffer. The setup performs display
>> hot-plugging once. If no display can be detected, DRM probe helpers
>> re-run the detection on each hotplug event.
>>
>> A call to drm_client_dev_unregister() releases all in-kernel clients
>> automatically. No further action is required within i915. If the
>> fbdev
>> framebuffer has been fully set up, struct fb_ops.fb_destroy
>> implements
>> the release. For partially initialized emulation, the fbdev client
>> reverts the initial setup. Do the same for xe and remove its call to
>> intel_fbdev_fini().
>>
>> v8:
>> - setup client in intel_display_driver_register (Jouni)
>> - mention xe in commit message
>>
>> v7:
>> - update xe driver
>> - reword commit message
>>
>> v6:
>> - use 'i915' for i915 device (Jouni)
>> - remove unnecessary code for non-atomic mode setting (Jouni, Ville)
>> - fix function name in commit message (Jouni)
>>
>> v3:
>> -  as before, silently ignore devices without displays
>>
>> v2:
>> -  let drm_client_register() handle initial hotplug
>> -  fix driver name in error message (Jani)
>> -  fix non-fbdev build (kernel test robot)
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Jouni Högander <jouni.hogander@intel.com>

Thank you so much. All patches has R-bs. Can you add the series to the 
intel tree?

Best regards
Thomas

>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c  |   1 -
>>   .../drm/i915/display/intel_display_driver.c   |  20 +-
>>   drivers/gpu/drm/i915/display/intel_fbdev.c    | 177 ++++++++--------
>> --
>>   drivers/gpu/drm/i915/display/intel_fbdev.h    |  20 +-
>>   drivers/gpu/drm/xe/display/xe_display.c       |   2 -
>>   5 files changed, 80 insertions(+), 140 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
>> b/drivers/gpu/drm/i915/display/intel_display.c
>> index 614e60420a29a..161a5aabf6746 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -85,7 +85,6 @@
>>   #include "intel_dvo.h"
>>   #include "intel_fb.h"
>>   #include "intel_fbc.h"
>> -#include "intel_fbdev.h"
>>   #include "intel_fdi.h"
>>   #include "intel_fifo_underrun.h"
>>   #include "intel_frontbuffer.h"
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c
>> b/drivers/gpu/drm/i915/display/intel_display_driver.c
>> index e5f052d4ff1cc..ed8589fa35448 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
>> @@ -514,10 +514,6 @@ int intel_display_driver_probe(struct
>> drm_i915_private *i915)
>>   
>>          intel_overlay_setup(i915);
>>   
>> -       ret = intel_fbdev_init(&i915->drm);
>> -       if (ret)
>> -               return ret;
>> -
>>          /* Only enable hotplug handling once the fbdev is fully set
>> up. */
>>          intel_hpd_init(i915);
>>   
>> @@ -544,16 +540,6 @@ void intel_display_driver_register(struct
>> drm_i915_private *i915)
>>   
>>          intel_display_debugfs_register(i915);
>>   
>> -       /*
>> -        * Some ports require correctly set-up hpd registers for
>> -        * detection to work properly (leading to ghost connected
>> -        * connector status), e.g. VGA on gm45.  Hence we can only
>> set
>> -        * up the initial fbdev config after hpd irqs are fully
>> -        * enabled. We do it last so that the async config cannot run
>> -        * before the connectors are registered.
>> -        */
>> -       intel_fbdev_initial_config_async(i915);
>> -
>>          /*
>>           * We need to coordinate the hotplugs with the asynchronous
>>           * fbdev configuration, for which we use the
>> @@ -562,6 +548,8 @@ void intel_display_driver_register(struct
>> drm_i915_private *i915)
>>          drm_kms_helper_poll_init(&i915->drm);
>>          intel_hpd_poll_disable(i915);
>>   
>> +       intel_fbdev_setup(i915);
>> +
>>          intel_display_device_info_print(DISPLAY_INFO(i915),
>>                                          DISPLAY_RUNTIME_INFO(i915),
>> &p);
>>   }
>> @@ -597,9 +585,6 @@ void intel_display_driver_remove_noirq(struct
>> drm_i915_private *i915)
>>           */
>>          intel_hpd_poll_fini(i915);
>>   
>> -       /* poll work can call into fbdev, hence clean that up
>> afterwards */
>> -       intel_fbdev_fini(i915);
>> -
>>          intel_unregister_dsm_handler();
>>   
>>          /* flush any delayed tasks or pending work */
>> @@ -640,7 +625,6 @@ void intel_display_driver_unregister(struct
>> drm_i915_private *i915)
>>   
>>          drm_client_dev_unregister(&i915->drm);
>>   
>> -       intel_fbdev_unregister(i915);
>>          /*
>>           * After flushing the fbdev (incl. a late async config which
>>           * will have delayed queuing of a hotplug event), then flush
>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
>> b/drivers/gpu/drm/i915/display/intel_fbdev.c
>> index f783de611a7f5..bda702c2cab8e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
>> @@ -24,7 +24,6 @@
>>    *     David Airlie
>>    */
>>   
>> -#include <linux/async.h>
>>   #include <linux/console.h>
>>   #include <linux/delay.h>
>>   #include <linux/errno.h>
>> @@ -39,6 +38,7 @@
>>   #include <linux/vga_switcheroo.h>
>>   
>>   #include <drm/drm_crtc.h>
>> +#include <drm/drm_crtc_helper.h>
>>   #include <drm/drm_fb_helper.h>
>>   #include <drm/drm_fourcc.h>
>>   #include <drm/drm_gem_framebuffer_helper.h>
>> @@ -58,7 +58,6 @@ struct intel_fbdev {
>>          struct intel_framebuffer *fb;
>>          struct i915_vma *vma;
>>          unsigned long vma_flags;
>> -       async_cookie_t cookie;
>>          int preferred_bpp;
>>   
>>          /* Whether or not fbdev hpd processing is temporarily
>> suspended */
>> @@ -135,6 +134,26 @@ static int intel_fbdev_mmap(struct fb_info
>> *info, struct vm_area_struct *vma)
>>          return i915_gem_fb_mmap(obj, vma);
>>   }
>>   
>> +static void intel_fbdev_fb_destroy(struct fb_info *info)
>> +{
>> +       struct drm_fb_helper *fb_helper = info->par;
>> +       struct intel_fbdev *ifbdev = container_of(fb_helper, struct
>> intel_fbdev, helper);
>> +
>> +       drm_fb_helper_fini(&ifbdev->helper);
>> +
>> +       /*
>> +        * 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.
>> +        */
>> +       intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags);
>> +       drm_framebuffer_remove(&ifbdev->fb->base);
>> +
>> +       drm_client_release(&fb_helper->client);
>> +       drm_fb_helper_unprepare(&ifbdev->helper);
>> +       kfree(ifbdev);
>> +}
>> +
>>   __diag_push();
>>   __diag_ignore_all("-Woverride-init", "Allow field initialization
>> overrides for fb ops");
>>   
>> @@ -147,6 +166,7 @@ static const struct fb_ops intelfb_ops = {
>>          .fb_pan_display = intel_fbdev_pan_display,
>>          __FB_DEFAULT_DEFERRED_OPS_DRAW(intel_fbdev),
>>          .fb_mmap = intel_fbdev_mmap,
>> +       .fb_destroy = intel_fbdev_fb_destroy,
>>   };
>>   
>>   __diag_pop();
>> @@ -158,7 +178,6 @@ static int intelfb_create(struct drm_fb_helper
>> *helper,
>>          struct intel_framebuffer *intel_fb = ifbdev->fb;
>>          struct drm_device *dev = helper->dev;
>>          struct drm_i915_private *dev_priv = to_i915(dev);
>> -       struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
>>          const struct i915_gtt_view view = {
>>                  .type = I915_GTT_VIEW_NORMAL,
>>          };
>> @@ -250,7 +269,7 @@ static int intelfb_create(struct drm_fb_helper
>> *helper,
>>          ifbdev->vma_flags = flags;
>>   
>>          intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
>> -       vga_switcheroo_client_fb_set(pdev, info);
>> +
>>          return 0;
>>   
>>   out_unpin:
>> @@ -276,26 +295,6 @@ static const struct drm_fb_helper_funcs
>> intel_fb_helper_funcs = {
>>          .fb_dirty = intelfb_dirty,
>>   };
>>   
>> -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);
>> -
>> -       if (ifbdev->vma)
>> -               intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags);
>> -
>> -       if (ifbdev->fb)
>> -               drm_framebuffer_remove(&ifbdev->fb->base);
>> -
>> -       drm_client_release(&ifbdev->helper.client);
>> -       drm_fb_helper_unprepare(&ifbdev->helper);
>> -       kfree(ifbdev);
>> -}
>> -
>>   /*
>>    * Build an intel_fbdev struct using a BIOS allocated framebuffer,
>> if possible.
>>    * The core display code will have read out the current plane
>> configuration,
>> @@ -459,16 +458,6 @@ static void intel_fbdev_suspend_worker(struct
>> work_struct *work)
>>                                  true);
>>   }
>>   
>> -static void intel_fbdev_sync(struct intel_fbdev *ifbdev)
>> -{
>> -       if (!ifbdev->cookie)
>> -               return;
>> -
>> -       /* Only serialises with all preceding async calls, hence +1
>> */
>> -       async_synchronize_cookie(ifbdev->cookie + 1);
>> -       ifbdev->cookie = 0;
>> -}
>> -
>>   /* Suspends/resumes fbdev processing of incoming HPD events. When
>> resuming HPD
>>    * processing, fbdev will perform a full connector reprobe if a
>> hotplug event
>>    * was received while HPD was suspended.
>> @@ -559,8 +548,6 @@ static int intel_fbdev_output_poll_changed(struct
>> drm_device *dev)
>>          if (!ifbdev)
>>                  return -EINVAL;
>>   
>> -       intel_fbdev_sync(ifbdev);
>> -
>>          mutex_lock(&ifbdev->hpd_lock);
>>          send_hpd = !ifbdev->hpd_suspended;
>>          ifbdev->hpd_waiting = true;
>> @@ -580,7 +567,6 @@ static int intel_fbdev_restore_mode(struct
>> drm_i915_private *dev_priv)
>>          if (!ifbdev)
>>                  return -EINVAL;
>>   
>> -       intel_fbdev_sync(ifbdev);
>>          if (!ifbdev->vma)
>>                  return -ENOMEM;
>>   
>> @@ -598,7 +584,20 @@ static int intel_fbdev_restore_mode(struct
>> drm_i915_private *dev_priv)
>>    */
>>   
>>   static void intel_fbdev_client_unregister(struct drm_client_dev
>> *client)
>> -{ }
>> +{
>> +       struct drm_fb_helper *fb_helper =
>> drm_fb_helper_from_client(client);
>> +       struct drm_device *dev = fb_helper->dev;
>> +       struct pci_dev *pdev = to_pci_dev(dev->dev);
>> +
>> +       if (fb_helper->info) {
>> +               vga_switcheroo_client_fb_set(pdev, NULL);
>> +               drm_fb_helper_unregister_info(fb_helper);
>> +       } else {
>> +               drm_fb_helper_unprepare(fb_helper);
>> +               drm_client_release(&fb_helper->client);
>> +               kfree(fb_helper);
>> +       }
>> +}
>>   
>>   static int intel_fbdev_client_restore(struct drm_client_dev *client)
>>   {
>> @@ -616,7 +615,31 @@ static int intel_fbdev_client_restore(struct
>> drm_client_dev *client)
>>   
>>   static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
>>   {
>> -       return intel_fbdev_output_poll_changed(client->dev);
>> +       struct drm_fb_helper *fb_helper =
>> drm_fb_helper_from_client(client);
>> +       struct drm_device *dev = client->dev;
>> +       struct pci_dev *pdev = to_pci_dev(dev->dev);
>> +       int ret;
>> +
>> +       if (dev->fb_helper)
>> +               return intel_fbdev_output_poll_changed(dev);
>> +
>> +       ret = drm_fb_helper_init(dev, fb_helper);
>> +       if (ret)
>> +               goto err_drm_err;
>> +
>> +       ret = drm_fb_helper_initial_config(fb_helper);
>> +       if (ret)
>> +               goto err_drm_fb_helper_fini;
>> +
>> +       vga_switcheroo_client_fb_set(pdev, fb_helper->info);
>> +
>> +       return 0;
>> +
>> +err_drm_fb_helper_fini:
>> +       drm_fb_helper_fini(fb_helper);
>> +err_drm_err:
>> +       drm_err(dev, "Failed to setup i915 fbdev emulation
>> (ret=%d)\n", ret);
>> +       return ret;
>>   }
>>   
>>   static const struct drm_client_funcs intel_fbdev_client_funcs = {
>> @@ -626,22 +649,23 @@ static const struct drm_client_funcs
>> intel_fbdev_client_funcs = {
>>          .hotplug        = intel_fbdev_client_hotplug,
>>   };
>>   
>> -int intel_fbdev_init(struct drm_device *dev)
>> +void intel_fbdev_setup(struct drm_i915_private *i915)
>>   {
>> -       struct drm_i915_private *dev_priv = to_i915(dev);
>> +       struct drm_device *dev = &i915->drm;
>>          struct intel_fbdev *ifbdev;
>>          int ret;
>>   
>> -       if (drm_WARN_ON(dev, !HAS_DISPLAY(dev_priv)))
>> -               return -ENODEV;
>> +       if (!HAS_DISPLAY(i915))
>> +               return;
>>   
>>          ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
>>          if (!ifbdev)
>> -               return -ENOMEM;
>> -
>> -       mutex_init(&ifbdev->hpd_lock);
>> +               return;
>>          drm_fb_helper_prepare(dev, &ifbdev->helper, 32,
>> &intel_fb_helper_funcs);
>>   
>> +       i915->display.fbdev.fbdev = ifbdev;
>> +       INIT_WORK(&i915->display.fbdev.suspend_work,
>> intel_fbdev_suspend_worker);
>> +       mutex_init(&ifbdev->hpd_lock);
>>          if (intel_fbdev_init_bios(dev, ifbdev))
>>                  ifbdev->helper.preferred_bpp = ifbdev->preferred_bpp;
>>          else
>> @@ -649,68 +673,19 @@ int intel_fbdev_init(struct drm_device *dev)
>>   
>>          ret = drm_client_init(dev, &ifbdev->helper.client, "intel-
>> fbdev",
>>                                &intel_fbdev_client_funcs);
>> -       if (ret)
>> +       if (ret) {
>> +               drm_err(dev, "Failed to register client: %d\n", ret);
>>                  goto err_drm_fb_helper_unprepare;
>> +       }
>>   
>> -       ret = drm_fb_helper_init(dev, &ifbdev->helper);
>> -       if (ret)
>> -               goto err_drm_client_release;
>> -
>> -       dev_priv->display.fbdev.fbdev = ifbdev;
>> -       INIT_WORK(&dev_priv->display.fbdev.suspend_work,
>> intel_fbdev_suspend_worker);
>> +       drm_client_register(&ifbdev->helper.client);
>>   
>> -       return 0;
>> +       return;
>>   
>> -err_drm_client_release:
>> -       drm_client_release(&ifbdev->helper.client);
>>   err_drm_fb_helper_unprepare:
>>          drm_fb_helper_unprepare(&ifbdev->helper);
>> +       mutex_destroy(&ifbdev->hpd_lock);
>>          kfree(ifbdev);
>> -       return ret;
>> -}
>> -
>> -static void intel_fbdev_initial_config(void *data, async_cookie_t
>> cookie)
>> -{
>> -       struct intel_fbdev *ifbdev = data;
>> -
>> -       /* Due to peculiar init order wrt to hpd handling this is
>> separate. */
>> -       if (drm_fb_helper_initial_config(&ifbdev->helper))
>> -               intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
>> -}
>> -
>> -void intel_fbdev_initial_config_async(struct drm_i915_private
>> *dev_priv)
>> -{
>> -       struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
>> -
>> -       if (!ifbdev)
>> -               return;
>> -
>> -       ifbdev->cookie = async_schedule(intel_fbdev_initial_config,
>> ifbdev);
>> -}
>> -
>> -void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
>> -{
>> -       struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
>> -
>> -       if (!ifbdev)
>> -               return;
>> -
>> -       intel_fbdev_set_suspend(&dev_priv->drm,
>> FBINFO_STATE_SUSPENDED, true);
>> -
>> -       if (!current_is_async())
>> -               intel_fbdev_sync(ifbdev);
>> -
>> -       drm_fb_helper_unregister_info(&ifbdev->helper);
>> -}
>> -
>> -void intel_fbdev_fini(struct drm_i915_private *dev_priv)
>> -{
>> -       struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv-
>>> display.fbdev.fbdev);
>> -
>> -       if (!ifbdev)
>> -               return;
>> -
>> -       intel_fbdev_destroy(ifbdev);
>>   }
>>   
>>   struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev
>> *fbdev)
>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h
>> b/drivers/gpu/drm/i915/display/intel_fbdev.h
>> index 8c953f102ba22..08de2d5b34338 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.h
>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.h
>> @@ -14,27 +14,11 @@ struct intel_fbdev;
>>   struct intel_framebuffer;
>>   
>>   #ifdef CONFIG_DRM_FBDEV_EMULATION
>> -int intel_fbdev_init(struct drm_device *dev);
>> -void intel_fbdev_initial_config_async(struct drm_i915_private
>> *dev_priv);
>> -void intel_fbdev_unregister(struct drm_i915_private *dev_priv);
>> -void intel_fbdev_fini(struct drm_i915_private *dev_priv);
>> +void intel_fbdev_setup(struct drm_i915_private *dev_priv);
>>   void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool
>> synchronous);
>>   struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev
>> *fbdev);
>>   #else
>> -static inline int intel_fbdev_init(struct drm_device *dev)
>> -{
>> -       return 0;
>> -}
>> -
>> -static inline void intel_fbdev_initial_config_async(struct
>> drm_i915_private *dev_priv)
>> -{
>> -}
>> -
>> -static inline void intel_fbdev_unregister(struct drm_i915_private
>> *dev_priv)
>> -{
>> -}
>> -
>> -static inline void intel_fbdev_fini(struct drm_i915_private
>> *dev_priv)
>> +static inline void intel_fbdev_setup(struct drm_i915_private
>> *dev_priv)
>>   {
>>   }
>>   
>> diff --git a/drivers/gpu/drm/xe/display/xe_display.c
>> b/drivers/gpu/drm/xe/display/xe_display.c
>> index cdbc3f04c80a7..ca5cbe1d8a03b 100644
>> --- a/drivers/gpu/drm/xe/display/xe_display.c
>> +++ b/drivers/gpu/drm/xe/display/xe_display.c
>> @@ -214,9 +214,7 @@ void xe_display_fini(struct xe_device *xe)
>>          if (!xe->info.enable_display)
>>                  return;
>>   
>> -       /* poll work can call into fbdev, hence clean that up
>> afterwards */
>>          intel_hpd_poll_fini(xe);
>> -       intel_fbdev_fini(xe);
>>   
>>          intel_hdcp_component_fini(xe);
>>          intel_audio_deinit(xe);

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH v8 6/6] drm/{i915,xe}: Implement fbdev emulation as in-kernel client

[Hogander, Jouni]

On Tue, 2024-04-23 at 13:13 +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 22.04.24 um 16:11 schrieb Hogander, Jouni:
> > On Tue, 2024-04-09 at 10:04 +0200, Thomas Zimmermann wrote:
> > > Replace all code that initializes or releases fbdev emulation
> > > throughout the driver. Instead initialize the fbdev client by a
> > > single call to intel_fbdev_setup() after i915 has registered its
> > > DRM device. Just like similar code in other drivers, i915 fbdev
> > > emulation now acts like a regular DRM client. Do the same for xe.
> > > 
> > > The fbdev client setup consists of the initial preparation and
> > > the
> > > hot-plugging of the display. The latter creates the fbdev device
> > > and sets up the fbdev framebuffer. The setup performs display
> > > hot-plugging once. If no display can be detected, DRM probe
> > > helpers
> > > re-run the detection on each hotplug event.
> > > 
> > > A call to drm_client_dev_unregister() releases all in-kernel
> > > clients
> > > automatically. No further action is required within i915. If the
> > > fbdev
> > > framebuffer has been fully set up, struct fb_ops.fb_destroy
> > > implements
> > > the release. For partially initialized emulation, the fbdev
> > > client
> > > reverts the initial setup. Do the same for xe and remove its call
> > > to
> > > intel_fbdev_fini().
> > > 
> > > v8:
> > > - setup client in intel_display_driver_register (Jouni)
> > > - mention xe in commit message
> > > 
> > > v7:
> > > - update xe driver
> > > - reword commit message
> > > 
> > > v6:
> > > - use 'i915' for i915 device (Jouni)
> > > - remove unnecessary code for non-atomic mode setting (Jouni,
> > > Ville)
> > > - fix function name in commit message (Jouni)
> > > 
> > > v3:
> > > -  as before, silently ignore devices without displays
> > > 
> > > v2:
> > > -  let drm_client_register() handle initial hotplug
> > > -  fix driver name in error message (Jani)
> > > -  fix non-fbdev build (kernel test robot)
> > > 
> > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
> 
> Thank you so much. All patches has R-bs. Can you add the series to
> the 
> intel tree?

Is it ok to merge patch 01/06 via intel tree as well?

Rodrigo, This set is containing Xe display changes as well. Is it ok to
push this via drm-intel?

BR,

Jouni Högander

> 
> Best regards
> Thomas
> 
> > 
> > > ---
> > >   drivers/gpu/drm/i915/display/intel_display.c  |   1 -
> > >   .../drm/i915/display/intel_display_driver.c   |  20 +-
> > >   drivers/gpu/drm/i915/display/intel_fbdev.c    | 177 ++++++++---
> > > -----
> > > --
> > >   drivers/gpu/drm/i915/display/intel_fbdev.h    |  20 +-
> > >   drivers/gpu/drm/xe/display/xe_display.c       |   2 -
> > >   5 files changed, 80 insertions(+), 140 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 614e60420a29a..161a5aabf6746 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -85,7 +85,6 @@
> > >   #include "intel_dvo.h"
> > >   #include "intel_fb.h"
> > >   #include "intel_fbc.h"
> > > -#include "intel_fbdev.h"
> > >   #include "intel_fdi.h"
> > >   #include "intel_fifo_underrun.h"
> > >   #include "intel_frontbuffer.h"
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c
> > > b/drivers/gpu/drm/i915/display/intel_display_driver.c
> > > index e5f052d4ff1cc..ed8589fa35448 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> > > @@ -514,10 +514,6 @@ int intel_display_driver_probe(struct
> > > drm_i915_private *i915)
> > >   
> > >          intel_overlay_setup(i915);
> > >   
> > > -       ret = intel_fbdev_init(&i915->drm);
> > > -       if (ret)
> > > -               return ret;
> > > -
> > >          /* Only enable hotplug handling once the fbdev is fully
> > > set
> > > up. */
> > >          intel_hpd_init(i915);
> > >   
> > > @@ -544,16 +540,6 @@ void intel_display_driver_register(struct
> > > drm_i915_private *i915)
> > >   
> > >          intel_display_debugfs_register(i915);
> > >   
> > > -       /*
> > > -        * Some ports require correctly set-up hpd registers for
> > > -        * detection to work properly (leading to ghost connected
> > > -        * connector status), e.g. VGA on gm45.  Hence we can
> > > only
> > > set
> > > -        * up the initial fbdev config after hpd irqs are fully
> > > -        * enabled. We do it last so that the async config cannot
> > > run
> > > -        * before the connectors are registered.
> > > -        */
> > > -       intel_fbdev_initial_config_async(i915);
> > > -
> > >          /*
> > >           * We need to coordinate the hotplugs with the
> > > asynchronous
> > >           * fbdev configuration, for which we use the
> > > @@ -562,6 +548,8 @@ void intel_display_driver_register(struct
> > > drm_i915_private *i915)
> > >          drm_kms_helper_poll_init(&i915->drm);
> > >          intel_hpd_poll_disable(i915);
> > >   
> > > +       intel_fbdev_setup(i915);
> > > +
> > >          intel_display_device_info_print(DISPLAY_INFO(i915),
> > >                                          DISPLAY_RUNTIME_INFO(i91
> > > 5),
> > > &p);
> > >   }
> > > @@ -597,9 +585,6 @@ void intel_display_driver_remove_noirq(struct
> > > drm_i915_private *i915)
> > >           */
> > >          intel_hpd_poll_fini(i915);
> > >   
> > > -       /* poll work can call into fbdev, hence clean that up
> > > afterwards */
> > > -       intel_fbdev_fini(i915);
> > > -
> > >          intel_unregister_dsm_handler();
> > >   
> > >          /* flush any delayed tasks or pending work */
> > > @@ -640,7 +625,6 @@ void intel_display_driver_unregister(struct
> > > drm_i915_private *i915)
> > >   
> > >          drm_client_dev_unregister(&i915->drm);
> > >   
> > > -       intel_fbdev_unregister(i915);
> > >          /*
> > >           * After flushing the fbdev (incl. a late async config
> > > which
> > >           * will have delayed queuing of a hotplug event), then
> > > flush
> > > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
> > > b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > > index f783de611a7f5..bda702c2cab8e 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > > @@ -24,7 +24,6 @@
> > >    *     David Airlie
> > >    */
> > >   
> > > -#include <linux/async.h>
> > >   #include <linux/console.h>
> > >   #include <linux/delay.h>
> > >   #include <linux/errno.h>
> > > @@ -39,6 +38,7 @@
> > >   #include <linux/vga_switcheroo.h>
> > >   
> > >   #include <drm/drm_crtc.h>
> > > +#include <drm/drm_crtc_helper.h>
> > >   #include <drm/drm_fb_helper.h>
> > >   #include <drm/drm_fourcc.h>
> > >   #include <drm/drm_gem_framebuffer_helper.h>
> > > @@ -58,7 +58,6 @@ struct intel_fbdev {
> > >          struct intel_framebuffer *fb;
> > >          struct i915_vma *vma;
> > >          unsigned long vma_flags;
> > > -       async_cookie_t cookie;
> > >          int preferred_bpp;
> > >   
> > >          /* Whether or not fbdev hpd processing is temporarily
> > > suspended */
> > > @@ -135,6 +134,26 @@ static int intel_fbdev_mmap(struct fb_info
> > > *info, struct vm_area_struct *vma)
> > >          return i915_gem_fb_mmap(obj, vma);
> > >   }
> > >   
> > > +static void intel_fbdev_fb_destroy(struct fb_info *info)
> > > +{
> > > +       struct drm_fb_helper *fb_helper = info->par;
> > > +       struct intel_fbdev *ifbdev = container_of(fb_helper,
> > > struct
> > > intel_fbdev, helper);
> > > +
> > > +       drm_fb_helper_fini(&ifbdev->helper);
> > > +
> > > +       /*
> > > +        * 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.
> > > +        */
> > > +       intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags);
> > > +       drm_framebuffer_remove(&ifbdev->fb->base);
> > > +
> > > +       drm_client_release(&fb_helper->client);
> > > +       drm_fb_helper_unprepare(&ifbdev->helper);
> > > +       kfree(ifbdev);
> > > +}
> > > +
> > >   __diag_push();
> > >   __diag_ignore_all("-Woverride-init", "Allow field
> > > initialization
> > > overrides for fb ops");
> > >   
> > > @@ -147,6 +166,7 @@ static const struct fb_ops intelfb_ops = {
> > >          .fb_pan_display = intel_fbdev_pan_display,
> > >          __FB_DEFAULT_DEFERRED_OPS_DRAW(intel_fbdev),
> > >          .fb_mmap = intel_fbdev_mmap,
> > > +       .fb_destroy = intel_fbdev_fb_destroy,
> > >   };
> > >   
> > >   __diag_pop();
> > > @@ -158,7 +178,6 @@ static int intelfb_create(struct
> > > drm_fb_helper
> > > *helper,
> > >          struct intel_framebuffer *intel_fb = ifbdev->fb;
> > >          struct drm_device *dev = helper->dev;
> > >          struct drm_i915_private *dev_priv = to_i915(dev);
> > > -       struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> > >          const struct i915_gtt_view view = {
> > >                  .type = I915_GTT_VIEW_NORMAL,
> > >          };
> > > @@ -250,7 +269,7 @@ static int intelfb_create(struct
> > > drm_fb_helper
> > > *helper,
> > >          ifbdev->vma_flags = flags;
> > >   
> > >          intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
> > > -       vga_switcheroo_client_fb_set(pdev, info);
> > > +
> > >          return 0;
> > >   
> > >   out_unpin:
> > > @@ -276,26 +295,6 @@ static const struct drm_fb_helper_funcs
> > > intel_fb_helper_funcs = {
> > >          .fb_dirty = intelfb_dirty,
> > >   };
> > >   
> > > -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);
> > > -
> > > -       if (ifbdev->vma)
> > > -               intel_unpin_fb_vma(ifbdev->vma, ifbdev-
> > > >vma_flags);
> > > -
> > > -       if (ifbdev->fb)
> > > -               drm_framebuffer_remove(&ifbdev->fb->base);
> > > -
> > > -       drm_client_release(&ifbdev->helper.client);
> > > -       drm_fb_helper_unprepare(&ifbdev->helper);
> > > -       kfree(ifbdev);
> > > -}
> > > -
> > >   /*
> > >    * Build an intel_fbdev struct using a BIOS allocated
> > > framebuffer,
> > > if possible.
> > >    * The core display code will have read out the current plane
> > > configuration,
> > > @@ -459,16 +458,6 @@ static void
> > > intel_fbdev_suspend_worker(struct
> > > work_struct *work)
> > >                                  true);
> > >   }
> > >   
> > > -static void intel_fbdev_sync(struct intel_fbdev *ifbdev)
> > > -{
> > > -       if (!ifbdev->cookie)
> > > -               return;
> > > -
> > > -       /* Only serialises with all preceding async calls, hence
> > > +1
> > > */
> > > -       async_synchronize_cookie(ifbdev->cookie + 1);
> > > -       ifbdev->cookie = 0;
> > > -}
> > > -
> > >   /* Suspends/resumes fbdev processing of incoming HPD events.
> > > When
> > > resuming HPD
> > >    * processing, fbdev will perform a full connector reprobe if a
> > > hotplug event
> > >    * was received while HPD was suspended.
> > > @@ -559,8 +548,6 @@ static int
> > > intel_fbdev_output_poll_changed(struct
> > > drm_device *dev)
> > >          if (!ifbdev)
> > >                  return -EINVAL;
> > >   
> > > -       intel_fbdev_sync(ifbdev);
> > > -
> > >          mutex_lock(&ifbdev->hpd_lock);
> > >          send_hpd = !ifbdev->hpd_suspended;
> > >          ifbdev->hpd_waiting = true;
> > > @@ -580,7 +567,6 @@ static int intel_fbdev_restore_mode(struct
> > > drm_i915_private *dev_priv)
> > >          if (!ifbdev)
> > >                  return -EINVAL;
> > >   
> > > -       intel_fbdev_sync(ifbdev);
> > >          if (!ifbdev->vma)
> > >                  return -ENOMEM;
> > >   
> > > @@ -598,7 +584,20 @@ static int intel_fbdev_restore_mode(struct
> > > drm_i915_private *dev_priv)
> > >    */
> > >   
> > >   static void intel_fbdev_client_unregister(struct drm_client_dev
> > > *client)
> > > -{ }
> > > +{
> > > +       struct drm_fb_helper *fb_helper =
> > > drm_fb_helper_from_client(client);
> > > +       struct drm_device *dev = fb_helper->dev;
> > > +       struct pci_dev *pdev = to_pci_dev(dev->dev);
> > > +
> > > +       if (fb_helper->info) {
> > > +               vga_switcheroo_client_fb_set(pdev, NULL);
> > > +               drm_fb_helper_unregister_info(fb_helper);
> > > +       } else {
> > > +               drm_fb_helper_unprepare(fb_helper);
> > > +               drm_client_release(&fb_helper->client);
> > > +               kfree(fb_helper);
> > > +       }
> > > +}
> > >   
> > >   static int intel_fbdev_client_restore(struct drm_client_dev
> > > *client)
> > >   {
> > > @@ -616,7 +615,31 @@ static int intel_fbdev_client_restore(struct
> > > drm_client_dev *client)
> > >   
> > >   static int intel_fbdev_client_hotplug(struct drm_client_dev
> > > *client)
> > >   {
> > > -       return intel_fbdev_output_poll_changed(client->dev);
> > > +       struct drm_fb_helper *fb_helper =
> > > drm_fb_helper_from_client(client);
> > > +       struct drm_device *dev = client->dev;
> > > +       struct pci_dev *pdev = to_pci_dev(dev->dev);
> > > +       int ret;
> > > +
> > > +       if (dev->fb_helper)
> > > +               return intel_fbdev_output_poll_changed(dev);
> > > +
> > > +       ret = drm_fb_helper_init(dev, fb_helper);
> > > +       if (ret)
> > > +               goto err_drm_err;
> > > +
> > > +       ret = drm_fb_helper_initial_config(fb_helper);
> > > +       if (ret)
> > > +               goto err_drm_fb_helper_fini;
> > > +
> > > +       vga_switcheroo_client_fb_set(pdev, fb_helper->info);
> > > +
> > > +       return 0;
> > > +
> > > +err_drm_fb_helper_fini:
> > > +       drm_fb_helper_fini(fb_helper);
> > > +err_drm_err:
> > > +       drm_err(dev, "Failed to setup i915 fbdev emulation
> > > (ret=%d)\n", ret);
> > > +       return ret;
> > >   }
> > >   
> > >   static const struct drm_client_funcs intel_fbdev_client_funcs =
> > > {
> > > @@ -626,22 +649,23 @@ static const struct drm_client_funcs
> > > intel_fbdev_client_funcs = {
> > >          .hotplug        = intel_fbdev_client_hotplug,
> > >   };
> > >   
> > > -int intel_fbdev_init(struct drm_device *dev)
> > > +void intel_fbdev_setup(struct drm_i915_private *i915)
> > >   {
> > > -       struct drm_i915_private *dev_priv = to_i915(dev);
> > > +       struct drm_device *dev = &i915->drm;
> > >          struct intel_fbdev *ifbdev;
> > >          int ret;
> > >   
> > > -       if (drm_WARN_ON(dev, !HAS_DISPLAY(dev_priv)))
> > > -               return -ENODEV;
> > > +       if (!HAS_DISPLAY(i915))
> > > +               return;
> > >   
> > >          ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
> > >          if (!ifbdev)
> > > -               return -ENOMEM;
> > > -
> > > -       mutex_init(&ifbdev->hpd_lock);
> > > +               return;
> > >          drm_fb_helper_prepare(dev, &ifbdev->helper, 32,
> > > &intel_fb_helper_funcs);
> > >   
> > > +       i915->display.fbdev.fbdev = ifbdev;
> > > +       INIT_WORK(&i915->display.fbdev.suspend_work,
> > > intel_fbdev_suspend_worker);
> > > +       mutex_init(&ifbdev->hpd_lock);
> > >          if (intel_fbdev_init_bios(dev, ifbdev))
> > >                  ifbdev->helper.preferred_bpp = ifbdev-
> > > >preferred_bpp;
> > >          else
> > > @@ -649,68 +673,19 @@ int intel_fbdev_init(struct drm_device
> > > *dev)
> > >   
> > >          ret = drm_client_init(dev, &ifbdev->helper.client,
> > > "intel-
> > > fbdev",
> > >                                &intel_fbdev_client_funcs);
> > > -       if (ret)
> > > +       if (ret) {
> > > +               drm_err(dev, "Failed to register client: %d\n",
> > > ret);
> > >                  goto err_drm_fb_helper_unprepare;
> > > +       }
> > >   
> > > -       ret = drm_fb_helper_init(dev, &ifbdev->helper);
> > > -       if (ret)
> > > -               goto err_drm_client_release;
> > > -
> > > -       dev_priv->display.fbdev.fbdev = ifbdev;
> > > -       INIT_WORK(&dev_priv->display.fbdev.suspend_work,
> > > intel_fbdev_suspend_worker);
> > > +       drm_client_register(&ifbdev->helper.client);
> > >   
> > > -       return 0;
> > > +       return;
> > >   
> > > -err_drm_client_release:
> > > -       drm_client_release(&ifbdev->helper.client);
> > >   err_drm_fb_helper_unprepare:
> > >          drm_fb_helper_unprepare(&ifbdev->helper);
> > > +       mutex_destroy(&ifbdev->hpd_lock);
> > >          kfree(ifbdev);
> > > -       return ret;
> > > -}
> > > -
> > > -static void intel_fbdev_initial_config(void *data,
> > > async_cookie_t
> > > cookie)
> > > -{
> > > -       struct intel_fbdev *ifbdev = data;
> > > -
> > > -       /* Due to peculiar init order wrt to hpd handling this is
> > > separate. */
> > > -       if (drm_fb_helper_initial_config(&ifbdev->helper))
> > > -               intel_fbdev_unregister(to_i915(ifbdev-
> > > >helper.dev));
> > > -}
> > > -
> > > -void intel_fbdev_initial_config_async(struct drm_i915_private
> > > *dev_priv)
> > > -{
> > > -       struct intel_fbdev *ifbdev = dev_priv-
> > > >display.fbdev.fbdev;
> > > -
> > > -       if (!ifbdev)
> > > -               return;
> > > -
> > > -       ifbdev->cookie =
> > > async_schedule(intel_fbdev_initial_config,
> > > ifbdev);
> > > -}
> > > -
> > > -void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
> > > -{
> > > -       struct intel_fbdev *ifbdev = dev_priv-
> > > >display.fbdev.fbdev;
> > > -
> > > -       if (!ifbdev)
> > > -               return;
> > > -
> > > -       intel_fbdev_set_suspend(&dev_priv->drm,
> > > FBINFO_STATE_SUSPENDED, true);
> > > -
> > > -       if (!current_is_async())
> > > -               intel_fbdev_sync(ifbdev);
> > > -
> > > -       drm_fb_helper_unregister_info(&ifbdev->helper);
> > > -}
> > > -
> > > -void intel_fbdev_fini(struct drm_i915_private *dev_priv)
> > > -{
> > > -       struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv-
> > > > display.fbdev.fbdev);
> > > -
> > > -       if (!ifbdev)
> > > -               return;
> > > -
> > > -       intel_fbdev_destroy(ifbdev);
> > >   }
> > >   
> > >   struct intel_framebuffer *intel_fbdev_framebuffer(struct
> > > intel_fbdev
> > > *fbdev)
> > > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h
> > > b/drivers/gpu/drm/i915/display/intel_fbdev.h
> > > index 8c953f102ba22..08de2d5b34338 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_fbdev.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.h
> > > @@ -14,27 +14,11 @@ struct intel_fbdev;
> > >   struct intel_framebuffer;
> > >   
> > >   #ifdef CONFIG_DRM_FBDEV_EMULATION
> > > -int intel_fbdev_init(struct drm_device *dev);
> > > -void intel_fbdev_initial_config_async(struct drm_i915_private
> > > *dev_priv);
> > > -void intel_fbdev_unregister(struct drm_i915_private *dev_priv);
> > > -void intel_fbdev_fini(struct drm_i915_private *dev_priv);
> > > +void intel_fbdev_setup(struct drm_i915_private *dev_priv);
> > >   void intel_fbdev_set_suspend(struct drm_device *dev, int state,
> > > bool
> > > synchronous);
> > >   struct intel_framebuffer *intel_fbdev_framebuffer(struct
> > > intel_fbdev
> > > *fbdev);
> > >   #else
> > > -static inline int intel_fbdev_init(struct drm_device *dev)
> > > -{
> > > -       return 0;
> > > -}
> > > -
> > > -static inline void intel_fbdev_initial_config_async(struct
> > > drm_i915_private *dev_priv)
> > > -{
> > > -}
> > > -
> > > -static inline void intel_fbdev_unregister(struct
> > > drm_i915_private
> > > *dev_priv)
> > > -{
> > > -}
> > > -
> > > -static inline void intel_fbdev_fini(struct drm_i915_private
> > > *dev_priv)
> > > +static inline void intel_fbdev_setup(struct drm_i915_private
> > > *dev_priv)
> > >   {
> > >   }
> > >   
> > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c
> > > b/drivers/gpu/drm/xe/display/xe_display.c
> > > index cdbc3f04c80a7..ca5cbe1d8a03b 100644
> > > --- a/drivers/gpu/drm/xe/display/xe_display.c
> > > +++ b/drivers/gpu/drm/xe/display/xe_display.c
> > > @@ -214,9 +214,7 @@ void xe_display_fini(struct xe_device *xe)
> > >          if (!xe->info.enable_display)
> > >                  return;
> > >   
> > > -       /* poll work can call into fbdev, hence clean that up
> > > afterwards */
> > >          intel_hpd_poll_fini(xe);
> > > -       intel_fbdev_fini(xe);
> > >   
> > >          intel_hdcp_component_fini(xe);
> > >          intel_audio_deinit(xe);
> 

Re: [PATCH v8 6/6] drm/{i915,xe}: Implement fbdev emulation as in-kernel client

[Thomas Zimmermann]

Hi

Am 23.04.24 um 13:36 schrieb Hogander, Jouni:
> On Tue, 2024-04-23 at 13:13 +0200, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 22.04.24 um 16:11 schrieb Hogander, Jouni:
>>> On Tue, 2024-04-09 at 10:04 +0200, Thomas Zimmermann wrote:
>>>> Replace all code that initializes or releases fbdev emulation
>>>> throughout the driver. Instead initialize the fbdev client by a
>>>> single call to intel_fbdev_setup() after i915 has registered its
>>>> DRM device. Just like similar code in other drivers, i915 fbdev
>>>> emulation now acts like a regular DRM client. Do the same for xe.
>>>>
>>>> The fbdev client setup consists of the initial preparation and
>>>> the
>>>> hot-plugging of the display. The latter creates the fbdev device
>>>> and sets up the fbdev framebuffer. The setup performs display
>>>> hot-plugging once. If no display can be detected, DRM probe
>>>> helpers
>>>> re-run the detection on each hotplug event.
>>>>
>>>> A call to drm_client_dev_unregister() releases all in-kernel
>>>> clients
>>>> automatically. No further action is required within i915. If the
>>>> fbdev
>>>> framebuffer has been fully set up, struct fb_ops.fb_destroy
>>>> implements
>>>> the release. For partially initialized emulation, the fbdev
>>>> client
>>>> reverts the initial setup. Do the same for xe and remove its call
>>>> to
>>>> intel_fbdev_fini().
>>>>
>>>> v8:
>>>> - setup client in intel_display_driver_register (Jouni)
>>>> - mention xe in commit message
>>>>
>>>> v7:
>>>> - update xe driver
>>>> - reword commit message
>>>>
>>>> v6:
>>>> - use 'i915' for i915 device (Jouni)
>>>> - remove unnecessary code for non-atomic mode setting (Jouni,
>>>> Ville)
>>>> - fix function name in commit message (Jouni)
>>>>
>>>> v3:
>>>> -  as before, silently ignore devices without displays
>>>>
>>>> v2:
>>>> -  let drm_client_register() handle initial hotplug
>>>> -  fix driver name in error message (Jani)
>>>> -  fix non-fbdev build (kernel test robot)
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
>> Thank you so much. All patches has R-bs. Can you add the series to
>> the
>> intel tree?
> Is it ok to merge patch 01/06 via intel tree as well?

Sure, np.

Best regards
Thomas

>
> Rodrigo, This set is containing Xe display changes as well. Is it ok to
> push this via drm-intel?
>
> BR,
>
> Jouni Högander
>
>> Best regards
>> Thomas
>>
>>>> ---
>>>>    drivers/gpu/drm/i915/display/intel_display.c  |   1 -
>>>>    .../drm/i915/display/intel_display_driver.c   |  20 +-
>>>>    drivers/gpu/drm/i915/display/intel_fbdev.c    | 177 ++++++++---
>>>> -----
>>>> --
>>>>    drivers/gpu/drm/i915/display/intel_fbdev.h    |  20 +-
>>>>    drivers/gpu/drm/xe/display/xe_display.c       |   2 -
>>>>    5 files changed, 80 insertions(+), 140 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
>>>> b/drivers/gpu/drm/i915/display/intel_display.c
>>>> index 614e60420a29a..161a5aabf6746 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>>> @@ -85,7 +85,6 @@
>>>>    #include "intel_dvo.h"
>>>>    #include "intel_fb.h"
>>>>    #include "intel_fbc.h"
>>>> -#include "intel_fbdev.h"
>>>>    #include "intel_fdi.h"
>>>>    #include "intel_fifo_underrun.h"
>>>>    #include "intel_frontbuffer.h"
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c
>>>> b/drivers/gpu/drm/i915/display/intel_display_driver.c
>>>> index e5f052d4ff1cc..ed8589fa35448 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
>>>> @@ -514,10 +514,6 @@ int intel_display_driver_probe(struct
>>>> drm_i915_private *i915)
>>>>    
>>>>           intel_overlay_setup(i915);
>>>>    
>>>> -       ret = intel_fbdev_init(&i915->drm);
>>>> -       if (ret)
>>>> -               return ret;
>>>> -
>>>>           /* Only enable hotplug handling once the fbdev is fully
>>>> set
>>>> up. */
>>>>           intel_hpd_init(i915);
>>>>    
>>>> @@ -544,16 +540,6 @@ void intel_display_driver_register(struct
>>>> drm_i915_private *i915)
>>>>    
>>>>           intel_display_debugfs_register(i915);
>>>>    
>>>> -       /*
>>>> -        * Some ports require correctly set-up hpd registers for
>>>> -        * detection to work properly (leading to ghost connected
>>>> -        * connector status), e.g. VGA on gm45.  Hence we can
>>>> only
>>>> set
>>>> -        * up the initial fbdev config after hpd irqs are fully
>>>> -        * enabled. We do it last so that the async config cannot
>>>> run
>>>> -        * before the connectors are registered.
>>>> -        */
>>>> -       intel_fbdev_initial_config_async(i915);
>>>> -
>>>>           /*
>>>>            * We need to coordinate the hotplugs with the
>>>> asynchronous
>>>>            * fbdev configuration, for which we use the
>>>> @@ -562,6 +548,8 @@ void intel_display_driver_register(struct
>>>> drm_i915_private *i915)
>>>>           drm_kms_helper_poll_init(&i915->drm);
>>>>           intel_hpd_poll_disable(i915);
>>>>    
>>>> +       intel_fbdev_setup(i915);
>>>> +
>>>>           intel_display_device_info_print(DISPLAY_INFO(i915),
>>>>                                           DISPLAY_RUNTIME_INFO(i91
>>>> 5),
>>>> &p);
>>>>    }
>>>> @@ -597,9 +585,6 @@ void intel_display_driver_remove_noirq(struct
>>>> drm_i915_private *i915)
>>>>            */
>>>>           intel_hpd_poll_fini(i915);
>>>>    
>>>> -       /* poll work can call into fbdev, hence clean that up
>>>> afterwards */
>>>> -       intel_fbdev_fini(i915);
>>>> -
>>>>           intel_unregister_dsm_handler();
>>>>    
>>>>           /* flush any delayed tasks or pending work */
>>>> @@ -640,7 +625,6 @@ void intel_display_driver_unregister(struct
>>>> drm_i915_private *i915)
>>>>    
>>>>           drm_client_dev_unregister(&i915->drm);
>>>>    
>>>> -       intel_fbdev_unregister(i915);
>>>>           /*
>>>>            * After flushing the fbdev (incl. a late async config
>>>> which
>>>>            * will have delayed queuing of a hotplug event), then
>>>> flush
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>> b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>> index f783de611a7f5..bda702c2cab8e 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>> @@ -24,7 +24,6 @@
>>>>     *     David Airlie
>>>>     */
>>>>    
>>>> -#include <linux/async.h>
>>>>    #include <linux/console.h>
>>>>    #include <linux/delay.h>
>>>>    #include <linux/errno.h>
>>>> @@ -39,6 +38,7 @@
>>>>    #include <linux/vga_switcheroo.h>
>>>>    
>>>>    #include <drm/drm_crtc.h>
>>>> +#include <drm/drm_crtc_helper.h>
>>>>    #include <drm/drm_fb_helper.h>
>>>>    #include <drm/drm_fourcc.h>
>>>>    #include <drm/drm_gem_framebuffer_helper.h>
>>>> @@ -58,7 +58,6 @@ struct intel_fbdev {
>>>>           struct intel_framebuffer *fb;
>>>>           struct i915_vma *vma;
>>>>           unsigned long vma_flags;
>>>> -       async_cookie_t cookie;
>>>>           int preferred_bpp;
>>>>    
>>>>           /* Whether or not fbdev hpd processing is temporarily
>>>> suspended */
>>>> @@ -135,6 +134,26 @@ static int intel_fbdev_mmap(struct fb_info
>>>> *info, struct vm_area_struct *vma)
>>>>           return i915_gem_fb_mmap(obj, vma);
>>>>    }
>>>>    
>>>> +static void intel_fbdev_fb_destroy(struct fb_info *info)
>>>> +{
>>>> +       struct drm_fb_helper *fb_helper = info->par;
>>>> +       struct intel_fbdev *ifbdev = container_of(fb_helper,
>>>> struct
>>>> intel_fbdev, helper);
>>>> +
>>>> +       drm_fb_helper_fini(&ifbdev->helper);
>>>> +
>>>> +       /*
>>>> +        * 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.
>>>> +        */
>>>> +       intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags);
>>>> +       drm_framebuffer_remove(&ifbdev->fb->base);
>>>> +
>>>> +       drm_client_release(&fb_helper->client);
>>>> +       drm_fb_helper_unprepare(&ifbdev->helper);
>>>> +       kfree(ifbdev);
>>>> +}
>>>> +
>>>>    __diag_push();
>>>>    __diag_ignore_all("-Woverride-init", "Allow field
>>>> initialization
>>>> overrides for fb ops");
>>>>    
>>>> @@ -147,6 +166,7 @@ static const struct fb_ops intelfb_ops = {
>>>>           .fb_pan_display = intel_fbdev_pan_display,
>>>>           __FB_DEFAULT_DEFERRED_OPS_DRAW(intel_fbdev),
>>>>           .fb_mmap = intel_fbdev_mmap,
>>>> +       .fb_destroy = intel_fbdev_fb_destroy,
>>>>    };
>>>>    
>>>>    __diag_pop();
>>>> @@ -158,7 +178,6 @@ static int intelfb_create(struct
>>>> drm_fb_helper
>>>> *helper,
>>>>           struct intel_framebuffer *intel_fb = ifbdev->fb;
>>>>           struct drm_device *dev = helper->dev;
>>>>           struct drm_i915_private *dev_priv = to_i915(dev);
>>>> -       struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
>>>>           const struct i915_gtt_view view = {
>>>>                   .type = I915_GTT_VIEW_NORMAL,
>>>>           };
>>>> @@ -250,7 +269,7 @@ static int intelfb_create(struct
>>>> drm_fb_helper
>>>> *helper,
>>>>           ifbdev->vma_flags = flags;
>>>>    
>>>>           intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
>>>> -       vga_switcheroo_client_fb_set(pdev, info);
>>>> +
>>>>           return 0;
>>>>    
>>>>    out_unpin:
>>>> @@ -276,26 +295,6 @@ static const struct drm_fb_helper_funcs
>>>> intel_fb_helper_funcs = {
>>>>           .fb_dirty = intelfb_dirty,
>>>>    };
>>>>    
>>>> -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);
>>>> -
>>>> -       if (ifbdev->vma)
>>>> -               intel_unpin_fb_vma(ifbdev->vma, ifbdev-
>>>>> vma_flags);
>>>> -
>>>> -       if (ifbdev->fb)
>>>> -               drm_framebuffer_remove(&ifbdev->fb->base);
>>>> -
>>>> -       drm_client_release(&ifbdev->helper.client);
>>>> -       drm_fb_helper_unprepare(&ifbdev->helper);
>>>> -       kfree(ifbdev);
>>>> -}
>>>> -
>>>>    /*
>>>>     * Build an intel_fbdev struct using a BIOS allocated
>>>> framebuffer,
>>>> if possible.
>>>>     * The core display code will have read out the current plane
>>>> configuration,
>>>> @@ -459,16 +458,6 @@ static void
>>>> intel_fbdev_suspend_worker(struct
>>>> work_struct *work)
>>>>                                   true);
>>>>    }
>>>>    
>>>> -static void intel_fbdev_sync(struct intel_fbdev *ifbdev)
>>>> -{
>>>> -       if (!ifbdev->cookie)
>>>> -               return;
>>>> -
>>>> -       /* Only serialises with all preceding async calls, hence
>>>> +1
>>>> */
>>>> -       async_synchronize_cookie(ifbdev->cookie + 1);
>>>> -       ifbdev->cookie = 0;
>>>> -}
>>>> -
>>>>    /* Suspends/resumes fbdev processing of incoming HPD events.
>>>> When
>>>> resuming HPD
>>>>     * processing, fbdev will perform a full connector reprobe if a
>>>> hotplug event
>>>>     * was received while HPD was suspended.
>>>> @@ -559,8 +548,6 @@ static int
>>>> intel_fbdev_output_poll_changed(struct
>>>> drm_device *dev)
>>>>           if (!ifbdev)
>>>>                   return -EINVAL;
>>>>    
>>>> -       intel_fbdev_sync(ifbdev);
>>>> -
>>>>           mutex_lock(&ifbdev->hpd_lock);
>>>>           send_hpd = !ifbdev->hpd_suspended;
>>>>           ifbdev->hpd_waiting = true;
>>>> @@ -580,7 +567,6 @@ static int intel_fbdev_restore_mode(struct
>>>> drm_i915_private *dev_priv)
>>>>           if (!ifbdev)
>>>>                   return -EINVAL;
>>>>    
>>>> -       intel_fbdev_sync(ifbdev);
>>>>           if (!ifbdev->vma)
>>>>                   return -ENOMEM;
>>>>    
>>>> @@ -598,7 +584,20 @@ static int intel_fbdev_restore_mode(struct
>>>> drm_i915_private *dev_priv)
>>>>     */
>>>>    
>>>>    static void intel_fbdev_client_unregister(struct drm_client_dev
>>>> *client)
>>>> -{ }
>>>> +{
>>>> +       struct drm_fb_helper *fb_helper =
>>>> drm_fb_helper_from_client(client);
>>>> +       struct drm_device *dev = fb_helper->dev;
>>>> +       struct pci_dev *pdev = to_pci_dev(dev->dev);
>>>> +
>>>> +       if (fb_helper->info) {
>>>> +               vga_switcheroo_client_fb_set(pdev, NULL);
>>>> +               drm_fb_helper_unregister_info(fb_helper);
>>>> +       } else {
>>>> +               drm_fb_helper_unprepare(fb_helper);
>>>> +               drm_client_release(&fb_helper->client);
>>>> +               kfree(fb_helper);
>>>> +       }
>>>> +}
>>>>    
>>>>    static int intel_fbdev_client_restore(struct drm_client_dev
>>>> *client)
>>>>    {
>>>> @@ -616,7 +615,31 @@ static int intel_fbdev_client_restore(struct
>>>> drm_client_dev *client)
>>>>    
>>>>    static int intel_fbdev_client_hotplug(struct drm_client_dev
>>>> *client)
>>>>    {
>>>> -       return intel_fbdev_output_poll_changed(client->dev);
>>>> +       struct drm_fb_helper *fb_helper =
>>>> drm_fb_helper_from_client(client);
>>>> +       struct drm_device *dev = client->dev;
>>>> +       struct pci_dev *pdev = to_pci_dev(dev->dev);
>>>> +       int ret;
>>>> +
>>>> +       if (dev->fb_helper)
>>>> +               return intel_fbdev_output_poll_changed(dev);
>>>> +
>>>> +       ret = drm_fb_helper_init(dev, fb_helper);
>>>> +       if (ret)
>>>> +               goto err_drm_err;
>>>> +
>>>> +       ret = drm_fb_helper_initial_config(fb_helper);
>>>> +       if (ret)
>>>> +               goto err_drm_fb_helper_fini;
>>>> +
>>>> +       vga_switcheroo_client_fb_set(pdev, fb_helper->info);
>>>> +
>>>> +       return 0;
>>>> +
>>>> +err_drm_fb_helper_fini:
>>>> +       drm_fb_helper_fini(fb_helper);
>>>> +err_drm_err:
>>>> +       drm_err(dev, "Failed to setup i915 fbdev emulation
>>>> (ret=%d)\n", ret);
>>>> +       return ret;
>>>>    }
>>>>    
>>>>    static const struct drm_client_funcs intel_fbdev_client_funcs =
>>>> {
>>>> @@ -626,22 +649,23 @@ static const struct drm_client_funcs
>>>> intel_fbdev_client_funcs = {
>>>>           .hotplug        = intel_fbdev_client_hotplug,
>>>>    };
>>>>    
>>>> -int intel_fbdev_init(struct drm_device *dev)
>>>> +void intel_fbdev_setup(struct drm_i915_private *i915)
>>>>    {
>>>> -       struct drm_i915_private *dev_priv = to_i915(dev);
>>>> +       struct drm_device *dev = &i915->drm;
>>>>           struct intel_fbdev *ifbdev;
>>>>           int ret;
>>>>    
>>>> -       if (drm_WARN_ON(dev, !HAS_DISPLAY(dev_priv)))
>>>> -               return -ENODEV;
>>>> +       if (!HAS_DISPLAY(i915))
>>>> +               return;
>>>>    
>>>>           ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
>>>>           if (!ifbdev)
>>>> -               return -ENOMEM;
>>>> -
>>>> -       mutex_init(&ifbdev->hpd_lock);
>>>> +               return;
>>>>           drm_fb_helper_prepare(dev, &ifbdev->helper, 32,
>>>> &intel_fb_helper_funcs);
>>>>    
>>>> +       i915->display.fbdev.fbdev = ifbdev;
>>>> +       INIT_WORK(&i915->display.fbdev.suspend_work,
>>>> intel_fbdev_suspend_worker);
>>>> +       mutex_init(&ifbdev->hpd_lock);
>>>>           if (intel_fbdev_init_bios(dev, ifbdev))
>>>>                   ifbdev->helper.preferred_bpp = ifbdev-
>>>>> preferred_bpp;
>>>>           else
>>>> @@ -649,68 +673,19 @@ int intel_fbdev_init(struct drm_device
>>>> *dev)
>>>>    
>>>>           ret = drm_client_init(dev, &ifbdev->helper.client,
>>>> "intel-
>>>> fbdev",
>>>>                                 &intel_fbdev_client_funcs);
>>>> -       if (ret)
>>>> +       if (ret) {
>>>> +               drm_err(dev, "Failed to register client: %d\n",
>>>> ret);
>>>>                   goto err_drm_fb_helper_unprepare;
>>>> +       }
>>>>    
>>>> -       ret = drm_fb_helper_init(dev, &ifbdev->helper);
>>>> -       if (ret)
>>>> -               goto err_drm_client_release;
>>>> -
>>>> -       dev_priv->display.fbdev.fbdev = ifbdev;
>>>> -       INIT_WORK(&dev_priv->display.fbdev.suspend_work,
>>>> intel_fbdev_suspend_worker);
>>>> +       drm_client_register(&ifbdev->helper.client);
>>>>    
>>>> -       return 0;
>>>> +       return;
>>>>    
>>>> -err_drm_client_release:
>>>> -       drm_client_release(&ifbdev->helper.client);
>>>>    err_drm_fb_helper_unprepare:
>>>>           drm_fb_helper_unprepare(&ifbdev->helper);
>>>> +       mutex_destroy(&ifbdev->hpd_lock);
>>>>           kfree(ifbdev);
>>>> -       return ret;
>>>> -}
>>>> -
>>>> -static void intel_fbdev_initial_config(void *data,
>>>> async_cookie_t
>>>> cookie)
>>>> -{
>>>> -       struct intel_fbdev *ifbdev = data;
>>>> -
>>>> -       /* Due to peculiar init order wrt to hpd handling this is
>>>> separate. */
>>>> -       if (drm_fb_helper_initial_config(&ifbdev->helper))
>>>> -               intel_fbdev_unregister(to_i915(ifbdev-
>>>>> helper.dev));
>>>> -}
>>>> -
>>>> -void intel_fbdev_initial_config_async(struct drm_i915_private
>>>> *dev_priv)
>>>> -{
>>>> -       struct intel_fbdev *ifbdev = dev_priv-
>>>>> display.fbdev.fbdev;
>>>> -
>>>> -       if (!ifbdev)
>>>> -               return;
>>>> -
>>>> -       ifbdev->cookie =
>>>> async_schedule(intel_fbdev_initial_config,
>>>> ifbdev);
>>>> -}
>>>> -
>>>> -void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
>>>> -{
>>>> -       struct intel_fbdev *ifbdev = dev_priv-
>>>>> display.fbdev.fbdev;
>>>> -
>>>> -       if (!ifbdev)
>>>> -               return;
>>>> -
>>>> -       intel_fbdev_set_suspend(&dev_priv->drm,
>>>> FBINFO_STATE_SUSPENDED, true);
>>>> -
>>>> -       if (!current_is_async())
>>>> -               intel_fbdev_sync(ifbdev);
>>>> -
>>>> -       drm_fb_helper_unregister_info(&ifbdev->helper);
>>>> -}
>>>> -
>>>> -void intel_fbdev_fini(struct drm_i915_private *dev_priv)
>>>> -{
>>>> -       struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv-
>>>>> display.fbdev.fbdev);
>>>> -
>>>> -       if (!ifbdev)
>>>> -               return;
>>>> -
>>>> -       intel_fbdev_destroy(ifbdev);
>>>>    }
>>>>    
>>>>    struct intel_framebuffer *intel_fbdev_framebuffer(struct
>>>> intel_fbdev
>>>> *fbdev)
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h
>>>> b/drivers/gpu/drm/i915/display/intel_fbdev.h
>>>> index 8c953f102ba22..08de2d5b34338 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.h
>>>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.h
>>>> @@ -14,27 +14,11 @@ struct intel_fbdev;
>>>>    struct intel_framebuffer;
>>>>    
>>>>    #ifdef CONFIG_DRM_FBDEV_EMULATION
>>>> -int intel_fbdev_init(struct drm_device *dev);
>>>> -void intel_fbdev_initial_config_async(struct drm_i915_private
>>>> *dev_priv);
>>>> -void intel_fbdev_unregister(struct drm_i915_private *dev_priv);
>>>> -void intel_fbdev_fini(struct drm_i915_private *dev_priv);
>>>> +void intel_fbdev_setup(struct drm_i915_private *dev_priv);
>>>>    void intel_fbdev_set_suspend(struct drm_device *dev, int state,
>>>> bool
>>>> synchronous);
>>>>    struct intel_framebuffer *intel_fbdev_framebuffer(struct
>>>> intel_fbdev
>>>> *fbdev);
>>>>    #else
>>>> -static inline int intel_fbdev_init(struct drm_device *dev)
>>>> -{
>>>> -       return 0;
>>>> -}
>>>> -
>>>> -static inline void intel_fbdev_initial_config_async(struct
>>>> drm_i915_private *dev_priv)
>>>> -{
>>>> -}
>>>> -
>>>> -static inline void intel_fbdev_unregister(struct
>>>> drm_i915_private
>>>> *dev_priv)
>>>> -{
>>>> -}
>>>> -
>>>> -static inline void intel_fbdev_fini(struct drm_i915_private
>>>> *dev_priv)
>>>> +static inline void intel_fbdev_setup(struct drm_i915_private
>>>> *dev_priv)
>>>>    {
>>>>    }
>>>>    
>>>> diff --git a/drivers/gpu/drm/xe/display/xe_display.c
>>>> b/drivers/gpu/drm/xe/display/xe_display.c
>>>> index cdbc3f04c80a7..ca5cbe1d8a03b 100644
>>>> --- a/drivers/gpu/drm/xe/display/xe_display.c
>>>> +++ b/drivers/gpu/drm/xe/display/xe_display.c
>>>> @@ -214,9 +214,7 @@ void xe_display_fini(struct xe_device *xe)
>>>>           if (!xe->info.enable_display)
>>>>                   return;
>>>>    
>>>> -       /* poll work can call into fbdev, hence clean that up
>>>> afterwards */
>>>>           intel_hpd_poll_fini(xe);
>>>> -       intel_fbdev_fini(xe);
>>>>    
>>>>           intel_hdcp_component_fini(xe);
>>>>           intel_audio_deinit(xe);

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH v8 6/6] drm/{i915,xe}: Implement fbdev emulation as in-kernel client

[Jani Nikula]

On Tue, 23 Apr 2024, "Hogander, Jouni" <jouni.hogander@intel.com> wrote:
> On Tue, 2024-04-23 at 13:13 +0200, Thomas Zimmermann wrote:
>> Thank you so much. All patches has R-bs. Can you add the series to
>> the intel tree?
>
> Is it ok to merge patch 01/06 via intel tree as well?
>
> Rodrigo, This set is containing Xe display changes as well. Is it ok to
> push this via drm-intel?

For that we'll need an ack from the xe maintainers; usually Lucas for
the display stuff.

BR,
Jani.


-- 
Jani Nikula, Intel

Re: [PATCH v8 0/6] drm/{i915,xe}: Convert fbdev to DRM client

[Lucas De Marchi]

On Tue, Apr 09, 2024 at 10:04:22AM GMT, Thomas Zimmermann wrote:
>(was: drm/i915: Convert fbdev to DRM client)
>
>Convert i915's fbdev code to struct drm_client. Replaces the current
>ad-hoc integration. The conversion includes a number of cleanups. Also
>update the xe driver accordingly.
>
>As with the other drivers' fbdev emulation, fbdev in i915 is now
>an in-kernel DRM client that runs after the DRM device has been
>registered. This allows to remove the asynchronous initialization.
>
>i915 and xe are the final drivers with an fbdev emulation that is not
>build upon struct drm_client. Once reviewed, the patches would ideally go
>into drm-misc-next, so that the old fbdev helper code can be removed.
>
>We can also attempt to add additional in-kernel clients. A DRM-based
>dmesg log or a bootsplash are commonly mentioned. DRM can then switch
>easily among the existing clients if/when required.
>
>v8:
>- do setup and cleanup in intel_display_driver_{register,unregister}()
>  (Jani, Jouni)
>- mention xe in several commit messages (Rodrigo, Jani)
>- resort patches to put driver-independent changes first
>- slightly reword cover letter
>
>v7:
>- update xe driver
>
>v6:
>- reorder patches to fix build (Jouni)
>- remove unnecessary handling of non-atomic commits (Jouni, Ville)
>- return errors from callbacks (Jouni)
>- various minor fixes
>
>v5:
>- style fixes (checkpatch)
>	
>v4:
><no changes>
>		
>v3:
>- support module unloading (Jani, CI bot)
>- as before, silently ignore devices without displays (CI  bot)
>
>v2:		
>- fix error handling (Jani)
>- fix non-fbdev builds
>- various minor fixes and cleanups
>
>Thomas Zimmermann (6):
>  drm/client: Export drm_client_dev_unregister()
>  drm/i915: Move fbdev functions
>  drm/i915: Initialize fbdev DRM client with callback functions
>  drm/{i915,xe}: Unregister in-kernel clients
>  drm/{i915,xe}: Implement fbdev client callbacks
>  drm/{i915,xe}: Implement fbdev emulation as in-kernel client
>
> drivers/gpu/drm/drm_client.c                  |  13 +
> drivers/gpu/drm/i915/display/intel_display.c  |   1 -
> .../drm/i915/display/intel_display_driver.c   |  24 +-
> drivers/gpu/drm/i915/display/intel_fbdev.c    | 265 ++++++++++--------
> drivers/gpu/drm/i915/display/intel_fbdev.h    |  29 +-
> drivers/gpu/drm/i915/i915_driver.c            |  22 --
> drivers/gpu/drm/xe/display/xe_display.c       |  11 -
> drivers/gpu/drm/xe/xe_device.c                |   1 +


Acked-by: Lucas De Marchi <lucas.demarchi@intel.com>

thanks
Lucas De Marchi

> 8 files changed, 167 insertions(+), 199 deletions(-)
>
>-- 
>2.44.0
>

Re: [PATCH v8 6/6] drm/{i915,xe}: Implement fbdev emulation as in-kernel client

[Jani Nikula]

On Tue, 23 Apr 2024, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Hi
>
> Am 23.04.24 um 13:36 schrieb Hogander, Jouni:
>> On Tue, 2024-04-23 at 13:13 +0200, Thomas Zimmermann wrote:
>>> Hi
>>>
>>> Am 22.04.24 um 16:11 schrieb Hogander, Jouni:
>>>> On Tue, 2024-04-09 at 10:04 +0200, Thomas Zimmermann wrote:
>>>>> Replace all code that initializes or releases fbdev emulation
>>>>> throughout the driver. Instead initialize the fbdev client by a
>>>>> single call to intel_fbdev_setup() after i915 has registered its
>>>>> DRM device. Just like similar code in other drivers, i915 fbdev
>>>>> emulation now acts like a regular DRM client. Do the same for xe.
>>>>>
>>>>> The fbdev client setup consists of the initial preparation and
>>>>> the
>>>>> hot-plugging of the display. The latter creates the fbdev device
>>>>> and sets up the fbdev framebuffer. The setup performs display
>>>>> hot-plugging once. If no display can be detected, DRM probe
>>>>> helpers
>>>>> re-run the detection on each hotplug event.
>>>>>
>>>>> A call to drm_client_dev_unregister() releases all in-kernel
>>>>> clients
>>>>> automatically. No further action is required within i915. If the
>>>>> fbdev
>>>>> framebuffer has been fully set up, struct fb_ops.fb_destroy
>>>>> implements
>>>>> the release. For partially initialized emulation, the fbdev
>>>>> client
>>>>> reverts the initial setup. Do the same for xe and remove its call
>>>>> to
>>>>> intel_fbdev_fini().
>>>>>
>>>>> v8:
>>>>> - setup client in intel_display_driver_register (Jouni)
>>>>> - mention xe in commit message
>>>>>
>>>>> v7:
>>>>> - update xe driver
>>>>> - reword commit message
>>>>>
>>>>> v6:
>>>>> - use 'i915' for i915 device (Jouni)
>>>>> - remove unnecessary code for non-atomic mode setting (Jouni,
>>>>> Ville)
>>>>> - fix function name in commit message (Jouni)
>>>>>
>>>>> v3:
>>>>> -  as before, silently ignore devices without displays
>>>>>
>>>>> v2:
>>>>> -  let drm_client_register() handle initial hotplug
>>>>> -  fix driver name in error message (Jani)
>>>>> -  fix non-fbdev build (kernel test robot)
>>>>>
>>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
>>> Thank you so much. All patches has R-bs. Can you add the series to
>>> the
>>> intel tree?
>> Is it ok to merge patch 01/06 via intel tree as well?
>
> Sure, np.

Pushed the series to drm-intel-next, thanks for the patches and
review. And the patience in waiting for us to getting this merged!

BR,
Jani.


>
> Best regards
> Thomas
>
>>
>> Rodrigo, This set is containing Xe display changes as well. Is it ok to
>> push this via drm-intel?
>>
>> BR,
>>
>> Jouni Högander
>>
>>> Best regards
>>> Thomas
>>>
>>>>> ---
>>>>>    drivers/gpu/drm/i915/display/intel_display.c  |   1 -
>>>>>    .../drm/i915/display/intel_display_driver.c   |  20 +-
>>>>>    drivers/gpu/drm/i915/display/intel_fbdev.c    | 177 ++++++++---
>>>>> -----
>>>>> --
>>>>>    drivers/gpu/drm/i915/display/intel_fbdev.h    |  20 +-
>>>>>    drivers/gpu/drm/xe/display/xe_display.c       |   2 -
>>>>>    5 files changed, 80 insertions(+), 140 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
>>>>> b/drivers/gpu/drm/i915/display/intel_display.c
>>>>> index 614e60420a29a..161a5aabf6746 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>>>> @@ -85,7 +85,6 @@
>>>>>    #include "intel_dvo.h"
>>>>>    #include "intel_fb.h"
>>>>>    #include "intel_fbc.h"
>>>>> -#include "intel_fbdev.h"
>>>>>    #include "intel_fdi.h"
>>>>>    #include "intel_fifo_underrun.h"
>>>>>    #include "intel_frontbuffer.h"
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c
>>>>> b/drivers/gpu/drm/i915/display/intel_display_driver.c
>>>>> index e5f052d4ff1cc..ed8589fa35448 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
>>>>> @@ -514,10 +514,6 @@ int intel_display_driver_probe(struct
>>>>> drm_i915_private *i915)
>>>>>    
>>>>>           intel_overlay_setup(i915);
>>>>>    
>>>>> -       ret = intel_fbdev_init(&i915->drm);
>>>>> -       if (ret)
>>>>> -               return ret;
>>>>> -
>>>>>           /* Only enable hotplug handling once the fbdev is fully
>>>>> set
>>>>> up. */
>>>>>           intel_hpd_init(i915);
>>>>>    
>>>>> @@ -544,16 +540,6 @@ void intel_display_driver_register(struct
>>>>> drm_i915_private *i915)
>>>>>    
>>>>>           intel_display_debugfs_register(i915);
>>>>>    
>>>>> -       /*
>>>>> -        * Some ports require correctly set-up hpd registers for
>>>>> -        * detection to work properly (leading to ghost connected
>>>>> -        * connector status), e.g. VGA on gm45.  Hence we can
>>>>> only
>>>>> set
>>>>> -        * up the initial fbdev config after hpd irqs are fully
>>>>> -        * enabled. We do it last so that the async config cannot
>>>>> run
>>>>> -        * before the connectors are registered.
>>>>> -        */
>>>>> -       intel_fbdev_initial_config_async(i915);
>>>>> -
>>>>>           /*
>>>>>            * We need to coordinate the hotplugs with the
>>>>> asynchronous
>>>>>            * fbdev configuration, for which we use the
>>>>> @@ -562,6 +548,8 @@ void intel_display_driver_register(struct
>>>>> drm_i915_private *i915)
>>>>>           drm_kms_helper_poll_init(&i915->drm);
>>>>>           intel_hpd_poll_disable(i915);
>>>>>    
>>>>> +       intel_fbdev_setup(i915);
>>>>> +
>>>>>           intel_display_device_info_print(DISPLAY_INFO(i915),
>>>>>                                           DISPLAY_RUNTIME_INFO(i91
>>>>> 5),
>>>>> &p);
>>>>>    }
>>>>> @@ -597,9 +585,6 @@ void intel_display_driver_remove_noirq(struct
>>>>> drm_i915_private *i915)
>>>>>            */
>>>>>           intel_hpd_poll_fini(i915);
>>>>>    
>>>>> -       /* poll work can call into fbdev, hence clean that up
>>>>> afterwards */
>>>>> -       intel_fbdev_fini(i915);
>>>>> -
>>>>>           intel_unregister_dsm_handler();
>>>>>    
>>>>>           /* flush any delayed tasks or pending work */
>>>>> @@ -640,7 +625,6 @@ void intel_display_driver_unregister(struct
>>>>> drm_i915_private *i915)
>>>>>    
>>>>>           drm_client_dev_unregister(&i915->drm);
>>>>>    
>>>>> -       intel_fbdev_unregister(i915);
>>>>>           /*
>>>>>            * After flushing the fbdev (incl. a late async config
>>>>> which
>>>>>            * will have delayed queuing of a hotplug event), then
>>>>> flush
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>>> b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>>> index f783de611a7f5..bda702c2cab8e 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>>> @@ -24,7 +24,6 @@
>>>>>     *     David Airlie
>>>>>     */
>>>>>    
>>>>> -#include <linux/async.h>
>>>>>    #include <linux/console.h>
>>>>>    #include <linux/delay.h>
>>>>>    #include <linux/errno.h>
>>>>> @@ -39,6 +38,7 @@
>>>>>    #include <linux/vga_switcheroo.h>
>>>>>    
>>>>>    #include <drm/drm_crtc.h>
>>>>> +#include <drm/drm_crtc_helper.h>
>>>>>    #include <drm/drm_fb_helper.h>
>>>>>    #include <drm/drm_fourcc.h>
>>>>>    #include <drm/drm_gem_framebuffer_helper.h>
>>>>> @@ -58,7 +58,6 @@ struct intel_fbdev {
>>>>>           struct intel_framebuffer *fb;
>>>>>           struct i915_vma *vma;
>>>>>           unsigned long vma_flags;
>>>>> -       async_cookie_t cookie;
>>>>>           int preferred_bpp;
>>>>>    
>>>>>           /* Whether or not fbdev hpd processing is temporarily
>>>>> suspended */
>>>>> @@ -135,6 +134,26 @@ static int intel_fbdev_mmap(struct fb_info
>>>>> *info, struct vm_area_struct *vma)
>>>>>           return i915_gem_fb_mmap(obj, vma);
>>>>>    }
>>>>>    
>>>>> +static void intel_fbdev_fb_destroy(struct fb_info *info)
>>>>> +{
>>>>> +       struct drm_fb_helper *fb_helper = info->par;
>>>>> +       struct intel_fbdev *ifbdev = container_of(fb_helper,
>>>>> struct
>>>>> intel_fbdev, helper);
>>>>> +
>>>>> +       drm_fb_helper_fini(&ifbdev->helper);
>>>>> +
>>>>> +       /*
>>>>> +        * 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.
>>>>> +        */
>>>>> +       intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags);
>>>>> +       drm_framebuffer_remove(&ifbdev->fb->base);
>>>>> +
>>>>> +       drm_client_release(&fb_helper->client);
>>>>> +       drm_fb_helper_unprepare(&ifbdev->helper);
>>>>> +       kfree(ifbdev);
>>>>> +}
>>>>> +
>>>>>    __diag_push();
>>>>>    __diag_ignore_all("-Woverride-init", "Allow field
>>>>> initialization
>>>>> overrides for fb ops");
>>>>>    
>>>>> @@ -147,6 +166,7 @@ static const struct fb_ops intelfb_ops = {
>>>>>           .fb_pan_display = intel_fbdev_pan_display,
>>>>>           __FB_DEFAULT_DEFERRED_OPS_DRAW(intel_fbdev),
>>>>>           .fb_mmap = intel_fbdev_mmap,
>>>>> +       .fb_destroy = intel_fbdev_fb_destroy,
>>>>>    };
>>>>>    
>>>>>    __diag_pop();
>>>>> @@ -158,7 +178,6 @@ static int intelfb_create(struct
>>>>> drm_fb_helper
>>>>> *helper,
>>>>>           struct intel_framebuffer *intel_fb = ifbdev->fb;
>>>>>           struct drm_device *dev = helper->dev;
>>>>>           struct drm_i915_private *dev_priv = to_i915(dev);
>>>>> -       struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
>>>>>           const struct i915_gtt_view view = {
>>>>>                   .type = I915_GTT_VIEW_NORMAL,
>>>>>           };
>>>>> @@ -250,7 +269,7 @@ static int intelfb_create(struct
>>>>> drm_fb_helper
>>>>> *helper,
>>>>>           ifbdev->vma_flags = flags;
>>>>>    
>>>>>           intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
>>>>> -       vga_switcheroo_client_fb_set(pdev, info);
>>>>> +
>>>>>           return 0;
>>>>>    
>>>>>    out_unpin:
>>>>> @@ -276,26 +295,6 @@ static const struct drm_fb_helper_funcs
>>>>> intel_fb_helper_funcs = {
>>>>>           .fb_dirty = intelfb_dirty,
>>>>>    };
>>>>>    
>>>>> -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);
>>>>> -
>>>>> -       if (ifbdev->vma)
>>>>> -               intel_unpin_fb_vma(ifbdev->vma, ifbdev-
>>>>>> vma_flags);
>>>>> -
>>>>> -       if (ifbdev->fb)
>>>>> -               drm_framebuffer_remove(&ifbdev->fb->base);
>>>>> -
>>>>> -       drm_client_release(&ifbdev->helper.client);
>>>>> -       drm_fb_helper_unprepare(&ifbdev->helper);
>>>>> -       kfree(ifbdev);
>>>>> -}
>>>>> -
>>>>>    /*
>>>>>     * Build an intel_fbdev struct using a BIOS allocated
>>>>> framebuffer,
>>>>> if possible.
>>>>>     * The core display code will have read out the current plane
>>>>> configuration,
>>>>> @@ -459,16 +458,6 @@ static void
>>>>> intel_fbdev_suspend_worker(struct
>>>>> work_struct *work)
>>>>>                                   true);
>>>>>    }
>>>>>    
>>>>> -static void intel_fbdev_sync(struct intel_fbdev *ifbdev)
>>>>> -{
>>>>> -       if (!ifbdev->cookie)
>>>>> -               return;
>>>>> -
>>>>> -       /* Only serialises with all preceding async calls, hence
>>>>> +1
>>>>> */
>>>>> -       async_synchronize_cookie(ifbdev->cookie + 1);
>>>>> -       ifbdev->cookie = 0;
>>>>> -}
>>>>> -
>>>>>    /* Suspends/resumes fbdev processing of incoming HPD events.
>>>>> When
>>>>> resuming HPD
>>>>>     * processing, fbdev will perform a full connector reprobe if a
>>>>> hotplug event
>>>>>     * was received while HPD was suspended.
>>>>> @@ -559,8 +548,6 @@ static int
>>>>> intel_fbdev_output_poll_changed(struct
>>>>> drm_device *dev)
>>>>>           if (!ifbdev)
>>>>>                   return -EINVAL;
>>>>>    
>>>>> -       intel_fbdev_sync(ifbdev);
>>>>> -
>>>>>           mutex_lock(&ifbdev->hpd_lock);
>>>>>           send_hpd = !ifbdev->hpd_suspended;
>>>>>           ifbdev->hpd_waiting = true;
>>>>> @@ -580,7 +567,6 @@ static int intel_fbdev_restore_mode(struct
>>>>> drm_i915_private *dev_priv)
>>>>>           if (!ifbdev)
>>>>>                   return -EINVAL;
>>>>>    
>>>>> -       intel_fbdev_sync(ifbdev);
>>>>>           if (!ifbdev->vma)
>>>>>                   return -ENOMEM;
>>>>>    
>>>>> @@ -598,7 +584,20 @@ static int intel_fbdev_restore_mode(struct
>>>>> drm_i915_private *dev_priv)
>>>>>     */
>>>>>    
>>>>>    static void intel_fbdev_client_unregister(struct drm_client_dev
>>>>> *client)
>>>>> -{ }
>>>>> +{
>>>>> +       struct drm_fb_helper *fb_helper =
>>>>> drm_fb_helper_from_client(client);
>>>>> +       struct drm_device *dev = fb_helper->dev;
>>>>> +       struct pci_dev *pdev = to_pci_dev(dev->dev);
>>>>> +
>>>>> +       if (fb_helper->info) {
>>>>> +               vga_switcheroo_client_fb_set(pdev, NULL);
>>>>> +               drm_fb_helper_unregister_info(fb_helper);
>>>>> +       } else {
>>>>> +               drm_fb_helper_unprepare(fb_helper);
>>>>> +               drm_client_release(&fb_helper->client);
>>>>> +               kfree(fb_helper);
>>>>> +       }
>>>>> +}
>>>>>    
>>>>>    static int intel_fbdev_client_restore(struct drm_client_dev
>>>>> *client)
>>>>>    {
>>>>> @@ -616,7 +615,31 @@ static int intel_fbdev_client_restore(struct
>>>>> drm_client_dev *client)
>>>>>    
>>>>>    static int intel_fbdev_client_hotplug(struct drm_client_dev
>>>>> *client)
>>>>>    {
>>>>> -       return intel_fbdev_output_poll_changed(client->dev);
>>>>> +       struct drm_fb_helper *fb_helper =
>>>>> drm_fb_helper_from_client(client);
>>>>> +       struct drm_device *dev = client->dev;
>>>>> +       struct pci_dev *pdev = to_pci_dev(dev->dev);
>>>>> +       int ret;
>>>>> +
>>>>> +       if (dev->fb_helper)
>>>>> +               return intel_fbdev_output_poll_changed(dev);
>>>>> +
>>>>> +       ret = drm_fb_helper_init(dev, fb_helper);
>>>>> +       if (ret)
>>>>> +               goto err_drm_err;
>>>>> +
>>>>> +       ret = drm_fb_helper_initial_config(fb_helper);
>>>>> +       if (ret)
>>>>> +               goto err_drm_fb_helper_fini;
>>>>> +
>>>>> +       vga_switcheroo_client_fb_set(pdev, fb_helper->info);
>>>>> +
>>>>> +       return 0;
>>>>> +
>>>>> +err_drm_fb_helper_fini:
>>>>> +       drm_fb_helper_fini(fb_helper);
>>>>> +err_drm_err:
>>>>> +       drm_err(dev, "Failed to setup i915 fbdev emulation
>>>>> (ret=%d)\n", ret);
>>>>> +       return ret;
>>>>>    }
>>>>>    
>>>>>    static const struct drm_client_funcs intel_fbdev_client_funcs =
>>>>> {
>>>>> @@ -626,22 +649,23 @@ static const struct drm_client_funcs
>>>>> intel_fbdev_client_funcs = {
>>>>>           .hotplug        = intel_fbdev_client_hotplug,
>>>>>    };
>>>>>    
>>>>> -int intel_fbdev_init(struct drm_device *dev)
>>>>> +void intel_fbdev_setup(struct drm_i915_private *i915)
>>>>>    {
>>>>> -       struct drm_i915_private *dev_priv = to_i915(dev);
>>>>> +       struct drm_device *dev = &i915->drm;
>>>>>           struct intel_fbdev *ifbdev;
>>>>>           int ret;
>>>>>    
>>>>> -       if (drm_WARN_ON(dev, !HAS_DISPLAY(dev_priv)))
>>>>> -               return -ENODEV;
>>>>> +       if (!HAS_DISPLAY(i915))
>>>>> +               return;
>>>>>    
>>>>>           ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
>>>>>           if (!ifbdev)
>>>>> -               return -ENOMEM;
>>>>> -
>>>>> -       mutex_init(&ifbdev->hpd_lock);
>>>>> +               return;
>>>>>           drm_fb_helper_prepare(dev, &ifbdev->helper, 32,
>>>>> &intel_fb_helper_funcs);
>>>>>    
>>>>> +       i915->display.fbdev.fbdev = ifbdev;
>>>>> +       INIT_WORK(&i915->display.fbdev.suspend_work,
>>>>> intel_fbdev_suspend_worker);
>>>>> +       mutex_init(&ifbdev->hpd_lock);
>>>>>           if (intel_fbdev_init_bios(dev, ifbdev))
>>>>>                   ifbdev->helper.preferred_bpp = ifbdev-
>>>>>> preferred_bpp;
>>>>>           else
>>>>> @@ -649,68 +673,19 @@ int intel_fbdev_init(struct drm_device
>>>>> *dev)
>>>>>    
>>>>>           ret = drm_client_init(dev, &ifbdev->helper.client,
>>>>> "intel-
>>>>> fbdev",
>>>>>                                 &intel_fbdev_client_funcs);
>>>>> -       if (ret)
>>>>> +       if (ret) {
>>>>> +               drm_err(dev, "Failed to register client: %d\n",
>>>>> ret);
>>>>>                   goto err_drm_fb_helper_unprepare;
>>>>> +       }
>>>>>    
>>>>> -       ret = drm_fb_helper_init(dev, &ifbdev->helper);
>>>>> -       if (ret)
>>>>> -               goto err_drm_client_release;
>>>>> -
>>>>> -       dev_priv->display.fbdev.fbdev = ifbdev;
>>>>> -       INIT_WORK(&dev_priv->display.fbdev.suspend_work,
>>>>> intel_fbdev_suspend_worker);
>>>>> +       drm_client_register(&ifbdev->helper.client);
>>>>>    
>>>>> -       return 0;
>>>>> +       return;
>>>>>    
>>>>> -err_drm_client_release:
>>>>> -       drm_client_release(&ifbdev->helper.client);
>>>>>    err_drm_fb_helper_unprepare:
>>>>>           drm_fb_helper_unprepare(&ifbdev->helper);
>>>>> +       mutex_destroy(&ifbdev->hpd_lock);
>>>>>           kfree(ifbdev);
>>>>> -       return ret;
>>>>> -}
>>>>> -
>>>>> -static void intel_fbdev_initial_config(void *data,
>>>>> async_cookie_t
>>>>> cookie)
>>>>> -{
>>>>> -       struct intel_fbdev *ifbdev = data;
>>>>> -
>>>>> -       /* Due to peculiar init order wrt to hpd handling this is
>>>>> separate. */
>>>>> -       if (drm_fb_helper_initial_config(&ifbdev->helper))
>>>>> -               intel_fbdev_unregister(to_i915(ifbdev-
>>>>>> helper.dev));
>>>>> -}
>>>>> -
>>>>> -void intel_fbdev_initial_config_async(struct drm_i915_private
>>>>> *dev_priv)
>>>>> -{
>>>>> -       struct intel_fbdev *ifbdev = dev_priv-
>>>>>> display.fbdev.fbdev;
>>>>> -
>>>>> -       if (!ifbdev)
>>>>> -               return;
>>>>> -
>>>>> -       ifbdev->cookie =
>>>>> async_schedule(intel_fbdev_initial_config,
>>>>> ifbdev);
>>>>> -}
>>>>> -
>>>>> -void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
>>>>> -{
>>>>> -       struct intel_fbdev *ifbdev = dev_priv-
>>>>>> display.fbdev.fbdev;
>>>>> -
>>>>> -       if (!ifbdev)
>>>>> -               return;
>>>>> -
>>>>> -       intel_fbdev_set_suspend(&dev_priv->drm,
>>>>> FBINFO_STATE_SUSPENDED, true);
>>>>> -
>>>>> -       if (!current_is_async())
>>>>> -               intel_fbdev_sync(ifbdev);
>>>>> -
>>>>> -       drm_fb_helper_unregister_info(&ifbdev->helper);
>>>>> -}
>>>>> -
>>>>> -void intel_fbdev_fini(struct drm_i915_private *dev_priv)
>>>>> -{
>>>>> -       struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv-
>>>>>> display.fbdev.fbdev);
>>>>> -
>>>>> -       if (!ifbdev)
>>>>> -               return;
>>>>> -
>>>>> -       intel_fbdev_destroy(ifbdev);
>>>>>    }
>>>>>    
>>>>>    struct intel_framebuffer *intel_fbdev_framebuffer(struct
>>>>> intel_fbdev
>>>>> *fbdev)
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h
>>>>> b/drivers/gpu/drm/i915/display/intel_fbdev.h
>>>>> index 8c953f102ba22..08de2d5b34338 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.h
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.h
>>>>> @@ -14,27 +14,11 @@ struct intel_fbdev;
>>>>>    struct intel_framebuffer;
>>>>>    
>>>>>    #ifdef CONFIG_DRM_FBDEV_EMULATION
>>>>> -int intel_fbdev_init(struct drm_device *dev);
>>>>> -void intel_fbdev_initial_config_async(struct drm_i915_private
>>>>> *dev_priv);
>>>>> -void intel_fbdev_unregister(struct drm_i915_private *dev_priv);
>>>>> -void intel_fbdev_fini(struct drm_i915_private *dev_priv);
>>>>> +void intel_fbdev_setup(struct drm_i915_private *dev_priv);
>>>>>    void intel_fbdev_set_suspend(struct drm_device *dev, int state,
>>>>> bool
>>>>> synchronous);
>>>>>    struct intel_framebuffer *intel_fbdev_framebuffer(struct
>>>>> intel_fbdev
>>>>> *fbdev);
>>>>>    #else
>>>>> -static inline int intel_fbdev_init(struct drm_device *dev)
>>>>> -{
>>>>> -       return 0;
>>>>> -}
>>>>> -
>>>>> -static inline void intel_fbdev_initial_config_async(struct
>>>>> drm_i915_private *dev_priv)
>>>>> -{
>>>>> -}
>>>>> -
>>>>> -static inline void intel_fbdev_unregister(struct
>>>>> drm_i915_private
>>>>> *dev_priv)
>>>>> -{
>>>>> -}
>>>>> -
>>>>> -static inline void intel_fbdev_fini(struct drm_i915_private
>>>>> *dev_priv)
>>>>> +static inline void intel_fbdev_setup(struct drm_i915_private
>>>>> *dev_priv)
>>>>>    {
>>>>>    }
>>>>>    
>>>>> diff --git a/drivers/gpu/drm/xe/display/xe_display.c
>>>>> b/drivers/gpu/drm/xe/display/xe_display.c
>>>>> index cdbc3f04c80a7..ca5cbe1d8a03b 100644
>>>>> --- a/drivers/gpu/drm/xe/display/xe_display.c
>>>>> +++ b/drivers/gpu/drm/xe/display/xe_display.c
>>>>> @@ -214,9 +214,7 @@ void xe_display_fini(struct xe_device *xe)
>>>>>           if (!xe->info.enable_display)
>>>>>                   return;
>>>>>    
>>>>> -       /* poll work can call into fbdev, hence clean that up
>>>>> afterwards */
>>>>>           intel_hpd_poll_fini(xe);
>>>>> -       intel_fbdev_fini(xe);
>>>>>    
>>>>>           intel_hdcp_component_fini(xe);
>>>>>           intel_audio_deinit(xe);

-- 
Jani Nikula, Intel