[igt-dev] [PATCH 2/2] tests/i915/kms_draw_crc: Test Cleanup
Modem, Bhanuprakash
bhanuprakash.modem at intel.com
Fri Aug 12 10:37:04 UTC 2022
On Wed-10-08-2022 06:57 pm, Nidhi Gupta wrote:
> v2: -Replace drm function call with existing library functions
> (Bhanu)
>
> v3: -Replace PIPE_A with compatible pipe/output combo.
> (Bhanu)
>
> v4: -use for_each_pipe_with_single_output() for finding
> compatible pipe/ouput combo.
> -remove crtc_id as not used anywhere.
> -set fb to primary plane before commiting.
> (Bhanu)
>
> v5: -set fb to primary plane before commiting
> for fill_fb_subtest() also.
> -use existing IGT lib helper igt_display_has_format_mod()
> instead of format_is_supported().
> (Bhanu)
> -added igt_display_fini(..) for cleanup
> (Juha-Pekka Heikkila)
>
> Signed-off-by: Nidhi Gupta <nidhi1.gupta at intel.com>
> ---
> tests/i915/kms_draw_crc.c | 113 ++++++++++++--------------------------
> 1 file changed, 36 insertions(+), 77 deletions(-)
>
> diff --git a/tests/i915/kms_draw_crc.c b/tests/i915/kms_draw_crc.c
> index 48c7d931..f1887470 100644
> --- a/tests/i915/kms_draw_crc.c
> +++ b/tests/i915/kms_draw_crc.c
> @@ -29,15 +29,10 @@
>
> #define MAX_CONNECTORS 32
>
> -struct modeset_params {
> - uint32_t crtc_id;
> - uint32_t connector_id;
> - drmModeModeInfoPtr mode;
> -};
> -
> int drm_fd;
> -drmModeResPtr drm_res;
> -drmModeConnectorPtr drm_connectors[MAX_CONNECTORS];
> +igt_display_t display;
> +igt_output_t *output;
> +drmModeModeInfoPtr mode;
> struct buf_ops *bops;
> igt_pipe_crc_t *pipe_crc;
>
> @@ -60,34 +55,24 @@ struct base_crc {
> };
> struct base_crc base_crcs[ARRAY_SIZE(formats)];
>
> -struct modeset_params ms;
> -
> static void find_modeset_params(void)
> {
> - int i;
> - uint32_t crtc_id;
> - drmModeConnectorPtr connector = NULL;
> - drmModeModeInfoPtr mode = NULL;
> -
> - for (i = 0; i < drm_res->count_connectors; i++) {
> - drmModeConnectorPtr c = drm_connectors[i];
> -
> - if (c->count_modes) {
> - connector = c;
> - mode = &c->modes[0];
> - break;
> - }
> - }
> - igt_require(connector);
> + enum pipe pipe;
>
> - crtc_id = kmstest_find_crtc_for_connector(drm_fd, drm_res, connector,
> - 0);
> - igt_assert(mode);
> + igt_display_reset(&display);
> + igt_display_commit(&display);
>
> - ms.connector_id = connector->connector_id;
> - ms.crtc_id = crtc_id;
> - ms.mode = mode;
> + for_each_pipe_with_single_output(&display, pipe, output) {
s/for_each_pipe_with_single_output/for_each_pipe_with_valid_output/
> + igt_output_set_pipe(output, pipe);
>
> + mode = igt_output_get_mode(output);
> + if (!mode)
> + continue;
I guess, every connected output will have atleast one valid mode, so we
can drop this check.
Otherwise, we must unset output/pipe & continue, and we need a check to
handle if there is no valid mode found with all connected outputs.
By addressing above commets, this patch is
Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
- Bhanu
> +
> + pipe_crc = igt_pipe_crc_new(drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
> + /*Only one pipe/output is enough*/
> + break;
> + }
> }
>
> static uint32_t get_color(uint32_t drm_format, bool r, bool g, bool b)
> @@ -122,9 +107,14 @@ static void get_method_crc(enum igt_draw_method method, uint32_t drm_format,
> {
> struct igt_fb fb;
> int rc;
> + igt_plane_t *primary;
>
> - igt_create_fb(drm_fd, ms.mode->hdisplay, ms.mode->vdisplay,
> + primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +
> + igt_create_fb(drm_fd, mode->hdisplay, mode->vdisplay,
> drm_format, modifier, &fb);
> + igt_plane_set_fb(primary, &fb);
> +
> igt_draw_rect_fb(drm_fd, bops, 0, &fb, method,
> 0, 0, fb.width, fb.height,
> get_color(drm_format, 0, 0, 1));
> @@ -144,8 +134,7 @@ static void get_method_crc(enum igt_draw_method method, uint32_t drm_format,
> igt_draw_rect_fb(drm_fd, bops, 0, &fb, method, 1, 1, 15, 15,
> get_color(drm_format, 0, 1, 1));
>
> - rc = drmModeSetCrtc(drm_fd, ms.crtc_id, fb.fb_id, 0, 0,
> - &ms.connector_id, 1, ms.mode);
> + rc = igt_display_commit2(&display, display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> igt_assert_eq(rc, 0);
>
> igt_pipe_crc_collect_crc(pipe_crc, crc);
> @@ -153,25 +142,6 @@ static void get_method_crc(enum igt_draw_method method, uint32_t drm_format,
> igt_remove_fb(drm_fd, &fb);
> }
>
> -static bool format_is_supported(uint32_t format, uint64_t modifier)
> -{
> - uint32_t gem_handle, fb_id;
> - unsigned int offsets[4] = {};
> - unsigned int strides[4] = {};
> - int ret;
> -
> - gem_handle = igt_create_bo_with_dimensions(drm_fd, 64, 64,
> - format, modifier,
> - 0, NULL, &strides[0], NULL);
> - ret = __kms_addfb(drm_fd, gem_handle, 64, 64,
> - format, modifier, strides, offsets, 1,
> - DRM_MODE_FB_MODIFIERS, &fb_id);
> - drmModeRmFB(drm_fd, fb_id);
> - gem_close(drm_fd, gem_handle);
> -
> - return ret == 0;
> -}
> -
> static void draw_method_subtest(enum igt_draw_method method,
> uint32_t format_index, uint64_t modifier)
> {
> @@ -198,13 +168,12 @@ static void get_fill_crc(uint64_t modifier, igt_crc_t *crc)
> struct igt_fb fb;
> int rc;
>
> - igt_create_fb(drm_fd, ms.mode->hdisplay, ms.mode->vdisplay,
> + igt_create_fb(drm_fd, mode->hdisplay, mode->vdisplay,
> DRM_FORMAT_XRGB8888, modifier, &fb);
>
> igt_draw_fill_fb(drm_fd, &fb, 0xFF);
>
> - rc = drmModeSetCrtc(drm_fd, ms.crtc_id, fb.fb_id, 0, 0,
> - &ms.connector_id, 1, ms.mode);
> + rc = igt_display_commit2(&display, display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> igt_assert_eq(rc, 0);
>
> igt_pipe_crc_collect_crc(pipe_crc, crc);
> @@ -217,18 +186,22 @@ static void fill_fb_subtest(void)
> int rc;
> struct igt_fb fb;
> igt_crc_t base_crc, crc;
> + igt_plane_t *primary;
> bool has_4tile = intel_get_device_info(intel_get_drm_devid(drm_fd))->has_4tile;
>
> - igt_create_fb(drm_fd, ms.mode->hdisplay, ms.mode->vdisplay,
> + primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +
> + igt_create_fb(drm_fd, mode->hdisplay, mode->vdisplay,
> DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, &fb);
>
> + igt_plane_set_fb(primary, &fb);
> +
> igt_draw_rect_fb(drm_fd, bops, 0, &fb,
> gem_has_mappable_ggtt(drm_fd) ? IGT_DRAW_MMAP_GTT :
> IGT_DRAW_MMAP_WC,
> 0, 0, fb.width, fb.height, 0xFF);
>
> - rc = drmModeSetCrtc(drm_fd, ms.crtc_id, fb.fb_id, 0, 0,
> - &ms.connector_id, 1, ms.mode);
> + rc = igt_display_commit2(&display, display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> igt_assert_eq(rc, 0);
>
> igt_pipe_crc_collect_crc(pipe_crc, &base_crc);
> @@ -251,40 +224,26 @@ static void fill_fb_subtest(void)
>
> static void setup_environment(void)
> {
> - int i;
> -
> drm_fd = drm_open_driver_master(DRIVER_INTEL);
> igt_require(drm_fd >= 0);
> -
> - drm_res = drmModeGetResources(drm_fd);
> - igt_require(drm_res);
> - igt_assert(drm_res->count_connectors <= MAX_CONNECTORS);
> -
> - for (i = 0; i < drm_res->count_connectors; i++)
> - drm_connectors[i] = drmModeGetConnectorCurrent(drm_fd,
> - drm_res->connectors[i]);
> + igt_display_require(&display, drm_fd);
> + igt_display_require_output(&display);
>
> kmstest_set_vt_graphics_mode();
>
> bops = buf_ops_create(drm_fd);
>
> find_modeset_params();
> - pipe_crc = igt_pipe_crc_new(drm_fd, kmstest_get_crtc_idx(drm_res, ms.crtc_id),
> - INTEL_PIPE_CRC_SOURCE_AUTO);
> }
>
> static void teardown_environment(void)
> {
> - int i;
> + igt_display_fini(&display);
>
> igt_pipe_crc_free(pipe_crc);
>
> buf_ops_destroy(bops);
>
> - for (i = 0; i < drm_res->count_connectors; i++)
> - drmModeFreeConnector(drm_connectors[i]);
> -
> - drmModeFreeResources(drm_res);
> close(drm_fd);
> }
>
> @@ -346,7 +305,7 @@ igt_main
> !gem_has_mappable_ggtt(drm_fd))
> continue;
>
> - if (!format_is_supported(formats[format_idx], modifier))
> + if (!igt_display_has_format_mod(&display, formats[format_idx], modifier))
> continue;
>
> igt_dynamic_f("%s-%s-%s",
More information about the igt-dev
mailing list