[igt-dev] [PATCH i-g-t v7 1/2] tests/kms_content_protection: Add MST subtests
Karthik B S
karthik.b.s at intel.com
Wed Feb 17 04:00:33 UTC 2021
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:
>> 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 <anshuman.gupta at intel.com>
>> Signed-off-by: Karthik B S <karthik.b.s at intel.com>
>> ---
>> 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.
>
>> + 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.
>
>
>> + 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
>
>> +
>> + 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);
More information about the igt-dev
mailing list