Re: [Intel-gfx] [PATCH v3 01/40] drm: hdcp2.2 authentication msg definitions

[Ramalingam C <ramalingam.c@intel.com>]

Thank you for the review comments Uma Shankar!


On Wednesday 09 May 2018 03:31 PM, Shankar, Uma wrote:
>
>> -----Original Message-----
>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>> Ramalingam C
>> Sent: Tuesday, April 3, 2018 7:27 PM
>> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>> seanpaul@chromium.org; daniel@ffwll.ch; chris@chris-wilson.co.uk;
>> jani.nikula@linux.intel.com; Winkler, Tomas <tomas.winkler@intel.com>;
>> Usyskin, Alexander <alexander.usyskin@intel.com>
>> Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>	
>> Subject: [Intel-gfx] [PATCH v3 01/40] drm: hdcp2.2 authentication msg definitions
>>
>> This patch defines the hdcp2.2 protocol messages for the
> Drop redundant "the" and one of the hdcp2.2. Not required.
ok
>
>> HDCP2.2 authentication.
>>
>> v2:
>>   bit_fields are removed. Instead bitmasking used. [Tomas and Jani]
>>   prefix HDCP_2_2_ is added to the macros. [Tomas]
>> v3:
>>   No Changes.
>>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> ---
>> include/drm/drm_hdcp.h | 183
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 183 insertions(+)
>>
>> diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h index
>> 562fa7df2637..5e0a5ed1a08e 100644
>> --- a/include/drm/drm_hdcp.h
>> +++ b/include/drm/drm_hdcp.h
>> @@ -38,4 +38,187 @@
>> #define DRM_HDCP_DDC_BSTATUS			0x41
>> #define DRM_HDCP_DDC_KSV_FIFO			0x43
>>
>> +#define DRM_HDCP_1_4_SRM_ID			0x8
>> +#define DRM_HDCP_1_4_VRL_LENGTH_SIZE		3
>> +#define DRM_HDCP_1_4_DCP_SIG_SIZE		40
>> +
>> +/**
>> + * Protocol message definition for HDCP2.2 specification  */
> Multi Line comment style not needed here.
Ok
>
>> +
>> +#define HDCP_STREAM_TYPE0			0x00
>> +#define HDCP_STREAM_TYPE1			0x01
>> +
>> +/* HDCP2.2 Msg IDs */
>> +#define HDCP_2_2_NULL_MSG			1
>> +#define HDCP_2_2_AKE_INIT			2
>> +#define HDCP_2_2_AKE_SEND_CERT			3
>> +#define HDCP_2_2_AKE_NO_STORED_KM		4
>> +#define HDCP_2_2_AKE_STORED_KM			5
>> +#define HDCP_2_2_AKE_SEND_HPRIME		7
>> +#define HDCP_2_2_AKE_SEND_PARING_INFO		8
> Typo in Pairing
Fixed
>
>> +#define HDCP_2_2_LC_INIT			9
>> +#define HDCP_2_2_LC_SEND_LPRIME			10
>> +#define HDCP_2_2_SKE_SEND_EKS			11
>> +#define HDCP_2_2_REP_SEND_RECVID_LIST		12
>> +#define HDCP_2_2_REP_SEND_ACK			15
>> +#define HDCP_2_2_REP_STREAM_MANAGE		16
>> +#define HDCP_2_2_REP_STREAM_READY		17
>> +#define HDCP_2_2_ERRATA_DP_STREAM_TYPE		50
>> +
>> +#define HDCP_2_2_RTX_LEN			8
>> +#define HDCP_2_2_RRX_LEN			8
>> +
>> +#define HDCP_2_2_K_PUB_RX_MOD_N_LEN		128
>> +#define HDCP_2_2_K_PUB_RX_EXP_E_LEN		3
>> +#define HDCP_2_2_K_PUB_RX_LEN
>> 	(HDCP_2_2_K_PUB_RX_MOD_N_LEN + \
>> +
>> HDCP_2_2_K_PUB_RX_EXP_E_LEN)
>> +
>> +#define HDCP_2_2_DCP_LLC_SIG_LEN		384
>> +
>> +#define HDCP_2_2_E_KPUB_KM_LEN			128
>> +#define HDCP_2_2_E_KH_KM_M_LEN			(16 + 16)
>> +#define HDCP_2_2_H_PRIME_LEN			32
>> +#define HDCP_2_2_E_KH_KM_LEN			16
>> +#define HDCP_2_2_RN_LEN				8
>> +#define HDCP_2_2_L_PRIME_LEN			32
>> +#define HDCP_2_2_E_DKEY_KS_LEN			16
>> +#define HDCP_2_2_RIV_LEN			8
>> +#define HDCP_2_2_SEQ_NUM_LEN			3
>> +#define HDCP_2_2_LPRIME_HALF_LEN		(HDCP_2_2_L_PRIME_LEN / 2)
>> +#define HDCP_2_2_RECEIVER_ID_LEN		DRM_HDCP_KSV_LEN
>> +#define HDCP_2_2_MAX_DEVICE_COUNT		31
>> +#define HDCP_2_2_RECEIVER_IDS_MAX_LEN
>> 	(HDCP_2_2_RECEIVER_ID_LEN * \
>> +
>> HDCP_2_2_MAX_DEVICE_COUNT)
>> +#define HDCP_2_2_MPRIME_LEN			32
>> +
>> +/**
> Remove an extra "*".
>
>> + * TODO: This has to be changed for DP MST, as multiple stream on
>> + * same port is possible.
>> + * For HDCP2.2 on HDMI and DP SST this value is always 1.
>> + */
>> +#define HDCP_2_2_MAX_CONTENT_STREAMS_CNT	1
>> +#define HDCP_2_2_TXCAP_MASK_LEN			2
>> +#define HDCP_2_2_RXCAPS_LEN			3
>> +#define HDCP_2_2_RX_REPEATER(x)			(x & BIT(0))
>> +#define HDCP_2_2_DP_HDCP_CAPABLE(x)		(x & BIT(1))
>> +#define HDCP_2_2_RXINFO_LEN			2
>> +
>> +/* HDCP1.x compliant device in downstream */
>> +#define HDCP_2_2_HDCP1_DEVICE_CONNECTED(x)	(x & BIT(0))
>> +
>> +/* HDCP2.0 Compliant repeater in downstream */
>> +#define HDCP_2_2_HDCP_2_0_REP_CONNECTED(x)	(x & BIT(1))
>> +#define HDCP_2_2_MAX_CASCADE_EXCEEDED(x)	(x & BIT(2))
>> +#define HDCP_2_2_MAX_DEVS_EXCEEDED(x)		(x & BIT(3))
>> +#define HDCP_2_2_DEV_COUNT_LO(x)		((x & (0xF << 4)) >> 4)
>> +#define HDCP_2_2_DEV_COUNT_HI(x)		(x & BIT(0))
> As per 2.2 spec this is a 5 bit field and no mention of HI and LO is there for dev count.
> A comment explaining why this masking style is chosen will be helpful since it doesn't
> match exactly with spec.
dev count is spread across multi byte value. And these macros are processing
the each byte separately. Agreed that a comment would help here.
>
>> +#define HDCP_2_2_DEPTH(x)			((x & (0x7 << 1)) >> 1)
> Same here, may be add a generic comment explaining the logic for
> all the relevant macro definitions.
>> +
>> +struct hdcp2_cert_rx {
>> +	uint8_t	receiver_id[HDCP_2_2_RECEIVER_ID_LEN];
>> +	uint8_t	kpub_rx[HDCP_2_2_K_PUB_RX_LEN];
>> +	uint8_t	reserved[2];
>> +	uint8_t	dcp_signature[HDCP_2_2_DCP_LLC_SIG_LEN];
>> +} __packed;
>> +
>> +struct hdcp2_streamid_type {
>> +	uint8_t stream_id;
>> +	uint8_t stream_type;
>> +} __packed;
>> +
>> +/**
> Drop the extra "*".
Fixed.
>
>> + * The TxCaps field specified in the HDCP HDMI, DP specs
>> + * This field is big endian as specified in the errata.
>> + */
>> +struct hdcp2_tx_caps {
>> +	/* Transmitter must set this to 0x2 */
>> +	uint8_t			version;
>> +
>> +	/* Reserved for HDCP and DP Spec. Read as Zero */
>> +	uint8_t			tx_cap_mask[HDCP_2_2_TXCAP_MASK_LEN];
> Drop extra tabs here.
Keeping the extra tabs for the uniformity. If it is ok, I would like to 
keep it as it is now.
>
>> +} __packed;
>> +
>> +/*
>> + * Main structures for HDCP2.2 protocol communication  */ struct
> No need of multi-line comment style.
>
>> +hdcp2_ake_init {
>> +	uint8_t			msg_id;
>> +	uint8_t			r_tx[HDCP_2_2_RTX_LEN];
>> +	struct hdcp2_tx_caps	tx_caps;
>> +} __packed;
>> +
>> +struct hdcp2_ake_send_cert {
>> +	uint8_t			msg_id;
>> +	struct hdcp2_cert_rx	cert_rx;
>> +	uint8_t			r_rx[HDCP_2_2_RRX_LEN];
>> +	uint8_t			rx_caps[HDCP_2_2_RXCAPS_LEN];
>> +} __packed;
>> +
>> +struct hdcp2_ake_no_stored_km {
>> +	uint8_t			msg_id;
>> +	uint8_t			e_kpub_km[HDCP_2_2_E_KPUB_KM_LEN];
> Drop extra tabs. Do it universally in all structure definitions.
>
>> +} __packed;
>> +
>> +struct hdcp2_ake_stored_km {
>> +	uint8_t			msg_id;
>> +	uint8_t			e_kh_km_m[HDCP_2_2_E_KH_KM_M_LEN];
>> +} __packed;
>> +
>> +struct hdcp2_ake_send_hprime {
>> +	uint8_t			msg_id;
>> +	uint8_t			h_prime[HDCP_2_2_H_PRIME_LEN];
>> +} __packed;
>> +
>> +struct hdcp2_ake_send_pairing_info {
>> +	uint8_t			msg_id;
>> +	uint8_t			e_kh_km[HDCP_2_2_E_KH_KM_LEN];
>> +} __packed;
>> +
>> +struct hdcp2_lc_init {
>> +	uint8_t			msg_id;
>> +	uint8_t			r_n[HDCP_2_2_RN_LEN];
>> +} __packed;
>> +
>> +struct hdcp2_lc_send_lprime {
>> +	uint8_t			msg_id;
>> +	uint8_t			l_prime[HDCP_2_2_L_PRIME_LEN];
>> +} __packed;
>> +
>> +struct hdcp2_ske_send_eks {
>> +	uint8_t			msg_id;
>> +	uint8_t			e_dkey_ks[HDCP_2_2_E_DKEY_KS_LEN];
>> +	uint8_t			riv[HDCP_2_2_RIV_LEN];
>> +} __packed;
>> +
>> +struct hdcp2_rep_send_receiverid_list {
>> +	uint8_t			msg_id;
>> +	uint8_t			rx_info[HDCP_2_2_RXINFO_LEN];
>> +	uint8_t			seq_num_v[HDCP_2_2_SEQ_NUM_LEN];
>> +	uint8_t			v_prime[HDCP_2_2_LPRIME_HALF_LEN];
>> +	uint8_t
>> 	receiver_ids[HDCP_2_2_RECEIVER_IDS_MAX_LEN];
>> +} __packed;
>> +
>> +struct hdcp2_rep_send_ack {
>> +	uint8_t			msg_id;
>> +	uint8_t			v[HDCP_2_2_LPRIME_HALF_LEN];
>> +} __packed;
>> +
>> +struct hdcp2_rep_stream_manage {
>> +	uint8_t			msg_id;
>> +	uint8_t			seq_num_m[HDCP_2_2_SEQ_NUM_LEN];
>> +	__be16			k;
>> +	struct hdcp2_streamid_type
>> streams[HDCP_2_2_MAX_CONTENT_STREAMS_CNT];
>> +} __packed;
>> +
>> +struct hdcp2_rep_stream_ready {
>> +	uint8_t			msg_id;
>> +	uint8_t			m_prime[HDCP_2_2_MPRIME_LEN];
>> +} __packed;
>> +
>> +struct hdcp2_dp_errata_stream_type {
>> +	uint8_t			msg_id;
>> +	uint8_t			stream_type;
>> +} __packed;
>> +
>> #endif
>> --
>> 2.7.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel