[igt-dev] [PATCH i-g-t 2/2] tests/kms_dp_dsc: Add a subtest to force DSC output BPP
Manasi Navare
manasi.d.navare at intel.com
Tue Jun 11 19:55:25 UTC 2019
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