<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="#0563C1" vlink="#954F72" style="word-wrap:break-word"> <div class="WordSection1"> Hi ,<o:p></o:p> 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:<o:p></o:p> - HCDP2.2 Monitor connected to HDCP 2.2 Monitor<o:p></o:p> - HCDP2.2 Monitor connected to HDCP 1.4 Monitor<o:p></o:p> - HCDP1.4 Monitor connected to HDCP 1.4 Monitor<o:p></o:p> - HCDP2.2 Monitor standalone<o:p></o:p> - HDCP1.4 Monitor standalone <o:p></o:p> 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.<o:p></o:p> <o:p> </o:p> Tested-by: Suraj K <suraj.kandpal@intel.com><o:p></o:p> <o:p> </o:p> <o:p> </o:p> -----Original Message----- From: Gupta, Anshuman <anshuman.gupta@intel.com> Sent: Thursday, September 2, 2021 6:40 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 3/3] drm/i915/hdcp: reuse rx_info for mst stream type1 capability check <o:p> </o:p> <o:p> </o:p> <o:p> </o:p> > -----Original Message-----<o:p></o:p> > From: Li, Juston <<a href="mailto:juston.li@intel.com">juston.li@intel.com</a>><o:p></o:p> > Sent: Friday, August 20, 2021 12:19 AM<o:p></o:p> > To: <a href="mailto:intel-gfx@lists.freedesktop.org">intel-gfx@lists.freedesktop.org</a><o:p></o:p> > Cc: <a href="mailto:seanpaul@chromium.org">seanpaul@chromium.org</a>; Gupta, Anshuman <<a href="mailto:anshuman.gupta@intel.com">anshuman.gupta@intel.com</a>>; <o:p></o:p> > C, Ramalingam <<a href="mailto:ramalingam.c@intel.com">ramalingam.c@intel.com</a>>; Vivi, Rodrigo <o:p></o:p> > <<a href="mailto:rodrigo.vivi@intel.com">rodrigo.vivi@intel.com</a>>; Li, Juston <<a href="mailto:juston.li@intel.com">juston.li@intel.com</a>><o:p></o:p> > Subject: [Intel-gfx] [PATCH v6 3/3] drm/i915/hdcp: reuse rx_info for <o:p></o:p> > mst stream<o:p></o:p> > type1 capability check<o:p></o:p> > <o:p></o:p> > On some MST docking stations, rx_info can only be read after <o:p></o:p> > RepeaterAuth_Send_ReceiverID_List and the RxStatus READY bit is set <o:p></o:p> > otherwise the read will return -EIO.<o:p></o:p> > <o:p></o:p> > This behavior causes the mst stream type1 capability test to fail to <o:p></o:p> > read rx_info and determine if the topology supports type1 and fallback to type0.<o:p></o:p> > <o:p></o:p> > To fix this, check for type1 capability when we receive rx_info within <o:p></o:p> > the AKE flow when we read RepeaterAuth_Send_ReceiverID_List instead of <o:p></o:p> > an explicit read just for type1 capability checking.<o:p></o:p> > <o:p></o:p> > This does require moving where we set stream_types to after<o:p></o:p> > hdcp2_authenticate_sink() when we get rx_info but this occurs before <o:p></o:p> > we do hdcp2_propagate_stream_management_info.<o:p></o:p> > <o:p></o:p> > Also, legacy HDCP 2.0/2.1 are not type 1 capable either so check for <o:p></o:p> > that as well.<o:p></o:p> > <o:p></o:p> > Changes since v5:<o:p></o:p> > - rename intel_set_stream_types() to intel_hdcp_prepare_streams()<o:p></o:p> > (Anshuman)<o:p></o:p> > <o:p></o:p> > Changes since v4:<o:p></o:p> > - move topology_type1_capable to intel_digital_port and rename it as<o:p></o:p> > hdcp_mst_type1_capable (Anshuman)<o:p></o:p> > - make a helper function intel_set_stream_types() to set stream types<o:p></o:p> > in hdcp2_authenticate_and_encrypt() (Anshuman)<o:p></o:p> > - break on failure to set stream types and retry instead of returning<o:p></o:p> > - remove no longer used declaration for streams_type1_capable()<o:p></o:p> > <o:p></o:p> > Changes since v2:<o:p></o:p> > - Remove no longer used variables in _intel_hdcp2_enable()<o:p></o:p> > <o:p></o:p> > Signed-off-by: Juston Li <<a href="mailto:juston.li@intel.com">juston.li@intel.com</a>><o:p></o:p> > Reviewed-by: Ramalingam C <<a href="mailto:ramalingam.c@intel.com">ramalingam.c@intel.com</a>><o:p></o:p> > Reviewed-by: Anshuman Gupta <<a href="mailto:anshuman.gupta@intel.com">anshuman.gupta@intel.com</a>><o:p></o:p> Adding Suraj to this thread.<o:p></o:p> > ---<o:p></o:p> > .../drm/i915/display/intel_display_types.h | 6 +-<o:p></o:p> > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 39 -----------<o:p></o:p> > drivers/gpu/drm/i915/display/intel_hdcp.c | 64 +++++++++++--------<o:p></o:p> > 3 files changed, 38 insertions(+), 71 deletions(-)<o:p></o:p> > <o:p></o:p> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h<o:p></o:p> > b/drivers/gpu/drm/i915/display/intel_display_types.h<o:p></o:p> > index 6beeeeba1bed..34e90a841280 100644<o:p></o:p> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h<o:p></o:p> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h<o:p></o:p> > @@ -428,10 +428,6 @@ struct intel_hdcp_shim { int <o:p> </o:p> > (*hdcp_2_2_capable)(struct intel_digital_port *dig_port, bool <o:p></o:p> > *capable);<o:p></o:p> > <o:p></o:p> > -/* Detects whether a HDCP 1.4 sink connected in MST topology */ -int <o:p></o:p> > (*streams_type1_capable)(struct intel_connector *connector,<o:p></o:p> > - bool *capable);<o:p></o:p> > -<o:p></o:p> > /* Write HDCP2.2 messages */<o:p></o:p> > int (*write_2_2_msg)(struct intel_digital_port *dig_port,<o:p></o:p> > void *buf, size_t size);<o:p></o:p> > @@ -1684,6 +1680,8 @@ struct intel_digital_port { bool <o:p></o:p> > hdcp_auth_status;<o:p></o:p> > /* HDCP port data need to pass to security f/w */ struct <o:p></o:p> > hdcp_port_data hdcp_port_data;<o:p></o:p> > +/* Whether the MST topology supports HDCP Type 1 Content */ bool <o:p></o:p> > +hdcp_mst_type1_capable;<o:p></o:p> > <o:p></o:p> > void (*write_infoframe)(struct intel_encoder *encoder, const struct <o:p></o:p> > intel_crtc_state *crtc_state, diff -- git <o:p></o:p> > a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c<o:p></o:p> > b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c<o:p></o:p> > index fbfb3c4d16bb..540a669e01dd 100644<o:p></o:p> > --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c<o:p></o:p> > +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c<o:p></o:p> > @@ -478,23 +478,6 @@ int intel_dp_hdcp2_write_msg(struct <o:p></o:p> > intel_digital_port *dig_port, return size; }<o:p></o:p> > <o:p></o:p> > -static int<o:p></o:p> > -get_rxinfo_hdcp_1_dev_downstream(struct intel_digital_port *dig_port, <o:p></o:p> > bool<o:p></o:p> > *hdcp_1_x) -{<o:p></o:p> > -u8 rx_info[HDCP_2_2_RXINFO_LEN];<o:p></o:p> > -int ret;<o:p></o:p> > -<o:p></o:p> > -ret = drm_dp_dpcd_read(&dig_port->dp.aux,<o:p></o:p> > - DP_HDCP_2_2_REG_RXINFO_OFFSET,<o:p></o:p> > - (void *)rx_info, HDCP_2_2_RXINFO_LEN);<o:p></o:p> > -<o:p></o:p> > -if (ret != HDCP_2_2_RXINFO_LEN)<o:p></o:p> > -return ret >= 0 ? -EIO : ret;<o:p></o:p> > -<o:p></o:p> > -*hdcp_1_x = HDCP_2_2_HDCP1_DEVICE_CONNECTED(rx_info[1]) ? true<o:p></o:p> > : false;<o:p></o:p> > -return 0;<o:p></o:p> > -}<o:p></o:p> > -<o:p></o:p> > static<o:p></o:p> > ssize_t get_receiver_id_list_rx_info(struct intel_digital_port <o:p></o:p> > *dig_port, u32 *dev_cnt, u8 *byte) { @@ -665,27 +648,6 @@ int <o:p></o:p> > intel_dp_hdcp2_capable(struct intel_digital_port *dig_port, return 0; <o:p></o:p> > }<o:p></o:p> > <o:p></o:p> > -static<o:p></o:p> > -int intel_dp_mst_streams_type1_capable(struct intel_connector *connector,<o:p></o:p> > - bool *capable)<o:p></o:p> > -{<o:p></o:p> > -struct intel_digital_port *dig_port = <o:p></o:p> > intel_attached_dig_port(connector);<o:p></o:p> > -struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); <o:p></o:p> > -int ret; -bool hdcp_1_x;<o:p></o:p> > -<o:p></o:p> > -ret = get_rxinfo_hdcp_1_dev_downstream(dig_port, &hdcp_1_x); -if <o:p></o:p> > (ret) { -drm_dbg_kms(&i915->drm,<o:p></o:p> > - "[%s:%d] failed to read RxInfo ret=%d\n",<o:p></o:p> > - connector->base.name, connector->base.base.id,<o:p></o:p> > ret);<o:p></o:p> > -return ret;<o:p></o:p> > -}<o:p></o:p> > -<o:p></o:p> > -*capable = !hdcp_1_x;<o:p></o:p> > -return 0;<o:p></o:p> > -}<o:p></o:p> > -<o:p></o:p> > static const struct intel_hdcp_shim intel_dp_hdcp_shim = { <o:p></o:p> > .write_an_aksv = intel_dp_hdcp_write_an_aksv, .read_bksv = <o:p></o:p> > intel_dp_hdcp_read_bksv, @@ -834,7 +796,6 @@ static const struct <o:p></o:p> > intel_hdcp_shim intel_dp_mst_hdcp_shim = { .stream_2_2_encryption = <o:p></o:p> > intel_dp_mst_hdcp2_stream_encryption,<o:p></o:p> > .check_2_2_link = intel_dp_mst_hdcp2_check_link, .hdcp_2_2_capable = <o:p></o:p> > intel_dp_hdcp2_capable, -.streams_type1_capable = <o:p> </o:p> > intel_dp_mst_streams_type1_capable,<o:p></o:p> > .protocol = HDCP_PROTOCOL_DP,<o:p></o:p> > };<o:p></o:p> > <o:p></o:p> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c<o:p></o:p> > b/drivers/gpu/drm/i915/display/intel_hdcp.c<o:p></o:p> > index ebc2e32aec0b..9b9fd9d13043 100644<o:p></o:p> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c<o:p></o:p> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c<o:p></o:p> > @@ -33,21 +33,6 @@ static int intel_conn_to_vcpi(struct <o:p></o:p> > intel_connector<o:p></o:p> > *connector)<o:p></o:p> > return connector->port? connector->port->vcpi.vcpi : 0; }<o:p></o:p> > <o:p></o:p> > -static bool<o:p></o:p> > -intel_streams_type1_capable(struct intel_connector *connector) -{ <o:p></o:p> > -const struct intel_hdcp_shim *shim = connector->hdcp.shim; -bool <o:p></o:p> > capable = false;<o:p></o:p> > -<o:p></o:p> > -if (!shim)<o:p></o:p> > -return capable;<o:p></o:p> > -<o:p></o:p> > -if (shim->streams_type1_capable)<o:p></o:p> > -shim->streams_type1_capable(connector, &capable);<o:p></o:p> > -<o:p></o:p> > -return capable;<o:p></o:p> > -}<o:p></o:p> > -<o:p></o:p> > /*<o:p></o:p> > * intel_hdcp_required_content_stream selects the most highest common <o:p></o:p> > possible HDCP<o:p></o:p> > * content_type for all streams in DP MST topology because security <o:p></o:p> > f/w doesn't @@ -86,7 +71,7 @@ <o:p></o:p> > intel_hdcp_required_content_stream(struct<o:p></o:p> > intel_digital_port *dig_port)<o:p></o:p> > if (conn_dig_port != dig_port)<o:p></o:p> > continue;<o:p></o:p> > <o:p></o:p> > -if (!enforce_type0 &&<o:p></o:p> > !intel_streams_type1_capable(connector))<o:p></o:p> > +if (!enforce_type0 && !dig_port->hdcp_mst_type1_capable)<o:p></o:p> > enforce_type0 = true;<o:p></o:p> > <o:p></o:p> > data->streams[data->k].stream_id =<o:p></o:p> > intel_conn_to_vcpi(connector); @@ -112,6 +97,25 @@ <o:p> </o:p> > intel_hdcp_required_content_stream(struct intel_digital_port <o:p></o:p> > *dig_port) return 0; }<o:p></o:p> > <o:p></o:p> > +static int intel_hdcp_prepare_streams(struct intel_connector<o:p></o:p> > +*connector) {<o:p></o:p> > +struct intel_digital_port *dig_port =<o:p></o:p> > intel_attached_dig_port(connector);<o:p></o:p> > +struct hdcp_port_data *data = &dig_port->hdcp_port_data; struct <o:p></o:p> > +intel_hdcp *hdcp = &connector->hdcp; int ret;<o:p></o:p> > +<o:p></o:p> > +if (!intel_encoder_is_mst(intel_attached_encoder(connector))) {<o:p></o:p> > +data->k = 1;<o:p></o:p> > +data->streams[0].stream_type = hdcp->content_type;<o:p></o:p> > +} else {<o:p></o:p> > +ret = intel_hdcp_required_content_stream(dig_port);<o:p></o:p> > +if (ret)<o:p></o:p> > +return ret;<o:p></o:p> > +}<o:p></o:p> > +<o:p></o:p> > +return 0;<o:p></o:p> > +}<o:p></o:p> > +<o:p></o:p> > static<o:p></o:p> > bool intel_hdcp_is_ksv_valid(u8 *ksv) { @@ -1632,6 +1636,14 @@ int <o:p></o:p> > hdcp2_authenticate_repeater_topology(struct<o:p></o:p> > intel_connector *connector)<o:p></o:p> > return -EINVAL;<o:p></o:p> > }<o:p></o:p> > <o:p></o:p> > +/*<o:p></o:p> > + * MST topology is not Type 1 capable if it contains a downstream<o:p></o:p> > + * device that is only HDCP 1.x or Legacy HDCP 2.0/2.1 compliant.<o:p></o:p> > + */<o:p></o:p> > +dig_port->hdcp_mst_type1_capable =<o:p></o:p> > +!HDCP_2_2_HDCP1_DEVICE_CONNECTED(rx_info[1]) && <o:p></o:p> > +!HDCP_2_2_HDCP_2_0_REP_CONNECTED(rx_info[1]);<o:p></o:p> > +<o:p></o:p> > /* Converting and Storing the seq_num_v to local variable as DWORD */ <o:p></o:p> > seq_num_v = drm_hdcp_be24_to_cpu((const u8 <o:p></o:p> > *)msgs.recvid_list.seq_num_v); @@ -1876,6 +1888,14 @@ static int <o:p></o:p> > hdcp2_authenticate_and_encrypt(struct intel_connector *connector) for <o:p></o:p> > (i = 0; i < tries && !dig_port->hdcp_auth_status; i++) { ret = <o:p></o:p> > hdcp2_authenticate_sink(connector);<o:p></o:p> > if (!ret) {<o:p></o:p> > +ret = intel_hdcp_prepare_streams(connector);<o:p></o:p> > +if (ret) {<o:p></o:p> > +drm_dbg_kms(&i915->drm,<o:p></o:p> > + "Prepare streams failed.(%d)\n",<o:p></o:p> > + ret);<o:p></o:p> > +break;<o:p></o:p> > +}<o:p></o:p> > +<o:p></o:p> > ret =<o:p></o:p> > hdcp2_propagate_stream_management_info(connector);<o:p></o:p> > if (ret) {<o:p></o:p> > drm_dbg_kms(&i915->drm,<o:p></o:p> > @@ -1921,9 +1941,7 @@ static int hdcp2_authenticate_and_encrypt(struct<o:p></o:p> > intel_connector *connector)<o:p></o:p> > <o:p></o:p> > static int _intel_hdcp2_enable(struct intel_connector *connector) { <o:p></o:p> > -struct intel_digital_port *dig_port = <o:p></o:p> > intel_attached_dig_port(connector);<o:p></o:p> > struct drm_i915_private *i915 = to_i915(connector->base.dev); -struct <o:p></o:p> > hdcp_port_data *data = &dig_port->hdcp_port_data; struct intel_hdcp <o:p></o:p> > *hdcp = &connector->hdcp; int ret;<o:p></o:p> > <o:p></o:p> > @@ -1931,16 +1949,6 @@ static int _intel_hdcp2_enable(struct <o:p></o:p> > intel_connector *connector)<o:p></o:p> > connector->base.name, connector->base.base.id,<o:p></o:p> > hdcp->content_type);<o:p></o:p> > <o:p></o:p> > -/* Stream which requires encryption */ -if <o:p></o:p> > (!intel_encoder_is_mst(intel_attached_encoder(connector))) {<o:p></o:p> > -data->k = 1;<o:p></o:p> > -data->streams[0].stream_type = hdcp->content_type;<o:p></o:p> > -} else {<o:p></o:p> > -ret = intel_hdcp_required_content_stream(dig_port);<o:p></o:p> > -if (ret)<o:p></o:p> > -return ret;<o:p></o:p> > -}<o:p></o:p> > -<o:p></o:p> > ret = hdcp2_authenticate_and_encrypt(connector);<o:p></o:p> > if (ret) {<o:p></o:p> > drm_dbg_kms(&i915->drm, "HDCP2 Type%d Enabling Failed.<o:p></o:p> > (%d)\n",<o:p></o:p> > --<o:p></o:p> > 2.31.1<o:p></o:p> <o:p> </o:p> <o:p> </o:p> </div> </body> </html>