[igt-dev] [PATCH i-g-t 1/2] tests/kms_display_modes: Add negative test for extended display
Modem, Bhanuprakash
bhanuprakash.modem at intel.com
Wed May 3 05:43:05 UTC 2023
Hi Thasleem,
On Tue-02-05-2023 04:32 pm, Mohammed Thasleem wrote:
> Added negative test to validte ENOSPC when two 2k-4k or 4k-4k
> moniters connected through MST.
> This test added to provide bandwidth issue in MST config.
>
> Example:
> When two monitors connected through MST, the second monitor
> also tries to use the same mode. So two such modes may not
> fit into the link bandwidth. So, iterate through connected
> outputs & modes and find a invalid combination.
>
> v2: Rebased on tip.
> v3: -Code cleanup and updated description.
> -Free path_blob before call return. (Kamil)
> v4: Updated code formatting and function description. (Jeevan)
> v5: Updated commit description and minor changes.
>
> Signed-off-by: Mohammed Thasleem <mohammed.thasleem at intel.com>
> Reviewed-by: Jeevan B <jeevan.b at intel.com>
> ---
> tests/kms_display_modes.c | 166 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 166 insertions(+)
>
> diff --git a/tests/kms_display_modes.c b/tests/kms_display_modes.c
> index d69c7b931..27c13ba05 100644
> --- a/tests/kms_display_modes.c
> +++ b/tests/kms_display_modes.c
> @@ -26,14 +26,113 @@
>
> #include "igt.h"
>
> +#define HDISPLAY_2K 2560
> +#define VDISPLAY_2K 1440
Unused macros, please drop.
> +
> +#define HDISPLAY_4K 3840
> +#define VDISPLAY_4K 2160
> +
> IGT_TEST_DESCRIPTION("Test Display Modes");
>
> typedef struct {
> int drm_fd;
> igt_display_t display;
> + drmModeModeInfo mode_mst[2];
> + igt_output_t *mst_output[2];
> int n_pipes;
> } data_t;
>
> +/*Get higher mode supported by panel*/
Please follow the proper commenting style.
Example:
/* This is single line comment. */
/*
* This is multi-line
* comment.
*/
> +static drmModeModeInfo *get_highres_mode(igt_output_t *output)
> +{
> + drmModeConnector *connector = output->config.connector;
> + drmModeModeInfo *highest_mode = NULL;
> +
> + igt_sort_connector_modes(connector, sort_drm_modes_by_res_dsc);
> + highest_mode = &connector->modes[0];
> +
> + return highest_mode;
> +}
> +
> +/*Get the 4k or less then 4k mode of connected panel*/
> +static drmModeModeInfo *get_mode(igt_output_t *output)
> +{
> + int j;
> + drmModeModeInfo *required_mode = NULL;
> + drmModeConnector *connector = output->config.connector;
> +
> + required_mode = igt_output_get_mode(output);
> + if (required_mode->vdisplay <= VDISPLAY_4K &&
> + required_mode->hdisplay <= HDISPLAY_4K) {
> + return required_mode;
> + }
Please write a comment about this logic.
> +
> + igt_sort_connector_modes(connector, sort_drm_modes_by_res_dsc);
> + for (j = 0; j < connector->count_modes; j++) {
> + if (connector->modes[j].vdisplay <= VDISPLAY_4K &&
> + connector->modes[j].hdisplay <= HDISPLAY_4K) {
> + required_mode = &connector->modes[j];
> + break;
> + }
> + }
> +
> + return required_mode;
> +}
> +
> +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(data_t *data, 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);
> +
> + drmModeFreePropertyBlob(path_blob);
> +
> + /*
> + * 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;
> + }
Not understood this block, we already checking the encoder == "DP MST"
and again reading the connector_id from PATH property. Is it not a
redundant?
> +
> + return true;
> +}
> +
> static void run_extendedmode_basic(data_t *data,
> enum pipe pipe1, igt_output_t *output1,
> enum pipe pipe2, igt_output_t *output2)
> @@ -173,8 +272,46 @@ static void run_extendedmode_test(data_t *data) {
> }
> }
>
> +static void run_extendedmode_negative(data_t *data, int pipe1, int pipe2)
> +{
> + struct igt_fb fbs[2];
> + igt_display_t *display = &data->display;
> + igt_plane_t *plane[2];
> + int ret;
> +
> + igt_display_reset(display);
> +
> + igt_output_set_pipe(data->mst_output[0], pipe1);
> + igt_output_set_pipe(data->mst_output[1], pipe2);
> +
> + igt_create_color_fb(data->drm_fd, data->mode_mst[0].hdisplay, data->mode_mst[0].vdisplay,
> + DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, 1, 0, 0, &fbs[0]);
> + igt_create_color_fb(data->drm_fd, data->mode_mst[1].hdisplay, data->mode_mst[1].vdisplay,
> + DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, 0, 0, 1, &fbs[1]);
> +
> + plane[0] = igt_pipe_get_plane_type(&display->pipes[pipe1], DRM_PLANE_TYPE_PRIMARY);
> + plane[1] = igt_pipe_get_plane_type(&display->pipes[pipe2], DRM_PLANE_TYPE_PRIMARY);
> +
> + igt_plane_set_fb(plane[0], &fbs[0]);
> + igt_fb_set_size(&fbs[0], plane[0], data->mode_mst[0].hdisplay, data->mode_mst[0].vdisplay);
> + igt_plane_set_size(plane[0], data->mode_mst[0].hdisplay, data->mode_mst[0].vdisplay);
> +
> + igt_plane_set_fb(plane[1], &fbs[1]);
> + igt_fb_set_size(&fbs[1], plane[1], data->mode_mst[1].hdisplay, data->mode_mst[1].vdisplay);
> + igt_plane_set_size(plane[1], data->mode_mst[1].hdisplay, data->mode_mst[1].vdisplay);
> +
> + igt_output_override_mode(data->mst_output[0], &data->mode_mst[0]);
> + igt_output_override_mode(data->mst_output[1], &data->mode_mst[1]);
> +
> + ret = igt_display_try_commit2(display, COMMIT_ATOMIC);
Don't we need to check for the bigjoiner constraint?
Example: 4K on pipe-c + 8K on pipe-d will throw -EINVAL.
> + igt_assert(ret != 0 && errno == ENOSPC);
> +}
> +
> igt_main
> {
> + int dp_mst_outputs = 0, count = 0;
> + enum pipe pipe1, pipe2;
> + igt_output_t *output;
> data_t data;
>
> igt_fixture {
> @@ -182,12 +319,41 @@ igt_main
> kmstest_set_vt_graphics_mode();
> igt_display_require(&data.display, data.drm_fd);
> igt_display_require_output(&data.display);
> +
> + for_each_connected_output(&data.display, output) {
> + data.mst_output[count++] = output;
> + if (output_is_dp_mst(&data, output, dp_mst_outputs))
> + dp_mst_outputs++;
> + }
> }
>
> igt_describe("Test for validating display extended mode with a pair of connected displays");
> igt_subtest_with_dynamic("extended-mode-basic")
> run_extendedmode_test(&data);
>
> + igt_describe("Negative test for validating display extended mode with a pair of connected "
> + "2k-4k or 4k-4k displays");
> + igt_subtest_with_dynamic("mst-extended-mode-negative") {
> + igt_require_f(dp_mst_outputs > 1, "MST not found more then one\n");
> +
> + memcpy(&data.mode_mst[0], get_mode(data.mst_output[0]), sizeof(drmModeModeInfo));
> + memcpy(&data.mode_mst[1], get_highres_mode(data.mst_output[1]),
----------------^
Fix the alignment (an extra space)
> + sizeof(drmModeModeInfo));
> + igt_require_f((data.mode_mst[1].hdisplay >= HDISPLAY_4K &&
> + data.mode_mst[1].vdisplay >= VDISPLAY_4K), "4k panel not found\n");
> +
> + for_each_pipe(&data.display, pipe1) {
> + for_each_pipe(&data.display, pipe2) {
> + if (pipe1 == pipe2)
> + continue;
> +
> + igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe1),
> + kmstest_pipe_name(pipe2))
> + run_extendedmode_negative(&data, pipe1, pipe2);
> + }
> + }
> + }
> +
> igt_fixture {
> igt_display_fini(&data.display);
> }
More information about the igt-dev
mailing list