Re: [PATCH v3 08/10] drm/i915: Populate downstream info for HDCP2.2

Daniel Vetter
2019-03-27T10:31:16Z

On Fri, Mar 22, 2019 at 06:14:46AM +0530, Ramalingam C wrote:
> Populates the downstream info for HDCP2.2 encryption also. On success
> of encryption Blob is updated.
>
> Additional two variable are added to downstream info blob. Such as
> ver_in_force and content type.
>
> v2:
> s/cp_downstream/content_protection_downstream [daniel]
> v3:
> s/content_protection_downstream/hdcp_topology [daniel]
>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
> drivers/gpu/drm/i915/intel_hdcp.c | 26 +++++++++++++++++++++++++-
> include/uapi/drm/drm_mode.h | 3 +++
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 0e3d7afecd5a..1eea553cdf81 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -1281,6 +1281,12 @@ static int hdcp2_authentication_key_exchange(struct intel_connector *connector)
> return -EPERM;
> }
>
> + hdcp->topology_info->ver_in_force = DRM_MODE_HDCP22_IN_FORCE;
> + hdcp->topology_info->content_type = hdcp->content_type;
> + memcpy(hdcp->topology_info->bksv, msgs.send_cert.cert_rx.receiver_id,
> + HDCP_2_2_RECEIVER_ID_LEN);
> + hdcp->topology_info->is_repeater = hdcp->is_repeater;
> +
> /*
> * Here msgs.no_stored_km will hold msgs corresponding to the km
> * stored also.
> @@ -1472,6 +1478,11 @@ int hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
> return -EPERM;
> }
>
> + hdcp->topology_info->device_count = device_cnt;
> + hdcp->topology_info->depth = HDCP_2_2_DEPTH(rx_info[0]);
> + memcpy(hdcp->topology_info->ksv_list, msgs.recvid_list.receiver_ids,
> + device_cnt * HDCP_2_2_RECEIVER_ID_LEN);
> +
> ret = hdcp2_verify_rep_topology_prepare_ack(connector,
> &msgs.recvid_list,
> &msgs.rep_ack);
> @@ -1658,6 +1669,12 @@ static int _intel_hdcp2_enable(struct intel_connector *connector)
> if (ret) {
> DRM_DEBUG_KMS("HDCP2 Type%d Enabling Failed. (%d)\n",
> hdcp->content_type, ret);
> +
> + memset(hdcp->topology_info, 0,
> + sizeof(struct hdcp_topology_info));
> + drm_connector_update_hdcp_topology_property(&connector->base,
> + hdcp->topology_info);
> +
> return ret;
> }
>
> @@ -1665,12 +1682,16 @@ static int _intel_hdcp2_enable(struct intel_connector *connector)
> connector->base.name, connector->base.base.id,
> hdcp->content_type);
>
> + drm_connector_update_hdcp_topology_property(&connector->base,
> + hdcp->topology_info);
> hdcp->hdcp2_encrypted = true;
> +
> return 0;
> }
>
> static int _intel_hdcp2_disable(struct intel_connector *connector)
> {
> + struct intel_hdcp *hdcp = &connector->hdcp;
> int ret;
>
> DRM_DEBUG_KMS("[%s:%d] HDCP2.2 is being Disabled\n",
> @@ -1681,8 +1702,11 @@ static int _intel_hdcp2_disable(struct intel_connector *connector)
> if (hdcp2_deauthenticate_port(connector) < 0)
> DRM_DEBUG_KMS("Port deauth failed.\n");
>
> - connector->hdcp.hdcp2_encrypted = false;
> + hdcp->hdcp2_encrypted = false;
>
> + memset(hdcp->topology_info, 0, sizeof(struct hdcp_topology_info));
> + drm_connector_update_hdcp_topology_property(&connector->base,
> + hdcp->topology_info);
> return ret;
> }
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 733db7283e57..22f5940a47cc 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -223,6 +223,9 @@ struct hdcp_topology_info {
> /* Version of HDCP authenticated (1.4/2.2) */
> __u32 ver_in_force;
>
> + /* Applicable only for HDCP2.2 */
> + __u8 content_type;

Even more misplaced hunks. You can't change an uapi struct once it's baked
in.

Same thought as on the hdcp1 patch, perhaps we even want 2 functions: One
for hdcp1, and one for hdcp2, to avoid the possiblity of screw-ups. After
all this interface is fairly fragile, and supposed to work across drivers.

Aside: Where's the userspace for this? Kinda wondering how an open source
hdcp repeater implementation would even work ...
-Daniel

> +
> /* KSV of immediate HDCP Sink. In Little-Endian Format. */
> char bksv[DRM_MODE_HDCP_KSV_LEN];
>
> --
> 2.19.1
>

--
Daniel Vetter
Software Engineer, Intel Corporation