Re: [PATCH v4 3/3] drm/i915/hdcp: reuse rx_info for mst stream type1 capability check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2021-08-12 at 12:40 +0530, Anshuman Gupta wrote:
> On 2021-08-11 at 14:23:14 -0700, Juston Li wrote:
> > On some MST docking stations, rx_info can only be read after
> > RepeaterAuth_Send_ReceiverID_List and the RxStatus READY bit is set
> > otherwise the read will return -EIO.
> > 
> > This behavior causes the mst stream type1 capability test to fail
> > to
> > read rx_info and determine if the topology supports type1 and
> > fallback
> > to type0.
> > 
> > To fix this, check for type1 capability when we receive rx_info
> > within
> > the AKE flow when we read RepeaterAuth_Send_ReceiverID_List instead
> > of an explicit read just for type1 capability checking.
> > 
> > This does require moving where we set stream_types to after
> > hdcp2_authenticate_sink() when we get rx_info but this occurs
> > before we
> > do hdcp2_propagate_stream_management_info.
> > 
> > Also, legacy HDCP 2.0/2.1 are not type 1 capable either so check
> > for
> > that as well.
> > 
> > Changes since v2:
> >  - Remove no longer used variables in _intel_hdcp2_enable()
> > 
> > Signed-off-by: Juston Li <juston.li@xxxxxxxxx>
> > Reviewed-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
> > ---
> >  .../drm/i915/display/intel_display_types.h    |  2 +
> >  drivers/gpu/drm/i915/display/intel_dp_hdcp.c  | 39 ---------------
> >  drivers/gpu/drm/i915/display/intel_hdcp.c     | 49 ++++++++-------
> > ----
> >  3 files changed, 23 insertions(+), 67 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index dbdfe54d0340..c8b687ff0374 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -516,6 +516,8 @@ struct intel_hdcp {
> >         enum transcoder cpu_transcoder;
> >         /* Only used for DP MST stream encryption */
> >         enum transcoder stream_transcoder;
> > +
> > +       bool topology_type1_capable;
> Topology is not going to be change for each connector in mst
> topology.
> IMHO dig_port should contain this, prefix by mst as this is specific
> to mst usages.

Sure, will move to intel_digital_port and I settled on renaming it
hdcp_mst_type1_capable

> >  };
> >  
> >  struct intel_connector {
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> > b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> > index fbfb3c4d16bb..540a669e01dd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> > @@ -478,23 +478,6 @@ int intel_dp_hdcp2_write_msg(struct
> > intel_digital_port *dig_port,
> >         return size;
> >  }
> >  
> > -static int
> > -get_rxinfo_hdcp_1_dev_downstream(struct intel_digital_port
> > *dig_port, bool *hdcp_1_x)
> > -{
> > -       u8 rx_info[HDCP_2_2_RXINFO_LEN];
> > -       int ret;
> > -
> > -       ret = drm_dp_dpcd_read(&dig_port->dp.aux,
> > -                              DP_HDCP_2_2_REG_RXINFO_OFFSET,
> > -                              (void *)rx_info,
> > HDCP_2_2_RXINFO_LEN);
> > -
> > -       if (ret != HDCP_2_2_RXINFO_LEN)
> > -               return ret >= 0 ? -EIO : ret;
> > -
> > -       *hdcp_1_x = HDCP_2_2_HDCP1_DEVICE_CONNECTED(rx_info[1]) ?
> > true : false;
> > -       return 0;
> > -}
> > -
> >  static
> >  ssize_t get_receiver_id_list_rx_info(struct intel_digital_port
> > *dig_port, u32 *dev_cnt, u8 *byte)
> >  {
> > @@ -665,27 +648,6 @@ int intel_dp_hdcp2_capable(struct
> > intel_digital_port *dig_port,
> >         return 0;
> >  }
> >  
> > -static
> > -int intel_dp_mst_streams_type1_capable(struct intel_connector
> > *connector,
> > -                                      bool *capable)
> > -{
> > -       struct intel_digital_port *dig_port =
> > intel_attached_dig_port(connector);
> > -       struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> > -       int ret;
> > -       bool hdcp_1_x;
> > -
> > -       ret = get_rxinfo_hdcp_1_dev_downstream(dig_port,
> > &hdcp_1_x);
> > -       if (ret) {
> > -               drm_dbg_kms(&i915->drm,
> > -                           "[%s:%d] failed to read RxInfo
> > ret=%d\n",
> > -                           connector->base.name, connector-
> > >base.base.id, ret);
> > -               return ret;
> > -       }
> > -
> > -       *capable = !hdcp_1_x;
> > -       return 0;
> > -}
> > -
> >  static const struct intel_hdcp_shim intel_dp_hdcp_shim = {
> >         .write_an_aksv = intel_dp_hdcp_write_an_aksv,
> >         .read_bksv = intel_dp_hdcp_read_bksv,
> > @@ -834,7 +796,6 @@ static const struct intel_hdcp_shim
> > intel_dp_mst_hdcp_shim = {
> >         .stream_2_2_encryption =
> > intel_dp_mst_hdcp2_stream_encryption,
> >         .check_2_2_link = intel_dp_mst_hdcp2_check_link,
> >         .hdcp_2_2_capable = intel_dp_hdcp2_capable,
> > -       .streams_type1_capable =
> > intel_dp_mst_streams_type1_capable,
> >         .protocol = HDCP_PROTOCOL_DP,
> >  };
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index ebc2e32aec0b..1a2a98e2c6e3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > @@ -33,21 +33,6 @@ static int intel_conn_to_vcpi(struct
> > intel_connector *connector)
> >         return connector->port  ? connector->port->vcpi.vcpi : 0;
> >  }
> >  
> > -static bool
> > -intel_streams_type1_capable(struct intel_connector *connector)
> > -{
> > -       const struct intel_hdcp_shim *shim = connector->hdcp.shim;
> > -       bool capable = false;
> > -
> > -       if (!shim)
> > -               return capable;
> > -
> > -       if (shim->streams_type1_capable)
> > -               shim->streams_type1_capable(connector, &capable);
> > -
> > -       return capable;
> > -}
> > -
> >  /*
> >   * intel_hdcp_required_content_stream selects the most highest
> > common possible HDCP
> >   * content_type for all streams in DP MST topology because
> > security f/w doesn't
> > @@ -86,7 +71,7 @@ intel_hdcp_required_content_stream(struct
> > intel_digital_port *dig_port)
> >                 if (conn_dig_port != dig_port)
> >                         continue;
> >  
> > -               if (!enforce_type0 &&
> > !intel_streams_type1_capable(connector))
> > +               if (!enforce_type0 && !connector-
> > >hdcp.topology_type1_capable)
> >                         enforce_type0 = true;
> >  
> >                 data->streams[data->k].stream_id =
> > intel_conn_to_vcpi(connector);
> > @@ -1632,6 +1617,14 @@ int
> > hdcp2_authenticate_repeater_topology(struct intel_connector
> > *connector)
> >                 return -EINVAL;
> >         }
> >  
> > +       /*
> > +        * A topology is not Type 1 capable if it contains a
> > downstream device
> > +        * that is HDCP 1.x or Legacy HDCP 2.0/2.1 compliant.
> > +        */
> > +       hdcp->topology_type1_capable =
> > +               !HDCP_2_2_HDCP1_DEVICE_CONNECTED(rx_info[1]) &&
> > +               !HDCP_2_2_HDCP_2_0_REP_CONNECTED(rx_info[1]);
> > +
> >         /* Converting and Storing the seq_num_v to local variable
> > as DWORD */
> >         seq_num_v =
> >                 drm_hdcp_be24_to_cpu((const u8
> > *)msgs.recvid_list.seq_num_v);
> > @@ -1871,11 +1864,23 @@ static int
> > hdcp2_authenticate_and_encrypt(struct intel_connector *connector)
> >  {
> >         struct intel_digital_port *dig_port =
> > intel_attached_dig_port(connector);
> >         struct drm_i915_private *i915 = to_i915(connector-
> > >base.dev);
> > +       struct hdcp_port_data *data = &dig_port->hdcp_port_data;
> > +       struct intel_hdcp *hdcp = &connector->hdcp;
> >         int ret = 0, i, tries = 3;
> >  
> >         for (i = 0; i < tries && !dig_port->hdcp_auth_status; i++)
> > {
> >                 ret = hdcp2_authenticate_sink(connector);
> >                 if (!ret) {
> > +                       /* Stream which requires encryption */
> > +                       if
> > (!intel_encoder_is_mst(intel_attached_encoder(connector))) {
> > +                               data->k = 1;
> > +                               data->streams[0].stream_type =
> > hdcp->content_type;
> > +                       } else {
> > +                               ret =
> > intel_hdcp_required_content_stream(dig_port);
> > +                               if (ret)
> > +                                       return ret;
> > +                       }
> > +
> It will be good to use helper function to init streams here.

Sure, will move this to a helper function.
Also fix shouldn't return here, should break and let loop retry.

Thanks!

> Thanks,
> Anshuman Gupta.
> >                         ret =
> > hdcp2_propagate_stream_management_info(connector);
> >                         if (ret) {
> >                                 drm_dbg_kms(&i915->drm,
> > @@ -1921,9 +1926,7 @@ static int
> > hdcp2_authenticate_and_encrypt(struct intel_connector *connector)
> >  
> >  static int _intel_hdcp2_enable(struct intel_connector *connector)
> >  {
> > -       struct intel_digital_port *dig_port =
> > intel_attached_dig_port(connector);
> >         struct drm_i915_private *i915 = to_i915(connector-
> > >base.dev);
> > -       struct hdcp_port_data *data = &dig_port->hdcp_port_data;
> >         struct intel_hdcp *hdcp = &connector->hdcp;
> >         int ret;
> >  
> > @@ -1931,16 +1934,6 @@ static int _intel_hdcp2_enable(struct
> > intel_connector *connector)
> >                     connector->base.name, connector->base.base.id,
> >                     hdcp->content_type);
> >  
> > -       /* Stream which requires encryption */
> > -       if
> > (!intel_encoder_is_mst(intel_attached_encoder(connector))) {
> > -               data->k = 1;
> > -               data->streams[0].stream_type = hdcp->content_type;
> > -       } else {
> > -               ret = intel_hdcp_required_content_stream(dig_port);
> > -               if (ret)
> > -                       return ret;
> > -       }
> > -
> >         ret = hdcp2_authenticate_and_encrypt(connector);
> >         if (ret) {
> >                 drm_dbg_kms(&i915->drm, "HDCP2 Type%d  Enabling
> > Failed. (%d)\n",
> > -- 
> > 2.31.1
> > 





[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux