[PATCH 0/9] drm: Add privacy-screen class and connector properties

Tue Jul 14 15:00:37 UTC 2020

On Thu, Jul 9, 2020 at 8:48 AM Hans de Goede <hdegoede at redhat.com> wrote:
>
> Hi,
>
> On 7/8/20 11:25 PM, Alex Deucher wrote:
> > On Wed, Jul 8, 2020 at 12:43 PM Hans de Goede <hdegoede at redhat.com> wrote:
> >>
> >> Hi All,
> >>
> >> Here is the privacy-screen related code which we discussed a while ago.
> >> 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 3-5 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 6).
> >>
> >> 4. Make the thinkpad_acpi code register a privacy-screen device on
> >> ThinkPads with a privacy-screen (patches 7-8)
> >>
> >> 5. Make the i915 driver export the privacy-screen functionality through
> >> the connector properties on the eDP connector.
> >
> > Care to create a patch 10 for amdgpu?  Lenovo sells AMD thinkpads with
> > a privacy screen as well, presumably it works
> > the same way.
>
> Yes the AMD based Thinkpads should work the same way.
>
> We will need similar changes for amdgpu and very likely also for
> nouveau. The problem is I don't really have hw to test this.
>
> Do you have access to a recent thinkpad with amdgpu ? It does not need
> to have a privacy screen, as long as it is new enough that the ACPI
> tables have the GSSS and SSSS methods you can test by ignoring
> the presence bit for the privacy-screen, I use this little change for
> that:

Thanks for the hints Hans.  If I can find some time, I will give it a try.

Alex

>
>  From 9438bababe77dfccbade5c2377bdc7d6a777a6c6 Mon Sep 17 00:00:00 2001
> From: Hans de Goede <hdegoede at redhat.com>
> Date: Wed, 27 May 2020 14:38:42 +0200
> Subject: [PATCH] platform/x86: thinkpad_acpi: Hack to allow testing
>   on devices without a privacy-screen
>
> Hack to allow testing on devices without a privacy-screen.
>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>   drivers/gpu/drm/drm_privacy_screen_x86.c | 4 ++++
>   drivers/platform/x86/thinkpad_acpi.c     | 4 ++--
>   2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_privacy_screen_x86.c b/drivers/gpu/drm/drm_privacy_screen_x86.c
> index f486d9087819..87725766a90d 100644
> --- a/drivers/gpu/drm/drm_privacy_screen_x86.c
> +++ b/drivers/gpu/drm/drm_privacy_screen_x86.c
> @@ -41,7 +41,11 @@ static bool __init detect_thinkpad_privacy_screen(void)
>         if (ACPI_FAILURE(status))
>                 return false;
>
> +#if 1
> +       return true;
> +#else
>         return (output & 0x10000) ? true : false;
> +#endif
>   }
>
>   static const struct arch_init_data arch_init_data[] __initconst = {
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 1583c18f7f77..92aad746d1f8 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -9747,8 +9747,8 @@ static int tpacpi_lcdshadow_init(struct ibm_init_struct *iibm)
>         if (!acpi_evalf(lcdshadow_get_handle, &output, NULL, "dd", 0))
>                 return -EIO;
>
> -       if (!(output & 0x10000))
> -               return 0;
> +//     if (!(output & 0x10000))
> +//             return 0;
>
>         lcdshadow_dev = drm_privacy_screen_register(&tpacpi_pdev->dev,
>                                                     &lcdshadow_ops);
> --
> 2.26.2
>
>
> So if you have a machine with an AMDGPU and with the mentioned ACPI methods,
> then you should be able to implement this yourself. You can read/write the new
> props under X11 with xrandr. And you monitor if the changes make it to the
> hardware by doing:
>
> cat /proc/acpi/ibm/lcdshadow
>
> And you can simulate external changes (like through a hotkey handled by the embedded-controller) by doing:
>
> echo 0 > /proc/acpi/ibm/lcdshadow
> echo 1 > /proc/acpi/ibm/lcdshadow
>
> When you do this you should see udev change events for the properties, you
> can test for those by doing:
>
> sudo udevadm monitor -u -p
>
> ###
>
> With all that said, I can take a shot at blindly implementing this for amdgpu
> but I would greatly prefer an actually tested patch, even if it is tested in
> the way described above. When the patch is ready you can just send it to me
> and I'll add my s-o-b and add it as patch 10 in the patch-set for the next
> version.
>
> Regards,
>
> Hans
>
>
> >> I was a bit in doubt if I should calls this series a RFC, or just call
> >> it v1, since there is no real userspace code using this yet. It was
> >> tested using xrandr property access and udevadm event monitoring.
> >> I do expect / hope we will have patches for a userspace consumer of the
> >> new properties (mutter) ready soon.
> >>
> >> But since the code is completely ready, including API documentation,
> >> I've decided to just call this v1. Hopefully we can get patches for the
> >> first userspace consumer of this ready during the review of this.
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
>