[PATCH i-g-t v7 1/1] tests/intel/kms_dirty_fbc: Add kms_dirty_fbc test for DIRTYFB ioctl with fbc
Govindapillai, Vinod
vinod.govindapillai at intel.com
Mon Feb 3 12:12:43 UTC 2025
Hi Santhosh,
Thanks for the update! But some part of the tests are not accurate now!
Please have a look at the comments inline.
On Mon, 2025-02-03 at 08:16 +0530, Santhosh Reddy Guddati wrote:
> - Introduce new test file kms_fbc_dirty_rect.c to verify
> DIRTYFB ioctl functionality with FBC enabled.
> - Added subtests for basic FBC dirty rectangle, different formats,
> and dirtyfb ioctl.
> - Update meson.build to include the new test.
>
> v2: Fix typo , add version check for feature support,
> extend support for all pipes (Rama Naidu).
>
> v3: Add new subtest to scatter dirty rectangles at differnt places in a
> frame and commit all rectangles at once (Rama Naidu).
> Add a negative case with invalid coordinates (Vinod)
>
> v4: Add subtest `fbc-dirty-rectangle-basic` to perform sanity checks
> by sending multiple damaged areas with non-PSR modes.(Vinod)
> Update `meson.build` to include the new test.
>
> v5: Include checks to ensure FBC is enabled during tests.(Vinod)
> Add dynamic subtests for fbc-dirty-rectangle-different-formats
> and fbc-dirty-rectangle-dirtyfb-ioctl. (Vinod)
>
> v6: Update meson.build to include kms_fbc_dirty_rect.
>
> v7: Add commit description, remove i915 check and remove
> redundant checks (Swathi)
> Reformat code by removing redundant usage.
> Add new test to verify dirty rect outside-visible-region.
>
> Signed-off-by: Santhosh Reddy Guddati <santhosh.reddy.guddati at intel.com>
> ---
> tests/intel/kms_fbc_dirty_rect.c | 324 +++++++++++++++++++++++++++++++
> tests/meson.build | 1 +
> 2 files changed, 325 insertions(+)
> create mode 100644 tests/intel/kms_fbc_dirty_rect.c
>
> diff --git a/tests/intel/kms_fbc_dirty_rect.c b/tests/intel/kms_fbc_dirty_rect.c
> new file mode 100644
> index 000000000..b72c98ed3
> --- /dev/null
> +++ b/tests/intel/kms_fbc_dirty_rect.c
> @@ -0,0 +1,324 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +/**
> + * TEST: kms dirty fbc
> + * Category: Display
> + * Description: Test DIRTYFB ioctl functionality with FBC enabled.
> + * Driver requirement: i915, xe
> + * Functionality: dirtyfb, fbc
> + * Mega feature: General Display Features
> + * Test category: functionality test
> + */
> +
> +#include "igt.h"
> +#include "i915/intel_fbc.h"
> +
> +/**
> + * SUBTEST: fbc-dirty-rectangle-basic
> + * Description: Sanity test to verify FBC DR by sending multiple damaged areas with non psr modes
> + *
> + * SUBTEST: fbc-dirty-rectangle-different-formats
> + * Description: Sanity test to verify FBC DR by sending multiple
> + * damaged areas with different formats.
> + *
> + * SUBTEST: fbc-dirty-rectangle-outside-visible-region
> + * Description: Test that FBC DR works with damage area outside the visible region
> + */
> +
> +#define SQUARE_SIZE 100
> +#define SQUARE_OFFSET 100
> +#define SQUARE_OFFSET_2 600
> +#define SQUARE_OFFSET_OUT 1000
> +
> +typedef struct {
> + int drm_fd;
> + int debugfs_fd;
> + igt_display_t display;
> + drmModeModeInfo *mode;
> + igt_output_t *output;
> + igt_pipe_crc_t *pipe_crc;
> + enum pipe pipe;
> + u32 format;
> +
> + igt_crc_t ref_crc;
> +
> + enum {
> + FEATURE_NONE = 0,
> + FEATURE_PSR = 1,
> + FEATURE_FBC = 2,
> + FEATURE_DRRS = 4,
> + FEATURE_COUNT = 8,
> + FEATURE_DEFAULT = 8,
> + } feature;
> +} data_t;
> +
> +
> +static void set_damage_clip(struct drm_mode_rect *damage, int x1, int y1, int x2, int y2)
> +{
> + damage->x1 = x1;
> + damage->y1 = y1;
> + damage->x2 = x2;
> + damage->y2 = y2;
> +}
> +
> +static void set_and_check_crc(data_t *data, igt_plane_t *primary,
> + igt_pipe_crc_t *pipe_crc, struct igt_fb *fb,
> + struct drm_mode_rect *rect, int rect_size,
> + igt_crc_t *expected_crc)
May be the name of this function should be set_fb_and_collect_crc()?
> +{
> + igt_plane_replace_prop_blob(primary, IGT_PLANE_FB_DAMAGE_CLIPS, rect, rect_size);
Also I think it is better to get this damage clip as a separate function. something like
set_damage_area(plane, rects) and call it from out of this function.
> + igt_plane_set_fb(primary, fb);
> + igt_display_commit2(&data->display, COMMIT_ATOMIC);
> + igt_assert_f(intel_fbc_is_enabled(data->drm_fd, data->pipe,
> + IGT_LOG_WARN), "FBC is not enabled\n");
> + igt_pipe_crc_collect_crc(pipe_crc, expected_crc);
> +}
> +
> +static void dirty_rect_create_fb(data_t *data, struct igt_fb *fb, int nrects,
> + struct drm_mode_rect *rect)
> +{
> + cairo_t *cr;
> +
> + igt_info("Creating framebuffer with %d damaged areas format %s\n",
> + nrects, igt_format_str(data->format));
> + igt_create_color_fb(data->drm_fd, data->mode->hdisplay, data->mode->vdisplay,
> + data->format, DRM_FORMAT_MOD_LINEAR,
> + 0.0, 0.0, 1.0, fb);
Wonder if you can pass color as a parameter and create the fb with color passed and not always blue.
This would be handy in the case where we check the damage area outside the visible rect wont cause
any crc mismatches. more details as part of that test.
Also we need to a create a big FB. So need to pass FB width and height as parameter.
> +
> + if (!nrects || !rect)
> + return;
> +
> + cr = igt_get_cairo_ctx(data->drm_fd, fb);
> +
> + for (int i = 0; i < nrects; i++) {
> + igt_paint_color_alpha(cr, rect[i].x1, rect[i].y1,
> + rect[i].x2 - rect[i].x1,
> + rect[i].y2 - rect[i].y1,
> + 1.0, 1.0, 1.0, 1.0);
> + }
> + igt_put_cairo_ctx(cr);
> +}
> +
Some explanation of what this test does should be good.
First a series of screens are drawn as full screen updates and crc's are collected. These crc's act
as reference crcs. Then screens are updated using the FBC dirty rect feature and compared with these
reference crcs. If the crcs match, the test is successful.
1. Full blue screen - main_fb, main_fb_crc
2. A white square on upper left on blue screen as full screen update - rect1_fb,
rect_fb_rect1_fb_crc
3. A second white square at bit lower of the first rect on blue screen as a full screen update -
rect2_fb, rect2_fb_crc
4. Both the rects are drawn on the blue screen as full screen update - rect_combined_fb,
rect_combined_fb_crc
These above are all full screen updates to collect reference crcs. Now we attempt updating the
screen with FBC dirty rect.
1. Full blue screen
2. Set rect_combine_fb with damage area update - damage area are provided as rects. After this the
crc should match with crc collected before rect_combined_fb-crc.
3. Now clear the area of the first rect as a dirty rect update. Use the main_fb and damage area as
the rect1 coordinates. After this the crc should match rect2_fb_crc
4. Now clear the area of the second rect as a dirty rect update. Use main_fb and damage area as the
rect2 coordinates. After this crc should match main_fb_crc
This is just a basic explanation of what this does. So some better explanation would be useful .
> +static void fbc_dirty_rectangle_complete_set(data_t *data)
> +{
> + igt_pipe_crc_t *pipe_crc = data->pipe_crc;
> + igt_display_t *display = &data->display;
> + igt_output_t *output = data->output;
> + igt_plane_t *primary;
> + drmModeModeInfo *mode;
> + struct igt_fb main_fb, rect_1_fb, rect_2_fb, rect_combined_fb;
> + struct drm_mode_rect rect1, rect2, rect_combined[2], full_rect;
> + igt_crc_t main_fb_crc, rect_1_fb_crc, rect_2_fb_crc, rect_combined_fb_crc, crc;
> +
> + mode = igt_output_get_mode(output);
> + igt_output_set_pipe(output, data->pipe);
> + if (!pipe_crc) {
> + pipe_crc = igt_pipe_crc_new(data->drm_fd, data->pipe, IGT_PIPE_CRC_SOURCE_AUTO);
> + igt_assert(pipe_crc);
> + }
> + primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +
> + set_damage_clip(&full_rect, 0, 0, mode->hdisplay - 1, mode->vdisplay - 1);
> + set_damage_clip(&rect1, SQUARE_OFFSET, SQUARE_OFFSET, SQUARE_OFFSET + SQUARE_SIZE,
> SQUARE_OFFSET + SQUARE_SIZE);
> + set_damage_clip(&rect2, SQUARE_OFFSET_2, SQUARE_OFFSET_2, SQUARE_OFFSET_2 + SQUARE_SIZE,
> SQUARE_OFFSET_2 + SQUARE_SIZE);
> + rect_combined[0] = rect1;
> + rect_combined[1] = rect2;
> +
> + dirty_rect_create_fb(data, &main_fb, 0, NULL);
> + dirty_rect_create_fb(data, &rect_1_fb, 1, &rect1);
> + dirty_rect_create_fb(data, &rect_2_fb, 1, &rect2);
> + dirty_rect_create_fb(data, &rect_combined_fb, 2, rect_combined);
> +
> + /* main_fb blank blue screen - get and store crc */
> + set_and_check_crc(data, primary, pipe_crc, &main_fb, &full_rect,
> + sizeof(full_rect), &main_fb_crc);
set_damage_area(primary, full_rect)
set_fb_collect_crc(...)
> +
> + /* Whole blue screen with one white rect and collect crc */
> + set_and_check_crc(data, primary, pipe_crc, &rect_1_fb, &full_rect,
> + sizeof(full_rect), &rect_1_fb_crc);
> +
> + /* Second white rect and collect crc */
> + set_and_check_crc(data, primary, pipe_crc, &rect_2_fb, &full_rect,
> + sizeof(full_rect), &rect_2_fb_crc);
> +
> + /* Both rects and collect crc */
> + set_and_check_crc(data, primary, pipe_crc, &rect_combined_fb, &full_rect,
> + sizeof(full_rect), &rect_combined_fb_crc);
> +
> + /* Put full blank screen back */
> + set_and_check_crc(data, primary, pipe_crc, &main_fb, &full_rect,
> + sizeof(full_rect), &crc);
> + igt_assert_crc_equal(&crc, &main_fb_crc);
> +
> + /* Set combined rect - draw two white rects using damage area */
> + set_and_check_crc(data, primary, pipe_crc, &rect_combined_fb, rect_combined,
> + sizeof(rect_combined), &crc);
> + igt_assert_crc_equal(&crc, &rect_combined_fb_crc);
set_damage_area(primary, combined_rect)
set_fb_and_collect_crc(.. rect_combine_fb)
> +
> + /* Clear first rect. Only the second rect should be visible here! */
> + igt_plane_set_fb(primary, &rect_2_fb);
> + igt_display_commit2(display, COMMIT_ATOMIC);
This is not correct. This basically becomes a full screen update for rect2_fb and make this damage
area check useless.
> + set_and_check_crc(data, primary, pipe_crc, &rect_2_fb, &rect1,
> + sizeof(rect1), &crc);
> + igt_assert_crc_equal(&crc, &rect_2_fb_crc);
Better to clear using the main fb.
This should be something like this...
set_damage_area(primary, rect1)
set_fb_collect_crc(... main_fb, crc)
igt_assert_crc_equal(&crc, &rect_2_fb_crc);
> +
> + /* Clear the second rect as well. Only the first rect should be visible here */
Not correct. The first rect is already gone as part of the previous case. No rects after this.
> + igt_plane_set_fb(primary, &rect_1_fb);
> + igt_display_commit2(display, COMMIT_ATOMIC);
same as before.. not correct. This basically becomes a full screen update for rect_1_fb and make
this damage area check useless.
> + set_and_check_crc(data, primary, pipe_crc, &rect_1_fb, &rect2,
> + sizeof(rect2), &crc);
> + igt_assert_crc_equal(&crc, &rect_1_fb_crc);
both rects are gone. same comments are before!
set_damage_area(primary, rect2)
set_fb_collect_crc(... main_fb, crc)
igt_assert_crc_equal(&crc, &main_fb_crc);
> +
> + igt_plane_set_fb(primary, NULL);
> + igt_remove_fb(data->drm_fd, &main_fb);
> + igt_remove_fb(data->drm_fd, &rect_1_fb);
> + igt_remove_fb(data->drm_fd, &rect_2_fb);
> + igt_remove_fb(data->drm_fd, &rect_combined_fb);
> + igt_display_commit2(display, COMMIT_ATOMIC);
> +}
> +
> +static void prepare_test(data_t *data, igt_output_t *output)
> +{
> + bool is_fbc_supported = false;
> +
> + is_fbc_supported = intel_fbc_supported_on_chipset(data->drm_fd, data->pipe);
> +
> + igt_require_f(is_fbc_supported, "FBC not supported on this platform\n");
> +
> + if (data->feature & FEATURE_FBC)
> + intel_fbc_enable(data->drm_fd);
> +
> +}
> +
Some explanation would be useful. Here the test should create bigger FBs and makes a damage area
outside the visible rect. Then switch FBs with damage area and there should not be any diff. Because
FBC dirty update handle line wise, we need to ensure that any damage are update shouldnt trigger any
screen update. So it is better to have the second FB in a different color. If the FBC dirty rect
programming not working properly then it will update the visible part as well!
create_fb(bigger_fb, green, fb1)
create_fb(bigger_fb, blue, fb2)
create a damage area rect as "rect1" outside visible area but withing the FB dimensions
set_damage_area(primary, full_screen)
set_fb_collect_crc(fb1, fb1_crc)
Green screen now on screen.
set_damage_area(primary, rect1)
set_fb_collect_crc(fb1, fb2_crc)
Now fb1 and fb2 crcs whould match as the damage area was out side the visible area.
> +static void fbc_test_outside_region(data_t *data)
> +{
> + struct igt_fb main_fb, rect_outside_fb;
> + igt_pipe_crc_t *crc = data->pipe_crc;
> + igt_plane_t *primary;
> + igt_crc_t main_fb_crc, rect_outside_fb_crc;
> + struct drm_mode_rect main_rect, rect_outside;
> +
> + prepare_test(data, data->output);
> +
> + igt_output_get_mode(data->output);
> + igt_output_set_pipe(data->output, data->pipe);
> +
> + crc = igt_pipe_crc_new(data->drm_fd, data->pipe, IGT_PIPE_CRC_SOURCE_AUTO);
> + primary = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY);
> +
> + // Set damage area outside the visible region
> + set_damage_clip(&main_rect, 0, 0, data->mode->hdisplay - 1,
> + data->mode->vdisplay - 1);
> + set_damage_clip(&rect_outside, data->mode->hdisplay, data->mode->vdisplay,
> + data->mode->hdisplay + SQUARE_OFFSET_OUT,
> + data->mode->vdisplay + SQUARE_OFFSET_OUT);
> +
> + dirty_rect_create_fb(data, &main_fb, 0, NULL);
> + dirty_rect_create_fb(data, &rect_outside_fb, 1, &rect_outside);
> +
> + set_and_check_crc(data, primary, crc, &main_fb, &main_rect,
> + sizeof(main_rect), &main_fb_crc);
> + set_and_check_crc(data, primary, crc, &rect_outside_fb, &rect_outside,
> + sizeof(rect_outside), &rect_outside_fb_crc);
> +
> + // Verify that the CRC remains unchanged
> + igt_assert_crc_equal(&main_fb_crc, &rect_outside_fb_crc);
> +
> + igt_plane_set_fb(primary, NULL);
> + igt_remove_fb(data->drm_fd, &main_fb);
> + igt_remove_fb(data->drm_fd, &rect_outside_fb);
> + igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +}
> +
> +static void fbc_dirty_rectangle_basic(data_t *data)
> +{
> + prepare_test(data, NULL);
> + return fbc_dirty_rectangle_complete_set(data);
Probalby fbc_dirty_rectangle_complete_setfbc_dirty_rectangle_complete_set.. doesent make much sense.
I guess you can use that as basic
BR
vinod
> +}
> +
> +igt_main
> +{
> + data_t data = {0};
> + int display_ver;
> +
> + igt_fixture {
> + data.drm_fd = drm_open_driver_master(DRIVER_INTEL | DRIVER_XE);
> + data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> + data.feature = FEATURE_FBC;
> + kmstest_set_vt_graphics_mode();
> + igt_require_pipe_crc(data.drm_fd);
> + igt_display_require(&data.display, data.drm_fd);
> + igt_display_require_output(&data.display);
> + display_ver = intel_display_ver(intel_get_drm_devid(data.drm_fd));
> + igt_require_f(display_ver >= 30, "FBC with dirty region is not supported\n");
> + }
> +
> + igt_subtest_with_dynamic("fbc-dirty-rectangle-basic") {
> + data.format = DRM_FORMAT_XRGB8888;
> + for_each_pipe(&data.display, data.pipe) {
> + for_each_valid_output_on_pipe(&data.display, data.pipe, data.output) {
> + data.mode = igt_output_get_mode(data.output);
> + igt_display_reset(&data.display);
> + igt_output_set_pipe(data.output, data.pipe);
> + igt_dynamic_f("%s-%s",
> + kmstest_pipe_name(data.pipe),
> + igt_output_name(data.output)) {
> + fbc_dirty_rectangle_basic(&data);
> + }
> + }
> + }
> + }
> +
> + igt_subtest_with_dynamic("fbc-dirty-rectangle-different-formats") {
> + uint32_t formats[] = {DRM_FORMAT_XRGB8888,
> + DRM_FORMAT_ARGB8888,
> + DRM_FORMAT_RGB565};
> + int num_formats = ARRAY_SIZE(formats);
> +
> + for_each_pipe(&data.display, data.pipe) {
> + for_each_valid_output_on_pipe(&data.display, data.pipe, data.output) {
> + data.mode = igt_output_get_mode(data.output);
> + igt_display_reset(&data.display);
> + igt_output_set_pipe(data.output, data.pipe);
> +
> + for (int i = 0; i < num_formats; i++) {
> + igt_dynamic_f("%s-%s-format-%s",
> + kmstest_pipe_name(data.pipe),
> + igt_output_name(data.output),
> + igt_format_str(formats[i])) {
> + data.format = formats[i];
> + fbc_dirty_rectangle_basic(&data);
> + }
> + }
> + }
> + }
> + }
> +
> + igt_subtest_with_dynamic("fbc-dirty-rectangle-outside-visible-region") {
> + data.feature = FEATURE_FBC;
> + data.format = DRM_FORMAT_XRGB8888;
> + igt_require_f(display_ver >= 30, "FBC with dirty region is not supported\n");
> +
> + for_each_pipe(&data.display, data.pipe) {
> + for_each_valid_output_on_pipe(&data.display, data.pipe, data.output) {
> + data.mode = igt_output_get_mode(data.output);
> + igt_display_reset(&data.display);
> + igt_output_set_pipe(data.output, data.pipe);
> +
> + igt_dynamic_f("%s-%s-outside-visible-region",
> + kmstest_pipe_name(data.pipe),
> + igt_output_name(data.output)) {
> + fbc_test_outside_region(&data);
> + }
> + }
> + }
> + }
> +
> + igt_fixture {
> + igt_display_fini(&data.display);
> + close(data.drm_fd);
> + }
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 33dffad31..fde1078e3 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -251,6 +251,7 @@ intel_kms_progs = [
> 'kms_dsc',
> 'kms_fb_coherency',
> 'kms_fbcon_fbt',
> + 'kms_fbc_dirty_rect',
> 'kms_fence_pin_leak',
> 'kms_flip_scaled_crc',
> 'kms_flip_tiling',
More information about the igt-dev
mailing list