[igt-dev] [PATCH i-g-t 2/2] tests/kms_dp_dsc: Add a subtest to force DSC output BPP
Srivatsa, Anusha
anusha.srivatsa at intel.com
Thu Jun 20 00:44:08 UTC 2019
The subtest is still unstable.
The debugfs node - i915_dsc_fec_support is both read and write. We use it to check if sink supports DSC and to force the DSC, Which I think leave it unstable when used for the second time - for the bpp subtest.
IT is either the debugfs node state or maybe the crtc state from the force-dsc subtest that is causing the unstable nature,
Is it a good option to separate the debugfs nodes to read and write specific ?
Manasi, what do you think?
Anusha
>-----Original Message-----
>From: Navare, Manasi D
>Sent: Tuesday, June 11, 2019 12:55 PM
>To: Srivatsa, Anusha <anusha.srivatsa at intel.com>
>Cc: igt-dev at lists.freedesktop.org; Latvala, Petri <petri.latvala at intel.com>
>Subject: Re: [PATCH i-g-t 2/2] tests/kms_dp_dsc: Add a subtest to force DSC
>output BPP
>
>On Mon, Jun 10, 2019 at 04:26:17PM -0700, Anusha Srivatsa wrote:
>> This subtest uses the accepted DSC BPPs and tries to force a modeset
>> by setting a certain BPP as the output BPP for a connector.
>>
>> Cc: Petri Latvala <petri.latvala at intel.com>
>> Cc: Manasi Navare <manasi.d.navare at intel.com>
>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
>> ---
>> tests/kms_dp_dsc.c | 94
>> +++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 92 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c index
>> 7f2bf276..f936eb33 100644
>> --- a/tests/kms_dp_dsc.c
>> +++ b/tests/kms_dp_dsc.c
>> @@ -46,7 +46,8 @@
>>
>> enum dsc_test_type
>> {
>> - test_basic_dsc_enable
>> + test_basic_dsc_enable,
>> + test_basic_dsc_enable_bpp
>> };
>>
>> typedef struct {
>> @@ -67,6 +68,7 @@ typedef struct {
>>
>> bool force_dsc_en_orig;
>> int force_dsc_restore_fd = -1;
>> +int new_bpp;
>>
>> static inline void manual(const char *expected) { @@ -157,6 +159,17
>> @@ static void restore_force_dsc_en(void)
>> force_dsc_restore_fd = -1;
>> }
>>
>> +static void force_dp_dsc_enable_bpp(data_t *data) {
>> + char file_name[128] = {0};
>> + char buffer[20];
>> + sprintf(buffer, "%d", new_bpp);
>> + strcpy(file_name, data->conn_name);
>> + strcat(file_name, "/i915_dsc_bpp_slice_support");
>> + igt_debug ("Forcing DSC BPP to %d on %s\n", new_bpp, data-
>>conn_name);
>> + igt_sysfs_write(data->debugfs_fd, file_name, buffer,
>> +sizeof(buffer));
>
>Check the return value of igt_sysfs_write and assert if write failed
>
>> +}
>> +
>> static void test_cleanup(data_t *data) {
>> igt_plane_t *primary;
>> @@ -229,6 +242,44 @@ static void update_display(data_t *data, enum
>dsc_test_type test_type)
>> "Default DSC enable failed on Connector: %s Pipe:
>%s\n",
>> data->conn_name,
>> kmstest_pipe_name(data->pipe));
>> + } else if (test_type == test_basic_dsc_enable_bpp) {
>> + bool enabled;
>> +
>> + igt_debug("DSC is supported on %s\n", data->conn_name);
>> +
>
>Unnecessary newline
>
>> + save_force_dsc_en(data);
>> + force_dp_dsc_enable(data);
>> +
>> + igt_debug("Trying to set BPP to %d\n", new_bpp);
>> +
>> + force_dp_dsc_enable_bpp(data);
>> +
>> + igt_output_set_pipe(data->output, data->pipe);
>> + igt_create_pattern_fb(data->drm_fd, data->mode->hdisplay,
>> + data->mode->vdisplay,
>> + DRM_FORMAT_XRGB8888,
>> + LOCAL_DRM_FORMAT_MOD_NONE,
>> + &data->fb_test_pattern);
>> + primary = igt_output_get_plane_type(data->output,
>> + DRM_PLANE_TYPE_PRIMARY);
>> + /* Now set the output to the desired mode */
>> + igt_plane_set_fb(primary, &data->fb_test_pattern);
>> + igt_display_commit(&data->display);
>> +
>> + /*
>> + * Until we have CRC check support, manually check if RGB test
>> + * pattern has no corruption.
>> + */
>> + manual("RGB test pattern without corruption");
>> +
>> + enabled = is_dp_dsc_enabled(data);
>> + restore_force_dsc_en();
>
>Not sure if we need to restore the DSC BPP value? What does the kernel code set
>the force_dsc_bpp value to if not forced by IGT?
>
>> +
>> + igt_assert_f(enabled,
>> + "Default DSC BPP enable failed on Connector: %s
>Pipe: %s\n",
>> + data->conn_name,
>> + kmstest_pipe_name(data->pipe));
>> +
>> } else {
>> igt_assert(!"Unknown test type\n");
>> }
>> @@ -256,7 +307,9 @@ igt_main
>> igt_output_t *output;
>> drmModeRes *res;
>> drmModeConnector *connector;
>> - int i, test_conn_cnt, test_cnt;
>> + int i, j, test_conn_cnt, test_cnt;
>> + int dp_dsc_supported_compressed_bpp[] = {8, 10, 12, 15};
>
> May be a add a comment here saying these are the supported compressed BPPs
>on Gen 11
>
>> +
>> int tests[] = {DRM_MODE_CONNECTOR_eDP,
>> DRM_MODE_CONNECTOR_DisplayPort};
>>
>> igt_fixture {
>> @@ -300,6 +353,43 @@ igt_main
>> }
>> igt_skip_on(test_conn_cnt == 0);
>> }
>> +
>> + for (j = 0; j < ARRAY_SIZE(dp_dsc_supported_compressed_bpp);
>j++) {
>> + new_bpp = dp_dsc_supported_compressed_bpp[j];
>> + igt_subtest_f("basic-dsc-enable-%dbpp-%s", new_bpp,
>> + kmstest_connector_type_str(tests[test_cnt]))
>{
>> + test_conn_cnt = 0;
>
>Check the indentation for the code within the igt_subtest_f, it needs one more
>tab
>
>> + for (i = 0; i < res->count_connectors; i++) {
>> + connector =
>drmModeGetConnectorCurrent(data.drm_fd,
>> + res-
>>connectors[i]);
>> + if (connector->connection !=
>DRM_MODE_CONNECTED ||
>> + connector->connector_type !=
>> + tests[test_cnt])
>> + continue;
>> + output =
>igt_output_from_connector(&data.display, connector);
>> + sprintf(data.conn_name, "%s-%d",
>> + kmstest_connector_type_str(connector-
>>connector_type),
>> + connector->connector_type_id);
>> + printf("connector name: %s test_conn_count
>%d\n", data.conn_name,
>> +test_conn_cnt++);
>
>printf is redundant here, if you want you could have this in igt_debug so its
>printed only for debug
>
>> + if(!is_dp_dsc_supported(&data)) {
>> + igt_debug("DSC not supported on connector
>%s \n",
>> + data.conn_name);
>> + continue;
>> + }
>> + if (connector->connector_type ==
>DRM_MODE_CONNECTOR_DisplayPort &&
>> + !is_dp_fec_supported(&data)) {
>> + igt_debug("DSC cannot be enabled
>without FEC on %s\n",
>> + data.conn_name);
>> + continue;
>> + }
>> +
>
>Redundant newline
>
>Manasi
>
>
>> + test_conn_cnt++;
>> + run_test(&data, output,
>test_basic_dsc_enable_bpp);
>> + }
>> + igt_skip_on(test_conn_cnt == 0);
>> + }
>> + }
>> +
>> }
>>
>> igt_fixture {
>> --
>> 2.17.1
>>
More information about the igt-dev
mailing list