[PATCH 0/4] drm/i915: Convert fbdev to DRM client

[PATCH 0/4] drm/i915: Convert fbdev to DRM client

[Thomas Zimmermann]

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

As with most other driver's fbdev emulation, fbdev in i915 is now
just another DRM client that runs after the DRM device has been
registered. This allows to remove the asynchronous initialization,
hopefully.

Once all drivers' fbdev emulation has been converted to struct drm_client,
we can 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.

Tested on Intel HD 530 hardware.

Thomas Zimmermann (4):
  drm/i915: Move fbdev functions
  drm/i915: Initialize fbdev DRM client with callback functions
  drm/i915: Implement fbdev client callbacks
  drm/i915: Implement fbdev emulation as in-kernel client

 drivers/gpu/drm/i915/display/intel_display.c |  31 ---
 drivers/gpu/drm/i915/display/intel_fbdev.c   | 254 ++++++++++---------
 drivers/gpu/drm/i915/display/intel_fbdev.h   |  29 +--
 drivers/gpu/drm/i915/i915_driver.c           |  24 +-
 4 files changed, 143 insertions(+), 195 deletions(-)


base-commit: 6e5f96153989e454041848f66a5227be9bd0bbc3
-- 
2.40.0

[Intel-gfx] [PATCH 0/4] drm/i915: Convert fbdev to DRM client

[Thomas Zimmermann]

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

As with most other driver's fbdev emulation, fbdev in i915 is now
just another DRM client that runs after the DRM device has been
registered. This allows to remove the asynchronous initialization,
hopefully.

Once all drivers' fbdev emulation has been converted to struct drm_client,
we can 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.

Tested on Intel HD 530 hardware.

Thomas Zimmermann (4):
  drm/i915: Move fbdev functions
  drm/i915: Initialize fbdev DRM client with callback functions
  drm/i915: Implement fbdev client callbacks
  drm/i915: Implement fbdev emulation as in-kernel client

 drivers/gpu/drm/i915/display/intel_display.c |  31 ---
 drivers/gpu/drm/i915/display/intel_fbdev.c   | 254 ++++++++++---------
 drivers/gpu/drm/i915/display/intel_fbdev.h   |  29 +--
 drivers/gpu/drm/i915/i915_driver.c           |  24 +-
 4 files changed, 143 insertions(+), 195 deletions(-)


base-commit: 6e5f96153989e454041848f66a5227be9bd0bbc3
-- 
2.40.0

[PATCH 1/4] drm/i915: Move fbdev functions

[Thomas Zimmermann]

Move functions within intel_fbdev.c to simplify later updates. No
functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 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 673bcdfb7ff6..88de79279ce5 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -531,58 +531,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)
@@ -593,31 +541,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.
@@ -733,6 +656,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(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);
+}
+
+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.40.0

[Intel-gfx] [PATCH 1/4] drm/i915: Move fbdev functions

[Thomas Zimmermann]

Move functions within intel_fbdev.c to simplify later updates. No
functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 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 673bcdfb7ff6..88de79279ce5 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -531,58 +531,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)
@@ -593,31 +541,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.
@@ -733,6 +656,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(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);
+}
+
+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.40.0

[PATCH 2/4] drm/i915: Initialize fbdev DRM client with callback functions

[Thomas Zimmermann]

Initialize i915's fbdev client by giving an instance of struct
drm_client_funcsi 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.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 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 88de79279ce5..290da5e94bc5 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -364,6 +364,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);
 }
@@ -656,6 +657,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);
@@ -677,16 +702,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, "i915-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_client_release(&ifbdev->helper.client);
+	kfree(ifbdev);
+	return ret;
 }
 
 static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
-- 
2.40.0

[Intel-gfx] [PATCH 2/4] drm/i915: Initialize fbdev DRM client with callback functions

[Thomas Zimmermann]

Initialize i915's fbdev client by giving an instance of struct
drm_client_funcsi 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.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 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 88de79279ce5..290da5e94bc5 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -364,6 +364,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);
 }
@@ -656,6 +657,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);
@@ -677,16 +702,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, "i915-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_client_release(&ifbdev->helper.client);
+	kfree(ifbdev);
+	return ret;
 }
 
 static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
-- 
2.40.0

Re: [PATCH 2/4] drm/i915: Initialize fbdev DRM client with callback functions

[Jani Nikula]

On Tue, 28 Mar 2023, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Initialize i915's fbdev client by giving an instance of struct
> drm_client_funcsi 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.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  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 88de79279ce5..290da5e94bc5 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -364,6 +364,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);
>  }
> @@ -656,6 +657,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);
> @@ -677,16 +702,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, "i915-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_client_release(&ifbdev->helper.client);

I suppose this should be

drm_fb_helper_unprepare(&ifbdev->helper);

instead of the double drm_client_release(). And we're missing this
cleanup already.

BR,
Jani.


> +	kfree(ifbdev);
> +	return ret;
>  }
>  
>  static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Initialize fbdev DRM client with callback functions

[Jani Nikula]

On Tue, 28 Mar 2023, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Initialize i915's fbdev client by giving an instance of struct
> drm_client_funcsi 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.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  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 88de79279ce5..290da5e94bc5 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -364,6 +364,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);
>  }
> @@ -656,6 +657,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);
> @@ -677,16 +702,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, "i915-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_client_release(&ifbdev->helper.client);

I suppose this should be

drm_fb_helper_unprepare(&ifbdev->helper);

instead of the double drm_client_release(). And we're missing this
cleanup already.

BR,
Jani.


> +	kfree(ifbdev);
> +	return ret;
>  }
>  
>  static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH 2/4] drm/i915: Initialize fbdev DRM client with callback functions

[Sam Ravnborg]

Hi Thomas,

On Tue, Mar 28, 2023 at 01:14:20PM +0200, Thomas Zimmermann wrote:
> Initialize i915's fbdev client by giving an instance of struct
> drm_client_funcsi to drm_client_init(). Also clean up with
An extra i had sneaked in here

	Sam

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Initialize fbdev DRM client with callback functions

[Sam Ravnborg]

Hi Thomas,

On Tue, Mar 28, 2023 at 01:14:20PM +0200, Thomas Zimmermann wrote:
> Initialize i915's fbdev client by giving an instance of struct
> drm_client_funcsi to drm_client_init(). Also clean up with
An extra i had sneaked in here

	Sam

[PATCH 3/4] drm/i915: 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.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/i915/display/intel_display.c |  1 -
 drivers/gpu/drm/i915/display/intel_fbdev.c   | 11 ++++++++--
 drivers/gpu/drm/i915/display/intel_fbdev.h   |  9 --------
 drivers/gpu/drm/i915/i915_driver.c           | 22 --------------------
 4 files changed, 9 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b53a1d969344..430a1016e013 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8189,7 +8189,6 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
 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 290da5e94bc5..bdb9e6f43602 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -623,7 +623,7 @@ 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 void intel_fbdev_output_poll_changed(struct drm_device *dev)
 {
 	struct intel_fbdev *ifbdev = to_i915(dev)->display.fbdev.fbdev;
 	bool send_hpd;
@@ -642,7 +642,7 @@ void intel_fbdev_output_poll_changed(struct drm_device *dev)
 		drm_fb_helper_hotplug_event(&ifbdev->helper);
 }
 
-void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
+static void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
 {
 	struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
 
@@ -666,11 +666,18 @@ 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);
+
+	intel_fbdev_restore_mode(dev_priv);
+	vga_switcheroo_process_delayed_switch();
+
 	return 0;
 }
 
 static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
 {
+	intel_fbdev_output_poll_changed(client->dev);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h b/drivers/gpu/drm/i915/display/intel_fbdev.h
index 04fd523a5023..8c953f102ba2 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 8dde3512d2d1..be804fd4f79f 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -926,27 +926,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;
@@ -1804,7 +1783,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,
 
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-- 
2.40.0

[Intel-gfx] [PATCH 3/4] drm/i915: 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.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/i915/display/intel_display.c |  1 -
 drivers/gpu/drm/i915/display/intel_fbdev.c   | 11 ++++++++--
 drivers/gpu/drm/i915/display/intel_fbdev.h   |  9 --------
 drivers/gpu/drm/i915/i915_driver.c           | 22 --------------------
 4 files changed, 9 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b53a1d969344..430a1016e013 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8189,7 +8189,6 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
 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 290da5e94bc5..bdb9e6f43602 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -623,7 +623,7 @@ 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 void intel_fbdev_output_poll_changed(struct drm_device *dev)
 {
 	struct intel_fbdev *ifbdev = to_i915(dev)->display.fbdev.fbdev;
 	bool send_hpd;
@@ -642,7 +642,7 @@ void intel_fbdev_output_poll_changed(struct drm_device *dev)
 		drm_fb_helper_hotplug_event(&ifbdev->helper);
 }
 
-void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
+static void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
 {
 	struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
 
@@ -666,11 +666,18 @@ 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);
+
+	intel_fbdev_restore_mode(dev_priv);
+	vga_switcheroo_process_delayed_switch();
+
 	return 0;
 }
 
 static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
 {
+	intel_fbdev_output_poll_changed(client->dev);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h b/drivers/gpu/drm/i915/display/intel_fbdev.h
index 04fd523a5023..8c953f102ba2 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 8dde3512d2d1..be804fd4f79f 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -926,27 +926,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;
@@ -1804,7 +1783,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,
 
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-- 
2.40.0

[PATCH 4/4] drm/i915: 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 i915_fbdev_setup() after i915 has registered its
DRM device. Just like in most drivers, i915 fbdev emulation now
acts like a regular DRM client.

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_dev_unregister() releases the client 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.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/i915/display/intel_display.c |  30 ---
 drivers/gpu/drm/i915/display/intel_fbdev.c   | 184 +++++++++----------
 drivers/gpu/drm/i915/display/intel_fbdev.h   |  20 +-
 drivers/gpu/drm/i915/i915_driver.c           |   2 +
 4 files changed, 87 insertions(+), 149 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 430a1016e013..1bbeb7a061e3 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"
@@ -8588,11 +8587,6 @@ int intel_modeset_init(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);
 	intel_hpd_poll_disable(i915);
 
@@ -8796,9 +8790,6 @@ void intel_modeset_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 */
@@ -8864,21 +8855,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
-	 * fbdev->async_cookie.
-	 */
 	drm_kms_helper_poll_init(&i915->drm);
 }
 
@@ -8887,14 +8863,8 @@ void intel_display_driver_unregister(struct drm_i915_private *i915)
 	if (!HAS_DISPLAY(i915))
 		return;
 
-	intel_fbdev_unregister(i915);
 	intel_audio_deinit(i915);
 
-	/*
-	 * After flushing the fbdev (incl. a late async config which
-	 * will have delayed queuing of a hotplug event), then flush
-	 * the hotplug events.
-	 */
 	drm_kms_helper_poll_fini(&i915->drm);
 	drm_atomic_helper_shutdown(&i915->drm);
 
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index bdb9e6f43602..8b618db30ee5 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>
@@ -38,6 +37,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>
 
@@ -55,7 +55,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 */
@@ -120,6 +119,26 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var,
 	return ret;
 }
 
+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);
+}
+
 static const struct fb_ops intelfb_ops = {
 	.owner = THIS_MODULE,
 	DRM_FB_HELPER_DEFAULT_OPS,
@@ -131,6 +150,7 @@ static const struct fb_ops intelfb_ops = {
 	.fb_imageblit = drm_fb_helper_cfb_imageblit,
 	.fb_pan_display = intel_fbdev_pan_display,
 	.fb_blank = intel_fbdev_blank,
+	.fb_destroy = intel_fbdev_fb_destroy,
 };
 
 static int intelfb_alloc(struct drm_fb_helper *helper,
@@ -198,7 +218,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);
 	struct i915_ggtt *ggtt = to_gt(dev_priv)->ggtt;
 	const struct i915_gtt_view view = {
 		.type = I915_GTT_VIEW_NORMAL,
@@ -323,7 +342,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:
@@ -349,26 +368,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,
@@ -532,16 +531,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.
@@ -631,8 +620,6 @@ static void intel_fbdev_output_poll_changed(struct drm_device *dev)
 	if (!ifbdev)
 		return;
 
-	intel_fbdev_sync(ifbdev);
-
 	mutex_lock(&ifbdev->hpd_lock);
 	send_hpd = !ifbdev->hpd_suspended;
 	ifbdev->hpd_waiting = true;
@@ -649,7 +636,6 @@ static void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
 	if (!ifbdev)
 		return;
 
-	intel_fbdev_sync(ifbdev);
 	if (!ifbdev->vma)
 		return;
 
@@ -662,7 +648,20 @@ static void 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)
 {
@@ -676,9 +675,36 @@ static int intel_fbdev_client_restore(struct drm_client_dev *client)
 
 static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
 {
-	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) {
+		intel_fbdev_output_poll_changed(dev);
+		return 0;
+	}
+
+	ret = drm_fb_helper_init(dev, fb_helper);
+	if (ret)
+		goto err_drm_err;
+
+	if (!drm_drv_uses_atomic_modeset(dev))
+		drm_helper_disable_unused_functions(dev);
+
+	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 radeon fbdev emulation (ret=%d)\n", ret);
+	return ret;
 }
 
 static const struct drm_client_funcs intel_fbdev_client_funcs = {
@@ -688,22 +714,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 *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_device *dev = &dev_priv->drm;
 	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;
+		return;
 
-	mutex_init(&ifbdev->hpd_lock);
+	ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
+	if (!ifbdev)
+		return;
 	drm_fb_helper_prepare(dev, &ifbdev->helper, 32, &intel_fb_helper_funcs);
 
+	dev_priv->display.fbdev.fbdev = ifbdev;
+	INIT_WORK(&dev_priv->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
@@ -711,68 +738,23 @@ int intel_fbdev_init(struct drm_device *dev)
 
 	ret = drm_client_init(dev, &ifbdev->helper.client, "i915-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);
+	ret = intel_fbdev_client_hotplug(&ifbdev->helper.client);
 	if (ret)
-		goto err_drm_client_release;
+		drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
 
-	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_client_release(&ifbdev->helper.client);
+	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 8c953f102ba2..8123c813caaf 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)
+void intel_fbdev_setup(struct drm_i915_private *dev_priv);
 {
 }
 
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index be804fd4f79f..0b6722bcfbb2 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -820,6 +820,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	i915->do_release = true;
 
+	intel_fbdev_setup(i915);
+
 	return 0;
 
 out_cleanup_gem:
-- 
2.40.0

[Intel-gfx] [PATCH 4/4] drm/i915: 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 i915_fbdev_setup() after i915 has registered its
DRM device. Just like in most drivers, i915 fbdev emulation now
acts like a regular DRM client.

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_dev_unregister() releases the client 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.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/i915/display/intel_display.c |  30 ---
 drivers/gpu/drm/i915/display/intel_fbdev.c   | 184 +++++++++----------
 drivers/gpu/drm/i915/display/intel_fbdev.h   |  20 +-
 drivers/gpu/drm/i915/i915_driver.c           |   2 +
 4 files changed, 87 insertions(+), 149 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 430a1016e013..1bbeb7a061e3 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"
@@ -8588,11 +8587,6 @@ int intel_modeset_init(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);
 	intel_hpd_poll_disable(i915);
 
@@ -8796,9 +8790,6 @@ void intel_modeset_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 */
@@ -8864,21 +8855,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
-	 * fbdev->async_cookie.
-	 */
 	drm_kms_helper_poll_init(&i915->drm);
 }
 
@@ -8887,14 +8863,8 @@ void intel_display_driver_unregister(struct drm_i915_private *i915)
 	if (!HAS_DISPLAY(i915))
 		return;
 
-	intel_fbdev_unregister(i915);
 	intel_audio_deinit(i915);
 
-	/*
-	 * After flushing the fbdev (incl. a late async config which
-	 * will have delayed queuing of a hotplug event), then flush
-	 * the hotplug events.
-	 */
 	drm_kms_helper_poll_fini(&i915->drm);
 	drm_atomic_helper_shutdown(&i915->drm);
 
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index bdb9e6f43602..8b618db30ee5 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>
@@ -38,6 +37,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>
 
@@ -55,7 +55,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 */
@@ -120,6 +119,26 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var,
 	return ret;
 }
 
+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);
+}
+
 static const struct fb_ops intelfb_ops = {
 	.owner = THIS_MODULE,
 	DRM_FB_HELPER_DEFAULT_OPS,
@@ -131,6 +150,7 @@ static const struct fb_ops intelfb_ops = {
 	.fb_imageblit = drm_fb_helper_cfb_imageblit,
 	.fb_pan_display = intel_fbdev_pan_display,
 	.fb_blank = intel_fbdev_blank,
+	.fb_destroy = intel_fbdev_fb_destroy,
 };
 
 static int intelfb_alloc(struct drm_fb_helper *helper,
@@ -198,7 +218,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);
 	struct i915_ggtt *ggtt = to_gt(dev_priv)->ggtt;
 	const struct i915_gtt_view view = {
 		.type = I915_GTT_VIEW_NORMAL,
@@ -323,7 +342,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:
@@ -349,26 +368,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,
@@ -532,16 +531,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.
@@ -631,8 +620,6 @@ static void intel_fbdev_output_poll_changed(struct drm_device *dev)
 	if (!ifbdev)
 		return;
 
-	intel_fbdev_sync(ifbdev);
-
 	mutex_lock(&ifbdev->hpd_lock);
 	send_hpd = !ifbdev->hpd_suspended;
 	ifbdev->hpd_waiting = true;
@@ -649,7 +636,6 @@ static void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
 	if (!ifbdev)
 		return;
 
-	intel_fbdev_sync(ifbdev);
 	if (!ifbdev->vma)
 		return;
 
@@ -662,7 +648,20 @@ static void 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)
 {
@@ -676,9 +675,36 @@ static int intel_fbdev_client_restore(struct drm_client_dev *client)
 
 static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
 {
-	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) {
+		intel_fbdev_output_poll_changed(dev);
+		return 0;
+	}
+
+	ret = drm_fb_helper_init(dev, fb_helper);
+	if (ret)
+		goto err_drm_err;
+
+	if (!drm_drv_uses_atomic_modeset(dev))
+		drm_helper_disable_unused_functions(dev);
+
+	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 radeon fbdev emulation (ret=%d)\n", ret);
+	return ret;
 }
 
 static const struct drm_client_funcs intel_fbdev_client_funcs = {
@@ -688,22 +714,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 *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_device *dev = &dev_priv->drm;
 	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;
+		return;
 
-	mutex_init(&ifbdev->hpd_lock);
+	ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
+	if (!ifbdev)
+		return;
 	drm_fb_helper_prepare(dev, &ifbdev->helper, 32, &intel_fb_helper_funcs);
 
+	dev_priv->display.fbdev.fbdev = ifbdev;
+	INIT_WORK(&dev_priv->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
@@ -711,68 +738,23 @@ int intel_fbdev_init(struct drm_device *dev)
 
 	ret = drm_client_init(dev, &ifbdev->helper.client, "i915-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);
+	ret = intel_fbdev_client_hotplug(&ifbdev->helper.client);
 	if (ret)
-		goto err_drm_client_release;
+		drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
 
-	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_client_release(&ifbdev->helper.client);
+	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 8c953f102ba2..8123c813caaf 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)
+void intel_fbdev_setup(struct drm_i915_private *dev_priv);
 {
 }
 
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index be804fd4f79f..0b6722bcfbb2 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -820,6 +820,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	i915->do_release = true;
 
+	intel_fbdev_setup(i915);
+
 	return 0;
 
 out_cleanup_gem:
-- 
2.40.0

Re: [PATCH 4/4] drm/i915: Implement fbdev emulation as in-kernel client

[Jani Nikula]

On Tue, 28 Mar 2023, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Replace all code that initializes or releases fbdev emulation
> throughout the driver. Instead initialize the fbdev client by a
> single call to i915_fbdev_setup() after i915 has registered its
> DRM device. Just like in most drivers, i915 fbdev emulation now
> acts like a regular DRM client.
>
> 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_dev_unregister() releases the client 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.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

I like the direction in the series, but I'm afraid I don't have the time
for proper detailed review of this right now. Just a couple of nits I
stumbled on inline.

Anyone else have a chance to get this reviewed?

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/display/intel_display.c |  30 ---
>  drivers/gpu/drm/i915/display/intel_fbdev.c   | 184 +++++++++----------
>  drivers/gpu/drm/i915/display/intel_fbdev.h   |  20 +-
>  drivers/gpu/drm/i915/i915_driver.c           |   2 +
>  4 files changed, 87 insertions(+), 149 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 430a1016e013..1bbeb7a061e3 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"
> @@ -8588,11 +8587,6 @@ int intel_modeset_init(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);
>  	intel_hpd_poll_disable(i915);
>  
> @@ -8796,9 +8790,6 @@ void intel_modeset_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 */
> @@ -8864,21 +8855,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
> -	 * fbdev->async_cookie.
> -	 */
>  	drm_kms_helper_poll_init(&i915->drm);
>  }
>  
> @@ -8887,14 +8863,8 @@ void intel_display_driver_unregister(struct drm_i915_private *i915)
>  	if (!HAS_DISPLAY(i915))
>  		return;
>  
> -	intel_fbdev_unregister(i915);
>  	intel_audio_deinit(i915);
>  
> -	/*
> -	 * After flushing the fbdev (incl. a late async config which
> -	 * will have delayed queuing of a hotplug event), then flush
> -	 * the hotplug events.
> -	 */
>  	drm_kms_helper_poll_fini(&i915->drm);
>  	drm_atomic_helper_shutdown(&i915->drm);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index bdb9e6f43602..8b618db30ee5 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>
> @@ -38,6 +37,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>
>  
> @@ -55,7 +55,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 */
> @@ -120,6 +119,26 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var,
>  	return ret;
>  }
>  
> +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);
> +}
> +
>  static const struct fb_ops intelfb_ops = {
>  	.owner = THIS_MODULE,
>  	DRM_FB_HELPER_DEFAULT_OPS,
> @@ -131,6 +150,7 @@ static const struct fb_ops intelfb_ops = {
>  	.fb_imageblit = drm_fb_helper_cfb_imageblit,
>  	.fb_pan_display = intel_fbdev_pan_display,
>  	.fb_blank = intel_fbdev_blank,
> +	.fb_destroy = intel_fbdev_fb_destroy,
>  };
>  
>  static int intelfb_alloc(struct drm_fb_helper *helper,
> @@ -198,7 +218,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);
>  	struct i915_ggtt *ggtt = to_gt(dev_priv)->ggtt;
>  	const struct i915_gtt_view view = {
>  		.type = I915_GTT_VIEW_NORMAL,
> @@ -323,7 +342,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:
> @@ -349,26 +368,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,
> @@ -532,16 +531,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.
> @@ -631,8 +620,6 @@ static void intel_fbdev_output_poll_changed(struct drm_device *dev)
>  	if (!ifbdev)
>  		return;
>  
> -	intel_fbdev_sync(ifbdev);
> -
>  	mutex_lock(&ifbdev->hpd_lock);
>  	send_hpd = !ifbdev->hpd_suspended;
>  	ifbdev->hpd_waiting = true;
> @@ -649,7 +636,6 @@ static void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
>  	if (!ifbdev)
>  		return;
>  
> -	intel_fbdev_sync(ifbdev);
>  	if (!ifbdev->vma)
>  		return;
>  
> @@ -662,7 +648,20 @@ static void 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)
>  {
> @@ -676,9 +675,36 @@ static int intel_fbdev_client_restore(struct drm_client_dev *client)
>  
>  static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
>  {
> -	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) {
> +		intel_fbdev_output_poll_changed(dev);
> +		return 0;
> +	}
> +
> +	ret = drm_fb_helper_init(dev, fb_helper);
> +	if (ret)
> +		goto err_drm_err;
> +
> +	if (!drm_drv_uses_atomic_modeset(dev))
> +		drm_helper_disable_unused_functions(dev);
> +
> +	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 radeon fbdev emulation (ret=%d)\n", ret);
                                      ^^^^^^

Hrrmmh. ;)

> +	return ret;
>  }
>  
>  static const struct drm_client_funcs intel_fbdev_client_funcs = {
> @@ -688,22 +714,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 *dev_priv)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_device *dev = &dev_priv->drm;
>  	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;
> +		return;
>  
> -	mutex_init(&ifbdev->hpd_lock);
> +	ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
> +	if (!ifbdev)
> +		return;
>  	drm_fb_helper_prepare(dev, &ifbdev->helper, 32, &intel_fb_helper_funcs);
>  
> +	dev_priv->display.fbdev.fbdev = ifbdev;
> +	INIT_WORK(&dev_priv->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
> @@ -711,68 +738,23 @@ int intel_fbdev_init(struct drm_device *dev)
>  
>  	ret = drm_client_init(dev, &ifbdev->helper.client, "i915-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);
> +	ret = intel_fbdev_client_hotplug(&ifbdev->helper.client);
>  	if (ret)
> -		goto err_drm_client_release;
> +		drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
>  
> -	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_client_release(&ifbdev->helper.client);
> +	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 8c953f102ba2..8123c813caaf 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)
> +void intel_fbdev_setup(struct drm_i915_private *dev_priv);
>  {
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index be804fd4f79f..0b6722bcfbb2 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -820,6 +820,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	i915->do_release = true;
>  
> +	intel_fbdev_setup(i915);
> +

Maybe we could call this from intel_display_driver_register()?

We're trying to separate display related stuff from the rest, and call
as few display functions from top level driver code as
possible. (Clearly, we're nowhere near this goal yet.)


>  	return 0;
>  
>  out_cleanup_gem:

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Implement fbdev emulation as in-kernel client

[Jani Nikula]

On Tue, 28 Mar 2023, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Replace all code that initializes or releases fbdev emulation
> throughout the driver. Instead initialize the fbdev client by a
> single call to i915_fbdev_setup() after i915 has registered its
> DRM device. Just like in most drivers, i915 fbdev emulation now
> acts like a regular DRM client.
>
> 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_dev_unregister() releases the client 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.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

I like the direction in the series, but I'm afraid I don't have the time
for proper detailed review of this right now. Just a couple of nits I
stumbled on inline.

Anyone else have a chance to get this reviewed?

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/display/intel_display.c |  30 ---
>  drivers/gpu/drm/i915/display/intel_fbdev.c   | 184 +++++++++----------
>  drivers/gpu/drm/i915/display/intel_fbdev.h   |  20 +-
>  drivers/gpu/drm/i915/i915_driver.c           |   2 +
>  4 files changed, 87 insertions(+), 149 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 430a1016e013..1bbeb7a061e3 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"
> @@ -8588,11 +8587,6 @@ int intel_modeset_init(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);
>  	intel_hpd_poll_disable(i915);
>  
> @@ -8796,9 +8790,6 @@ void intel_modeset_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 */
> @@ -8864,21 +8855,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
> -	 * fbdev->async_cookie.
> -	 */
>  	drm_kms_helper_poll_init(&i915->drm);
>  }
>  
> @@ -8887,14 +8863,8 @@ void intel_display_driver_unregister(struct drm_i915_private *i915)
>  	if (!HAS_DISPLAY(i915))
>  		return;
>  
> -	intel_fbdev_unregister(i915);
>  	intel_audio_deinit(i915);
>  
> -	/*
> -	 * After flushing the fbdev (incl. a late async config which
> -	 * will have delayed queuing of a hotplug event), then flush
> -	 * the hotplug events.
> -	 */
>  	drm_kms_helper_poll_fini(&i915->drm);
>  	drm_atomic_helper_shutdown(&i915->drm);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index bdb9e6f43602..8b618db30ee5 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>
> @@ -38,6 +37,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>
>  
> @@ -55,7 +55,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 */
> @@ -120,6 +119,26 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var,
>  	return ret;
>  }
>  
> +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);
> +}
> +
>  static const struct fb_ops intelfb_ops = {
>  	.owner = THIS_MODULE,
>  	DRM_FB_HELPER_DEFAULT_OPS,
> @@ -131,6 +150,7 @@ static const struct fb_ops intelfb_ops = {
>  	.fb_imageblit = drm_fb_helper_cfb_imageblit,
>  	.fb_pan_display = intel_fbdev_pan_display,
>  	.fb_blank = intel_fbdev_blank,
> +	.fb_destroy = intel_fbdev_fb_destroy,
>  };
>  
>  static int intelfb_alloc(struct drm_fb_helper *helper,
> @@ -198,7 +218,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);
>  	struct i915_ggtt *ggtt = to_gt(dev_priv)->ggtt;
>  	const struct i915_gtt_view view = {
>  		.type = I915_GTT_VIEW_NORMAL,
> @@ -323,7 +342,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:
> @@ -349,26 +368,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,
> @@ -532,16 +531,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.
> @@ -631,8 +620,6 @@ static void intel_fbdev_output_poll_changed(struct drm_device *dev)
>  	if (!ifbdev)
>  		return;
>  
> -	intel_fbdev_sync(ifbdev);
> -
>  	mutex_lock(&ifbdev->hpd_lock);
>  	send_hpd = !ifbdev->hpd_suspended;
>  	ifbdev->hpd_waiting = true;
> @@ -649,7 +636,6 @@ static void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
>  	if (!ifbdev)
>  		return;
>  
> -	intel_fbdev_sync(ifbdev);
>  	if (!ifbdev->vma)
>  		return;
>  
> @@ -662,7 +648,20 @@ static void 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)
>  {
> @@ -676,9 +675,36 @@ static int intel_fbdev_client_restore(struct drm_client_dev *client)
>  
>  static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
>  {
> -	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) {
> +		intel_fbdev_output_poll_changed(dev);
> +		return 0;
> +	}
> +
> +	ret = drm_fb_helper_init(dev, fb_helper);
> +	if (ret)
> +		goto err_drm_err;
> +
> +	if (!drm_drv_uses_atomic_modeset(dev))
> +		drm_helper_disable_unused_functions(dev);
> +
> +	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 radeon fbdev emulation (ret=%d)\n", ret);
                                      ^^^^^^

Hrrmmh. ;)

> +	return ret;
>  }
>  
>  static const struct drm_client_funcs intel_fbdev_client_funcs = {
> @@ -688,22 +714,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 *dev_priv)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_device *dev = &dev_priv->drm;
>  	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;
> +		return;
>  
> -	mutex_init(&ifbdev->hpd_lock);
> +	ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
> +	if (!ifbdev)
> +		return;
>  	drm_fb_helper_prepare(dev, &ifbdev->helper, 32, &intel_fb_helper_funcs);
>  
> +	dev_priv->display.fbdev.fbdev = ifbdev;
> +	INIT_WORK(&dev_priv->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
> @@ -711,68 +738,23 @@ int intel_fbdev_init(struct drm_device *dev)
>  
>  	ret = drm_client_init(dev, &ifbdev->helper.client, "i915-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);
> +	ret = intel_fbdev_client_hotplug(&ifbdev->helper.client);
>  	if (ret)
> -		goto err_drm_client_release;
> +		drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
>  
> -	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_client_release(&ifbdev->helper.client);
> +	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 8c953f102ba2..8123c813caaf 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)
> +void intel_fbdev_setup(struct drm_i915_private *dev_priv);
>  {
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index be804fd4f79f..0b6722bcfbb2 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -820,6 +820,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	i915->do_release = true;
>  
> +	intel_fbdev_setup(i915);
> +

Maybe we could call this from intel_display_driver_register()?

We're trying to separate display related stuff from the rest, and call
as few display functions from top level driver code as
possible. (Clearly, we're nowhere near this goal yet.)


>  	return 0;
>  
>  out_cleanup_gem:

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH 4/4] drm/i915: Implement fbdev emulation as in-kernel client

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 17427 bytes --]

Hi

Am 28.03.23 um 17:31 schrieb Jani Nikula:
> On Tue, 28 Mar 2023, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Replace all code that initializes or releases fbdev emulation
>> throughout the driver. Instead initialize the fbdev client by a
>> single call to i915_fbdev_setup() after i915 has registered its
>> DRM device. Just like in most drivers, i915 fbdev emulation now
>> acts like a regular DRM client.
>>
>> 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_dev_unregister() releases the client 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.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> I like the direction in the series, but I'm afraid I don't have the time
> for proper detailed review of this right now. Just a couple of nits I
> stumbled on inline.

Thanks a look for looking at it. I keep this patchset up for a little 
longer and maybe someone sends in further comments. Otherwise I'll send 
out an update and ask around on irc.

Best regards
Thomas

> 
> Anyone else have a chance to get this reviewed?
> 
> BR,
> Jani.
> 
> 
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c |  30 ---
>>   drivers/gpu/drm/i915/display/intel_fbdev.c   | 184 +++++++++----------
>>   drivers/gpu/drm/i915/display/intel_fbdev.h   |  20 +-
>>   drivers/gpu/drm/i915/i915_driver.c           |   2 +
>>   4 files changed, 87 insertions(+), 149 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 430a1016e013..1bbeb7a061e3 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"
>> @@ -8588,11 +8587,6 @@ int intel_modeset_init(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);
>>   	intel_hpd_poll_disable(i915);
>>   
>> @@ -8796,9 +8790,6 @@ void intel_modeset_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 */
>> @@ -8864,21 +8855,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
>> -	 * fbdev->async_cookie.
>> -	 */
>>   	drm_kms_helper_poll_init(&i915->drm);
>>   }
>>   
>> @@ -8887,14 +8863,8 @@ void intel_display_driver_unregister(struct drm_i915_private *i915)
>>   	if (!HAS_DISPLAY(i915))
>>   		return;
>>   
>> -	intel_fbdev_unregister(i915);
>>   	intel_audio_deinit(i915);
>>   
>> -	/*
>> -	 * After flushing the fbdev (incl. a late async config which
>> -	 * will have delayed queuing of a hotplug event), then flush
>> -	 * the hotplug events.
>> -	 */
>>   	drm_kms_helper_poll_fini(&i915->drm);
>>   	drm_atomic_helper_shutdown(&i915->drm);
>>   
>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
>> index bdb9e6f43602..8b618db30ee5 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>
>> @@ -38,6 +37,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>
>>   
>> @@ -55,7 +55,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 */
>> @@ -120,6 +119,26 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var,
>>   	return ret;
>>   }
>>   
>> +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);
>> +}
>> +
>>   static const struct fb_ops intelfb_ops = {
>>   	.owner = THIS_MODULE,
>>   	DRM_FB_HELPER_DEFAULT_OPS,
>> @@ -131,6 +150,7 @@ static const struct fb_ops intelfb_ops = {
>>   	.fb_imageblit = drm_fb_helper_cfb_imageblit,
>>   	.fb_pan_display = intel_fbdev_pan_display,
>>   	.fb_blank = intel_fbdev_blank,
>> +	.fb_destroy = intel_fbdev_fb_destroy,
>>   };
>>   
>>   static int intelfb_alloc(struct drm_fb_helper *helper,
>> @@ -198,7 +218,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);
>>   	struct i915_ggtt *ggtt = to_gt(dev_priv)->ggtt;
>>   	const struct i915_gtt_view view = {
>>   		.type = I915_GTT_VIEW_NORMAL,
>> @@ -323,7 +342,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:
>> @@ -349,26 +368,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,
>> @@ -532,16 +531,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.
>> @@ -631,8 +620,6 @@ static void intel_fbdev_output_poll_changed(struct drm_device *dev)
>>   	if (!ifbdev)
>>   		return;
>>   
>> -	intel_fbdev_sync(ifbdev);
>> -
>>   	mutex_lock(&ifbdev->hpd_lock);
>>   	send_hpd = !ifbdev->hpd_suspended;
>>   	ifbdev->hpd_waiting = true;
>> @@ -649,7 +636,6 @@ static void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
>>   	if (!ifbdev)
>>   		return;
>>   
>> -	intel_fbdev_sync(ifbdev);
>>   	if (!ifbdev->vma)
>>   		return;
>>   
>> @@ -662,7 +648,20 @@ static void 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)
>>   {
>> @@ -676,9 +675,36 @@ static int intel_fbdev_client_restore(struct drm_client_dev *client)
>>   
>>   static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
>>   {
>> -	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) {
>> +		intel_fbdev_output_poll_changed(dev);
>> +		return 0;
>> +	}
>> +
>> +	ret = drm_fb_helper_init(dev, fb_helper);
>> +	if (ret)
>> +		goto err_drm_err;
>> +
>> +	if (!drm_drv_uses_atomic_modeset(dev))
>> +		drm_helper_disable_unused_functions(dev);
>> +
>> +	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 radeon fbdev emulation (ret=%d)\n", ret);
>                                        ^^^^^^
> 
> Hrrmmh. ;)
> 
>> +	return ret;
>>   }
>>   
>>   static const struct drm_client_funcs intel_fbdev_client_funcs = {
>> @@ -688,22 +714,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 *dev_priv)
>>   {
>> -	struct drm_i915_private *dev_priv = to_i915(dev);
>> +	struct drm_device *dev = &dev_priv->drm;
>>   	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;
>> +		return;
>>   
>> -	mutex_init(&ifbdev->hpd_lock);
>> +	ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
>> +	if (!ifbdev)
>> +		return;
>>   	drm_fb_helper_prepare(dev, &ifbdev->helper, 32, &intel_fb_helper_funcs);
>>   
>> +	dev_priv->display.fbdev.fbdev = ifbdev;
>> +	INIT_WORK(&dev_priv->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
>> @@ -711,68 +738,23 @@ int intel_fbdev_init(struct drm_device *dev)
>>   
>>   	ret = drm_client_init(dev, &ifbdev->helper.client, "i915-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);
>> +	ret = intel_fbdev_client_hotplug(&ifbdev->helper.client);
>>   	if (ret)
>> -		goto err_drm_client_release;
>> +		drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
>>   
>> -	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_client_release(&ifbdev->helper.client);
>> +	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 8c953f102ba2..8123c813caaf 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)
>> +void intel_fbdev_setup(struct drm_i915_private *dev_priv);
>>   {
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>> index be804fd4f79f..0b6722bcfbb2 100644
>> --- a/drivers/gpu/drm/i915/i915_driver.c
>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>> @@ -820,6 +820,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>   
>>   	i915->do_release = true;
>>   
>> +	intel_fbdev_setup(i915);
>> +
> 
> Maybe we could call this from intel_display_driver_register()?
> 
> We're trying to separate display related stuff from the rest, and call
> as few display functions from top level driver code as
> possible. (Clearly, we're nowhere near this goal yet.)
> 
> 
>>   	return 0;
>>   
>>   out_cleanup_gem:
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Implement fbdev emulation as in-kernel client

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 17427 bytes --]

Hi

Am 28.03.23 um 17:31 schrieb Jani Nikula:
> On Tue, 28 Mar 2023, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Replace all code that initializes or releases fbdev emulation
>> throughout the driver. Instead initialize the fbdev client by a
>> single call to i915_fbdev_setup() after i915 has registered its
>> DRM device. Just like in most drivers, i915 fbdev emulation now
>> acts like a regular DRM client.
>>
>> 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_dev_unregister() releases the client 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.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> I like the direction in the series, but I'm afraid I don't have the time
> for proper detailed review of this right now. Just a couple of nits I
> stumbled on inline.

Thanks a look for looking at it. I keep this patchset up for a little 
longer and maybe someone sends in further comments. Otherwise I'll send 
out an update and ask around on irc.

Best regards
Thomas

> 
> Anyone else have a chance to get this reviewed?
> 
> BR,
> Jani.
> 
> 
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c |  30 ---
>>   drivers/gpu/drm/i915/display/intel_fbdev.c   | 184 +++++++++----------
>>   drivers/gpu/drm/i915/display/intel_fbdev.h   |  20 +-
>>   drivers/gpu/drm/i915/i915_driver.c           |   2 +
>>   4 files changed, 87 insertions(+), 149 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 430a1016e013..1bbeb7a061e3 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"
>> @@ -8588,11 +8587,6 @@ int intel_modeset_init(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);
>>   	intel_hpd_poll_disable(i915);
>>   
>> @@ -8796,9 +8790,6 @@ void intel_modeset_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 */
>> @@ -8864,21 +8855,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
>> -	 * fbdev->async_cookie.
>> -	 */
>>   	drm_kms_helper_poll_init(&i915->drm);
>>   }
>>   
>> @@ -8887,14 +8863,8 @@ void intel_display_driver_unregister(struct drm_i915_private *i915)
>>   	if (!HAS_DISPLAY(i915))
>>   		return;
>>   
>> -	intel_fbdev_unregister(i915);
>>   	intel_audio_deinit(i915);
>>   
>> -	/*
>> -	 * After flushing the fbdev (incl. a late async config which
>> -	 * will have delayed queuing of a hotplug event), then flush
>> -	 * the hotplug events.
>> -	 */
>>   	drm_kms_helper_poll_fini(&i915->drm);
>>   	drm_atomic_helper_shutdown(&i915->drm);
>>   
>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
>> index bdb9e6f43602..8b618db30ee5 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>
>> @@ -38,6 +37,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>
>>   
>> @@ -55,7 +55,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 */
>> @@ -120,6 +119,26 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var,
>>   	return ret;
>>   }
>>   
>> +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);
>> +}
>> +
>>   static const struct fb_ops intelfb_ops = {
>>   	.owner = THIS_MODULE,
>>   	DRM_FB_HELPER_DEFAULT_OPS,
>> @@ -131,6 +150,7 @@ static const struct fb_ops intelfb_ops = {
>>   	.fb_imageblit = drm_fb_helper_cfb_imageblit,
>>   	.fb_pan_display = intel_fbdev_pan_display,
>>   	.fb_blank = intel_fbdev_blank,
>> +	.fb_destroy = intel_fbdev_fb_destroy,
>>   };
>>   
>>   static int intelfb_alloc(struct drm_fb_helper *helper,
>> @@ -198,7 +218,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);
>>   	struct i915_ggtt *ggtt = to_gt(dev_priv)->ggtt;
>>   	const struct i915_gtt_view view = {
>>   		.type = I915_GTT_VIEW_NORMAL,
>> @@ -323,7 +342,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:
>> @@ -349,26 +368,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,
>> @@ -532,16 +531,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.
>> @@ -631,8 +620,6 @@ static void intel_fbdev_output_poll_changed(struct drm_device *dev)
>>   	if (!ifbdev)
>>   		return;
>>   
>> -	intel_fbdev_sync(ifbdev);
>> -
>>   	mutex_lock(&ifbdev->hpd_lock);
>>   	send_hpd = !ifbdev->hpd_suspended;
>>   	ifbdev->hpd_waiting = true;
>> @@ -649,7 +636,6 @@ static void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
>>   	if (!ifbdev)
>>   		return;
>>   
>> -	intel_fbdev_sync(ifbdev);
>>   	if (!ifbdev->vma)
>>   		return;
>>   
>> @@ -662,7 +648,20 @@ static void 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)
>>   {
>> @@ -676,9 +675,36 @@ static int intel_fbdev_client_restore(struct drm_client_dev *client)
>>   
>>   static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
>>   {
>> -	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) {
>> +		intel_fbdev_output_poll_changed(dev);
>> +		return 0;
>> +	}
>> +
>> +	ret = drm_fb_helper_init(dev, fb_helper);
>> +	if (ret)
>> +		goto err_drm_err;
>> +
>> +	if (!drm_drv_uses_atomic_modeset(dev))
>> +		drm_helper_disable_unused_functions(dev);
>> +
>> +	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 radeon fbdev emulation (ret=%d)\n", ret);
>                                        ^^^^^^
> 
> Hrrmmh. ;)
> 
>> +	return ret;
>>   }
>>   
>>   static const struct drm_client_funcs intel_fbdev_client_funcs = {
>> @@ -688,22 +714,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 *dev_priv)
>>   {
>> -	struct drm_i915_private *dev_priv = to_i915(dev);
>> +	struct drm_device *dev = &dev_priv->drm;
>>   	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;
>> +		return;
>>   
>> -	mutex_init(&ifbdev->hpd_lock);
>> +	ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
>> +	if (!ifbdev)
>> +		return;
>>   	drm_fb_helper_prepare(dev, &ifbdev->helper, 32, &intel_fb_helper_funcs);
>>   
>> +	dev_priv->display.fbdev.fbdev = ifbdev;
>> +	INIT_WORK(&dev_priv->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
>> @@ -711,68 +738,23 @@ int intel_fbdev_init(struct drm_device *dev)
>>   
>>   	ret = drm_client_init(dev, &ifbdev->helper.client, "i915-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);
>> +	ret = intel_fbdev_client_hotplug(&ifbdev->helper.client);
>>   	if (ret)
>> -		goto err_drm_client_release;
>> +		drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
>>   
>> -	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_client_release(&ifbdev->helper.client);
>> +	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 8c953f102ba2..8123c813caaf 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)
>> +void intel_fbdev_setup(struct drm_i915_private *dev_priv);
>>   {
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>> index be804fd4f79f..0b6722bcfbb2 100644
>> --- a/drivers/gpu/drm/i915/i915_driver.c
>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>> @@ -820,6 +820,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>   
>>   	i915->do_release = true;
>>   
>> +	intel_fbdev_setup(i915);
>> +
> 
> Maybe we could call this from intel_display_driver_register()?
> 
> We're trying to separate display related stuff from the rest, and call
> as few display functions from top level driver code as
> possible. (Clearly, we're nowhere near this goal yet.)
> 
> 
>>   	return 0;
>>   
>>   out_cleanup_gem:
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 4/4] drm/i915: Implement fbdev emulation as in-kernel client

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1803 bytes --]

Hi

Am 28.03.23 um 17:31 schrieb Jani Nikula:
[...]
>> +
>> +err_drm_fb_helper_fini:
>> +	drm_fb_helper_fini(fb_helper);
>> +err_drm_err:
>> +	drm_err(dev, "Failed to setup radeon fbdev emulation (ret=%d)\n", ret);
>                                        ^^^^^^
> 
> Hrrmmh. ;)

O:)

> 
>> +	return ret;
>>   }
>>   
>>   static const struct drm_client_funcs intel_fbdev_client_funcs = {
>> @@ -688,22 +714,23 @@ static const struct drm_client_funcs intel_fbdev_client_funcs = {
>>   	.hotplug	= intel_fbdev_client_hotplug,
>>   };
>>   

[...]

>> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>> index be804fd4f79f..0b6722bcfbb2 100644
>> --- a/drivers/gpu/drm/i915/i915_driver.c
>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>> @@ -820,6 +820,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>   
>>   	i915->do_release = true;
>>   
>> +	intel_fbdev_setup(i915);
>> +
> 
> Maybe we could call this from intel_display_driver_register()?

Fbdev interact with vga-switcheroo. I'd assume that the fbdev client 
setup needs to run after i915_switcheroo_register(). If 
intel_fbdev_setup() cannot go into the probe callback, it could be 
located at the end of i915_driver_register().

Best regards
Thomas

> 
> We're trying to separate display related stuff from the rest, and call
> as few display functions from top level driver code as
> possible. (Clearly, we're nowhere near this goal yet.)
> 
> 
>>   	return 0;
>>   
>>   out_cleanup_gem:
> 

-- 
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)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Implement fbdev emulation as in-kernel client

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1803 bytes --]

Hi

Am 28.03.23 um 17:31 schrieb Jani Nikula:
[...]
>> +
>> +err_drm_fb_helper_fini:
>> +	drm_fb_helper_fini(fb_helper);
>> +err_drm_err:
>> +	drm_err(dev, "Failed to setup radeon fbdev emulation (ret=%d)\n", ret);
>                                        ^^^^^^
> 
> Hrrmmh. ;)

O:)

> 
>> +	return ret;
>>   }
>>   
>>   static const struct drm_client_funcs intel_fbdev_client_funcs = {
>> @@ -688,22 +714,23 @@ static const struct drm_client_funcs intel_fbdev_client_funcs = {
>>   	.hotplug	= intel_fbdev_client_hotplug,
>>   };
>>   

[...]

>> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>> index be804fd4f79f..0b6722bcfbb2 100644
>> --- a/drivers/gpu/drm/i915/i915_driver.c
>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>> @@ -820,6 +820,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>   
>>   	i915->do_release = true;
>>   
>> +	intel_fbdev_setup(i915);
>> +
> 
> Maybe we could call this from intel_display_driver_register()?

Fbdev interact with vga-switcheroo. I'd assume that the fbdev client 
setup needs to run after i915_switcheroo_register(). If 
intel_fbdev_setup() cannot go into the probe callback, it could be 
located at the end of i915_driver_register().

Best regards
Thomas

> 
> We're trying to separate display related stuff from the rest, and call
> as few display functions from top level driver code as
> possible. (Clearly, we're nowhere near this goal yet.)
> 
> 
>>   	return 0;
>>   
>>   out_cleanup_gem:
> 

-- 
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)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Implement fbdev emulation as in-kernel client

[kernel test robot]

Hi Thomas,

I love your patch! Yet something to improve:

[auto build test ERROR on 6e5f96153989e454041848f66a5227be9bd0bbc3]

url:    https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/drm-i915-Move-fbdev-functions/20230328-191627
base:   6e5f96153989e454041848f66a5227be9bd0bbc3
patch link:    https://lore.kernel.org/r/20230328111422.23986-5-tzimmermann%40suse.de
patch subject: [Intel-gfx] [PATCH 4/4] drm/i915: Implement fbdev emulation as in-kernel client
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20230329/202303290420.6rd5p4br-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/6db03fde4587438371c6f7871675cba6389a1319
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Thomas-Zimmermann/drm-i915-Move-fbdev-functions/20230328-191627
        git checkout 6db03fde4587438371c6f7871675cba6389a1319
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303290420.6rd5p4br-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/i915_driver.c:55:
>> drivers/gpu/drm/i915/display/intel_fbdev.h:22:1: error: expected identifier or '(' before '{' token
      22 | {
         | ^


vim +22 drivers/gpu/drm/i915/display/intel_fbdev.h

6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h         Jani Nikula       2019-04-05  15  
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h         Jani Nikula       2019-04-05  16  #ifdef CONFIG_DRM_FBDEV_EMULATION
6db03fde458743 drivers/gpu/drm/i915/display/intel_fbdev.h Thomas Zimmermann 2023-03-28  17  void intel_fbdev_setup(struct drm_i915_private *dev_priv);
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h         Jani Nikula       2019-04-05  18  void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
c1c04560ac038d drivers/gpu/drm/i915/display/intel_fbdev.h Jani Nikula       2022-02-15  19  struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev);
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h         Jani Nikula       2019-04-05  20  #else
6db03fde458743 drivers/gpu/drm/i915/display/intel_fbdev.h Thomas Zimmermann 2023-03-28  21  void intel_fbdev_setup(struct drm_i915_private *dev_priv);
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h         Jani Nikula       2019-04-05 @22  {
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h         Jani Nikula       2019-04-05  23  }
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h         Jani Nikula       2019-04-05  24  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Implement fbdev emulation as in-kernel client

[kernel test robot]

Hi Thomas,

I love your patch! Yet something to improve:

[auto build test ERROR on 6e5f96153989e454041848f66a5227be9bd0bbc3]

url:    https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/drm-i915-Move-fbdev-functions/20230328-191627
base:   6e5f96153989e454041848f66a5227be9bd0bbc3
patch link:    https://lore.kernel.org/r/20230328111422.23986-5-tzimmermann%40suse.de
patch subject: [Intel-gfx] [PATCH 4/4] drm/i915: Implement fbdev emulation as in-kernel client
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20230329/202303290420.6rd5p4br-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/6db03fde4587438371c6f7871675cba6389a1319
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Thomas-Zimmermann/drm-i915-Move-fbdev-functions/20230328-191627
        git checkout 6db03fde4587438371c6f7871675cba6389a1319
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303290420.6rd5p4br-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/i915_driver.c:55:
>> drivers/gpu/drm/i915/display/intel_fbdev.h:22:1: error: expected identifier or '(' before '{' token
      22 | {
         | ^


vim +22 drivers/gpu/drm/i915/display/intel_fbdev.h

6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h         Jani Nikula       2019-04-05  15  
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h         Jani Nikula       2019-04-05  16  #ifdef CONFIG_DRM_FBDEV_EMULATION
6db03fde458743 drivers/gpu/drm/i915/display/intel_fbdev.h Thomas Zimmermann 2023-03-28  17  void intel_fbdev_setup(struct drm_i915_private *dev_priv);
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h         Jani Nikula       2019-04-05  18  void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
c1c04560ac038d drivers/gpu/drm/i915/display/intel_fbdev.h Jani Nikula       2022-02-15  19  struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev);
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h         Jani Nikula       2019-04-05  20  #else
6db03fde458743 drivers/gpu/drm/i915/display/intel_fbdev.h Thomas Zimmermann 2023-03-28  21  void intel_fbdev_setup(struct drm_i915_private *dev_priv);
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h         Jani Nikula       2019-04-05 @22  {
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h         Jani Nikula       2019-04-05  23  }
6dfccb95cf17cd drivers/gpu/drm/i915/intel_fbdev.h         Jani Nikula       2019-04-05  24  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Convert fbdev to DRM client

[Patchwork]

== Series Details ==

Series: drm/i915: Convert fbdev to DRM client
URL   : https://patchwork.freedesktop.org/series/115714/
State : warning

== Summary ==

Error: dim checkpatch failed
0eb95c1b5ac4 drm/i915: Move fbdev functions
-:119: CHECK:ALLOC_SIZEOF_STRUCT: Prefer kzalloc(sizeof(*ifbdev)...) over kzalloc(sizeof(struct intel_fbdev)...)
#119: FILE: drivers/gpu/drm/i915/display/intel_fbdev.c:668:
+	ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL);

-:120: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!ifbdev"
#120: FILE: drivers/gpu/drm/i915/display/intel_fbdev.c:669:
+	if (ifbdev == NULL)

total: 0 errors, 0 warnings, 2 checks, 172 lines checked
91a0ee910d74 drm/i915: Initialize fbdev DRM client with callback functions
cdadc97d2d4a drm/i915: Implement fbdev client callbacks
2862fcef6190 drm/i915: Implement fbdev emulation as in-kernel client

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Convert fbdev to DRM client

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 38950 bytes --]

== Series Details ==

Series: drm/i915: Convert fbdev to DRM client
URL   : https://patchwork.freedesktop.org/series/115714/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12931 -> Patchwork_115714v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_115714v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_115714v1, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/index.html

Participating hosts (36 -> 37)
------------------------------

  Additional (1): fi-kbl-soraka 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_115714v1:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_lmem_swapping@basic:
    - bat-dg1-5:          NOTRUN -> [FAIL][1] +3 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg1-5/igt@gem_lmem_swapping@basic.html
    - bat-dg1-7:          NOTRUN -> [FAIL][2] +3 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg1-7/igt@gem_lmem_swapping@basic.html
    - bat-dg2-11:         NOTRUN -> [FAIL][3] +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-11/igt@gem_lmem_swapping@basic.html

  * igt@gem_lmem_swapping@verify-random:
    - bat-dg2-9:          NOTRUN -> [FAIL][4] +3 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-9/igt@gem_lmem_swapping@verify-random.html
    - bat-dg2-8:          NOTRUN -> [FAIL][5] +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-8/igt@gem_lmem_swapping@verify-random.html

  * igt@i915_module_load@reload:
    - bat-dg1-5:          [PASS][6] -> [WARN][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-dg1-5/igt@i915_module_load@reload.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg1-5/igt@i915_module_load@reload.html
    - fi-pnv-d510:        [PASS][8] -> [WARN][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-pnv-d510/igt@i915_module_load@reload.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-pnv-d510/igt@i915_module_load@reload.html
    - bat-dg1-7:          [PASS][10] -> [WARN][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-dg1-7/igt@i915_module_load@reload.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg1-7/igt@i915_module_load@reload.html
    - bat-adlp-9:         [PASS][12] -> [WARN][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-adlp-9/igt@i915_module_load@reload.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-adlp-9/igt@i915_module_load@reload.html
    - bat-rplp-1:         [PASS][14] -> [WARN][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-rplp-1/igt@i915_module_load@reload.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-rplp-1/igt@i915_module_load@reload.html
    - fi-rkl-11600:       [PASS][16] -> [WARN][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-rkl-11600/igt@i915_module_load@reload.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-rkl-11600/igt@i915_module_load@reload.html
    - bat-adls-5:         [PASS][18] -> [WARN][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-adls-5/igt@i915_module_load@reload.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-adls-5/igt@i915_module_load@reload.html
    - bat-jsl-3:          [PASS][20] -> [WARN][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-jsl-3/igt@i915_module_load@reload.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-jsl-3/igt@i915_module_load@reload.html
    - bat-adln-1:         [PASS][22] -> [WARN][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-adln-1/igt@i915_module_load@reload.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-adln-1/igt@i915_module_load@reload.html
    - bat-rpls-2:         [PASS][24] -> [WARN][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-rpls-2/igt@i915_module_load@reload.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-rpls-2/igt@i915_module_load@reload.html
    - fi-elk-e7500:       [PASS][26] -> [WARN][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-elk-e7500/igt@i915_module_load@reload.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-elk-e7500/igt@i915_module_load@reload.html
    - bat-jsl-1:          [PASS][28] -> [WARN][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-jsl-1/igt@i915_module_load@reload.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-jsl-1/igt@i915_module_load@reload.html
    - bat-rpls-1:         [PASS][30] -> [WARN][31]
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-rpls-1/igt@i915_module_load@reload.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-rpls-1/igt@i915_module_load@reload.html

  * igt@i915_pm_rpm@module-reload:
    - fi-bsw-nick:        [PASS][32] -> [FAIL][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-bsw-nick/igt@i915_pm_rpm@module-reload.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-bsw-nick/igt@i915_pm_rpm@module-reload.html
    - bat-rplp-1:         [PASS][34] -> [FAIL][35] +2 similar issues
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-rplp-1/igt@i915_pm_rpm@module-reload.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-rplp-1/igt@i915_pm_rpm@module-reload.html
    - fi-apl-guc:         [PASS][36] -> [FAIL][37]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-apl-guc/igt@i915_pm_rpm@module-reload.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-apl-guc/igt@i915_pm_rpm@module-reload.html
    - bat-dg2-9:          [PASS][38] -> [FAIL][39]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-dg2-9/igt@i915_pm_rpm@module-reload.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-9/igt@i915_pm_rpm@module-reload.html
    - bat-dg2-11:         [PASS][40] -> [FAIL][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-dg2-11/igt@i915_pm_rpm@module-reload.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-11/igt@i915_pm_rpm@module-reload.html
    - fi-kbl-x1275:       [PASS][42] -> [FAIL][43]
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
    - fi-kbl-soraka:      NOTRUN -> [FAIL][44]
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-soraka/igt@i915_pm_rpm@module-reload.html
    - fi-hsw-4770:        [PASS][45] -> [FAIL][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-hsw-4770/igt@i915_pm_rpm@module-reload.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-hsw-4770/igt@i915_pm_rpm@module-reload.html
    - fi-cfl-8109u:       [PASS][47] -> [FAIL][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html
    - fi-kbl-7567u:       [PASS][49] -> [FAIL][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-kbl-7567u/igt@i915_pm_rpm@module-reload.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-7567u/igt@i915_pm_rpm@module-reload.html
    - fi-kbl-8809g:       [PASS][51] -> [FAIL][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-kbl-8809g/igt@i915_pm_rpm@module-reload.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-8809g/igt@i915_pm_rpm@module-reload.html
    - bat-dg2-8:          [PASS][53] -> [FAIL][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-dg2-8/igt@i915_pm_rpm@module-reload.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-8/igt@i915_pm_rpm@module-reload.html
    - fi-kbl-guc:         [PASS][55] -> [FAIL][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html
    - bat-jsl-1:          [PASS][57] -> [FAIL][58] +2 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-jsl-1/igt@i915_pm_rpm@module-reload.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-jsl-1/igt@i915_pm_rpm@module-reload.html
    - fi-tgl-1115g4:      [PASS][59] -> [FAIL][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-tgl-1115g4/igt@i915_pm_rpm@module-reload.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-tgl-1115g4/igt@i915_pm_rpm@module-reload.html
    - fi-bsw-n3050:       [PASS][61] -> [FAIL][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-bsw-n3050/igt@i915_pm_rpm@module-reload.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-bsw-n3050/igt@i915_pm_rpm@module-reload.html
    - bat-rpls-1:         [PASS][63] -> [FAIL][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-rpls-1/igt@i915_pm_rpm@module-reload.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-rpls-1/igt@i915_pm_rpm@module-reload.html
    - fi-cfl-guc:         [PASS][65] -> [FAIL][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-cfl-guc/igt@i915_pm_rpm@module-reload.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-cfl-guc/igt@i915_pm_rpm@module-reload.html
    - fi-skl-6600u:       [PASS][67] -> [FAIL][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-skl-6600u/igt@i915_pm_rpm@module-reload.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-skl-6600u/igt@i915_pm_rpm@module-reload.html
    - bat-rpls-2:         [PASS][69] -> [FAIL][70]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-rpls-2/igt@i915_pm_rpm@module-reload.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-rpls-2/igt@i915_pm_rpm@module-reload.html
    - bat-adlp-9:         [PASS][71] -> [FAIL][72] +1 similar issue
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-adlp-9/igt@i915_pm_rpm@module-reload.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-adlp-9/igt@i915_pm_rpm@module-reload.html
    - fi-skl-guc:         NOTRUN -> [FAIL][73]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-skl-guc/igt@i915_pm_rpm@module-reload.html
    - fi-cfl-8700k:       [PASS][74] -> [FAIL][75]
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-cfl-8700k/igt@i915_pm_rpm@module-reload.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-cfl-8700k/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live:
    - bat-adln-1:         NOTRUN -> [SKIP][76]
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-adln-1/igt@i915_selftest@live.html
    - bat-rpls-2:         NOTRUN -> [SKIP][77]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-rpls-2/igt@i915_selftest@live.html
    - bat-dg2-8:          NOTRUN -> [SKIP][78]
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-8/igt@i915_selftest@live.html
    - bat-jsl-1:          NOTRUN -> [SKIP][79]
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-jsl-1/igt@i915_selftest@live.html
    - bat-rpls-1:         NOTRUN -> [SKIP][80]
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-rpls-1/igt@i915_selftest@live.html
    - bat-dg1-5:          NOTRUN -> [SKIP][81]
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg1-5/igt@i915_selftest@live.html
    - bat-dg1-7:          NOTRUN -> [SKIP][82]
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg1-7/igt@i915_selftest@live.html
    - bat-adlp-9:         NOTRUN -> [SKIP][83]
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-adlp-9/igt@i915_selftest@live.html
    - bat-dg2-11:         NOTRUN -> [SKIP][84]
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-11/igt@i915_selftest@live.html
    - bat-rplp-1:         NOTRUN -> [SKIP][85]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-rplp-1/igt@i915_selftest@live.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][86]
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-rkl-11600/igt@i915_selftest@live.html
    - bat-adls-5:         NOTRUN -> [SKIP][87]
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-adls-5/igt@i915_selftest@live.html
    - bat-jsl-3:          NOTRUN -> [SKIP][88]
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-jsl-3/igt@i915_selftest@live.html
    - bat-dg2-9:          NOTRUN -> [SKIP][89]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-9/igt@i915_selftest@live.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - bat-adln-1:         [PASS][90] -> [FAIL][91] +2 similar issues
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-adln-1/igt@i915_suspend@basic-s2idle-without-i915.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-adln-1/igt@i915_suspend@basic-s2idle-without-i915.html
    - fi-elk-e7500:       [PASS][92] -> [FAIL][93] +1 similar issue
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-elk-e7500/igt@i915_suspend@basic-s2idle-without-i915.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-elk-e7500/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-rkl-11600:       [PASS][94] -> [FAIL][95] +2 similar issues
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html
    - bat-adls-5:         [PASS][96] -> [FAIL][97] +2 similar issues
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-adls-5/igt@i915_suspend@basic-s3-without-i915.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-adls-5/igt@i915_suspend@basic-s3-without-i915.html
    - bat-dg1-5:          [PASS][98] -> [FAIL][99] +2 similar issues
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-dg1-5/igt@i915_suspend@basic-s3-without-i915.html
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg1-5/igt@i915_suspend@basic-s3-without-i915.html
    - fi-pnv-d510:        [PASS][100] -> [FAIL][101] +1 similar issue
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-pnv-d510/igt@i915_suspend@basic-s3-without-i915.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-pnv-d510/igt@i915_suspend@basic-s3-without-i915.html
    - bat-dg1-7:          [PASS][102] -> [FAIL][103] +2 similar issues
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-dg1-7/igt@i915_suspend@basic-s3-without-i915.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg1-7/igt@i915_suspend@basic-s3-without-i915.html
    - bat-jsl-3:          [PASS][104] -> [FAIL][105] +2 similar issues
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-jsl-3/igt@i915_suspend@basic-s3-without-i915.html
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-jsl-3/igt@i915_suspend@basic-s3-without-i915.html
    - fi-glk-j4005:       [PASS][106] -> [FAIL][107] +2 similar issues
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-glk-j4005/igt@i915_suspend@basic-s3-without-i915.html
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-glk-j4005/igt@i915_suspend@basic-s3-without-i915.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_pm_rpm@module-reload:
    - {bat-kbl-2}:        [PASS][108] -> [FAIL][109]
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-kbl-2/igt@i915_pm_rpm@module-reload.html
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-kbl-2/igt@i915_pm_rpm@module-reload.html

  
Known issues
------------

  Here are the changes found in Patchwork_115714v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@dmabuf@all-tests@dma_fence_chain:
    - fi-kbl-soraka:      NOTRUN -> [ABORT][110] ([i915#7830] / [i915#8129])
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-soraka/igt@dmabuf@all-tests@dma_fence_chain.html

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][111] ([fdo#109271] / [i915#2190])
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-skl-guc:         NOTRUN -> [SKIP][112] ([fdo#109271] / [i915#4613]) +3 similar issues
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-skl-guc/igt@gem_lmem_swapping@basic.html
    - fi-kbl-soraka:      NOTRUN -> [SKIP][113] ([fdo#109271] / [i915#4613]) +3 similar issues
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@i915_module_load@reload:
    - fi-skl-6600u:       [PASS][114] -> [WARN][115] ([i915#6596])
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-skl-6600u/igt@i915_module_load@reload.html
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-skl-6600u/igt@i915_module_load@reload.html
    - fi-apl-guc:         [PASS][116] -> [WARN][117] ([i915#6596])
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-apl-guc/igt@i915_module_load@reload.html
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-apl-guc/igt@i915_module_load@reload.html
    - fi-glk-j4005:       [PASS][118] -> [WARN][119] ([i915#6596])
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-glk-j4005/igt@i915_module_load@reload.html
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-glk-j4005/igt@i915_module_load@reload.html
    - fi-skl-guc:         NOTRUN -> [WARN][120] ([i915#6596])
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-skl-guc/igt@i915_module_load@reload.html
    - bat-dg2-11:         [PASS][121] -> [WARN][122] ([i915#6596])
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-dg2-11/igt@i915_module_load@reload.html
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-11/igt@i915_module_load@reload.html
    - fi-kbl-soraka:      NOTRUN -> [WARN][123] ([i915#6596])
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-soraka/igt@i915_module_load@reload.html
    - fi-kbl-7567u:       [PASS][124] -> [WARN][125] ([i915#6596])
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-kbl-7567u/igt@i915_module_load@reload.html
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-7567u/igt@i915_module_load@reload.html
    - fi-cfl-8700k:       [PASS][126] -> [WARN][127] ([i915#6596])
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-cfl-8700k/igt@i915_module_load@reload.html
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-cfl-8700k/igt@i915_module_load@reload.html
    - fi-bsw-nick:        [PASS][128] -> [WARN][129] ([i915#6596])
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-bsw-nick/igt@i915_module_load@reload.html
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-bsw-nick/igt@i915_module_load@reload.html
    - fi-cfl-guc:         [PASS][130] -> [WARN][131] ([i915#6596])
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-cfl-guc/igt@i915_module_load@reload.html
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-cfl-guc/igt@i915_module_load@reload.html
    - bat-dg2-9:          [PASS][132] -> [WARN][133] ([i915#6596])
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-dg2-9/igt@i915_module_load@reload.html
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-9/igt@i915_module_load@reload.html
    - fi-kbl-x1275:       [PASS][134] -> [WARN][135] ([i915#6596])
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-kbl-x1275/igt@i915_module_load@reload.html
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-x1275/igt@i915_module_load@reload.html
    - fi-hsw-4770:        [PASS][136] -> [WARN][137] ([i915#6596])
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-hsw-4770/igt@i915_module_load@reload.html
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-hsw-4770/igt@i915_module_load@reload.html
    - fi-cfl-8109u:       [PASS][138] -> [WARN][139] ([i915#6596])
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-cfl-8109u/igt@i915_module_load@reload.html
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-cfl-8109u/igt@i915_module_load@reload.html
    - fi-kbl-8809g:       [PASS][140] -> [WARN][141] ([i915#6596])
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-kbl-8809g/igt@i915_module_load@reload.html
   [141]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-8809g/igt@i915_module_load@reload.html
    - fi-ivb-3770:        [PASS][142] -> [WARN][143] ([i915#6596])
   [142]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-ivb-3770/igt@i915_module_load@reload.html
   [143]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-ivb-3770/igt@i915_module_load@reload.html
    - bat-dg2-8:          [PASS][144] -> [WARN][145] ([i915#6596])
   [144]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-dg2-8/igt@i915_module_load@reload.html
   [145]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-8/igt@i915_module_load@reload.html
    - fi-kbl-guc:         [PASS][146] -> [WARN][147] ([i915#6596])
   [146]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-kbl-guc/igt@i915_module_load@reload.html
   [147]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-guc/igt@i915_module_load@reload.html
    - fi-ilk-650:         [PASS][148] -> [WARN][149] ([i915#6596])
   [148]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-ilk-650/igt@i915_module_load@reload.html
   [149]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-ilk-650/igt@i915_module_load@reload.html
    - fi-tgl-1115g4:      [PASS][150] -> [WARN][151] ([i915#6596])
   [150]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-tgl-1115g4/igt@i915_module_load@reload.html
   [151]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-tgl-1115g4/igt@i915_module_load@reload.html
    - fi-bsw-n3050:       [PASS][152] -> [WARN][153] ([i915#6596])
   [152]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-bsw-n3050/igt@i915_module_load@reload.html
   [153]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-bsw-n3050/igt@i915_module_load@reload.html

  * igt@i915_selftest@live:
    - fi-kbl-x1275:       NOTRUN -> [SKIP][154] ([fdo#109271])
   [154]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-x1275/igt@i915_selftest@live.html
    - fi-hsw-4770:        NOTRUN -> [SKIP][155] ([fdo#109271])
   [155]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-hsw-4770/igt@i915_selftest@live.html
    - fi-cfl-8109u:       NOTRUN -> [SKIP][156] ([fdo#109271])
   [156]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-cfl-8109u/igt@i915_selftest@live.html
    - fi-kbl-8809g:       NOTRUN -> [SKIP][157] ([fdo#109271])
   [157]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-8809g/igt@i915_selftest@live.html
    - fi-ivb-3770:        NOTRUN -> [SKIP][158] ([fdo#109271])
   [158]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-ivb-3770/igt@i915_selftest@live.html
    - fi-elk-e7500:       NOTRUN -> [SKIP][159] ([fdo#109271])
   [159]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-elk-e7500/igt@i915_selftest@live.html
    - fi-kbl-guc:         NOTRUN -> [SKIP][160] ([fdo#109271])
   [160]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-guc/igt@i915_selftest@live.html
    - fi-ilk-650:         NOTRUN -> [SKIP][161] ([fdo#109271])
   [161]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-ilk-650/igt@i915_selftest@live.html
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][162] ([i915#1245])
   [162]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-tgl-1115g4/igt@i915_selftest@live.html
    - fi-bsw-n3050:       NOTRUN -> [SKIP][163] ([fdo#109271])
   [163]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-bsw-n3050/igt@i915_selftest@live.html
    - fi-skl-6600u:       NOTRUN -> [SKIP][164] ([fdo#109271])
   [164]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-skl-6600u/igt@i915_selftest@live.html
    - fi-apl-guc:         NOTRUN -> [SKIP][165] ([fdo#109271])
   [165]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-apl-guc/igt@i915_selftest@live.html
    - fi-pnv-d510:        NOTRUN -> [SKIP][166] ([fdo#109271])
   [166]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-pnv-d510/igt@i915_selftest@live.html
    - fi-glk-j4005:       NOTRUN -> [SKIP][167] ([fdo#109271])
   [167]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-glk-j4005/igt@i915_selftest@live.html
    - fi-kbl-7567u:       NOTRUN -> [SKIP][168] ([fdo#109271])
   [168]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-7567u/igt@i915_selftest@live.html
    - fi-cfl-8700k:       NOTRUN -> [SKIP][169] ([fdo#109271])
   [169]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-cfl-8700k/igt@i915_selftest@live.html
    - fi-bsw-nick:        NOTRUN -> [SKIP][170] ([fdo#109271])
   [170]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-bsw-nick/igt@i915_selftest@live.html
    - fi-cfl-guc:         NOTRUN -> [SKIP][171] ([fdo#109271])
   [171]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-cfl-guc/igt@i915_selftest@live.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - fi-cfl-8109u:       [PASS][172] -> [FAIL][173] ([i915#6559]) +1 similar issue
   [172]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-cfl-8109u/igt@i915_suspend@basic-s2idle-without-i915.html
   [173]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-cfl-8109u/igt@i915_suspend@basic-s2idle-without-i915.html
    - fi-kbl-7567u:       [PASS][174] -> [FAIL][175] ([i915#6559]) +1 similar issue
   [174]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-kbl-7567u/igt@i915_suspend@basic-s2idle-without-i915.html
   [175]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-7567u/igt@i915_suspend@basic-s2idle-without-i915.html
    - fi-cfl-8700k:       [PASS][176] -> [FAIL][177] ([i915#6559]) +1 similar issue
   [176]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-cfl-8700k/igt@i915_suspend@basic-s2idle-without-i915.html
   [177]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-cfl-8700k/igt@i915_suspend@basic-s2idle-without-i915.html
    - fi-kbl-8809g:       [PASS][178] -> [FAIL][179] ([i915#6559]) +1 similar issue
   [178]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-kbl-8809g/igt@i915_suspend@basic-s2idle-without-i915.html
   [179]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-8809g/igt@i915_suspend@basic-s2idle-without-i915.html
    - fi-ivb-3770:        [PASS][180] -> [FAIL][181] ([i915#6559]) +1 similar issue
   [180]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-ivb-3770/igt@i915_suspend@basic-s2idle-without-i915.html
   [181]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-ivb-3770/igt@i915_suspend@basic-s2idle-without-i915.html
    - bat-dg2-8:          [PASS][182] -> [FAIL][183] ([i915#6559])
   [182]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-dg2-8/igt@i915_suspend@basic-s2idle-without-i915.html
   [183]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-8/igt@i915_suspend@basic-s2idle-without-i915.html
    - fi-bsw-nick:        [PASS][184] -> [FAIL][185] ([i915#6559]) +1 similar issue
   [184]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-bsw-nick/igt@i915_suspend@basic-s2idle-without-i915.html
   [185]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-bsw-nick/igt@i915_suspend@basic-s2idle-without-i915.html
    - fi-tgl-1115g4:      [PASS][186] -> [FAIL][187] ([i915#6559])
   [186]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-tgl-1115g4/igt@i915_suspend@basic-s2idle-without-i915.html
   [187]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-tgl-1115g4/igt@i915_suspend@basic-s2idle-without-i915.html
    - fi-cfl-guc:         [PASS][188] -> [FAIL][189] ([i915#6559]) +1 similar issue
   [188]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-cfl-guc/igt@i915_suspend@basic-s2idle-without-i915.html
   [189]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-cfl-guc/igt@i915_suspend@basic-s2idle-without-i915.html
    - bat-dg2-9:          [PASS][190] -> [FAIL][191] ([i915#6559]) +1 similar issue
   [190]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-dg2-9/igt@i915_suspend@basic-s2idle-without-i915.html
   [191]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-9/igt@i915_suspend@basic-s2idle-without-i915.html
    - fi-kbl-x1275:       [PASS][192] -> [FAIL][193] ([i915#6559])
   [192]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-kbl-x1275/igt@i915_suspend@basic-s2idle-without-i915.html
   [193]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-x1275/igt@i915_suspend@basic-s2idle-without-i915.html
    - fi-hsw-4770:        [PASS][194] -> [FAIL][195] ([i915#6559]) +1 similar issue
   [194]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-hsw-4770/igt@i915_suspend@basic-s2idle-without-i915.html
   [195]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-hsw-4770/igt@i915_suspend@basic-s2idle-without-i915.html
    - bat-rpls-2:         [PASS][196] -> [FAIL][197] ([i915#6559]) +1 similar issue
   [196]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-rpls-2/igt@i915_suspend@basic-s2idle-without-i915.html
   [197]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-rpls-2/igt@i915_suspend@basic-s2idle-without-i915.html
    - fi-kbl-guc:         [PASS][198] -> [FAIL][199] ([i915#6559]) +1 similar issue
   [198]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-kbl-guc/igt@i915_suspend@basic-s2idle-without-i915.html
   [199]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-guc/igt@i915_suspend@basic-s2idle-without-i915.html
    - fi-ilk-650:         [PASS][200] -> [FAIL][201] ([i915#6559]) +1 similar issue
   [200]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-ilk-650/igt@i915_suspend@basic-s2idle-without-i915.html
   [201]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-ilk-650/igt@i915_suspend@basic-s2idle-without-i915.html
    - fi-bsw-n3050:       [PASS][202] -> [FAIL][203] ([i915#6559]) +1 similar issue
   [202]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-bsw-n3050/igt@i915_suspend@basic-s2idle-without-i915.html
   [203]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-bsw-n3050/igt@i915_suspend@basic-s2idle-without-i915.html
    - bat-rpls-1:         [PASS][204] -> [FAIL][205] ([i915#6559]) +1 similar issue
   [204]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-rpls-1/igt@i915_suspend@basic-s2idle-without-i915.html
   [205]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-rpls-1/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-skl-6600u:       [PASS][206] -> [FAIL][207] ([i915#6559]) +1 similar issue
   [206]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-skl-6600u/igt@i915_suspend@basic-s3-without-i915.html
   [207]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-skl-6600u/igt@i915_suspend@basic-s3-without-i915.html
    - fi-apl-guc:         [PASS][208] -> [FAIL][209] ([i915#6559]) +1 similar issue
   [208]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-apl-guc/igt@i915_suspend@basic-s3-without-i915.html
   [209]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-apl-guc/igt@i915_suspend@basic-s3-without-i915.html
    - bat-adlp-9:         [PASS][210] -> [FAIL][211] ([i915#6559])
   [210]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-adlp-9/igt@i915_suspend@basic-s3-without-i915.html
   [211]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-adlp-9/igt@i915_suspend@basic-s3-without-i915.html
    - fi-skl-guc:         NOTRUN -> [FAIL][212] ([i915#6559]) +1 similar issue
   [212]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-skl-guc/igt@i915_suspend@basic-s3-without-i915.html
    - bat-dg2-11:         [PASS][213] -> [FAIL][214] ([i915#6559]) +1 similar issue
   [213]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-dg2-11/igt@i915_suspend@basic-s3-without-i915.html
   [214]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-11/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_chamelium_frames@hdmi-crc-fast:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][215] ([fdo#109271]) +17 similar issues
   [215]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-soraka/igt@kms_chamelium_frames@hdmi-crc-fast.html

  * igt@kms_chamelium_hpd@common-hpd-after-suspend:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][216] ([i915#7828])
   [216]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-tgl-1115g4/igt@kms_chamelium_hpd@common-hpd-after-suspend.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
    - bat-dg2-11:         NOTRUN -> [SKIP][217] ([i915#5354]) +1 similar issue
   [217]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-skl-guc:         NOTRUN -> [SKIP][218] ([fdo#109271]) +20 similar issues
   [218]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-skl-guc/igt@kms_setmode@basic-clone-single-crtc.html

  
#### Possible fixes ####

  * igt@i915_module_load@load:
    - fi-skl-guc:         [ABORT][219] -> [PASS][220]
   [219]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-skl-guc/igt@i915_module_load@load.html
   [220]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-skl-guc/igt@i915_module_load@load.html

  
#### Warnings ####

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-tgl-1115g4:      [INCOMPLETE][221] ([i915#7443]) -> [FAIL][222] ([i915#6559])
   [221]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-tgl-1115g4/igt@i915_suspend@basic-s3-without-i915.html
   [222]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-tgl-1115g4/igt@i915_suspend@basic-s3-without-i915.html
    - fi-kbl-x1275:       [SKIP][223] ([fdo#109271]) -> [FAIL][224] ([i915#6559])
   [223]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/fi-kbl-x1275/igt@i915_suspend@basic-s3-without-i915.html
   [224]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/fi-kbl-x1275/igt@i915_suspend@basic-s3-without-i915.html
    - bat-dg2-8:          [SKIP][225] ([i915#6645]) -> [FAIL][226] ([i915#6559])
   [225]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12931/bat-dg2-8/igt@i915_suspend@basic-s3-without-i915.html
   [226]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/bat-dg2-8/igt@i915_suspend@basic-s3-without-i915.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1245]: https://gitlab.freedesktop.org/drm/intel/issues/1245
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6559]: https://gitlab.freedesktop.org/drm/intel/issues/6559
  [i915#6596]: https://gitlab.freedesktop.org/drm/intel/issues/6596
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
  [i915#7443]: https://gitlab.freedesktop.org/drm/intel/issues/7443
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7830]: https://gitlab.freedesktop.org/drm/intel/issues/7830
  [i915#8129]: https://gitlab.freedesktop.org/drm/intel/issues/8129


Build changes
-------------

  * Linux: CI_DRM_12931 -> Patchwork_115714v1

  CI-20190529: 20190529
  CI_DRM_12931: e1b8055e62c6f94ef94db3e7f125704ac0fab0b5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7223: 2cbfa210fc95d126edf9a60ae6ab4e96cf4fca7f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_115714v1: e1b8055e62c6f94ef94db3e7f125704ac0fab0b5 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

4b3b7e91e591 drm/i915: Implement fbdev emulation as in-kernel client
c3a0d642b8a4 drm/i915: Implement fbdev client callbacks
462bf201a9e5 drm/i915: Initialize fbdev DRM client with callback functions
c067f21a9905 drm/i915: Move fbdev functions

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115714v1/index.html

[-- Attachment #2: Type: text/html, Size: 45917 bytes --]