Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Juha-Pekka Heikkilä" <juhapekka.heikkila@gmail.com>
To: Sowmiya S <sowmiya.s@intel.com>, igt-dev@lists.freedesktop.org
Cc: swati2.sharma@intel.com, chaitanya.kumar.borah@intel.com
Subject: Re: [PATCH i-g-t v2 2/3] tests/intel/kms_pipe_stress: fix plane source crop and separate cursor loop
Date: Mon, 29 Jun 2026 22:28:51 +0300	[thread overview]
Message-ID: <ae1eb7d5-2563-42fd-8026-6856395ba364@gmail.com> (raw)
In-Reply-To: <20260602162422.3360253-3-sowmiya.s@intel.com>

This original test look bit messy to my eye but I think this change does 
what it says and is all ok.

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>

On 02/06/2026 19.24, Sowmiya S wrote:
> Pass 3/4 destination size as initial source crop to
> universal_plane_set_fb() to avoid an immediate
> downscale. Keep source in sync with igt_fb_set_size()
> in the reduction loop so the downscale ratio never
> exceeds the hardware limit (~3x).
> 
> Move cursor handling out of the overlay loop into a
> dedicated second for_each_plane_on_crtc loop with a
> DRM_MODE_ATOMIC_TEST_ONLY commit. If the test commit
> rejects the cursor (e.g. unsupported format or modifier),
> disable it gracefully so the final commit with only overlay
> planes can succeed.
> 
> Signed-off-by: Sowmiya S <sowmiya.s@intel.com>
> ---
>   tests/intel/kms_pipe_stress.c | 110 +++++++++++++++++++++-------------
>   1 file changed, 68 insertions(+), 42 deletions(-)
> 
> diff --git a/tests/intel/kms_pipe_stress.c b/tests/intel/kms_pipe_stress.c
> index 8724a674c..a84433b9d 100644
> --- a/tests/intel/kms_pipe_stress.c
> +++ b/tests/intel/kms_pipe_stress.c
> @@ -425,57 +425,83 @@ static int crtc_stress(struct data *data, igt_output_t *output,
>   	if (!data->num_planes[crtc->pipe] || !new_mode)
>   		return 0;
>   
> -	for_each_plane_on_crtc(crtc,
> -			       plane) {
> +	/* Set up overlay/primary planes up to the configured limit. */
> +	for_each_plane_on_crtc(crtc, plane) {
>   		int plane_width, plane_height;
> -		if (plane->type == DRM_PLANE_TYPE_CURSOR) {
> -			cursor_plane_set_fb(plane,
> -					    &data->cursor_fb[crtc->pipe],
> -					    cursor_width, cursor_height);
> -			plane_width = cursor_width;
> -			plane_height = cursor_height;
> -		} else {
> -			universal_plane_set_fb(plane,
> -					       &data->fb[crtc->pipe * MAX_PLANES + i],
> -					       mode->hdisplay, mode->vdisplay);
>   
> -			plane_width = (mode->hdisplay * 3) / 4;
> -			plane_height = (mode->vdisplay * 3) / 4;
> +		if (plane->type == DRM_PLANE_TYPE_CURSOR)
> +			continue;
>   
> -			ret = try_plane_scaling(data, plane, plane_width, plane_height);
> +		plane_width = (mode->hdisplay * 3) / 4;
> +		plane_height = (mode->vdisplay * 3) / 4;
>   
> -			while (ret) {
> -				if (plane_width <= cursor_width || plane_height <= cursor_height)
> -					break;
> +		/*
> +		 * Set the source crop to match the initial destination
> +		 * size (no upscaling).  Keeping source == destination
> +		 * prevents the downscale ratio from exceeding the
> +		 * hardware limit (~3×) when the destination is halved
> +		 * in the reduction loop below.
> +		 */
> +		universal_plane_set_fb(plane,
> +				       &data->fb[crtc->pipe * MAX_PLANES + i],
> +				       plane_width, plane_height);
>   
> -				plane_width /= 2;
> -				plane_height /= 2;
> +		ret = try_plane_scaling(data, plane, plane_width, plane_height);
>   
> -				ret = try_plane_scaling(data, plane, plane_width, plane_height);
> +		while (ret) {
> +			if (plane_width <= cursor_width || plane_height <= cursor_height)
> +				break;
>   
> -				igt_info("Reduced plane %d size to %dx%d\n",
> -					 plane->index, plane_width, plane_height);
> -			}
> -			if (ret) {
> -				igt_info("Plane %d pipe %d try commit failed, exiting\n", i,
> -					 crtc->pipe);
> -				data->num_planes[crtc->pipe] = i;
> -				igt_info("Max num planes for pipe %d set to %d\n",
> -					 crtc->pipe, i);
> -				/*
> -				 * We have now determined max amount of full sized planes, we will just
> -				 * keep it in mind and be smarter next time. Also lets remove unneeded fbs.
> -				 * Don't destroy cursor_fb as will take care about it at the end.
> -				 */
> +			plane_width /= 2;
> +			plane_height /= 2;
> +
> +			/* Keep source crop in sync with the reduced destination. */
> +			igt_fb_set_size(&data->fb[crtc->pipe * MAX_PLANES + i],
> +					plane, plane_width, plane_height);
> +
> +			ret = try_plane_scaling(data, plane, plane_width, plane_height);
> +
> +			igt_info("Reduced plane %d size to %dx%d\n",
> +				 plane->index, plane_width, plane_height);
> +		}
> +		if (ret) {
> +			igt_info("Plane %d pipe %d try commit failed, exiting\n", i,
> +				 crtc->pipe);
> +			data->num_planes[crtc->pipe] = i;
> +			igt_info("Max num planes for pipe %d set to %d\n",
> +				 crtc->pipe, i);
> +			/*
> +			 * We have now determined max amount of full sized planes, we will just
> +			 * keep it in mind and be smarter next time. Also lets remove unneeded fbs.
> +			 * Don't destroy cursor_fb as will take care about it at the end.
> +			 */
> +			igt_plane_set_fb(plane, NULL);
> +			cleanup_plane_fbs(data, crtc, i, MAX_PLANES);
> +		}
> +
> +		if (++i >= data->num_planes[crtc->pipe])
> +			break;
> +	}
> +
> +	/*
> +	 * Always attempt to enable the cursor plane.  First validate it with
> +	 * a TEST_ONLY commit: some format/modifier combinations (e.g.
> +	 * XRGB8888, 4-tiled) are not accepted by the cursor hardware.
> +	 * If the test commit rejects the cursor, gracefully disable it so
> +	 * the final actual commit with only overlay planes can succeed.
> +	 */
> +	for_each_plane_on_crtc(crtc, plane) {
> +		if (plane->type == DRM_PLANE_TYPE_CURSOR) {
> +			cursor_plane_set_fb(plane,
> +					    &data->cursor_fb[crtc->pipe],
> +					    cursor_width, cursor_height);
> +			if (igt_display_try_commit_atomic(&data->display,
> +							  DRM_MODE_ATOMIC_TEST_ONLY |
> +							  DRM_MODE_ATOMIC_ALLOW_MODESET, NULL)) {
>   				igt_plane_set_fb(plane, NULL);
> -				cleanup_plane_fbs(data,
> -						  crtc,
> -						  i,
> -						  MAX_PLANES);
> +				igt_info("Cursor fb not supported by hw (format/modifier), disabling\n");
>   			}
> -
> -			if (++i >= data->num_planes[crtc->pipe])
> -				break;
> +			break;
>   		}
>   	}
>   


  reply	other threads:[~2026-06-29 19:29 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-02 16:24 [PATCH i-g-t v2 0/3] tests/intel/kms_pipe_stress: fix plane setup and cursor handling Sowmiya S
2026-06-02 16:24 ` [PATCH i-g-t v2 1/3] tests/intel/kms_pipe_stress: fix inverted condition skipping plane setup Sowmiya S
2026-06-02 16:24 ` [PATCH i-g-t v2 2/3] tests/intel/kms_pipe_stress: fix plane source crop and separate cursor loop Sowmiya S
2026-06-29 19:28   ` Juha-Pekka Heikkilä [this message]
2026-06-02 16:24 ` [PATCH i-g-t v2 3/3] tests/intel/kms_pipe_stress: use ARGB8888/LINEAR for cursor framebuffer Sowmiya S
2026-06-04  7:22   ` Borah, Chaitanya Kumar
2026-06-03  6:10 ` ✓ Xe.CI.BAT: success for tests/intel/kms_pipe_stress: fix plane setup and cursor handling (rev2) Patchwork
2026-06-03  6:23 ` ✓ i915.CI.BAT: " Patchwork
2026-06-03 17:46 ` ✗ Xe.CI.FULL: failure " Patchwork
2026-06-04  2:29 ` ✗ i915.CI.Full: " Patchwork
2026-06-09 15:51 ` ✓ i915.CI.Full: success " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ae1eb7d5-2563-42fd-8026-6856395ba364@gmail.com \
    --to=juhapekka.heikkila@gmail.com \
    --cc=chaitanya.kumar.borah@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=sowmiya.s@intel.com \
    --cc=swati2.sharma@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox