Re: [PATCH v3] drm/fb-helper: Detect when lid is closed during initialization

Thomas Zimmermann
2024-06-14T08:16:02Z

Hi Mario

Am 13.06.24 um 07:17 schrieb Mario Limonciello:
> If the lid on a laptop is closed when eDP connectors are populated
> then it remains enabled when the initial framebuffer configuration
> is built.
>
> When creating the initial framebuffer configuration detect the
> lid status and if it's closed disable any eDP connectors.
>
> Also set up a workqueue to monitor for any future lid events.

After reading through this patchset, I think fbdev emulation is not the
right place for this code, as lid state is global.

You could put this into drm_client_modeset.c and track lid state per
client. drm_fb_helper_lid_work() would call the client's hotplug
callback. But preferable, lid state should be tracked per DRM device in
struct drm_mode_config and call drm_client_dev_hotplug() on each
lid-state event. Thoughts? Best regards Thomas
>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Reported-by: Chris Bainbridge <chris.bainbridge@gmail.com>
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v2->v3:
> * Use input device instead of ACPI device
> * Detect lid open/close events
> ---
> drivers/gpu/drm/drm_client_modeset.c | 29 ++++++
> drivers/gpu/drm/drm_fb_helper.c | 132 +++++++++++++++++++++++++++
> include/drm/drm_device.h | 6 ++
> include/drm/drm_fb_helper.h | 2 +
> 4 files changed, 169 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 31af5cf37a09..b8adfe87334b 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -257,6 +257,34 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors,
> enabled[i] = drm_connector_enabled(connectors[i], false);
> }
>
> +static void drm_client_match_edp_lid(struct drm_device *dev,
> + struct drm_connector **connectors,
> + unsigned int connector_count,
> + bool *enabled)
> +{
> + int i;
> +
> + for (i = 0; i < connector_count; i++) {
> + struct drm_connector *connector = connectors[i];
> +
> + switch (connector->connector_type) {
> + case DRM_MODE_CONNECTOR_LVDS:
> + case DRM_MODE_CONNECTOR_eDP:
> + if (!enabled[i])
> + continue;
> + break;
> + default:
> + continue;
> + }
> +
> + if (dev->lid_closed) {
> + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
> + connector->base.id, connector->name);
> + enabled[i] = false;
> + }
> + }
> +}
> +
> static bool drm_client_target_cloned(struct drm_device *dev,
> struct drm_connector **connectors,
> unsigned int connector_count,
> @@ -844,6 +872,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
> memset(crtcs, 0, connector_count * sizeof(*crtcs));
> memset(offsets, 0, connector_count * sizeof(*offsets));
>
> + drm_client_match_edp_lid(dev, connectors, connector_count, enabled);
> if (!drm_client_target_cloned(dev, connectors, connector_count, modes,
> offsets, enabled, width, height) &&
> !drm_client_target_preferred(dev, connectors, connector_count, modes,
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index d612133e2cf7..41dd5887599a 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -30,6 +30,8 @@
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> #include <linux/console.h>
> +#include <linux/input.h>
> +#include <linux/mod_devicetable.h>
> #include <linux/pci.h>
> #include <linux/sysrq.h>
> #include <linux/vga_switcheroo.h>
> @@ -413,6 +415,128 @@ static void drm_fb_helper_damage_work(struct work_struct *work)
> drm_fb_helper_fb_dirty(helper);
> }
>
> +static void drm_fb_helper_lid_event(struct input_handle *handle, unsigned int type,
> + unsigned int code, int value)
> +{
> + if (type == EV_SW && code == SW_LID) {
> + struct drm_fb_helper *fb_helper = handle->handler->private;
> +
> + if (value != fb_helper->dev->lid_closed) {
> + fb_helper->dev->lid_closed = value;
> + queue_work(fb_helper->input_wq, &fb_helper->lid_work);
> + }
> + }
> +}
> +
> +struct drm_fb_lid {
> + struct input_handle handle;
> +};
> +
> +static int drm_fb_helper_lid_connect(struct input_handler *handler,
> + struct input_dev *dev,
> + const struct input_device_id *id)
> +{
> + struct drm_fb_helper *fb_helper = handler->private;
> + struct drm_fb_lid *lid;
> + char *name;
> + int error;
> +
> + lid = kzalloc(sizeof(*lid), GFP_KERNEL);
> + if (!lid)
> + return -ENOMEM;
> +
> + name = kasprintf(GFP_KERNEL, "drm-fb-helper-lid-%s", dev_name(&dev->dev));
> + if (!name) {
> + error = -ENOMEM;
> + goto err_free_lid;
> + }
> +
> + lid->handle.dev = dev;
> + lid->handle.handler = handler;
> + lid->handle.name = name;
> + lid->handle.private = lid;
> +
> + error = input_register_handle(&lid->handle);
> + if (error)
> + goto err_free_name;
> +
> + error = input_open_device(&lid->handle);
> + if (error)
> + goto err_unregister_handle;
> +
> + fb_helper->dev->lid_closed = dev->sw[SW_LID];
> + drm_dbg_kms(fb_helper->dev, "initial lid state is set to %d\n", fb_helper->dev->lid_closed);
> +
> + return 0;
> +
> +err_unregister_handle:
> + input_unregister_handle(&lid->handle);
> +err_free_name:
> + kfree(name);
> +err_free_lid:
> + kfree(lid);
> + return error;
> +}
> +
> +static void drm_fb_helper_lid_disconnect(struct input_handle *handle)
> +{
> + struct drm_fb_lid *lid = handle->private;
> +
> + input_close_device(handle);
> + input_unregister_handle(handle);
> +
> + kfree(handle->name);
> + kfree(lid);
> +}
> +
> +static const struct input_device_id drm_fb_helper_lid_ids[] = {
> + {
> + .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_SWBIT,
> + .evbit = { BIT_MASK(EV_SW) },
> + .swbit = { [BIT_WORD(SW_LID)] = BIT_MASK(SW_LID) },
> + },
> + { },
> +};
> +
> +static struct input_handler drm_fb_helper_lid_handler = {
> + .event = drm_fb_helper_lid_event,
> + .connect = drm_fb_helper_lid_connect,
> + .disconnect = drm_fb_helper_lid_disconnect,
> + .name = "drm-fb-helper-lid",
> + .id_table = drm_fb_helper_lid_ids,
> +};
> +
> +static void drm_fb_helper_lid_work(struct work_struct *work)
> +{
> + struct drm_fb_helper *fb_helper = container_of(work, struct drm_fb_helper,
> + lid_work);
> + drm_fb_helper_hotplug_event(fb_helper);
> +}
> +
> +static int drm_fb_helper_create_lid_handler(struct drm_fb_helper *fb_helper)
> +{
> + int ret = 0;
> +
> + if (fb_helper->deferred_setup)
> + return 0;
> +
> + fb_helper->input_wq = create_singlethread_workqueue("drm-fb-lid");
> + if (fb_helper->input_wq == NULL)
> + return -ENOMEM;
> +
> + drm_fb_helper_lid_handler.private = fb_helper;
> + ret = input_register_handler(&drm_fb_helper_lid_handler);
> + if (ret)
> + goto remove_wq;
> +
> + return 0;
> +
> +remove_wq:
> + destroy_workqueue(fb_helper->input_wq);
> + fb_helper->input_wq = NULL;
> + return ret;
> +}
> +
> /**
> * drm_fb_helper_prepare - setup a drm_fb_helper structure
> * @dev: DRM device
> @@ -445,6 +569,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
> spin_lock_init(&helper->damage_lock);
> INIT_WORK(&helper->resume_work, drm_fb_helper_resume_worker);
> INIT_WORK(&helper->damage_work, drm_fb_helper_damage_work);
> + INIT_WORK(&helper->lid_work, drm_fb_helper_lid_work);
> helper->damage_clip.x1 = helper->damage_clip.y1 = ~0;
> mutex_init(&helper->lock);
> helper->funcs = funcs;
> @@ -593,6 +718,9 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
> if (!drm_fbdev_emulation)
> return;
>
> + input_unregister_handler(&drm_fb_helper_lid_handler);
> + destroy_workqueue(fb_helper->input_wq);
> +
> cancel_work_sync(&fb_helper->resume_work);
> cancel_work_sync(&fb_helper->damage_work);
>
> @@ -1842,6 +1970,10 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper)
> width = dev->mode_config.max_width;
> height = dev->mode_config.max_height;
>
> + ret = drm_fb_helper_create_lid_handler(fb_helper);
> + if (ret)
> + return ret;
> +
> drm_client_modeset_probe(&fb_helper->client, width, height);
> ret = drm_fb_helper_single_fb_probe(fb_helper);
> if (ret < 0) {
> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
> index 63767cf24371..619af597784c 100644
> --- a/include/drm/drm_device.h
> +++ b/include/drm/drm_device.h
> @@ -316,6 +316,12 @@ struct drm_device {
> * Root directory for debugfs files.
> */
> struct dentry *debugfs_root;
> +
> + /**
> + * @lid_closed: Flag to tell the lid switch state
> + */
> + bool lid_closed;
> +
> };
>
> #endif
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 375737fd6c36..7fb36c10299d 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -143,6 +143,8 @@ struct drm_fb_helper {
> spinlock_t damage_lock;
> struct work_struct damage_work;
> struct work_struct resume_work;
> + struct work_struct lid_work;
> + struct workqueue_struct *input_wq;
>
> /**
> * @lock:

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