Re: [PATCH v5 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen
On Mon, Jan 10, 2022 at 3:24 AM Hans de Goede wrote:
>
> Hi All,
>
> On 1/7/22 20:02, Rajat Jain wrote:
> > Allow a privacy screen provider to stash its private data pointer in the
> > drm_privacy_screen, and update the drm_privacy_screen_register() call to
> > accept that. Also introduce a *_get_drvdata() so that it can retrieved
> > back when needed.
> >
> > This also touches the IBM Thinkpad platform driver, the only user of
> > privacy screen today, to pass NULL for now to the updated API.
> >
> > Signed-off-by: Rajat Jain
> > Reviewed-by: Hans de Goede
>
> I've pushed this series to drm-misc-next now.
Thank you so much. I see it.
Thanks & Best Regards,
Rajat
>
> Regards,
>
> Hans
>
>
>
> > ---
> > v5: Same as v4
> > v4: Added "Reviewed-by" from Hans
> > v3: Initial version. Came up due to review comments on v2 of other patches.
> > v2: No v2
> > v1: No v1
> >
> > drivers/gpu/drm/drm_privacy_screen.c| 5 -
> > drivers/platform/x86/thinkpad_acpi.c| 2 +-
> > include/drm/drm_privacy_screen_driver.h | 13 -
> > 3 files changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_privacy_screen.c
> > b/drivers/gpu/drm/drm_privacy_screen.c
> > index beaf99e9120a..03b149cc455b 100644
> > --- a/drivers/gpu/drm/drm_privacy_screen.c
> > +++ b/drivers/gpu/drm/drm_privacy_screen.c
> > @@ -387,7 +387,8 @@ static void drm_privacy_screen_device_release(struct
> > device *dev)
> > * * An ERR_PTR(errno) on failure.
> > */
> > struct drm_privacy_screen *drm_privacy_screen_register(
> > - struct device *parent, const struct drm_privacy_screen_ops *ops)
> > + struct device *parent, const struct drm_privacy_screen_ops *ops,
> > + void *data)
> > {
> > struct drm_privacy_screen *priv;
> > int ret;
> > @@ -404,6 +405,7 @@ struct drm_privacy_screen *drm_privacy_screen_register(
> > priv->dev.parent = parent;
> > priv->dev.release = drm_privacy_screen_device_release;
> > dev_set_name(>dev, "privacy_screen-%s", dev_name(parent));
> > + priv->drvdata = data;
> > priv->ops = ops;
> >
> > priv->ops->get_hw_state(priv);
> > @@ -439,6 +441,7 @@ void drm_privacy_screen_unregister(struct
> > drm_privacy_screen *priv)
> > mutex_unlock(_privacy_screen_devs_lock);
> >
> > mutex_lock(>lock);
> > + priv->drvdata = NULL;
> > priv->ops = NULL;
> > mutex_unlock(>lock);
> >
> > diff --git a/drivers/platform/x86/thinkpad_acpi.c
> > b/drivers/platform/x86/thinkpad_acpi.c
> > index 341655d711ce..ccbfda2b0095 100644
> > --- a/drivers/platform/x86/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > @@ -9782,7 +9782,7 @@ static int tpacpi_lcdshadow_init(struct
> > ibm_init_struct *iibm)
> > return 0;
> >
> > lcdshadow_dev = drm_privacy_screen_register(_pdev->dev,
> > - _ops);
> > + _ops, NULL);
> > if (IS_ERR(lcdshadow_dev))
> > return PTR_ERR(lcdshadow_dev);
> >
> > diff --git a/include/drm/drm_privacy_screen_driver.h
> > b/include/drm/drm_privacy_screen_driver.h
> > index 24591b607675..4ef246d5706f 100644
> > --- a/include/drm/drm_privacy_screen_driver.h
> > +++ b/include/drm/drm_privacy_screen_driver.h
> > @@ -73,10 +73,21 @@ struct drm_privacy_screen {
> >* for more info.
> >*/
> > enum drm_privacy_screen_status hw_state;
> > + /**
> > + * @drvdata: Private data owned by the privacy screen provider
> > + */
> > + void *drvdata;
> > };
> >
> > +static inline
> > +void *drm_privacy_screen_get_drvdata(struct drm_privacy_screen *priv)
> > +{
> > + return priv->drvdata;
> > +}
> > +
> > struct drm_privacy_screen *drm_privacy_screen_register(
> > - struct device *parent, const struct drm_privacy_screen_ops *ops);
> > + struct device *parent, const struct drm_privacy_screen_ops *ops,
> > + void *data);
> > void drm_privacy_screen_unregister(struct drm_privacy_screen *priv);
> >
> > void drm_privacy_screen_call_notifier_chain(struct drm_privacy_screen
> > *priv);
> >
>
[PATCH v5 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen
Allow a privacy screen provider to stash its private data pointer in the
drm_privacy_screen, and update the drm_privacy_screen_register() call to
accept that. Also introduce a *_get_drvdata() so that it can retrieved
back when needed.
This also touches the IBM Thinkpad platform driver, the only user of
privacy screen today, to pass NULL for now to the updated API.
Signed-off-by: Rajat Jain
Reviewed-by: Hans de Goede
---
v5: Same as v4
v4: Added "Reviewed-by" from Hans
v3: Initial version. Came up due to review comments on v2 of other patches.
v2: No v2
v1: No v1
drivers/gpu/drm/drm_privacy_screen.c| 5 -
drivers/platform/x86/thinkpad_acpi.c| 2 +-
include/drm/drm_privacy_screen_driver.h | 13 -
3 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_privacy_screen.c
b/drivers/gpu/drm/drm_privacy_screen.c
index beaf99e9120a..03b149cc455b 100644
--- a/drivers/gpu/drm/drm_privacy_screen.c
+++ b/drivers/gpu/drm/drm_privacy_screen.c
@@ -387,7 +387,8 @@ static void drm_privacy_screen_device_release(struct device
*dev)
* * An ERR_PTR(errno) on failure.
*/
struct drm_privacy_screen *drm_privacy_screen_register(
- struct device *parent, const struct drm_privacy_screen_ops *ops)
+ struct device *parent, const struct drm_privacy_screen_ops *ops,
+ void *data)
{
struct drm_privacy_screen *priv;
int ret;
@@ -404,6 +405,7 @@ struct drm_privacy_screen *drm_privacy_screen_register(
priv->dev.parent = parent;
priv->dev.release = drm_privacy_screen_device_release;
dev_set_name(>dev, "privacy_screen-%s", dev_name(parent));
+ priv->drvdata = data;
priv->ops = ops;
priv->ops->get_hw_state(priv);
@@ -439,6 +441,7 @@ void drm_privacy_screen_unregister(struct
drm_privacy_screen *priv)
mutex_unlock(_privacy_screen_devs_lock);
mutex_lock(>lock);
+ priv->drvdata = NULL;
priv->ops = NULL;
mutex_unlock(>lock);
diff --git a/drivers/platform/x86/thinkpad_acpi.c
b/drivers/platform/x86/thinkpad_acpi.c
index 341655d711ce..ccbfda2b0095 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9782,7 +9782,7 @@ static int tpacpi_lcdshadow_init(struct ibm_init_struct
*iibm)
return 0;
lcdshadow_dev = drm_privacy_screen_register(_pdev->dev,
- _ops);
+ _ops, NULL);
if (IS_ERR(lcdshadow_dev))
return PTR_ERR(lcdshadow_dev);
diff --git a/include/drm/drm_privacy_screen_driver.h
b/include/drm/drm_privacy_screen_driver.h
index 24591b607675..4ef246d5706f 100644
--- a/include/drm/drm_privacy_screen_driver.h
+++ b/include/drm/drm_privacy_screen_driver.h
@@ -73,10 +73,21 @@ struct drm_privacy_screen {
* for more info.
*/
enum drm_privacy_screen_status hw_state;
+ /**
+* @drvdata: Private data owned by the privacy screen provider
+*/
+ void *drvdata;
};
+static inline
+void *drm_privacy_screen_get_drvdata(struct drm_privacy_screen *priv)
+{
+ return priv->drvdata;
+}
+
struct drm_privacy_screen *drm_privacy_screen_register(
- struct device *parent, const struct drm_privacy_screen_ops *ops);
+ struct device *parent, const struct drm_privacy_screen_ops *ops,
+ void *data);
void drm_privacy_screen_unregister(struct drm_privacy_screen *priv);
void drm_privacy_screen_call_notifier_chain(struct drm_privacy_screen *priv);
--
2.34.1.575.g55b058a8bb-goog
[PATCH v4 3/3] drm/privacy_screen_x86: Add entry for ChromeOS privacy-screen
Add a static entry in the x86 table, to detect and wait for
privacy-screen on some ChromeOS platforms.
Please note that this means that if CONFIG_CHROMEOS_PRIVACY_SCREEN is
enabled, and if "GOOG0010" device is found in ACPI, then the i915 probe
shall return EPROBE_DEFER until a platform driver actually registers the
privacy-screen: https://hansdegoede.livejournal.com/25948.html
Signed-off-by: Rajat Jain
---
v4: * Simplify the detect_chromeos_privacy_screen() function
* Don't change the existing print statement
v3: * Remove the pr_info() from detect_chromeos_privacy_screen(), instead
enhance the one already present in drm_privacy_screen_lookup_init()
v2: * Use #if instead of #elif
* Reorder the patches in the series.
* Rebased on drm-tip
drivers/gpu/drm/drm_privacy_screen_x86.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/drm_privacy_screen_x86.c
b/drivers/gpu/drm/drm_privacy_screen_x86.c
index a2cafb294ca6..88802cd7a1ee 100644
--- a/drivers/gpu/drm/drm_privacy_screen_x86.c
+++ b/drivers/gpu/drm/drm_privacy_screen_x86.c
@@ -47,6 +47,13 @@ static bool __init detect_thinkpad_privacy_screen(void)
}
#endif
+#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
+static bool __init detect_chromeos_privacy_screen(void)
+{
+ return acpi_dev_present("GOOG0010", NULL, -1);
+}
+#endif
+
static const struct arch_init_data arch_init_data[] __initconst = {
#if IS_ENABLED(CONFIG_THINKPAD_ACPI)
{
@@ -58,6 +65,16 @@ static const struct arch_init_data arch_init_data[]
__initconst = {
.detect = detect_thinkpad_privacy_screen,
},
#endif
+#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
+ {
+ .lookup = {
+ .dev_id = NULL,
+ .con_id = NULL,
+ .provider = "privacy_screen-GOOG0010:00",
+ },
+ .detect = detect_chromeos_privacy_screen,
+ },
+#endif
};
void __init drm_privacy_screen_lookup_init(void)
--
2.34.1.307.g9b7440fafd-goog
[PATCH v4 2/3] platform/chrome: Add driver for ChromeOS privacy-screen
This adds the ACPI driver for the ChromeOS privacy screen that is
present on some chromeos devices.
Note that ideally, we'd want this privacy screen driver to be probed
BEFORE the drm probe in order to avoid a drm probe deferral:
https://hansdegoede.livejournal.com/25948.html
In practise, I found that ACPI drivers are bound to their devices AFTER
the drm probe on chromebooks. So on chromebooks with privacy-screen,
this patch along with the other one in this series results in a probe
deferral of about 250ms for i915 driver. However, it did not result in
any user noticeable delay of splash screen in my personal experience.
In future if this probe deferral turns out to be an issue, we can
consider turning this ACPI driver into something that is probed
earlier than the drm drivers.
Signed-off-by: Rajat Jain
---
v4: Same as v3 (No changes)
v3: * Renamed everything chromeos_priv_scrn_* to chromeos_privacy_screen_*
(and added line breaks to accommodate longer names within 80 chars)
* Cleanup / Added few comments
* Use the newly added drm_privacy_screen_get_drvdata()
* Provide the cleanup function chromeos_privacy_screen_remove()
v2: * Reword the commit log
* Make the Kconfig into a tristate
* Reorder the patches in the series.
drivers/platform/chrome/Kconfig | 11 ++
drivers/platform/chrome/Makefile | 1 +
.../platform/chrome/chromeos_privacy_screen.c | 152 ++
3 files changed, 164 insertions(+)
create mode 100644 drivers/platform/chrome/chromeos_privacy_screen.c
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index ccc23d8686e8..75e93efd669f 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -243,6 +243,17 @@ config CROS_USBPD_NOTIFY
To compile this driver as a module, choose M here: the
module will be called cros_usbpd_notify.
+config CHROMEOS_PRIVACY_SCREEN
+ tristate "ChromeOS Privacy Screen support"
+ depends on ACPI
+ depends on DRM
+ select DRM_PRIVACY_SCREEN
+ help
+ This driver provides the support needed for the in-built electronic
+ privacy screen that is present on some ChromeOS devices. When enabled,
+ this should probably always be built into the kernel to avoid or
+ minimize drm probe deferral.
+
source "drivers/platform/chrome/wilco_ec/Kconfig"
endif # CHROMEOS_PLATFORMS
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index f901d2e43166..5d4be9735d9d 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -4,6 +4,7 @@
CFLAGS_cros_ec_trace.o:= -I$(src)
obj-$(CONFIG_CHROMEOS_LAPTOP) += chromeos_laptop.o
+obj-$(CONFIG_CHROMEOS_PRIVACY_SCREEN) += chromeos_privacy_screen.o
obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o
obj-$(CONFIG_CHROMEOS_TBMC)+= chromeos_tbmc.o
obj-$(CONFIG_CROS_EC) += cros_ec.o
diff --git a/drivers/platform/chrome/chromeos_privacy_screen.c
b/drivers/platform/chrome/chromeos_privacy_screen.c
new file mode 100644
index ..07c9f257c723
--- /dev/null
+++ b/drivers/platform/chrome/chromeos_privacy_screen.c
@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * ChromeOS Privacy Screen support
+ *
+ * Copyright (C) 2022 Google LLC
+ *
+ * This is the Chromeos privacy screen provider, present on certain
chromebooks,
+ * represented by a GOOG0010 device in the ACPI. This ACPI device, if present,
+ * will cause the i915 drm driver to probe defer until this driver registers
+ * the privacy-screen.
+ */
+
+#include
+#include
+
+/*
+ * The DSM (Device Specific Method) constants below are the agreed API with
+ * the firmware team, on how to control privacy screen using ACPI methods.
+ */
+#define PRIV_SCRN_DSM_REVID1 /* DSM version */
+#define PRIV_SCRN_DSM_FN_GET_STATUS1 /* Get privacy screen status */
+#define PRIV_SCRN_DSM_FN_ENABLE2 /* Enable privacy
screen */
+#define PRIV_SCRN_DSM_FN_DISABLE 3 /* Disable privacy screen */
+
+static const guid_t chromeos_privacy_screen_dsm_guid =
+ GUID_INIT(0xc7033113, 0x8720, 0x4ceb,
+ 0x90, 0x90, 0x9d, 0x52, 0xb3, 0xe5, 0x2d, 0x73);
+
+static void
+chromeos_privacy_screen_get_hw_state(struct drm_privacy_screen
+*drm_privacy_screen)
+{
+ union acpi_object *obj;
+ acpi_handle handle;
+ struct device *privacy_screen =
+ drm_privacy_screen_get_drvdata(drm_privacy_screen);
+
+ handle = acpi_device_handle(to_acpi_device(privacy_screen));
+ obj = acpi_evaluate_dsm(handle, _privacy_screen_dsm_guid,
+ PRIV_SCRN_DSM_REVID,
+ PRIV_SCRN_DSM_FN_GET_STATUS, NULL);
+ if (!obj) {
+ de
[PATCH v3 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen
Allow a privacy screen provider to stash its private data pointer in the
drm_privacy_screen, and update the drm_privacy_screen_register() call to
accept that. Also introduce a *_get_drvdata() so that it can retrieved
back when needed.
This also touches the IBM Thinkpad platform driver, the only user of
privacy screen today, to pass NULL for now to the updated API.
Signed-off-by: Rajat Jain
---
v3: Initial version. Came up due to review comments on v2 of other patches.
v2: No v2
v1: No v1
drivers/gpu/drm/drm_privacy_screen.c| 5 -
drivers/platform/x86/thinkpad_acpi.c| 2 +-
include/drm/drm_privacy_screen_driver.h | 13 -
3 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_privacy_screen.c
b/drivers/gpu/drm/drm_privacy_screen.c
index beaf99e9120a..03b149cc455b 100644
--- a/drivers/gpu/drm/drm_privacy_screen.c
+++ b/drivers/gpu/drm/drm_privacy_screen.c
@@ -387,7 +387,8 @@ static void drm_privacy_screen_device_release(struct device
*dev)
* * An ERR_PTR(errno) on failure.
*/
struct drm_privacy_screen *drm_privacy_screen_register(
- struct device *parent, const struct drm_privacy_screen_ops *ops)
+ struct device *parent, const struct drm_privacy_screen_ops *ops,
+ void *data)
{
struct drm_privacy_screen *priv;
int ret;
@@ -404,6 +405,7 @@ struct drm_privacy_screen *drm_privacy_screen_register(
priv->dev.parent = parent;
priv->dev.release = drm_privacy_screen_device_release;
dev_set_name(>dev, "privacy_screen-%s", dev_name(parent));
+ priv->drvdata = data;
priv->ops = ops;
priv->ops->get_hw_state(priv);
@@ -439,6 +441,7 @@ void drm_privacy_screen_unregister(struct
drm_privacy_screen *priv)
mutex_unlock(_privacy_screen_devs_lock);
mutex_lock(>lock);
+ priv->drvdata = NULL;
priv->ops = NULL;
mutex_unlock(>lock);
diff --git a/drivers/platform/x86/thinkpad_acpi.c
b/drivers/platform/x86/thinkpad_acpi.c
index 341655d711ce..ccbfda2b0095 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9782,7 +9782,7 @@ static int tpacpi_lcdshadow_init(struct ibm_init_struct
*iibm)
return 0;
lcdshadow_dev = drm_privacy_screen_register(_pdev->dev,
- _ops);
+ _ops, NULL);
if (IS_ERR(lcdshadow_dev))
return PTR_ERR(lcdshadow_dev);
diff --git a/include/drm/drm_privacy_screen_driver.h
b/include/drm/drm_privacy_screen_driver.h
index 24591b607675..4ef246d5706f 100644
--- a/include/drm/drm_privacy_screen_driver.h
+++ b/include/drm/drm_privacy_screen_driver.h
@@ -73,10 +73,21 @@ struct drm_privacy_screen {
* for more info.
*/
enum drm_privacy_screen_status hw_state;
+ /**
+* @drvdata: Private data owned by the privacy screen provider
+*/
+ void *drvdata;
};
+static inline
+void *drm_privacy_screen_get_drvdata(struct drm_privacy_screen *priv)
+{
+ return priv->drvdata;
+}
+
struct drm_privacy_screen *drm_privacy_screen_register(
- struct device *parent, const struct drm_privacy_screen_ops *ops);
+ struct device *parent, const struct drm_privacy_screen_ops *ops,
+ void *data);
void drm_privacy_screen_unregister(struct drm_privacy_screen *priv);
void drm_privacy_screen_call_notifier_chain(struct drm_privacy_screen *priv);
--
2.34.1.307.g9b7440fafd-goog
[PATCH 2/2] platform/chrome: Add driver for ChromeOS privacy-screen
This adds the ACPI driver for the ChromeOS privacy screen that is
present on some chromeos devices.
Note that I found that ACPI drivers are bound to their devices AFTER
the drm probe. So on chromebooks with privacy-screen, this causes a
probe deferral for i915 driver, which results in a delay of about 250ms
in my experiments. However, per my personal experience, it did not
result in any user perceived delay of splash screen
(https://hansdegoede.livejournal.com/25948.html)
In future if this probe deferral turns out to be an issue, we can
consider turning this ACPI driver into something that is probed earlier
than the drm drivers.
Signed-off-by: Rajat Jain
---
This patch is rebased on top of linux-next/master
drivers/platform/chrome/Kconfig | 10 ++
drivers/platform/chrome/Makefile | 1 +
drivers/platform/chrome/chromeos_priv_scrn.c | 133 +++
3 files changed, 144 insertions(+)
create mode 100644 drivers/platform/chrome/chromeos_priv_scrn.c
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index ccc23d8686e8..3f874bbd3d03 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -243,6 +243,16 @@ config CROS_USBPD_NOTIFY
To compile this driver as a module, choose M here: the
module will be called cros_usbpd_notify.
+config CHROMEOS_PRIVACY_SCREEN
+ bool "ChromeOS Privacy Screen support"
+ depends on ACPI
+ depends on DRM
+ default n
+ select DRM_PRIVACY_SCREEN
+ help
+ This driver provides the support needed for the in-built electronic
+ privacy screen that is present on some ChromeOS devices.
+
source "drivers/platform/chrome/wilco_ec/Kconfig"
endif # CHROMEOS_PLATFORMS
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index f901d2e43166..cfa0bb4e9e34 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -4,6 +4,7 @@
CFLAGS_cros_ec_trace.o:= -I$(src)
obj-$(CONFIG_CHROMEOS_LAPTOP) += chromeos_laptop.o
+obj-$(CONFIG_CHROMEOS_PRIVACY_SCREEN) += chromeos_priv_scrn.o
obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o
obj-$(CONFIG_CHROMEOS_TBMC)+= chromeos_tbmc.o
obj-$(CONFIG_CROS_EC) += cros_ec.o
diff --git a/drivers/platform/chrome/chromeos_priv_scrn.c
b/drivers/platform/chrome/chromeos_priv_scrn.c
new file mode 100644
index ..00536154acd6
--- /dev/null
+++ b/drivers/platform/chrome/chromeos_priv_scrn.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * chromeos_priv_scrn.c - ChromeOS Privacy Screen support
+ *
+ * Copyright (C) 2022 The Chromium OS Authors
+ *
+ */
+
+#include
+#include
+
+/*
+ * The DSM (Define Specific Method) constants below are the agreed API with
+ * the firmware team, on how to control privacy screen using ACPI methods.
+ */
+#define PRIV_SCRN_DSM_REVID1 /* DSM version */
+#define PRIV_SCRN_DSM_FN_GET_STATUS1 /* Get privacy screen status */
+#define PRIV_SCRN_DSM_FN_ENABLE2 /* Enable privacy
screen */
+#define PRIV_SCRN_DSM_FN_DISABLE 3 /* Disable privacy screen */
+
+static const guid_t chromeos_priv_scrn_dsm_guid =
+ GUID_INIT(0xc7033113, 0x8720, 0x4ceb,
+ 0x90, 0x90, 0x9d, 0x52, 0xb3, 0xe5, 0x2d, 0x73);
+
+static void
+chromeos_priv_scrn_get_hw_state(struct drm_privacy_screen *drm_priv_scrn)
+{
+ union acpi_object *obj;
+ acpi_handle handle;
+ struct device *priv_scrn = drm_priv_scrn->dev.parent;
+
+ if (!priv_scrn)
+ return;
+
+ handle = acpi_device_handle(to_acpi_device(priv_scrn));
+ obj = acpi_evaluate_dsm(handle, _priv_scrn_dsm_guid,
+ PRIV_SCRN_DSM_REVID,
+ PRIV_SCRN_DSM_FN_GET_STATUS, NULL);
+ if (!obj) {
+ dev_err(priv_scrn, "_DSM failed to get privacy-screen state\n");
+ return;
+ }
+
+ if (obj->type != ACPI_TYPE_INTEGER)
+ dev_err(priv_scrn, "Bad _DSM to get privacy-screen state\n");
+ else if (obj->integer.value == 1)
+ drm_priv_scrn->hw_state = drm_priv_scrn->sw_state =
+ PRIVACY_SCREEN_ENABLED;
+ else
+ drm_priv_scrn->hw_state = drm_priv_scrn->sw_state =
+ PRIVACY_SCREEN_DISABLED;
+
+ ACPI_FREE(obj);
+}
+
+static int
+chromeos_priv_scrn_set_sw_state(struct drm_privacy_screen *drm_priv_scrn,
+ enum drm_privacy_screen_status state)
+{
+ union acpi_object *obj = NULL;
+ acpi_handle handle;
+ struct device *priv_scrn = drm_priv_scrn->dev.parent;
+
+ if (!priv_scrn)
+ return -ENODEV;
+
+ handle = acpi_device_handle(to_acpi_de
[PATCH 1/2] drm/privacy_screen_x86: Add entry for ChromeOS privacy-screen
Add a static entry in the x86 table, to detect and wait for
privacy-screen on some ChromeOS platforms.
Please note that this means that if CONFIG_CHROMEOS_PRIVACY_SCREEN is
enabled, and if "GOOG0010" device is found in ACPI, then the i915 probe
shall return EPROBE_DEFER until a platform driver actually registers the
privacy-screen: https://hansdegoede.livejournal.com/25948.html
Signed-off-by: Rajat Jain
---
This patch is rebased on top of linux-next/master
drivers/gpu/drm/drm_privacy_screen_x86.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_privacy_screen_x86.c
b/drivers/gpu/drm/drm_privacy_screen_x86.c
index a2cafb294ca6..3728870a98e7 100644
--- a/drivers/gpu/drm/drm_privacy_screen_x86.c
+++ b/drivers/gpu/drm/drm_privacy_screen_x86.c
@@ -45,6 +45,17 @@ static bool __init detect_thinkpad_privacy_screen(void)
return (output & 0x1) ? true : false;
}
+#elif IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
+
+static bool __init detect_chromeos_privacy_screen(void)
+{
+ if (!acpi_dev_present("GOOG0010", NULL, -1))
+ return false;
+
+ pr_info("%s: Need to wait for ChromeOS privacy-screen", __func__);
+ return true;
+
+}
#endif
static const struct arch_init_data arch_init_data[] __initconst = {
@@ -57,6 +68,15 @@ static const struct arch_init_data arch_init_data[]
__initconst = {
},
.detect = detect_thinkpad_privacy_screen,
},
+#elif IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
+ {
+ .lookup = {
+ .dev_id = NULL,
+ .con_id = NULL,
+ .provider = "privacy_screen-GOOG0010:00",
+ },
+ .detect = detect_chromeos_privacy_screen,
+ },
#endif
};
--
2.34.1.173.g76aa8bc2d0-goog
Re: [PATCH v2 0/9] drm: Add privacy-screen class and connector properties
Hello DRM / GPU maintainers, On Tue, Aug 3, 2021 at 8:20 AM Marco Trevisan wrote: > > Hi Rajat, > > The merge proposals are now in place after discussing a bit more with > upstream: > > - > 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 It seems that the subject kernel patch series (privacy screen support) from Hans, has now satisfied all the requirements that were needed for it to be accepted upstream, and there aren't any open comments that need to be addressed. I was wondering when would it be applied to the upstream kernel? Thanks, Rajat > > This is all implemented and working for the wayland backend, for X11 I'm > looking at it right now, even though it seems that we don't get any > RRScreenChangeNotify on hotkey changes, and monitoring udev directly > seems overkill. Anything should be adjusted at lower levels? > > Cheers > > On lug 13 2021, at 9:25 pm, Rajat Jain wrote: > > > Hello Hans, Marco, et al, > > > > On Tue, Apr 27, 2021 at 10:03 AM Marco Trevisan > > wrote: > >> > >> Hi, > >> > >> >>> There now is GNOME userspace code using the new properties: > >> >>> https://hackmd.io/@3v1n0/rkyIy3BOw > >> >> > >> >> Thanks for working on this. > >> >> > >> >> Can these patches be submitted as merge requests against the upstream > >> >> projects? It would be nice to get some feedback from the maintainers, > >> >> and be able to easily leave some comments there as well. > >> > >> FYI, I've discussed with other uptream developers about these while > >> doing them, and afterwards on how to improve them. > >> > >> > I guess Marco was waiting for the kernel bits too land before > >> > submitting these, > >> > but I agree that it would probably be good to have these submitted > >> > now, we > >> > can mark them as WIP to avoid them getting merged before the kernel side > >> > is finalized. > >> > >> I'll submit them in the next days once I'm done with the refactor I've > >> in mind, and will notify the list. > > > > Any updates on the privacy-screen patchset? Can Hans' patchset be > > accepted upstream now? > > > > Thanks, > > > > Rajat > > > >> > >> And for sure we can keep them in WIP till the final bits aren't completed. > >> > >> Cheers > >
[PATCH v9 4/5] drm/i915: Add helper code for ACPI privacy screen
Add helper functions that can allow i915 to detect and control
an integrated privacy screen via ACPI methods. These shall be used
in the next patch.
Signed-off-by: Rajat Jain
---
v9: same as v8
v8: Initial version. formed by refactoring the previous patch 4.
print the connector name in the debug messages.
drivers/gpu/drm/i915/Makefile | 3 +-
.../drm/i915/display/intel_privacy_screen.c | 184 ++
.../drm/i915/display/intel_privacy_screen.h | 27 +++
3 files changed, 213 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 9f887a86e555d..da42389107f9c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -209,7 +209,8 @@ i915-y += \
display/intel_vga.o
i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
- display/intel_opregion.o
+ display/intel_opregion.o \
+ display/intel_privacy_screen.o
i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
diff --git a/drivers/gpu/drm/i915/display/intel_privacy_screen.c
b/drivers/gpu/drm/i915/display/intel_privacy_screen.c
new file mode 100644
index 0..66039103c821b
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Intel ACPI privacy screen code
+ *
+ * Copyright © 2020 Google Inc.
+ *
+ * This code can help detect and control an integrated EPS (electronic
+ * privacy screen) via ACPI functions. It expects an ACPI node for the
+ * drm connector device with the following elements:
+ *
+ * UUID should be "c7033113-8720-4ceb-9090-9d52b3e52d73"
+ *
+ * _ADR = ACPI address per Spec (also see intel_acpi_device_id_update())
+ * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
+ * Pages 1119 - 1123.
+ *
+ * _DSM method that will perform the following functions according to
+ * Local1 argument passed to it:
+ * - Local1 = 0 (EPS capabilities): Report EPS presence and capabilities.
+ * - Local1 = 1 (EPS State) : _DSM returns 1 if EPS is enabled, 0 otherwise.
+ * - Local1 = 2 (EPS Enable) : _DSM enables EPS
+ * - Local1 = 3 (EPS Disable): _DSM disables EPS
+ *
+ * Here is a sample ACPI node:
+ *
+ * Scope (\_SB.PCI0.GFX0) // Intel graphics device (PCI device)
+ * {
+ * Method (_DOD, 0, NotSerialized) // _DOD: Display Output Devices
+ * {
+ * Return (Package (0x01)
+ * {
+ * 0x80010400
+ * })
+ * }
+ *
+ * Device (LCD)
+ * {
+ * Name (_ADR, 0x80010400) // _ADR: Address
+ * Name (_STA, 0x0F) // _STA: Status
+ *
+ * Method (EPSP, 0, NotSerialized) // EPS Present
+ * {
+ * Return (0x01)
+ * }
+ *
+ * Method (EPSS, 0, NotSerialized) // EPS State
+ * {
+ * Local0 = \_SB.PCI0.GRXS (0xCD)
+ * Return (Local0)
+ * }
+ *
+ * Method (EPSE, 0, NotSerialized) // EPS Enable
+ * {
+ * \_SB.PCI0.STXS (0xCD)
+ * }
+ *
+ * Method (EPSD, 0, NotSerialized) // EPS Disable
+ * {
+ * \_SB.PCI0.CTXS (0xCD)
+ * }
+ *
+ * Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method
+ * {
+ * ToBuffer (Arg0, Local0)
+ * If ((Local0 == ToUUID
("c7033113-8720-4ceb-9090-9d52b3e52d73")))
+ * {
+ * ToInteger (Arg2, Local1)
+ * If ((Local1 == Zero))
+ * {
+ * Local2 = EPSP ()
+ * If ((Local2 == One))
+ * {
+ * Return (Buffer (One)
+ * {
+ * 0x0F
+ * })
+ * }
+ * }
+ *
+ * If ((Local1 == One))
+ * {
+ * Return (EPSS ())
+ * }
+ *
+ * If ((Local1 == 0x02))
+ * {
+ * EPSE ()
+ * }
+ *
+ * If ((Local1 == 0x03))
+ * {
+ * EPSD ()
+ * }
+ *
+ * Return (Buffer (One)
+ * {
+ * 0x00
+ * })
+ * }
+ *
+ * Return (Buffer (One)
+ * {
+ * 0x00
+ * })
+ * }
+ * }
+ * }
+ */
+
+#include
+
+#include "intel_privacy_screen.h"
+
+#define CONN_NAME(conn)\
+ (conn->base.kdev ? dev_name(conn->base.kdev) : "NONAME")
+
+#define CONNECTOR_DSM_REVI
[PATCH v8 4/5] drm/i915: Add helper code for ACPI privacy screen
Add helper functions that can allow i915 to detect and control
an integrated privacy screen via ACPI methods. These shall be used
in the next patch.
Signed-off-by: Rajat Jain
---
v8: Initial version. formed by refactoring the previous patch 4.
print the connector name in the debug messages.
drivers/gpu/drm/i915/Makefile | 3 +-
.../drm/i915/display/intel_privacy_screen.c | 184 ++
.../drm/i915/display/intel_privacy_screen.h | 27 +++
3 files changed, 213 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 9f887a86e555d..da42389107f9c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -209,7 +209,8 @@ i915-y += \
display/intel_vga.o
i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
- display/intel_opregion.o
+ display/intel_opregion.o \
+ display/intel_privacy_screen.o
i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
diff --git a/drivers/gpu/drm/i915/display/intel_privacy_screen.c
b/drivers/gpu/drm/i915/display/intel_privacy_screen.c
new file mode 100644
index 0..66039103c821b
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_privacy_screen.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Intel ACPI privacy screen code
+ *
+ * Copyright © 2020 Google Inc.
+ *
+ * This code can help detect and control an integrated EPS (electronic
+ * privacy screen) via ACPI functions. It expects an ACPI node for the
+ * drm connector device with the following elements:
+ *
+ * UUID should be "c7033113-8720-4ceb-9090-9d52b3e52d73"
+ *
+ * _ADR = ACPI address per Spec (also see intel_acpi_device_id_update())
+ * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
+ * Pages 1119 - 1123.
+ *
+ * _DSM method that will perform the following functions according to
+ * Local1 argument passed to it:
+ * - Local1 = 0 (EPS capabilities): Report EPS presence and capabilities.
+ * - Local1 = 1 (EPS State) : _DSM returns 1 if EPS is enabled, 0 otherwise.
+ * - Local1 = 2 (EPS Enable) : _DSM enables EPS
+ * - Local1 = 3 (EPS Disable): _DSM disables EPS
+ *
+ * Here is a sample ACPI node:
+ *
+ * Scope (\_SB.PCI0.GFX0) // Intel graphics device (PCI device)
+ * {
+ * Method (_DOD, 0, NotSerialized) // _DOD: Display Output Devices
+ * {
+ * Return (Package (0x01)
+ * {
+ * 0x80010400
+ * })
+ * }
+ *
+ * Device (LCD)
+ * {
+ * Name (_ADR, 0x80010400) // _ADR: Address
+ * Name (_STA, 0x0F) // _STA: Status
+ *
+ * Method (EPSP, 0, NotSerialized) // EPS Present
+ * {
+ * Return (0x01)
+ * }
+ *
+ * Method (EPSS, 0, NotSerialized) // EPS State
+ * {
+ * Local0 = \_SB.PCI0.GRXS (0xCD)
+ * Return (Local0)
+ * }
+ *
+ * Method (EPSE, 0, NotSerialized) // EPS Enable
+ * {
+ * \_SB.PCI0.STXS (0xCD)
+ * }
+ *
+ * Method (EPSD, 0, NotSerialized) // EPS Disable
+ * {
+ * \_SB.PCI0.CTXS (0xCD)
+ * }
+ *
+ * Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method
+ * {
+ * ToBuffer (Arg0, Local0)
+ * If ((Local0 == ToUUID
("c7033113-8720-4ceb-9090-9d52b3e52d73")))
+ * {
+ * ToInteger (Arg2, Local1)
+ * If ((Local1 == Zero))
+ * {
+ * Local2 = EPSP ()
+ * If ((Local2 == One))
+ * {
+ * Return (Buffer (One)
+ * {
+ * 0x0F
+ * })
+ * }
+ * }
+ *
+ * If ((Local1 == One))
+ * {
+ * Return (EPSS ())
+ * }
+ *
+ * If ((Local1 == 0x02))
+ * {
+ * EPSE ()
+ * }
+ *
+ * If ((Local1 == 0x03))
+ * {
+ * EPSD ()
+ * }
+ *
+ * Return (Buffer (One)
+ * {
+ * 0x00
+ * })
+ * }
+ *
+ * Return (Buffer (One)
+ * {
+ * 0x00
+ * })
+ * }
+ * }
+ * }
+ */
+
+#include
+
+#include "intel_privacy_screen.h"
+
+#define CONN_NAME(conn)\
+ (conn->base.kdev ? dev_name(conn->base.kdev) : "NONAME")
+
+#define CONNECTOR_DSM_REVID 1
+
+#define CO
Re: [PATCH v7 4/4] drm/i915: Add support for integrated privacy screen
Hi Jani, On Mon, Mar 9, 2020 at 5:18 PM Rajat Jain wrote: > > Hi Jani, > > I have 1 question / need 1 help about this patch: Kind ignore, I found the answer, and posted my new patchset here: https://patchwork.freedesktop.org/series/74607/ I got a "failed to apply" email from the patchwork. Can you please let me know on which branch is it trying to apply it? I have currently rebased my patchset to drm-intel-next-queued. Thanks & Best Regards, Rajat > > On Mon, Mar 9, 2020 at 5:06 PM Rajat Jain wrote: > > > > Add support for an ACPI based integrated privacy screen that is > > available on some systems. > > > > Signed-off-by: Rajat Jain > > --- > > v7: * Move the privacy-screen property back into drm core. > > * Do the actual HW EPS toggling at commit time. > > * Provide a sample ACPI node for reference in comments. > > v6: Always initialize prop in intel_attach_privacy_screen_property() > > v5: fix a cosmetic checkpatch warning > > v4: Fix a typo in intel_privacy_screen.h > > v3: * Change license to GPL-2.0 OR MIT > > * Move privacy screen enum from UAPI to intel_display_types.h > > * Rename parameter name and some other minor changes. > > v2: Formed by splitting the original patch into multiple patches. > > - All code has been moved into i915 now. > > - Privacy screen is a i915 property > > - Have a local state variable to store the prvacy screen. Don't read > > it from hardware. > > > > drivers/gpu/drm/i915/Makefile | 3 +- > > drivers/gpu/drm/i915/display/intel_atomic.c | 1 + > > drivers/gpu/drm/i915/display/intel_dp.c | 30 ++- > > .../drm/i915/display/intel_privacy_screen.c | 175 ++ > > .../drm/i915/display/intel_privacy_screen.h | 27 +++ > > 5 files changed, 234 insertions(+), 2 deletions(-) > > create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c > > create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h > > > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > > index 9f887a86e555d..da42389107f9c 100644 > > --- a/drivers/gpu/drm/i915/Makefile > > +++ b/drivers/gpu/drm/i915/Makefile > > @@ -209,7 +209,8 @@ i915-y += \ > > display/intel_vga.o > > i915-$(CONFIG_ACPI) += \ > > display/intel_acpi.o \ > > - display/intel_opregion.o > > + display/intel_opregion.o \ > > + display/intel_privacy_screen.o > > i915-$(CONFIG_DRM_FBDEV_EMULATION) += \ > > display/intel_fbdev.o > > > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c > > b/drivers/gpu/drm/i915/display/intel_atomic.c > > index d043057d2fa03..fc6264b4a7f73 100644 > > --- a/drivers/gpu/drm/i915/display/intel_atomic.c > > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c > > @@ -150,6 +150,7 @@ int intel_digital_connector_atomic_check(struct > > drm_connector *conn, > > new_conn_state->base.picture_aspect_ratio != > > old_conn_state->base.picture_aspect_ratio || > > new_conn_state->base.content_type != > > old_conn_state->base.content_type || > > new_conn_state->base.scaling_mode != > > old_conn_state->base.scaling_mode || > > + new_conn_state->base.privacy_screen_status != > > old_conn_state->base.privacy_screen_status || > > !blob_equal(new_conn_state->base.hdr_output_metadata, > > old_conn_state->base.hdr_output_metadata)) > > crtc_state->mode_changed = true; > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index 41c623b029464..a39b0c420b50a 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -62,6 +62,7 @@ > > #include "intel_lspcon.h" > > #include "intel_lvds.h" > > #include "intel_panel.h" > > +#include "intel_privacy_screen.h" > > #include "intel_psr.h" > > #include "intel_sideband.h" > > #include "intel_tc.h" > > @@ -5886,6 +5887,12 @@ intel_dp_connector_register(struct drm_connector > > *connector) > > dev_priv->acpi_scan_done = true; > > } > > > > + /* Check for integrated Privacy screen support */ > > + if (intel_privacy_screen_present(to_intel_connector(connector))) > > + drm_connector_attach_privacy
Re: [PATCH v7 4/4] drm/i915: Add support for integrated privacy screen
Hi Jani,
I have 1 question / need 1 help about this patch:
On Mon, Mar 9, 2020 at 5:06 PM Rajat Jain wrote:
>
> Add support for an ACPI based integrated privacy screen that is
> available on some systems.
>
> Signed-off-by: Rajat Jain
> ---
> v7: * Move the privacy-screen property back into drm core.
> * Do the actual HW EPS toggling at commit time.
> * Provide a sample ACPI node for reference in comments.
> v6: Always initialize prop in intel_attach_privacy_screen_property()
> v5: fix a cosmetic checkpatch warning
> v4: Fix a typo in intel_privacy_screen.h
> v3: * Change license to GPL-2.0 OR MIT
> * Move privacy screen enum from UAPI to intel_display_types.h
> * Rename parameter name and some other minor changes.
> v2: Formed by splitting the original patch into multiple patches.
> - All code has been moved into i915 now.
> - Privacy screen is a i915 property
> - Have a local state variable to store the prvacy screen. Don't read
> it from hardware.
>
> drivers/gpu/drm/i915/Makefile | 3 +-
> drivers/gpu/drm/i915/display/intel_atomic.c | 1 +
> drivers/gpu/drm/i915/display/intel_dp.c | 30 ++-
> .../drm/i915/display/intel_privacy_screen.c | 175 ++
> .../drm/i915/display/intel_privacy_screen.h | 27 +++
> 5 files changed, 234 insertions(+), 2 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 9f887a86e555d..da42389107f9c 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -209,7 +209,8 @@ i915-y += \
> display/intel_vga.o
> i915-$(CONFIG_ACPI) += \
> display/intel_acpi.o \
> - display/intel_opregion.o
> + display/intel_opregion.o \
> + display/intel_privacy_screen.o
> i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> display/intel_fbdev.o
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index d043057d2fa03..fc6264b4a7f73 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -150,6 +150,7 @@ int intel_digital_connector_atomic_check(struct
> drm_connector *conn,
> new_conn_state->base.picture_aspect_ratio !=
> old_conn_state->base.picture_aspect_ratio ||
> new_conn_state->base.content_type !=
> old_conn_state->base.content_type ||
> new_conn_state->base.scaling_mode !=
> old_conn_state->base.scaling_mode ||
> + new_conn_state->base.privacy_screen_status !=
> old_conn_state->base.privacy_screen_status ||
> !blob_equal(new_conn_state->base.hdr_output_metadata,
> old_conn_state->base.hdr_output_metadata))
> crtc_state->mode_changed = true;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 41c623b029464..a39b0c420b50a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -62,6 +62,7 @@
> #include "intel_lspcon.h"
> #include "intel_lvds.h"
> #include "intel_panel.h"
> +#include "intel_privacy_screen.h"
> #include "intel_psr.h"
> #include "intel_sideband.h"
> #include "intel_tc.h"
> @@ -5886,6 +5887,12 @@ intel_dp_connector_register(struct drm_connector
> *connector)
> dev_priv->acpi_scan_done = true;
> }
>
> + /* Check for integrated Privacy screen support */
> + if (intel_privacy_screen_present(to_intel_connector(connector)))
> + drm_connector_attach_privacy_screen_property(connector);
> + else
> + drm_connector_destroy_privacy_screen_property(connector);
> +
> DRM_DEBUG_KMS("registering %s bus for %s\n",
> intel_dp->aux.name, connector->kdev->kobj.name);
>
> @@ -6881,9 +6888,30 @@ intel_dp_add_properties(struct intel_dp *intel_dp,
> struct drm_connector *connect
> drm_connector_attach_scaling_mode_property(connector,
> allowed_scalers);
>
> connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
> +
> + drm_connector_create_privacy_screen_property(connector);
> }
> }
>
> +static void intel_dp_update_privacy_screen(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state,
> +
[PATCH v7 4/4] drm/i915: Add support for integrated privacy screen
Add support for an ACPI based integrated privacy screen that is
available on some systems.
Signed-off-by: Rajat Jain
---
v7: * Move the privacy-screen property back into drm core.
* Do the actual HW EPS toggling at commit time.
* Provide a sample ACPI node for reference in comments.
v6: Always initialize prop in intel_attach_privacy_screen_property()
v5: fix a cosmetic checkpatch warning
v4: Fix a typo in intel_privacy_screen.h
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
it from hardware.
drivers/gpu/drm/i915/Makefile | 3 +-
drivers/gpu/drm/i915/display/intel_atomic.c | 1 +
drivers/gpu/drm/i915/display/intel_dp.c | 30 ++-
.../drm/i915/display/intel_privacy_screen.c | 175 ++
.../drm/i915/display/intel_privacy_screen.h | 27 +++
5 files changed, 234 insertions(+), 2 deletions(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 9f887a86e555d..da42389107f9c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -209,7 +209,8 @@ i915-y += \
display/intel_vga.o
i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
- display/intel_opregion.o
+ display/intel_opregion.o \
+ display/intel_privacy_screen.o
i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
b/drivers/gpu/drm/i915/display/intel_atomic.c
index d043057d2fa03..fc6264b4a7f73 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -150,6 +150,7 @@ int intel_digital_connector_atomic_check(struct
drm_connector *conn,
new_conn_state->base.picture_aspect_ratio !=
old_conn_state->base.picture_aspect_ratio ||
new_conn_state->base.content_type !=
old_conn_state->base.content_type ||
new_conn_state->base.scaling_mode !=
old_conn_state->base.scaling_mode ||
+ new_conn_state->base.privacy_screen_status !=
old_conn_state->base.privacy_screen_status ||
!blob_equal(new_conn_state->base.hdr_output_metadata,
old_conn_state->base.hdr_output_metadata))
crtc_state->mode_changed = true;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 41c623b029464..a39b0c420b50a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -62,6 +62,7 @@
#include "intel_lspcon.h"
#include "intel_lvds.h"
#include "intel_panel.h"
+#include "intel_privacy_screen.h"
#include "intel_psr.h"
#include "intel_sideband.h"
#include "intel_tc.h"
@@ -5886,6 +5887,12 @@ intel_dp_connector_register(struct drm_connector
*connector)
dev_priv->acpi_scan_done = true;
}
+ /* Check for integrated Privacy screen support */
+ if (intel_privacy_screen_present(to_intel_connector(connector)))
+ drm_connector_attach_privacy_screen_property(connector);
+ else
+ drm_connector_destroy_privacy_screen_property(connector);
+
DRM_DEBUG_KMS("registering %s bus for %s\n",
intel_dp->aux.name, connector->kdev->kobj.name);
@@ -6881,9 +6888,30 @@ intel_dp_add_properties(struct intel_dp *intel_dp,
struct drm_connector *connect
drm_connector_attach_scaling_mode_property(connector,
allowed_scalers);
connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
+
+ drm_connector_create_privacy_screen_property(connector);
}
}
+static void intel_dp_update_privacy_screen(struct intel_encoder *encoder,
+ const struct intel_crtc_state *crtc_state,
+ const struct drm_connector_state *conn_state)
+{
+ struct drm_connector *connector = conn_state->connector;
+
+ if (connector->privacy_screen_property)
+ intel_privacy_screen_set_val(to_intel_connector(connector),
+conn_state->privacy_screen_status);
+}
+
+static void intel_dp_update_pipe(struct intel_encoder *encoder,
+const struct intel_crtc_state *crtc_state,
+const struct drm_connector_state *conn_state)
+{
+ intel_dp_upda
Re: [PATCH v6 3/3] drm/i915: Add support for integrated privacy screens
Hi Jani,
Thank you for the comments. Please see my responses inline.
On Thu, Mar 5, 2020 at 2:02 AM Jani Nikula wrote:
>
> On Wed, 04 Mar 2020, Rajat Jain wrote:
> > Certain laptops now come with panels that have integrated privacy
> > screens on them. This patch adds support for such panels by adding
> > a privacy-screen property to the intel_connector for the panel, that
> > the userspace can then use to control and check the status.
> >
> > Identifying the presence of privacy screen, and controlling it, is done
> > via ACPI _DSM methods.
> >
> > Currently, this is done only for the Intel display ports. But in future,
> > this can be done for any other ports if the hardware becomes available
> > (e.g. external monitors supporting integrated privacy screens?).
>
> I think you should add the property at the drm core level in
> drm_connector.c, not in i915, to ensure we have the same property across
> drivers. Even if, for now, you leave the acpi implementation part in
> i915.
OK, will do. In order to do that I may need to introduce driver level
hooks that i915 driver can populate and drm core can call (or may be
some functions to add privacy screen property that drm core exports
and i915 driver will call).
>
> More comments inline.
>
> >
> > Signed-off-by: Rajat Jain
> > ---
> > v6: Always initialize prop in intel_attach_privacy_screen_property()
> > v5: fix a cosmetic checkpatch warning
> > v4: Fix a typo in intel_privacy_screen.h
> > v3: * Change license to GPL-2.0 OR MIT
> > * Move privacy screen enum from UAPI to intel_display_types.h
> > * Rename parameter name and some other minor changes.
> > v2: Formed by splitting the original patch into multiple patches.
> > - All code has been moved into i915 now.
> > - Privacy screen is a i915 property
> > - Have a local state variable to store the prvacy screen. Don't read
> > it from hardware.
> >
> > drivers/gpu/drm/i915/Makefile | 3 +-
> > drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++-
> > .../gpu/drm/i915/display/intel_connector.c| 35 +
> > .../gpu/drm/i915/display/intel_connector.h| 1 +
> > .../drm/i915/display/intel_display_types.h| 18 +
> > drivers/gpu/drm/i915/display/intel_dp.c | 6 ++
> > .../drm/i915/display/intel_privacy_screen.c | 72 +++
> > .../drm/i915/display/intel_privacy_screen.h | 27 +++
> > 8 files changed, 171 insertions(+), 4 deletions(-)
> > create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> > create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 991a8c537d123..825951b4cd006 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -208,7 +208,8 @@ i915-y += \
> > display/intel_vga.o
> > i915-$(CONFIG_ACPI) += \
> > display/intel_acpi.o \
> > - display/intel_opregion.o
> > + display/intel_opregion.o \
> > + display/intel_privacy_screen.o
> > i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> > display/intel_fbdev.o
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> > b/drivers/gpu/drm/i915/display/intel_atomic.c
> > index d043057d2fa03..4ed537c87 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > @@ -40,6 +40,7 @@
> > #include "intel_global_state.h"
> > #include "intel_hdcp.h"
> > #include "intel_psr.h"
> > +#include "intel_privacy_screen.h"
> > #include "intel_sprite.h"
> >
> > /**
> > @@ -60,11 +61,14 @@ int intel_digital_connector_atomic_get_property(struct
> > drm_connector *connector,
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct intel_digital_connector_state *intel_conn_state =
> > to_intel_digital_connector_state(state);
> > + struct intel_connector *intel_connector =
> > to_intel_connector(connector);
> >
> > if (property == dev_priv->force_audio_property)
> > *val = intel_conn_state->force_audio;
> > else if (property == dev_priv->broadcast_rgb_property)
> > *val = intel_conn_state->broadcast_rgb;
> > + else if (property == intel_connector->privacy_screen_property)
> > + *val = intel_conn_state->privacy_screen_status;
> > else {
> >
[PATCH v6 3/3] drm/i915: Add support for integrated privacy screens
Certain laptops now come with panels that have integrated privacy
screens on them. This patch adds support for such panels by adding
a privacy-screen property to the intel_connector for the panel, that
the userspace can then use to control and check the status.
Identifying the presence of privacy screen, and controlling it, is done
via ACPI _DSM methods.
Currently, this is done only for the Intel display ports. But in future,
this can be done for any other ports if the hardware becomes available
(e.g. external monitors supporting integrated privacy screens?).
Signed-off-by: Rajat Jain
---
v6: Always initialize prop in intel_attach_privacy_screen_property()
v5: fix a cosmetic checkpatch warning
v4: Fix a typo in intel_privacy_screen.h
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
it from hardware.
drivers/gpu/drm/i915/Makefile | 3 +-
drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++-
.../gpu/drm/i915/display/intel_connector.c| 35 +
.../gpu/drm/i915/display/intel_connector.h| 1 +
.../drm/i915/display/intel_display_types.h| 18 +
drivers/gpu/drm/i915/display/intel_dp.c | 6 ++
.../drm/i915/display/intel_privacy_screen.c | 72 +++
.../drm/i915/display/intel_privacy_screen.h | 27 +++
8 files changed, 171 insertions(+), 4 deletions(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 991a8c537d123..825951b4cd006 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -208,7 +208,8 @@ i915-y += \
display/intel_vga.o
i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
- display/intel_opregion.o
+ display/intel_opregion.o \
+ display/intel_privacy_screen.o
i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
b/drivers/gpu/drm/i915/display/intel_atomic.c
index d043057d2fa03..4ed537c87 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -40,6 +40,7 @@
#include "intel_global_state.h"
#include "intel_hdcp.h"
#include "intel_psr.h"
+#include "intel_privacy_screen.h"
#include "intel_sprite.h"
/**
@@ -60,11 +61,14 @@ int intel_digital_connector_atomic_get_property(struct
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
else if (property == dev_priv->broadcast_rgb_property)
*val = intel_conn_state->broadcast_rgb;
+ else if (property == intel_connector->privacy_screen_property)
+ *val = intel_conn_state->privacy_screen_status;
else {
drm_dbg_atomic(_priv->drm,
"Unknown property [PROP:%d:%s]\n",
@@ -93,15 +97,18 @@ int intel_digital_connector_atomic_set_property(struct
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
if (property == dev_priv->force_audio_property) {
intel_conn_state->force_audio = val;
return 0;
- }
-
- if (property == dev_priv->broadcast_rgb_property) {
+ } else if (property == dev_priv->broadcast_rgb_property) {
intel_conn_state->broadcast_rgb = val;
return 0;
+ } else if (property == intel_connector->privacy_screen_property) {
+ intel_privacy_screen_set_val(intel_connector, val);
+ intel_conn_state->privacy_screen_status = val;
+ return 0;
}
drm_dbg_atomic(_priv->drm, "Unknown property [PROP:%d:%s]\n",
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
b/drivers/gpu/drm/i915/display/intel_connector.c
index 903e49659f561..55f80219cb269 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i9
[PATCH v6 3/3] drm/i915: Add support for integrated privacy screens
Certain laptops now come with panels that have integrated privacy
screens on them. This patch adds support for such panels by adding
a privacy-screen property to the intel_connector for the panel, that
the userspace can then use to control and check the status.
Identifying the presence of privacy screen, and controlling it, is done
via ACPI _DSM methods.
Currently, this is done only for the Intel display ports. But in future,
this can be done for any other ports if the hardware becomes available
(e.g. external monitors supporting integrated privacy screens?).
Signed-off-by: Rajat Jain
---
v6: Always initialize prop in intel_attach_privacy_screen_property()
v5: fix a cosmetic checkpatch warning
v4: Fix a typo in intel_privacy_screen.h
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
it from hardware.
drivers/gpu/drm/i915/Makefile | 3 +-
drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++-
.../gpu/drm/i915/display/intel_connector.c| 35 +
.../gpu/drm/i915/display/intel_connector.h| 1 +
.../drm/i915/display/intel_display_types.h| 18 +
drivers/gpu/drm/i915/display/intel_dp.c | 6 ++
.../drm/i915/display/intel_privacy_screen.c | 72 +++
.../drm/i915/display/intel_privacy_screen.h | 27 +++
8 files changed, 171 insertions(+), 4 deletions(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 991a8c537d123..825951b4cd006 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -208,7 +208,8 @@ i915-y += \
display/intel_vga.o
i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
- display/intel_opregion.o
+ display/intel_opregion.o \
+ display/intel_privacy_screen.o
i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
b/drivers/gpu/drm/i915/display/intel_atomic.c
index d043057d2fa03..4ed537c87 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -40,6 +40,7 @@
#include "intel_global_state.h"
#include "intel_hdcp.h"
#include "intel_psr.h"
+#include "intel_privacy_screen.h"
#include "intel_sprite.h"
/**
@@ -60,11 +61,14 @@ int intel_digital_connector_atomic_get_property(struct
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
else if (property == dev_priv->broadcast_rgb_property)
*val = intel_conn_state->broadcast_rgb;
+ else if (property == intel_connector->privacy_screen_property)
+ *val = intel_conn_state->privacy_screen_status;
else {
drm_dbg_atomic(_priv->drm,
"Unknown property [PROP:%d:%s]\n",
@@ -93,15 +97,18 @@ int intel_digital_connector_atomic_set_property(struct
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
if (property == dev_priv->force_audio_property) {
intel_conn_state->force_audio = val;
return 0;
- }
-
- if (property == dev_priv->broadcast_rgb_property) {
+ } else if (property == dev_priv->broadcast_rgb_property) {
intel_conn_state->broadcast_rgb = val;
return 0;
+ } else if (property == intel_connector->privacy_screen_property) {
+ intel_privacy_screen_set_val(intel_connector, val);
+ intel_conn_state->privacy_screen_status = val;
+ return 0;
}
drm_dbg_atomic(_priv->drm, "Unknown property [PROP:%d:%s]\n",
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
b/drivers/gpu/drm/i915/display/intel_connector.c
index 903e49659f561..55f80219cb269 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i9
Re: [External] Re: [PATCH] thinkpad_acpi: Add sysfs entry for lcdshadow feature
Hi Mark, On Thu, Feb 20, 2020 at 11:03 AM Mark Pearson wrote: > > Hi Rajat, > > > -Original Message- > > From: Rajat Jain > > Sent: Thursday, February 20, 2020 1:39 PM > > > > > > For this particular issue what is the best way to contribute and get > > > involved? We'd like to make it so ePrivacy can be used more easily from > > > Linux. I agree a more generic way of controlling it would be good. > > > I looked at the proposed patch from Rajat > > > (https://lkml.org/lkml/2019/10/22/967) - it seems like a good solution to > > > me. > > > We can help with testing that on our platforms if that would be useful. > > > > Thanks you, just so that you know, the latest patchset is at: > > https://lkml.org/lkml/2019/12/20/794 > > > > It would be great to get some additional testing if possible. I can > > send a sample ACPI (for our platform) in case it helps. > > > Sounds good - we'll definitely try this out and see how it goes. I suspect > we'll have some questions once we try it out and get more familiar. > > > > > > > I need to understand how we connect that implementation with the ACPI > > > controls we have (as I believe what we have are thinkpad specific and not > > > to > > > a drm spec; we need to confirm that). We also have the ACPI events that > > > notify if ePrivacy was changed by the hotkeys and that seems like > > > something > > > that should be done in thinkpad_acpi.c and not the drm code. > > > > > > Not sure if the two need to be connected somehow (or if handling the > > > event is actually not important and polling is acceptable)? > > > > So there was some brief discussion about this on my patches - but > > atleast on the platforms I have seen, there was no way to change the > > privacy screen out of software / kernel control. Essentially, if there > > are hotkeys, they would send an input event to the kernel, who'd send > > them to userspace, who'd use the DRM method to toggle the privacy > > screen. Thus the current version of the patch only supports > > controlling the privacy screen via set() method. The get() method just > > returns the cached value.I hope that works for you. > > > OK - on the thinkpads we have function+D as a 'hotkey' to control the > feature...and my understanding is that bypasses everything and goes straight > to the firmware. > > The changes Nitin had been working on in thinkpad_acpi.c was to make this > more Linux and friendly - provide a sysfs hook for user space to connect to > with the aim of allowing it to be configured from user space and have on > screen display when it was triggered etc. > > I'm personally not sure yet how this ties up with the DRM method - more > digging required. I'm intrigued to see if it works on our systems (sadly I > don't have anything with that feature available on my desk right now...I need > to get my hands on one) Just FYI, Here is the brief discussion we had about an interrupt mechanism to support a (hardware based) "kill switch" for the privacy screen. https://lkml.org/lkml/2019/10/25/992 Thanks, Rajat > > Thanks > Mark ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v5,3/3] drm/i915: Add support for integrated privacy screens
On Fri, Jan 24, 2020 at 4:11 PM Guenter Roeck wrote:
>
> On Fri, Dec 20, 2019 at 12:03:53PM -0800, Rajat Jain wrote:
> > Certain laptops now come with panels that have integrated privacy
> > screens on them. This patch adds support for such panels by adding
> > a privacy-screen property to the intel_connector for the panel, that
> > the userspace can then use to control and check the status.
> >
> > Identifying the presence of privacy screen, and controlling it, is done
> > via ACPI _DSM methods.
> >
> > Currently, this is done only for the Intel display ports. But in future,
> > this can be done for any other ports if the hardware becomes available
> > (e.g. external monitors supporting integrated privacy screens?).
> >
> > Signed-off-by: Rajat Jain
> > ---
> > v5: fix a cosmetic checkpatch warning
> > v4: Fix a typo in intel_privacy_screen.h
> > v3: * Change license to GPL-2.0 OR MIT
> > * Move privacy screen enum from UAPI to intel_display_types.h
> > * Rename parameter name and some other minor changes.
> > v2: Formed by splitting the original patch into multiple patches.
> > - All code has been moved into i915 now.
> > - Privacy screen is a i915 property
> > - Have a local state variable to store the prvacy screen. Don't read
> > it from hardware.
> >
> > drivers/gpu/drm/i915/Makefile | 3 +-
> > drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++-
> > .../gpu/drm/i915/display/intel_connector.c| 35 +
> > .../gpu/drm/i915/display/intel_connector.h| 1 +
> > .../drm/i915/display/intel_display_types.h| 18 +
> > drivers/gpu/drm/i915/display/intel_dp.c | 6 ++
> > .../drm/i915/display/intel_privacy_screen.c | 72 +++
> > .../drm/i915/display/intel_privacy_screen.h | 27 +++
> > 8 files changed, 171 insertions(+), 4 deletions(-)
> > create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> > create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 90dcf09f52cc..f7067c8f0407 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -197,7 +197,8 @@ i915-y += \
> > display/intel_vga.o
> > i915-$(CONFIG_ACPI) += \
> > display/intel_acpi.o \
> > - display/intel_opregion.o
> > + display/intel_opregion.o \
> > + display/intel_privacy_screen.o
> > i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> > display/intel_fbdev.o
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> > b/drivers/gpu/drm/i915/display/intel_atomic.c
> > index c2875b10adf9..c73b81c4c3f6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > @@ -37,6 +37,7 @@
> > #include "intel_atomic.h"
> > #include "intel_display_types.h"
> > #include "intel_hdcp.h"
> > +#include "intel_privacy_screen.h"
> > #include "intel_sprite.h"
> >
> > /**
> > @@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct
> > drm_connector *connector,
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct intel_digital_connector_state *intel_conn_state =
> > to_intel_digital_connector_state(state);
> > + struct intel_connector *intel_connector =
> > to_intel_connector(connector);
> >
> > if (property == dev_priv->force_audio_property)
> > *val = intel_conn_state->force_audio;
> > else if (property == dev_priv->broadcast_rgb_property)
> > *val = intel_conn_state->broadcast_rgb;
> > + else if (property == intel_connector->privacy_screen_property)
> > + *val = intel_conn_state->privacy_screen_status;
> > else {
> > DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
> >property->base.id, property->name);
> > @@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct
> > drm_connector *connector,
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct intel_digital_connector_state *intel_conn_state =
> > to_intel_digital_connector_state(state);
> > + struct intel_connector *intel_connector =
> > to_intel_connector(connector);
> >
> > if (property == dev_priv->
Re: [PATCH v5 3/3] drm/i915: Add support for integrated privacy screens
Hello Jani,
On Fri, Dec 20, 2019 at 12:04 PM Rajat Jain wrote:
>
> Certain laptops now come with panels that have integrated privacy
> screens on them. This patch adds support for such panels by adding
> a privacy-screen property to the intel_connector for the panel, that
> the userspace can then use to control and check the status.
>
> Identifying the presence of privacy screen, and controlling it, is done
> via ACPI _DSM methods.
>
> Currently, this is done only for the Intel display ports. But in future,
> this can be done for any other ports if the hardware becomes available
> (e.g. external monitors supporting integrated privacy screens?).
>
> Signed-off-by: Rajat Jain
I was wondering if you got a chance to look at this patchset. I'd
appreciate it if you could please take a look and provide your
comments.
Thanks & Best Regards,
Rajat Jain
> ---
> v5: fix a cosmetic checkpatch warning
> v4: Fix a typo in intel_privacy_screen.h
> v3: * Change license to GPL-2.0 OR MIT
> * Move privacy screen enum from UAPI to intel_display_types.h
> * Rename parameter name and some other minor changes.
> v2: Formed by splitting the original patch into multiple patches.
> - All code has been moved into i915 now.
> - Privacy screen is a i915 property
> - Have a local state variable to store the prvacy screen. Don't read
> it from hardware.
>
> drivers/gpu/drm/i915/Makefile | 3 +-
> drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++-
> .../gpu/drm/i915/display/intel_connector.c| 35 +
> .../gpu/drm/i915/display/intel_connector.h| 1 +
> .../drm/i915/display/intel_display_types.h| 18 +
> drivers/gpu/drm/i915/display/intel_dp.c | 6 ++
> .../drm/i915/display/intel_privacy_screen.c | 72 +++
> .../drm/i915/display/intel_privacy_screen.h | 27 +++
> 8 files changed, 171 insertions(+), 4 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 90dcf09f52cc..f7067c8f0407 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -197,7 +197,8 @@ i915-y += \
> display/intel_vga.o
> i915-$(CONFIG_ACPI) += \
> display/intel_acpi.o \
> - display/intel_opregion.o
> + display/intel_opregion.o \
> + display/intel_privacy_screen.o
> i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> display/intel_fbdev.o
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index c2875b10adf9..c73b81c4c3f6 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -37,6 +37,7 @@
> #include "intel_atomic.h"
> #include "intel_display_types.h"
> #include "intel_hdcp.h"
> +#include "intel_privacy_screen.h"
> #include "intel_sprite.h"
>
> /**
> @@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct
> drm_connector *connector,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_digital_connector_state *intel_conn_state =
> to_intel_digital_connector_state(state);
> + struct intel_connector *intel_connector =
> to_intel_connector(connector);
>
> if (property == dev_priv->force_audio_property)
> *val = intel_conn_state->force_audio;
> else if (property == dev_priv->broadcast_rgb_property)
> *val = intel_conn_state->broadcast_rgb;
> + else if (property == intel_connector->privacy_screen_property)
> + *val = intel_conn_state->privacy_screen_status;
> else {
> DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
> property->base.id, property->name);
> @@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct
> drm_connector *connector,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_digital_connector_state *intel_conn_state =
> to_intel_digital_connector_state(state);
> + struct intel_connector *intel_connector =
> to_intel_connector(connector);
>
> if (property == dev_priv->force_audio_property) {
> intel_conn_state->force_audio = val;
> return 0;
> - }
> -
> - if (property == dev_priv->broadcast_rgb_property) {
> + } else if (property == dev_priv->broadcast_rgb_prop
[PATCH v5 1/3] drm/i915: Move the code to populate ACPI device ID into intel_acpi
Move the code that populates the ACPI device ID for devices, into
more appripriate intel_acpi.c. This is done in preparation for more
users of this code (in next patch).
Signed-off-by: Rajat Jain
---
v5: same as v4
v4: Same as v3
v3: * Renamed the function to intel_acpi_*
* Used forward declaration for structure instead of header file inclusion.
* Fix a typo
v2: v1 doesn't exist. Found existing code in i915 driver to assign the ACPI ID
which is what I plan to re-use.
drivers/gpu/drm/i915/display/intel_acpi.c | 89 +++
drivers/gpu/drm/i915/display/intel_acpi.h | 5 ++
drivers/gpu/drm/i915/display/intel_opregion.c | 80 +
3 files changed, 98 insertions(+), 76 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c
b/drivers/gpu/drm/i915/display/intel_acpi.c
index 3456d33feb46..e21fb14d5e07 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.c
+++ b/drivers/gpu/drm/i915/display/intel_acpi.c
@@ -10,6 +10,7 @@
#include "i915_drv.h"
#include "intel_acpi.h"
+#include "intel_display_types.h"
#define INTEL_DSM_REVISION_ID 1 /* For Calpella anyway... */
#define INTEL_DSM_FN_PLATFORM_MUX_INFO 1 /* No args */
@@ -156,3 +157,91 @@ void intel_register_dsm_handler(void)
void intel_unregister_dsm_handler(void)
{
}
+
+/*
+ * ACPI Specification, Revision 5.0, Appendix B.3.2 _DOD (Enumerate All Devices
+ * Attached to the Display Adapter).
+ */
+#define ACPI_DISPLAY_INDEX_SHIFT 0
+#define ACPI_DISPLAY_INDEX_MASK(0xf << 0)
+#define ACPI_DISPLAY_PORT_ATTACHMENT_SHIFT 4
+#define ACPI_DISPLAY_PORT_ATTACHMENT_MASK (0xf << 4)
+#define ACPI_DISPLAY_TYPE_SHIFT8
+#define ACPI_DISPLAY_TYPE_MASK (0xf << 8)
+#define ACPI_DISPLAY_TYPE_OTHER(0 << 8)
+#define ACPI_DISPLAY_TYPE_VGA (1 << 8)
+#define ACPI_DISPLAY_TYPE_TV (2 << 8)
+#define ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL (3 << 8)
+#define ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL (4 << 8)
+#define ACPI_VENDOR_SPECIFIC_SHIFT 12
+#define ACPI_VENDOR_SPECIFIC_MASK (0xf << 12)
+#define ACPI_BIOS_CAN_DETECT (1 << 16)
+#define ACPI_DEPENDS_ON_VGA(1 << 17)
+#define ACPI_PIPE_ID_SHIFT 18
+#define ACPI_PIPE_ID_MASK (7 << 18)
+#define ACPI_DEVICE_ID_SCHEME (1ULL << 31)
+
+static u32 acpi_display_type(struct intel_connector *connector)
+{
+ u32 display_type;
+
+ switch (connector->base.connector_type) {
+ case DRM_MODE_CONNECTOR_VGA:
+ case DRM_MODE_CONNECTOR_DVIA:
+ display_type = ACPI_DISPLAY_TYPE_VGA;
+ break;
+ case DRM_MODE_CONNECTOR_Composite:
+ case DRM_MODE_CONNECTOR_SVIDEO:
+ case DRM_MODE_CONNECTOR_Component:
+ case DRM_MODE_CONNECTOR_9PinDIN:
+ case DRM_MODE_CONNECTOR_TV:
+ display_type = ACPI_DISPLAY_TYPE_TV;
+ break;
+ case DRM_MODE_CONNECTOR_DVII:
+ case DRM_MODE_CONNECTOR_DVID:
+ case DRM_MODE_CONNECTOR_DisplayPort:
+ case DRM_MODE_CONNECTOR_HDMIA:
+ case DRM_MODE_CONNECTOR_HDMIB:
+ display_type = ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL;
+ break;
+ case DRM_MODE_CONNECTOR_LVDS:
+ case DRM_MODE_CONNECTOR_eDP:
+ case DRM_MODE_CONNECTOR_DSI:
+ display_type = ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL;
+ break;
+ case DRM_MODE_CONNECTOR_Unknown:
+ case DRM_MODE_CONNECTOR_VIRTUAL:
+ display_type = ACPI_DISPLAY_TYPE_OTHER;
+ break;
+ default:
+ MISSING_CASE(connector->base.connector_type);
+ display_type = ACPI_DISPLAY_TYPE_OTHER;
+ break;
+ }
+
+ return display_type;
+}
+
+void intel_acpi_device_id_update(struct drm_i915_private *dev_priv)
+{
+ struct drm_device *drm_dev = _priv->drm;
+ struct intel_connector *connector;
+ struct drm_connector_list_iter conn_iter;
+ u8 display_index[16] = {};
+
+ /* Populate the ACPI IDs for all connectors for a given drm_device */
+ drm_connector_list_iter_begin(drm_dev, _iter);
+ for_each_intel_connector_iter(connector, _iter) {
+ u32 device_id, type;
+
+ device_id = acpi_display_type(connector);
+
+ /* Use display type specific display index. */
+ type = (device_id & ACPI_DISPLAY_TYPE_MASK)
+ >> ACPI_DISPLAY_TYPE_SHIFT;
+ device_id |= display_index[type]++ << ACPI_DISPLAY_INDEX_SHIFT;
+
+ connector->acpi_device_id = device_id;
+ }
+ drm_connector_list_iter_end(_iter);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_acp
[PATCH v5 3/3] drm/i915: Add support for integrated privacy screens
Certain laptops now come with panels that have integrated privacy
screens on them. This patch adds support for such panels by adding
a privacy-screen property to the intel_connector for the panel, that
the userspace can then use to control and check the status.
Identifying the presence of privacy screen, and controlling it, is done
via ACPI _DSM methods.
Currently, this is done only for the Intel display ports. But in future,
this can be done for any other ports if the hardware becomes available
(e.g. external monitors supporting integrated privacy screens?).
Signed-off-by: Rajat Jain
---
v5: fix a cosmetic checkpatch warning
v4: Fix a typo in intel_privacy_screen.h
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
it from hardware.
drivers/gpu/drm/i915/Makefile | 3 +-
drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++-
.../gpu/drm/i915/display/intel_connector.c| 35 +
.../gpu/drm/i915/display/intel_connector.h| 1 +
.../drm/i915/display/intel_display_types.h| 18 +
drivers/gpu/drm/i915/display/intel_dp.c | 6 ++
.../drm/i915/display/intel_privacy_screen.c | 72 +++
.../drm/i915/display/intel_privacy_screen.h | 27 +++
8 files changed, 171 insertions(+), 4 deletions(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 90dcf09f52cc..f7067c8f0407 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -197,7 +197,8 @@ i915-y += \
display/intel_vga.o
i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
- display/intel_opregion.o
+ display/intel_opregion.o \
+ display/intel_privacy_screen.o
i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
b/drivers/gpu/drm/i915/display/intel_atomic.c
index c2875b10adf9..c73b81c4c3f6 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -37,6 +37,7 @@
#include "intel_atomic.h"
#include "intel_display_types.h"
#include "intel_hdcp.h"
+#include "intel_privacy_screen.h"
#include "intel_sprite.h"
/**
@@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
else if (property == dev_priv->broadcast_rgb_property)
*val = intel_conn_state->broadcast_rgb;
+ else if (property == intel_connector->privacy_screen_property)
+ *val = intel_conn_state->privacy_screen_status;
else {
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
property->base.id, property->name);
@@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
if (property == dev_priv->force_audio_property) {
intel_conn_state->force_audio = val;
return 0;
- }
-
- if (property == dev_priv->broadcast_rgb_property) {
+ } else if (property == dev_priv->broadcast_rgb_property) {
intel_conn_state->broadcast_rgb = val;
return 0;
+ } else if (property == intel_connector->privacy_screen_property) {
+ intel_privacy_screen_set_val(intel_connector, val);
+ intel_conn_state->privacy_screen_status = val;
+ return 0;
}
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
b/drivers/gpu/drm/i915/display/intel_connector.c
index 1133c4e97bb4..f3e041c737de 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -296,3 +29
Re: [PATCH v2 3/3] drm/i915: Add support for integrated privacy screens
HI Jani,
On Thu, Dec 5, 2019 at 1:34 AM Rajat Jain wrote:
>
> On Wed, Nov 20, 2019 at 7:04 AM Jani Nikula
> wrote:
> >
> > On Mon, 04 Nov 2019, Rajat Jain wrote:
> > > Certain laptops now come with panels that have integrated privacy
> > > screens on them. This patch adds support for such panels by adding
> > > a privacy-screen property to the intel_connector for the panel, that
> > > the userspace can then use to control and check the status.
> > >
> > > Identifying the presence of privacy screen, and controlling it, is done
> > > via ACPI _DSM methods.
> > >
> > > Currently, this is done only for the Intel display ports. But in future,
> > > this can be done for any other ports if the hardware becomes available
> > > (e.g. external monitors supporting integrated privacy screens?).
> > >
> > > Signed-off-by: Rajat Jain
> > > Change-Id: Ic9ff07fc4a50797d2d0dfb919f11aa0821a4b548
> > > ---
> > > v2: Formed by splitting the original patch into multiple patches.
> > > - All code has been moved into i915 now.
> > > - Privacy screen is a i915 property
> > > - Have a local state variable to store the prvacy screen. Don't read
> > > it from hardware.
> > >
> > > drivers/gpu/drm/i915/Makefile | 3 +-
> > > drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++-
> > > .../gpu/drm/i915/display/intel_connector.c| 35 ++
> > > .../gpu/drm/i915/display/intel_connector.h| 1 +
> > > .../drm/i915/display/intel_display_types.h| 4 ++
> > > drivers/gpu/drm/i915/display/intel_dp.c | 5 ++
> > > .../drm/i915/display/intel_privacy_screen.c | 70 +++
> > > .../drm/i915/display/intel_privacy_screen.h | 25 +++
> > > include/uapi/drm/i915_drm.h | 14
> > > 9 files changed, 166 insertions(+), 4 deletions(-)
> > > create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> > > create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
> > >
> > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > > index 2587ea834f06..3589ebcf27bc 100644
> > > --- a/drivers/gpu/drm/i915/Makefile
> > > +++ b/drivers/gpu/drm/i915/Makefile
> > > @@ -185,7 +185,8 @@ i915-y += \
> > > display/intel_tc.o
> > > i915-$(CONFIG_ACPI) += \
> > > display/intel_acpi.o \
> > > - display/intel_opregion.o
> > > + display/intel_opregion.o \
> > > + display/intel_privacy_screen.o
> >
> > Mmmh, wonder if there'll be non-ACPI based privacy screens. I guess we
> > can sort this out then. *shrug*
> >
> > > i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> > > display/intel_fbdev.o
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> > > b/drivers/gpu/drm/i915/display/intel_atomic.c
> > > index d3fb75bb9eb1..378772d3449c 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > > @@ -37,6 +37,7 @@
> > > #include "intel_atomic.h"
> > > #include "intel_display_types.h"
> > > #include "intel_hdcp.h"
> > > +#include "intel_privacy_screen.h"
> > > #include "intel_sprite.h"
> > >
> > > /**
> > > @@ -57,11 +58,14 @@ int
> > > intel_digital_connector_atomic_get_property(struct drm_connector
> > > *connector,
> > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > struct intel_digital_connector_state *intel_conn_state =
> > > to_intel_digital_connector_state(state);
> > > + struct intel_connector *intel_connector =
> > > to_intel_connector(connector);
> > >
> > > if (property == dev_priv->force_audio_property)
> > > *val = intel_conn_state->force_audio;
> > > else if (property == dev_priv->broadcast_rgb_property)
> > > *val = intel_conn_state->broadcast_rgb;
> > > + else if (property == intel_connector->privacy_screen_property)
> > > + *val = intel_conn_state->privacy_screen_status;
> > > else {
> > > DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
> > >property->base.id, property->name);
> > > @
[PATCH RESEND v4 3/3] drm/i915: Add support for integrated privacy screens
Certain laptops now come with panels that have integrated privacy
screens on them. This patch adds support for such panels by adding
a privacy-screen property to the intel_connector for the panel, that
the userspace can then use to control and check the status.
Identifying the presence of privacy screen, and controlling it, is done
via ACPI _DSM methods.
Currently, this is done only for the Intel display ports. But in future,
this can be done for any other ports if the hardware becomes available
(e.g. external monitors supporting integrated privacy screens?).
Signed-off-by: Rajat Jain
---
v4: Fix a typo in intel_privacy_screen.h
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
it from hardware.
drivers/gpu/drm/i915/Makefile | 3 +-
drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++-
.../gpu/drm/i915/display/intel_connector.c| 35 +
.../gpu/drm/i915/display/intel_connector.h| 1 +
.../drm/i915/display/intel_display_types.h| 18 +
drivers/gpu/drm/i915/display/intel_dp.c | 6 ++
.../drm/i915/display/intel_privacy_screen.c | 72 +++
.../drm/i915/display/intel_privacy_screen.h | 26 +++
8 files changed, 170 insertions(+), 4 deletions(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 90dcf09f52cc..f7067c8f0407 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -197,7 +197,8 @@ i915-y += \
display/intel_vga.o
i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
- display/intel_opregion.o
+ display/intel_opregion.o \
+ display/intel_privacy_screen.o
i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
b/drivers/gpu/drm/i915/display/intel_atomic.c
index c2875b10adf9..c73b81c4c3f6 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -37,6 +37,7 @@
#include "intel_atomic.h"
#include "intel_display_types.h"
#include "intel_hdcp.h"
+#include "intel_privacy_screen.h"
#include "intel_sprite.h"
/**
@@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
else if (property == dev_priv->broadcast_rgb_property)
*val = intel_conn_state->broadcast_rgb;
+ else if (property == intel_connector->privacy_screen_property)
+ *val = intel_conn_state->privacy_screen_status;
else {
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
property->base.id, property->name);
@@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
if (property == dev_priv->force_audio_property) {
intel_conn_state->force_audio = val;
return 0;
- }
-
- if (property == dev_priv->broadcast_rgb_property) {
+ } else if (property == dev_priv->broadcast_rgb_property) {
intel_conn_state->broadcast_rgb = val;
return 0;
+ } else if (property == intel_connector->privacy_screen_property) {
+ intel_privacy_screen_set_val(intel_connector, val);
+ intel_conn_state->privacy_screen_status = val;
+ return 0;
}
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
b/drivers/gpu/drm/i915/display/intel_connector.c
index 1133c4e97bb4..f3e041c737de 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -296,3 +296,38 @@ intel_attach_colorspace_property(struct drm_connector
*connec
[PATCH v4 3/3] drm/i915: Add support for integrated privacy screens
Certain laptops now come with panels that have integrated privacy
screens on them. This patch adds support for such panels by adding
a privacy-screen property to the intel_connector for the panel, that
the userspace can then use to control and check the status.
Identifying the presence of privacy screen, and controlling it, is done
via ACPI _DSM methods.
Currently, this is done only for the Intel display ports. But in future,
this can be done for any other ports if the hardware becomes available
(e.g. external monitors supporting integrated privacy screens?).
Signed-off-by: Rajat Jain
---
v4: Fix a typo in intel_privacy_screen.h
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
it from hardware.
drivers/gpu/drm/i915/Makefile | 3 +-
drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++-
.../gpu/drm/i915/display/intel_connector.c| 35 +
.../gpu/drm/i915/display/intel_connector.h| 1 +
.../drm/i915/display/intel_display_types.h| 18 +
drivers/gpu/drm/i915/display/intel_dp.c | 6 ++
.../drm/i915/display/intel_privacy_screen.c | 72 +++
.../drm/i915/display/intel_privacy_screen.h | 26 +++
8 files changed, 170 insertions(+), 4 deletions(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 90dcf09f52cc..f7067c8f0407 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -197,7 +197,8 @@ i915-y += \
display/intel_vga.o
i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
- display/intel_opregion.o
+ display/intel_opregion.o \
+ display/intel_privacy_screen.o
i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
b/drivers/gpu/drm/i915/display/intel_atomic.c
index c2875b10adf9..c73b81c4c3f6 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -37,6 +37,7 @@
#include "intel_atomic.h"
#include "intel_display_types.h"
#include "intel_hdcp.h"
+#include "intel_privacy_screen.h"
#include "intel_sprite.h"
/**
@@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
else if (property == dev_priv->broadcast_rgb_property)
*val = intel_conn_state->broadcast_rgb;
+ else if (property == intel_connector->privacy_screen_property)
+ *val = intel_conn_state->privacy_screen_status;
else {
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
property->base.id, property->name);
@@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
if (property == dev_priv->force_audio_property) {
intel_conn_state->force_audio = val;
return 0;
- }
-
- if (property == dev_priv->broadcast_rgb_property) {
+ } else if (property == dev_priv->broadcast_rgb_property) {
intel_conn_state->broadcast_rgb = val;
return 0;
+ } else if (property == intel_connector->privacy_screen_property) {
+ intel_privacy_screen_set_val(intel_connector, val);
+ intel_conn_state->privacy_screen_status = val;
+ return 0;
}
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
b/drivers/gpu/drm/i915/display/intel_connector.c
index 1133c4e97bb4..f3e041c737de 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -296,3 +296,38 @@ intel_attach_colorspace_property(struct drm_connector
*connec
Re: [PATCH v3 3/3] drm/i915: Add support for integrated privacy screens
On Thu, Dec 5, 2019 at 1:34 AM Rajat Jain wrote:
>
> Certain laptops now come with panels that have integrated privacy
> screens on them. This patch adds support for such panels by adding
> a privacy-screen property to the intel_connector for the panel, that
> the userspace can then use to control and check the status.
>
> Identifying the presence of privacy screen, and controlling it, is done
> via ACPI _DSM methods.
>
> Currently, this is done only for the Intel display ports. But in future,
> this can be done for any other ports if the hardware becomes available
> (e.g. external monitors supporting integrated privacy screens?).
>
> Signed-off-by: Rajat Jain
Hi Jani,
Just checking to see if you got a chance to look at this...
Thanks,
Rajat
> ---
> v3: * Change license to GPL-2.0 OR MIT
> * Move privacy screen enum from UAPI to intel_display_types.h
> * Rename parameter name and some other minor changes.
> v2: Formed by splitting the original patch into multiple patches.
> - All code has been moved into i915 now.
> - Privacy screen is a i915 property
> - Have a local state variable to store the prvacy screen. Don't read
> it from hardware.
>
> drivers/gpu/drm/i915/Makefile | 3 +-
> drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++-
> .../gpu/drm/i915/display/intel_connector.c| 35 +
> .../gpu/drm/i915/display/intel_connector.h| 1 +
> .../drm/i915/display/intel_display_types.h| 18 +
> drivers/gpu/drm/i915/display/intel_dp.c | 6 ++
> .../drm/i915/display/intel_privacy_screen.c | 72 +++
> .../drm/i915/display/intel_privacy_screen.h | 25 +++
> 8 files changed, 169 insertions(+), 4 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 90dcf09f52cc..f7067c8f0407 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -197,7 +197,8 @@ i915-y += \
> display/intel_vga.o
> i915-$(CONFIG_ACPI) += \
> display/intel_acpi.o \
> - display/intel_opregion.o
> + display/intel_opregion.o \
> + display/intel_privacy_screen.o
> i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> display/intel_fbdev.o
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index c2875b10adf9..c73b81c4c3f6 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -37,6 +37,7 @@
> #include "intel_atomic.h"
> #include "intel_display_types.h"
> #include "intel_hdcp.h"
> +#include "intel_privacy_screen.h"
> #include "intel_sprite.h"
>
> /**
> @@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct
> drm_connector *connector,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_digital_connector_state *intel_conn_state =
> to_intel_digital_connector_state(state);
> + struct intel_connector *intel_connector =
> to_intel_connector(connector);
>
> if (property == dev_priv->force_audio_property)
> *val = intel_conn_state->force_audio;
> else if (property == dev_priv->broadcast_rgb_property)
> *val = intel_conn_state->broadcast_rgb;
> + else if (property == intel_connector->privacy_screen_property)
> + *val = intel_conn_state->privacy_screen_status;
> else {
> DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
> property->base.id, property->name);
> @@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct
> drm_connector *connector,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_digital_connector_state *intel_conn_state =
> to_intel_digital_connector_state(state);
> + struct intel_connector *intel_connector =
> to_intel_connector(connector);
>
> if (property == dev_priv->force_audio_property) {
> intel_conn_state->force_audio = val;
> return 0;
> - }
> -
> - if (property == dev_priv->broadcast_rgb_property) {
> + } else if (property == dev_priv->broadcast_rgb_property) {
> intel_conn_state->broadcast_rgb = val;
> return 0;
> + } else if (property == intel_connector->privacy_screen_property) {
> +
[PATCH v3 1/3] drm/i915: Move the code to populate ACPI device ID into intel_acpi
Move the code that populates the ACPI device ID for devices, into
more appripriate intel_acpi.c. This is done in preparation for more
users of this code (in next patch).
Signed-off-by: Rajat Jain
---
v3: * Renamed the function to intel_acpi_*
* Used forward declaration for structure instead of header file inclusion.
* Fix a typo
v2: v1 doesn't exist. Found existing code in i915 driver to assign the ACPI ID
which is what I plan to re-use.
drivers/gpu/drm/i915/display/intel_acpi.c | 89 +++
drivers/gpu/drm/i915/display/intel_acpi.h | 5 ++
drivers/gpu/drm/i915/display/intel_opregion.c | 80 +
3 files changed, 98 insertions(+), 76 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c
b/drivers/gpu/drm/i915/display/intel_acpi.c
index 3456d33feb46..e21fb14d5e07 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.c
+++ b/drivers/gpu/drm/i915/display/intel_acpi.c
@@ -10,6 +10,7 @@
#include "i915_drv.h"
#include "intel_acpi.h"
+#include "intel_display_types.h"
#define INTEL_DSM_REVISION_ID 1 /* For Calpella anyway... */
#define INTEL_DSM_FN_PLATFORM_MUX_INFO 1 /* No args */
@@ -156,3 +157,91 @@ void intel_register_dsm_handler(void)
void intel_unregister_dsm_handler(void)
{
}
+
+/*
+ * ACPI Specification, Revision 5.0, Appendix B.3.2 _DOD (Enumerate All Devices
+ * Attached to the Display Adapter).
+ */
+#define ACPI_DISPLAY_INDEX_SHIFT 0
+#define ACPI_DISPLAY_INDEX_MASK(0xf << 0)
+#define ACPI_DISPLAY_PORT_ATTACHMENT_SHIFT 4
+#define ACPI_DISPLAY_PORT_ATTACHMENT_MASK (0xf << 4)
+#define ACPI_DISPLAY_TYPE_SHIFT8
+#define ACPI_DISPLAY_TYPE_MASK (0xf << 8)
+#define ACPI_DISPLAY_TYPE_OTHER(0 << 8)
+#define ACPI_DISPLAY_TYPE_VGA (1 << 8)
+#define ACPI_DISPLAY_TYPE_TV (2 << 8)
+#define ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL (3 << 8)
+#define ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL (4 << 8)
+#define ACPI_VENDOR_SPECIFIC_SHIFT 12
+#define ACPI_VENDOR_SPECIFIC_MASK (0xf << 12)
+#define ACPI_BIOS_CAN_DETECT (1 << 16)
+#define ACPI_DEPENDS_ON_VGA(1 << 17)
+#define ACPI_PIPE_ID_SHIFT 18
+#define ACPI_PIPE_ID_MASK (7 << 18)
+#define ACPI_DEVICE_ID_SCHEME (1ULL << 31)
+
+static u32 acpi_display_type(struct intel_connector *connector)
+{
+ u32 display_type;
+
+ switch (connector->base.connector_type) {
+ case DRM_MODE_CONNECTOR_VGA:
+ case DRM_MODE_CONNECTOR_DVIA:
+ display_type = ACPI_DISPLAY_TYPE_VGA;
+ break;
+ case DRM_MODE_CONNECTOR_Composite:
+ case DRM_MODE_CONNECTOR_SVIDEO:
+ case DRM_MODE_CONNECTOR_Component:
+ case DRM_MODE_CONNECTOR_9PinDIN:
+ case DRM_MODE_CONNECTOR_TV:
+ display_type = ACPI_DISPLAY_TYPE_TV;
+ break;
+ case DRM_MODE_CONNECTOR_DVII:
+ case DRM_MODE_CONNECTOR_DVID:
+ case DRM_MODE_CONNECTOR_DisplayPort:
+ case DRM_MODE_CONNECTOR_HDMIA:
+ case DRM_MODE_CONNECTOR_HDMIB:
+ display_type = ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL;
+ break;
+ case DRM_MODE_CONNECTOR_LVDS:
+ case DRM_MODE_CONNECTOR_eDP:
+ case DRM_MODE_CONNECTOR_DSI:
+ display_type = ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL;
+ break;
+ case DRM_MODE_CONNECTOR_Unknown:
+ case DRM_MODE_CONNECTOR_VIRTUAL:
+ display_type = ACPI_DISPLAY_TYPE_OTHER;
+ break;
+ default:
+ MISSING_CASE(connector->base.connector_type);
+ display_type = ACPI_DISPLAY_TYPE_OTHER;
+ break;
+ }
+
+ return display_type;
+}
+
+void intel_acpi_device_id_update(struct drm_i915_private *dev_priv)
+{
+ struct drm_device *drm_dev = _priv->drm;
+ struct intel_connector *connector;
+ struct drm_connector_list_iter conn_iter;
+ u8 display_index[16] = {};
+
+ /* Populate the ACPI IDs for all connectors for a given drm_device */
+ drm_connector_list_iter_begin(drm_dev, _iter);
+ for_each_intel_connector_iter(connector, _iter) {
+ u32 device_id, type;
+
+ device_id = acpi_display_type(connector);
+
+ /* Use display type specific display index. */
+ type = (device_id & ACPI_DISPLAY_TYPE_MASK)
+ >> ACPI_DISPLAY_TYPE_SHIFT;
+ device_id |= display_index[type]++ << ACPI_DISPLAY_INDEX_SHIFT;
+
+ connector->acpi_device_id = device_id;
+ }
+ drm_connector_list_iter_end(_iter);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.h
b/drivers/gpu/drm/i915
Re: [PATCH v2 2/3] drm/i915: Lookup and attach ACPI device node for connectors
On Wed, Nov 20, 2019 at 6:51 AM Jani Nikula wrote:
>
> On Mon, 04 Nov 2019, Rajat Jain wrote:
> > Lookup and attach ACPI nodes for intel connectors. The lookup is done
> > in compliance with ACPI Spec 6.3
> > https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
> > (Ref: Pages 1119 - 1123).
> >
> > This can be useful for any connector specific platform properties. (This
> > will be used for privacy screen in next patch).
> >
> > Signed-off-by: Rajat Jain
> > Change-Id: I798e70714a4402554c8cd2a8e58268353f75814f
> > ---
> > v2: formed by splitting the original patch into ACPI lookup, and privacy
> > screen property. Also move it into i915 now that I found existing code
> > in i915 that can be re-used.
> >
> > drivers/gpu/drm/i915/display/intel_acpi.c | 50 +++
> > drivers/gpu/drm/i915/display/intel_acpi.h | 4 +-
> > .../drm/i915/display/intel_display_types.h| 3 ++
> > drivers/gpu/drm/i915/display/intel_dp.c | 4 ++
> > 4 files changed, 60 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c
> > b/drivers/gpu/drm/i915/display/intel_acpi.c
> > index 748d9b3125dd..0c10516430b1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_acpi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_acpi.c
> > @@ -243,3 +243,53 @@ void intel_populate_acpi_ids_for_all_connectors(struct
> > drm_device *drm_dev)
> > }
> > drm_connector_list_iter_end(_iter);
> > }
> > +
> > +/*
> > + * Ref: ACPI Spec 6.3
> > + * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
> > + * Pages 1119 - 1123 describe, what I believe, a standard way of
> > + * identifying / addressing "display panels" in the ACPI. It provides
> > + * a way for the ACPI to define devices for the display panels attached
> > + * to the system. It thus provides a way for the BIOS to export any panel
> > + * specific properties to the system via ACPI (like device trees).
> > + *
> > + * The following functions looks up the ACPI node for a connector and
> > returns
> > + * it. Technically it is independent from the i915 code, and
> > + * ideally may be called for all connectors. It is generally a good idea to
> > + * be able to attach an ACPI node to describe anything if needed. (This can
> > + * help in future for other panel specific features maybe). However, it
> > + * needs an acpi device ID which is build using an index within a
> > particular
> > + * type of port (Ref to the pages of spec mentioned above, and to code in
> > + * intel_populate_acpi_ids_for_all_connectors()). This device index
> > + * unfortunately is not available in DRM code, so currently its call is
> > + * originated from i915 driver. If in future this is useful for other
> > drivers
> > + * and we can find a generic way of getting a device index, we should move
> > this
> > + * function to drm code, maybe.
> > + */
> > +void intel_connector_lookup_acpi_node(struct intel_connector
> > *intel_connector)
>
> Nitpick, I'd expect a "lookup" function to return whatever it is looking
> up, not modify its argument.
I folded this function into the other function as you suggested below.
>
> > +{
> > + struct drm_device *drm_dev = intel_connector->base.dev;
> > + struct device *dev = _dev->pdev->dev;
> > + struct acpi_device *conn_dev;
> > + u64 conn_addr;
> > +
> > + /*
> > + * Repopulate ACPI IDs for all connectors is needed because the
> > display
> > + * index may have changed as a result of hotplugging and unplugging
> > + * connectors
> > + */
>
> I think that can only be true for DP MST. For everything else, I don't
> think so. Anyway, why are we doing it here then, depending on whether
> someone calls this function or not? If it matters, we should be doing
> this whenever there's a chance they've changed, right?
>
Actually I removed that comment now. To be really honest, my
understanding about the need to do this on every resume was only based
on the observation that this was being done on every call to
intel_opregion_resume() in addition to intel_opregion_register(). I'm
not sure if my understanding is correct, so unless the original author
of said code intel_opregion_* chimes in, I'm hesitant to change that
code. For privacy screen purposes, this works fine.
> > + intel_populate_acpi_ids_for_all_connectors(drm_dev);
> > +
> > + /* Build the _ADR to look for */
> > + conn_addr = intel_c
Re: [PATCH v2 1/3] drm/i915: Move the code to populate ACPI device ID into intel_acpi
Hi Jani,
Thanks for the review.
On Wed, Nov 20, 2019 at 6:41 AM Jani Nikula wrote:
>
> On Mon, 04 Nov 2019, Rajat Jain wrote:
> > Move the code that populates the ACPI device ID for devices, into
> > more appripriate intel_acpi.c. This is done in preparation for more
> > users of this code (in next patch).
>
> I don't think your use of the code makes sense (I'll explain in reply to
> the other patches)
OK, I'll discuss this there.
> but I could be persuaded to move the code to
> intel_acpi.c.
>
> > Signed-off-by: Rajat Jain
> > Change-Id: Ifb3bd458734985c2a78ba682e6f0a2e63e0626ca
>
> Please drop Change-Ids.
Done.
>
> > ---
> > v2: v1 doesn't exist. Found existing code in i915 driver to assign the ACPI
> > ID
> > which is what I plan to re-use.
> >
> >
> > drivers/gpu/drm/i915/display/intel_acpi.c | 87 +++
> > drivers/gpu/drm/i915/display/intel_acpi.h | 6 ++
> > drivers/gpu/drm/i915/display/intel_opregion.c | 80 +
> > 3 files changed, 97 insertions(+), 76 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c
> > b/drivers/gpu/drm/i915/display/intel_acpi.c
> > index 3456d33feb46..748d9b3125dd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_acpi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_acpi.c
> > @@ -156,3 +156,90 @@ void intel_register_dsm_handler(void)
> > void intel_unregister_dsm_handler(void)
> > {
> > }
> > +
> > +/*
> > + * ACPI Specification, Revision 5.0, Appendix B.3.2 _DOD (Enumerate All
> > Devices
> > + * Attached to the Display Adapter).
> > + */
> > +#define ACPI_DISPLAY_INDEX_SHIFT 0
> > +#define ACPI_DISPLAY_INDEX_MASK (0xf << 0)
> > +#define ACPI_DISPLAY_PORT_ATTACHMENT_SHIFT 4
> > +#define ACPI_DISPLAY_PORT_ATTACHMENT_MASK(0xf << 4)
> > +#define ACPI_DISPLAY_TYPE_SHIFT 8
> > +#define ACPI_DISPLAY_TYPE_MASK (0xf << 8)
> > +#define ACPI_DISPLAY_TYPE_OTHER (0 << 8)
> > +#define ACPI_DISPLAY_TYPE_VGA(1 << 8)
> > +#define ACPI_DISPLAY_TYPE_TV (2 << 8)
> > +#define ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL (3 << 8)
> > +#define ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL (4 << 8)
> > +#define ACPI_VENDOR_SPECIFIC_SHIFT 12
> > +#define ACPI_VENDOR_SPECIFIC_MASK(0xf << 12)
> > +#define ACPI_BIOS_CAN_DETECT (1 << 16)
> > +#define ACPI_DEPENDS_ON_VGA (1 << 17)
> > +#define ACPI_PIPE_ID_SHIFT 18
> > +#define ACPI_PIPE_ID_MASK(7 << 18)
> > +#define ACPI_DEVICE_ID_SCHEME(1ULL << 31)
> > +
> > +static u32 acpi_display_type(struct intel_connector *connector)
> > +{
> > + u32 display_type;
> > +
> > + switch (connector->base.connector_type) {
> > + case DRM_MODE_CONNECTOR_VGA:
> > + case DRM_MODE_CONNECTOR_DVIA:
> > + display_type = ACPI_DISPLAY_TYPE_VGA;
> > + break;
> > + case DRM_MODE_CONNECTOR_Composite:
> > + case DRM_MODE_CONNECTOR_SVIDEO:
> > + case DRM_MODE_CONNECTOR_Component:
> > + case DRM_MODE_CONNECTOR_9PinDIN:
> > + case DRM_MODE_CONNECTOR_TV:
> > + display_type = ACPI_DISPLAY_TYPE_TV;
> > + break;
> > + case DRM_MODE_CONNECTOR_DVII:
> > + case DRM_MODE_CONNECTOR_DVID:
> > + case DRM_MODE_CONNECTOR_DisplayPort:
> > + case DRM_MODE_CONNECTOR_HDMIA:
> > + case DRM_MODE_CONNECTOR_HDMIB:
> > + display_type = ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL;
> > + break;
> > + case DRM_MODE_CONNECTOR_LVDS:
> > + case DRM_MODE_CONNECTOR_eDP:
> > + case DRM_MODE_CONNECTOR_DSI:
> > + display_type = ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL;
> > + break;
> > + case DRM_MODE_CONNECTOR_Unknown:
> > + case DRM_MODE_CONNECTOR_VIRTUAL:
> > + display_type = ACPI_DISPLAY_TYPE_OTHER;
> > + break;
> > + default:
> > + MISSING_CASE(connector->base.connector_type);
> > + display_type = ACPI_DISPLAY_TYPE_OTHER;
> > + break;
> > + }
> > +
> > + return display_type;
> > +}
> > +
> > +void intel_populate_acpi_ids_for_all_connectors(struct drm_device *drm_dev)
>
> Plase use intel_foo_ prefix for
[PATCH v3 3/3] drm/i915: Add support for integrated privacy screens
Certain laptops now come with panels that have integrated privacy
screens on them. This patch adds support for such panels by adding
a privacy-screen property to the intel_connector for the panel, that
the userspace can then use to control and check the status.
Identifying the presence of privacy screen, and controlling it, is done
via ACPI _DSM methods.
Currently, this is done only for the Intel display ports. But in future,
this can be done for any other ports if the hardware becomes available
(e.g. external monitors supporting integrated privacy screens?).
Signed-off-by: Rajat Jain
---
v3: * Change license to GPL-2.0 OR MIT
* Move privacy screen enum from UAPI to intel_display_types.h
* Rename parameter name and some other minor changes.
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
it from hardware.
drivers/gpu/drm/i915/Makefile | 3 +-
drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++-
.../gpu/drm/i915/display/intel_connector.c| 35 +
.../gpu/drm/i915/display/intel_connector.h| 1 +
.../drm/i915/display/intel_display_types.h| 18 +
drivers/gpu/drm/i915/display/intel_dp.c | 6 ++
.../drm/i915/display/intel_privacy_screen.c | 72 +++
.../drm/i915/display/intel_privacy_screen.h | 25 +++
8 files changed, 169 insertions(+), 4 deletions(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 90dcf09f52cc..f7067c8f0407 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -197,7 +197,8 @@ i915-y += \
display/intel_vga.o
i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
- display/intel_opregion.o
+ display/intel_opregion.o \
+ display/intel_privacy_screen.o
i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
b/drivers/gpu/drm/i915/display/intel_atomic.c
index c2875b10adf9..c73b81c4c3f6 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -37,6 +37,7 @@
#include "intel_atomic.h"
#include "intel_display_types.h"
#include "intel_hdcp.h"
+#include "intel_privacy_screen.h"
#include "intel_sprite.h"
/**
@@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
else if (property == dev_priv->broadcast_rgb_property)
*val = intel_conn_state->broadcast_rgb;
+ else if (property == intel_connector->privacy_screen_property)
+ *val = intel_conn_state->privacy_screen_status;
else {
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
property->base.id, property->name);
@@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
if (property == dev_priv->force_audio_property) {
intel_conn_state->force_audio = val;
return 0;
- }
-
- if (property == dev_priv->broadcast_rgb_property) {
+ } else if (property == dev_priv->broadcast_rgb_property) {
intel_conn_state->broadcast_rgb = val;
return 0;
+ } else if (property == intel_connector->privacy_screen_property) {
+ intel_privacy_screen_set_val(intel_connector, val);
+ intel_conn_state->privacy_screen_status = val;
+ return 0;
}
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
b/drivers/gpu/drm/i915/display/intel_connector.c
index 1133c4e97bb4..f3e041c737de 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -296,3 +296,38 @@ intel_attach_colorspace_property(struct drm_connector
*connector)
drm_object_attach_property(>bas
[PATCH v3 2/3] drm/i915: Lookup and attach ACPI device node for connectors
Lookup and attach ACPI nodes for intel connectors. The lookup is done
in compliance with ACPI Spec 6.3
https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
(Ref: Pages 1119 - 1123).
This can be useful for any connector specific platform properties. (This
will be used for privacy screen in next patch).
Signed-off-by: Rajat Jain
---
v3: fold the code into existing acpi_device_id_update() function
v2: formed by splitting the original patch into ACPI lookup, and privacy
screen property. Also move it into i915 now that I found existing code
in i915 that can be re-used.
drivers/gpu/drm/i915/display/intel_acpi.c | 24 +++
.../drm/i915/display/intel_display_types.h| 3 +++
drivers/gpu/drm/i915/display/intel_dp.c | 3 +++
3 files changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c
b/drivers/gpu/drm/i915/display/intel_acpi.c
index e21fb14d5e07..101a56c08996 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.c
+++ b/drivers/gpu/drm/i915/display/intel_acpi.c
@@ -222,11 +222,23 @@ static u32 acpi_display_type(struct intel_connector
*connector)
return display_type;
}
+/*
+ * Ref: ACPI Spec 6.3
+ * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
+ * Pages 1119 - 1123 describe, what I believe, a standard way of
+ * identifying / addressing "display panels" in the ACPI. It provides
+ * a way for the ACPI to define devices for the display panels attached
+ * to the system. It thus provides a way for the BIOS to export any panel
+ * specific properties to the system via ACPI (like device trees).
+ */
void intel_acpi_device_id_update(struct drm_i915_private *dev_priv)
{
struct drm_device *drm_dev = _priv->drm;
struct intel_connector *connector;
struct drm_connector_list_iter conn_iter;
+ struct device *dev = _dev->pdev->dev;
+ struct acpi_device *conn_dev;
+ u64 conn_addr;
u8 display_index[16] = {};
/* Populate the ACPI IDs for all connectors for a given drm_device */
@@ -242,6 +254,18 @@ void intel_acpi_device_id_update(struct drm_i915_private
*dev_priv)
device_id |= display_index[type]++ << ACPI_DISPLAY_INDEX_SHIFT;
connector->acpi_device_id = device_id;
+
+ /* Build the _ADR to look for */
+ conn_addr = device_id | ACPI_DEVICE_ID_SCHEME |
+ ACPI_BIOS_CAN_DETECT;
+
+ DRM_DEV_INFO(dev, "Checking connector ACPI node at _ADR=%llX\n",
+conn_addr);
+
+ /* Look up the connector device, under the PCI device */
+ conn_dev = acpi_find_child_device(ACPI_COMPANION(dev),
+ conn_addr, false);
+ connector->acpi_handle = conn_dev ? conn_dev->handle : NULL;
}
drm_connector_list_iter_end(_iter);
}
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 1a7334dbe802..0a4a04116091 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -407,6 +407,9 @@ struct intel_connector {
/* ACPI device id for ACPI and driver cooperation */
u32 acpi_device_id;
+ /* ACPI handle corresponding to this connector display, if found */
+ void *acpi_handle;
+
/* Reads out the current hw, returning true if the connector is enabled
* and active (i.e. dpms ON state). */
bool (*get_hw_state)(struct intel_connector *);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index c61ac0c3acb5..6b209193cbbb 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -45,6 +45,7 @@
#include "i915_debugfs.h"
#include "i915_drv.h"
#include "i915_trace.h"
+#include "intel_acpi.h"
#include "intel_atomic.h"
#include "intel_audio.h"
#include "intel_connector.h"
@@ -6628,6 +6629,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct
drm_connector *connect
connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
+ /* Lookup the ACPI node corresponding to the connector */
+ intel_acpi_device_id_update(dev_priv);
}
}
--
2.24.0.393.g34dc348eaf-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/3] drm/i915: Add support for integrated privacy screens
On Wed, Nov 20, 2019 at 7:04 AM Jani Nikula wrote:
>
> On Mon, 04 Nov 2019, Rajat Jain wrote:
> > Certain laptops now come with panels that have integrated privacy
> > screens on them. This patch adds support for such panels by adding
> > a privacy-screen property to the intel_connector for the panel, that
> > the userspace can then use to control and check the status.
> >
> > Identifying the presence of privacy screen, and controlling it, is done
> > via ACPI _DSM methods.
> >
> > Currently, this is done only for the Intel display ports. But in future,
> > this can be done for any other ports if the hardware becomes available
> > (e.g. external monitors supporting integrated privacy screens?).
> >
> > Signed-off-by: Rajat Jain
> > Change-Id: Ic9ff07fc4a50797d2d0dfb919f11aa0821a4b548
> > ---
> > v2: Formed by splitting the original patch into multiple patches.
> > - All code has been moved into i915 now.
> > - Privacy screen is a i915 property
> > - Have a local state variable to store the prvacy screen. Don't read
> > it from hardware.
> >
> > drivers/gpu/drm/i915/Makefile | 3 +-
> > drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++-
> > .../gpu/drm/i915/display/intel_connector.c| 35 ++
> > .../gpu/drm/i915/display/intel_connector.h| 1 +
> > .../drm/i915/display/intel_display_types.h| 4 ++
> > drivers/gpu/drm/i915/display/intel_dp.c | 5 ++
> > .../drm/i915/display/intel_privacy_screen.c | 70 +++
> > .../drm/i915/display/intel_privacy_screen.h | 25 +++
> > include/uapi/drm/i915_drm.h | 14
> > 9 files changed, 166 insertions(+), 4 deletions(-)
> > create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> > create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 2587ea834f06..3589ebcf27bc 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -185,7 +185,8 @@ i915-y += \
> > display/intel_tc.o
> > i915-$(CONFIG_ACPI) += \
> > display/intel_acpi.o \
> > - display/intel_opregion.o
> > + display/intel_opregion.o \
> > + display/intel_privacy_screen.o
>
> Mmmh, wonder if there'll be non-ACPI based privacy screens. I guess we
> can sort this out then. *shrug*
>
> > i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> > display/intel_fbdev.o
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> > b/drivers/gpu/drm/i915/display/intel_atomic.c
> > index d3fb75bb9eb1..378772d3449c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > @@ -37,6 +37,7 @@
> > #include "intel_atomic.h"
> > #include "intel_display_types.h"
> > #include "intel_hdcp.h"
> > +#include "intel_privacy_screen.h"
> > #include "intel_sprite.h"
> >
> > /**
> > @@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct
> > drm_connector *connector,
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct intel_digital_connector_state *intel_conn_state =
> > to_intel_digital_connector_state(state);
> > + struct intel_connector *intel_connector =
> > to_intel_connector(connector);
> >
> > if (property == dev_priv->force_audio_property)
> > *val = intel_conn_state->force_audio;
> > else if (property == dev_priv->broadcast_rgb_property)
> > *val = intel_conn_state->broadcast_rgb;
> > + else if (property == intel_connector->privacy_screen_property)
> > + *val = intel_conn_state->privacy_screen_status;
> > else {
> > DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
> >property->base.id, property->name);
> > @@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct
> > drm_connector *connector,
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct intel_digital_connector_state *intel_conn_state =
> > to_intel_digital_connector_state(state);
> > + struct intel_connector *intel_connector =
> > to_intel_connector(connector);
> >
> > if (property == dev_priv->force_audio_property) {
> > intel_conn_state
Re: [PATCH v2 3/3] drm/i915: Add support for integrated privacy screens
Hi Jani, On Wed, Nov 20, 2019 at 7:11 AM Jani Nikula wrote: > > On Tue, 12 Nov 2019, Rajat Jain wrote: > > On Mon, Nov 4, 2019 at 11:41 AM Rajat Jain wrote: > >> > >> Certain laptops now come with panels that have integrated privacy > >> screens on them. This patch adds support for such panels by adding > >> a privacy-screen property to the intel_connector for the panel, that > >> the userspace can then use to control and check the status. > >> > >> Identifying the presence of privacy screen, and controlling it, is done > >> via ACPI _DSM methods. > >> > >> Currently, this is done only for the Intel display ports. But in future, > >> this can be done for any other ports if the hardware becomes available > >> (e.g. external monitors supporting integrated privacy screens?). > >> > >> Signed-off-by: Rajat Jain > >> Change-Id: Ic9ff07fc4a50797d2d0dfb919f11aa0821a4b548 > > > > > > Hi Folks, > > > > I posted a v2 taking care of the comments I received (also split it > > into 3 patches now, and resused some ACPI code I found in i915 > > driver). . Wondering if any one got a chance to look at this? > > For future reference, please post the updated series standalone, *not* > in reply to long, old threads. Besides myself, it'll also help our CI > find your patches and crunch a bunch of tests on them. Will do. > > Also, do you have an open userspace for this? See [1]. I think this > looks like good stuff to me, but then I'm not responsible for any > userspace component that would actually use this. Not sure what you meant but the user for this on Chromebooks (what I work on) would be the Chrome browser most likely. Thanks & Best Regards, Rajat > > BR, > Jani. > > > [1] > https://www.kernel.org/doc/html/latest/gpu/drm-uapi.html#open-source-userspace-requirements > > > > > > > Thanks, > > > > Rajat > > > >> --- > >> v2: Formed by splitting the original patch into multiple patches. > >> - All code has been moved into i915 now. > >> - Privacy screen is a i915 property > >> - Have a local state variable to store the prvacy screen. Don't read > >> it from hardware. > >> > >> drivers/gpu/drm/i915/Makefile | 3 +- > >> drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++- > >> .../gpu/drm/i915/display/intel_connector.c| 35 ++ > >> .../gpu/drm/i915/display/intel_connector.h| 1 + > >> .../drm/i915/display/intel_display_types.h| 4 ++ > >> drivers/gpu/drm/i915/display/intel_dp.c | 5 ++ > >> .../drm/i915/display/intel_privacy_screen.c | 70 +++ > >> .../drm/i915/display/intel_privacy_screen.h | 25 +++ > >> include/uapi/drm/i915_drm.h | 14 > >> 9 files changed, 166 insertions(+), 4 deletions(-) > >> create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c > >> create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h > >> > >> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > >> index 2587ea834f06..3589ebcf27bc 100644 > >> --- a/drivers/gpu/drm/i915/Makefile > >> +++ b/drivers/gpu/drm/i915/Makefile > >> @@ -185,7 +185,8 @@ i915-y += \ > >> display/intel_tc.o > >> i915-$(CONFIG_ACPI) += \ > >> display/intel_acpi.o \ > >> - display/intel_opregion.o > >> + display/intel_opregion.o \ > >> + display/intel_privacy_screen.o > >> i915-$(CONFIG_DRM_FBDEV_EMULATION) += \ > >> display/intel_fbdev.o > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c > >> b/drivers/gpu/drm/i915/display/intel_atomic.c > >> index d3fb75bb9eb1..378772d3449c 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_atomic.c > >> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c > >> @@ -37,6 +37,7 @@ > >> #include "intel_atomic.h" > >> #include "intel_display_types.h" > >> #include "intel_hdcp.h" > >> +#include "intel_privacy_screen.h" > >> #include "intel_sprite.h" > >> > >> /** > >> @@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct > >> drm_connector *connector, > >> struct drm_i915_private *dev_priv = to_i915(dev); > >> struct intel_digital_connector_state *intel_conn_state = > >>
Re: [PATCH v2 3/3] drm/i915: Add support for integrated privacy screens
On Mon, Nov 4, 2019 at 11:41 AM Rajat Jain wrote:
>
> Certain laptops now come with panels that have integrated privacy
> screens on them. This patch adds support for such panels by adding
> a privacy-screen property to the intel_connector for the panel, that
> the userspace can then use to control and check the status.
>
> Identifying the presence of privacy screen, and controlling it, is done
> via ACPI _DSM methods.
>
> Currently, this is done only for the Intel display ports. But in future,
> this can be done for any other ports if the hardware becomes available
> (e.g. external monitors supporting integrated privacy screens?).
>
> Signed-off-by: Rajat Jain
> Change-Id: Ic9ff07fc4a50797d2d0dfb919f11aa0821a4b548
Hi Folks,
I posted a v2 taking care of the comments I received (also split it
into 3 patches now, and resused some ACPI code I found in i915
driver). . Wondering if any one got a chance to look at this?
Thanks,
Rajat
> ---
> v2: Formed by splitting the original patch into multiple patches.
> - All code has been moved into i915 now.
> - Privacy screen is a i915 property
> - Have a local state variable to store the prvacy screen. Don't read
> it from hardware.
>
> drivers/gpu/drm/i915/Makefile | 3 +-
> drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++-
> .../gpu/drm/i915/display/intel_connector.c| 35 ++
> .../gpu/drm/i915/display/intel_connector.h| 1 +
> .../drm/i915/display/intel_display_types.h| 4 ++
> drivers/gpu/drm/i915/display/intel_dp.c | 5 ++
> .../drm/i915/display/intel_privacy_screen.c | 70 +++
> .../drm/i915/display/intel_privacy_screen.h | 25 +++
> include/uapi/drm/i915_drm.h | 14
> 9 files changed, 166 insertions(+), 4 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
> create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 2587ea834f06..3589ebcf27bc 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -185,7 +185,8 @@ i915-y += \
> display/intel_tc.o
> i915-$(CONFIG_ACPI) += \
> display/intel_acpi.o \
> - display/intel_opregion.o
> + display/intel_opregion.o \
> + display/intel_privacy_screen.o
> i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
> display/intel_fbdev.o
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index d3fb75bb9eb1..378772d3449c 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -37,6 +37,7 @@
> #include "intel_atomic.h"
> #include "intel_display_types.h"
> #include "intel_hdcp.h"
> +#include "intel_privacy_screen.h"
> #include "intel_sprite.h"
>
> /**
> @@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct
> drm_connector *connector,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_digital_connector_state *intel_conn_state =
> to_intel_digital_connector_state(state);
> + struct intel_connector *intel_connector =
> to_intel_connector(connector);
>
> if (property == dev_priv->force_audio_property)
> *val = intel_conn_state->force_audio;
> else if (property == dev_priv->broadcast_rgb_property)
> *val = intel_conn_state->broadcast_rgb;
> + else if (property == intel_connector->privacy_screen_property)
> + *val = intel_conn_state->privacy_screen_status;
> else {
> DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
> property->base.id, property->name);
> @@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct
> drm_connector *connector,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_digital_connector_state *intel_conn_state =
> to_intel_digital_connector_state(state);
> + struct intel_connector *intel_connector =
> to_intel_connector(connector);
>
> if (property == dev_priv->force_audio_property) {
> intel_conn_state->force_audio = val;
> return 0;
> - }
> -
> - if (property == dev_priv->broadcast_rgb_property) {
> + } else if (property == dev_priv->broadcast_rgb_property) {
> intel_conn_state->broadcast_rgb = val;
> return 0;
> + } else if (
[PATCH v2 3/3] drm/i915: Add support for integrated privacy screens
Certain laptops now come with panels that have integrated privacy
screens on them. This patch adds support for such panels by adding
a privacy-screen property to the intel_connector for the panel, that
the userspace can then use to control and check the status.
Identifying the presence of privacy screen, and controlling it, is done
via ACPI _DSM methods.
Currently, this is done only for the Intel display ports. But in future,
this can be done for any other ports if the hardware becomes available
(e.g. external monitors supporting integrated privacy screens?).
Signed-off-by: Rajat Jain
Change-Id: Ic9ff07fc4a50797d2d0dfb919f11aa0821a4b548
---
v2: Formed by splitting the original patch into multiple patches.
- All code has been moved into i915 now.
- Privacy screen is a i915 property
- Have a local state variable to store the prvacy screen. Don't read
it from hardware.
drivers/gpu/drm/i915/Makefile | 3 +-
drivers/gpu/drm/i915/display/intel_atomic.c | 13 +++-
.../gpu/drm/i915/display/intel_connector.c| 35 ++
.../gpu/drm/i915/display/intel_connector.h| 1 +
.../drm/i915/display/intel_display_types.h| 4 ++
drivers/gpu/drm/i915/display/intel_dp.c | 5 ++
.../drm/i915/display/intel_privacy_screen.c | 70 +++
.../drm/i915/display/intel_privacy_screen.h | 25 +++
include/uapi/drm/i915_drm.h | 14
9 files changed, 166 insertions(+), 4 deletions(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.c
create mode 100644 drivers/gpu/drm/i915/display/intel_privacy_screen.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 2587ea834f06..3589ebcf27bc 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -185,7 +185,8 @@ i915-y += \
display/intel_tc.o
i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
- display/intel_opregion.o
+ display/intel_opregion.o \
+ display/intel_privacy_screen.o
i915-$(CONFIG_DRM_FBDEV_EMULATION) += \
display/intel_fbdev.o
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
b/drivers/gpu/drm/i915/display/intel_atomic.c
index d3fb75bb9eb1..378772d3449c 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -37,6 +37,7 @@
#include "intel_atomic.h"
#include "intel_display_types.h"
#include "intel_hdcp.h"
+#include "intel_privacy_screen.h"
#include "intel_sprite.h"
/**
@@ -57,11 +58,14 @@ int intel_digital_connector_atomic_get_property(struct
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
else if (property == dev_priv->broadcast_rgb_property)
*val = intel_conn_state->broadcast_rgb;
+ else if (property == intel_connector->privacy_screen_property)
+ *val = intel_conn_state->privacy_screen_status;
else {
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
property->base.id, property->name);
@@ -89,15 +93,18 @@ int intel_digital_connector_atomic_set_property(struct
drm_connector *connector,
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_digital_connector_state *intel_conn_state =
to_intel_digital_connector_state(state);
+ struct intel_connector *intel_connector = to_intel_connector(connector);
if (property == dev_priv->force_audio_property) {
intel_conn_state->force_audio = val;
return 0;
- }
-
- if (property == dev_priv->broadcast_rgb_property) {
+ } else if (property == dev_priv->broadcast_rgb_property) {
intel_conn_state->broadcast_rgb = val;
return 0;
+ } else if (property == intel_connector->privacy_screen_property) {
+ intel_privacy_screen_set_val(intel_connector, val);
+ intel_conn_state->privacy_screen_status = val;
+ return 0;
}
DRM_DEBUG_ATOMIC("Unknown property [PROP:%d:%s]\n",
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
b/drivers/gpu/drm/i915/display/intel_connector.c
index 308ec63207ee..3ccbf52aedf9 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -281,3 +281,38 @@ intel_attach_colorspace_property(struct drm_connector
*connector)
drm_object_attach_property(>base,
[PATCH v2 1/3] drm/i915: Move the code to populate ACPI device ID into intel_acpi
Move the code that populates the ACPI device ID for devices, into
more appripriate intel_acpi.c. This is done in preparation for more
users of this code (in next patch).
Signed-off-by: Rajat Jain
Change-Id: Ifb3bd458734985c2a78ba682e6f0a2e63e0626ca
---
v2: v1 doesn't exist. Found existing code in i915 driver to assign the ACPI ID
which is what I plan to re-use.
drivers/gpu/drm/i915/display/intel_acpi.c | 87 +++
drivers/gpu/drm/i915/display/intel_acpi.h | 6 ++
drivers/gpu/drm/i915/display/intel_opregion.c | 80 +
3 files changed, 97 insertions(+), 76 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c
b/drivers/gpu/drm/i915/display/intel_acpi.c
index 3456d33feb46..748d9b3125dd 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.c
+++ b/drivers/gpu/drm/i915/display/intel_acpi.c
@@ -156,3 +156,90 @@ void intel_register_dsm_handler(void)
void intel_unregister_dsm_handler(void)
{
}
+
+/*
+ * ACPI Specification, Revision 5.0, Appendix B.3.2 _DOD (Enumerate All Devices
+ * Attached to the Display Adapter).
+ */
+#define ACPI_DISPLAY_INDEX_SHIFT 0
+#define ACPI_DISPLAY_INDEX_MASK(0xf << 0)
+#define ACPI_DISPLAY_PORT_ATTACHMENT_SHIFT 4
+#define ACPI_DISPLAY_PORT_ATTACHMENT_MASK (0xf << 4)
+#define ACPI_DISPLAY_TYPE_SHIFT8
+#define ACPI_DISPLAY_TYPE_MASK (0xf << 8)
+#define ACPI_DISPLAY_TYPE_OTHER(0 << 8)
+#define ACPI_DISPLAY_TYPE_VGA (1 << 8)
+#define ACPI_DISPLAY_TYPE_TV (2 << 8)
+#define ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL (3 << 8)
+#define ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL (4 << 8)
+#define ACPI_VENDOR_SPECIFIC_SHIFT 12
+#define ACPI_VENDOR_SPECIFIC_MASK (0xf << 12)
+#define ACPI_BIOS_CAN_DETECT (1 << 16)
+#define ACPI_DEPENDS_ON_VGA(1 << 17)
+#define ACPI_PIPE_ID_SHIFT 18
+#define ACPI_PIPE_ID_MASK (7 << 18)
+#define ACPI_DEVICE_ID_SCHEME (1ULL << 31)
+
+static u32 acpi_display_type(struct intel_connector *connector)
+{
+ u32 display_type;
+
+ switch (connector->base.connector_type) {
+ case DRM_MODE_CONNECTOR_VGA:
+ case DRM_MODE_CONNECTOR_DVIA:
+ display_type = ACPI_DISPLAY_TYPE_VGA;
+ break;
+ case DRM_MODE_CONNECTOR_Composite:
+ case DRM_MODE_CONNECTOR_SVIDEO:
+ case DRM_MODE_CONNECTOR_Component:
+ case DRM_MODE_CONNECTOR_9PinDIN:
+ case DRM_MODE_CONNECTOR_TV:
+ display_type = ACPI_DISPLAY_TYPE_TV;
+ break;
+ case DRM_MODE_CONNECTOR_DVII:
+ case DRM_MODE_CONNECTOR_DVID:
+ case DRM_MODE_CONNECTOR_DisplayPort:
+ case DRM_MODE_CONNECTOR_HDMIA:
+ case DRM_MODE_CONNECTOR_HDMIB:
+ display_type = ACPI_DISPLAY_TYPE_EXTERNAL_DIGITAL;
+ break;
+ case DRM_MODE_CONNECTOR_LVDS:
+ case DRM_MODE_CONNECTOR_eDP:
+ case DRM_MODE_CONNECTOR_DSI:
+ display_type = ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL;
+ break;
+ case DRM_MODE_CONNECTOR_Unknown:
+ case DRM_MODE_CONNECTOR_VIRTUAL:
+ display_type = ACPI_DISPLAY_TYPE_OTHER;
+ break;
+ default:
+ MISSING_CASE(connector->base.connector_type);
+ display_type = ACPI_DISPLAY_TYPE_OTHER;
+ break;
+ }
+
+ return display_type;
+}
+
+void intel_populate_acpi_ids_for_all_connectors(struct drm_device *drm_dev)
+{
+ struct intel_connector *connector;
+ struct drm_connector_list_iter conn_iter;
+ u8 display_index[16] = {};
+ u32 device_id, type;
+
+ /* Populate the ACPI IDs for all connectors for a given drm_device */
+ drm_connector_list_iter_begin(drm_dev, _iter);
+ for_each_intel_connector_iter(connector, _iter) {
+
+ device_id = acpi_display_type(connector);
+
+ /* Use display type specific display index. */
+ type = (device_id & ACPI_DISPLAY_TYPE_MASK)
+ >> ACPI_DISPLAY_TYPE_SHIFT;
+ device_id |= display_index[type]++ << ACPI_DISPLAY_INDEX_SHIFT;
+
+ connector->acpi_device_id = device_id;
+ }
+ drm_connector_list_iter_end(_iter);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.h
b/drivers/gpu/drm/i915/display/intel_acpi.h
index 1c576b3fb712..8f6d850df6fa 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.h
+++ b/drivers/gpu/drm/i915/display/intel_acpi.h
@@ -6,12 +6,18 @@
#ifndef __INTEL_ACPI_H__
#define __INTEL_ACPI_H__
+#include "intel_display_types.h"
+
#ifdef CONFIG_ACPI
void intel_register_dsm_handler(void);
void intel_unregister_dsm_handle
[PATCH v2 2/3] drm/i915: Lookup and attach ACPI device node for connectors
Lookup and attach ACPI nodes for intel connectors. The lookup is done
in compliance with ACPI Spec 6.3
https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
(Ref: Pages 1119 - 1123).
This can be useful for any connector specific platform properties. (This
will be used for privacy screen in next patch).
Signed-off-by: Rajat Jain
Change-Id: I798e70714a4402554c8cd2a8e58268353f75814f
---
v2: formed by splitting the original patch into ACPI lookup, and privacy
screen property. Also move it into i915 now that I found existing code
in i915 that can be re-used.
drivers/gpu/drm/i915/display/intel_acpi.c | 50 +++
drivers/gpu/drm/i915/display/intel_acpi.h | 4 +-
.../drm/i915/display/intel_display_types.h| 3 ++
drivers/gpu/drm/i915/display/intel_dp.c | 4 ++
4 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c
b/drivers/gpu/drm/i915/display/intel_acpi.c
index 748d9b3125dd..0c10516430b1 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.c
+++ b/drivers/gpu/drm/i915/display/intel_acpi.c
@@ -243,3 +243,53 @@ void intel_populate_acpi_ids_for_all_connectors(struct
drm_device *drm_dev)
}
drm_connector_list_iter_end(_iter);
}
+
+/*
+ * Ref: ACPI Spec 6.3
+ * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
+ * Pages 1119 - 1123 describe, what I believe, a standard way of
+ * identifying / addressing "display panels" in the ACPI. It provides
+ * a way for the ACPI to define devices for the display panels attached
+ * to the system. It thus provides a way for the BIOS to export any panel
+ * specific properties to the system via ACPI (like device trees).
+ *
+ * The following functions looks up the ACPI node for a connector and returns
+ * it. Technically it is independent from the i915 code, and
+ * ideally may be called for all connectors. It is generally a good idea to
+ * be able to attach an ACPI node to describe anything if needed. (This can
+ * help in future for other panel specific features maybe). However, it
+ * needs an acpi device ID which is build using an index within a particular
+ * type of port (Ref to the pages of spec mentioned above, and to code in
+ * intel_populate_acpi_ids_for_all_connectors()). This device index
+ * unfortunately is not available in DRM code, so currently its call is
+ * originated from i915 driver. If in future this is useful for other drivers
+ * and we can find a generic way of getting a device index, we should move this
+ * function to drm code, maybe.
+ */
+void intel_connector_lookup_acpi_node(struct intel_connector *intel_connector)
+{
+ struct drm_device *drm_dev = intel_connector->base.dev;
+ struct device *dev = _dev->pdev->dev;
+ struct acpi_device *conn_dev;
+ u64 conn_addr;
+
+ /*
+* Repopulate ACPI IDs for all connectors is needed because the display
+* index may have changed as a result of hotplugging and unplugging
+* connectors
+*/
+ intel_populate_acpi_ids_for_all_connectors(drm_dev);
+
+ /* Build the _ADR to look for */
+ conn_addr = intel_connector->acpi_device_id;
+ conn_addr |= ACPI_DEVICE_ID_SCHEME;
+ conn_addr |= ACPI_BIOS_CAN_DETECT;
+
+ DRM_DEV_INFO(dev, "Looking for connector ACPI node at _ADR=%llX\n",
+conn_addr);
+
+ /* Look up the connector device, under the PCI device */
+ conn_dev = acpi_find_child_device(ACPI_COMPANION(dev), conn_addr,
+ false);
+ intel_connector->acpi_handle = conn_dev ? conn_dev->handle : NULL;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.h
b/drivers/gpu/drm/i915/display/intel_acpi.h
index 8f6d850df6fa..61a4392fac4a 100644
--- a/drivers/gpu/drm/i915/display/intel_acpi.h
+++ b/drivers/gpu/drm/i915/display/intel_acpi.h
@@ -9,14 +9,16 @@
#include "intel_display_types.h"
#ifdef CONFIG_ACPI
+void intel_connector_lookup_acpi_node(struct intel_connector *connector);
void intel_register_dsm_handler(void);
void intel_unregister_dsm_handler(void);
void intel_populate_acpi_ids_for_all_connectors(struct drm_device *drm_dev);
#else
+static inline void
+intel_connector_lookup_acpi_node(struct intel_connector *connector) { return; }
static inline void intel_register_dsm_handler(void) { return; }
static inline void intel_unregister_dsm_handler(void) { return; }
static inline void
-static inline void
intel_populate_acpi_ids_for_all_connectors(struct drm_device *drm_dev) { }
#endif /* CONFIG_ACPI */
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 449abaea619f..c2706afc069b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -400,6 +400,9 @@ struct intel_connector {
/* ACPI device i
Re: [PATCH] drm: Add support for integrated privacy screens
On Fri, Oct 25, 2019 at 4:36 AM Thierry Reding wrote: > > On Thu, Oct 24, 2019 at 01:45:16PM -0700, Rajat Jain wrote: > > Hi, > > > > Thanks for your review and comments. Please see inline below. > > > > On Thu, Oct 24, 2019 at 4:20 AM Thierry Reding > > wrote: > > > > > > On Tue, Oct 22, 2019 at 05:12:06PM -0700, Rajat Jain wrote: > > > > Certain laptops now come with panels that have integrated privacy > > > > screens on them. This patch adds support for such panels by adding > > > > a privacy-screen property to the drm_connector for the panel, that > > > > the userspace can then use to control and check the status. The idea > > > > was discussed here: > > > > > > > > https://lkml.org/lkml/2019/10/1/786 > > > > > > > > ACPI methods are used to identify, query and control privacy screen: > > > > > > > > * Identifying an ACPI object corresponding to the panel: The patch > > > > follows ACPI Spec 6.3 (available at > > > > https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf). > > > > Pages 1119 - 1123 describe what I believe, is a standard way of > > > > identifying / addressing "display panels" in the ACPI tables, thus > > > > allowing kernel to attach ACPI nodes to the panel. IMHO, this ability > > > > to identify and attach ACPI nodes to drm connectors may be useful for > > > > reasons other privacy-screens, in future. > > > > > > > > * Identifying the presence of privacy screen, and controlling it, is > > > > done > > > > via ACPI _DSM methods. > > > > > > > > Currently, this is done only for the Intel display ports. But in future, > > > > this can be done for any other ports if the hardware becomes available > > > > (e.g. external monitors supporting integrated privacy screens?). > > > > > > > > Also, this code can be extended in future to support non-ACPI methods > > > > (e.g. using a kernel GPIO driver to toggle a gpio that controls the > > > > privacy-screen). > > > > > > > > Signed-off-by: Rajat Jain > > > > --- > > > > drivers/gpu/drm/Makefile| 1 + > > > > drivers/gpu/drm/drm_atomic_uapi.c | 5 + > > > > drivers/gpu/drm/drm_connector.c | 38 + > > > > drivers/gpu/drm/drm_privacy_screen.c| 176 > > > > drivers/gpu/drm/i915/display/intel_dp.c | 3 + > > > > include/drm/drm_connector.h | 18 +++ > > > > include/drm/drm_mode_config.h | 7 + > > > > include/drm/drm_privacy_screen.h| 33 + > > > > 8 files changed, 281 insertions(+) > > > > create mode 100644 drivers/gpu/drm/drm_privacy_screen.c > > > > create mode 100644 include/drm/drm_privacy_screen.h > > > > > > I like this much better than the prior proposal to use sysfs. However > > > the support currently looks a bit tangled. I realize that we only have a > > > single implementation for this in hardware right now, so there's no use > > > in over-engineering things, but I think we can do a better job from the > > > start without getting into too many abstractions. See below. > > > > > > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > > > > index 82ff826b33cc..e1fc33d69bb7 100644 > > > > --- a/drivers/gpu/drm/Makefile > > > > +++ b/drivers/gpu/drm/Makefile > > > > @@ -19,6 +19,7 @@ drm-y := drm_auth.o drm_cache.o \ > > > > drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \ > > > > drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o > > > > > > > > +drm-$(CONFIG_ACPI) += drm_privacy_screen.o > > > > drm-$(CONFIG_DRM_LEGACY) += drm_legacy_misc.o drm_bufs.o drm_context.o > > > > drm_dma.o drm_scatter.o drm_lock.o > > > > drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o > > > > drm-$(CONFIG_DRM_VM) += drm_vm.o > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > > > > b/drivers/gpu/drm/drm_atomic_uapi.c > > > > index 7a26bfb5329c..44131165e4ea 100644 > > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c > > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > > > > @@ -30,6 +30,7 @@ > > > > #include > > > > #include > > > > #include > > &g
Re: [PATCH] drm: Add support for integrated privacy screens
Hi,
Thanks for your review and comments. Please see inline below.
On Thu, Oct 24, 2019 at 4:20 AM Thierry Reding wrote:
>
> On Tue, Oct 22, 2019 at 05:12:06PM -0700, Rajat Jain wrote:
> > Certain laptops now come with panels that have integrated privacy
> > screens on them. This patch adds support for such panels by adding
> > a privacy-screen property to the drm_connector for the panel, that
> > the userspace can then use to control and check the status. The idea
> > was discussed here:
> >
> > https://lkml.org/lkml/2019/10/1/786
> >
> > ACPI methods are used to identify, query and control privacy screen:
> >
> > * Identifying an ACPI object corresponding to the panel: The patch
> > follows ACPI Spec 6.3 (available at
> > https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf).
> > Pages 1119 - 1123 describe what I believe, is a standard way of
> > identifying / addressing "display panels" in the ACPI tables, thus
> > allowing kernel to attach ACPI nodes to the panel. IMHO, this ability
> > to identify and attach ACPI nodes to drm connectors may be useful for
> > reasons other privacy-screens, in future.
> >
> > * Identifying the presence of privacy screen, and controlling it, is done
> > via ACPI _DSM methods.
> >
> > Currently, this is done only for the Intel display ports. But in future,
> > this can be done for any other ports if the hardware becomes available
> > (e.g. external monitors supporting integrated privacy screens?).
> >
> > Also, this code can be extended in future to support non-ACPI methods
> > (e.g. using a kernel GPIO driver to toggle a gpio that controls the
> > privacy-screen).
> >
> > Signed-off-by: Rajat Jain
> > ---
> > drivers/gpu/drm/Makefile| 1 +
> > drivers/gpu/drm/drm_atomic_uapi.c | 5 +
> > drivers/gpu/drm/drm_connector.c | 38 +
> > drivers/gpu/drm/drm_privacy_screen.c| 176
> > drivers/gpu/drm/i915/display/intel_dp.c | 3 +
> > include/drm/drm_connector.h | 18 +++
> > include/drm/drm_mode_config.h | 7 +
> > include/drm/drm_privacy_screen.h| 33 +
> > 8 files changed, 281 insertions(+)
> > create mode 100644 drivers/gpu/drm/drm_privacy_screen.c
> > create mode 100644 include/drm/drm_privacy_screen.h
>
> I like this much better than the prior proposal to use sysfs. However
> the support currently looks a bit tangled. I realize that we only have a
> single implementation for this in hardware right now, so there's no use
> in over-engineering things, but I think we can do a better job from the
> start without getting into too many abstractions. See below.
>
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 82ff826b33cc..e1fc33d69bb7 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -19,6 +19,7 @@ drm-y := drm_auth.o drm_cache.o \
> > drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
> > drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o
> >
> > +drm-$(CONFIG_ACPI) += drm_privacy_screen.o
> > drm-$(CONFIG_DRM_LEGACY) += drm_legacy_misc.o drm_bufs.o drm_context.o
> > drm_dma.o drm_scatter.o drm_lock.o
> > drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
> > drm-$(CONFIG_DRM_VM) += drm_vm.o
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 7a26bfb5329c..44131165e4ea 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -30,6 +30,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > @@ -766,6 +767,8 @@ static int drm_atomic_connector_set_property(struct
> > drm_connector *connector,
> > fence_ptr);
> > } else if (property == connector->max_bpc_property) {
> > state->max_requested_bpc = val;
> > + } else if (property == config->privacy_screen_property) {
> > + drm_privacy_screen_set_val(connector, val);
>
> This doesn't look right. Shouldn't you store the value in the connector
> state and then leave it up to the connector driver to set it
> appropriately? I think that also has the advantage of untangling this
> support a little.
Hopefully this gets answered in my explanations below.
>
> > } else if (connector->funcs->atomic_set_property) {
> > return c
Re: New sysfs interface for privacy screens
Hi Folks, On Mon, Oct 7, 2019 at 11:13 PM Jani Nikula wrote: > On Mon, 07 Oct 2019, Mat King wrote: > > That makes sense; just to confirm can a property be added or removed > > after the connector is registered? > > You need to create the property before registering the drm device. You > can attach/detach the property later, but I should think you know by the > time you're registering the connector whether it supports the privacy > screen or not. > I just posted a patch for this here: https://lkml.org/lkml/2019/10/22/967 Would appreciate review and comments. Thanks & Best Regards, Rajat > > BR, > Jani. > > -- > Jani Nikula, Intel Open Source Graphics Center > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: New sysfs interface for privacy screens
On Tue, Oct 22, 2019 at 5:14 PM Rajat Jain wrote: > > Hi Folks, > > On Mon, Oct 7, 2019 at 11:13 PM Jani Nikula > wrote: >> >> On Mon, 07 Oct 2019, Mat King wrote: >> > That makes sense; just to confirm can a property be added or removed >> > after the connector is registered? >> >> You need to create the property before registering the drm device. You >> can attach/detach the property later, but I should think you know by the >> time you're registering the connector whether it supports the privacy >> screen or not. > > I just posted a patch for this here: https://lkml.org/lkml/2019/10/22/967 Would appreciate review and comments. Thanks & Best Regards, Rajat > >> >> >> BR, >> Jani. >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
[PATCH] drm: Add support for integrated privacy screens
Certain laptops now come with panels that have integrated privacy
screens on them. This patch adds support for such panels by adding
a privacy-screen property to the drm_connector for the panel, that
the userspace can then use to control and check the status. The idea
was discussed here:
https://lkml.org/lkml/2019/10/1/786
ACPI methods are used to identify, query and control privacy screen:
* Identifying an ACPI object corresponding to the panel: The patch
follows ACPI Spec 6.3 (available at
https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf).
Pages 1119 - 1123 describe what I believe, is a standard way of
identifying / addressing "display panels" in the ACPI tables, thus
allowing kernel to attach ACPI nodes to the panel. IMHO, this ability
to identify and attach ACPI nodes to drm connectors may be useful for
reasons other privacy-screens, in future.
* Identifying the presence of privacy screen, and controlling it, is done
via ACPI _DSM methods.
Currently, this is done only for the Intel display ports. But in future,
this can be done for any other ports if the hardware becomes available
(e.g. external monitors supporting integrated privacy screens?).
Also, this code can be extended in future to support non-ACPI methods
(e.g. using a kernel GPIO driver to toggle a gpio that controls the
privacy-screen).
Signed-off-by: Rajat Jain
---
drivers/gpu/drm/Makefile| 1 +
drivers/gpu/drm/drm_atomic_uapi.c | 5 +
drivers/gpu/drm/drm_connector.c | 38 +
drivers/gpu/drm/drm_privacy_screen.c| 176
drivers/gpu/drm/i915/display/intel_dp.c | 3 +
include/drm/drm_connector.h | 18 +++
include/drm/drm_mode_config.h | 7 +
include/drm/drm_privacy_screen.h| 33 +
8 files changed, 281 insertions(+)
create mode 100644 drivers/gpu/drm/drm_privacy_screen.c
create mode 100644 include/drm/drm_privacy_screen.h
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 82ff826b33cc..e1fc33d69bb7 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -19,6 +19,7 @@ drm-y :=drm_auth.o drm_cache.o \
drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o
+drm-$(CONFIG_ACPI) += drm_privacy_screen.o
drm-$(CONFIG_DRM_LEGACY) += drm_legacy_misc.o drm_bufs.o drm_context.o
drm_dma.o drm_scatter.o drm_lock.o
drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
drm-$(CONFIG_DRM_VM) += drm_vm.o
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 7a26bfb5329c..44131165e4ea 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include
#include
@@ -766,6 +767,8 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
fence_ptr);
} else if (property == connector->max_bpc_property) {
state->max_requested_bpc = val;
+ } else if (property == config->privacy_screen_property) {
+ drm_privacy_screen_set_val(connector, val);
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
@@ -842,6 +845,8 @@ drm_atomic_connector_get_property(struct drm_connector
*connector,
*val = 0;
} else if (property == connector->max_bpc_property) {
*val = state->max_requested_bpc;
+ } else if (property == config->privacy_screen_property) {
+ *val = drm_privacy_screen_get_val(connector);
} else if (connector->funcs->atomic_get_property) {
return connector->funcs->atomic_get_property(connector,
state, property, val);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4c766624b20d..a31e0382132b 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -821,6 +821,11 @@ static const struct drm_prop_enum_list
drm_panel_orientation_enum_list[] = {
{ DRM_MODE_PANEL_ORIENTATION_RIGHT_UP, "Right Side Up" },
};
+static const struct drm_prop_enum_list drm_privacy_screen_enum_list[] = {
+ { DRM_PRIVACY_SCREEN_DISABLED, "Disabled" },
+ { DRM_PRIVACY_SCREEN_ENABLED, "Enabled" },
+};
+
static const struct drm_prop_enum_list drm_dvi_i_select_enum_list[] = {
{ DRM_MODE_SUBCONNECTOR_Automatic, "Automatic" }, /* DVI-I and TV-out */
{ DRM_MODE_SUBCONNECTOR_DVID, "DVI-D" }, /* DVI-I */
@@ -2253,6 +2258,39 @@ static void drm_tile_group_free(struct kref *kref)
kfree(tg);
}
+/**
+ * drm_connector_init_priva
Re: New sysfs interface for privacy screens
On Mon, Oct 7, 2019 at 9:19 AM Mat King wrote:
>
> On Mon, Oct 7, 2019 at 7:09 AM Sean Paul wrote:
> >
> > On Thu, Oct 3, 2019 at 3:57 PM Mat King wrote:
> > >
> > > On Thu, Oct 3, 2019 at 2:59 AM Jani Nikula
> > > wrote:
> > > >
> > > > On Wed, 02 Oct 2019, Mat King wrote:
> > > > > On Wed, Oct 2, 2019 at 4:46 AM Jani Nikula
> > > > > wrote:
> > > > >>
> > > > >> On Wed, 02 Oct 2019, Daniel Thompson
> > > > >> wrote:
> > > > >> > On Wed, Oct 02, 2019 at 12:30:05PM +0300, Jani Nikula wrote:
> > > > >> >> On Tue, 01 Oct 2019, Mat King wrote:
> > > > >> >> > Resending in plain text mode
> >
> > /snip
> >
> > >
> > > So my proposal would now be to add a new standard property to
> > > drm_connector called "privacy_screen" this property would be an enum
> > > which can take one of three values.
> > >
> > > PRIVACY_UNSUPPORTED - Privacy is not available for this connector
> > > PRIVACY_DISABLED - Privacy is available but turned off
> > > PRIVACY_ENABLED - Privacy is available and turned on
> >
> > Agree with Jani, use the property presence to determine if it's supported
>
> That makes sense; just to confirm can a property be added or removed
> after the connector is registered?
>
> >
> > >
> > > When the connector is initized the privacy screen property is set to
> > > PRIVACY_UNSUPPORTED and cannot be changed unless a drm_privacy_screen
> > > is registered to the connector. drm_privacy_screen will look something
> > > like
> > >
> > > struct drm_privacy_screen_ops {
> > > int (*get_privacy_state)(struct drm_privacy_screen *);
> > > int (*set_privacy_state)(struct drm_privacy_screen *, int);
> > > }
> > >
> > > struct drm_privacy_screen {
> > > /* The privacy screen device */
> > > struct device *dev;
> > >
> > > /* The connector that the privacy screen is attached */
> > > struct drm_connector *connector;
> > >
> > > /* Ops to get and set the privacy screen state */
> > > struct drm_privacy_screen_ops *ops;
> > >
> > > /* The current state of the privacy screen */
> > > int state;
> > > }
> > >
> > > Privacy screen device drivers will call a function to register the
> > > privacy screen with the connector.
> >
> > Do we actually need dedicated drivers for privacy screen? It seems
> > like something that is panel-specific hardware, so I'd suggest just
> > using the panel driver.
>
> The privacy screen is physically part of the display but the control
> interface, at least in all current use cases, is ACPI. Is there a way
> to control an ACPI device with the panel driver?
I feel that doing it in a dedicated driver has the advantage that if
we can standardise the control interface, it can be used across
different panels. So a new panel can be supported using the existing
driver by merely instantiating the right ACPI HID "privacy screen"
device as a child device of the parent display / panel device. This
parent-child relation would also give the kernel the connection needed
about "which display does this privacy screen attach to". In future,if
non-x86 platforms need the feature using a different control interface
(say via a GPIO driver), the privacy screen driver can be updated to
support that also.
Thanks,
Rajat
>
> >
> > Sean
> >
> > >
> > > int drm_privacy_screen_register(struct drm_privacy_screen_ops *ops,
> > > struct device *dev, struct drm_connector *);
> > >
> > > Calling this will set a new field on the connector "struct
> > > drm_privacy_screen *privacy_screen" and change the value of the
> > > property to ops->get_privacy_state(). When
> > > drm_mode_connector_set_obj_prop() is called with the
> > > privacy_screen_proptery if a privacy_screen is registered to the
> > > connector the ops->set_privacy_state() will be called with the new
> > > value.
> > >
> > > Setting of this property (and all drm properties) is done in user
> > > space using ioctrl.
> > >
> > > Registering the privacy screen with a connector may be tricky because
> > > the driver for the privacy screen will need to be able to identify
> > > which connector it belongs to and we will have to deal with connectors
> > > being added both before and after the privacy screen device is added
> > > by it's driver.
> > >
> > > How does that sound? I will work on a patch if that all sounds about
> > > right.
> > >
> > > One question I still have is there a way to not accept a value that is
> > > passed to drm_mode_connector_set_obj_prop()? In this case if a privacy
> > > screen is not registered the property must stay PRIVACY_UNSUPPORTED
> > > and if a privacy screen is registered then PRIVACY_UNSUPPORTED must
> > > never be set.
Re: New sysfs interface for privacy screens
Hi, Me and Mat are working on this together, and I had a followup to something Mat asked earlier. On Thu, Oct 3, 2019 at 12:57 PM Mat King wrote: > > On Thu, Oct 3, 2019 at 2:59 AM Jani Nikula > wrote: > > > > On Wed, 02 Oct 2019, Mat King wrote: > > > On Wed, Oct 2, 2019 at 4:46 AM Jani Nikula > > > wrote: > > >> > > >> On Wed, 02 Oct 2019, Daniel Thompson wrote: > > >> > On Wed, Oct 02, 2019 at 12:30:05PM +0300, Jani Nikula wrote: > > >> >> On Tue, 01 Oct 2019, Mat King wrote: > > >> >> > Resending in plain text mode > > >> >> > > One question I still have is there a way to not accept a value that is > passed to drm_mode_connector_set_obj_prop()? In this case if a privacy > screen is not registered the property must stay PRIVACY_UNSUPPORTED > and if a privacy screen is registered then PRIVACY_UNSUPPORTED must > never be set. > One question I still have is there a way to not accept a value that is > passed to drm_mode_connector_set_obj_prop()? In this case if a privacy > screen is not registered the property must stay PRIVACY_UNSUPPORTED > and if a privacy screen is registered then PRIVACY_UNSUPPORTED must > never be set. Any guidance here on this? Essentially I think we are looking for a way to 1) expose the "capability" of having a privacy screen feature from kernel to userland, and 2) then a way to "control" (enable / disable) the feature from userland to kernel. I understand DRM connector property is the suggested way to do this. But I was not clear if DRM connector properties a recommended way to do (1) also. If yes, then Mat's posted a proposal (https://lkml.org/lkml/2019/10/3/2068) but I did not see any comment specifically if that idea looked reasonable. Also looking for any guidance to Mat's question above. Thanks, Rajat > > > >> >> > I have been looking into adding Linux support for electronic privacy > > >> >> > screens which is a feature on some new laptops which is built into > > >> >> > the > > >> >> > display and allows users to turn it on instead of needing to use a > > >> >> > physical privacy filter. In discussions with my colleagues the idea > > >> >> > of > > >> >> > using either /sys/class/backlight or /sys/class/leds but this new > > >> >> > feature does not seem to quite fit into either of those classes. > > >> >> > > > >> >> > I am proposing adding a class called "privacy_screen" to interface > > >> >> > with these devices. The initial API would be simple just a single > > >> >> > property called "privacy_state" which when set to 1 would mean that > > >> >> > privacy is enabled and 0 when privacy is disabled. > > >> >> > > > >> >> > Current known use cases will use ACPI _DSM in order to interface > > >> >> > with > > >> >> > the privacy screens, but this class would allow device driver > > >> >> > authors > > >> >> > to use other interfaces as well. > > >> >> > > > >> >> > Example: > > >> >> > > > >> >> > # get privacy screen state > > >> >> > cat /sys/class/privacy_screen/cros_privacy/privacy_state # 1: > > >> >> > privacy > > >> >> > enabled 0: privacy disabled > > >> >> > > > >> >> > # set privacy enabled > > >> >> > echo 1 > /sys/class/privacy_screen/cros_privacy/privacy_state > > >> >> > > > >> >> > Does this approach seem to be reasonable? > > >> >> > > >> >> What part of the userspace would be managing the privacy screen? > > >> >> Should > > >> >> there be a connection between the display and the privacy screen that > > >> >> covers the display? How would the userspace make that connection if > > >> >> it's > > >> >> a sysfs interface? > > >> >> > > >> >> I don't know how the privacy screen operates, but if it draws any > > >> >> power, > > >> >> you'll want to disable it when you switch off the display it covers. > > >> >> > > >> >> If the privacy screen control was part of the graphics subsystem > > >> >> (say, a > > >> >> DRM connector property, which feels somewhat natural), I think it > > >> >> would > > >> >> make it easier for userspace to have policies such as enabling the > > >> >> privacy screen automatically depending on the content you're viewing, > > >> >> but only if the content is on the display that has a privacy screen. > > >> > > > >> > Connectors versus sysfs came up on a backlight thread recently. > > >> > > > >> > Daniel Vetter wrote an excellent summary on why it has been (and still > > >> > is) difficult to migrate backlight controls towards the DRM connector > > >> > interface: > > >> > https://lkml.org/lkml/2019/8/20/752 > > >> > > > >> > Many of the backlight legacy problems do not apply to privacy screens > > >> > but I do suggest reading this post and some of the neighbouring parts > > >> > of the thread. In particular the ACPI driver versus real driver issues > > >> > Daniel mentioned could occur again. Hopefully not though, I mean how > > >> > wrong can a 1-bit control go? (actually no... don't answer that). > > >> > > > >> > It would definitely be a shame to build up an unnecessary sysfs legacy > > >> > for privacy
