Re: [Intel-gfx] [PATCH v6 2/3] drm/i915/hdcp: read RxInfo once when reading RepeaterAuth_Send_ReceiverID_List

["Kandpal, Suraj" <suraj.kandpal@intel.com>]

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