Re: [PATCH 01/12] drm/{i915, xe}: Suspend/resume fbdev emulation via client interfaces

Thomas Zimmermann <tzimmermann@xxxxxxx> · Wed, 12 Feb 2025 08:43:39 +0100

Hi

Am 11.02.25 um 21:55 schrieb Rodrigo Vivi:
On Thu, Dec 12, 2024 at 06:08:42PM +0100, Thomas Zimmermann wrote:
Implement drm_client_dev_suspend() and drm_client_dev_resume() for
i915's fbdev emulation and call the helper via DRM client interfaces.
This is required to convert i915 and xe to DRM's generic fbdev client.
No functional changes.
Well, there is one functional change that is the clientlist iterator
and the clientlist_mutex hold. So I would avoid the statement of
no functional change. But indeed it seems there's not relevant
functional changes...

Fair point. I'll reword this.


the code itself looks right to me:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>

Thanks

Best regards
Thomas


Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
---
  drivers/gpu/drm/i915/display/intel_fbdev.c | 16 ++++++++++++++++
  drivers/gpu/drm/i915/i915_driver.c         |  9 +++++----
  drivers/gpu/drm/xe/display/xe_display.c    |  9 +++++----
  3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 00852ff5b247..a51d1dfd1b58 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -642,11 +642,27 @@ static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
  	return ret;
  }
  
+static int intel_fbdev_client_suspend(struct drm_client_dev *client, bool holds_console_lock)
+{
+	intel_fbdev_set_suspend(client->dev, FBINFO_STATE_SUSPENDED, true);
+
+	return 0;
+}
+
+static int intel_fbdev_client_resume(struct drm_client_dev *client, bool holds_console_lock)
+{
+	intel_fbdev_set_suspend(client->dev, FBINFO_STATE_RUNNING, false);
+
+	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,
+	.suspend	= intel_fbdev_client_suspend,
+	.resume		= intel_fbdev_client_resume,
  };
  
  void intel_fbdev_setup(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index eb3fcc9e77a5..e385e4947a91 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -41,6 +41,8 @@
  #include <linux/vt.h>
  
  #include <drm/drm_atomic_helper.h>
+#include <drm/drm_client.h>
+#include <drm/drm_client_event.h>
  #include <drm/drm_ioctl.h>
  #include <drm/drm_managed.h>
  #include <drm/drm_probe_helper.h>
@@ -55,7 +57,6 @@
  #include "display/intel_dp.h"
  #include "display/intel_dpt.h"
  #include "display/intel_encoder.h"
-#include "display/intel_fbdev.h"
  #include "display/intel_hotplug.h"
  #include "display/intel_overlay.h"
  #include "display/intel_pch_refclk.h"
@@ -958,7 +959,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
  	intel_runtime_pm_disable(&i915->runtime_pm);
  	intel_power_domains_disable(display);
  
-	intel_fbdev_set_suspend(&i915->drm, FBINFO_STATE_SUSPENDED, true);
+	drm_client_dev_suspend(&i915->drm, false);
  	if (HAS_DISPLAY(i915)) {
  		drm_kms_helper_poll_disable(&i915->drm);
  		intel_display_driver_disable_user_access(display);
@@ -1041,7 +1042,7 @@ static int i915_drm_suspend(struct drm_device *dev)
  	/* We do a lot of poking in a lot of registers, make sure they work
  	 * properly. */
  	intel_power_domains_disable(display);
-	intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true);
+	drm_client_dev_suspend(dev, false);
  	if (HAS_DISPLAY(dev_priv)) {
  		drm_kms_helper_poll_disable(dev);
  		intel_display_driver_disable_user_access(display);
@@ -1227,7 +1228,7 @@ static int i915_drm_resume(struct drm_device *dev)
  
  	intel_opregion_resume(display);
  
-	intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
+	drm_client_dev_resume(dev, false);
  
  	intel_power_domains_enable(display);
  
diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
index 317fa66adf18..bc73c9999c57 100644
--- a/drivers/gpu/drm/xe/display/xe_display.c
+++ b/drivers/gpu/drm/xe/display/xe_display.c
@@ -8,6 +8,8 @@
  
  #include <linux/fb.h>
  
+#include <drm/drm_client.h>
+#include <drm/drm_client_event.h>
  #include <drm/drm_drv.h>
  #include <drm/drm_managed.h>
  #include <drm/drm_probe_helper.h>
@@ -25,7 +27,6 @@
  #include "intel_dmc_wl.h"
  #include "intel_dp.h"
  #include "intel_encoder.h"
-#include "intel_fbdev.h"
  #include "intel_hdcp.h"
  #include "intel_hotplug.h"
  #include "intel_opregion.h"
@@ -336,7 +337,7 @@ static void __xe_display_pm_suspend(struct xe_device *xe, bool runtime)
  	 */
  	intel_power_domains_disable(display);
  	if (!runtime)
-		intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true);
+		drm_client_dev_suspend(&xe->drm, false);
  
  	if (!runtime && has_display(xe)) {
  		drm_kms_helper_poll_disable(&xe->drm);
@@ -374,7 +375,7 @@ void xe_display_pm_shutdown(struct xe_device *xe)
  		return;
  
  	intel_power_domains_disable(display);
-	intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true);
+	drm_client_dev_suspend(&xe->drm, false);
  	if (has_display(xe)) {
  		drm_kms_helper_poll_disable(&xe->drm);
  		intel_display_driver_disable_user_access(display);
@@ -494,7 +495,7 @@ static void __xe_display_pm_resume(struct xe_device *xe, bool runtime)
  	intel_opregion_resume(display);
  
  	if (!runtime)
-		intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_RUNNING, false);
+		drm_client_dev_resume(&xe->drm, false);
  
  	intel_power_domains_enable(display);
  }
--
2.47.1


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







