Re: [PATCH] drm: Add support for integrated privacy screens

[Daniel Vetter]

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 <rajatja@xxxxxxxxxx>

New properties need property docs in the relevant section:

https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#kms-properties

$ make htmldocs

for building them locally.

Cheers, Daniel

> ---
>  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 <drm/drm_atomic.h>
>  #include <drm/drm_print.h>
>  #include <drm/drm_drv.h>
> +#include <drm/drm_privacy_screen.h>
>  #include <drm/drm_writeback.h>
>  #include <drm/drm_vblank.h>
>  
> @@ -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_privacy_screen_property -
> + *	create and attach the connecter's privacy-screen property.
> + * @connector: connector for which to init the privacy-screen property.
> + *
> + * This function creates and attaches the "privacy-screen" property to the
> + * connector. Initial state of privacy-screen is set to disabled.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_init_privacy_screen_property(struct drm_connector *connector)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *prop;
> +
> +	prop = dev->mode_config.privacy_screen_property;
> +	if (!prop) {
> +		prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
> +				"privacy-screen", drm_privacy_screen_enum_list,
> +				ARRAY_SIZE(drm_privacy_screen_enum_list));
> +		if (!prop)
> +			return -ENOMEM;
> +
> +		dev->mode_config.privacy_screen_property = prop;
> +	}
> +
> +	drm_object_attach_property(&connector->base, prop,
> +				   DRM_PRIVACY_SCREEN_DISABLED);
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_init_privacy_screen_property);
> +
>  /**
>   * drm_mode_put_tile_group - drop a reference to a tile group.
>   * @dev: DRM device
> diff --git a/drivers/gpu/drm/drm_privacy_screen.c b/drivers/gpu/drm/drm_privacy_screen.c
> new file mode 100644
> index 000000000000..1d68e8aa6c5f
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_privacy_screen.c
> @@ -0,0 +1,176 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * DRM privacy Screen code
> + *
> + * Copyright © 2019 Google Inc.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/pci.h>
> +
> +#include <drm/drm_connector.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_print.h>
> +
> +#define DRM_CONN_DSM_REVID 1
> +
> +#define DRM_CONN_DSM_FN_PRIVACY_GET_STATUS	1
> +#define DRM_CONN_DSM_FN_PRIVACY_ENABLE		2
> +#define DRM_CONN_DSM_FN_PRIVACY_DISABLE		3
> +
> +static const guid_t drm_conn_dsm_guid =
> +	GUID_INIT(0xC7033113, 0x8720, 0x4CEB,
> +		  0x90, 0x90, 0x9D, 0x52, 0xB3, 0xE5, 0x2D, 0x73);
> +
> +/*
> + * Makes _DSM call to set privacy screen status or get privacy screen. Return
> + * value matters only for PRIVACY_GET_STATUS case. Returns 0 if disabled, 1 if
> + * enabled.
> + */
> +static int acpi_privacy_screen_call_dsm(acpi_handle conn_handle, u64 func)
> +{
> +	union acpi_object *obj;
> +	int ret = 0;
> +
> +	obj = acpi_evaluate_dsm(conn_handle, &drm_conn_dsm_guid,
> +				DRM_CONN_DSM_REVID, func, NULL);
> +	if (!obj) {
> +		DRM_DEBUG_DRIVER("failed to evaluate _DSM for fn %llx\n", func);
> +		/* Can't do much. For get_val, assume privacy_screen disabled */
> +		goto done;
> +	}
> +
> +	if (func == DRM_CONN_DSM_FN_PRIVACY_GET_STATUS &&
> +	    obj->type == ACPI_TYPE_INTEGER)
> +		ret = !!obj->integer.value;
> +done:
> +	ACPI_FREE(obj);
> +	return ret;
> +}
> +
> +void drm_privacy_screen_set_val(struct drm_connector *connector,
> +				 enum drm_privacy_screen val)
> +{
> +	acpi_handle acpi_handle = connector->privacy_screen_handle;
> +
> +	if (!acpi_handle)
> +		return;
> +
> +	if (val == DRM_PRIVACY_SCREEN_DISABLED)
> +		acpi_privacy_screen_call_dsm(acpi_handle,
> +					     DRM_CONN_DSM_FN_PRIVACY_DISABLE);
> +	else if (val == DRM_PRIVACY_SCREEN_ENABLED)
> +		acpi_privacy_screen_call_dsm(acpi_handle,
> +					     DRM_CONN_DSM_FN_PRIVACY_ENABLE);
> +}
> +
> +enum drm_privacy_screen drm_privacy_screen_get_val(struct drm_connector
> +						   *connector)
> +{
> +	acpi_handle acpi_handle = connector->privacy_screen_handle;
> +
> +	if (acpi_handle &&
> +	    acpi_privacy_screen_call_dsm(acpi_handle,
> +					 DRM_CONN_DSM_FN_PRIVACY_GET_STATUS))
> +		return DRM_PRIVACY_SCREEN_ENABLED;
> +
> +	return DRM_PRIVACY_SCREEN_DISABLED;
> +}
> +
> +/*
> + * See ACPI Spec v6.3, Table B-2, "Display Type" for details.
> + * In short, these macros define the base _ADR values for ACPI nodes
> + */
> +#define ACPI_BASE_ADR_FOR_OTHERS	(0ULL << 8)
> +#define ACPI_BASE_ADR_FOR_VGA		(1ULL << 8)
> +#define ACPI_BASE_ADR_FOR_TV		(2ULL << 8)
> +#define ACPI_BASE_ADR_FOR_EXT_MON	(3ULL << 8)
> +#define ACPI_BASE_ADR_FOR_INTEGRATED	(4ULL << 8)
> +
> +#define ACPI_DEVICE_ID_SCHEME		(1ULL << 31)
> +#define ACPI_FIRMWARE_CAN_DETECT	(1ULL << 16)
> +
> +/*
> + * 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. Thus 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 function looks up the ACPI node for a connector and links
> + * to it. Technically it is independent from the privacy_screen 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 a "port index" which I believe is the index within a particular
> + * type of port (Ref to the pages of spec mentioned above). This port index
> + * unfortunately is not available in DRM code, so currently its call is
> + * originated from i915 driver.
> + */
> +static int drm_connector_attach_acpi_node(struct drm_connector *connector,
> +					  u8 port_index)
> +{
> +	struct device *dev = &connector->dev->pdev->dev;
> +	struct acpi_device *conn_dev;
> +	u64 conn_addr;
> +
> +	/*
> +	 * Determine what _ADR to look for, depending on device type and
> +	 * port number. Potentially we only care about the
> +	 * eDP / integrated displays?
> +	 */
> +	switch (connector->connector_type) {
> +	case DRM_MODE_CONNECTOR_eDP:
> +		conn_addr = ACPI_BASE_ADR_FOR_INTEGRATED + port_index;
> +		break;
> +	case DRM_MODE_CONNECTOR_VGA:
> +		conn_addr = ACPI_BASE_ADR_FOR_VGA + port_index;
> +		break;
> +	case DRM_MODE_CONNECTOR_TV:
> +		conn_addr = ACPI_BASE_ADR_FOR_TV + port_index;
> +		break;
> +	case DRM_MODE_CONNECTOR_DisplayPort:
> +		conn_addr = ACPI_BASE_ADR_FOR_EXT_MON + port_index;
> +		break;
> +	default:
> +		return -ENOTSUPP;
> +	}
> +
> +	conn_addr |= ACPI_DEVICE_ID_SCHEME;
> +	conn_addr |= ACPI_FIRMWARE_CAN_DETECT;
> +
> +	DRM_DEV_DEBUG(dev, "%s: Finding drm_connector ACPI node at _ADR=%llX\n",
> +		      __func__, conn_addr);
> +
> +	/* Look up the connector device, under the PCI device */
> +	conn_dev = acpi_find_child_device(ACPI_COMPANION(dev),
> +					  conn_addr, false);
> +	if (!conn_dev)
> +		return -ENODEV;
> +
> +	connector->privacy_screen_handle = conn_dev->handle;
> +	return 0;
> +}
> +
> +bool drm_privacy_screen_present(struct drm_connector *connector, u8 port_index)
> +{
> +	acpi_handle handle;
> +
> +	if (drm_connector_attach_acpi_node(connector, port_index))
> +		return false;
> +
> +	handle = connector->privacy_screen_handle;
> +	if (!acpi_check_dsm(handle, &drm_conn_dsm_guid,
> +			    DRM_CONN_DSM_REVID,
> +			    1 << DRM_CONN_DSM_FN_PRIVACY_GET_STATUS |
> +			    1 << DRM_CONN_DSM_FN_PRIVACY_ENABLE |
> +			    1 << DRM_CONN_DSM_FN_PRIVACY_DISABLE)) {
> +		DRM_WARN("%s: Odd, connector ACPI node but no privacy scrn?\n",
> +			 connector->dev->dev);
> +		return false;
> +	}
> +	DRM_DEV_INFO(connector->dev->dev, "supports privacy screen\n");
> +	return true;
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 57e9f0ba331b..3ff3962d27db 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -39,6 +39,7 @@
>  #include <drm/drm_dp_helper.h>
>  #include <drm/drm_edid.h>
>  #include <drm/drm_hdcp.h>
> +#include <drm/drm_privacy_screen.h>
>  #include <drm/drm_probe_helper.h>
>  #include <drm/i915_drm.h>
>  
> @@ -6354,6 +6355,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>  
>  		connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
>  
> +		if (drm_privacy_screen_present(connector, port - PORT_A))
> +			drm_connector_init_privacy_screen_property(connector);
>  	}
>  }
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 681cb590f952..63b8318bd68c 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -225,6 +225,20 @@ enum drm_link_status {
>  	DRM_LINK_STATUS_BAD = DRM_MODE_LINK_STATUS_BAD,
>  };
>  
> +/**
> + * enum drm_privacy_screen - privacy_screen status
> + *
> + * This enum is used to track and control the state of the privacy screen.
> + * There are no separate #defines for the uapi!
> + *
> + * @DRM_PRIVACY_SCREEN_DISABLED: The privacy-screen on the panel is disabled
> + * @DRM_PRIVACY_SCREEN_ENABLED:  The privacy-screen on the panel is enabled
> + */
> +enum drm_privacy_screen {
> +	DRM_PRIVACY_SCREEN_DISABLED = 0,
> +	DRM_PRIVACY_SCREEN_ENABLED = 1,
> +};
> +
>  /**
>   * enum drm_panel_orientation - panel_orientation info for &drm_display_info
>   *
> @@ -1410,6 +1424,9 @@ struct drm_connector {
>  
>  	/** @hdr_sink_metadata: HDR Metadata Information read from sink */
>  	struct hdr_sink_metadata hdr_sink_metadata;
> +
> +	/* Handle used by privacy screen code */
> +	void *privacy_screen_handle;
>  };
>  
>  #define obj_to_connector(x) container_of(x, struct drm_connector, base)
> @@ -1543,6 +1560,7 @@ int drm_connector_init_panel_orientation_property(
>  	struct drm_connector *connector, int width, int height);
>  int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>  					  int min, int max);
> +int drm_connector_init_privacy_screen_property(struct drm_connector *connector);
>  
>  /**
>   * struct drm_tile_group - Tile group metadata
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 3bcbe30339f0..6d5d23da90d4 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -813,6 +813,13 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *panel_orientation_property;
>  
> +	/**
> +	 * @privacy_screen_property: Optional connector property to indicate
> +	 * and control the state (enabled / disabled) of privacy-screen on the
> +	 * panel, if present.
> +	 */
> +	struct drm_property *privacy_screen_property;
> +
>  	/**
>  	 * @writeback_fb_id_property: Property for writeback connectors, storing
>  	 * the ID of the output framebuffer.
> diff --git a/include/drm/drm_privacy_screen.h b/include/drm/drm_privacy_screen.h
> new file mode 100644
> index 000000000000..c589bbc47656
> --- /dev/null
> +++ b/include/drm/drm_privacy_screen.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright © 2019 Google Inc.
> + */
> +
> +#ifndef __DRM_PRIVACY_SCREEN_H__
> +#define __DRM_PRIVACY_SCREEN_H__
> +
> +#ifdef CONFIG_ACPI
> +bool drm_privacy_screen_present(struct drm_connector *connector, u8 port);
> +void drm_privacy_screen_set_val(struct drm_connector *connector,
> +				enum drm_privacy_screen val);
> +enum drm_privacy_screen drm_privacy_screen_get_val(struct drm_connector
> +						   *connector);
> +#else
> +static inline bool drm_privacy_screen_present(struct drm_connector *connector,
> +					      u8 port)
> +{
> +	return false;
> +}
> +
> +void drm_privacy_screen_set_val(struct drm_connector *connector,
> +				enum drm_privacy_screen val)
> +{ }
> +
> +enum drm_privacy_screen drm_privacy_screen_get_val(
> +					struct drm_connector *connector)
> +{
> +	return DRM_PRIVACY_SCREEN_DISABLED;
> +}
> +#endif /* CONFIG_ACPI */
> +
> +#endif /* __DRM_PRIVACY_SCREEN_H__ */
> -- 
> 2.23.0.866.gb869b98d4c-goog
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch