Re: [PATCH i-g-t v5 4/8] tests/kms_properties: give non-primary planes their own fb

[Harry Wentland <harry.wentland@amd.com>]

On 2026-09-02 13:58, Melissa Wen wrote:
> On AMD drivers, a CRTC remains active only while its primary plane is
> enabled; therefore, handing the primary's fb over to a non-primary plane
> would take the CRTC down with it. Create a dedicated fb for each
> non-primary plane before testing its colorops and discard it again
> afterward, leaving the primary plane and its prepare_crtc() fb intact.
> This is groundwork for the next commit, which checks colorop properties
> on an active color pipeline.
> 

This patch confuses me a bit. You mention that amdgpu needs an
FB on primary planes for a crtc to be active, which makes sense.
But this patch then only deals with non-primary planes. Does
amdgpu have a need for (non-primary) planes to have an attached
FB before allowing a COLOR_PIPELINE?

Harry

> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
> 
> v2:
> - detach different changes from a single commit (Chaitanya)
> v3:
> - move hunk from next patch to fix mem leak (Alex H/Chaitanya)
> ---
>   tests/kms_properties.c | 20 +++++++++++++++++++-
>   1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/kms_properties.c b/tests/kms_properties.c
> index 2b4cb152b..c55a271da 100644
> --- a/tests/kms_properties.c
> +++ b/tests/kms_properties.c
> @@ -237,7 +237,7 @@ static void run_colorop_property_tests(igt_display_t *display,
>   				       igt_crtc_t *crtc, igt_output_t *output,
>   				       bool atomic)
>   {
> -	struct igt_fb fb;
> +	struct igt_fb fb, afb;
>   	igt_plane_t *plane;
>   	igt_colorop_t *colorop;
>   	int i;
> @@ -255,6 +255,18 @@ static void run_colorop_property_tests(igt_display_t *display,
>   			 igt_crtc_name(crtc), plane->index,
>   			 kmstest_plane_type_name(plane->type), output->name);
>   
> +		/* A non-primary plane needs an fb of its own: AMD keeps the
> +		 * CRTC active only while the primary plane is enabled.
> +		 */
> +		if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
> +			drmModeModeInfo *mode = igt_output_get_mode(output);
> +
> +			igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
> +					      DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, &afb);
> +
> +			igt_plane_set_fb(plane, &afb);
> +		}
> +
>   		/* iterate over all color pipelines on plane */
>   		for (i = 0; i < plane->num_color_pipelines; ++i) {
>   			/* iterate over all colorops in pipeline*/
> @@ -272,6 +284,12 @@ static void run_colorop_property_tests(igt_display_t *display,
>   				colorop = igt_find_colorop(display, colorop_id);
>   			}
>   		}
> +
> +		/* only the fb created above needs to go away here */
> +		if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
> +			igt_plane_set_fb(plane, NULL);
> +			igt_remove_fb(display->drm_fd, &afb);
> +		}
>   	}
>   
>   	cleanup_crtc(display, crtc, output,