On Thu, 16 Sep 2021, Hans de Goede <hdegoede@xxxxxxxxxx> 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@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? 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 >>> >> > -- Jani Nikula, Intel Open Source Graphics Center
