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@xxxxxxxxxx> 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 >> >
