On 2/16/2021 4:47 PM, Nautiyal, Ankit K wrote:

Hi Karthik,

Over all I agree with the patch and the approach. I have a couple of 
suggestion/corrections inline:

Hi Ankit,

Thank you for the review.

On 2/4/2021 1:36 PM, Karthik B S wrote:

quoted

Add subtests to verify content protection simultaneously on
multiple outputs on the same MST topology.

v3: -Remove the logging which are no longer required. (Anshuman)
     -Add logic to verify that CP is still enabled on other connectors
      while disabling it on one of the connectors. (Anshuman)

v4: -Rename is_output_support_cp_capable(). (Anshuman, Ram)
     -Rephrase the comment in is_dp_mst_output. (Anshuman)
     -Remove the redundant HDCP check hunk before HDCP disable. 
(Anshuman)
     -Check the link on the HDCP enabled outputs when one of the outputs
      in the same DP MST topology is disabled. (Anshuman)
     -Revert the change in test_content_protection_cleanup(). (Anshuman)
     -Create fb's in a common function for both MST and SST. (Ram)
     -Rename is_dp_mst_output() to output_is_dp_mst(). (Ram)
     -Remove the redundant igt_debug() before HDCP enable. (Ram)
     -Rephrase the igt_assert statement during HDCP enable. (Ram)
     -Optimize the execution time by checking the link for all MST
      connectors together. (Ram)
     -No need of loop where we disable CP on one of the MST outputs. 
(Ram)
     -During verification by disabling one of the MST outputs,
      check for the negative case until the required timeout. (Ram)

v5: -Modify skip message when HDCP support is not present. (Anshuman)
     -Remove the definition of color_t structure as it is no longer 
used.

v6: -Optimize execution time in LIC test. (Anshuman)

v7: -Check HDCP capability after modeset on MST. (Anshuman)

Signed-off-by: Anshuman Gupta <redacted>
Signed-off-by: Karthik B S <redacted>
---
  tests/kms_content_protection.c | 277 ++++++++++++++++++++++++++++++---
  1 file changed, 255 insertions(+), 22 deletions(-)

diff --git a/tests/kms_content_protection.c 

b/tests/kms_content_protection.c
index 303ed418..4ea79748 100644

--- a/tests/kms_content_protection.c
+++ b/tests/kms_content_protection.c

@@ -179,16 +179,10 @@ static void modeset_with_fb(const enum pipe 

pipe, igt_output_t *output,
      igt_output_override_mode(output, &mode);
      igt_output_set_pipe(output, pipe);
  -    igt_create_color_fb(display->drm_fd, mode.hdisplay, 
mode.vdisplay,
-                DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
-                1.f, 0.f, 0.f, &data.red);
-    igt_create_color_fb(display->drm_fd, mode.hdisplay, mode.vdisplay,
-                DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
-                0.f, 1.f, 0.f, &data.green);
-
      primary = igt_output_get_plane_type(output, 
DRM_PLANE_TYPE_PRIMARY);
      igt_display_commit2(display, s);
      igt_plane_set_fb(primary, &data.red);
+    igt_fb_set_size(&data.red, primary, mode.hdisplay, mode.vdisplay);
        /* Wait for Flip completion before starting the HDCP 
authentication */
      commit_display_and_wait_for_flip(s);

@@ -458,37 +452,83 @@ static bool sink_hdcp2_capable(igt_output_t 

*output)
      return strstr(buf, "HDCP2.2");
  }
  -static void
-test_content_protection(enum igt_commit_style s, int content_type)
+static void prepare_modeset_on_mst_output(igt_output_t *output, enum 
pipe pipe)
  {
-    igt_display_t *display = &data.display;
-    igt_output_t *output;
-    int valid_tests = 0;
+    drmModeConnectorPtr c = output->config.connector;
+    drmModeModeInfo *mode;
+    igt_plane_t *primary;
+    int i, width, height;
  -    if (data.cp_tests & CP_MEI_RELOAD)
-        igt_require_f(igt_kmod_is_loaded("mei_hdcp"),
-                  "mei_hdcp module is not loaded\n");
+    mode = igt_output_get_mode(output);
  -    for_each_connected_output(display, output) {
+    /*
+     * TODO: Add logic to use the highest possible modes on each 
output.
+     * Currently using 2k modes by default on all the outputs.
+     */

Highest possible mode on all the MST might not be successful. The 
current policy seems fair to use 2K mode.

In a particular MST setup this might fail as well and in that case the 
test will fail with ENOSPACE.

Perhaps we can use igt_display_try_commit() and skip the test if the 
modeset itself fails.

Even if we want to test with  higher modes we need to use try commit 
and find the suitable modes that will work.

Sure. Agreed that if the modeset fails we will simply create noise by 
failing the HDCP test when we still haven't even tried enabling HDCP.

I will use a try commit and skip the test if the modeset fails.

quoted

+    igt_debug("Before mode override: Output %s Mode hdisplay %d Mode 
vdisplay %d\n",
+           output->name, mode->hdisplay, mode->vdisplay);
+
+    if (mode->hdisplay > 1920 && mode->vdisplay > 1080) {
+        for (i = 0; i < c->count_modes; i++) {
+            if (c->modes[i].hdisplay <= 1920 && c->modes[i].vdisplay 
<= 1080) {
+                mode = &c->modes[i];
+                igt_output_override_mode(output, mode);
+                break;
+            }
+        }
+    }
+
+    igt_debug("After mode overide: Output %s Mode hdisplay %d Mode 
vdisplay %d\n",
+           output->name, mode->hdisplay, mode->vdisplay);
+
+    width = mode->hdisplay;
+    height = mode->vdisplay;
+
+    igt_output_set_pipe(output, pipe);
+    primary = igt_output_get_plane_type(output, 
DRM_PLANE_TYPE_PRIMARY);
+    igt_plane_set_fb(primary, NULL);
+    igt_plane_set_fb(primary, pipe % 2 ? &data.red : &data.green);
+    igt_fb_set_size(pipe % 2 ? &data.red : &data.green, primary, 
width, height);
+    igt_plane_set_size(primary, width, height);
+}
+
+static bool output_hdcp_capable(igt_output_t *output, int content_type)
+{
          if (!output->props[IGT_CONNECTOR_CONTENT_PROTECTION])
-            continue;
+            return false;
            if (!output->props[IGT_CONNECTOR_HDCP_CONTENT_TYPE] &&
              content_type)
-            continue;
-
-        igt_info("CP Test execution on %s\n", output->name);
+            return false;
            if (content_type && !sink_hdcp2_capable(output)) {
              igt_info("\tSkip %s (Sink has no HDCP2.2 support)\n",
                   output->name);
-            continue;
+            return false;
          } else if (!sink_hdcp_capable(output)) {
              igt_info("\tSkip %s (Sink has no HDCP support)\n",
                   output->name);
-            continue;
+            return false;
          }
  +        return true;
+}
+
+static void
+test_content_protection(enum igt_commit_style s, int content_type)
+{
+    igt_display_t *display = &data.display;
+    igt_output_t *output;
+    int valid_tests = 0;
+
+    if (data.cp_tests & CP_MEI_RELOAD)
+        igt_require_f(igt_kmod_is_loaded("mei_hdcp"),
+                  "mei_hdcp module is not loaded\n");
+
+    for_each_connected_output(display, output) {
+        if (!output_hdcp_capable(output, content_type))
+            continue;
+
          test_content_protection_on_output(output, s, content_type);
          valid_tests++;
      }

@@ -496,6 +536,150 @@ test_content_protection(enum igt_commit_style 

s, int content_type)
      igt_require_f(valid_tests, "No connector found with HDCP 
capability\n");
  }
  +static int parse_path_blob(char *blob_data)
+{
+    int connector_id;
+    char *encoder;
+
+    encoder = strtok(blob_data, ":");
+    igt_assert_f(!strcmp(encoder, "mst"), "PATH connector property 
expected to have 'mst'\n");
+
+    connector_id = atoi(strtok(NULL, "-"));
+
+    return connector_id;
+}
+
+static bool output_is_dp_mst(igt_output_t *output, int i)
+{
+    drmModePropertyBlobPtr path_blob = NULL;
+    uint64_t path_blob_id;
+    drmModeConnector *connector = output->config.connector;
+    struct kmstest_connector_config config;
+    const char *encoder;
+    int connector_id;
+    static int prev_connector_id;
+
+    kmstest_get_connector_config(data.drm_fd, 
output->config.connector->connector_id, -1, &config);
+    encoder = kmstest_encoder_type_str(config.encoder->encoder_type);
+
+    if (strcmp(encoder, "DP MST"))
+        return false;
+
+    igt_assert(kmstest_get_property(data.drm_fd, 
connector->connector_id,
+           DRM_MODE_OBJECT_CONNECTOR, "PATH", NULL,
+           &path_blob_id, NULL));
+
+    igt_assert(path_blob = drmModeGetPropertyBlob(data.drm_fd, 
path_blob_id));
+
+    connector_id = parse_path_blob((char *) path_blob->data);
+
+    /*
+     * Discarding outputs of other DP MST topology.
+     * Testing only on outputs on the topology we got previously
+     */
+    if (i == 0) {
+        prev_connector_id = connector_id;
+    } else {
+        if (connector_id != prev_connector_id)
+            return false;
+    }
+
+    drmModeFreePropertyBlob(path_blob);
+
+    return true;
+}
+
+static void test_cp_lic_on_mst(igt_output_t *mst_outputs[], int 
valid_outputs, bool first_output)
+{
+    int ret, count;
+    uint64_t val;
+
+    /* Only wait for the first output, this optimizes the test 
execution time */
+    ret = wait_for_prop_value(mst_outputs[first_output], CP_DESIRED, 
LIC_PERIOD_MSEC);
+    igt_assert_f(!ret, "Content Protection LIC Failed on %s\n", 
mst_outputs[0]->name);
+
+    for (count = first_output + 1; count < valid_outputs; count++) {
+        val = igt_output_get_prop(mst_outputs[count], 
IGT_CONNECTOR_CONTENT_PROTECTION);
+        igt_assert_f(val != CP_DESIRED, "Content Protection LIC 
Failed on %s\n", mst_outputs[count]->name);
+    }
+}
+
+static void
+test_content_protection_mst(int content_type)
+{
+    igt_display_t *display = &data.display;
+    igt_output_t *output;
+    int valid_outputs = 0, dp_mst_outputs = 0, ret, count, max_pipe 
= 0, i;
+    enum pipe pipe;
+    igt_output_t *mst_output[IGT_MAX_PIPES], 
*hdcp_mst_output[IGT_MAX_PIPES];
+
+    for_each_pipe(display, pipe)
+        max_pipe++;
+
+    pipe = PIPE_A;
+
+    for_each_connected_output(display, output) {
+        if (!output_is_dp_mst(output, valid_outputs))

This seems to be typo, I think dp_mst_outputs should be instead of 
valid_outputs.

Yes. I missed this while making the changes in the previous version.

I will fix this.

quoted

+            continue;
+
+        igt_assert_f(igt_pipe_connector_valid(pipe, output), 
"Output-pipe combination invalid\n");
+
+        prepare_modeset_on_mst_output(output, pipe);
+        mst_output[dp_mst_outputs++] = output;
+
+        pipe++;
+
+        if (pipe > max_pipe)
+            break;
+    }
+
+    igt_require_f(dp_mst_outputs > 1, "No DP MST set up with >= 2 
outputs found in a single topology\n");
+
+    igt_display_commit2(display, COMMIT_ATOMIC);

As mentioned above, we can use try commit here and skip in caseof 
modeset failure.

Yes, I will update this to skip in case of modeset failure.

Thanks,

Karthik.B.S

Regards,

Ankit

quoted

+
+    for (count = 0; count < dp_mst_outputs; count++) {
+        if (!output_hdcp_capable(mst_output[count], content_type))
+            continue;
+
+        hdcp_mst_output[valid_outputs++] = mst_output[count];
+    }
+
+    igt_require_f(valid_outputs > 1, "DP MST outputs do not have the 
required HDCP support\n");
+
+    for (count = 0; count < valid_outputs; count++) {
+        igt_output_set_prop_value(hdcp_mst_output[count], 
IGT_CONNECTOR_CONTENT_PROTECTION, CP_DESIRED);
+
+        if (output->props[IGT_CONNECTOR_HDCP_CONTENT_TYPE])
+            igt_output_set_prop_value(hdcp_mst_output[count], 
IGT_CONNECTOR_HDCP_CONTENT_TYPE, content_type);
+    }
+
+    igt_display_commit2(display, COMMIT_ATOMIC);
+
+    for (count = 0; count < valid_outputs; count++) {
+        ret = wait_for_prop_value(hdcp_mst_output[count], 
CP_ENABLED, KERNEL_AUTH_TIME_ALLOWED_MSEC);
+        igt_assert_f(ret, "Content Protection not enabled on %s\n", 
hdcp_mst_output[count]->name);
+    }
+
+    if (data.cp_tests & CP_LIC)
+        test_cp_lic_on_mst(hdcp_mst_output, valid_outputs, 0);
+
+    /*
+     * Verify if CP is still enabled on other outputs by disabling 
CP on the first output.
+     */
+    igt_debug("CP Prop being UNDESIRED on %s\n", 
hdcp_mst_output[0]->name);
+    test_cp_disable(hdcp_mst_output[0], COMMIT_ATOMIC);
+
+    /* CP is expected to be still enabled on other outputs*/
+    for (i = 1; i < valid_outputs; i++) {
+        /* Wait for the timeout to verify CP is not disabled */
+        ret = wait_for_prop_value(hdcp_mst_output[i], CP_UNDESIRED, 
KERNEL_DISABLE_TIME_ALLOWED_MSEC);
+        igt_assert_f(!ret, "Content Protection not enabled on %s\n", 
hdcp_mst_output[i]->name);
+    }
+
+    if (data.cp_tests & CP_LIC)
+        test_cp_lic_on_mst(hdcp_mst_output, valid_outputs, 1);
+}
+
  static void test_content_protection_cleanup(void)
  {
      igt_display_t *display = &data.display;

@@ -514,6 +698,31 @@ static void test_content_protection_cleanup(void)

          igt_info("CP Prop being UNDESIRED on %s\n", output->name);
          test_cp_disable(output, COMMIT_ATOMIC);
      }
+
+    igt_remove_fb(data.drm_fd, &data.red);
+    igt_remove_fb(data.drm_fd, &data.green);
+}
+
+static void create_fbs(void)
+{
+    igt_output_t *output;
+    int width, height;
+    drmModeModeInfo *mode;
+
+    for_each_connected_output(&data.display, output) {
+        mode = igt_output_get_mode(output);
+        igt_assert(mode);
+
+        width = max(width, mode->hdisplay);
+        height = max(height, mode->vdisplay);
+    }
+
+    igt_create_color_fb(data.drm_fd, width, height,
+                DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
+                1.f, 0.f, 0.f, &data.red);
+    igt_create_color_fb(data.drm_fd, width, height,
+                DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
+                0.f, 1.f, 0.f, &data.green);
  }
    igt_main

@@ -522,6 +731,8 @@ igt_main

          data.drm_fd = drm_open_driver_master(DRIVER_ANY);
            igt_display_require(&data.display, data.drm_fd);
+
+        create_fbs();
      }
        igt_subtest("legacy") {

@@ -592,6 +803,28 @@ igt_main

          test_content_protection(COMMIT_ATOMIC, HDCP_CONTENT_TYPE_0);
      }
  +    igt_describe("Test Content protection over DP MST");
+    igt_subtest("dp-mst-type-0") {
+        test_content_protection_mst(HDCP_CONTENT_TYPE_0);
+    }
+
+    igt_describe("Test Content protection over DP MST with LIC");
+    igt_subtest("dp-mst-lic-type-0") {
+        data.cp_tests = CP_LIC;
+        test_content_protection_mst(HDCP_CONTENT_TYPE_0);
+    }
+
+    igt_describe("Test Content protection over DP MST");
+    igt_subtest("dp-mst-type-1") {
+        test_content_protection_mst(HDCP_CONTENT_TYPE_1);
+    }
+
+    igt_describe("Test Content protection over DP MST with LIC");
+    igt_subtest("dp-mst-lic-type-1") {
+        data.cp_tests = CP_LIC;
+        test_content_protection_mst(HDCP_CONTENT_TYPE_1);
+    }
+
      igt_fixture {
          test_content_protection_cleanup();
          igt_display_fini(&data.display);

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev