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

[Nautiyal, Ankit K]

On 1/27/2024 12:14 PM, Kandpal, Suraj wrote:
=
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

I see for HDCP1.4 also we need to have a separate functions if we want 
to get correct HDCP capability for remote sinks.

I think lets not change existing hdcp_2_capable function. IMHO it would 
be better to have another shim function for hdcp capabilities of remote 
sink.


Regards,

Ankit

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.kand...@intel.com>
---
   .../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;