RE: [PATCH 2/7] drm/i915/hdcp: HDCP Capability for the downstream device

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

 



=
> 
> On 1/24/2024 6:50 PM, Nautiyal, Ankit K wrote:
> >
> > On 1/12/2024 1:11 PM, Suraj Kandpal wrote:
> >> Currently we are only checking capability of remote device and not
> >> immediate downstream device but during capability check we need are
> >> concerned with only the HDCP capability of downstream device.
> >> During i915_display_info reporting we need HDCP Capability for both
> >> the monitors and downstream branch device if any this patch adds that.
> >
> >
> > I agree cases where MST hub/docker and sink are of different
> > capabilities, this creates a confusion.
> >
> > with this change, perhaps kms_content_protection IGT can also be
> > changed to check for MST hub's capability.
> >
> > Only thing is that for hdmi the 'remote_req' doesnt make sense.
> >
> Instead of changing the hdcp_2_2_capable can we just have a separate
> function for intel_dp_remote_hdcp2_capable(), which uses aux =
> &connector->port->aux.

Yes I agree about the hdmi has a argument it wont use but I went with the lesser
Of the two evils . If I went with the approach suggested the problem would be that
debug fs calls intel_hdcp2_capable which has other checks for mei, gsc and If HW supports
hdcp2 or not so all this would also have to be put in this function intel_dp_remote_hdcp2_capable()
which I feel would create much more of a mess

Regards,
Suraj Kandpal 
> 
> The common code for reading HDCP2_2 Rx caps can be pulled out in a
> separate function, which we can call only in case of MST when we read
> remote.
> 
> Also we might need to have similar thing for HDCP1.4.
> 
> 
> Regards,
> 
> Ankit
> 
> 
> >>
> >> Signed-off-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
> >> ---
> >>   .../drm/i915/display/intel_display_debugfs.c  | 19
> >> +++++++++++++++----
> >>   .../drm/i915/display/intel_display_types.h    |  2 +-
> >>   drivers/gpu/drm/i915/display/intel_dp_hdcp.c  |  4 ++--
> >>   drivers/gpu/drm/i915/display/intel_hdcp.c     |  6 +++---
> >>   drivers/gpu/drm/i915/display/intel_hdcp.h     |  2 +-
> >>   drivers/gpu/drm/i915/display/intel_hdmi.c     |  2 +-
> >>   6 files changed, 23 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> >> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> >> index d951edb36687..457f13357fad 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> >> @@ -210,7 +210,8 @@ static void intel_panel_info(struct seq_file *m,
> >>   }
> >>     static void intel_hdcp_info(struct seq_file *m,
> >> -                struct intel_connector *intel_connector)
> >> +                struct intel_connector *intel_connector,
> >> +                bool remote_req)
> >>   {
> >>       bool hdcp_cap, hdcp2_cap;
> >>   @@ -220,7 +221,7 @@ static void intel_hdcp_info(struct seq_file *m,
> >>       }
> >>         hdcp_cap = intel_hdcp_capable(intel_connector);
> >> -    hdcp2_cap = intel_hdcp2_capable(intel_connector);
> >> +    hdcp2_cap = intel_hdcp2_capable(intel_connector, remote_req);
> >>         if (hdcp_cap)
> >>           seq_puts(m, "HDCP1.4 ");
> >> @@ -307,7 +308,12 @@ static void intel_connector_info(struct seq_file
> >> *m,
> >>       }
> >>         seq_puts(m, "\tHDCP version: ");
> >> -    intel_hdcp_info(m, intel_connector);
> >> +    intel_hdcp_info(m, intel_connector, true);
> >> +
> >> +    if (intel_encoder_is_mst(encoder)) {
> >> +        seq_puts(m, "\tHDCP Branch Device version: ");
> >> +        intel_hdcp_info(m, intel_connector, false);
> >> +    }
> >>         seq_printf(m, "\tmax bpc: %u\n",
> >> connector->display_info.bpc);
> >>   @@ -1153,7 +1159,12 @@ static int
> >> i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
> >>         seq_printf(m, "%s:%d HDCP version: ", connector->base.name,
> >>              connector->base.base.id);
> >> -    intel_hdcp_info(m, connector);
> >> +    intel_hdcp_info(m, connector, true);
> >> +
> >> +    if (intel_encoder_is_mst(connector->encoder)) {
> >> +        seq_puts(m, "\tHDCP Branch Device version: ");
> >
> >
> > Perhaps MST HUB HDCP version?
> >
> >
> >> +        intel_hdcp_info(m, connector, false);
> >> +    }
> >>     out:
> >> drm_modeset_unlock(&i915->drm.mode_config.connection_mutex);
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> >> b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> index ae2e8cff9d69..aa559598f049 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> @@ -507,7 +507,7 @@ struct intel_hdcp_shim {
> >>         /* Detects whether sink is HDCP2.2 capable */
> >>       int (*hdcp_2_2_capable)(struct intel_connector *connector,
> >> -                bool *capable);
> >> +                bool *capable, bool remote_req);
> >>         /* Write HDCP2.2 messages */
> >>       int (*write_2_2_msg)(struct intel_connector *connector, diff
> >> --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> >> b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> >> index bec49061b2e1..90b027ba3302 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
> >> @@ -649,13 +649,13 @@ int intel_dp_hdcp2_check_link(struct
> >> intel_digital_port *dig_port,
> >>     static
> >>   int intel_dp_hdcp2_capable(struct intel_connector *connector,
> >> -               bool *capable)
> >> +               bool *capable, bool remote_req)
> >>   {
> >>       struct drm_dp_aux *aux;
> >>       u8 rx_caps[3];
> >>       int ret;
> >>   -    aux = intel_dp_hdcp_get_aux(connector, true);
> >> +    aux = intel_dp_hdcp_get_aux(connector, remote_req);
> >
> > Inline with the comments on the previous patch, this would also be
> > required to change.
> >
> > Otherwise the patch looks good to me.
> >
> >
> > Regards,
> >
> > Ankit
> >
> >>         *capable = false;
> >>       ret = drm_dp_dpcd_read(aux,
> >> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> b/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> index c3e692e7f790..b88a4713e6a8 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> @@ -161,7 +161,7 @@ bool intel_hdcp_capable(struct intel_connector
> >> *connector)
> >>   }
> >>     /* Is HDCP2.2 capable on Platform and Sink */ -bool
> >> intel_hdcp2_capable(struct intel_connector *connector)
> >> +bool intel_hdcp2_capable(struct intel_connector *connector, bool
> >> remote_req)
> >>   {
> >>       struct drm_i915_private *i915 = to_i915(connector->base.dev);
> >>       struct intel_hdcp *hdcp = &connector->hdcp; @@ -186,7 +186,7 @@
> >> bool intel_hdcp2_capable(struct intel_connector
> >> *connector)
> >>       mutex_unlock(&i915->display.hdcp.hdcp_mutex);
> >>         /* Sink's capability for HDCP2.2 */
> >> -    hdcp->shim->hdcp_2_2_capable(connector, &capable);
> >> +    hdcp->shim->hdcp_2_2_capable(connector, &capable, remote_req);
> >>         return capable;
> >>   }
> >> @@ -2374,7 +2374,7 @@ static int _intel_hdcp_enable(struct
> >> intel_atomic_state *state,
> >>        * Considering that HDCP2.2 is more secure than HDCP1.4, If the
> >> setup
> >>        * is capable of HDCP2.2, it is preferred to use HDCP2.2.
> >>        */
> >> -    if (intel_hdcp2_capable(connector)) {
> >> +    if (intel_hdcp2_capable(connector, false)) {
> >>           ret = intel_hdcp_set_streams(dig_port, state);
> >>           if (!ret) {
> >>               ret = _intel_hdcp2_enable(connector); diff --git
> >> a/drivers/gpu/drm/i915/display/intel_hdcp.h
> >> b/drivers/gpu/drm/i915/display/intel_hdcp.h
> >> index a9c784fd9ba5..72268e593cec 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_hdcp.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
> >> @@ -39,7 +39,7 @@ void intel_hdcp_update_pipe(struct
> >> intel_atomic_state *state,
> >>                   const struct drm_connector_state *conn_state);
> >>   bool is_hdcp_supported(struct drm_i915_private *i915, enum port
> >> port);
> >>   bool intel_hdcp_capable(struct intel_connector *connector); -bool
> >> intel_hdcp2_capable(struct intel_connector *connector);
> >> +bool intel_hdcp2_capable(struct intel_connector *connector, bool
> >> remote_req);
> >>   void intel_hdcp_component_init(struct drm_i915_private *i915);
> >>   void intel_hdcp_component_fini(struct drm_i915_private *i915);
> >>   void intel_hdcp_cleanup(struct intel_connector *connector); diff
> >> --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> >> b/drivers/gpu/drm/i915/display/intel_hdmi.c
> >> index 7020e5806109..d7feef05bc47 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> >> @@ -1733,7 +1733,7 @@ int intel_hdmi_hdcp2_check_link(struct
> >> intel_digital_port *dig_port,
> >>     static
> >>   int intel_hdmi_hdcp2_capable(struct intel_connector *connector,
> >> -                 bool *capable)
> >> +                 bool *capable, bool remote_req)
> >
> >
> >>   {
> >>       struct intel_digital_port *dig_port =
> >> intel_attached_dig_port(connector);
> >>       u8 hdcp2_version;




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

  Powered by Linux