diff mbox series

[3/9] drm/privacy-screen: Add X86 specific arch init code

Message ID 20210906073519.4615-4-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show
Series drm: Add privacy-screen class and connector properties | expand

Commit Message

Hans de Goede Sept. 6, 2021, 7:35 a.m. UTC 
Add X86 specific arch init code, which fills the privacy-screen lookup
table by checking for various vendor specific ACPI interfaces for
controlling the privacy-screen.

This initial version only checks for the Lenovo Thinkpad specific ACPI
methods for privacy-screen control.

Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/Makefile                 |  2 +-
 drivers/gpu/drm/drm_privacy_screen_x86.c | 86 ++++++++++++++++++++++++
 include/drm/drm_privacy_screen_machine.h |  5 ++
 3 files changed, 92 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_privacy_screen_x86.c

Comments

Jani Nikula Sept. 16, 2021, 8:51 a.m. UTC | #1 
On Mon, 06 Sep 2021, Hans de Goede <hdegoede@redhat.com> wrote:
> Add X86 specific arch init code, which fills the privacy-screen lookup
> table by checking for various vendor specific ACPI interfaces for
> controlling the privacy-screen.
>
> This initial version only checks for the Lenovo Thinkpad specific ACPI
> methods for privacy-screen control.
>
> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/Makefile                 |  2 +-
>  drivers/gpu/drm/drm_privacy_screen_x86.c | 86 ++++++++++++++++++++++++
>  include/drm/drm_privacy_screen_machine.h |  5 ++
>  3 files changed, 92 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/drm_privacy_screen_x86.c
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 788fc37096f6..12997ca5670d 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -32,7 +32,7 @@ drm-$(CONFIG_OF) += drm_of.o
>  drm-$(CONFIG_PCI) += drm_pci.o
>  drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
>  drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> -drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o
> +drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o drm_privacy_screen_x86.o

Would be nice to avoid building drm_privacy_screen_x86.o altogether for
CONFIG_X86=n, and avoid...

>  
>  obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o
>  
> diff --git a/drivers/gpu/drm/drm_privacy_screen_x86.c b/drivers/gpu/drm/drm_privacy_screen_x86.c
> new file mode 100644
> index 000000000000..a2cafb294ca6
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_privacy_screen_x86.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright (C) 2020 Red Hat, Inc.
> + *
> + * Authors:
> + * Hans de Goede <hdegoede@redhat.com>
> + */
> +
> +#include <linux/acpi.h>
> +#include <drm/drm_privacy_screen_machine.h>
> +
> +#ifdef CONFIG_X86

...ifdefs that cover the entire file. This can be a future improvement,
though.

> +static struct drm_privacy_screen_lookup arch_lookup;
> +
> +struct arch_init_data {
> +	struct drm_privacy_screen_lookup lookup;
> +	bool (*detect)(void);
> +};
> +
> +#if IS_ENABLED(CONFIG_THINKPAD_ACPI)
> +static acpi_status __init acpi_set_handle(acpi_handle handle, u32 level,
> +					  void *context, void **return_value)
> +{
> +	*(acpi_handle *)return_value = handle;
> +	return AE_CTRL_TERMINATE;
> +}
> +
> +static bool __init detect_thinkpad_privacy_screen(void)
> +{
> +	union acpi_object obj = { .type = ACPI_TYPE_INTEGER };
> +	struct acpi_object_list args = { .count = 1, .pointer = &obj, };
> +	acpi_handle ec_handle = NULL;
> +	unsigned long long output;
> +	acpi_status status;
> +
> +	/* Get embedded-controller handle */
> +	status = acpi_get_devices("PNP0C09", acpi_set_handle, NULL, &ec_handle);
> +	if (ACPI_FAILURE(status) || !ec_handle)
> +		return false;
> +
> +	/* And call the privacy-screen get-status method */
> +	status = acpi_evaluate_integer(ec_handle, "HKEY.GSSS", &args, &output);
> +	if (ACPI_FAILURE(status))
> +		return false;
> +
> +	return (output & 0x10000) ? true : false;
> +}
> +#endif
> +
> +static const struct arch_init_data arch_init_data[] __initconst = {
> +#if IS_ENABLED(CONFIG_THINKPAD_ACPI)
> +	{
> +		.lookup = {
> +			.dev_id = NULL,
> +			.con_id = NULL,
> +			.provider = "privacy_screen-thinkpad_acpi",
> +		},
> +		.detect = detect_thinkpad_privacy_screen,
> +	},
> +#endif
> +};
> +
> +void __init drm_privacy_screen_lookup_init(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(arch_init_data); i++) {
> +		if (!arch_init_data[i].detect())
> +			continue;
> +
> +		pr_info("Found '%s' privacy-screen provider\n",
> +			arch_init_data[i].lookup.provider);
> +
> +		/* Make a copy because arch_init_data is __initconst */
> +		arch_lookup = arch_init_data[i].lookup;
> +		drm_privacy_screen_lookup_add(&arch_lookup);
> +		break;
> +	}
> +}
> +
> +void drm_privacy_screen_lookup_exit(void)
> +{
> +	if (arch_lookup.provider)
> +		drm_privacy_screen_lookup_remove(&arch_lookup);
> +}
> +#endif /* ifdef CONFIG_X86 */
> diff --git a/include/drm/drm_privacy_screen_machine.h b/include/drm/drm_privacy_screen_machine.h
> index aaa0d38cce92..02e5371904d3 100644
> --- a/include/drm/drm_privacy_screen_machine.h
> +++ b/include/drm/drm_privacy_screen_machine.h
> @@ -31,11 +31,16 @@ struct drm_privacy_screen_lookup {
>  void drm_privacy_screen_lookup_add(struct drm_privacy_screen_lookup *lookup);
>  void drm_privacy_screen_lookup_remove(struct drm_privacy_screen_lookup *lookup);
>  
> +#if IS_ENABLED(CONFIG_DRM_PRIVACY_SCREEN) && IS_ENABLED(CONFIG_X86)
> +void drm_privacy_screen_lookup_init(void);
> +void drm_privacy_screen_lookup_exit(void);
> +#else
>  static inline void drm_privacy_screen_lookup_init(void)
>  {
>  }
>  static inline void drm_privacy_screen_lookup_exit(void)
>  {
>  }
> +#endif
>  
>  #endif
Hans de Goede Sept. 16, 2021, 9:18 a.m. UTC | #2 
Hi,

On 9/16/21 10:51 AM, Jani Nikula wrote:
> On Mon, 06 Sep 2021, Hans de Goede <hdegoede@redhat.com> wrote:
>> Add X86 specific arch init code, which fills the privacy-screen lookup
>> table by checking for various vendor specific ACPI interfaces for
>> controlling the privacy-screen.
>>
>> This initial version only checks for the Lenovo Thinkpad specific ACPI
>> methods for privacy-screen control.
>>
>> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/gpu/drm/Makefile                 |  2 +-
>>  drivers/gpu/drm/drm_privacy_screen_x86.c | 86 ++++++++++++++++++++++++
>>  include/drm/drm_privacy_screen_machine.h |  5 ++
>>  3 files changed, 92 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/gpu/drm/drm_privacy_screen_x86.c
>>
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 788fc37096f6..12997ca5670d 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -32,7 +32,7 @@ drm-$(CONFIG_OF) += drm_of.o
>>  drm-$(CONFIG_PCI) += drm_pci.o
>>  drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
>>  drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
>> -drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o
>> +drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o drm_privacy_screen_x86.o
> 
> Would be nice to avoid building drm_privacy_screen_x86.o altogether for
> CONFIG_X86=n, and avoid...

Right unfortunately AFAIK I cannot write something like:

drm-$(CONFIG_DRM_PRIVACY_SCREEN && CONFIG_X86) += drm_privacy_screen_x86.o

So this would require adding a (non user selectable)
CONFIG_DRM_PRIVACY_SCREEN_X86 in Kconfig looking something
like this:

config DRM_PRIVACY_SCREEN
	bool
	default n
	select DRM_PRIVACY_SCREEN_X86 if X86

config DRM_PRIVACY_SCREEN_X86
	bool
	default n

Which is also not really pretty. 


>>  obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o
>>  
>> diff --git a/drivers/gpu/drm/drm_privacy_screen_x86.c b/drivers/gpu/drm/drm_privacy_screen_x86.c
>> new file mode 100644
>> index 000000000000..a2cafb294ca6
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_privacy_screen_x86.c
>> @@ -0,0 +1,86 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright (C) 2020 Red Hat, Inc.
>> + *
>> + * Authors:
>> + * Hans de Goede <hdegoede@redhat.com>
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#include <drm/drm_privacy_screen_machine.h>
>> +
>> +#ifdef CONFIG_X86
> 
> ...ifdefs that cover the entire file. This can be a future improvement,
> though.

Thanks I would be happy to do a follow-up patch if we can come-up with
a better solution which we all like.

For now I would indeed prefer to move forward with this patch-set as is
because it has been rather long in the making, so it will be good if
I can finally get it upstream.

Regards,

Hans


> 
>> +static struct drm_privacy_screen_lookup arch_lookup;
>> +
>> +struct arch_init_data {
>> +	struct drm_privacy_screen_lookup lookup;
>> +	bool (*detect)(void);
>> +};
>> +
>> +#if IS_ENABLED(CONFIG_THINKPAD_ACPI)
>> +static acpi_status __init acpi_set_handle(acpi_handle handle, u32 level,
>> +					  void *context, void **return_value)
>> +{
>> +	*(acpi_handle *)return_value = handle;
>> +	return AE_CTRL_TERMINATE;
>> +}
>> +
>> +static bool __init detect_thinkpad_privacy_screen(void)
>> +{
>> +	union acpi_object obj = { .type = ACPI_TYPE_INTEGER };
>> +	struct acpi_object_list args = { .count = 1, .pointer = &obj, };
>> +	acpi_handle ec_handle = NULL;
>> +	unsigned long long output;
>> +	acpi_status status;
>> +
>> +	/* Get embedded-controller handle */
>> +	status = acpi_get_devices("PNP0C09", acpi_set_handle, NULL, &ec_handle);
>> +	if (ACPI_FAILURE(status) || !ec_handle)
>> +		return false;
>> +
>> +	/* And call the privacy-screen get-status method */
>> +	status = acpi_evaluate_integer(ec_handle, "HKEY.GSSS", &args, &output);
>> +	if (ACPI_FAILURE(status))
>> +		return false;
>> +
>> +	return (output & 0x10000) ? true : false;
>> +}
>> +#endif
>> +
>> +static const struct arch_init_data arch_init_data[] __initconst = {
>> +#if IS_ENABLED(CONFIG_THINKPAD_ACPI)
>> +	{
>> +		.lookup = {
>> +			.dev_id = NULL,
>> +			.con_id = NULL,
>> +			.provider = "privacy_screen-thinkpad_acpi",
>> +		},
>> +		.detect = detect_thinkpad_privacy_screen,
>> +	},
>> +#endif
>> +};
>> +
>> +void __init drm_privacy_screen_lookup_init(void)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(arch_init_data); i++) {
>> +		if (!arch_init_data[i].detect())
>> +			continue;
>> +
>> +		pr_info("Found '%s' privacy-screen provider\n",
>> +			arch_init_data[i].lookup.provider);
>> +
>> +		/* Make a copy because arch_init_data is __initconst */
>> +		arch_lookup = arch_init_data[i].lookup;
>> +		drm_privacy_screen_lookup_add(&arch_lookup);
>> +		break;
>> +	}
>> +}
>> +
>> +void drm_privacy_screen_lookup_exit(void)
>> +{
>> +	if (arch_lookup.provider)
>> +		drm_privacy_screen_lookup_remove(&arch_lookup);
>> +}
>> +#endif /* ifdef CONFIG_X86 */
>> diff --git a/include/drm/drm_privacy_screen_machine.h b/include/drm/drm_privacy_screen_machine.h
>> index aaa0d38cce92..02e5371904d3 100644
>> --- a/include/drm/drm_privacy_screen_machine.h
>> +++ b/include/drm/drm_privacy_screen_machine.h
>> @@ -31,11 +31,16 @@ struct drm_privacy_screen_lookup {
>>  void drm_privacy_screen_lookup_add(struct drm_privacy_screen_lookup *lookup);
>>  void drm_privacy_screen_lookup_remove(struct drm_privacy_screen_lookup *lookup);
>>  
>> +#if IS_ENABLED(CONFIG_DRM_PRIVACY_SCREEN) && IS_ENABLED(CONFIG_X86)
>> +void drm_privacy_screen_lookup_init(void);
>> +void drm_privacy_screen_lookup_exit(void);
>> +#else
>>  static inline void drm_privacy_screen_lookup_init(void)
>>  {
>>  }
>>  static inline void drm_privacy_screen_lookup_exit(void)
>>  {
>>  }
>> +#endif
>>  
>>  #endif
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 788fc37096f6..12997ca5670d 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -32,7 +32,7 @@  drm-$(CONFIG_OF) += drm_of.o
 drm-$(CONFIG_PCI) += drm_pci.o
 drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
 drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
-drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o
+drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o drm_privacy_screen_x86.o
 
 obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o
 
diff --git a/drivers/gpu/drm/drm_privacy_screen_x86.c b/drivers/gpu/drm/drm_privacy_screen_x86.c
new file mode 100644
index 000000000000..a2cafb294ca6
--- /dev/null
+++ b/drivers/gpu/drm/drm_privacy_screen_x86.c
@@ -0,0 +1,86 @@ 
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright (C) 2020 Red Hat, Inc.
+ *
+ * Authors:
+ * Hans de Goede <hdegoede@redhat.com>
+ */
+
+#include <linux/acpi.h>
+#include <drm/drm_privacy_screen_machine.h>
+
+#ifdef CONFIG_X86
+static struct drm_privacy_screen_lookup arch_lookup;
+
+struct arch_init_data {
+	struct drm_privacy_screen_lookup lookup;
+	bool (*detect)(void);
+};
+
+#if IS_ENABLED(CONFIG_THINKPAD_ACPI)
+static acpi_status __init acpi_set_handle(acpi_handle handle, u32 level,
+					  void *context, void **return_value)
+{
+	*(acpi_handle *)return_value = handle;
+	return AE_CTRL_TERMINATE;
+}
+
+static bool __init detect_thinkpad_privacy_screen(void)
+{
+	union acpi_object obj = { .type = ACPI_TYPE_INTEGER };
+	struct acpi_object_list args = { .count = 1, .pointer = &obj, };
+	acpi_handle ec_handle = NULL;
+	unsigned long long output;
+	acpi_status status;
+
+	/* Get embedded-controller handle */
+	status = acpi_get_devices("PNP0C09", acpi_set_handle, NULL, &ec_handle);
+	if (ACPI_FAILURE(status) || !ec_handle)
+		return false;
+
+	/* And call the privacy-screen get-status method */
+	status = acpi_evaluate_integer(ec_handle, "HKEY.GSSS", &args, &output);
+	if (ACPI_FAILURE(status))
+		return false;
+
+	return (output & 0x10000) ? true : false;
+}
+#endif
+
+static const struct arch_init_data arch_init_data[] __initconst = {
+#if IS_ENABLED(CONFIG_THINKPAD_ACPI)
+	{
+		.lookup = {
+			.dev_id = NULL,
+			.con_id = NULL,
+			.provider = "privacy_screen-thinkpad_acpi",
+		},
+		.detect = detect_thinkpad_privacy_screen,
+	},
+#endif
+};
+
+void __init drm_privacy_screen_lookup_init(void)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(arch_init_data); i++) {
+		if (!arch_init_data[i].detect())
+			continue;
+
+		pr_info("Found '%s' privacy-screen provider\n",
+			arch_init_data[i].lookup.provider);
+
+		/* Make a copy because arch_init_data is __initconst */
+		arch_lookup = arch_init_data[i].lookup;
+		drm_privacy_screen_lookup_add(&arch_lookup);
+		break;
+	}
+}
+
+void drm_privacy_screen_lookup_exit(void)
+{
+	if (arch_lookup.provider)
+		drm_privacy_screen_lookup_remove(&arch_lookup);
+}
+#endif /* ifdef CONFIG_X86 */
diff --git a/include/drm/drm_privacy_screen_machine.h b/include/drm/drm_privacy_screen_machine.h
index aaa0d38cce92..02e5371904d3 100644
--- a/include/drm/drm_privacy_screen_machine.h
+++ b/include/drm/drm_privacy_screen_machine.h
@@ -31,11 +31,16 @@  struct drm_privacy_screen_lookup {
 void drm_privacy_screen_lookup_add(struct drm_privacy_screen_lookup *lookup);
 void drm_privacy_screen_lookup_remove(struct drm_privacy_screen_lookup *lookup);
 
+#if IS_ENABLED(CONFIG_DRM_PRIVACY_SCREEN) && IS_ENABLED(CONFIG_X86)
+void drm_privacy_screen_lookup_init(void);
+void drm_privacy_screen_lookup_exit(void);
+#else
 static inline void drm_privacy_screen_lookup_init(void)
 {
 }
 static inline void drm_privacy_screen_lookup_exit(void)
 {
 }
+#endif
 
 #endif