diff mbox series

[v6,2/3] drm/i915/hdcp: read RxInfo once when reading RepeaterAuth_Send_ReceiverID_List

Message ID 20210819184835.1181323-3-juston.li@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/hdcp: HDCP2.2 MST dock fixes | expand

Commit Message

Juston Li Aug. 19, 2021, 6:48 p.m. UTC 
When reading RepeaterAuth_Send_ReceiverID_List, RxInfo is read by itself
once to retrieve the DEVICE_COUNT to calculate the size of the
ReceiverID list then read a second time as a part of reading ReceiverID
list.

On some MST docking stations, RxInfo can only be read after the RxStatus
READY bit is set otherwise the read will return -EIO. The spec states that
the READY bit should be cleared as soon as RxInfo has been read.

In this case, the first RxInfo read succeeds but after the READY bit is
cleared, the second read fails.

Fix it by reading RxInfo once and storing it before reading the rest of
RepeaterAuth_Send_ReceiverID_List once we know the size.

Modify get_receiver_id_list_size() to read and store RxInfo in the
message buffer and also parse DEVICE_COUNT so we know the size of
RepeaterAuth_Send_ReceiverID_List.

Afterwards, retrieve the rest of the message at the offset for
seq_num_V.

Changes in v5:
- Don't change the offset define for Send_ReceiverID_List
  When reading, update message offset to account for RxInfo being read

Changes in v4:
- rebase and edit commit message

Changes in v3:
- remove comment

Changes in v2:
- remove unnecessary moving of drm_i915_private from patch 1

Signed-off-by: Juston Li <juston.li@intel.com>
Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 31 ++++++++++----------
 1 file changed, 16 insertions(+), 15 deletions(-)

Comments

Gupta, Anshuman Sept. 2, 2021, 1:09 p.m. UTC | #1 
> -----Original Message-----
> From: Li, Juston <juston.li@intel.com>
> Sent: Friday, August 20, 2021 12:19 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: seanpaul@chromium.org; Gupta, Anshuman <anshuman.gupta@intel.com>;
> C, Ramalingam <ramalingam.c@intel.com>; Vivi, Rodrigo
> <rodrigo.vivi@intel.com>; Li, Juston <juston.li@intel.com>
> Subject: [Intel-gfx] [PATCH v6 2/3] drm/i915/hdcp: read RxInfo once when
> reading RepeaterAuth_Send_ReceiverID_List
> 
> When reading RepeaterAuth_Send_ReceiverID_List, RxInfo is read by itself once
> to retrieve the DEVICE_COUNT to calculate the size of the ReceiverID list then
> read a second time as a part of reading ReceiverID list.
> 
> On some MST docking stations, RxInfo can only be read after the RxStatus
> READY bit is set otherwise the read will return -EIO. The spec states that the
> READY bit should be cleared as soon as RxInfo has been read.
> 
> In this case, the first RxInfo read succeeds but after the READY bit is cleared, the
> second read fails.
> 
> Fix it by reading RxInfo once and storing it before reading the rest of
> RepeaterAuth_Send_ReceiverID_List once we know the size.
> 
> Modify get_receiver_id_list_size() to read and store RxInfo in the message
> buffer and also parse DEVICE_COUNT so we know the size of
> RepeaterAuth_Send_ReceiverID_List.
> 
> Afterwards, retrieve the rest of the message at the offset for seq_num_V.
> 
> Changes in v5:
> - Don't change the offset define for Send_ReceiverID_List
>   When reading, update message offset to account for RxInfo being read
> 
> Changes in v4:
> - rebase and edit commit message
> 
> Changes in v3:
> - remove comment
> 
> Changes in v2:
> - remove unnecessary moving of drm_i915_private from patch 1
> 
> Signed-off-by: Juston Li <juston.li@intel.com>
> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
Adding Suraj to this thread.
> ---
>  drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 31 ++++++++++----------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> index 1d0096654776..fbfb3c4d16bb 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> @@ -496,11 +496,10 @@ get_rxinfo_hdcp_1_dev_downstream(struct
> intel_digital_port *dig_port, bool *hdcp  }
> 
>  static
> -ssize_t get_receiver_id_list_size(struct intel_digital_port *dig_port)
> +ssize_t get_receiver_id_list_rx_info(struct intel_digital_port
> +*dig_port, u32 *dev_cnt, u8 *byte)
>  {
> -	u8 rx_info[HDCP_2_2_RXINFO_LEN];
> -	u32 dev_cnt;
>  	ssize_t ret;
> +	u8 *rx_info = byte;
> 
>  	ret = drm_dp_dpcd_read(&dig_port->dp.aux,
>  			       DP_HDCP_2_2_REG_RXINFO_OFFSET, @@ -508,15
> +507,11 @@ ssize_t get_receiver_id_list_size(struct intel_digital_port
> *dig_port)
>  	if (ret != HDCP_2_2_RXINFO_LEN)
>  		return ret >= 0 ? -EIO : ret;
> 
> -	dev_cnt = (HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 |
> +	*dev_cnt = (HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 |
>  		   HDCP_2_2_DEV_COUNT_LO(rx_info[1]));
> 
> -	if (dev_cnt > HDCP_2_2_MAX_DEVICE_COUNT)
> -		dev_cnt = HDCP_2_2_MAX_DEVICE_COUNT;
> -
> -	ret = sizeof(struct hdcp2_rep_send_receiverid_list) -
> -		HDCP_2_2_RECEIVER_IDS_MAX_LEN +
> -		(dev_cnt * HDCP_2_2_RECEIVER_ID_LEN);
> +	if (*dev_cnt > HDCP_2_2_MAX_DEVICE_COUNT)
> +		*dev_cnt = HDCP_2_2_MAX_DEVICE_COUNT;
> 
>  	return ret;
>  }
> @@ -534,6 +529,7 @@ int intel_dp_hdcp2_read_msg(struct intel_digital_port
> *dig_port,
>  	const struct hdcp2_dp_msg_data *hdcp2_msg_data;
>  	ktime_t msg_end = ktime_set(0, 0);
>  	bool msg_expired;
> +	u32 dev_cnt;
> 
>  	hdcp2_msg_data = get_hdcp2_dp_msg_data(msg_id);
>  	if (!hdcp2_msg_data)
> @@ -546,17 +542,22 @@ int intel_dp_hdcp2_read_msg(struct intel_digital_port
> *dig_port,
> 
>  	hdcp->cp_irq_count_cached = atomic_read(&hdcp->cp_irq_count);
> 
> +	/* DP adaptation msgs has no msg_id */
> +	byte++;
> +
>  	if (msg_id == HDCP_2_2_REP_SEND_RECVID_LIST) {
> -		ret = get_receiver_id_list_size(dig_port);
> +		ret = get_receiver_id_list_rx_info(dig_port, &dev_cnt, byte);
>  		if (ret < 0)
>  			return ret;
> 
> -		size = ret;
> +		byte += ret;
> +		size = sizeof(struct hdcp2_rep_send_receiverid_list) -
> +		HDCP_2_2_RXINFO_LEN - HDCP_2_2_RECEIVER_IDS_MAX_LEN
> +
> +		(dev_cnt * HDCP_2_2_RECEIVER_ID_LEN);
> +		offset += HDCP_2_2_RXINFO_LEN;
>  	}
> -	bytes_to_recv = size - 1;
> 
> -	/* DP adaptation msgs has no msg_id */
> -	byte++;
> +	bytes_to_recv = size - 1;
> 
>  	while (bytes_to_recv) {
>  		len = bytes_to_recv > DP_AUX_MAX_PAYLOAD_BYTES ?
> --
> 2.31.1
Kandpal, Suraj Sept. 3, 2021, 5:23 a.m. UTC | #2 
Hi ,

I have tested the patch 20856 against ci_drm_10499 and the patch 20921 against ci_drm_10537 kernel using the IGT kms_test_protection test on TGL platform with the following setups:
- HCDP2.2 Monitor connected to HDCP 2.2 Monitor
- HCDP2.2 Monitor connected to HDCP 1.4 Monitor
- HCDP1.4 Monitor connected to HDCP 1.4 Monitor
- HCDP2.2 Monitor standalone
- HDCP1.4 Monitor standalone 

I did not see any delta between the Patchwork and CI_DRM kernel and all DP MST Scenarios seem to work fine. We can go ahead and merge this patch.

 

Tested-by: Suraj K <suraj.kandpal@intel.com>

-----Original Message-----
From: Gupta, Anshuman <anshuman.gupta@intel.com> 
Sent: Thursday, September 2, 2021 6:39 PM
To: Li, Juston <juston.li@intel.com>; intel-gfx@lists.freedesktop.org; Kandpal, Suraj <suraj.kandpal@intel.com>
Cc: seanpaul@chromium.org; C, Ramalingam <ramalingam.c@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>
Subject: RE: [Intel-gfx] [PATCH v6 2/3] drm/i915/hdcp: read RxInfo once when reading RepeaterAuth_Send_ReceiverID_List



> -----Original Message-----
> From: Li, Juston <juston.li@intel.com>
> Sent: Friday, August 20, 2021 12:19 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: seanpaul@chromium.org; Gupta, Anshuman <anshuman.gupta@intel.com>; 
> C, Ramalingam <ramalingam.c@intel.com>; Vivi, Rodrigo 
> <rodrigo.vivi@intel.com>; Li, Juston <juston.li@intel.com>
> Subject: [Intel-gfx] [PATCH v6 2/3] drm/i915/hdcp: read RxInfo once 
> when reading RepeaterAuth_Send_ReceiverID_List
> 
> When reading RepeaterAuth_Send_ReceiverID_List, RxInfo is read by 
> itself once to retrieve the DEVICE_COUNT to calculate the size of the 
> ReceiverID list then read a second time as a part of reading ReceiverID list.
> 
> On some MST docking stations, RxInfo can only be read after the 
> RxStatus READY bit is set otherwise the read will return -EIO. The 
> spec states that the READY bit should be cleared as soon as RxInfo has been read.
> 
> In this case, the first RxInfo read succeeds but after the READY bit 
> is cleared, the second read fails.
> 
> Fix it by reading RxInfo once and storing it before reading the rest 
> of RepeaterAuth_Send_ReceiverID_List once we know the size.
> 
> Modify get_receiver_id_list_size() to read and store RxInfo in the 
> message buffer and also parse DEVICE_COUNT so we know the size of 
> RepeaterAuth_Send_ReceiverID_List.
> 
> Afterwards, retrieve the rest of the message at the offset for seq_num_V.
> 
> Changes in v5:
> - Don't change the offset define for Send_ReceiverID_List
>   When reading, update message offset to account for RxInfo being read
> 
> Changes in v4:
> - rebase and edit commit message
> 
> Changes in v3:
> - remove comment
> 
> Changes in v2:
> - remove unnecessary moving of drm_i915_private from patch 1
> 
> Signed-off-by: Juston Li <juston.li@intel.com>
> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
Adding Suraj to this thread.
> ---
>  drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 31 
> ++++++++++----------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> index 1d0096654776..fbfb3c4d16bb 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> @@ -496,11 +496,10 @@ get_rxinfo_hdcp_1_dev_downstream(struct
> intel_digital_port *dig_port, bool *hdcp  }
> 
>  static
> -ssize_t get_receiver_id_list_size(struct intel_digital_port 
> *dig_port)
> +ssize_t get_receiver_id_list_rx_info(struct intel_digital_port 
> +*dig_port, u32 *dev_cnt, u8 *byte)
>  {
> -u8 rx_info[HDCP_2_2_RXINFO_LEN];
> -u32 dev_cnt;
>  ssize_t ret;
> +u8 *rx_info = byte;
> 
>  ret = drm_dp_dpcd_read(&dig_port->dp.aux,
>         DP_HDCP_2_2_REG_RXINFO_OFFSET, @@ -508,15
> +507,11 @@ ssize_t get_receiver_id_list_size(struct intel_digital_port
> *dig_port)
>  if (ret != HDCP_2_2_RXINFO_LEN)
>  return ret >= 0 ? -EIO : ret;
> 
> -dev_cnt = (HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 |
> +*dev_cnt = (HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 |
>     HDCP_2_2_DEV_COUNT_LO(rx_info[1]));
> 
> -if (dev_cnt > HDCP_2_2_MAX_DEVICE_COUNT) -dev_cnt = 
> HDCP_2_2_MAX_DEVICE_COUNT;
> -
> -ret = sizeof(struct hdcp2_rep_send_receiverid_list) - 
> -HDCP_2_2_RECEIVER_IDS_MAX_LEN + -(dev_cnt * 
> HDCP_2_2_RECEIVER_ID_LEN);
> +if (*dev_cnt > HDCP_2_2_MAX_DEVICE_COUNT) *dev_cnt = 
> +HDCP_2_2_MAX_DEVICE_COUNT;
> 
>  return ret;
>  }
> @@ -534,6 +529,7 @@ int intel_dp_hdcp2_read_msg(struct 
> intel_digital_port *dig_port,  const struct hdcp2_dp_msg_data 
> *hdcp2_msg_data;  ktime_t msg_end = ktime_set(0, 0);  bool 
> msg_expired;
> +u32 dev_cnt;
> 
>  hdcp2_msg_data = get_hdcp2_dp_msg_data(msg_id);  if (!hdcp2_msg_data) 
> @@ -546,17 +542,22 @@ int intel_dp_hdcp2_read_msg(struct 
> intel_digital_port *dig_port,
> 
>  hdcp->cp_irq_count_cached = atomic_read(&hdcp->cp_irq_count);
> 
> +/* DP adaptation msgs has no msg_id */
> +byte++;
> +
>  if (msg_id == HDCP_2_2_REP_SEND_RECVID_LIST) { -ret = 
> get_receiver_id_list_size(dig_port);
> +ret = get_receiver_id_list_rx_info(dig_port, &dev_cnt, byte);
>  if (ret < 0)
>  return ret;
> 
> -size = ret;
> +byte += ret;
> +size = sizeof(struct hdcp2_rep_send_receiverid_list) - 
> +HDCP_2_2_RXINFO_LEN - HDCP_2_2_RECEIVER_IDS_MAX_LEN
> +
> +(dev_cnt * HDCP_2_2_RECEIVER_ID_LEN); offset += HDCP_2_2_RXINFO_LEN;
>  }
> -bytes_to_recv = size - 1;
> 
> -/* DP adaptation msgs has no msg_id */
> -byte++;
> +bytes_to_recv = size - 1;
> 
>  while (bytes_to_recv) {
>  len = bytes_to_recv > DP_AUX_MAX_PAYLOAD_BYTES ?
> --
> 2.31.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
index 1d0096654776..fbfb3c4d16bb 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
@@ -496,11 +496,10 @@  get_rxinfo_hdcp_1_dev_downstream(struct intel_digital_port *dig_port, bool *hdcp
 }
 
 static
-ssize_t get_receiver_id_list_size(struct intel_digital_port *dig_port)
+ssize_t get_receiver_id_list_rx_info(struct intel_digital_port *dig_port, u32 *dev_cnt, u8 *byte)
 {
-	u8 rx_info[HDCP_2_2_RXINFO_LEN];
-	u32 dev_cnt;
 	ssize_t ret;
+	u8 *rx_info = byte;
 
 	ret = drm_dp_dpcd_read(&dig_port->dp.aux,
 			       DP_HDCP_2_2_REG_RXINFO_OFFSET,
@@ -508,15 +507,11 @@  ssize_t get_receiver_id_list_size(struct intel_digital_port *dig_port)
 	if (ret != HDCP_2_2_RXINFO_LEN)
 		return ret >= 0 ? -EIO : ret;
 
-	dev_cnt = (HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 |
+	*dev_cnt = (HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 |
 		   HDCP_2_2_DEV_COUNT_LO(rx_info[1]));
 
-	if (dev_cnt > HDCP_2_2_MAX_DEVICE_COUNT)
-		dev_cnt = HDCP_2_2_MAX_DEVICE_COUNT;
-
-	ret = sizeof(struct hdcp2_rep_send_receiverid_list) -
-		HDCP_2_2_RECEIVER_IDS_MAX_LEN +
-		(dev_cnt * HDCP_2_2_RECEIVER_ID_LEN);
+	if (*dev_cnt > HDCP_2_2_MAX_DEVICE_COUNT)
+		*dev_cnt = HDCP_2_2_MAX_DEVICE_COUNT;
 
 	return ret;
 }
@@ -534,6 +529,7 @@  int intel_dp_hdcp2_read_msg(struct intel_digital_port *dig_port,
 	const struct hdcp2_dp_msg_data *hdcp2_msg_data;
 	ktime_t msg_end = ktime_set(0, 0);
 	bool msg_expired;
+	u32 dev_cnt;
 
 	hdcp2_msg_data = get_hdcp2_dp_msg_data(msg_id);
 	if (!hdcp2_msg_data)
@@ -546,17 +542,22 @@  int intel_dp_hdcp2_read_msg(struct intel_digital_port *dig_port,
 
 	hdcp->cp_irq_count_cached = atomic_read(&hdcp->cp_irq_count);
 
+	/* DP adaptation msgs has no msg_id */
+	byte++;
+
 	if (msg_id == HDCP_2_2_REP_SEND_RECVID_LIST) {
-		ret = get_receiver_id_list_size(dig_port);
+		ret = get_receiver_id_list_rx_info(dig_port, &dev_cnt, byte);
 		if (ret < 0)
 			return ret;
 
-		size = ret;
+		byte += ret;
+		size = sizeof(struct hdcp2_rep_send_receiverid_list) -
+		HDCP_2_2_RXINFO_LEN - HDCP_2_2_RECEIVER_IDS_MAX_LEN +
+		(dev_cnt * HDCP_2_2_RECEIVER_ID_LEN);
+		offset += HDCP_2_2_RXINFO_LEN;
 	}
-	bytes_to_recv = size - 1;
 
-	/* DP adaptation msgs has no msg_id */
-	byte++;
+	bytes_to_recv = size - 1;
 
 	while (bytes_to_recv) {
 		len = bytes_to_recv > DP_AUX_MAX_PAYLOAD_BYTES ?