[PATCH v2 0/3] drm/client: Wire up sysrq for all clients and update drm_log

[PATCH v2 0/3] drm/client: Wire up sysrq for all clients and update drm_log

[Thomas Zimmermann]

DRM's fbdev emulation has long supported SysRq+v to bring up the
framebuffer console for emergency output. Wire up sysrq for all
DRM clients and make it work with drm_log.

Patch 1 and 2 set up DRM client functionality for sysrq. The patches
adopt existing conventions from fbdev emulation, so that there's no
visible change to users. Invoke SysRq+v to bring up the in-kernel DRM
client.

Patch 3 adds restore functionality to drm_log. This enables SysRq, but
also brings back drm_log when user space releases control of the display.

Tested on amdgpu and bochs.

v2:
- fix placeholder functions
- fix grammar in commit descriptions

Thomas Zimmermann (3):
  drm/client: Pass force parameter to client restore
  drm/client: Support emergency restore via sysrq for all clients
  drm/client: log: Implement struct drm_client_funcs.restore

 drivers/gpu/drm/Makefile                   |  3 +-
 drivers/gpu/drm/clients/drm_fbdev_client.c |  6 +-
 drivers/gpu/drm/clients/drm_log.c          | 13 ++++
 drivers/gpu/drm/drm_client.c               |  1 +
 drivers/gpu/drm/drm_client_event.c         |  4 +-
 drivers/gpu/drm/drm_client_sysrq.c         | 65 ++++++++++++++++++++
 drivers/gpu/drm/drm_drv.c                  |  3 +
 drivers/gpu/drm/drm_fb_helper.c            | 69 +++-------------------
 drivers/gpu/drm/drm_file.c                 |  2 +-
 drivers/gpu/drm/drm_internal.h             | 11 ++++
 include/drm/drm_client.h                   |  8 ++-
 include/drm/drm_client_event.h             |  4 +-
 include/drm/drm_device.h                   |  8 +++
 include/drm/drm_fb_helper.h                |  8 +--
 14 files changed, 126 insertions(+), 79 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_client_sysrq.c

-- 
2.51.1

[PATCH v2 1/3] drm/client: Pass force parameter to client restore

[Thomas Zimmermann]

Add force parameter to client restore and pass value through the
layers. The only currently used value is false.

If force is true, the client should restore its display even if it
does not hold the DRM master lock. This is be required for emergency
output, such as sysrq.

While at it, inline drm_fb_helper_lastclose(), which is a trivial
wrapper around drm_fb_helper_restore_fbdev_mode_unlocked().

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/clients/drm_fbdev_client.c |  6 ++++--
 drivers/gpu/drm/drm_client_event.c         |  4 ++--
 drivers/gpu/drm/drm_fb_helper.c            | 24 ++++++----------------
 drivers/gpu/drm/drm_file.c                 |  2 +-
 include/drm/drm_client.h                   |  8 +++++---
 include/drm/drm_client_event.h             |  4 ++--
 include/drm/drm_fb_helper.h                |  8 ++------
 7 files changed, 22 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/clients/drm_fbdev_client.c b/drivers/gpu/drm/clients/drm_fbdev_client.c
index 47e5f27eee58..28951e392482 100644
--- a/drivers/gpu/drm/clients/drm_fbdev_client.c
+++ b/drivers/gpu/drm/clients/drm_fbdev_client.c
@@ -38,9 +38,11 @@ static void drm_fbdev_client_unregister(struct drm_client_dev *client)
 	}
 }
 
-static int drm_fbdev_client_restore(struct drm_client_dev *client)
+static int drm_fbdev_client_restore(struct drm_client_dev *client, bool force)
 {
-	drm_fb_helper_lastclose(client->dev);
+	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
+
+	drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper, force);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/drm_client_event.c b/drivers/gpu/drm/drm_client_event.c
index d25dc5250983..7b3e362f7926 100644
--- a/drivers/gpu/drm/drm_client_event.c
+++ b/drivers/gpu/drm/drm_client_event.c
@@ -102,7 +102,7 @@ void drm_client_dev_hotplug(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_client_dev_hotplug);
 
-void drm_client_dev_restore(struct drm_device *dev)
+void drm_client_dev_restore(struct drm_device *dev, bool force)
 {
 	struct drm_client_dev *client;
 	int ret;
@@ -115,7 +115,7 @@ void drm_client_dev_restore(struct drm_device *dev)
 		if (!client->funcs || !client->funcs->restore)
 			continue;
 
-		ret = client->funcs->restore(client);
+		ret = client->funcs->restore(client, force);
 		drm_dbg_kms(dev, "%s: ret=%d\n", client->name, ret);
 		if (!ret) /* The first one to return zero gets the privilege to restore */
 			break;
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 53e9dc0543de..1392738ce2fe 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -255,6 +255,7 @@ __drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,
 /**
  * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
  * @fb_helper: driver-allocated fbdev helper, can be NULL
+ * @force: ignore present DRM master
  *
  * This helper should be called from fbdev emulation's &drm_client_funcs.restore
  * callback. It ensures that the user isn't greeted with a black screen when the
@@ -263,9 +264,9 @@ __drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,
  * Returns:
  * 0 on success, or a negative errno code otherwise.
  */
-int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
+int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper, bool force)
 {
-	return __drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper, false);
+	return __drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper, force);
 }
 EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
 
@@ -1328,9 +1329,9 @@ int drm_fb_helper_set_par(struct fb_info *info)
 	 * the KDSET IOCTL with KD_TEXT, and only after that drops the master
 	 * status when exiting.
 	 *
-	 * In the past this was caught by drm_fb_helper_lastclose(), but on
-	 * modern systems where logind always keeps a drm fd open to orchestrate
-	 * the vt switching, this doesn't work.
+	 * In the past this was caught by drm_fb_helper_restore_fbdev_mode_unlocked(),
+	 * but on modern systems where logind always keeps a drm fd open to
+	 * orchestrate the vt switching, this doesn't work.
 	 *
 	 * To not break the userspace ABI we have this special case here, which
 	 * is only used for the above case. Everything else uses the normal
@@ -1955,16 +1956,3 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 	return 0;
 }
 EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
-
-/**
- * drm_fb_helper_lastclose - DRM driver lastclose helper for fbdev emulation
- * @dev: DRM device
- *
- * This function is obsolete. Call drm_fb_helper_restore_fbdev_mode_unlocked()
- * instead.
- */
-void drm_fb_helper_lastclose(struct drm_device *dev)
-{
-	drm_fb_helper_restore_fbdev_mode_unlocked(dev->fb_helper);
-}
-EXPORT_SYMBOL(drm_fb_helper_lastclose);
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index eebd1a05ee97..be5e617ceb9f 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -405,7 +405,7 @@ EXPORT_SYMBOL(drm_open);
 
 static void drm_lastclose(struct drm_device *dev)
 {
-	drm_client_dev_restore(dev);
+	drm_client_dev_restore(dev, false);
 
 	if (dev_is_pci(dev->dev))
 		vga_switcheroo_process_delayed_switch();
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 5ecde0f6f591..c972a8a3385b 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -57,12 +57,14 @@ struct drm_client_funcs {
 	 *
 	 * Note that the core does not guarantee exclusion against concurrent
 	 * drm_open(). Clients need to ensure this themselves, for example by
-	 * using drm_master_internal_acquire() and
-	 * drm_master_internal_release().
+	 * using drm_master_internal_acquire() and drm_master_internal_release().
+	 *
+	 * If the caller passes force, the client should ignore any present DRM
+	 * master and restore the display anyway.
 	 *
 	 * This callback is optional.
 	 */
-	int (*restore)(struct drm_client_dev *client);
+	int (*restore)(struct drm_client_dev *client, bool force);
 
 	/**
 	 * @hotplug:
diff --git a/include/drm/drm_client_event.h b/include/drm/drm_client_event.h
index 985d6f02a4c4..79369c755bc9 100644
--- a/include/drm/drm_client_event.h
+++ b/include/drm/drm_client_event.h
@@ -10,7 +10,7 @@ struct drm_device;
 #if defined(CONFIG_DRM_CLIENT)
 void drm_client_dev_unregister(struct drm_device *dev);
 void drm_client_dev_hotplug(struct drm_device *dev);
-void drm_client_dev_restore(struct drm_device *dev);
+void drm_client_dev_restore(struct drm_device *dev, bool force);
 void drm_client_dev_suspend(struct drm_device *dev);
 void drm_client_dev_resume(struct drm_device *dev);
 #else
@@ -18,7 +18,7 @@ static inline void drm_client_dev_unregister(struct drm_device *dev)
 { }
 static inline void drm_client_dev_hotplug(struct drm_device *dev)
 { }
-static inline void drm_client_dev_restore(struct drm_device *dev)
+static inline void drm_client_dev_restore(struct drm_device *dev, bool force)
 { }
 static inline void drm_client_dev_suspend(struct drm_device *dev)
 { }
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index c1d38d54a112..63e3af8dd5ed 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -254,7 +254,8 @@ int drm_fb_helper_set_par(struct fb_info *info);
 int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
 			    struct fb_info *info);
 
-int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper);
+int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,
+					      bool force);
 
 struct fb_info *drm_fb_helper_alloc_info(struct drm_fb_helper *fb_helper);
 void drm_fb_helper_release_info(struct drm_fb_helper *fb_helper);
@@ -283,7 +284,6 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper);
 int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper);
 int drm_fb_helper_debug_enter(struct fb_info *info);
 int drm_fb_helper_debug_leave(struct fb_info *info);
-void drm_fb_helper_lastclose(struct drm_device *dev);
 #else
 static inline void drm_fb_helper_prepare(struct drm_device *dev,
 					 struct drm_fb_helper *helper,
@@ -409,10 +409,6 @@ static inline int drm_fb_helper_debug_leave(struct fb_info *info)
 {
 	return 0;
 }
-
-static inline void drm_fb_helper_lastclose(struct drm_device *dev)
-{
-}
 #endif
 
 #endif
-- 
2.51.1

[PATCH v2 2/3] drm/client: Support emergency restore via sysrq for all clients

[Thomas Zimmermann]

Move the sysrq functionality from DRM fbdev helpers to the DRM device
and in-kernel clients, so that it becomes available on all clients.

DRM fbdev helpers support emergency restoration of the console output
via a special key combination. Press SysRq+v to replace the current
compositor with the kernel's output on the framebuffer console. This
allows users to see the log messages during system emergencies.

By moving the functionality from fbdev helpers to the DRM device, any
in-kernel client can serve as emergency output. This can be used to
bring up drm_log, for example.

Each DRM device registers itself to the list of possible sysrq handlers.
On receiving SysRq+v, the DRM core goes over all registered devices and
restores an in-kernel DRM client for each of them.

See Documentation/admin-guide/sysrq.rst on how to invoke SysRq. Switch
VTs to bring back the user-space compositor.

v2:
- declare placeholders as 'static inline' (kernel test robot)
- fix grammar in commit description

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/Makefile           |  3 +-
 drivers/gpu/drm/drm_client.c       |  1 +
 drivers/gpu/drm/drm_client_sysrq.c | 65 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_drv.c          |  3 ++
 drivers/gpu/drm/drm_fb_helper.c    | 45 +--------------------
 drivers/gpu/drm/drm_internal.h     | 11 +++++
 include/drm/drm_device.h           |  8 ++++
 7 files changed, 91 insertions(+), 45 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_client_sysrq.c

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 7789f42027ff..33ff76ae52b2 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -76,7 +76,8 @@ drm-y := \
 drm-$(CONFIG_DRM_CLIENT) += \
 	drm_client.o \
 	drm_client_event.o \
-	drm_client_modeset.o
+	drm_client_modeset.o \
+	drm_client_sysrq.o
 drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_PANEL) += drm_panel.o
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 504ec5bdfa2c..a82d741e6630 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -11,6 +11,7 @@
 #include <linux/slab.h>
 
 #include <drm/drm_client.h>
+#include <drm/drm_client_event.h>
 #include <drm/drm_device.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_file.h>
diff --git a/drivers/gpu/drm/drm_client_sysrq.c b/drivers/gpu/drm/drm_client_sysrq.c
new file mode 100644
index 000000000000..eea660096f1b
--- /dev/null
+++ b/drivers/gpu/drm/drm_client_sysrq.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0 or MIT
+
+#include <linux/sysrq.h>
+
+#include <drm/drm_client_event.h>
+#include <drm/drm_device.h>
+#include <drm/drm_print.h>
+
+#include "drm_internal.h"
+
+#ifdef CONFIG_MAGIC_SYSRQ
+static LIST_HEAD(drm_client_sysrq_dev_list);
+static DEFINE_MUTEX(drm_client_sysrq_dev_lock);
+
+/* emergency restore, don't bother with error reporting */
+static void drm_client_sysrq_restore_work_fn(struct work_struct *ignored)
+{
+	struct drm_device *dev;
+
+	guard(mutex)(&drm_client_sysrq_dev_lock);
+
+	list_for_each_entry(dev, &drm_client_sysrq_dev_list, client_sysrq_list) {
+		if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+			continue;
+
+		drm_client_dev_restore(dev, true);
+	}
+}
+
+static DECLARE_WORK(drm_client_sysrq_restore_work, drm_client_sysrq_restore_work_fn);
+
+static void drm_client_sysrq_restore_handler(u8 ignored)
+{
+	schedule_work(&drm_client_sysrq_restore_work);
+}
+
+static const struct sysrq_key_op drm_client_sysrq_restore_op = {
+	.handler = drm_client_sysrq_restore_handler,
+	.help_msg = "force-fb(v)",
+	.action_msg = "Restore framebuffer console",
+};
+
+void drm_client_sysrq_register(struct drm_device *dev)
+{
+	guard(mutex)(&drm_client_sysrq_dev_lock);
+
+	if (list_empty(&drm_client_sysrq_dev_list))
+		register_sysrq_key('v', &drm_client_sysrq_restore_op);
+
+	list_add(&dev->client_sysrq_list, &drm_client_sysrq_dev_list);
+}
+
+void drm_client_sysrq_unregister(struct drm_device *dev)
+{
+	guard(mutex)(&drm_client_sysrq_dev_lock);
+
+	/* remove device from global restore list */
+	if (!drm_WARN_ON(dev, list_empty(&dev->client_sysrq_list)))
+		list_del(&dev->client_sysrq_list);
+
+	/* no devices left; unregister key */
+	if (list_empty(&drm_client_sysrq_dev_list))
+		unregister_sysrq_key('v', &drm_client_sysrq_restore_op);
+}
+#endif
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 8e3cb08241c8..2915118436ce 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -733,6 +733,7 @@ static int drm_dev_init(struct drm_device *dev,
 	INIT_LIST_HEAD(&dev->filelist);
 	INIT_LIST_HEAD(&dev->filelist_internal);
 	INIT_LIST_HEAD(&dev->clientlist);
+	INIT_LIST_HEAD(&dev->client_sysrq_list);
 	INIT_LIST_HEAD(&dev->vblank_event_list);
 
 	spin_lock_init(&dev->event_lock);
@@ -1100,6 +1101,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 			goto err_unload;
 	}
 	drm_panic_register(dev);
+	drm_client_sysrq_register(dev);
 
 	DRM_INFO("Initialized %s %d.%d.%d for %s on minor %d\n",
 		 driver->name, driver->major, driver->minor,
@@ -1144,6 +1146,7 @@ void drm_dev_unregister(struct drm_device *dev)
 {
 	dev->registered = false;
 
+	drm_client_sysrq_unregister(dev);
 	drm_panic_unregister(dev);
 
 	drm_client_dev_unregister(dev);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 1392738ce2fe..9a734017756b 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -32,7 +32,6 @@
 #include <linux/console.h>
 #include <linux/export.h>
 #include <linux/pci.h>
-#include <linux/sysrq.h>
 #include <linux/vga_switcheroo.h>
 
 #include <drm/drm_atomic.h>
@@ -270,42 +269,6 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper, b
 }
 EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
 
-#ifdef CONFIG_MAGIC_SYSRQ
-/* emergency restore, don't bother with error reporting */
-static void drm_fb_helper_restore_work_fn(struct work_struct *ignored)
-{
-	struct drm_fb_helper *helper;
-
-	mutex_lock(&kernel_fb_helper_lock);
-	list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
-		struct drm_device *dev = helper->dev;
-
-		if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
-			continue;
-
-		mutex_lock(&helper->lock);
-		drm_client_modeset_commit_locked(&helper->client);
-		mutex_unlock(&helper->lock);
-	}
-	mutex_unlock(&kernel_fb_helper_lock);
-}
-
-static DECLARE_WORK(drm_fb_helper_restore_work, drm_fb_helper_restore_work_fn);
-
-static void drm_fb_helper_sysrq(u8 dummy1)
-{
-	schedule_work(&drm_fb_helper_restore_work);
-}
-
-static const struct sysrq_key_op sysrq_drm_fb_helper_restore_op = {
-	.handler = drm_fb_helper_sysrq,
-	.help_msg = "force-fb(v)",
-	.action_msg = "Restore framebuffer console",
-};
-#else
-static const struct sysrq_key_op sysrq_drm_fb_helper_restore_op = { };
-#endif
-
 static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
 {
 	struct drm_fb_helper *fb_helper = info->par;
@@ -602,11 +565,8 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
 	drm_fb_helper_release_info(fb_helper);
 
 	mutex_lock(&kernel_fb_helper_lock);
-	if (!list_empty(&fb_helper->kernel_fb_list)) {
+	if (!list_empty(&fb_helper->kernel_fb_list))
 		list_del(&fb_helper->kernel_fb_list);
-		if (list_empty(&kernel_fb_helper_list))
-			unregister_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
-	}
 	mutex_unlock(&kernel_fb_helper_lock);
 
 	if (!fb_helper->client.funcs)
@@ -1840,9 +1800,6 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper)
 		 info->node, info->fix.id);
 
 	mutex_lock(&kernel_fb_helper_lock);
-	if (list_empty(&kernel_fb_helper_list))
-		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
-
 	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
 	mutex_unlock(&kernel_fb_helper_lock);
 
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 5a3bed48ab1f..f893b1e3a596 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -56,6 +56,17 @@ static inline void drm_client_debugfs_init(struct drm_device *dev)
 { }
 #endif
 
+/* drm_client_sysrq.c */
+#if defined(CONFIG_DRM_CLIENT) && defined(CONFIG_MAGIC_SYSRQ)
+void drm_client_sysrq_register(struct drm_device *dev);
+void drm_client_sysrq_unregister(struct drm_device *dev);
+#else
+static inline void drm_client_sysrq_register(struct drm_device *dev)
+{ }
+static inline void drm_client_sysrq_unregister(struct drm_device *dev)
+{ }
+#endif
+
 /* drm_file.c */
 extern struct mutex drm_global_mutex;
 bool drm_dev_needs_global_mutex(struct drm_device *dev);
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 778b2cca6c49..5af49c5c3778 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -238,6 +238,14 @@ struct drm_device {
 	 */
 	struct list_head clientlist;
 
+	/**
+	 * @client_sysrq_list:
+	 *
+	 * Entry into list of devices registered for sysrq. Allows in-kernel
+	 * clients on this device to handle sysrq keys.
+	 */
+	struct list_head client_sysrq_list;
+
 	/**
 	 * @vblank_disable_immediate:
 	 *
-- 
2.51.1

Re: [PATCH v2 2/3] drm/client: Support emergency restore via sysrq for all clients

[Jocelyn Falempe]

On 10/11/2025 16:44, Thomas Zimmermann wrote:
> Move the sysrq functionality from DRM fbdev helpers to the DRM device
> and in-kernel clients, so that it becomes available on all clients.
> 
> DRM fbdev helpers support emergency restoration of the console output
> via a special key combination. Press SysRq+v to replace the current
> compositor with the kernel's output on the framebuffer console. This
> allows users to see the log messages during system emergencies.
> 
> By moving the functionality from fbdev helpers to the DRM device, any
> in-kernel client can serve as emergency output. This can be used to
> bring up drm_log, for example.
> 
> Each DRM device registers itself to the list of possible sysrq handlers.
> On receiving SysRq+v, the DRM core goes over all registered devices and
> restores an in-kernel DRM client for each of them.
> 
> See Documentation/admin-guide/sysrq.rst on how to invoke SysRq. Switch
> VTs to bring back the user-space compositor.
> 

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

> v2:
> - declare placeholders as 'static inline' (kernel test robot)
> - fix grammar in commit description
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/Makefile           |  3 +-
>   drivers/gpu/drm/drm_client.c       |  1 +
>   drivers/gpu/drm/drm_client_sysrq.c | 65 ++++++++++++++++++++++++++++++
>   drivers/gpu/drm/drm_drv.c          |  3 ++
>   drivers/gpu/drm/drm_fb_helper.c    | 45 +--------------------
>   drivers/gpu/drm/drm_internal.h     | 11 +++++
>   include/drm/drm_device.h           |  8 ++++
>   7 files changed, 91 insertions(+), 45 deletions(-)
>   create mode 100644 drivers/gpu/drm/drm_client_sysrq.c
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 7789f42027ff..33ff76ae52b2 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -76,7 +76,8 @@ drm-y := \
>   drm-$(CONFIG_DRM_CLIENT) += \
>   	drm_client.o \
>   	drm_client_event.o \
> -	drm_client_modeset.o
> +	drm_client_modeset.o \
> +	drm_client_sysrq.o
>   drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
>   drm-$(CONFIG_COMPAT) += drm_ioc32.o
>   drm-$(CONFIG_DRM_PANEL) += drm_panel.o
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 504ec5bdfa2c..a82d741e6630 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -11,6 +11,7 @@
>   #include <linux/slab.h>
>   
>   #include <drm/drm_client.h>
> +#include <drm/drm_client_event.h>
>   #include <drm/drm_device.h>
>   #include <drm/drm_drv.h>
>   #include <drm/drm_file.h>
> diff --git a/drivers/gpu/drm/drm_client_sysrq.c b/drivers/gpu/drm/drm_client_sysrq.c
> new file mode 100644
> index 000000000000..eea660096f1b
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_client_sysrq.c
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +
> +#include <linux/sysrq.h>
> +
> +#include <drm/drm_client_event.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_print.h>
> +
> +#include "drm_internal.h"
> +
> +#ifdef CONFIG_MAGIC_SYSRQ
> +static LIST_HEAD(drm_client_sysrq_dev_list);
> +static DEFINE_MUTEX(drm_client_sysrq_dev_lock);
> +
> +/* emergency restore, don't bother with error reporting */
> +static void drm_client_sysrq_restore_work_fn(struct work_struct *ignored)
> +{
> +	struct drm_device *dev;
> +
> +	guard(mutex)(&drm_client_sysrq_dev_lock);
> +
> +	list_for_each_entry(dev, &drm_client_sysrq_dev_list, client_sysrq_list) {
> +		if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> +			continue;
> +
> +		drm_client_dev_restore(dev, true);
> +	}
> +}
> +
> +static DECLARE_WORK(drm_client_sysrq_restore_work, drm_client_sysrq_restore_work_fn);
> +
> +static void drm_client_sysrq_restore_handler(u8 ignored)
> +{
> +	schedule_work(&drm_client_sysrq_restore_work);
> +}
> +
> +static const struct sysrq_key_op drm_client_sysrq_restore_op = {
> +	.handler = drm_client_sysrq_restore_handler,
> +	.help_msg = "force-fb(v)",
> +	.action_msg = "Restore framebuffer console",
> +};
> +
> +void drm_client_sysrq_register(struct drm_device *dev)
> +{
> +	guard(mutex)(&drm_client_sysrq_dev_lock);
> +
> +	if (list_empty(&drm_client_sysrq_dev_list))
> +		register_sysrq_key('v', &drm_client_sysrq_restore_op);
> +
> +	list_add(&dev->client_sysrq_list, &drm_client_sysrq_dev_list);
> +}
> +
> +void drm_client_sysrq_unregister(struct drm_device *dev)
> +{
> +	guard(mutex)(&drm_client_sysrq_dev_lock);
> +
> +	/* remove device from global restore list */
> +	if (!drm_WARN_ON(dev, list_empty(&dev->client_sysrq_list)))
> +		list_del(&dev->client_sysrq_list);
> +
> +	/* no devices left; unregister key */
> +	if (list_empty(&drm_client_sysrq_dev_list))
> +		unregister_sysrq_key('v', &drm_client_sysrq_restore_op);
> +}
> +#endif
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 8e3cb08241c8..2915118436ce 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -733,6 +733,7 @@ static int drm_dev_init(struct drm_device *dev,
>   	INIT_LIST_HEAD(&dev->filelist);
>   	INIT_LIST_HEAD(&dev->filelist_internal);
>   	INIT_LIST_HEAD(&dev->clientlist);
> +	INIT_LIST_HEAD(&dev->client_sysrq_list);
>   	INIT_LIST_HEAD(&dev->vblank_event_list);
>   
>   	spin_lock_init(&dev->event_lock);
> @@ -1100,6 +1101,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>   			goto err_unload;
>   	}
>   	drm_panic_register(dev);
> +	drm_client_sysrq_register(dev);
>   
>   	DRM_INFO("Initialized %s %d.%d.%d for %s on minor %d\n",
>   		 driver->name, driver->major, driver->minor,
> @@ -1144,6 +1146,7 @@ void drm_dev_unregister(struct drm_device *dev)
>   {
>   	dev->registered = false;
>   
> +	drm_client_sysrq_unregister(dev);
>   	drm_panic_unregister(dev);
>   
>   	drm_client_dev_unregister(dev);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 1392738ce2fe..9a734017756b 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -32,7 +32,6 @@
>   #include <linux/console.h>
>   #include <linux/export.h>
>   #include <linux/pci.h>
> -#include <linux/sysrq.h>
>   #include <linux/vga_switcheroo.h>
>   
>   #include <drm/drm_atomic.h>
> @@ -270,42 +269,6 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper, b
>   }
>   EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
>   
> -#ifdef CONFIG_MAGIC_SYSRQ
> -/* emergency restore, don't bother with error reporting */
> -static void drm_fb_helper_restore_work_fn(struct work_struct *ignored)
> -{
> -	struct drm_fb_helper *helper;
> -
> -	mutex_lock(&kernel_fb_helper_lock);
> -	list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
> -		struct drm_device *dev = helper->dev;
> -
> -		if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> -			continue;
> -
> -		mutex_lock(&helper->lock);
> -		drm_client_modeset_commit_locked(&helper->client);
> -		mutex_unlock(&helper->lock);
> -	}
> -	mutex_unlock(&kernel_fb_helper_lock);
> -}
> -
> -static DECLARE_WORK(drm_fb_helper_restore_work, drm_fb_helper_restore_work_fn);
> -
> -static void drm_fb_helper_sysrq(u8 dummy1)
> -{
> -	schedule_work(&drm_fb_helper_restore_work);
> -}
> -
> -static const struct sysrq_key_op sysrq_drm_fb_helper_restore_op = {
> -	.handler = drm_fb_helper_sysrq,
> -	.help_msg = "force-fb(v)",
> -	.action_msg = "Restore framebuffer console",
> -};
> -#else
> -static const struct sysrq_key_op sysrq_drm_fb_helper_restore_op = { };
> -#endif
> -
>   static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
>   {
>   	struct drm_fb_helper *fb_helper = info->par;
> @@ -602,11 +565,8 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
>   	drm_fb_helper_release_info(fb_helper);
>   
>   	mutex_lock(&kernel_fb_helper_lock);
> -	if (!list_empty(&fb_helper->kernel_fb_list)) {
> +	if (!list_empty(&fb_helper->kernel_fb_list))
>   		list_del(&fb_helper->kernel_fb_list);
> -		if (list_empty(&kernel_fb_helper_list))
> -			unregister_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
> -	}
>   	mutex_unlock(&kernel_fb_helper_lock);
>   
>   	if (!fb_helper->client.funcs)
> @@ -1840,9 +1800,6 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper)
>   		 info->node, info->fix.id);
>   
>   	mutex_lock(&kernel_fb_helper_lock);
> -	if (list_empty(&kernel_fb_helper_list))
> -		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
> -
>   	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
>   	mutex_unlock(&kernel_fb_helper_lock);
>   
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 5a3bed48ab1f..f893b1e3a596 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -56,6 +56,17 @@ static inline void drm_client_debugfs_init(struct drm_device *dev)
>   { }
>   #endif
>   
> +/* drm_client_sysrq.c */
> +#if defined(CONFIG_DRM_CLIENT) && defined(CONFIG_MAGIC_SYSRQ)
> +void drm_client_sysrq_register(struct drm_device *dev);
> +void drm_client_sysrq_unregister(struct drm_device *dev);
> +#else
> +static inline void drm_client_sysrq_register(struct drm_device *dev)
> +{ }
> +static inline void drm_client_sysrq_unregister(struct drm_device *dev)
> +{ }
> +#endif
> +
>   /* drm_file.c */
>   extern struct mutex drm_global_mutex;
>   bool drm_dev_needs_global_mutex(struct drm_device *dev);
> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
> index 778b2cca6c49..5af49c5c3778 100644
> --- a/include/drm/drm_device.h
> +++ b/include/drm/drm_device.h
> @@ -238,6 +238,14 @@ struct drm_device {
>   	 */
>   	struct list_head clientlist;
>   
> +	/**
> +	 * @client_sysrq_list:
> +	 *
> +	 * Entry into list of devices registered for sysrq. Allows in-kernel
> +	 * clients on this device to handle sysrq keys.
> +	 */
> +	struct list_head client_sysrq_list;
> +
>   	/**
>   	 * @vblank_disable_immediate:
>   	 *

[PATCH v2 3/3] drm/client: log: Implement struct drm_client_funcs.restore

[Thomas Zimmermann]

Restore the log client's output when the DRM core invokes the restore
callback. Follow the existing behavior of fbdev emulation wrt. the
value of the force parameter.

If force is false, acquire the DRM master lock and reprogram the
display. This is the case when the user-space compositor exits and
the DRM core transfers the display back to the in-kernel client. This
also enables drm_log output during reboot and shutdown.

If force is true, reprogram without considering the master lock. This
overrides the current compositor and prints the log to the screen. In
case of system malfunction, users can enter SysRq+v to invoke the
emergency error reporting. See Documentation/admin-guide/sysrq.rst for
more information.

v2:
- s/exists/exits/ in second paragraph of commit description
- fix grammar in commit description

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/clients/drm_log.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
index 19e55aa0ed74..4d3005273b27 100644
--- a/drivers/gpu/drm/clients/drm_log.c
+++ b/drivers/gpu/drm/clients/drm_log.c
@@ -315,6 +315,18 @@ static void drm_log_client_unregister(struct drm_client_dev *client)
 	drm_client_release(client);
 }
 
+static int drm_log_client_restore(struct drm_client_dev *client, bool force)
+{
+	int ret;
+
+	if (force)
+		ret = drm_client_modeset_commit_locked(client);
+	else
+		ret = drm_client_modeset_commit(client);
+
+	return ret;
+}
+
 static int drm_log_client_hotplug(struct drm_client_dev *client)
 {
 	struct drm_log *dlog = client_to_drm_log(client);
@@ -348,6 +360,7 @@ static const struct drm_client_funcs drm_log_client_funcs = {
 	.owner		= THIS_MODULE,
 	.free		= drm_log_client_free,
 	.unregister	= drm_log_client_unregister,
+	.restore	= drm_log_client_restore,
 	.hotplug	= drm_log_client_hotplug,
 	.suspend	= drm_log_client_suspend,
 	.resume		= drm_log_client_resume,
-- 
2.51.1