Re: [PATCH i-g-t] tests/amdgpu: Tolerate bandwidth-limited MPO scale combinations in mpo-scale

[Tom Chung <chiahsuan.chung@amd.com>]

Reviewed-by: Tom Chung <chiahsuan.chung@amd.com>

On 7/7/2026 3:42 PM, James Lin wrote:
> [Why]
> amd_plane@mpo-scale-* iterates every source size (up to 4K) and scale
> factor and performs a hard igt_display_commit_atomic() for each MPO
> arrangement. Downscaling still reads the full source surface, so a
> heavily downscaled large source (e.g. 4K at 0.30x) costs as much display
> read bandwidth as a full-size plane. Combined with the full-screen
> overlay and DSC at high refresh rates, the configuration can legitimately
> exceed the display controller's bandwidth and be rejected by the driver
> (DC_FAIL_BANDWIDTH_VALIDATE -> -EINVAL). That is a hardware limit, not a
> scaler defect, but the hard commit turned the legitimate -EINVAL into an
> unparsable test failure (the run aborted during commit with no result
> line emitted).
>
> [How]
> Replace the hard commit in test_plane with igt_display_try_commit_atomic
> and tolerate only a legitimate rejection: a downscale (displayed area
> smaller than the source surface) rejected specifically with -EINVAL. Any
> other errno, or a failing upscale, still fails the test. Bound the number
> of tolerated skips with MPO_MAX_BW_SKIPS so an unexpectedly large number
> of rejections is treated as a real regression rather than isolated
> bandwidth limits. Skipped combinations restore the reference primary
> plane so subsequent iterations start from a committable state;
> configurations that do commit are still fully CRC-validated.
>
> Signed-off-by: James Lin <PingLei.Lin@amd.com>
> ---
>   tests/amdgpu/amd_plane.c | 57 +++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/tests/amdgpu/amd_plane.c b/tests/amdgpu/amd_plane.c
> index 9a08f8e29..0e849b9b4 100644
> --- a/tests/amdgpu/amd_plane.c
> +++ b/tests/amdgpu/amd_plane.c
> @@ -28,6 +28,14 @@ IGT_TEST_DESCRIPTION("Tests for Multi Plane Overlay for single and dual displays
>   
>   /* Maximum pipes on any AMD ASIC. */
>   #define MAX_PIPES 6
> +
> +/*
> + * Upper bound on how many scale/source combinations may be skipped because
> + * the driver legitimately rejects them on display-bandwidth grounds
> + * (DC_FAIL_BANDWIDTH_VALIDATE). Empirically only the heaviest downscales of
> + * the largest sources hit this; many more than that means a real regression.
> + */
> +#define MPO_MAX_BW_SKIPS 4
>   #define DISPLAYS_TO_TEST 2
>   
>   /* (De)gamma LUT. */
> @@ -50,6 +58,7 @@ typedef struct data {
>           int w[MAX_PIPES];
>           int h[MAX_PIPES];
>           int fd;
> +        int bw_skips;
>   } data_t;
>   
>   static const drmModeModeInfo test_mode_1 = {
> @@ -382,6 +391,7 @@ static void test_plane(data_t *data, int n, int x, int y, double dw, double dh,
>   
>   	igt_crc_t test_crc;
>   	igt_display_t *display = &data->display;
> +	int ret;
>   
>   	/* Reference: */
>   
> @@ -404,7 +414,48 @@ static void test_plane(data_t *data, int n, int x, int y, double dw, double dh,
>   	igt_plane_set_position(data->primary[n], x, y);
>   	igt_plane_set_size(data->primary[n], dw, dh);
>   
> -	igt_display_commit_atomic(display, 0, 0);
> +	/*
> +	 * Downscaling reads the full source surface, so a heavily downscaled
> +	 * large (e.g. 4K) MPO source costs as much display read bandwidth as a
> +	 * full-size plane. Combined with the full-screen overlay and (on some
> +	 * panels) DSC at high refresh rates, the resulting configuration can
> +	 * legitimately exceed the display controller's bandwidth and be
> +	 * rejected by the driver (DC_FAIL_BANDWIDTH_VALIDATE -> -EINVAL). That
> +	 * is a hardware limit, not a scaler defect, so skip such combinations
> +	 * rather than failing the test. Configurations that do commit are still
> +	 * fully CRC-validated below.
> +	 */
> +	ret = igt_display_try_commit_atomic(display, 0, NULL);
> +	if (ret != 0) {
> +		/*
> +		 * Only tolerate a *downscale* (displayed area smaller than the
> +		 * source surface) rejected specifically with -EINVAL, which is
> +		 * the DC_FAIL_BANDWIDTH_VALIDATE path. Any other errno, or a
> +		 * failing upscale, is a real bug and must fail the test.
> +		 */
> +		bool downscale = (dw < fbc[n].test_primary.width ||
> +				  dh < fbc[n].test_primary.height);
> +
> +		igt_assert_f(downscale && ret == -EINVAL,
> +			     "atomic commit rejected unexpectedly (ret=%d, downscale=%d): n=%d dw=%.0f dh=%.0f src=%dx%d pw=%d ph=%d\n",
> +			     ret, downscale, n, dw, dh,
> +			     fbc[n].test_primary.width,
> +			     fbc[n].test_primary.height, pw, ph);
> +
> +		data->bw_skips++;
> +		igt_assert_f(data->bw_skips <= MPO_MAX_BW_SKIPS,
> +			     "too many MPO scale combinations skipped (%d > %d); likely a real regression, not isolated bandwidth limits\n",
> +			     data->bw_skips, MPO_MAX_BW_SKIPS);
> +
> +		igt_info("Skipping unsupported scale (likely display bandwidth limit): n=%d dw=%.0f dh=%.0f pw=%d ph=%d (skip %d/%d)\n",
> +			 n, dw, dh, pw, ph, data->bw_skips, MPO_MAX_BW_SKIPS);
> +		igt_plane_set_fb(data->overlay[n], NULL);
> +		igt_plane_set_fb(data->primary[n], &fbc[n].ref_primary);
> +		igt_plane_set_position(data->primary[n], 0, 0);
> +		igt_plane_set_size(data->primary[n], pw, ph);
> +		igt_display_commit_atomic(display, 0, 0);
> +		return;
> +	}
>   	igt_pipe_crc_collect_crc(data->pipe_crc[n], &test_crc);
>   	igt_plane_set_fb(data->overlay[n], NULL);
>   
> @@ -616,6 +667,8 @@ static void test_display_mpo(data_t *data, enum test test, uint32_t format, int
>   
>   	test_init(data);
>   
> +	data->bw_skips = 0;
> +
>   	/* Skip test if we don't have 2 overlay planes */
>   	if (test == MPO_MULTI_OVERLAY)
>   		igt_skip_on(!data->overlay2[0]);
> @@ -662,6 +715,8 @@ static void test_display_mpo(data_t *data, enum test test, uint32_t format, int
>   	for (int n = 0; n < display_count; n++)
>   		igt_pipe_crc_collect_crc(data->pipe_crc[n], &fb[n].ref_crc);
>   
> +	igt_kmsg(KMSG_WARNING "MPOSCALE setup done (ref crc collected), entering scaling loop\n");
> +
>   	for (int i = 0; i < ARRAY_SIZE(videos); ++i) {
>   
>   		/* Video(mpo) should be in the middle when it transitions between displays. This