diff mbox series

[v2,2/9] drm: Add privacy-screen class (v2)

Message ID 20210421204804.589962-3-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show
Series drm: Add privacy-screen class and connector properties | expand

Commit Message

Hans de Goede April 21, 2021, 8:47 p.m. UTC
On some new laptops the LCD panel has a builtin electronic privacy-screen.
We want to export this functionality as a property on the drm connector
object. But often this functionality is not exposed on the GPU but on some
other (ACPI) device.

This commit adds a privacy-screen class allowing the driver for these
other devices to register themselves as a privacy-screen provider; and
allowing the drm/kms code to get a privacy-screen provider associated
with a specific GPU/connector combo.

Changes in v2:
- Make CONFIG_DRM_PRIVACY_SCREEN a bool which controls if the drm_privacy
  code gets built as part of the main drm module rather then making it
  a tristate which builds its own module.
- Add a #if IS_ENABLED(CONFIG_DRM_PRIVACY_SCREEN) check to
  drm_privacy_screen_consumer.h and define stubs when the check fails.
  Together these 2 changes fix several dependency issues.
- Remove module related code now that this is part of the main drm.ko
- Use drm_class as class for the privacy-screen devices instead of
  adding a separate class for this

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 Documentation/gpu/drm-kms-helpers.rst     |  15 +
 MAINTAINERS                               |   8 +
 drivers/gpu/drm/Kconfig                   |   4 +
 drivers/gpu/drm/Makefile                  |   1 +
 drivers/gpu/drm/drm_drv.c                 |   4 +
 drivers/gpu/drm/drm_privacy_screen.c      | 401 ++++++++++++++++++++++
 include/drm/drm_privacy_screen_consumer.h |  48 +++
 include/drm/drm_privacy_screen_driver.h   |  80 +++++
 include/drm/drm_privacy_screen_machine.h  |  41 +++
 9 files changed, 602 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_privacy_screen.c
 create mode 100644 include/drm/drm_privacy_screen_consumer.h
 create mode 100644 include/drm/drm_privacy_screen_driver.h
 create mode 100644 include/drm/drm_privacy_screen_machine.h

Comments

Emil Velikov June 1, 2021, 3:31 p.m. UTC | #1
Hi Hans,

What happened with this series, did it fall through the cracks?

On Wed, 21 Apr 2021 at 21:48, Hans de Goede <hdegoede@redhat.com> wrote:

> --- /dev/null
> +++ b/drivers/gpu/drm/drm_privacy_screen.c

> +#include "drm_internal.h"

I think we don't need this include, do we?


> --- /dev/null
> +++ b/include/drm/drm_privacy_screen_consumer.h

> +#include <drm/drm_connector.h>

Ditto

> --- /dev/null
> +++ b/include/drm/drm_privacy_screen_driver.h

> +#include <drm/drm_connector.h>

Ditto

I like how you avoided leaking any DRM details within the new code,
modulo the includes above. With above tweaks, the series is:
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>

Theoretically one could also remove the `depends on DRM` from patch
8/9 but I'm not sure how much that saves us.

HTH
-Emil
Marco Trevisan June 1, 2021, 4:50 p.m. UTC | #2
Hi Emil,

On giu 1 2021, at 5:31 pm, Emil Velikov <emil.l.velikov@gmail.com> wrote:

> Hi Hans,
> 
> What happened with this series, did it fall through the cracks?

It's mostly waiting me to finish to propose the changes to GNOME,
unfortunately I've been busy in the past weeks with downstream work, so
I couldn't finish it, but I hope I can be back at it soon.

Cheers

> 
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_privacy_screen.c
> 
>> +#include "drm_internal.h"
> 
> I think we don't need this include, do we?
> 
> 
>> --- /dev/null
>> +++ b/include/drm/drm_privacy_screen_consumer.h
> 
>> +#include <drm/drm_connector.h>
> 
> Ditto
> 
>> --- /dev/null
>> +++ b/include/drm/drm_privacy_screen_driver.h
> 
>> +#include <drm/drm_connector.h>
> 
> Ditto
> 
> I like how you avoided leaking any DRM details within the new code,
> modulo the includes above. With above tweaks, the series is:
> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
> 
> Theoretically one could also remove the `depends on DRM` from patch
> 8/9 but I'm not sure how much that saves us.
> 
> HTH
> -Emil
>
Hans de Goede June 3, 2021, 11:59 a.m. UTC | #3
Hi,

On 6/1/21 5:31 PM, Emil Velikov wrote:
> Hi Hans,
> 
> What happened with this series, did it fall through the cracks?

Sorta, as Marco already mentioned I think people are waiting for the
user-space branches which he has on his personal git repos to be submitted
as offical merge-req-s to GNOME.

> On Wed, 21 Apr 2021 at 21:48, Hans de Goede <hdegoede@redhat.com> wrote:
> 
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_privacy_screen.c
> 
>> +#include "drm_internal.h"
> 
> I think we don't need this include, do we?

The drm_privacy_screen device registered by a provider
uses /sys/class/drm as its class, quoting from
drm_privacy_screen.c drm_privacy_screen_register():

        priv->dev.class = drm_class;
        priv->dev.type = &drm_privacy_screen_type;
        priv->dev.parent = parent;
        priv->dev.release = drm_privacy_screen_device_release;
        dev_set_name(&priv->dev, "privacy_screen-%s", dev_name(parent));
        priv->ops = ops;

        priv->ops->get_hw_state(priv);

        ret = device_register(&priv->dev);

Notice the "priv->dev.class = drm_class", the drm_class
variable is declared in "drm_internal.h".

Note this was not present in v2. As I mentioned in the commit msg:

Changes in v2:
- Make CONFIG_DRM_PRIVACY_SCREEN a bool which controls if the drm_privacy
  code gets built as part of the main drm module rather then making it
  a tristate which builds its own module.
- Add a #if IS_ENABLED(CONFIG_DRM_PRIVACY_SCREEN) check to
  drm_privacy_screen_consumer.h and define stubs when the check fails.
  Together these 2 changes fix several dependency issues.
- Remove module related code now that this is part of the main drm.ko
- Use drm_class as class for the privacy-screen devices instead of
  adding a separate class for this

This is something which I changed in v2. I changed this since I didn't
really see any good reason for drm_privacy_screen devices having their
own class, rather then just having them sit under /sys/class/drm .

I'm open to changing this if people dislike this choice.



>> --- /dev/null
>> +++ b/include/drm/drm_privacy_screen_consumer.h
> 
>> +#include <drm/drm_connector.h>
> 
> Ditto

The "enum drm_privacy_screen_status" used in various places
comes from drm/drm_connector.h (it is the same enum which is
used for the possible values of the drm-connector properties).


>> --- /dev/null
>> +++ b/include/drm/drm_privacy_screen_driver.h
> 
>> +#include <drm/drm_connector.h>
> 
> Ditto
> 
> I like how you avoided leaking any DRM details within the new code,
> modulo the includes above.

I'm glad you like it. I did indeed try to make the code mostly
independent, but as you can see above there are still some
inter-dependencies.

Because of this, the CONFIG_DRM_PRIVACY_SCREEN option also does
not control building this into a separate module. Like many other
DRM Kconfig options, this controls if the privacy-screen code will
be added to drm.ko or not.

Despite being 99% independent, the 2 are still intertwined at such
a level that this is necessary. Specifically drm_core_init() calls
drm_privacy_screen_lookup_init() to initialize the static lookup
table which is used to see if there is a privacy-screen (and to which
GPU,output combo it should be mapped). So if CONFIG_DRM_PRIVACY_SCREEN
is enabled and drm.ko is builtin then it must be builtin too, at which
point it is easiest to just make it part of drm.ko .

And there also is the later added dep from drm_privacy_screen.c on
the drm_class symbol, which means there are now symbol-deps in both
directions, which makes building the code into drm.ko the only option.

> With above tweaks, the series is:
> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>

As I've tried to explain, the includes are necessary, does your
Reviewed-by still stands when I keep the includes ?

> Theoretically one could also remove the `depends on DRM` from patch
> 8/9 but I'm not sure how much that saves us.

The depends in is necessary since CONFIG_DRM_PRIVACY_SCREEN just controls
if privacy-screen support will be added to drm.ko, if it is enabled we
still need drm.ko ti actually be built for things to work.

> HTH

Yes, actually getting a review of this code helps a lot, thank you.

Regards,

Hans
Emil Velikov June 3, 2021, 1:48 p.m. UTC | #4
On Thu, 3 Jun 2021 at 12:59, Hans de Goede <hdegoede@redhat.com> wrote:

> >
> >> +#include "drm_internal.h"
> >
> > I think we don't need this include, do we?
>
> The drm_privacy_screen device registered by a provider
> uses /sys/class/drm as its class, quoting from
> drm_privacy_screen.c drm_privacy_screen_register():
>
>         priv->dev.class = drm_class;
>         priv->dev.type = &drm_privacy_screen_type;
>         priv->dev.parent = parent;
>         priv->dev.release = drm_privacy_screen_device_release;
>         dev_set_name(&priv->dev, "privacy_screen-%s", dev_name(parent));
>         priv->ops = ops;
>
>         priv->ops->get_hw_state(priv);
>
>         ret = device_register(&priv->dev);
>
> Notice the "priv->dev.class = drm_class", the drm_class
> variable is declared in "drm_internal.h".
>
I have been looking at v1 while replying here, oopsie.

> >> --- /dev/null
> >> +++ b/include/drm/drm_privacy_screen_consumer.h
> >
> >> +#include <drm/drm_connector.h>
> >
> > Ditto
>
> The "enum drm_privacy_screen_status" used in various places
> comes from drm/drm_connector.h (it is the same enum which is
> used for the possible values of the drm-connector properties).
>
Hmm indeed.

If really needed one could move/duplicate/duplicate-and-namespace the
enum. If duplicating, cross references would be great and with
namespacing BUILD_BUG_ON(drm_privacy_screen_status::foo !=
custom-enum::foo) to enforce consistency.

Each feels dirty and I'm not sure if it's worth it -  just a silly
idea, don't read too much into it.

>
> >> --- /dev/null
> >> +++ b/include/drm/drm_privacy_screen_driver.h
> >
> >> +#include <drm/drm_connector.h>
> >
> > Ditto
> >
> > I like how you avoided leaking any DRM details within the new code,
> > modulo the includes above.
>
> I'm glad you like it. I did indeed try to make the code mostly
> independent, but as you can see above there are still some
> inter-dependencies.
>
> Because of this, the CONFIG_DRM_PRIVACY_SCREEN option also does
> not control building this into a separate module. Like many other
> DRM Kconfig options, this controls if the privacy-screen code will
> be added to drm.ko or not.
>
> Despite being 99% independent, the 2 are still intertwined at such
> a level that this is necessary. Specifically drm_core_init() calls
> drm_privacy_screen_lookup_init() to initialize the static lookup
> table which is used to see if there is a privacy-screen (and to which
> GPU,output combo it should be mapped). So if CONFIG_DRM_PRIVACY_SCREEN
> is enabled and drm.ko is builtin then it must be builtin too, at which
> point it is easiest to just make it part of drm.ko .
>
Yes, the initialisation is called from core drm - it has to happen somewhere.

What I was thinking of, is that we can reuse it (with minor tweaks) if
vendors deploy the privacy screen principle for audio, camera, etc.
Kind of crazy examples, but who knows.

> > With above tweaks, the series is:
> > Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
>
> As I've tried to explain, the includes are necessary, does your
> Reviewed-by still stands when I keep the includes ?
>
Yes, r-b it stands.

Thanks for the in the detailed reply and drm_class pointer :-P
-Emil
diff mbox series

Patch

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 389892f36185..5d8715d2f998 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -423,3 +423,18 @@  Legacy CRTC/Modeset Helper Functions Reference
 
 .. kernel-doc:: drivers/gpu/drm/drm_crtc_helper.c
    :export:
+
+Privacy-screen class
+====================
+
+.. kernel-doc:: drivers/gpu/drm/drm_privacy_screen.c
+   :doc: overview
+
+.. kernel-doc:: include/drm/drm_privacy_screen_driver.h
+   :internal:
+
+.. kernel-doc:: include/drm/drm_privacy_screen_machine.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_privacy_screen.c
+   :export:
diff --git a/MAINTAINERS b/MAINTAINERS
index 0c91cd07db3a..5d3e7729e57c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6131,6 +6131,14 @@  F:	drivers/gpu/drm/drm_panel.c
 F:	drivers/gpu/drm/panel/
 F:	include/drm/drm_panel.h
 
+DRM PRIVACY-SCREEN CLASS
+M:	Hans de Goede <hdegoede@redhat.com>
+L:	dri-devel@lists.freedesktop.org
+S:	Maintained
+T:	git git://anongit.freedesktop.org/drm/drm-misc
+F:	drivers/gpu/drm/drm_privacy_screen*
+F:	include/drm/drm_privacy_screen*
+
 DRM TTM SUBSYSTEM
 M:	Christian Koenig <christian.koenig@amd.com>
 M:	Huang Rui <ray.huang@amd.com>
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 3c16bd1afd87..698ea8a32b2a 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -474,3 +474,7 @@  config DRM_PANEL_ORIENTATION_QUIRKS
 config DRM_LIB_RANDOM
 	bool
 	default n
+
+config DRM_PRIVACY_SCREEN
+	bool
+	default n
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 89e747fedc00..578853d18a3d 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -32,6 +32,7 @@  drm-$(CONFIG_AGP) += drm_agpsupport.o
 drm-$(CONFIG_PCI) += drm_pci.o
 drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
 drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
+drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o
 
 drm_vram_helper-y := drm_gem_vram_helper.o
 obj-$(CONFIG_DRM_VRAM_HELPER) += drm_vram_helper.o
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index c2f78dee9f2d..b33baa888be4 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -42,6 +42,7 @@ 
 #include <drm/drm_managed.h>
 #include <drm/drm_mode_object.h>
 #include <drm/drm_print.h>
+#include <drm/drm_privacy_screen_machine.h>
 
 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
@@ -1030,6 +1031,7 @@  static const struct file_operations drm_stub_fops = {
 
 static void drm_core_exit(void)
 {
+	drm_privacy_screen_lookup_exit();
 	unregister_chrdev(DRM_MAJOR, "drm");
 	debugfs_remove(drm_debugfs_root);
 	drm_sysfs_destroy();
@@ -1056,6 +1058,8 @@  static int __init drm_core_init(void)
 	if (ret < 0)
 		goto error;
 
+	drm_privacy_screen_lookup_init();
+
 	drm_core_init_complete = true;
 
 	DRM_DEBUG("Initialized\n");
diff --git a/drivers/gpu/drm/drm_privacy_screen.c b/drivers/gpu/drm/drm_privacy_screen.c
new file mode 100644
index 000000000000..294a09194bfb
--- /dev/null
+++ b/drivers/gpu/drm/drm_privacy_screen.c
@@ -0,0 +1,401 @@ 
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright (C) 2020 - 2021 Red Hat, Inc.
+ *
+ * Authors:
+ * Hans de Goede <hdegoede@redhat.com>
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <drm/drm_privacy_screen_machine.h>
+#include <drm/drm_privacy_screen_consumer.h>
+#include <drm/drm_privacy_screen_driver.h>
+#include "drm_internal.h"
+
+/**
+ * DOC: overview
+ *
+ * This class allows non KMS drivers, from e.g. drivers/platform/x86 to
+ * register a privacy-screen device, which the KMS drivers can then use
+ * to implement the standard privacy-screen properties, see
+ * :ref:`Standard Connector Properties<standard_connector_properties>`.
+ *
+ * KMS drivers using a privacy-screen class device are advised to use the
+ * drm_connector_attach_privacy_screen_provider() and
+ * drm_connector_update_privacy_screen() helpers for dealing with this.
+ */
+
+#define to_drm_privacy_screen(dev) \
+	container_of(dev, struct drm_privacy_screen, dev)
+
+static DEFINE_MUTEX(drm_privacy_screen_lookup_lock);
+static LIST_HEAD(drm_privacy_screen_lookup_list);
+
+static DEFINE_MUTEX(drm_privacy_screen_devs_lock);
+static LIST_HEAD(drm_privacy_screen_devs);
+
+/*** drm_privacy_screen_machine.h functions ***/
+
+/**
+ * drm_privacy_screen_lookup_add - add an entry to the static privacy-screen
+ *    lookup list
+ * @lookup: lookup list entry to add
+ *
+ * Add an entry to the static privacy-screen lookup list. Note the
+ * &struct list_head which is part of the &struct drm_privacy_screen_lookup
+ * gets added to a list owned by the privacy-screen core. So the passed in
+ * &struct drm_privacy_screen_lookup must not be free-ed until it is removed
+ * from the lookup list by calling drm_privacy_screen_lookup_remove().
+ */
+void drm_privacy_screen_lookup_add(struct drm_privacy_screen_lookup *lookup)
+{
+	mutex_lock(&drm_privacy_screen_lookup_lock);
+	list_add(&lookup->list, &drm_privacy_screen_lookup_list);
+	mutex_unlock(&drm_privacy_screen_lookup_lock);
+}
+EXPORT_SYMBOL(drm_privacy_screen_lookup_add);
+
+/**
+ * drm_privacy_screen_lookup_remove - remove an entry to the static
+ *    privacy-screen lookup list
+ * @lookup: lookup list entry to remove
+ *
+ * Remove an entry previously added with drm_privacy_screen_lookup_add()
+ * from the static privacy-screen lookup list.
+ */
+void drm_privacy_screen_lookup_remove(struct drm_privacy_screen_lookup *lookup)
+{
+	mutex_lock(&drm_privacy_screen_lookup_lock);
+	list_del(&lookup->list);
+	mutex_unlock(&drm_privacy_screen_lookup_lock);
+}
+EXPORT_SYMBOL(drm_privacy_screen_lookup_remove);
+
+/*** drm_privacy_screen_consumer.h functions ***/
+
+static struct drm_privacy_screen *drm_privacy_screen_get_by_name(
+	const char *name)
+{
+	struct drm_privacy_screen *priv;
+	struct device *dev = NULL;
+
+	mutex_lock(&drm_privacy_screen_devs_lock);
+
+	list_for_each_entry(priv, &drm_privacy_screen_devs, list) {
+		if (strcmp(dev_name(&priv->dev), name) == 0) {
+			dev = get_device(&priv->dev);
+			break;
+		}
+	}
+
+	mutex_unlock(&drm_privacy_screen_devs_lock);
+
+	return dev ? to_drm_privacy_screen(dev) : NULL;
+}
+
+/**
+ * drm_privacy_screen_get - get a privacy-screen provider
+ * @dev: consumer-device for which to get a privacy-screen provider
+ * @con_id: (video)connector name for which to get a privacy-screen provider
+ *
+ * Get a privacy-screen provider for a privacy-screen attached to the
+ * display described by the @dev and @con_id parameters.
+ *
+ * Return:
+ * * A pointer to a &struct drm_privacy_screen on success.
+ * * ERR_PTR(-ENODEV) if no matching privacy-screen is found
+ * * ERR_PTR(-EPROBE_DEFER) if there is a matching privacy-screen,
+ *                          but it has not been registered yet.
+ */
+struct drm_privacy_screen *drm_privacy_screen_get(struct device *dev,
+						  const char *con_id)
+{
+	const char *dev_id = dev ? dev_name(dev) : NULL;
+	struct drm_privacy_screen_lookup *l;
+	struct drm_privacy_screen *priv;
+	const char *provider = NULL;
+	int match, best = -1;
+
+	/*
+	 * For now we only support using a static lookup table, which is
+	 * populated by the drm_privacy_screen_arch_init() call. This should
+	 * be extended with device-tree / fw_node lookup when support is added
+	 * for device-tree using hardware with a privacy-screen.
+	 *
+	 * The lookup algorithm was shamelessly taken from the clock
+	 * framework:
+	 *
+	 * We do slightly fuzzy matching here:
+	 *  An entry with a NULL ID is assumed to be a wildcard.
+	 *  If an entry has a device ID, it must match
+	 *  If an entry has a connection ID, it must match
+	 * Then we take the most specific entry - with the following order
+	 * of precedence: dev+con > dev only > con only.
+	 */
+	mutex_lock(&drm_privacy_screen_lookup_lock);
+
+	list_for_each_entry(l, &drm_privacy_screen_lookup_list, list) {
+		match = 0;
+
+		if (l->dev_id) {
+			if (!dev_id || strcmp(l->dev_id, dev_id))
+				continue;
+
+			match += 2;
+		}
+
+		if (l->con_id) {
+			if (!con_id || strcmp(l->con_id, con_id))
+				continue;
+
+			match += 1;
+		}
+
+		if (match > best) {
+			provider = l->provider;
+			best = match;
+		}
+	}
+
+	mutex_unlock(&drm_privacy_screen_lookup_lock);
+
+	if (!provider)
+		return ERR_PTR(-ENODEV);
+
+	priv = drm_privacy_screen_get_by_name(provider);
+	if (!priv)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	return priv;
+}
+EXPORT_SYMBOL(drm_privacy_screen_get);
+
+/**
+ * drm_privacy_screen_put - release a privacy-screen reference
+ * @priv: privacy screen reference to release
+ *
+ * Release a privacy-screen provider reference gotten through
+ * drm_privacy_screen_get(). May be called with a NULL or ERR_PTR,
+ * in which case it is a no-op.
+ */
+void drm_privacy_screen_put(struct drm_privacy_screen *priv)
+{
+	if (IS_ERR_OR_NULL(priv))
+		return;
+
+	put_device(&priv->dev);
+}
+EXPORT_SYMBOL(drm_privacy_screen_put);
+
+/**
+ * drm_privacy_screen_set_sw_state - set a privacy-screen's sw-state
+ * @priv: privacy screen to set the sw-state for
+ * @sw_state: new sw-state value to set
+ *
+ * Set the sw-state of a privacy screen. If the privacy-screen is not
+ * in a locked hw-state, then the actual and hw-state of the privacy-screen
+ * will be immediately updated to the new value. If the privacy-screen is
+ * in a locked hw-state, then the new sw-state will be remembered as the
+ * requested state to put the privacy-screen in when it becomes unlocked.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int drm_privacy_screen_set_sw_state(struct drm_privacy_screen *priv,
+				    enum drm_privacy_screen_status sw_state)
+{
+	int ret = 0;
+
+	mutex_lock(&priv->lock);
+
+	if (!priv->ops) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	/*
+	 * As per the DRM connector properties documentation, setting the
+	 * sw_state while the hw_state is locked is allowed. In this case
+	 * it is a no-op other then storing the new sw_state so that it
+	 * can be honored when the state gets unlocked.
+	 */
+	if (priv->hw_state >= PRIVACY_SCREEN_DISABLED_LOCKED) {
+		priv->sw_state = sw_state;
+		goto out;
+	}
+
+	ret = priv->ops->set_sw_state(priv, sw_state);
+out:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+EXPORT_SYMBOL(drm_privacy_screen_set_sw_state);
+
+/**
+ * drm_privacy_screen_get_state - get privacy-screen's current state
+ * @priv: privacy screen to get the state for
+ * @sw_state_ret: address where to store the privacy-screens current sw-state
+ * @hw_state_ret: address where to store the privacy-screens current hw-state
+ *
+ * Get the current state of a privacy-screen, both the sw-state and the
+ * hw-state.
+ */
+void drm_privacy_screen_get_state(struct drm_privacy_screen *priv,
+				  enum drm_privacy_screen_status *sw_state_ret,
+				  enum drm_privacy_screen_status *hw_state_ret)
+{
+	mutex_lock(&priv->lock);
+	*sw_state_ret = priv->sw_state;
+	*hw_state_ret = priv->hw_state;
+	mutex_unlock(&priv->lock);
+}
+EXPORT_SYMBOL(drm_privacy_screen_get_state);
+
+/*** drm_privacy_screen_driver.h functions ***/
+
+static ssize_t sw_state_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct drm_privacy_screen *priv = to_drm_privacy_screen(dev);
+	const char * const sw_state_names[] = {
+		"Disabled",
+		"Enabled",
+	};
+	ssize_t ret;
+
+	mutex_lock(&priv->lock);
+
+	if (!priv->ops)
+		ret = -ENODEV;
+	else if (WARN_ON(priv->sw_state >= ARRAY_SIZE(sw_state_names)))
+		ret = -ENXIO;
+	else
+		ret = sprintf(buf, "%s\n", sw_state_names[priv->sw_state]);
+
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+/*
+ * RO: Do not allow setting the sw_state through sysfs, this MUST be done
+ * through the drm_properties on the drm_connector.
+ */
+static DEVICE_ATTR_RO(sw_state);
+
+static ssize_t hw_state_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct drm_privacy_screen *priv = to_drm_privacy_screen(dev);
+	const char * const hw_state_names[] = {
+		"Disabled",
+		"Enabled",
+		"Disabled, locked",
+		"Enabled, locked",
+	};
+	ssize_t ret;
+
+	mutex_lock(&priv->lock);
+
+	if (!priv->ops)
+		ret = -ENODEV;
+	else if (WARN_ON(priv->hw_state >= ARRAY_SIZE(hw_state_names)))
+		ret = -ENXIO;
+	else
+		ret = sprintf(buf, "%s\n", hw_state_names[priv->hw_state]);
+
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+static DEVICE_ATTR_RO(hw_state);
+
+static struct attribute *drm_privacy_screen_attrs[] = {
+	&dev_attr_sw_state.attr,
+	&dev_attr_hw_state.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(drm_privacy_screen);
+
+static struct device_type drm_privacy_screen_type = {
+	.name = "privacy_screen",
+	.groups = drm_privacy_screen_groups,
+};
+
+static void drm_privacy_screen_device_release(struct device *dev)
+{
+	struct drm_privacy_screen *priv = to_drm_privacy_screen(dev);
+
+	kfree(priv);
+}
+
+/**
+ * drm_privacy_screen_register - register a privacy-screen
+ * @parent: parent-device for the privacy-screen
+ * @ops: &struct drm_privacy_screen_ops pointer with ops for the privacy-screen
+ *
+ * Create and register a privacy-screen.
+ *
+ * Return:
+ * * A pointer to the created privacy-screen on success.
+ * * 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 drm_privacy_screen *priv;
+	int ret;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return ERR_PTR(-ENOMEM);
+
+	mutex_init(&priv->lock);
+
+	priv->dev.class = drm_class;
+	priv->dev.type = &drm_privacy_screen_type;
+	priv->dev.parent = parent;
+	priv->dev.release = drm_privacy_screen_device_release;
+	dev_set_name(&priv->dev, "privacy_screen-%s", dev_name(parent));
+	priv->ops = ops;
+
+	priv->ops->get_hw_state(priv);
+
+	ret = device_register(&priv->dev);
+	if (ret) {
+		put_device(&priv->dev);
+		return ERR_PTR(ret);
+	}
+
+	mutex_lock(&drm_privacy_screen_devs_lock);
+	list_add(&priv->list, &drm_privacy_screen_devs);
+	mutex_unlock(&drm_privacy_screen_devs_lock);
+
+	return priv;
+}
+EXPORT_SYMBOL(drm_privacy_screen_register);
+
+/**
+ * drm_privacy_screen_unregister - unregister privacy-screen
+ * @priv: privacy-screen to unregister
+ *
+ * Unregister a privacy-screen registered with drm_privacy_screen_register().
+ * May be called with a NULL or ERR_PTR, in which case it is a no-op.
+ */
+void drm_privacy_screen_unregister(struct drm_privacy_screen *priv)
+{
+	if (IS_ERR_OR_NULL(priv))
+		return;
+
+	mutex_lock(&drm_privacy_screen_devs_lock);
+	list_del(&priv->list);
+	mutex_unlock(&drm_privacy_screen_devs_lock);
+
+	mutex_lock(&priv->lock);
+	priv->ops = NULL;
+	mutex_unlock(&priv->lock);
+
+	device_unregister(&priv->dev);
+}
+EXPORT_SYMBOL(drm_privacy_screen_unregister);
diff --git a/include/drm/drm_privacy_screen_consumer.h b/include/drm/drm_privacy_screen_consumer.h
new file mode 100644
index 000000000000..941c88b46889
--- /dev/null
+++ b/include/drm/drm_privacy_screen_consumer.h
@@ -0,0 +1,48 @@ 
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright (C) 2020 Red Hat, Inc.
+ *
+ * Authors:
+ * Hans de Goede <hdegoede@redhat.com>
+ */
+
+#ifndef __DRM_PRIVACY_SCREEN_CONSUMER_H__
+#define __DRM_PRIVACY_SCREEN_CONSUMER_H__
+
+#include <linux/device.h>
+#include <drm/drm_connector.h>
+
+struct drm_privacy_screen;
+
+#if IS_ENABLED(CONFIG_DRM_PRIVACY_SCREEN)
+struct drm_privacy_screen *drm_privacy_screen_get(struct device *dev,
+						  const char *con_id);
+void drm_privacy_screen_put(struct drm_privacy_screen *priv);
+
+int drm_privacy_screen_set_sw_state(struct drm_privacy_screen *priv,
+				    enum drm_privacy_screen_status sw_state);
+void drm_privacy_screen_get_state(struct drm_privacy_screen *priv,
+				  enum drm_privacy_screen_status *sw_state_ret,
+				  enum drm_privacy_screen_status *hw_state_ret);
+#else
+static inline struct drm_privacy_screen *drm_privacy_screen_get(struct device *dev,
+								const char *con_id)
+{
+	return ERR_PTR(-ENODEV);
+}
+static inline void drm_privacy_screen_put(struct drm_privacy_screen *priv)
+{
+}
+static inline int drm_privacy_screen_set_sw_state(struct drm_privacy_screen *priv,
+						  enum drm_privacy_screen_status sw_state)
+{
+	return -ENODEV;
+}
+static inline void drm_privacy_screen_get_state(struct drm_privacy_screen *priv,
+						enum drm_privacy_screen_status *sw_state_ret,
+						enum drm_privacy_screen_status *hw_state_ret)
+{
+}
+#endif
+
+#endif
diff --git a/include/drm/drm_privacy_screen_driver.h b/include/drm/drm_privacy_screen_driver.h
new file mode 100644
index 000000000000..5187ae52eb03
--- /dev/null
+++ b/include/drm/drm_privacy_screen_driver.h
@@ -0,0 +1,80 @@ 
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright (C) 2020 Red Hat, Inc.
+ *
+ * Authors:
+ * Hans de Goede <hdegoede@redhat.com>
+ */
+
+#ifndef __DRM_PRIVACY_SCREEN_DRIVER_H__
+#define __DRM_PRIVACY_SCREEN_DRIVER_H__
+
+#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <drm/drm_connector.h>
+
+struct drm_privacy_screen;
+
+/**
+ * struct drm_privacy_screen_ops - drm_privacy_screen operations
+ *
+ * Defines the operations which the privacy-screen class code may call.
+ * These functions should be implemented by the privacy-screen driver.
+ */
+struct drm_privacy_screen_ops {
+	/**
+	 * @set_sw_state: Called to request a change of the privacy-screen
+	 * state. The privacy-screen class code contains a check to avoid this
+	 * getting called when the hw_state reports the state is locked.
+	 * It is the driver's responsibility to update sw_state and hw_state.
+	 * This is always called with the drm_privacy_screen's lock held.
+	 */
+	int (*set_sw_state)(struct drm_privacy_screen *priv,
+			    enum drm_privacy_screen_status sw_state);
+	/**
+	 * @get_hw_state: Called to request that the driver gets the current
+	 * privacy-screen state from the hardware and then updates sw_state and
+	 * hw_state accordingly. This will be called by the core just before
+	 * the privacy-screen is registered in sysfs.
+	 */
+	void (*get_hw_state)(struct drm_privacy_screen *priv);
+};
+
+/**
+ * struct drm_privacy_screen - central privacy-screen structure
+ *
+ * Central privacy-screen structure, this contains the struct device used
+ * to register the screen in sysfs, the screen's state, ops, etc.
+ */
+struct drm_privacy_screen {
+	/** @dev: device used to register the privacy-screen in sysfs. */
+	struct device dev;
+	/** @lock: mutex protection all fields in this struct. */
+	struct mutex lock;
+	/** @list: privacy-screen devices list list-entry. */
+	struct list_head list;
+	/**
+	 * @ops: &struct drm_privacy_screen_ops for this privacy-screen.
+	 * This is NULL if the driver has unregistered the privacy-screen.
+	 */
+	const struct drm_privacy_screen_ops *ops;
+	/**
+	 * @sw_state: The privacy-screen's software state, see
+	 * :ref:`Standard Connector Properties<standard_connector_properties>`
+	 * for more info.
+	 */
+	enum drm_privacy_screen_status sw_state;
+	/**
+	 * @hw_state: The privacy-screen's hardware state, see
+	 * :ref:`Standard Connector Properties<standard_connector_properties>`
+	 * for more info.
+	 */
+	enum drm_privacy_screen_status hw_state;
+};
+
+struct drm_privacy_screen *drm_privacy_screen_register(
+	struct device *parent, const struct drm_privacy_screen_ops *ops);
+void drm_privacy_screen_unregister(struct drm_privacy_screen *priv);
+
+#endif
diff --git a/include/drm/drm_privacy_screen_machine.h b/include/drm/drm_privacy_screen_machine.h
new file mode 100644
index 000000000000..aaa0d38cce92
--- /dev/null
+++ b/include/drm/drm_privacy_screen_machine.h
@@ -0,0 +1,41 @@ 
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright (C) 2020 Red Hat, Inc.
+ *
+ * Authors:
+ * Hans de Goede <hdegoede@redhat.com>
+ */
+
+#ifndef __DRM_PRIVACY_SCREEN_MACHINE_H__
+#define __DRM_PRIVACY_SCREEN_MACHINE_H__
+
+#include <linux/list.h>
+
+/**
+ * struct drm_privacy_screen_lookup -  static privacy-screen lookup list entry
+ *
+ * Used for the static lookup-list for mapping privacy-screen consumer
+ * dev-connector pairs to a privacy-screen provider.
+ */
+struct drm_privacy_screen_lookup {
+	/** @list: Lookup list list-entry. */
+	struct list_head list;
+	/** @dev_id: Consumer device name or NULL to match all devices. */
+	const char *dev_id;
+	/** @con_id: Consumer connector name or NULL to match all connectors. */
+	const char *con_id;
+	/** @provider: dev_name() of the privacy_screen provider. */
+	const char *provider;
+};
+
+void drm_privacy_screen_lookup_add(struct drm_privacy_screen_lookup *lookup);
+void drm_privacy_screen_lookup_remove(struct drm_privacy_screen_lookup *lookup);
+
+static inline void drm_privacy_screen_lookup_init(void)
+{
+}
+static inline void drm_privacy_screen_lookup_exit(void)
+{
+}
+
+#endif