Re: [PATCH v3 07/10] drm/i915: Populate downstream info for HDCP1.4

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Mar 22, 2019 at 06:14:45AM +0530, Ramalingam C wrote:
> Implements drm blob property content_protection_downstream_info
> property on HDCP capable connectors.
> 
> Downstream topology info is gathered across authentication stages
> and stored in intel_hdcp. When HDCP authentication is complete,
> new blob with latest downstream topology information is updated to
> content_protection_downstream_info property.
> 
> 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_drv.h  |  2 ++
>  drivers/gpu/drm/i915/intel_hdcp.c | 30 +++++++++++++++++++++++++++++-
>  include/drm/drm_hdcp.h            |  1 +
>  include/uapi/drm/drm_mode.h       |  5 +++++
>  4 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4db15ee81042..46325fb33507 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -482,6 +482,8 @@ struct intel_hdcp {
>  	wait_queue_head_t cp_irq_queue;
>  	atomic_t cp_irq_count;
>  	int cp_irq_count_cached;
> +
> +	struct hdcp_topology_info *topology_info;
>  };
>  
>  struct intel_connector {
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 921f07c64350..0e3d7afecd5a 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -580,6 +580,9 @@ int intel_hdcp_auth_downstream(struct intel_hdcp *hdcp,
>  	if (num_downstream == 0)
>  		return -EINVAL;
>  
> +	hdcp->topology_info->device_count = num_downstream;
> +	hdcp->topology_info->depth = DRM_HDCP_DEPTH(bstatus[1]);
> +
>  	ksv_fifo = kcalloc(DRM_HDCP_KSV_LEN, num_downstream, GFP_KERNEL);
>  	if (!ksv_fifo)
>  		return -ENOMEM;
> @@ -593,6 +596,8 @@ int intel_hdcp_auth_downstream(struct intel_hdcp *hdcp,
>  		return -EPERM;
>  	}
>  
> +	memcpy(hdcp->topology_info->ksv_list, ksv_fifo,
> +	       num_downstream * DRM_HDCP_KSV_LEN);
>  	/*
>  	 * When V prime mismatches, DP Spec mandates re-read of
>  	 * V prime atleast twice.
> @@ -694,15 +699,20 @@ static int intel_hdcp_auth(struct intel_connector *connector)
>  		return -EPERM;
>  	}
>  
> +	hdcp->topology_info->ver_in_force = DRM_MODE_HDCP14_IN_FORCE;
> +	memcpy(hdcp->topology_info->bksv, bksv.shim, DRM_MODE_HDCP_KSV_LEN);
> +
>  	I915_WRITE(PORT_HDCP_BKSVLO(port), bksv.reg[0]);
>  	I915_WRITE(PORT_HDCP_BKSVHI(port), bksv.reg[1]);
>  
>  	ret = shim->repeater_present(intel_dig_port, &repeater_present);
>  	if (ret)
>  		return ret;
> -	if (repeater_present)
> +	if (repeater_present) {
>  		I915_WRITE(HDCP_REP_CTL,
>  			   intel_hdcp_get_repeater_ctl(intel_dig_port));
> +		hdcp->topology_info->is_repeater = true;
> +	}
>  
>  	ret = shim->toggle_signalling(intel_dig_port, true);
>  	if (ret)
> @@ -798,6 +808,12 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
>  		return ret;
>  	}
>  
> +	memset(hdcp->topology_info, 0, sizeof(struct hdcp_topology_info));
> +
> +	if (drm_connector_update_hdcp_topology_property(&connector->base,
> +						connector->hdcp.topology_info))
> +		DRM_ERROR("Downstream_info update failed.\n");
> +
>  	DRM_DEBUG_KMS("HDCP is disabled\n");
>  	return 0;
>  }
> @@ -831,6 +847,10 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
>  		ret = intel_hdcp_auth(connector);
>  		if (!ret) {
>  			connector->hdcp.hdcp_encrypted = true;
> +			if (drm_connector_update_hdcp_topology_property(
> +					&connector->base,
> +					connector->hdcp.topology_info))
> +				DRM_ERROR("Downstream_info update failed.\n");
>  			return 0;
>  		}
>  
> @@ -1882,6 +1902,14 @@ int intel_hdcp_init(struct intel_connector *connector,
>  	if (ret)
>  		return ret;
>  
> +	ret = drm_connector_attach_hdcp_topology_property(&connector->base);
> +	if (ret)
> +		return ret;
> +
> +	hdcp->topology_info = kzalloc(sizeof(*hdcp->topology_info), GFP_KERNEL);
> +	if (!hdcp->topology_info)
> +		return -ENOMEM;

Since you need to assemble the info struct manually from information from
all over: What if we make that the interface of the update function (i.e.
you pass it the list of ksv, bksv, and a bunch of flags), and the core
function bakes that into a structure and correct property? Would be a
slightly cleaner interface I think, and no risk that someone accidentally
forgets to set one of the values and leaves something stale behind.

> +
>  	hdcp->shim = shim;
>  	mutex_init(&hdcp->mutex);
>  	INIT_DELAYED_WORK(&hdcp->check_work, intel_hdcp_check_work);
> diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
> index 652aaf5d658e..654a170f03eb 100644
> --- a/include/drm/drm_hdcp.h
> +++ b/include/drm/drm_hdcp.h
> @@ -23,6 +23,7 @@
>  #define DRM_HDCP_V_PRIME_PART_LEN		4
>  #define DRM_HDCP_V_PRIME_NUM_PARTS		5
>  #define DRM_HDCP_NUM_DOWNSTREAM(x)		(x & 0x7f)
> +#define DRM_HDCP_DEPTH(x)			((x) & 0x7)
>  #define DRM_HDCP_MAX_CASCADE_EXCEEDED(x)	(x & BIT(3))
>  #define DRM_HDCP_MAX_DEVICE_EXCEEDED(x)		(x & BIT(7))
>  
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 03d3aa2b1a49..733db7283e57 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -216,8 +216,13 @@ extern "C" {
>  
>  #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;

Looks like misplaced hunk. Maybe use one of the __u8 padding fields you
need anyway.
-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
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx