| Message ID | 20210906073519.4615-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm: Add privacy-screen class and connector properties | expand |
OK! Looked over all of these patches. Patches 2 and 4 have some comments that should be addressed, but otherwise this series is: Reviewed-by: Lyude Paul <lyude@redhat.com> Let me know when/if you need help pushing this upstream On Mon, 2021-09-06 at 09:35 +0200, Hans de Goede wrote: > Hi all, > > Here is the privacy-screen related code which I last posted in April 2021 > To the best of my knowledge there is consensus about / everyone is in > agreement with the new userspace API (2 connector properties) this > patch-set add (patch 1 of the series). > > This is unchanged (except for a rebase on drm-tip), what has changed is > that the first userspace consumer of the new properties is now fully ready > for merging (it is just waiting for the kernel bits to land first): > > - > https://gitlab.gnome.org/GNOME/gsettings-desktop-schemas/-/merge_requests/49 > - https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1952 > - https://gitlab.gnome.org/GNOME/gnome-control-center/-/merge_requests/1032 > > Having a userspace-consumer of the API fully ready for merging, clears the > last blocker for this series. It has already has been reviewed before > by Emil Velikov, but it could really do with another review. > > The new API works as designed and add the following features to GNOME: > > 1. Showing an OSD notification when the privacy-screen is toggled on/off > through hotkeys handled by the embedded-controller > 2. Allowing control of the privacy-screen from the GNOME control-panel, > including the on/off slider shown there updating to match the hw-setting > when the setting is changed with the control-panel open. > 3. Restoring the last user-setting at login > > This series consists of a number of different parts: > > 1. A new version of Rajat's privacy-screen connector properties patch, > this adds new userspace API in the form of new properties > > 2. Since on most devices the privacy screen is actually controlled by > some vendor specific ACPI/WMI interface which has a driver under > drivers/platform/x86, we need some "glue" code to make this functionality > available to KMS drivers. Patches 2-4 add a new privacy-screen class for > this, which allows non KMS drivers (and possibly KMS drivers too) to > register a privacy-screen device and also adds an interface for KMS drivers > to get access to the privacy-screen associated with a specific connector. > This is modelled similar to how we deal with e.g. PWMs and GPIOs in the > kernel, including separate includes for consumers and providers(drivers). > > 3. Some drm_connector helper functions to keep the actual changes needed > for this in individual KMS drivers as small as possible (patch 5). > > 4. Make the thinkpad_acpi code register a privacy-screen device on > ThinkPads with a privacy-screen (patches 6-8) > > 5. Make the i915 driver export the privacy-screen functionality through > the connector properties on the eDP connector. > > I believe that it would be best to merge the entire series, including > the thinkpad_acpi changes through drm-misc in one go. As the pdx86 > subsys maintainer I hereby give my ack for merging the thinkpad_acpi > changes through drm-misc. > > There is one small caveat with this series, which it is good to be > aware of. The i915 driver will now return -EPROBE_DEFER on Thinkpads > with an eprivacy screen, until the thinkpad_acpi driver is loaded. > This means that initrd generation tools will need to be updated to > include thinkpad_acpi when the i915 driver is added to the initrd. > Without this the loading of the i915 driver will be delayed to after > the switch to real rootfs. > > Regards, > > Hans > > > Hans de Goede (8): > drm: Add privacy-screen class (v3) > drm/privacy-screen: Add X86 specific arch init code > drm/privacy-screen: Add notifier support > drm/connector: Add a drm_connector privacy-screen helper functions > platform/x86: thinkpad_acpi: Add hotkey_notify_extended_hotkey() > helper > platform/x86: thinkpad_acpi: Get privacy-screen / lcdshadow ACPI > handles only once > platform/x86: thinkpad_acpi: Register a privacy-screen device > drm/i915: Add privacy-screen support > > Rajat Jain (1): > drm/connector: Add support for privacy-screen properties (v4) > > Documentation/gpu/drm-kms-helpers.rst | 15 + > Documentation/gpu/drm-kms.rst | 2 + > MAINTAINERS | 8 + > drivers/gpu/drm/Kconfig | 4 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/drm_atomic_uapi.c | 4 + > drivers/gpu/drm/drm_connector.c | 214 +++++++++ > drivers/gpu/drm/drm_drv.c | 4 + > drivers/gpu/drm/drm_privacy_screen.c | 468 +++++++++++++++++++ > drivers/gpu/drm/drm_privacy_screen_x86.c | 86 ++++ > drivers/gpu/drm/i915/display/intel_display.c | 5 + > drivers/gpu/drm/i915/display/intel_dp.c | 10 + > drivers/gpu/drm/i915/i915_pci.c | 12 + > drivers/platform/x86/Kconfig | 2 + > drivers/platform/x86/thinkpad_acpi.c | 131 ++++-- > include/drm/drm_connector.h | 56 +++ > include/drm/drm_privacy_screen_consumer.h | 65 +++ > include/drm/drm_privacy_screen_driver.h | 84 ++++ > include/drm/drm_privacy_screen_machine.h | 46 ++ > 19 files changed, 1175 insertions(+), 42 deletions(-) > create mode 100644 drivers/gpu/drm/drm_privacy_screen.c > create mode 100644 drivers/gpu/drm/drm_privacy_screen_x86.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 >
Hi, On 9/15/21 11:12 PM, Lyude Paul wrote: > OK! Looked over all of these patches. Patches 2 and 4 have some comments that > should be addressed, but otherwise this series is: > > Reviewed-by: Lyude Paul <lyude@redhat.com> Thank you! > Let me know when/if you need help pushing this upstream My plan was to just straight forward push the entire series to drm-misc-next. The only non drm bits are the drivers/platform/x86/thinkpad_acpi.c changes and I'm the drivers/platform/x86 subsys-maintainer and this plan has my blessing :) That only leaves the last patch in the series: "drm/i915: Add privacy-screen support" As something which could potentially be troublesome. I can also leave that out, while pushing the test to drm-misc-next and then push the i915 patch after a drm-misc-next merge into drm-intel-next. Jani, how would you like to handle the single i915 patch in this series? Regards, Hans > > On Mon, 2021-09-06 at 09:35 +0200, Hans de Goede wrote: >> Hi all, >> >> Here is the privacy-screen related code which I last posted in April 2021 >> To the best of my knowledge there is consensus about / everyone is in >> agreement with the new userspace API (2 connector properties) this >> patch-set add (patch 1 of the series). >> >> This is unchanged (except for a rebase on drm-tip), what has changed is >> that the first userspace consumer of the new properties is now fully ready >> for merging (it is just waiting for the kernel bits to land first): >> >> - >> https://gitlab.gnome.org/GNOME/gsettings-desktop-schemas/-/merge_requests/49 >> - https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1952 >> - https://gitlab.gnome.org/GNOME/gnome-control-center/-/merge_requests/1032 >> >> Having a userspace-consumer of the API fully ready for merging, clears the >> last blocker for this series. It has already has been reviewed before >> by Emil Velikov, but it could really do with another review. >> >> The new API works as designed and add the following features to GNOME: >> >> 1. Showing an OSD notification when the privacy-screen is toggled on/off >> through hotkeys handled by the embedded-controller >> 2. Allowing control of the privacy-screen from the GNOME control-panel, >> including the on/off slider shown there updating to match the hw-setting >> when the setting is changed with the control-panel open. >> 3. Restoring the last user-setting at login >> >> This series consists of a number of different parts: >> >> 1. A new version of Rajat's privacy-screen connector properties patch, >> this adds new userspace API in the form of new properties >> >> 2. Since on most devices the privacy screen is actually controlled by >> some vendor specific ACPI/WMI interface which has a driver under >> drivers/platform/x86, we need some "glue" code to make this functionality >> available to KMS drivers. Patches 2-4 add a new privacy-screen class for >> this, which allows non KMS drivers (and possibly KMS drivers too) to >> register a privacy-screen device and also adds an interface for KMS drivers >> to get access to the privacy-screen associated with a specific connector. >> This is modelled similar to how we deal with e.g. PWMs and GPIOs in the >> kernel, including separate includes for consumers and providers(drivers). >> >> 3. Some drm_connector helper functions to keep the actual changes needed >> for this in individual KMS drivers as small as possible (patch 5). >> >> 4. Make the thinkpad_acpi code register a privacy-screen device on >> ThinkPads with a privacy-screen (patches 6-8) >> >> 5. Make the i915 driver export the privacy-screen functionality through >> the connector properties on the eDP connector. >> >> I believe that it would be best to merge the entire series, including >> the thinkpad_acpi changes through drm-misc in one go. As the pdx86 >> subsys maintainer I hereby give my ack for merging the thinkpad_acpi >> changes through drm-misc. >> >> There is one small caveat with this series, which it is good to be >> aware of. The i915 driver will now return -EPROBE_DEFER on Thinkpads >> with an eprivacy screen, until the thinkpad_acpi driver is loaded. >> This means that initrd generation tools will need to be updated to >> include thinkpad_acpi when the i915 driver is added to the initrd. >> Without this the loading of the i915 driver will be delayed to after >> the switch to real rootfs. >> >> Regards, >> >> Hans >> >> >> Hans de Goede (8): >> drm: Add privacy-screen class (v3) >> drm/privacy-screen: Add X86 specific arch init code >> drm/privacy-screen: Add notifier support >> drm/connector: Add a drm_connector privacy-screen helper functions >> platform/x86: thinkpad_acpi: Add hotkey_notify_extended_hotkey() >> helper >> platform/x86: thinkpad_acpi: Get privacy-screen / lcdshadow ACPI >> handles only once >> platform/x86: thinkpad_acpi: Register a privacy-screen device >> drm/i915: Add privacy-screen support >> >> Rajat Jain (1): >> drm/connector: Add support for privacy-screen properties (v4) >> >> Documentation/gpu/drm-kms-helpers.rst | 15 + >> Documentation/gpu/drm-kms.rst | 2 + >> MAINTAINERS | 8 + >> drivers/gpu/drm/Kconfig | 4 + >> drivers/gpu/drm/Makefile | 1 + >> drivers/gpu/drm/drm_atomic_uapi.c | 4 + >> drivers/gpu/drm/drm_connector.c | 214 +++++++++ >> drivers/gpu/drm/drm_drv.c | 4 + >> drivers/gpu/drm/drm_privacy_screen.c | 468 +++++++++++++++++++ >> drivers/gpu/drm/drm_privacy_screen_x86.c | 86 ++++ >> drivers/gpu/drm/i915/display/intel_display.c | 5 + >> drivers/gpu/drm/i915/display/intel_dp.c | 10 + >> drivers/gpu/drm/i915/i915_pci.c | 12 + >> drivers/platform/x86/Kconfig | 2 + >> drivers/platform/x86/thinkpad_acpi.c | 131 ++++-- >> include/drm/drm_connector.h | 56 +++ >> include/drm/drm_privacy_screen_consumer.h | 65 +++ >> include/drm/drm_privacy_screen_driver.h | 84 ++++ >> include/drm/drm_privacy_screen_machine.h | 46 ++ >> 19 files changed, 1175 insertions(+), 42 deletions(-) >> create mode 100644 drivers/gpu/drm/drm_privacy_screen.c >> create mode 100644 drivers/gpu/drm/drm_privacy_screen_x86.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 >> >
On Thu, 16 Sep 2021, Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 9/15/21 11:12 PM, Lyude Paul wrote: >> OK! Looked over all of these patches. Patches 2 and 4 have some comments that >> should be addressed, but otherwise this series is: >> >> Reviewed-by: Lyude Paul <lyude@redhat.com> > > Thank you! > >> Let me know when/if you need help pushing this upstream > > My plan was to just straight forward push the entire series to > drm-misc-next. The only non drm bits are the drivers/platform/x86/thinkpad_acpi.c > changes and I'm the drivers/platform/x86 subsys-maintainer and this > plan has my blessing :) > > That only leaves the last patch in the series: > "drm/i915: Add privacy-screen support" > > As something which could potentially be troublesome. I can also > leave that out, while pushing the test to drm-misc-next and then > push the i915 patch after a drm-misc-next merge into drm-intel-next. > > Jani, how would you like to handle the single i915 patch in this > series? I think it's easiest to merge that via the same route as everything else goes. It's a fairly small patch with not that much conflict potential. That said, there are some issues in that patch I think still need addressing. I've commented on the patch. I don't mind you pushing the rest already, unless you think addressing the issues is easier by modifying the other patches (I don't think so). BR, Jani. > > Regards, > > Hans > > > > > >> >> On Mon, 2021-09-06 at 09:35 +0200, Hans de Goede wrote: >>> Hi all, >>> >>> Here is the privacy-screen related code which I last posted in April 2021 >>> To the best of my knowledge there is consensus about / everyone is in >>> agreement with the new userspace API (2 connector properties) this >>> patch-set add (patch 1 of the series). >>> >>> This is unchanged (except for a rebase on drm-tip), what has changed is >>> that the first userspace consumer of the new properties is now fully ready >>> for merging (it is just waiting for the kernel bits to land first): >>> >>> - >>> https://gitlab.gnome.org/GNOME/gsettings-desktop-schemas/-/merge_requests/49 >>> - https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1952 >>> - https://gitlab.gnome.org/GNOME/gnome-control-center/-/merge_requests/1032 >>> >>> Having a userspace-consumer of the API fully ready for merging, clears the >>> last blocker for this series. It has already has been reviewed before >>> by Emil Velikov, but it could really do with another review. >>> >>> The new API works as designed and add the following features to GNOME: >>> >>> 1. Showing an OSD notification when the privacy-screen is toggled on/off >>> through hotkeys handled by the embedded-controller >>> 2. Allowing control of the privacy-screen from the GNOME control-panel, >>> including the on/off slider shown there updating to match the hw-setting >>> when the setting is changed with the control-panel open. >>> 3. Restoring the last user-setting at login >>> >>> This series consists of a number of different parts: >>> >>> 1. A new version of Rajat's privacy-screen connector properties patch, >>> this adds new userspace API in the form of new properties >>> >>> 2. Since on most devices the privacy screen is actually controlled by >>> some vendor specific ACPI/WMI interface which has a driver under >>> drivers/platform/x86, we need some "glue" code to make this functionality >>> available to KMS drivers. Patches 2-4 add a new privacy-screen class for >>> this, which allows non KMS drivers (and possibly KMS drivers too) to >>> register a privacy-screen device and also adds an interface for KMS drivers >>> to get access to the privacy-screen associated with a specific connector. >>> This is modelled similar to how we deal with e.g. PWMs and GPIOs in the >>> kernel, including separate includes for consumers and providers(drivers). >>> >>> 3. Some drm_connector helper functions to keep the actual changes needed >>> for this in individual KMS drivers as small as possible (patch 5). >>> >>> 4. Make the thinkpad_acpi code register a privacy-screen device on >>> ThinkPads with a privacy-screen (patches 6-8) >>> >>> 5. Make the i915 driver export the privacy-screen functionality through >>> the connector properties on the eDP connector. >>> >>> I believe that it would be best to merge the entire series, including >>> the thinkpad_acpi changes through drm-misc in one go. As the pdx86 >>> subsys maintainer I hereby give my ack for merging the thinkpad_acpi >>> changes through drm-misc. >>> >>> There is one small caveat with this series, which it is good to be >>> aware of. The i915 driver will now return -EPROBE_DEFER on Thinkpads >>> with an eprivacy screen, until the thinkpad_acpi driver is loaded. >>> This means that initrd generation tools will need to be updated to >>> include thinkpad_acpi when the i915 driver is added to the initrd. >>> Without this the loading of the i915 driver will be delayed to after >>> the switch to real rootfs. >>> >>> Regards, >>> >>> Hans >>> >>> >>> Hans de Goede (8): >>> drm: Add privacy-screen class (v3) >>> drm/privacy-screen: Add X86 specific arch init code >>> drm/privacy-screen: Add notifier support >>> drm/connector: Add a drm_connector privacy-screen helper functions >>> platform/x86: thinkpad_acpi: Add hotkey_notify_extended_hotkey() >>> helper >>> platform/x86: thinkpad_acpi: Get privacy-screen / lcdshadow ACPI >>> handles only once >>> platform/x86: thinkpad_acpi: Register a privacy-screen device >>> drm/i915: Add privacy-screen support >>> >>> Rajat Jain (1): >>> drm/connector: Add support for privacy-screen properties (v4) >>> >>> Documentation/gpu/drm-kms-helpers.rst | 15 + >>> Documentation/gpu/drm-kms.rst | 2 + >>> MAINTAINERS | 8 + >>> drivers/gpu/drm/Kconfig | 4 + >>> drivers/gpu/drm/Makefile | 1 + >>> drivers/gpu/drm/drm_atomic_uapi.c | 4 + >>> drivers/gpu/drm/drm_connector.c | 214 +++++++++ >>> drivers/gpu/drm/drm_drv.c | 4 + >>> drivers/gpu/drm/drm_privacy_screen.c | 468 +++++++++++++++++++ >>> drivers/gpu/drm/drm_privacy_screen_x86.c | 86 ++++ >>> drivers/gpu/drm/i915/display/intel_display.c | 5 + >>> drivers/gpu/drm/i915/display/intel_dp.c | 10 + >>> drivers/gpu/drm/i915/i915_pci.c | 12 + >>> drivers/platform/x86/Kconfig | 2 + >>> drivers/platform/x86/thinkpad_acpi.c | 131 ++++-- >>> include/drm/drm_connector.h | 56 +++ >>> include/drm/drm_privacy_screen_consumer.h | 65 +++ >>> include/drm/drm_privacy_screen_driver.h | 84 ++++ >>> include/drm/drm_privacy_screen_machine.h | 46 ++ >>> 19 files changed, 1175 insertions(+), 42 deletions(-) >>> create mode 100644 drivers/gpu/drm/drm_privacy_screen.c >>> create mode 100644 drivers/gpu/drm/drm_privacy_screen_x86.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 >>> >> >
Hi all, Here is the privacy-screen related code which I last posted in April 2021 To the best of my knowledge there is consensus about / everyone is in agreement with the new userspace API (2 connector properties) this patch-set add (patch 1 of the series). This is unchanged (except for a rebase on drm-tip), what has changed is that the first userspace consumer of the new properties is now fully ready for merging (it is just waiting for the kernel bits to land first): - https://gitlab.gnome.org/GNOME/gsettings-desktop-schemas/-/merge_requests/49 - https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1952 - https://gitlab.gnome.org/GNOME/gnome-control-center/-/merge_requests/1032 Having a userspace-consumer of the API fully ready for merging, clears the last blocker for this series. It has already has been reviewed before by Emil Velikov, but it could really do with another review. The new API works as designed and add the following features to GNOME: 1. Showing an OSD notification when the privacy-screen is toggled on/off through hotkeys handled by the embedded-controller 2. Allowing control of the privacy-screen from the GNOME control-panel, including the on/off slider shown there updating to match the hw-setting when the setting is changed with the control-panel open. 3. Restoring the last user-setting at login This series consists of a number of different parts: 1. A new version of Rajat's privacy-screen connector properties patch, this adds new userspace API in the form of new properties 2. Since on most devices the privacy screen is actually controlled by some vendor specific ACPI/WMI interface which has a driver under drivers/platform/x86, we need some "glue" code to make this functionality available to KMS drivers. Patches 2-4 add a new privacy-screen class for this, which allows non KMS drivers (and possibly KMS drivers too) to register a privacy-screen device and also adds an interface for KMS drivers to get access to the privacy-screen associated with a specific connector. This is modelled similar to how we deal with e.g. PWMs and GPIOs in the kernel, including separate includes for consumers and providers(drivers). 3. Some drm_connector helper functions to keep the actual changes needed for this in individual KMS drivers as small as possible (patch 5). 4. Make the thinkpad_acpi code register a privacy-screen device on ThinkPads with a privacy-screen (patches 6-8) 5. Make the i915 driver export the privacy-screen functionality through the connector properties on the eDP connector. I believe that it would be best to merge the entire series, including the thinkpad_acpi changes through drm-misc in one go. As the pdx86 subsys maintainer I hereby give my ack for merging the thinkpad_acpi changes through drm-misc. There is one small caveat with this series, which it is good to be aware of. The i915 driver will now return -EPROBE_DEFER on Thinkpads with an eprivacy screen, until the thinkpad_acpi driver is loaded. This means that initrd generation tools will need to be updated to include thinkpad_acpi when the i915 driver is added to the initrd. Without this the loading of the i915 driver will be delayed to after the switch to real rootfs. Regards, Hans Hans de Goede (8): drm: Add privacy-screen class (v3) drm/privacy-screen: Add X86 specific arch init code drm/privacy-screen: Add notifier support drm/connector: Add a drm_connector privacy-screen helper functions platform/x86: thinkpad_acpi: Add hotkey_notify_extended_hotkey() helper platform/x86: thinkpad_acpi: Get privacy-screen / lcdshadow ACPI handles only once platform/x86: thinkpad_acpi: Register a privacy-screen device drm/i915: Add privacy-screen support Rajat Jain (1): drm/connector: Add support for privacy-screen properties (v4) Documentation/gpu/drm-kms-helpers.rst | 15 + Documentation/gpu/drm-kms.rst | 2 + MAINTAINERS | 8 + drivers/gpu/drm/Kconfig | 4 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_atomic_uapi.c | 4 + drivers/gpu/drm/drm_connector.c | 214 +++++++++ drivers/gpu/drm/drm_drv.c | 4 + drivers/gpu/drm/drm_privacy_screen.c | 468 +++++++++++++++++++ drivers/gpu/drm/drm_privacy_screen_x86.c | 86 ++++ drivers/gpu/drm/i915/display/intel_display.c | 5 + drivers/gpu/drm/i915/display/intel_dp.c | 10 + drivers/gpu/drm/i915/i915_pci.c | 12 + drivers/platform/x86/Kconfig | 2 + drivers/platform/x86/thinkpad_acpi.c | 131 ++++-- include/drm/drm_connector.h | 56 +++ include/drm/drm_privacy_screen_consumer.h | 65 +++ include/drm/drm_privacy_screen_driver.h | 84 ++++ include/drm/drm_privacy_screen_machine.h | 46 ++ 19 files changed, 1175 insertions(+), 42 deletions(-) create mode 100644 drivers/gpu/drm/drm_privacy_screen.c create mode 100644 drivers/gpu/drm/drm_privacy_screen_x86.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