Message-ID: <3fce43d9-260a-4539-9b25-3dff42c4fdd7@redhat.com>
Date: Fri, 21 Nov 2025 10:15:13 +0100
From: Jocelyn Falempe <jfalempe@...hat.com>
To: Thomas Zimmermann <tzimmermann@...e.de>, javierm@...hat.com,
 francesco@...la.it, rrameshbabu@...dia.com, simona@...ll.ch,
 airlied@...il.com, mripard@...nel.org, maarten.lankhorst@...ux.intel.com,
 gregkh@...uxfoundation.org, jirislaby@...nel.org
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 linux-serial@...r.kernel.org
Subject: Re: [PATCH v2 2/3] drm/client: Support emergency restore via sysrq
 for all clients

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@...hat.com>

> v2:
> - declare placeholders as 'static inline' (kernel test robot)
> - fix grammar in commit description
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@...e.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:
>   	 *

Powered by blists - more mailing lists