Re: [PATCH 2/9] drm: Add privacy-screen class (v3)

2021-09-16 Thread Hans de Goede 
Hi Lyude,

Thank you very much for the review of this series.

On 9/15/21 10:01 PM, Lyude Paul wrote:
> On Mon, 2021-09-06 at 09:35 +0200, Hans de Goede wrote:
>> On some new laptops the LCD panel has a builtin electronic privacy-screen.
>> We want to export this functionality as a property on the drm connector
>> object. But often this functionality is not exposed on the GPU but on some
>> other (ACPI) device.
>>
>> This commit adds a privacy-screen class allowing the driver for these
>> other devices to register themselves as a privacy-screen provider; and
>> allowing the drm/kms code to get a privacy-screen provider associated
>> with a specific GPU/connector combo.
>>
>> Changes in v2:
>> - Make CONFIG_DRM_PRIVACY_SCREEN a bool which controls if the drm_privacy
>>   code gets built as part of the main drm module rather then making it
>>   a tristate which builds its own module.
>> - Add a #if IS_ENABLED(CONFIG_DRM_PRIVACY_SCREEN) check to
>>   drm_privacy_screen_consumer.h and define stubs when the check fails.
>>   Together these 2 changes fix several dependency issues.
>> - Remove module related code now that this is part of the main drm.ko
>> - Use drm_class as class for the privacy-screen devices instead of
>>   adding a separate class for this
>>
>> Changes in v3:
>> - Make the static inline drm_privacy_screen_get_state() stub set sw_state
>>   and hw_state to PRIVACY_SCREEN_DISABLED to squelch an uninitialized
>>   variable warning when CONFIG_DRM_PRIVICAY_SCREEN is not set
>>
>> Reviewed-by: Emil Velikov 
>> Signed-off-by: Hans de Goede 
>> ---
>>  Documentation/gpu/drm-kms-helpers.rst |  15 +
>>  MAINTAINERS   |   8 +
>>  drivers/gpu/drm/Kconfig   |   4 +
>>  drivers/gpu/drm/Makefile  |   1 +
>>  drivers/gpu/drm/drm_drv.c |   4 +
>>  drivers/gpu/drm/drm_privacy_screen.c  | 401 ++
>>  include/drm/drm_privacy_screen_consumer.h |  50 +++
>>  include/drm/drm_privacy_screen_driver.h   |  80 +
>>  include/drm/drm_privacy_screen_machine.h  |  41 +++
>>  9 files changed, 604 insertions(+)
>>  create mode 100644 drivers/gpu/drm/drm_privacy_screen.c
>>  create mode 100644 include/drm/drm_privacy_screen_consumer.h
>>  create mode 100644 include/drm/drm_privacy_screen_driver.h
>>  create mode 100644 include/drm/drm_privacy_screen_machine.h
>>
>> diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-
>> kms-helpers.rst
>> index 389892f36185..5d8715d2f998 100644
>> --- a/Documentation/gpu/drm-kms-helpers.rst
>> +++ b/Documentation/gpu/drm-kms-helpers.rst
>> @@ -423,3 +423,18 @@ Legacy CRTC/Modeset Helper Functions Reference
>>  
>>  .. kernel-doc:: drivers/gpu/drm/drm_crtc_helper.c
>>     :export:
>> +
>> +Privacy-screen class
>> +
>> +
>> +.. kernel-doc:: drivers/gpu/drm/drm_privacy_screen.c
>> +   :doc: overview
>> +
>> +.. kernel-doc:: include/drm/drm_privacy_screen_driver.h
>> +   :internal:
>> +
>> +.. kernel-doc:: include/drm/drm_privacy_screen_machine.h
>> +   :internal:
>> +
>> +.. kernel-doc:: drivers/gpu/drm/drm_privacy_screen.c
>> +   :export:
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index ede4a37a53b3..a272ca600f98 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -6376,6 +6376,14 @@ F:   drivers/gpu/drm/drm_panel.c
>>  F: drivers/gpu/drm/panel/
>>  F: include/drm/drm_panel.h
>>  
>> +DRM PRIVACY-SCREEN CLASS
>> +M: Hans de Goede 
>> +L: dri-devel@lists.freedesktop.org
>> +S: Maintained
>> +T: git git://anongit.freedesktop.org/drm/drm-misc
>> +F: drivers/gpu/drm/drm_privacy_screen*
>> +F: include/drm/drm_privacy_screen*
>> +
>>  DRM TTM SUBSYSTEM
>>  M: Christian Koenig 
>>  M: Huang Rui 
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index b17e231ca6f7..7249b010ab90 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -481,3 +481,7 @@ config DRM_PANEL_ORIENTATION_QUIRKS
>>  config DRM_LIB_RANDOM
>> bool
>> default n
>> +
>> +config DRM_PRIVACY_SCREEN
>> +   bool
>> +   default n
> 
> This is probably worth documenting for folks configuring their kernels to
> explain what this actually does (something simple like "Controls programmable
> privacy screens found on some devices, if unsure select Y" would probably be
> fine)

Like the "config DRM_LIB_RANDOM" above it, this is not user selectable,
(notice there is no text after the "bool"), this is selected by drivers
which implement drm-privacy-screen control, such as the thinkpad_acpi
driver.

If no such drivers is selected and thus CONFIG_DRM_PRIVACY_SCREEN is
not enabled then include/drm/drm_privacy_screen_consumer.h
and drm_privacy_screen_machine.h provide no-op stubs so that the
integration with the drm-core still builds without needing #ifdef-s
in the drm-core.


> 
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 0dff40bb863c..788fc37096f6 100644

Re: [PATCH 2/9] drm: Add privacy-screen class (v3)

2021-09-15 Thread Lyude Paul 
On Mon, 2021-09-06 at 09:35 +0200, Hans de Goede wrote:
> On some new laptops the LCD panel has a builtin electronic privacy-screen.
> We want to export this functionality as a property on the drm connector
> object. But often this functionality is not exposed on the GPU but on some
> other (ACPI) device.
> 
> This commit adds a privacy-screen class allowing the driver for these
> other devices to register themselves as a privacy-screen provider; and
> allowing the drm/kms code to get a privacy-screen provider associated
> with a specific GPU/connector combo.
> 
> Changes in v2:
> - Make CONFIG_DRM_PRIVACY_SCREEN a bool which controls if the drm_privacy
>   code gets built as part of the main drm module rather then making it
>   a tristate which builds its own module.
> - Add a #if IS_ENABLED(CONFIG_DRM_PRIVACY_SCREEN) check to
>   drm_privacy_screen_consumer.h and define stubs when the check fails.
>   Together these 2 changes fix several dependency issues.
> - Remove module related code now that this is part of the main drm.ko
> - Use drm_class as class for the privacy-screen devices instead of
>   adding a separate class for this
> 
> Changes in v3:
> - Make the static inline drm_privacy_screen_get_state() stub set sw_state
>   and hw_state to PRIVACY_SCREEN_DISABLED to squelch an uninitialized
>   variable warning when CONFIG_DRM_PRIVICAY_SCREEN is not set
> 
> Reviewed-by: Emil Velikov 
> Signed-off-by: Hans de Goede 
> ---
>  Documentation/gpu/drm-kms-helpers.rst |  15 +
>  MAINTAINERS   |   8 +
>  drivers/gpu/drm/Kconfig   |   4 +
>  drivers/gpu/drm/Makefile  |   1 +
>  drivers/gpu/drm/drm_drv.c |   4 +
>  drivers/gpu/drm/drm_privacy_screen.c  | 401 ++
>  include/drm/drm_privacy_screen_consumer.h |  50 +++
>  include/drm/drm_privacy_screen_driver.h   |  80 +
>  include/drm/drm_privacy_screen_machine.h  |  41 +++
>  9 files changed, 604 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_privacy_screen.c
>  create mode 100644 include/drm/drm_privacy_screen_consumer.h
>  create mode 100644 include/drm/drm_privacy_screen_driver.h
>  create mode 100644 include/drm/drm_privacy_screen_machine.h
> 
> diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-
> kms-helpers.rst
> index 389892f36185..5d8715d2f998 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -423,3 +423,18 @@ Legacy CRTC/Modeset Helper Functions Reference
>  
>  .. kernel-doc:: drivers/gpu/drm/drm_crtc_helper.c
>     :export:
> +
> +Privacy-screen class
> +
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_privacy_screen.c
> +   :doc: overview
> +
> +.. kernel-doc:: include/drm/drm_privacy_screen_driver.h
> +   :internal:
> +
> +.. kernel-doc:: include/drm/drm_privacy_screen_machine.h
> +   :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_privacy_screen.c
> +   :export:
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ede4a37a53b3..a272ca600f98 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6376,6 +6376,14 @@ F:   drivers/gpu/drm/drm_panel.c
>  F: drivers/gpu/drm/panel/
>  F: include/drm/drm_panel.h
>  
> +DRM PRIVACY-SCREEN CLASS
> +M: Hans de Goede 
> +L: dri-devel@lists.freedesktop.org
> +S: Maintained
> +T: git git://anongit.freedesktop.org/drm/drm-misc
> +F: drivers/gpu/drm/drm_privacy_screen*
> +F: include/drm/drm_privacy_screen*
> +
>  DRM TTM SUBSYSTEM
>  M: Christian Koenig 
>  M: Huang Rui 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index b17e231ca6f7..7249b010ab90 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -481,3 +481,7 @@ config DRM_PANEL_ORIENTATION_QUIRKS
>  config DRM_LIB_RANDOM
> bool
> default n
> +
> +config DRM_PRIVACY_SCREEN
> +   bool
> +   default n

This is probably worth documenting for folks configuring their kernels to
explain what this actually does (something simple like "Controls programmable
privacy screens found on some devices, if unsure select Y" would probably be
fine)

> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 0dff40bb863c..788fc37096f6 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -32,6 +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
>  
>  obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o
>  
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 7a5097467ba5..dc293b771c3f 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -43,6 +43,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "drm_crtc_internal.h"
>  #include "drm_internal.h"
> @@ -102

[PATCH 2/9] drm: Add privacy-screen class (v3)

2021-09-06 Thread Hans de Goede 
On some new laptops the LCD panel has a builtin electronic privacy-screen.
We want to export this functionality as a property on the drm connector
object. But often this functionality is not exposed on the GPU but on some
other (ACPI) device.

This commit adds a privacy-screen class allowing the driver for these
other devices to register themselves as a privacy-screen provider; and
allowing the drm/kms code to get a privacy-screen provider associated
with a specific GPU/connector combo.

Changes in v2:
- Make CONFIG_DRM_PRIVACY_SCREEN a bool which controls if the drm_privacy
  code gets built as part of the main drm module rather then making it
  a tristate which builds its own module.
- Add a #if IS_ENABLED(CONFIG_DRM_PRIVACY_SCREEN) check to
  drm_privacy_screen_consumer.h and define stubs when the check fails.
  Together these 2 changes fix several dependency issues.
- Remove module related code now that this is part of the main drm.ko
- Use drm_class as class for the privacy-screen devices instead of
  adding a separate class for this

Changes in v3:
- Make the static inline drm_privacy_screen_get_state() stub set sw_state
  and hw_state to PRIVACY_SCREEN_DISABLED to squelch an uninitialized
  variable warning when CONFIG_DRM_PRIVICAY_SCREEN is not set

Reviewed-by: Emil Velikov 
Signed-off-by: Hans de Goede 
---
 Documentation/gpu/drm-kms-helpers.rst |  15 +
 MAINTAINERS   |   8 +
 drivers/gpu/drm/Kconfig   |   4 +
 drivers/gpu/drm/Makefile  |   1 +
 drivers/gpu/drm/drm_drv.c |   4 +
 drivers/gpu/drm/drm_privacy_screen.c  | 401 ++
 include/drm/drm_privacy_screen_consumer.h |  50 +++
 include/drm/drm_privacy_screen_driver.h   |  80 +
 include/drm/drm_privacy_screen_machine.h  |  41 +++
 9 files changed, 604 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_privacy_screen.c
 create mode 100644 include/drm/drm_privacy_screen_consumer.h
 create mode 100644 include/drm/drm_privacy_screen_driver.h
 create mode 100644 include/drm/drm_privacy_screen_machine.h

diff --git a/Documentation/gpu/drm-kms-helpers.rst 
b/Documentation/gpu/drm-kms-helpers.rst
index 389892f36185..5d8715d2f998 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -423,3 +423,18 @@ Legacy CRTC/Modeset Helper Functions Reference
 
 .. kernel-doc:: drivers/gpu/drm/drm_crtc_helper.c
:export:
+
+Privacy-screen class
+
+
+.. kernel-doc:: drivers/gpu/drm/drm_privacy_screen.c
+   :doc: overview
+
+.. kernel-doc:: include/drm/drm_privacy_screen_driver.h
+   :internal:
+
+.. kernel-doc:: include/drm/drm_privacy_screen_machine.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_privacy_screen.c
+   :export:
diff --git a/MAINTAINERS b/MAINTAINERS
index ede4a37a53b3..a272ca600f98 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6376,6 +6376,14 @@ F:   drivers/gpu/drm/drm_panel.c
 F: drivers/gpu/drm/panel/
 F: include/drm/drm_panel.h
 
+DRM PRIVACY-SCREEN CLASS
+M: Hans de Goede 
+L: dri-devel@lists.freedesktop.org
+S: Maintained
+T: git git://anongit.freedesktop.org/drm/drm-misc
+F: drivers/gpu/drm/drm_privacy_screen*
+F: include/drm/drm_privacy_screen*
+
 DRM TTM SUBSYSTEM
 M: Christian Koenig 
 M: Huang Rui 
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index b17e231ca6f7..7249b010ab90 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -481,3 +481,7 @@ config DRM_PANEL_ORIENTATION_QUIRKS
 config DRM_LIB_RANDOM
bool
default n
+
+config DRM_PRIVACY_SCREEN
+   bool
+   default n
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 0dff40bb863c..788fc37096f6 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -32,6 +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
 
 obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o
 
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 7a5097467ba5..dc293b771c3f 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -43,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
@@ -1029,6 +1030,7 @@ static const struct file_operations drm_stub_fops = {
 
 static void drm_core_exit(void)
 {
+   drm_privacy_screen_lookup_exit();
unregister_chrdev(DRM_MAJOR, "drm");
debugfs_remove(drm_debugfs_root);
drm_sysfs_destroy();
@@ -1056,6 +1058,8 @@ static int __init drm_core_init(void)
if (ret < 0)
goto error;
 
+   drm_privacy_screen_lookup_init();
+
drm_core_init_complete = true;
 
DRM_DEBUG("Initialized\n");
diff --git a/drivers/gpu/drm/drm_privacy_scr