Re: [igt-dev] [v5 i-g-t 05/14] tests/kms_content_protection: Fix mode selection for 2x tests

From: "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>
To: Bhanuprakash Modem <bhanuprakash.modem@intel.com>,
	igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [v5 i-g-t 05/14] tests/kms_content_protection: Fix mode selection for 2x tests
Date: Tue, 11 May 2021 11:24:59 +0530	[thread overview]
Message-ID: <cc6e5a18-3d44-9982-6e3a-e62aee292204@intel.com> (raw)
In-Reply-To: <20210508162303.8235-6-bhanuprakash.modem@intel.com>

Nice to see #TODO for the test getting implemented :)

The test was earlier trying with hard coded 2K mode on MST panels. This 
takes care of that.

Please find comments inline:

On 5/8/2021 9:52 PM, Bhanuprakash Modem wrote:
> 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.
>
> This patch will find a combination of modes that fit into the BW.
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> ---
>   tests/kms_content_protection.c | 41 +++++++++++++---------------------
>   1 file changed, 16 insertions(+), 25 deletions(-)
>
> diff --git a/tests/kms_content_protection.c b/tests/kms_content_protection.c
> index 5a75b871c..1d1a439a9 100644
> --- a/tests/kms_content_protection.c
> +++ b/tests/kms_content_protection.c
> @@ -452,39 +452,18 @@ static bool sink_hdcp2_capable(igt_output_t *output)
>   	return strstr(buf, "HDCP2.2");
>   }
>   
> -static void prepare_modeset_on_mst_output(igt_output_t *output, enum pipe pipe)
> +static void prepare_modeset_on_mst_output(igt_output_t *output)
>   {
> -	drmModeConnectorPtr c = output->config.connector;
>   	drmModeModeInfo *mode;
>   	igt_plane_t *primary;
> -	int i, width, height;
> +	int width, height;
> +	enum pipe pipe = output->pending_pipe;
>   
>   	mode = igt_output_get_mode(output);
>   
> -	/*
> -	 * TODO: Add logic to use the highest possible modes on each output.
> -	 * Currently using 2k modes by default on all the outputs.
> -	 */
> -	igt_debug("Before mode override: Output %s Mode hdisplay %d Mode vdisplay %d\n",
> -		   output->name, mode->hdisplay, mode->vdisplay);
> -
> -	if (mode->hdisplay > 1920 && mode->vdisplay > 1080) {
> -		for (i = 0; i < c->count_modes; i++) {
> -			if (c->modes[i].hdisplay <= 1920 && c->modes[i].vdisplay <= 1080) {
> -				mode = &c->modes[i];
> -				igt_output_override_mode(output, mode);
> -				break;
> -			}
> -		}
> -	}
> -
> -	igt_debug("After mode overide: Output %s Mode hdisplay %d Mode vdisplay %d\n",
> -		   output->name, mode->hdisplay, mode->vdisplay);
> -
>   	width = mode->hdisplay;
>   	height = mode->vdisplay;
>   
> -	igt_output_set_pipe(output, pipe);
>   	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>   	igt_plane_set_fb(primary, NULL);
>   	igt_plane_set_fb(primary, pipe % 2 ? &data.red : &data.green);
> @@ -624,7 +603,8 @@ test_content_protection_mst(int content_type)
>   
>   		igt_assert_f(igt_pipe_connector_valid(pipe, output), "Output-pipe combination invalid\n");
>   
> -		prepare_modeset_on_mst_output(output, pipe);
> +		igt_output_set_pipe(output, pipe);
> +		prepare_modeset_on_mst_output(output);
>   		mst_output[dp_mst_outputs++] = output;
>   
>   		pipe++;
> @@ -635,6 +615,17 @@ test_content_protection_mst(int content_type)
>   
>   	igt_require_f(dp_mst_outputs > 1, "No DP MST set up with >= 2 outputs found in a single topology\n");
>   
> +	if (igt_display_try_commit_atomic(display,
> +				DRM_MODE_ATOMIC_TEST_ONLY |
> +				DRM_MODE_ATOMIC_ALLOW_MODESET,
> +				NULL) != 0) {
> +		bool found = igt_override_all_active_output_modes_to_fit_bw(display);
> +		igt_require_f(found, "No valid mode combo found for MST modeset\n");
> +
> +		for (count = 0; count < dp_mst_outputs; count++)
> +			prepare_modeset_on_mst_output(mst_output[count]);
> +	}
> +
>   	ret = igt_display_try_commit2(display, COMMIT_ATOMIC);

The above statement should be in the else block for the if-block. If the 
try commit in the if block passes we don't need another commit.

With that fixed:

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

>   	igt_require_f(ret == 0, "Commit failure during MST modeset\n");
>   
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev