linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen
@ 2022-01-07 19:02 Rajat Jain
  2022-01-07 19:02 ` [PATCH v5 2/3] platform/chrome: Add driver for ChromeOS privacy-screen Rajat Jain
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Rajat Jain @ 2022-01-07 19:02 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Hans de Goede, David Airlie, Daniel Vetter, Benson Leung,
	Henrique de Moraes Holschuh, Mark Gross, linux-kernel, dri-devel,
	ibm-acpi-devel, platform-driver-x86, gwendal, seanpaul, marcheu,
	dtor, Dmitry Torokhov, Benson Leung
  Cc: Rajat Jain, rajatxjain

Allow a privacy screen provider to stash its private data pointer in the
drm_privacy_screen, and update the drm_privacy_screen_register() call to
accept that. Also introduce a *_get_drvdata() so that it can retrieved
back when needed.

This also touches the IBM Thinkpad platform driver, the only user of
privacy screen today, to pass NULL for now to the updated API.

Signed-off-by: Rajat Jain <rajatja@google.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---
v5: Same as v4
v4: Added "Reviewed-by" from Hans
v3: Initial version. Came up due to review comments on v2 of other patches.
v2: No v2
v1: No v1

 drivers/gpu/drm/drm_privacy_screen.c    |  5 ++++-
 drivers/platform/x86/thinkpad_acpi.c    |  2 +-
 include/drm/drm_privacy_screen_driver.h | 13 ++++++++++++-
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_privacy_screen.c b/drivers/gpu/drm/drm_privacy_screen.c
index beaf99e9120a..03b149cc455b 100644
--- a/drivers/gpu/drm/drm_privacy_screen.c
+++ b/drivers/gpu/drm/drm_privacy_screen.c
@@ -387,7 +387,8 @@ static void drm_privacy_screen_device_release(struct device *dev)
  * * An ERR_PTR(errno) on failure.
  */
 struct drm_privacy_screen *drm_privacy_screen_register(
-	struct device *parent, const struct drm_privacy_screen_ops *ops)
+	struct device *parent, const struct drm_privacy_screen_ops *ops,
+	void *data)
 {
 	struct drm_privacy_screen *priv;
 	int ret;
@@ -404,6 +405,7 @@ struct drm_privacy_screen *drm_privacy_screen_register(
 	priv->dev.parent = parent;
 	priv->dev.release = drm_privacy_screen_device_release;
 	dev_set_name(&priv->dev, "privacy_screen-%s", dev_name(parent));
+	priv->drvdata = data;
 	priv->ops = ops;
 
 	priv->ops->get_hw_state(priv);
@@ -439,6 +441,7 @@ void drm_privacy_screen_unregister(struct drm_privacy_screen *priv)
 	mutex_unlock(&drm_privacy_screen_devs_lock);
 
 	mutex_lock(&priv->lock);
+	priv->drvdata = NULL;
 	priv->ops = NULL;
 	mutex_unlock(&priv->lock);
 
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 341655d711ce..ccbfda2b0095 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9782,7 +9782,7 @@ static int tpacpi_lcdshadow_init(struct ibm_init_struct *iibm)
 		return 0;
 
 	lcdshadow_dev = drm_privacy_screen_register(&tpacpi_pdev->dev,
-						    &lcdshadow_ops);
+						    &lcdshadow_ops, NULL);
 	if (IS_ERR(lcdshadow_dev))
 		return PTR_ERR(lcdshadow_dev);
 
diff --git a/include/drm/drm_privacy_screen_driver.h b/include/drm/drm_privacy_screen_driver.h
index 24591b607675..4ef246d5706f 100644
--- a/include/drm/drm_privacy_screen_driver.h
+++ b/include/drm/drm_privacy_screen_driver.h
@@ -73,10 +73,21 @@ struct drm_privacy_screen {
 	 * for more info.
 	 */
 	enum drm_privacy_screen_status hw_state;
+	/**
+	 * @drvdata: Private data owned by the privacy screen provider
+	 */
+	void *drvdata;
 };
 
+static inline
+void *drm_privacy_screen_get_drvdata(struct drm_privacy_screen *priv)
+{
+	return priv->drvdata;
+}
+
 struct drm_privacy_screen *drm_privacy_screen_register(
-	struct device *parent, const struct drm_privacy_screen_ops *ops);
+	struct device *parent, const struct drm_privacy_screen_ops *ops,
+	void *data);
 void drm_privacy_screen_unregister(struct drm_privacy_screen *priv);
 
 void drm_privacy_screen_call_notifier_chain(struct drm_privacy_screen *priv);
-- 
2.34.1.575.g55b058a8bb-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v5 2/3] platform/chrome: Add driver for ChromeOS privacy-screen
  2022-01-07 19:02 [PATCH v5 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen Rajat Jain
@ 2022-01-07 19:02 ` Rajat Jain
  2022-01-07 19:09   ` Benson Leung
  2022-01-07 19:02 ` [PATCH v5 3/3] drm/privacy_screen_x86: Add entry " Rajat Jain
  2022-01-10 11:24 ` [PATCH v5 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen Hans de Goede
  2 siblings, 1 reply; 9+ messages in thread
From: Rajat Jain @ 2022-01-07 19:02 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Hans de Goede, David Airlie, Daniel Vetter, Benson Leung,
	Henrique de Moraes Holschuh, Mark Gross, linux-kernel, dri-devel,
	ibm-acpi-devel, platform-driver-x86, gwendal, seanpaul, marcheu,
	dtor, Dmitry Torokhov, Benson Leung
  Cc: Rajat Jain, rajatxjain

This adds the ACPI driver for the ChromeOS privacy screen that is
present on some chromeos devices.

Note that ideally, we'd want this privacy screen driver to be probed
BEFORE the drm probe in order to avoid a drm probe deferral:
https://hansdegoede.livejournal.com/25948.html

In practise, I found that ACPI drivers are bound to their devices AFTER
the drm probe on chromebooks. So on chromebooks with privacy-screen,
this patch along with the other one in this series results in a probe
deferral of about 250ms for i915 driver. However, it did not result in
any user noticeable delay of splash screen in my personal experience.

In future if this probe deferral turns out to be an issue, we can
consider turning this ACPI driver into something that is probed
earlier than the drm drivers.

Signed-off-by: Rajat Jain <rajatja@google.com>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
v5: * Add blank line, 2 more vowels to the driver name as per comments
    * Add Dmitry's "Reviewed by"
v4: Same as v3 (No changes)
v3: * Renamed everything chromeos_priv_scrn_* to chromeos_privacy_screen_*
     (and added line breaks to accommodate longer names within 80 chars)
    * Cleanup / Added few comments
    * Use the newly added drm_privacy_screen_get_drvdata()
    * Provide the cleanup function chromeos_privacy_screen_remove()
v2: * Reword the commit log
    * Make the Kconfig into a tristate
    * Reorder the patches in the series.

 drivers/platform/chrome/Kconfig               |  11 ++
 drivers/platform/chrome/Makefile              |   1 +
 .../platform/chrome/chromeos_privacy_screen.c | 153 ++++++++++++++++++
 3 files changed, 165 insertions(+)
 create mode 100644 drivers/platform/chrome/chromeos_privacy_screen.c

diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index ccc23d8686e8..75e93efd669f 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -243,6 +243,17 @@ config CROS_USBPD_NOTIFY
 	  To compile this driver as a module, choose M here: the
 	  module will be called cros_usbpd_notify.
 
+config CHROMEOS_PRIVACY_SCREEN
+	tristate "ChromeOS Privacy Screen support"
+	depends on ACPI
+	depends on DRM
+	select DRM_PRIVACY_SCREEN
+	help
+	  This driver provides the support needed for the in-built electronic
+	  privacy screen that is present on some ChromeOS devices. When enabled,
+	  this should probably always be built into the kernel to avoid or
+	  minimize drm probe deferral.
+
 source "drivers/platform/chrome/wilco_ec/Kconfig"
 
 endif # CHROMEOS_PLATFORMS
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index f901d2e43166..5d4be9735d9d 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -4,6 +4,7 @@
 CFLAGS_cros_ec_trace.o:=		-I$(src)
 
 obj-$(CONFIG_CHROMEOS_LAPTOP)		+= chromeos_laptop.o
+obj-$(CONFIG_CHROMEOS_PRIVACY_SCREEN)	+= chromeos_privacy_screen.o
 obj-$(CONFIG_CHROMEOS_PSTORE)		+= chromeos_pstore.o
 obj-$(CONFIG_CHROMEOS_TBMC)		+= chromeos_tbmc.o
 obj-$(CONFIG_CROS_EC)			+= cros_ec.o
diff --git a/drivers/platform/chrome/chromeos_privacy_screen.c b/drivers/platform/chrome/chromeos_privacy_screen.c
new file mode 100644
index 000000000000..77e9f5ee8e33
--- /dev/null
+++ b/drivers/platform/chrome/chromeos_privacy_screen.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ *  ChromeOS Privacy Screen support
+ *
+ * Copyright (C) 2022 Google LLC
+ *
+ * This is the Chromeos privacy screen provider, present on certain chromebooks,
+ * represented by a GOOG0010 device in the ACPI. This ACPI device, if present,
+ * will cause the i915 drm driver to probe defer until this driver registers
+ * the privacy-screen.
+ */
+
+#include <linux/acpi.h>
+#include <drm/drm_privacy_screen_driver.h>
+
+/*
+ * The DSM (Device Specific Method) constants below are the agreed API with
+ * the firmware team, on how to control privacy screen using ACPI methods.
+ */
+#define PRIV_SCRN_DSM_REVID		1	/* DSM version */
+#define PRIV_SCRN_DSM_FN_GET_STATUS	1	/* Get privacy screen status */
+#define PRIV_SCRN_DSM_FN_ENABLE		2	/* Enable privacy screen */
+#define PRIV_SCRN_DSM_FN_DISABLE	3	/* Disable privacy screen */
+
+static const guid_t chromeos_privacy_screen_dsm_guid =
+		    GUID_INIT(0xc7033113, 0x8720, 0x4ceb,
+			      0x90, 0x90, 0x9d, 0x52, 0xb3, 0xe5, 0x2d, 0x73);
+
+static void
+chromeos_privacy_screen_get_hw_state(struct drm_privacy_screen
+				     *drm_privacy_screen)
+{
+	union acpi_object *obj;
+	acpi_handle handle;
+	struct device *privacy_screen =
+		drm_privacy_screen_get_drvdata(drm_privacy_screen);
+
+	handle = acpi_device_handle(to_acpi_device(privacy_screen));
+	obj = acpi_evaluate_dsm(handle, &chromeos_privacy_screen_dsm_guid,
+				PRIV_SCRN_DSM_REVID,
+				PRIV_SCRN_DSM_FN_GET_STATUS, NULL);
+	if (!obj) {
+		dev_err(privacy_screen,
+			"_DSM failed to get privacy-screen state\n");
+		return;
+	}
+
+	if (obj->type != ACPI_TYPE_INTEGER)
+		dev_err(privacy_screen,
+			"Bad _DSM to get privacy-screen state\n");
+	else if (obj->integer.value == 1)
+		drm_privacy_screen->hw_state = drm_privacy_screen->sw_state =
+			PRIVACY_SCREEN_ENABLED;
+	else
+		drm_privacy_screen->hw_state = drm_privacy_screen->sw_state =
+			PRIVACY_SCREEN_DISABLED;
+
+	ACPI_FREE(obj);
+}
+
+static int
+chromeos_privacy_screen_set_sw_state(struct drm_privacy_screen
+				     *drm_privacy_screen,
+				     enum drm_privacy_screen_status state)
+{
+	union acpi_object *obj = NULL;
+	acpi_handle handle;
+	struct device *privacy_screen =
+		drm_privacy_screen_get_drvdata(drm_privacy_screen);
+
+	handle = acpi_device_handle(to_acpi_device(privacy_screen));
+
+	if (state == PRIVACY_SCREEN_DISABLED) {
+		obj = acpi_evaluate_dsm(handle,
+					&chromeos_privacy_screen_dsm_guid,
+					PRIV_SCRN_DSM_REVID,
+					PRIV_SCRN_DSM_FN_DISABLE, NULL);
+	} else if (state == PRIVACY_SCREEN_ENABLED) {
+		obj = acpi_evaluate_dsm(handle,
+					&chromeos_privacy_screen_dsm_guid,
+					PRIV_SCRN_DSM_REVID,
+					PRIV_SCRN_DSM_FN_ENABLE, NULL);
+	} else {
+		dev_err(privacy_screen,
+			"Bad attempt to set privacy-screen status to %u\n",
+			state);
+		return -EINVAL;
+	}
+
+	if (!obj) {
+		dev_err(privacy_screen,
+			"_DSM failed to set privacy-screen state\n");
+		return -EIO;
+	}
+
+	drm_privacy_screen->hw_state = drm_privacy_screen->sw_state = state;
+	ACPI_FREE(obj);
+	return 0;
+}
+
+static const struct drm_privacy_screen_ops chromeos_privacy_screen_ops = {
+	.get_hw_state = chromeos_privacy_screen_get_hw_state,
+	.set_sw_state = chromeos_privacy_screen_set_sw_state,
+};
+
+static int chromeos_privacy_screen_add(struct acpi_device *adev)
+{
+	struct drm_privacy_screen *drm_privacy_screen =
+		drm_privacy_screen_register(&adev->dev,
+					    &chromeos_privacy_screen_ops,
+					    &adev->dev);
+
+	if (IS_ERR(drm_privacy_screen)) {
+		dev_err(&adev->dev, "Error registering privacy-screen\n");
+		return PTR_ERR(drm_privacy_screen);
+	}
+
+	adev->driver_data = drm_privacy_screen;
+	dev_info(&adev->dev, "registered privacy-screen '%s'\n",
+		 dev_name(&drm_privacy_screen->dev));
+
+	return 0;
+}
+
+static int chromeos_privacy_screen_remove(struct acpi_device *adev)
+{
+	struct drm_privacy_screen *drm_privacy_screen =	acpi_driver_data(adev);
+
+	drm_privacy_screen_unregister(drm_privacy_screen);
+	return 0;
+}
+
+static const struct acpi_device_id chromeos_privacy_screen_device_ids[] = {
+	{"GOOG0010", 0}, /* Google's electronic privacy screen for eDP-1 */
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, chromeos_privacy_screen_device_ids);
+
+static struct acpi_driver chromeos_privacy_screen_driver = {
+	.name = "chromeos_privacy_screen_driver",
+	.class = "ChromeOS",
+	.ids = chromeos_privacy_screen_device_ids,
+	.ops = {
+		.add = chromeos_privacy_screen_add,
+		.remove = chromeos_privacy_screen_remove,
+	},
+};
+
+module_acpi_driver(chromeos_privacy_screen_driver);
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("ChromeOS ACPI Privacy Screen driver");
+MODULE_AUTHOR("Rajat Jain <rajatja@google.com>");
-- 
2.34.1.575.g55b058a8bb-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v5 3/3] drm/privacy_screen_x86: Add entry for ChromeOS privacy-screen
  2022-01-07 19:02 [PATCH v5 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen Rajat Jain
  2022-01-07 19:02 ` [PATCH v5 2/3] platform/chrome: Add driver for ChromeOS privacy-screen Rajat Jain
@ 2022-01-07 19:02 ` Rajat Jain
  2022-01-07 19:06   ` Benson Leung
  2022-01-10 11:24 ` [PATCH v5 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen Hans de Goede
  2 siblings, 1 reply; 9+ messages in thread
From: Rajat Jain @ 2022-01-07 19:02 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Hans de Goede, David Airlie, Daniel Vetter, Benson Leung,
	Henrique de Moraes Holschuh, Mark Gross, linux-kernel, dri-devel,
	ibm-acpi-devel, platform-driver-x86, gwendal, seanpaul, marcheu,
	dtor, Dmitry Torokhov, Benson Leung
  Cc: Rajat Jain, rajatxjain

Add a static entry in the x86 table, to detect and wait for
privacy-screen on some ChromeOS platforms.

Please note that this means that if CONFIG_CHROMEOS_PRIVACY_SCREEN is
enabled, and if "GOOG0010" device is found in ACPI, then the i915 probe
shall return EPROBE_DEFER until a platform driver actually registers the
privacy-screen: https://hansdegoede.livejournal.com/25948.html

Signed-off-by: Rajat Jain <rajatja@google.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---
v5: * Add Hans' "Reviewed by"
v4: * Simplify the detect_chromeos_privacy_screen() function
    * Don't change the existing print statement
v3: * Remove the pr_info() from detect_chromeos_privacy_screen(), instead
      enhance the one already present in drm_privacy_screen_lookup_init()
v2: * Use #if instead of #elif
    * Reorder the patches in the series.
    * Rebased on drm-tip

 drivers/gpu/drm/drm_privacy_screen_x86.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/drm_privacy_screen_x86.c b/drivers/gpu/drm/drm_privacy_screen_x86.c
index a2cafb294ca6..88802cd7a1ee 100644
--- a/drivers/gpu/drm/drm_privacy_screen_x86.c
+++ b/drivers/gpu/drm/drm_privacy_screen_x86.c
@@ -47,6 +47,13 @@ static bool __init detect_thinkpad_privacy_screen(void)
 }
 #endif
 
+#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
+static bool __init detect_chromeos_privacy_screen(void)
+{
+	return acpi_dev_present("GOOG0010", NULL, -1);
+}
+#endif
+
 static const struct arch_init_data arch_init_data[] __initconst = {
 #if IS_ENABLED(CONFIG_THINKPAD_ACPI)
 	{
@@ -58,6 +65,16 @@ static const struct arch_init_data arch_init_data[] __initconst = {
 		.detect = detect_thinkpad_privacy_screen,
 	},
 #endif
+#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
+	{
+		.lookup = {
+			.dev_id = NULL,
+			.con_id = NULL,
+			.provider = "privacy_screen-GOOG0010:00",
+		},
+		.detect = detect_chromeos_privacy_screen,
+	},
+#endif
 };
 
 void __init drm_privacy_screen_lookup_init(void)
-- 
2.34.1.575.g55b058a8bb-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 3/3] drm/privacy_screen_x86: Add entry for ChromeOS privacy-screen
  2022-01-07 19:02 ` [PATCH v5 3/3] drm/privacy_screen_x86: Add entry " Rajat Jain
@ 2022-01-07 19:06   ` Benson Leung
  2022-01-08 16:10     ` Hans de Goede
  0 siblings, 1 reply; 9+ messages in thread
From: Benson Leung @ 2022-01-07 19:06 UTC (permalink / raw)
  To: Rajat Jain, Hans de Goede
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Hans de Goede, David Airlie, Daniel Vetter, Benson Leung,
	Henrique de Moraes Holschuh, Mark Gross, linux-kernel, dri-devel,
	ibm-acpi-devel, platform-driver-x86, gwendal, seanpaul, marcheu,
	dtor, Dmitry Torokhov, rajatxjain

[-- Attachment #1: Type: text/plain, Size: 2665 bytes --]

Hi Rajat, 

Thanks for your changes here.

On Fri, Jan 07, 2022 at 11:02:08AM -0800, Rajat Jain wrote:
> Add a static entry in the x86 table, to detect and wait for
> privacy-screen on some ChromeOS platforms.
> 
> Please note that this means that if CONFIG_CHROMEOS_PRIVACY_SCREEN is
> enabled, and if "GOOG0010" device is found in ACPI, then the i915 probe
> shall return EPROBE_DEFER until a platform driver actually registers the
> privacy-screen: https://hansdegoede.livejournal.com/25948.html
> 
> Signed-off-by: Rajat Jain <rajatja@google.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Hi Hans,

Since this change depends on the privacy screen changes staged for v5.17,
I'm OK as platform/chrome maintainer to have this go through the drm tree.

Acked-By: Benson Leung <bleung@chromium.org>


> ---
> v5: * Add Hans' "Reviewed by"
> v4: * Simplify the detect_chromeos_privacy_screen() function
>     * Don't change the existing print statement
> v3: * Remove the pr_info() from detect_chromeos_privacy_screen(), instead
>       enhance the one already present in drm_privacy_screen_lookup_init()
> v2: * Use #if instead of #elif
>     * Reorder the patches in the series.
>     * Rebased on drm-tip
> 
>  drivers/gpu/drm/drm_privacy_screen_x86.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_privacy_screen_x86.c b/drivers/gpu/drm/drm_privacy_screen_x86.c
> index a2cafb294ca6..88802cd7a1ee 100644
> --- a/drivers/gpu/drm/drm_privacy_screen_x86.c
> +++ b/drivers/gpu/drm/drm_privacy_screen_x86.c
> @@ -47,6 +47,13 @@ static bool __init detect_thinkpad_privacy_screen(void)
>  }
>  #endif
>  
> +#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
> +static bool __init detect_chromeos_privacy_screen(void)
> +{
> +	return acpi_dev_present("GOOG0010", NULL, -1);
> +}
> +#endif
> +
>  static const struct arch_init_data arch_init_data[] __initconst = {
>  #if IS_ENABLED(CONFIG_THINKPAD_ACPI)
>  	{
> @@ -58,6 +65,16 @@ static const struct arch_init_data arch_init_data[] __initconst = {
>  		.detect = detect_thinkpad_privacy_screen,
>  	},
>  #endif
> +#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
> +	{
> +		.lookup = {
> +			.dev_id = NULL,
> +			.con_id = NULL,
> +			.provider = "privacy_screen-GOOG0010:00",
> +		},
> +		.detect = detect_chromeos_privacy_screen,
> +	},
> +#endif
>  };
>  
>  void __init drm_privacy_screen_lookup_init(void)
> -- 
> 2.34.1.575.g55b058a8bb-goog
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 2/3] platform/chrome: Add driver for ChromeOS privacy-screen
  2022-01-07 19:02 ` [PATCH v5 2/3] platform/chrome: Add driver for ChromeOS privacy-screen Rajat Jain
@ 2022-01-07 19:09   ` Benson Leung
  0 siblings, 0 replies; 9+ messages in thread
From: Benson Leung @ 2022-01-07 19:09 UTC (permalink / raw)
  To: Rajat Jain
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Hans de Goede, David Airlie, Daniel Vetter, Benson Leung,
	Henrique de Moraes Holschuh, Mark Gross, linux-kernel, dri-devel,
	ibm-acpi-devel, platform-driver-x86, gwendal, seanpaul, marcheu,
	dtor, Dmitry Torokhov, rajatxjain

[-- Attachment #1: Type: text/plain, Size: 9187 bytes --]

Hi Rajat,


On Fri, Jan 07, 2022 at 11:02:07AM -0800, Rajat Jain wrote:
> This adds the ACPI driver for the ChromeOS privacy screen that is
> present on some chromeos devices.
> 
> Note that ideally, we'd want this privacy screen driver to be probed
> BEFORE the drm probe in order to avoid a drm probe deferral:
> https://hansdegoede.livejournal.com/25948.html
> 
> In practise, I found that ACPI drivers are bound to their devices AFTER
> the drm probe on chromebooks. So on chromebooks with privacy-screen,
> this patch along with the other one in this series results in a probe
> deferral of about 250ms for i915 driver. However, it did not result in
> any user noticeable delay of splash screen in my personal experience.
> 
> In future if this probe deferral turns out to be an issue, we can
> consider turning this ACPI driver into something that is probed
> earlier than the drm drivers.
> 
> Signed-off-by: Rajat Jain <rajatja@google.com>
> Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Acked-By: Benson Leung <bleung@chromium.org>

Hi Hans,

Could you take this patch? This new driver in platform/chrome depends on
the drm/drm_privacy_screen_driver.h header in your tree.

Thanks!

> ---
> v5: * Add blank line, 2 more vowels to the driver name as per comments
>     * Add Dmitry's "Reviewed by"
> v4: Same as v3 (No changes)
> v3: * Renamed everything chromeos_priv_scrn_* to chromeos_privacy_screen_*
>      (and added line breaks to accommodate longer names within 80 chars)
>     * Cleanup / Added few comments
>     * Use the newly added drm_privacy_screen_get_drvdata()
>     * Provide the cleanup function chromeos_privacy_screen_remove()
> v2: * Reword the commit log
>     * Make the Kconfig into a tristate
>     * Reorder the patches in the series.
> 
>  drivers/platform/chrome/Kconfig               |  11 ++
>  drivers/platform/chrome/Makefile              |   1 +
>  .../platform/chrome/chromeos_privacy_screen.c | 153 ++++++++++++++++++
>  3 files changed, 165 insertions(+)
>  create mode 100644 drivers/platform/chrome/chromeos_privacy_screen.c
> 
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index ccc23d8686e8..75e93efd669f 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -243,6 +243,17 @@ config CROS_USBPD_NOTIFY
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called cros_usbpd_notify.
>  
> +config CHROMEOS_PRIVACY_SCREEN
> +	tristate "ChromeOS Privacy Screen support"
> +	depends on ACPI
> +	depends on DRM
> +	select DRM_PRIVACY_SCREEN
> +	help
> +	  This driver provides the support needed for the in-built electronic
> +	  privacy screen that is present on some ChromeOS devices. When enabled,
> +	  this should probably always be built into the kernel to avoid or
> +	  minimize drm probe deferral.
> +
>  source "drivers/platform/chrome/wilco_ec/Kconfig"
>  
>  endif # CHROMEOS_PLATFORMS
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index f901d2e43166..5d4be9735d9d 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -4,6 +4,7 @@
>  CFLAGS_cros_ec_trace.o:=		-I$(src)
>  
>  obj-$(CONFIG_CHROMEOS_LAPTOP)		+= chromeos_laptop.o
> +obj-$(CONFIG_CHROMEOS_PRIVACY_SCREEN)	+= chromeos_privacy_screen.o
>  obj-$(CONFIG_CHROMEOS_PSTORE)		+= chromeos_pstore.o
>  obj-$(CONFIG_CHROMEOS_TBMC)		+= chromeos_tbmc.o
>  obj-$(CONFIG_CROS_EC)			+= cros_ec.o
> diff --git a/drivers/platform/chrome/chromeos_privacy_screen.c b/drivers/platform/chrome/chromeos_privacy_screen.c
> new file mode 100644
> index 000000000000..77e9f5ee8e33
> --- /dev/null
> +++ b/drivers/platform/chrome/chromeos_privacy_screen.c
> @@ -0,0 +1,153 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + *  ChromeOS Privacy Screen support
> + *
> + * Copyright (C) 2022 Google LLC
> + *
> + * This is the Chromeos privacy screen provider, present on certain chromebooks,
> + * represented by a GOOG0010 device in the ACPI. This ACPI device, if present,
> + * will cause the i915 drm driver to probe defer until this driver registers
> + * the privacy-screen.
> + */
> +
> +#include <linux/acpi.h>
> +#include <drm/drm_privacy_screen_driver.h>
> +
> +/*
> + * The DSM (Device Specific Method) constants below are the agreed API with
> + * the firmware team, on how to control privacy screen using ACPI methods.
> + */
> +#define PRIV_SCRN_DSM_REVID		1	/* DSM version */
> +#define PRIV_SCRN_DSM_FN_GET_STATUS	1	/* Get privacy screen status */
> +#define PRIV_SCRN_DSM_FN_ENABLE		2	/* Enable privacy screen */
> +#define PRIV_SCRN_DSM_FN_DISABLE	3	/* Disable privacy screen */
> +
> +static const guid_t chromeos_privacy_screen_dsm_guid =
> +		    GUID_INIT(0xc7033113, 0x8720, 0x4ceb,
> +			      0x90, 0x90, 0x9d, 0x52, 0xb3, 0xe5, 0x2d, 0x73);
> +
> +static void
> +chromeos_privacy_screen_get_hw_state(struct drm_privacy_screen
> +				     *drm_privacy_screen)
> +{
> +	union acpi_object *obj;
> +	acpi_handle handle;
> +	struct device *privacy_screen =
> +		drm_privacy_screen_get_drvdata(drm_privacy_screen);
> +
> +	handle = acpi_device_handle(to_acpi_device(privacy_screen));
> +	obj = acpi_evaluate_dsm(handle, &chromeos_privacy_screen_dsm_guid,
> +				PRIV_SCRN_DSM_REVID,
> +				PRIV_SCRN_DSM_FN_GET_STATUS, NULL);
> +	if (!obj) {
> +		dev_err(privacy_screen,
> +			"_DSM failed to get privacy-screen state\n");
> +		return;
> +	}
> +
> +	if (obj->type != ACPI_TYPE_INTEGER)
> +		dev_err(privacy_screen,
> +			"Bad _DSM to get privacy-screen state\n");
> +	else if (obj->integer.value == 1)
> +		drm_privacy_screen->hw_state = drm_privacy_screen->sw_state =
> +			PRIVACY_SCREEN_ENABLED;
> +	else
> +		drm_privacy_screen->hw_state = drm_privacy_screen->sw_state =
> +			PRIVACY_SCREEN_DISABLED;
> +
> +	ACPI_FREE(obj);
> +}
> +
> +static int
> +chromeos_privacy_screen_set_sw_state(struct drm_privacy_screen
> +				     *drm_privacy_screen,
> +				     enum drm_privacy_screen_status state)
> +{
> +	union acpi_object *obj = NULL;
> +	acpi_handle handle;
> +	struct device *privacy_screen =
> +		drm_privacy_screen_get_drvdata(drm_privacy_screen);
> +
> +	handle = acpi_device_handle(to_acpi_device(privacy_screen));
> +
> +	if (state == PRIVACY_SCREEN_DISABLED) {
> +		obj = acpi_evaluate_dsm(handle,
> +					&chromeos_privacy_screen_dsm_guid,
> +					PRIV_SCRN_DSM_REVID,
> +					PRIV_SCRN_DSM_FN_DISABLE, NULL);
> +	} else if (state == PRIVACY_SCREEN_ENABLED) {
> +		obj = acpi_evaluate_dsm(handle,
> +					&chromeos_privacy_screen_dsm_guid,
> +					PRIV_SCRN_DSM_REVID,
> +					PRIV_SCRN_DSM_FN_ENABLE, NULL);
> +	} else {
> +		dev_err(privacy_screen,
> +			"Bad attempt to set privacy-screen status to %u\n",
> +			state);
> +		return -EINVAL;
> +	}
> +
> +	if (!obj) {
> +		dev_err(privacy_screen,
> +			"_DSM failed to set privacy-screen state\n");
> +		return -EIO;
> +	}
> +
> +	drm_privacy_screen->hw_state = drm_privacy_screen->sw_state = state;
> +	ACPI_FREE(obj);
> +	return 0;
> +}
> +
> +static const struct drm_privacy_screen_ops chromeos_privacy_screen_ops = {
> +	.get_hw_state = chromeos_privacy_screen_get_hw_state,
> +	.set_sw_state = chromeos_privacy_screen_set_sw_state,
> +};
> +
> +static int chromeos_privacy_screen_add(struct acpi_device *adev)
> +{
> +	struct drm_privacy_screen *drm_privacy_screen =
> +		drm_privacy_screen_register(&adev->dev,
> +					    &chromeos_privacy_screen_ops,
> +					    &adev->dev);
> +
> +	if (IS_ERR(drm_privacy_screen)) {
> +		dev_err(&adev->dev, "Error registering privacy-screen\n");
> +		return PTR_ERR(drm_privacy_screen);
> +	}
> +
> +	adev->driver_data = drm_privacy_screen;
> +	dev_info(&adev->dev, "registered privacy-screen '%s'\n",
> +		 dev_name(&drm_privacy_screen->dev));
> +
> +	return 0;
> +}
> +
> +static int chromeos_privacy_screen_remove(struct acpi_device *adev)
> +{
> +	struct drm_privacy_screen *drm_privacy_screen =	acpi_driver_data(adev);
> +
> +	drm_privacy_screen_unregister(drm_privacy_screen);
> +	return 0;
> +}
> +
> +static const struct acpi_device_id chromeos_privacy_screen_device_ids[] = {
> +	{"GOOG0010", 0}, /* Google's electronic privacy screen for eDP-1 */
> +	{}
> +};
> +MODULE_DEVICE_TABLE(acpi, chromeos_privacy_screen_device_ids);
> +
> +static struct acpi_driver chromeos_privacy_screen_driver = {
> +	.name = "chromeos_privacy_screen_driver",
> +	.class = "ChromeOS",
> +	.ids = chromeos_privacy_screen_device_ids,
> +	.ops = {
> +		.add = chromeos_privacy_screen_add,
> +		.remove = chromeos_privacy_screen_remove,
> +	},
> +};
> +
> +module_acpi_driver(chromeos_privacy_screen_driver);
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("ChromeOS ACPI Privacy Screen driver");
> +MODULE_AUTHOR("Rajat Jain <rajatja@google.com>");
> -- 
> 2.34.1.575.g55b058a8bb-goog
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 3/3] drm/privacy_screen_x86: Add entry for ChromeOS privacy-screen
  2022-01-07 19:06   ` Benson Leung
@ 2022-01-08 16:10     ` Hans de Goede
  2022-01-08 17:21       ` Rajat Jain
  0 siblings, 1 reply; 9+ messages in thread
From: Hans de Goede @ 2022-01-08 16:10 UTC (permalink / raw)
  To: Benson Leung, Rajat Jain
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Benson Leung,
	Henrique de Moraes Holschuh, Mark Gross, linux-kernel, dri-devel,
	ibm-acpi-devel, platform-driver-x86, gwendal, seanpaul, marcheu,
	dtor, Dmitry Torokhov, rajatxjain

Hi All,

On 1/7/22 20:06, Benson Leung wrote:
> Hi Rajat, 
> 
> Thanks for your changes here.
> 
> On Fri, Jan 07, 2022 at 11:02:08AM -0800, Rajat Jain wrote:
>> Add a static entry in the x86 table, to detect and wait for
>> privacy-screen on some ChromeOS platforms.
>>
>> Please note that this means that if CONFIG_CHROMEOS_PRIVACY_SCREEN is
>> enabled, and if "GOOG0010" device is found in ACPI, then the i915 probe
>> shall return EPROBE_DEFER until a platform driver actually registers the
>> privacy-screen: https://hansdegoede.livejournal.com/25948.html
>>
>> Signed-off-by: Rajat Jain <rajatja@google.com>
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> Hi Hans,
> 
> Since this change depends on the privacy screen changes staged for v5.17,
> I'm OK as platform/chrome maintainer to have this go through the drm tree.
> 
> Acked-By: Benson Leung <bleung@chromium.org>

Ok, I will merge this into drm-misc-next coming Monday.

Note I'm afraid that it is too late for 5.17, the drm-misc maintainers
have already created the final drm-misc tag for the 5.17 pull-req.

Regards,

Hans


> 
> 
>> ---
>> v5: * Add Hans' "Reviewed by"
>> v4: * Simplify the detect_chromeos_privacy_screen() function
>>     * Don't change the existing print statement
>> v3: * Remove the pr_info() from detect_chromeos_privacy_screen(), instead
>>       enhance the one already present in drm_privacy_screen_lookup_init()
>> v2: * Use #if instead of #elif
>>     * Reorder the patches in the series.
>>     * Rebased on drm-tip
>>
>>  drivers/gpu/drm/drm_privacy_screen_x86.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_privacy_screen_x86.c b/drivers/gpu/drm/drm_privacy_screen_x86.c
>> index a2cafb294ca6..88802cd7a1ee 100644
>> --- a/drivers/gpu/drm/drm_privacy_screen_x86.c
>> +++ b/drivers/gpu/drm/drm_privacy_screen_x86.c
>> @@ -47,6 +47,13 @@ static bool __init detect_thinkpad_privacy_screen(void)
>>  }
>>  #endif
>>  
>> +#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
>> +static bool __init detect_chromeos_privacy_screen(void)
>> +{
>> +	return acpi_dev_present("GOOG0010", NULL, -1);
>> +}
>> +#endif
>> +
>>  static const struct arch_init_data arch_init_data[] __initconst = {
>>  #if IS_ENABLED(CONFIG_THINKPAD_ACPI)
>>  	{
>> @@ -58,6 +65,16 @@ static const struct arch_init_data arch_init_data[] __initconst = {
>>  		.detect = detect_thinkpad_privacy_screen,
>>  	},
>>  #endif
>> +#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
>> +	{
>> +		.lookup = {
>> +			.dev_id = NULL,
>> +			.con_id = NULL,
>> +			.provider = "privacy_screen-GOOG0010:00",
>> +		},
>> +		.detect = detect_chromeos_privacy_screen,
>> +	},
>> +#endif
>>  };
>>  
>>  void __init drm_privacy_screen_lookup_init(void)
>> -- 
>> 2.34.1.575.g55b058a8bb-goog
>>
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 3/3] drm/privacy_screen_x86: Add entry for ChromeOS privacy-screen
  2022-01-08 16:10     ` Hans de Goede
@ 2022-01-08 17:21       ` Rajat Jain
  0 siblings, 0 replies; 9+ messages in thread
From: Rajat Jain @ 2022-01-08 17:21 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Benson Leung, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter, Benson Leung,
	Henrique de Moraes Holschuh, Mark Gross, linux-kernel, dri-devel,
	ibm-acpi-devel, platform-driver-x86, gwendal, seanpaul, marcheu,
	dtor, Dmitry Torokhov, rajatxjain

On Sat, Jan 8, 2022 at 8:10 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi All,
>
> On 1/7/22 20:06, Benson Leung wrote:
> > Hi Rajat,
> >
> > Thanks for your changes here.
> >
> > On Fri, Jan 07, 2022 at 11:02:08AM -0800, Rajat Jain wrote:
> >> Add a static entry in the x86 table, to detect and wait for
> >> privacy-screen on some ChromeOS platforms.
> >>
> >> Please note that this means that if CONFIG_CHROMEOS_PRIVACY_SCREEN is
> >> enabled, and if "GOOG0010" device is found in ACPI, then the i915 probe
> >> shall return EPROBE_DEFER until a platform driver actually registers the
> >> privacy-screen: https://hansdegoede.livejournal.com/25948.html
> >>
> >> Signed-off-by: Rajat Jain <rajatja@google.com>
> >> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> >
> > Hi Hans,
> >
> > Since this change depends on the privacy screen changes staged for v5.17,
> > I'm OK as platform/chrome maintainer to have this go through the drm tree.
> >
> > Acked-By: Benson Leung <bleung@chromium.org>
>
> Ok, I will merge this into drm-misc-next coming Monday.

Thank you so much for your help!

>
> Note I'm afraid that it is too late for 5.17, the drm-misc maintainers
> have already created the final drm-misc tag for the 5.17 pull-req.

No problem, this works.

Thanks & Best Regards,

Rajat

>
> Regards,
>
> Hans
>
>
> >
> >
> >> ---
> >> v5: * Add Hans' "Reviewed by"
> >> v4: * Simplify the detect_chromeos_privacy_screen() function
> >>     * Don't change the existing print statement
> >> v3: * Remove the pr_info() from detect_chromeos_privacy_screen(), instead
> >>       enhance the one already present in drm_privacy_screen_lookup_init()
> >> v2: * Use #if instead of #elif
> >>     * Reorder the patches in the series.
> >>     * Rebased on drm-tip
> >>
> >>  drivers/gpu/drm/drm_privacy_screen_x86.c | 17 +++++++++++++++++
> >>  1 file changed, 17 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_privacy_screen_x86.c b/drivers/gpu/drm/drm_privacy_screen_x86.c
> >> index a2cafb294ca6..88802cd7a1ee 100644
> >> --- a/drivers/gpu/drm/drm_privacy_screen_x86.c
> >> +++ b/drivers/gpu/drm/drm_privacy_screen_x86.c
> >> @@ -47,6 +47,13 @@ static bool __init detect_thinkpad_privacy_screen(void)
> >>  }
> >>  #endif
> >>
> >> +#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
> >> +static bool __init detect_chromeos_privacy_screen(void)
> >> +{
> >> +    return acpi_dev_present("GOOG0010", NULL, -1);
> >> +}
> >> +#endif
> >> +
> >>  static const struct arch_init_data arch_init_data[] __initconst = {
> >>  #if IS_ENABLED(CONFIG_THINKPAD_ACPI)
> >>      {
> >> @@ -58,6 +65,16 @@ static const struct arch_init_data arch_init_data[] __initconst = {
> >>              .detect = detect_thinkpad_privacy_screen,
> >>      },
> >>  #endif
> >> +#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
> >> +    {
> >> +            .lookup = {
> >> +                    .dev_id = NULL,
> >> +                    .con_id = NULL,
> >> +                    .provider = "privacy_screen-GOOG0010:00",
> >> +            },
> >> +            .detect = detect_chromeos_privacy_screen,
> >> +    },
> >> +#endif
> >>  };
> >>
> >>  void __init drm_privacy_screen_lookup_init(void)
> >> --
> >> 2.34.1.575.g55b058a8bb-goog
> >>
> >
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen
  2022-01-07 19:02 [PATCH v5 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen Rajat Jain
  2022-01-07 19:02 ` [PATCH v5 2/3] platform/chrome: Add driver for ChromeOS privacy-screen Rajat Jain
  2022-01-07 19:02 ` [PATCH v5 3/3] drm/privacy_screen_x86: Add entry " Rajat Jain
@ 2022-01-10 11:24 ` Hans de Goede
  2022-01-10 19:12   ` Rajat Jain
  2 siblings, 1 reply; 9+ messages in thread
From: Hans de Goede @ 2022-01-10 11:24 UTC (permalink / raw)
  To: Rajat Jain, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Benson Leung,
	Henrique de Moraes Holschuh, Mark Gross, linux-kernel, dri-devel,
	ibm-acpi-devel, platform-driver-x86, gwendal, seanpaul, marcheu,
	dtor, Dmitry Torokhov, Benson Leung
  Cc: rajatxjain

Hi All,

On 1/7/22 20:02, Rajat Jain wrote:
> Allow a privacy screen provider to stash its private data pointer in the
> drm_privacy_screen, and update the drm_privacy_screen_register() call to
> accept that. Also introduce a *_get_drvdata() so that it can retrieved
> back when needed.
> 
> This also touches the IBM Thinkpad platform driver, the only user of
> privacy screen today, to pass NULL for now to the updated API.
> 
> Signed-off-by: Rajat Jain <rajatja@google.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

I've pushed this series to drm-misc-next now.

Regards,

Hans



> ---
> v5: Same as v4
> v4: Added "Reviewed-by" from Hans
> v3: Initial version. Came up due to review comments on v2 of other patches.
> v2: No v2
> v1: No v1
> 
>  drivers/gpu/drm/drm_privacy_screen.c    |  5 ++++-
>  drivers/platform/x86/thinkpad_acpi.c    |  2 +-
>  include/drm/drm_privacy_screen_driver.h | 13 ++++++++++++-
>  3 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_privacy_screen.c b/drivers/gpu/drm/drm_privacy_screen.c
> index beaf99e9120a..03b149cc455b 100644
> --- a/drivers/gpu/drm/drm_privacy_screen.c
> +++ b/drivers/gpu/drm/drm_privacy_screen.c
> @@ -387,7 +387,8 @@ static void drm_privacy_screen_device_release(struct device *dev)
>   * * An ERR_PTR(errno) on failure.
>   */
>  struct drm_privacy_screen *drm_privacy_screen_register(
> -	struct device *parent, const struct drm_privacy_screen_ops *ops)
> +	struct device *parent, const struct drm_privacy_screen_ops *ops,
> +	void *data)
>  {
>  	struct drm_privacy_screen *priv;
>  	int ret;
> @@ -404,6 +405,7 @@ struct drm_privacy_screen *drm_privacy_screen_register(
>  	priv->dev.parent = parent;
>  	priv->dev.release = drm_privacy_screen_device_release;
>  	dev_set_name(&priv->dev, "privacy_screen-%s", dev_name(parent));
> +	priv->drvdata = data;
>  	priv->ops = ops;
>  
>  	priv->ops->get_hw_state(priv);
> @@ -439,6 +441,7 @@ void drm_privacy_screen_unregister(struct drm_privacy_screen *priv)
>  	mutex_unlock(&drm_privacy_screen_devs_lock);
>  
>  	mutex_lock(&priv->lock);
> +	priv->drvdata = NULL;
>  	priv->ops = NULL;
>  	mutex_unlock(&priv->lock);
>  
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 341655d711ce..ccbfda2b0095 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -9782,7 +9782,7 @@ static int tpacpi_lcdshadow_init(struct ibm_init_struct *iibm)
>  		return 0;
>  
>  	lcdshadow_dev = drm_privacy_screen_register(&tpacpi_pdev->dev,
> -						    &lcdshadow_ops);
> +						    &lcdshadow_ops, NULL);
>  	if (IS_ERR(lcdshadow_dev))
>  		return PTR_ERR(lcdshadow_dev);
>  
> diff --git a/include/drm/drm_privacy_screen_driver.h b/include/drm/drm_privacy_screen_driver.h
> index 24591b607675..4ef246d5706f 100644
> --- a/include/drm/drm_privacy_screen_driver.h
> +++ b/include/drm/drm_privacy_screen_driver.h
> @@ -73,10 +73,21 @@ struct drm_privacy_screen {
>  	 * for more info.
>  	 */
>  	enum drm_privacy_screen_status hw_state;
> +	/**
> +	 * @drvdata: Private data owned by the privacy screen provider
> +	 */
> +	void *drvdata;
>  };
>  
> +static inline
> +void *drm_privacy_screen_get_drvdata(struct drm_privacy_screen *priv)
> +{
> +	return priv->drvdata;
> +}
> +
>  struct drm_privacy_screen *drm_privacy_screen_register(
> -	struct device *parent, const struct drm_privacy_screen_ops *ops);
> +	struct device *parent, const struct drm_privacy_screen_ops *ops,
> +	void *data);
>  void drm_privacy_screen_unregister(struct drm_privacy_screen *priv);
>  
>  void drm_privacy_screen_call_notifier_chain(struct drm_privacy_screen *priv);
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen
  2022-01-10 11:24 ` [PATCH v5 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen Hans de Goede
@ 2022-01-10 19:12   ` Rajat Jain
  0 siblings, 0 replies; 9+ messages in thread
From: Rajat Jain @ 2022-01-10 19:12 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rajat Jain, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Benson Leung,
	Henrique de Moraes Holschuh, Mark Gross,
	Linux Kernel Mailing List, dri-devel, ibm-acpi-devel,
	Platform Driver, Gwendal Grignou, Sean Paul, marcheu,
	Dmitry Torokhov, Dmitry Torokhov, Benson Leung

On Mon, Jan 10, 2022 at 3:24 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi All,
>
> On 1/7/22 20:02, Rajat Jain wrote:
> > Allow a privacy screen provider to stash its private data pointer in the
> > drm_privacy_screen, and update the drm_privacy_screen_register() call to
> > accept that. Also introduce a *_get_drvdata() so that it can retrieved
> > back when needed.
> >
> > This also touches the IBM Thinkpad platform driver, the only user of
> > privacy screen today, to pass NULL for now to the updated API.
> >
> > Signed-off-by: Rajat Jain <rajatja@google.com>
> > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>
> I've pushed this series to drm-misc-next now.

Thank you so much. I see it.

Thanks & Best Regards,

Rajat


>
> Regards,
>
> Hans
>
>
>
> > ---
> > v5: Same as v4
> > v4: Added "Reviewed-by" from Hans
> > v3: Initial version. Came up due to review comments on v2 of other patches.
> > v2: No v2
> > v1: No v1
> >
> >  drivers/gpu/drm/drm_privacy_screen.c    |  5 ++++-
> >  drivers/platform/x86/thinkpad_acpi.c    |  2 +-
> >  include/drm/drm_privacy_screen_driver.h | 13 ++++++++++++-
> >  3 files changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_privacy_screen.c b/drivers/gpu/drm/drm_privacy_screen.c
> > index beaf99e9120a..03b149cc455b 100644
> > --- a/drivers/gpu/drm/drm_privacy_screen.c
> > +++ b/drivers/gpu/drm/drm_privacy_screen.c
> > @@ -387,7 +387,8 @@ static void drm_privacy_screen_device_release(struct device *dev)
> >   * * An ERR_PTR(errno) on failure.
> >   */
> >  struct drm_privacy_screen *drm_privacy_screen_register(
> > -     struct device *parent, const struct drm_privacy_screen_ops *ops)
> > +     struct device *parent, const struct drm_privacy_screen_ops *ops,
> > +     void *data)
> >  {
> >       struct drm_privacy_screen *priv;
> >       int ret;
> > @@ -404,6 +405,7 @@ struct drm_privacy_screen *drm_privacy_screen_register(
> >       priv->dev.parent = parent;
> >       priv->dev.release = drm_privacy_screen_device_release;
> >       dev_set_name(&priv->dev, "privacy_screen-%s", dev_name(parent));
> > +     priv->drvdata = data;
> >       priv->ops = ops;
> >
> >       priv->ops->get_hw_state(priv);
> > @@ -439,6 +441,7 @@ void drm_privacy_screen_unregister(struct drm_privacy_screen *priv)
> >       mutex_unlock(&drm_privacy_screen_devs_lock);
> >
> >       mutex_lock(&priv->lock);
> > +     priv->drvdata = NULL;
> >       priv->ops = NULL;
> >       mutex_unlock(&priv->lock);
> >
> > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> > index 341655d711ce..ccbfda2b0095 100644
> > --- a/drivers/platform/x86/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > @@ -9782,7 +9782,7 @@ static int tpacpi_lcdshadow_init(struct ibm_init_struct *iibm)
> >               return 0;
> >
> >       lcdshadow_dev = drm_privacy_screen_register(&tpacpi_pdev->dev,
> > -                                                 &lcdshadow_ops);
> > +                                                 &lcdshadow_ops, NULL);
> >       if (IS_ERR(lcdshadow_dev))
> >               return PTR_ERR(lcdshadow_dev);
> >
> > diff --git a/include/drm/drm_privacy_screen_driver.h b/include/drm/drm_privacy_screen_driver.h
> > index 24591b607675..4ef246d5706f 100644
> > --- a/include/drm/drm_privacy_screen_driver.h
> > +++ b/include/drm/drm_privacy_screen_driver.h
> > @@ -73,10 +73,21 @@ struct drm_privacy_screen {
> >        * for more info.
> >        */
> >       enum drm_privacy_screen_status hw_state;
> > +     /**
> > +      * @drvdata: Private data owned by the privacy screen provider
> > +      */
> > +     void *drvdata;
> >  };
> >
> > +static inline
> > +void *drm_privacy_screen_get_drvdata(struct drm_privacy_screen *priv)
> > +{
> > +     return priv->drvdata;
> > +}
> > +
> >  struct drm_privacy_screen *drm_privacy_screen_register(
> > -     struct device *parent, const struct drm_privacy_screen_ops *ops);
> > +     struct device *parent, const struct drm_privacy_screen_ops *ops,
> > +     void *data);
> >  void drm_privacy_screen_unregister(struct drm_privacy_screen *priv);
> >
> >  void drm_privacy_screen_call_notifier_chain(struct drm_privacy_screen *priv);
> >
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-01-10 19:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-07 19:02 [PATCH v5 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen Rajat Jain
2022-01-07 19:02 ` [PATCH v5 2/3] platform/chrome: Add driver for ChromeOS privacy-screen Rajat Jain
2022-01-07 19:09   ` Benson Leung
2022-01-07 19:02 ` [PATCH v5 3/3] drm/privacy_screen_x86: Add entry " Rajat Jain
2022-01-07 19:06   ` Benson Leung
2022-01-08 16:10     ` Hans de Goede
2022-01-08 17:21       ` Rajat Jain
2022-01-10 11:24 ` [PATCH v5 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen Hans de Goede
2022-01-10 19:12   ` Rajat Jain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).