Message-ID: <f5485db8-9e1d-4b95-a0ec-25ee8551795d@suse.de>
Date: Fri, 14 Jun 2024 10:15:53 +0200
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Mario Limonciello <mario.limonciello@....com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>
Cc: David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
 "open list:DRM DRIVERS" <dri-devel@...ts.freedesktop.org>,
 open list <linux-kernel@...r.kernel.org>, amd-gfx@...ts.freedesktop.org,
 Dmitry Torokhov <dmitry.torokhov@...il.com>,
 Chris Bainbridge <chris.bainbridge@...il.com>
Subject: Re: [PATCH v3] drm/fb-helper: Detect when lid is closed during
 initialization

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@...il.com>
> Reported-by: Chris Bainbridge <chris.bainbridge@...il.com>
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
> Signed-off-by: Mario Limonciello <mario.limonciello@....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)

Powered by blists - more mailing lists