On Fri, Mar 22, 2019 at 06:14:44AM +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 updated 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]
> 
> Signed-off-by: Ramalingam C <ramalinga...@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
>  drivers/gpu/drm/drm_connector.c   | 86 +++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h       | 12 +++++
>  include/uapi/drm/drm_mode.h       | 27 ++++++++++
>  4 files changed, 129 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 857ca6fa6fd0..4246e8988c29 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -826,6 +826,10 @@ drm_atomic_connector_get_property(struct drm_connector 
> *connector,
>               *val = state->content_protection;
>       } else if (property == connector->hdcp_content_type_property) {
>               *val = state->hdcp_content_type;
> +     } else if (property ==
> +                connector->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 ff61c3208307..0de8b441a449 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;
> @@ -986,6 +987,25 @@ DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name,
>   *   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)    |
> + * |                               |                              |
> + * |                                                              |
> + * +--------------------------------------------------------------+

I didn't check, but I think this doesn't format correctly in the html
output. I think you need to indent, and start with :: to denote a fixed
width font example.

> + *
> + *   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
> @@ -1614,6 +1634,72 @@ drm_connector_attach_hdcp_content_type_property(struct 
> drm_connector *
>  }
>  EXPORT_SYMBOL(drm_connector_attach_hdcp_content_type_property);
>  
> +/**
> + * 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;
> +
> +     prop = drm_property_create(dev, DRM_MODE_PROP_BLOB |
> +                                DRM_MODE_PROP_IMMUTABLE,
> +                                "HDCP Topology", 0);

Again global prop in dev->mode_config, and I think just add a flag to the
overall "attach content protection stuff to connnector" function.

> +     if (!prop)
> +             return -ENOMEM;
> +
> +     drm_object_attach_property(&connector->base, prop, 0);
> +
> +     connector->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,
> +                     connector->hdcp_topology_property);
> +     return ret;
> +}
> +EXPORT_SYMBOL(drm_connector_update_hdcp_topology_property);
> +
>  /**
>   * drm_mode_create_aspect_ratio_property - create aspect ratio property
>   * @dev: DRM device
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index f0830985367f..c016a0bcedac 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1047,6 +1047,13 @@ struct drm_connector {
>        */
>       struct drm_property *hdcp_content_type_property;
>  
> +     /**
> +      * @hdcp_topology_property: DRM BLOB property for hdcp downstream
> +      * topology information.
> +      */
> +     struct drm_property *hdcp_topology_property;
> +     struct drm_property_blob *hdcp_topology_blob_ptr;

Need kerneldoc for both or kerneldoc gets a bit unhappy.

> +
>       /**
>        * @path_blob_ptr:
>        *
> @@ -1324,6 +1331,11 @@ int drm_connector_attach_content_protection_property(
>               struct drm_connector *connector);
>  int drm_connector_attach_hdcp_content_type_property(
>               struct drm_connector *connector);
> +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);
>  int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
>  int drm_mode_create_colorspace_property(struct drm_connector *connector);
>  int drm_mode_create_content_type_property(struct drm_device *dev);
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 44412e8b77cd..03d3aa2b1a49 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -214,6 +214,33 @@ 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
> +
> +struct hdcp_topology_info {
> +     /* KSV of immediate HDCP Sink. In Little-Endian Format. */
> +     char bksv[DRM_MODE_HDCP_KSV_LEN];

This isn't aligned to __u32. Just make the length 128 bytes.

> +
> +     /* Whether Immediate HDCP sink is a repeater? */
> +     bool is_repeater;

no bool in uapi structs. Just go with __u8.

> +
> +     /* Depth received from immediate downstream repeater */
> +     __u8 depth;

Needs to be aligned with explicit padding.

> +
> +     /* 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.

Why is this ksv list be for hdcp2.2, but bksv is le for both case? I'm
confused.

> +      */
> +     char ksv_list[DRM_MODE_HDCP_KSV_LEN * DRM_MODE_HDCP_MAX_DEVICE_CNT];

Again better to align this. Also maybe make it a nested array (it's the
same underlying layout, but easier to use for userspace.)

For uapi struct recommendations, see 
https://gitlab.com/TeeFirefly/linux-kernel/blob/7408b38cfdf9b0c6c3bda97402c75bd27ef69a85/Documentation/ioctl/botching-up-ioctls.txt

Cheers, Daniel
> +};
> +
>  struct drm_mode_modeinfo {
>       __u32 clock;
>       __u16 hdisplay;
> -- 
> 2.19.1
> 

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

Reply via email to