Re: [PATCH v5 11/12] drm: Add CP downstream_info property

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Apr 18, 2019 at 02:28:04PM +0530, Ramalingam C wrote:
> This patch adds a optional CP downstream info blob property to the
> connectors. This enables the Userspace to read the information of HDCP
> authenticated downstream topology.
> 
> Driver will update this blob with all downstream information at the
> end of the authentication.
> 
> In case userspace configures this platform as repeater, then this
> information is needed for the authentication with upstream HDCP
> transmitter.
> 
> v2:
>   s/cp_downstream/content_protection_downstream [daniel]
> v3:
>   s/content_protection_downstream/hdcp_topology [daniel]
> v4:
>   hdcp_topology_info struct is added with explicit padding [Daniel]
> v5:
>   Rebased.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  3 ++
>  drivers/gpu/drm/drm_connector.c   | 20 ++++++++++
>  drivers/gpu/drm/drm_hdcp.c        | 65 +++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h       |  6 +++
>  include/drm/drm_hdcp.h            |  6 +++
>  include/drm/drm_mode_config.h     |  6 +++
>  include/uapi/drm/drm_mode.h       | 37 ++++++++++++++++++
>  7 files changed, 143 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 0b0747869963..1c9e1ab0d536 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -813,6 +813,9 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>  		*val = state->content_protection;
>  	} else if (property == config->hdcp_content_type_property) {
>  		*val = state->hdcp_content_type;
> +	} else if (property == config->hdcp_topology_property) {
> +		*val = connector->hdcp_topology_blob_ptr ?
> +			connector->hdcp_topology_blob_ptr->base.id : 0;
>  	} else if (property == config->writeback_fb_id_property) {
>  		/* Writeback framebuffer is one-shot, write and forget */
>  		*val = 0;
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 436cf8e764cc..033ced774d37 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev,
>  	mutex_init(&connector->mutex);
>  	connector->edid_blob_ptr = NULL;
>  	connector->tile_blob_ptr = NULL;
> +	connector->hdcp_topology_blob_ptr = NULL;
>  	connector->status = connector_status_unknown;
>  	connector->display_info.panel_orientation =
>  		DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> @@ -972,6 +973,25 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
>   *	authentication process. If content type is changed when
>   *	content_protection is not UNDESIRED, then kernel will disable the HDCP
>   *	and re-enable with new type in the same atomic commit
> + * HDCP Topology:
> + *	This blob property is used to pass the HDCP downstream topology details
> + *	of a HDCP encrypted connector, from kernel to userspace.
> + *	This provides all required information to userspace, so that userspace
> + *	can implement the HDCP repeater using the kernel as downstream ports of
> + *	the repeater. as illustrated below:
> + *
> + *                          HDCP Repeaters
> + * +--------------------------------------------------------------+
> + * |                                                              |
> + * |                               |                              |
> + * |   Userspace HDCP Receiver  +----->    KMD HDCP transmitters  |
> + * |      (Upstream Port)      <------+     (Downstream Ports)    |
> + * |                               |                              |
> + * |                                                              |
> + * +--------------------------------------------------------------+

Hm I think this misrenders, since the table isn't indented or marked as
fixed with. Did you check the output of $make htmldocs?

> + *
> + *	Kernel will populate this blob only when the HDCP authentication is
> + *	successful.
>   *
>   * max bpc:
>   *	This range property is used by userspace to limit the bit depth. When
> diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> index 5640c4e744fe..c0d3fc93fc50 100644
> --- a/drivers/gpu/drm/drm_hdcp.c
> +++ b/drivers/gpu/drm/drm_hdcp.c
> @@ -431,3 +431,68 @@ void drm_hdcp_update_content_protection(struct drm_connector *connector,
>  				 dev->mode_config.content_protection_property);
>  }
>  EXPORT_SYMBOL(drm_hdcp_update_content_protection);
> +
> +/**
> + * drm_connector_attach_hdcp_topology_property - attach hdcp topology property
> + *
> + * @connector: connector to attach hdcp topology property with.
> + *
> + * This is used to add support for hdcp topology support on select connectors.
> + * When Intel platform is configured as repeater, this downstream info is used
> + * by userspace, to complete the repeater authentication of HDCP specification
> + * with upstream HDCP transmitter.
> + *
> + * The blob_id of the hdcp topology info will be set to
> + * &drm_connector_state.hdcp_topology
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_attach_hdcp_topology_property(struct drm_connector *connector)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *prop = dev->mode_config.hdcp_topology_property;
> +
> +	if (!prop)
> +		prop = drm_property_create(dev, DRM_MODE_PROP_BLOB |
> +					   DRM_MODE_PROP_IMMUTABLE,
> +					   "HDCP Topology", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	drm_object_attach_property(&connector->base, prop, 0);
> +	dev->mode_config.hdcp_topology_property = prop;
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_attach_hdcp_topology_property);
> +
> +/**
> + * drm_connector_update_hdcp_topology_property - update the hdcp topology
> + * property of a connector
> + * @connector: drm connector, the topology is associated to
> + * @hdcp_topology_info: new content for the blob of hdcp_topology_property
> + *
> + * This function creates a new blob modeset object and assigns its id to the
> + * connector's hdcp_topology_property.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int
> +drm_connector_update_hdcp_topology_property(struct drm_connector *connector,
> +					const struct hdcp_topology_info *info)
> +{
> +	struct drm_device *dev = connector->dev;
> +	int ret;
> +
> +	if (!info)
> +		return -EINVAL;
> +
> +	ret = drm_property_replace_global_blob(dev,
> +			&connector->hdcp_topology_blob_ptr,
> +			sizeof(struct hdcp_topology_info),
> +			info, &connector->base,
> +			dev->mode_config.hdcp_topology_property);
> +	return ret;
> +}

Hm, I'd put this into the previous function that also sends out the
uevent. That way we can guarantee that we update everyting _before_ we
send out the uevent to signal to userspace that it should take some
action.

> +EXPORT_SYMBOL(drm_connector_update_hdcp_topology_property);
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 9e2f1a9de2a0..180f8f4d4526 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1050,6 +1050,12 @@ struct drm_connector {
>  	/** @properties: property tracking for this connector */
>  	struct drm_object_properties properties;
>  
> +	/**
> +	 * @hdcp_topology_blob_ptr: DRM BLOB pointer for hdcp downstream
> +	 * topology information.
> +	 */
> +	struct drm_property_blob *hdcp_topology_blob_ptr;
> +
>  	/**
>  	 * @scaling_mode_property: Optional atomic property to control the
>  	 * upscaling. See drm_connector_attach_content_protection_property().
> diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> index 9457c7ec0d37..011f35e121a5 100644
> --- a/include/drm/drm_hdcp.h
> +++ b/include/drm/drm_hdcp.h
> @@ -300,10 +300,16 @@ struct hdcp2_srm_header {
>  
>  struct drm_device;
>  struct drm_connector;
> +struct hdcp_topology_info;
>  
>  bool drm_hdcp_ksvs_revocated(struct drm_device *dev, u8 *ksvs, u32 ksv_count);
>  int drm_connector_attach_content_protection_property(
>  		struct drm_connector *connector, bool hdcp_content_type);
>  void drm_hdcp_update_content_protection(struct drm_connector *connector,
>  					u64 val);
> +int drm_connector_attach_hdcp_topology_property(
> +				struct drm_connector *connector);
> +int drm_connector_update_hdcp_topology_property(
> +				struct drm_connector *connector,
> +				const struct hdcp_topology_info *info);
>  #endif
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index b359b5b71eb9..4349b614fa15 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -848,6 +848,12 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *hdcp_content_type_property;
>  
> +	/**
> +	 * @hdcp_topology_property: DRM BLOB property for hdcp downstream
> +	 * topology information.
> +	 */
> +	struct drm_property *hdcp_topology_property;
> +
>  	/* dumb ioctl parameters */
>  	uint32_t preferred_depth, prefer_shadow;
>  
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 8ac03351fdee..9d7cdfab4962 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -213,6 +213,43 @@ extern "C" {
>  #define DRM_MODE_HDCP_CONTENT_TYPE0		0
>  #define DRM_MODE_HDCP_CONTENT_TYPE1		1
>  
> +#define DRM_MODE_HDCP_KSV_LEN			5
> +#define DRM_MODE_HDCP_MAX_DEVICE_CNT		127
> +#define DRM_MODE_HDCP14_IN_FORCE		(1 << 0)
> +#define DRM_MODE_HDCP22_IN_FORCE		(1 << 1)
> +
> +struct hdcp_topology_info {
> +	/* Version of HDCP authenticated (1.4/2.2) */
> +	__u32 ver_in_force;
> +
> +	/* Applicable only for HDCP2.2 */
> +	__u32 content_type;
> +
> +	/* KSV of immediate HDCP Sink. In Little-Endian Format. */
> +	__u8 bksv[DRM_MODE_HDCP_KSV_LEN];
> +
> +	/* Whether Immediate HDCP sink is a repeater? */
> +	__u8 is_repeater;
> +
> +	/* Depth received from immediate downstream repeater */
> +	__u8 depth;
> +	__u8 pad1;
> +
> +	/* Device count received from immediate downstream repeater */
> +	__u32 device_count;
> +
> +	/*
> +	 * Max buffer required to hold ksv list received from immediate
> +	 * repeater. In this array first device_count * DRM_MODE_HDCP_KSV_LEN
> +	 * will hold the valid ksv bytes.
> +	 * If authentication specification is
> +	 *	HDCP1.4 - each KSV's Bytes will be in Little-Endian format.
> +	 *	HDCP2.2 - each KSV's Bytes will be in Big-Endian format.
> +	 */
> +	__u8 ksv_list[DRM_MODE_HDCP_KSV_LEN * DRM_MODE_HDCP_MAX_DEVICE_CNT];
> +	__u8 pad2[5];
> +} __packed;

So I think we need userspace for this, but not sure where the discussions
on that are right now. Is this feasible, or this and follow-up i915 patch
more fyi and not for upstream?
-Daniel

> +
>  struct drm_mode_modeinfo {
>  	__u32 clock;
>  	__u16 hdisplay;
> -- 
> 2.19.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx