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
