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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 7/8/20 11:25 PM, Alex Deucher wrote:
On Wed, Jul 8, 2020 at 12:43 PM Hans de Goede <hdegoede@xxxxxxxxxx> 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:

From 9438bababe77dfccbade5c2377bdc7d6a777a6c6 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@xxxxxxxxxx>
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@xxxxxxxxxx>
---
 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux