Re: [PATCH v2 2/2] kms_content_protection: Add Content Protection test

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

 



On Tue, Dec 19, 2017 at 9:28 AM, Ramalingam C <ramalingam.c@xxxxxxxxx> wrote:
> Adding few more findings from the IGT usage with kernel solutions.
>
>
>
> On Wednesday 13 December 2017 04:07 PM, Ramalingam C wrote:
>>
>> Sean,
>>
>> Adding few more observations.
>>
>>
>> On Thursday 07 December 2017 05:47 AM, Sean Paul wrote:
>>>
>>> Pretty simple test:
>>> - initializes the output
>>> - clears the content protection property
>>> - verifies that it clears
>>> - sets the content protection property to desired
>>> - verifies that it transitions to enabled
>>>
>>> Does this for both legacy and atomic.
>>>
>>> Changes in v2:
>>> - Don't check for i915 gen
>>> - Skip test if Content Protection property is absent
>>>
>>> Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
>>> ---
>>>   lib/igt_kms.c                  |   3 +-
>>>   lib/igt_kms.h                  |   1 +
>>>   tests/Makefile.sources         |   1 +
>>>   tests/kms_content_protection.c | 129
>>> +++++++++++++++++++++++++++++++++++++++++
>>>   tests/meson.build              |   1 +
>>>   5 files changed, 134 insertions(+), 1 deletion(-)
>>>   create mode 100644 tests/kms_content_protection.c
>>>
>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>>> index 125ecb19..907db694 100644
>>> --- a/lib/igt_kms.c
>>> +++ b/lib/igt_kms.c
>>> @@ -190,7 +190,8 @@ const char
>>> *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
>>>       "scaling mode",
>>>       "CRTC_ID",
>>>       "DPMS",
>>> -    "Broadcast RGB"
>>> +    "Broadcast RGB",
>>> +    "Content Protection"
>>>   };
>>>     /*
>>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>>> index 2a480bf3..93e59dc7 100644
>>> --- a/lib/igt_kms.h
>>> +++ b/lib/igt_kms.h
>>> @@ -118,6 +118,7 @@ enum igt_atomic_connector_properties {
>>>          IGT_CONNECTOR_CRTC_ID,
>>>          IGT_CONNECTOR_DPMS,
>>>          IGT_CONNECTOR_BROADCAST_RGB,
>>> +       IGT_CONNECTOR_CONTENT_PROTECTION,
>>>          IGT_NUM_CONNECTOR_PROPS
>>>   };
>>>   diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>>> index 34ca71a0..e0466411 100644
>>> --- a/tests/Makefile.sources
>>> +++ b/tests/Makefile.sources
>>> @@ -179,6 +179,7 @@ TESTS_progs = \
>>>       kms_chv_cursor_fail \
>>>       kms_color \
>>>       kms_concurrent \
>>> +    kms_content_protection\
>>>       kms_crtc_background_color \
>>>       kms_cursor_crc \
>>>       kms_cursor_legacy \
>>> diff --git a/tests/kms_content_protection.c
>>> b/tests/kms_content_protection.c
>>> new file mode 100644
>>> index 00000000..5d61b257
>>> --- /dev/null
>>> +++ b/tests/kms_content_protection.c
>>> @@ -0,0 +1,129 @@
>>> +/*
>>> + * Copyright © 2017 Google, Inc.
>>> + *
>>> + * Permission is hereby granted, free of charge, to any person obtaining
>>> a
>>> + * copy of this software and associated documentation files (the
>>> "Software"),
>>> + * to deal in the Software without restriction, including without
>>> limitation
>>> + * the rights to use, copy, modify, merge, publish, distribute,
>>> sublicense,
>>> + * and/or sell copies of the Software, and to permit persons to whom the
>>> + * Software is furnished to do so, subject to the following conditions:
>>> + *
>>> + * The above copyright notice and this permission notice (including the
>>> next
>>> + * paragraph) shall be included in all copies or substantial portions of
>>> the
>>> + * Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>> EXPRESS OR
>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>> MERCHANTABILITY,
>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
>>> SHALL
>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>>> OTHER
>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>>> ARISING
>>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>>> DEALINGS
>>> + * IN THE SOFTWARE.
>>> + *
>>> + */
>>> +
>>> +#include "igt.h"
>>> +
>>> +IGT_TEST_DESCRIPTION("Test content protection (HDCP)");
>>> +
>>> +typedef struct {
>>> +    int drm_fd;
>>> +    igt_display_t display;
>>> +} data_t;
>>> +
>>> +static bool
>>> +wait_for_prop_value(igt_output_t *output, uint64_t expected)
>>> +{
>>> +    uint64_t val;
>>> +    int i;
>>> +
>>> +    for (i = 0; i < 2000; i++) {
>>
>> we need 5+Sec to complete the Second part of authentication, in case of
>> repeater with max downstream topology.
>> So we might need to wait for 6000(6Sec) loops!?
>>>
>>> +        val = igt_output_get_prop(output,
>>> +                      IGT_CONNECTOR_CONTENT_PROTECTION);
>>> +        if (val == expected)
>>> +            return true;
>>> +        usleep(1000);
>>> +    }
>>> +    igt_info("prop_value mismatch %ld != %ld\n", val, expected);
>>> +    return false;
>>> +}
>>> +
>>> +static void
>>> +test_pipe_output(igt_display_t *display, const enum pipe pipe,
>>> +         igt_output_t *output, enum igt_commit_style s)
>>> +{
>>> +    drmModeModeInfo mode;
>>> +    igt_plane_t *primary;
>>> +    struct igt_fb red;
>>> +    bool ret;
>>> +
>>> +    igt_assert(kmstest_get_connector_default_mode(
>>> +            display->drm_fd, output->config.connector, &mode));
>>> +
>>> +    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, &red);
>>> +    primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>>> +    igt_plane_set_fb(primary, &red);
>>> +    igt_display_commit2(display, s);
>>> +
>>> +    igt_output_set_prop_value(output, IGT_CONNECTOR_CONTENT_PROTECTION,
>>> 0);
>>> +    igt_display_commit2(display, s);
>>> +
>>> +    ret = wait_for_prop_value(output, 0);
>>> +    igt_require_f(ret, "Content Protection not cleared\n");
>>
>> When expected property state is not met, you might want to use
>> igt_assert_f to fail the subtest. Here you are just skipping the subtest.
>>>
>>> +
>>> +    igt_output_set_prop_value(output, IGT_CONNECTOR_CONTENT_PROTECTION,
>>> 1);
>>> +    igt_display_commit2(display, s);
>>> +
>>> +    ret = wait_for_prop_value(output, 2);
>
>
> As there are range of HDCP sinks which might fail to provide the R0 in
> 100mSec in the first attempt. But passes the condition in next attempt.
> In the testing it is observed that many panel's widely used showcase above
> mentioned behavior.
>
> So we need to retry on HDCP enable before declaring the HDCP failure.
>

Instead of retrying the whole thing, why not just make the 100ms
timeout bigger? What values >100ms have you observed?

>>> +    igt_require_f(ret, "Content Protection not enabled\n");
>>
>> same as above. igt_assert_f needed?
>>
>> And when you are done with a connector(output), we cant leave the content
>> protection in desired state.
>> Else when we enable the connector for some other IGT, content protection
>> authentication will be attempted by kernel.
>> So when we are done with a connector, we need to clear the content
>> protection state(set to OFF) of the connector.
>>
>>> +
>>> +    igt_plane_set_fb(primary, NULL);
>>> +    igt_output_set_pipe(output, PIPE_NONE);
>>> +}
>>> +
>>> +static void
>>> +test_content_protection(igt_display_t *display, enum igt_commit_style s)
>>> +{
>>> +    igt_output_t *output;
>>> +    enum pipe pipe;
>>> +    int valid_tests = 0;
>>> +
>>> +    for_each_pipe_with_valid_output(display, pipe, output) {
>
> From the testing of this IGT with kernel changes, observed a need of
> relooking at HDCP subtests definition. Reasoning is as follows.
>
> And here we are taking each pipe and their compatible outputs(connectors)
> for hdcp test. Like PIPEA + HDMI-A-1, PIPEB+HDMI-A-1 etc.
> But as HDCP is associated to connectors, we need to iterate on hdcp capable
> connector and choose the compatible pipe.
> In that way, we will be able test the HDCP on all hdcp capable connectors.
>

I'm confused. The documentation for this loop says it tries every
combination of connector+pipe for every connected connector. Can you
explain what I'm missing?

Sean

> And since irrespective of the hdcp status (pass/fail) on a connector, we
> need to test hdcp on other connectors one after another.
> So this can be achieved if the test of HDCP on each connector is implemented
> as different IGT subtests.
>
>>> +        if (!output->props[IGT_CONNECTOR_CONTENT_PROTECTION])
>>> +            continue;
>>> +
>>> +        test_pipe_output(display, pipe, output, s);
>>> +        valid_tests++;
>>> +    }
>>> +    igt_require_f(valid_tests, "no support for content proteciton
>>> found\n");
>>> +}
>>> +
>>> +igt_main
>>> +{
>>> +    data_t data = {};
>>> +
>>> +    igt_fixture {
>>> +        igt_skip_on_simulation();
>>> +
>>> +        data.drm_fd = drm_open_driver(DRIVER_ANY);
>>
>> You need Master privilege here. Hence might want to use
>> drm_open_driver_master() instead.
>>
>> -Ram
>>>
>>> +
>>> +        igt_display_init(&data.display, data.drm_fd);
>>> +    }
>>> +
>>> +
>>> +    igt_subtest("legacy")
>>> +        test_content_protection(&data.display, COMMIT_LEGACY);
>
> As discussed above, we need to consider connector-x-legacy as a subtest.
>>>
>>> +
>>> +    igt_subtest("atomic") {
>>> +        igt_require(data.display.is_atomic);
>>> +        test_content_protection(&data.display, COMMIT_ATOMIC);
>
> As discussed above, we need to consider connector-x-atomic as a subtest.
>
> -Ram
>
>>> +    }
>>> +
>>> +    igt_fixture
>>> +        igt_display_fini(&data.display);
>>> +}
>>> diff --git a/tests/meson.build b/tests/meson.build
>>> index 59ccd9a6..b12c35c0 100644
>>> --- a/tests/meson.build
>>> +++ b/tests/meson.build
>>> @@ -157,6 +157,7 @@ test_progs = [
>>>       'kms_chv_cursor_fail',
>>>       'kms_color',
>>>       'kms_concurrent',
>>> +    'kms_content_protection',
>>>       'kms_crtc_background_color',
>>>       'kms_cursor_crc',
>>>       'kms_cursor_legacy',
>>
>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux