On Wed, 2021-08-11 at 15:34 -0400, Rodrigo Vivi wrote: > On Tue, Aug 10, 2021 at 04:52:11PM -0700, Juston Li wrote: > > 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 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@xxxxxxxxx> > > Acked-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 30 ++++++++++------ > > ---- > > include/drm/drm_dp_helper.h | 2 +- > > 2 files changed, 16 insertions(+), 16 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..526fd58b9b51 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,21 @@ 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); > > } > > - 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 ? > > diff --git a/include/drm/drm_dp_helper.h > > b/include/drm/drm_dp_helper.h > > I was about to merge to drm-intel-next but then I noticed this.. > > So we need to cc Dave and Daniel here to get an ack from them. > > > index 3f2715eb965f..7476e7c6d0be 100644 > > --- a/include/drm/drm_dp_helper.h > > +++ b/include/drm/drm_dp_helper.h > > @@ -1413,7 +1413,7 @@ enum drm_dp_phy { > > #define > > DP_HDCP_2_2_LC_INIT_OFFSET DP_HDCP_2_2_REG_RN_OFFSET > > #define > > DP_HDCP_2_2_LC_SEND_LPRIME_OFFSET DP_HDCP_2_2_REG_LPRIME_OFFSE > > T > > #define > > DP_HDCP_2_2_SKE_SEND_EKS_OFFSET DP_HDCP_2_2_REG_EDKE > > Y_KS_OFFSET > > -#define > > DP_HDCP_2_2_REP_SEND_RECVID_LIST_OFFSET DP_HDCP_2_2_REG_RXIN > > FO_OFFSET > > +#define > > DP_HDCP_2_2_REP_SEND_RECVID_LIST_OFFSET DP_HDCP_2_2_REG_SEQ_ > > NUM_V_OFFSET > > I'm not familiar with this so I'm not sure if this should be a > separated patch > or better explained in the commit message. > > Dave/Daniel how do you prefer this? Actually this is just incorrect, I should not be changing this define. Instead, I'll change intel_dp_hdcp2_read_msg() to update the offset and add HDCP_2_2_RXINFO_LEN to it after reading RxInfo. eg. DP_HDCP_2_2_REP_SEND_RECVID_LIST_OFFSET + HDCP_2_2_RXINFO_LEN = DP_HDCP_2_2_REG_SEQ_NUM_V_OFFSET which is the offset we need to continue reading from. Will send out new revision Thanks for pointing this out Rodrigo! > > > #define > > DP_HDCP_2_2_REP_SEND_ACK_OFFSET DP_HDCP_2_2_REG_V_OF > > FSET > > #define > > DP_HDCP_2_2_REP_STREAM_MANAGE_OFFSET DP_HDCP_2_2_REG_SEQ_NUM_M_OF > > FSET > > #define > > DP_HDCP_2_2_REP_STREAM_READY_OFFSET DP_HDCP_2_2_REG_MPRIME_OFFSE > > T > > -- > > 2.31.1 > >