| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ac63816c-b12a-4127-b975-ee80b53ee11f@redhat.com>
Date: Mon, 10 Nov 2025 15:02:26 +0100
From: Jocelyn Falempe <jfalempe@...hat.com>
To: Thomas Zimmermann <tzimmermann@...e.de>, javierm@...hat.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 1/3] drm/client: Pass force parameter to client restore
On 07/11/2025 15:19, Thomas Zimmermann wrote:
> 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().
Thanks for this work, it looks good to me.
Reviewed-by: Jocelyn Falempe <jfalempe@...hat.com>
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@...e.de>
> ---
> 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
Powered by blists - more mailing lists