[PATCH i-g-t] Revert tests/kms_histogram: Fix build breakage

Kamil Konieczny kamil.konieczny at linux.intel.com
Wed Jan 8 22:55:32 UTC 2025


Hi Lucas,
On 2025-01-08 at 12:52:51 -0800, Lucas De Marchi wrote:
> This reverts commits
> 8911eff2c6c0 ("tests/kms_histogram: Add check for libghe version")
> 6e93c2e807b3 ("tests/kms_histogram: Added IGT support to validate global histogram")
> 
> Ths addition of libghe version check didn't really work as the released
> libghe exposes a 1.0.0 version:
> 
> 	Run-time dependency libghe found: YES 1.0

This looks like improper package generation in our CI,
we could go with revert or make more meson checks like in version 2
("tests/kms_histogram: Add check for ghe api call")
https://patchwork.freedesktop.org/series/143142/#rev2

in meson.build:

    cc = meson.get_compiler('c')

    if cc.has_function('histogram_compute_generate_data_bin', dependencies: libghe)
       config.set('HAVE_LIBGHE', 1)
    else
       message('libghe do not have the required function')
    endif

Btw proper function name is present now in https://github.com/intel/ghe

so I will resend v2 of above change with subject changed:

meson: Add check for libghe histogram function

Regards,
Kamil

> 
> Let's revert this and come back to that when there's a released libghe
> that works and doesn't keep changing the API.
> 
> Cc: Kamil Konieczny <kamil.konieczny at intel.com>
> Cc: Mohammed Thasleem <mohammed.thasleem at intel.com>
> Cc: Arun R Murthy <arun.r.murthy at intel.com>
> Cc: Nemesa Garg <nemesa.garg at intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>  lib/igt_kms.c         |  23 ---
>  lib/igt_kms.h         |   5 -
>  meson.build           |   5 -
>  tests/kms_histogram.c | 349 ------------------------------------------
>  tests/meson.build     |   2 -
>  5 files changed, 384 deletions(-)
>  delete mode 100644 tests/kms_histogram.c
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 95e3059f7..9d21cce1e 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -714,9 +714,6 @@ const char * const igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
>  	[IGT_CRTC_OUT_FENCE_PTR] = "OUT_FENCE_PTR",
>  	[IGT_CRTC_VRR_ENABLED] = "VRR_ENABLED",
>  	[IGT_CRTC_SCALING_FILTER] = "SCALING_FILTER",
> -	[IGT_CRTC_HISTOGRAM] = "HISTOGRAM_ENABLE",
> -	[IGT_CRTC_GLOBAL_HISTOGRAM] = "HISTOGRAM_DATA",
> -	[IGT_CRTC_GLOBAL_HIST_PIXEL_FACTOR] = "HISTOGRAM_IET",
>  };
>  
>  const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> @@ -2618,9 +2615,6 @@ static void igt_pipe_reset(igt_pipe_t *pipe)
>  	if (igt_pipe_obj_has_prop(pipe, IGT_CRTC_VRR_ENABLED))
>  		igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_VRR_ENABLED, 0);
>  
> -	if (igt_pipe_obj_has_prop(pipe, IGT_CRTC_HISTOGRAM))
> -		igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_HISTOGRAM, 0);
> -
>  	pipe->out_fence_fd = -1;
>  }
>  
> @@ -5649,23 +5643,6 @@ bool igt_lease_change_detected(struct udev_monitor *mon, int timeout_secs)
>  			      ARRAY_SIZE(props));
>  }
>  
> -/**
> - * igt_global_histogram_event_detected:
> - * @mon: A udev monitor initialized with #igt_watch_uevents
> - * @timeout_secs: How long to wait for a lease change event to occur.
> - *
> - * Detect if a global Histogram event was received since we last checked the monitor.
> - *
> - * Returns: true if a sysfs global Histogram event was received, false if we timed out
> - */
> -bool igt_global_histogram_event_detected(struct udev_monitor *mon, int timeout_secs)
> -{
> -	const char *props[1] = {"HISTOGRAM"};
> -	int expected_val = 1;
> -
> -	return event_detected(mon, timeout_secs, props, &expected_val, ARRAY_SIZE(props));
> -}
> -
>  /**
>   * igt_flush_uevents:
>   * @mon: A udev monitor initialized with #igt_watch_uevents
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 1e2a927ab..8810123fb 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -165,9 +165,6 @@ enum igt_atomic_crtc_properties {
>         IGT_CRTC_OUT_FENCE_PTR,
>         IGT_CRTC_VRR_ENABLED,
>         IGT_CRTC_SCALING_FILTER,
> -       IGT_CRTC_HISTOGRAM,
> -       IGT_CRTC_GLOBAL_HISTOGRAM,
> -       IGT_CRTC_GLOBAL_HIST_PIXEL_FACTOR,
>         IGT_NUM_CRTC_PROPS
>  };
>  
> @@ -1143,8 +1140,6 @@ void igt_cleanup_uevents(struct udev_monitor *mon);
>  bool igt_display_has_format_mod(igt_display_t *display, uint32_t format, uint64_t modifier);
>  bool igt_plane_has_format_mod(igt_plane_t *plane, uint32_t format, uint64_t modifier);
>  
> -bool igt_global_histogram_event_detected(struct udev_monitor *mon, int timeout_secs);
> -
>  /**
>   * igt_vblank_after_eq:
>   * @a: First vblank sequence number.
> diff --git a/meson.build b/meson.build
> index 5eda2d582..ed2a79d5a 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -197,11 +197,6 @@ else
>  	chamelium = disabler()
>  endif
>  
> -libghe = dependency('libghe', version : '>=0.2.0', required : false)
> -if libghe.found()
> -      config.set('HAVE_LIBGHE', 1)
> -endif
> -
>  build_info += 'Build Chamelium test: @0@'.format(chamelium.found())
>  
>  pthreads = dependency('threads')
> diff --git a/tests/kms_histogram.c b/tests/kms_histogram.c
> deleted file mode 100644
> index 3d59304ba..000000000
> --- a/tests/kms_histogram.c
> +++ /dev/null
> @@ -1,349 +0,0 @@
> -// SPDX-License-Identifier: MIT
> -/*
> - * Copyright © 2024 Intel Corporation
> - */
> -
> -/**
> - * TEST: kms histogram
> - * Category: Display
> - * Description: Test to verify histogram features.
> - * Functionality: histogram
> - * Mega feature: Display
> - * Test category: functionality test
> - */
> -
> -#include <errno.h>
> -#include <fcntl.h>
> -#include <limits.h>
> -#include <stdbool.h>
> -#include <stdio.h>
> -#include <string.h>
> -
> -#include "igt.h"
> -#include "igt_vec.h"
> -#ifdef HAVE_LIBGHE
> -#include "ghe.h"
> -#endif
> -
> -#define GLOBAL_HIST_DISABLE		0
> -#define GLOBAL_HIST_ENABLE		1
> -#define GLOBAL_HIST_DELAY		2
> -#define FLIP_COUNT			20
> -
> -/**
> - * SUBTEST: global-basic
> - * Description: Test to enable histogram, flip monochrome fbs, wait for
> - *		histogram event and then read the histogram data
> - *
> - * SUBTEST: global-color
> - * Description: Test to enable histogram, flip color fbs, wait for
> - *		histogram event and then read the histogram data
> - *
> - * SUBTEST: algo-basic
> - * Description: Test to enable histogram, flip monochrome fbs, wait for
> - *		histogram event and then read the histogram data and enhance pixels by
> - *		multiplying by a pixel factor using algo
> - *
> - * SUBTEST: algo-color
> - * Description: Test to enable histogram, flip color fbs, wait for histogram event
> - *		and then read the histogram data and enhance pixels by multiplying
> - *		by a pixel factor using algo
> - */
> -
> -IGT_TEST_DESCRIPTION("This test will verify the display histogram.");
> -
> -typedef struct data {
> -	igt_display_t display;
> -	int drm_fd;
> -	igt_fb_t fb[5];
> -} data_t;
> -
> -typedef void (*test_t)(data_t*, enum pipe, igt_output_t*, drmModePropertyBlobRes*);
> -
> -static void enable_and_verify_global_histogram(data_t *data, enum pipe pipe)
> -{
> -	uint32_t global_hist_value;
> -
> -	/* Enable global_hist */
> -	igt_pipe_set_prop_value(&data->display, pipe, IGT_CRTC_HISTOGRAM, GLOBAL_HIST_ENABLE);
> -	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> -
> -	/* Verify if global_hist is enabled */
> -	global_hist_value = igt_pipe_obj_get_prop(&data->display.pipes[pipe], IGT_CRTC_HISTOGRAM);
> -	igt_assert_f(global_hist_value == GLOBAL_HIST_ENABLE, "Failed to enable global_hist\n");
> -}
> -
> -static void disable_and_verify_global_histogram(data_t *data, enum pipe pipe)
> -{
> -	uint32_t global_hist_value;
> -
> -	/* Disable global_hist */
> -	igt_pipe_set_prop_value(&data->display, pipe, IGT_CRTC_HISTOGRAM, GLOBAL_HIST_DISABLE);
> -	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> -
> -	/* Verify if global_hist is disabled */
> -	global_hist_value = igt_pipe_obj_get_prop(&data->display.pipes[pipe], IGT_CRTC_HISTOGRAM);
> -	igt_assert_f(global_hist_value == GLOBAL_HIST_DISABLE, "Failed to disable global_hist\n");
> -}
> -
> -static void cleanup_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
> -{
> -	igt_plane_t *plane;
> -
> -	disable_and_verify_global_histogram(data, pipe);
> -
> -	for_each_plane_on_pipe(&data->display, pipe, plane)
> -		igt_plane_set_fb(plane, NULL);
> -	igt_output_set_pipe(output, PIPE_NONE);
> -	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> -	igt_remove_fb(data->display.drm_fd, &data->fb[0]);
> -	igt_remove_fb(data->display.drm_fd, &data->fb[1]);
> -	igt_remove_fb(data->display.drm_fd, &data->fb[2]);
> -	igt_remove_fb(data->display.drm_fd, &data->fb[3]);
> -	igt_remove_fb(data->display.drm_fd, &data->fb[4]);
> -}
> -
> -static drmModePropertyBlobRes *get_global_histogram_data(data_t *data, enum pipe pipe)
> -{
> -	uint64_t blob_id;
> -
> -	blob_id = igt_pipe_obj_get_prop(&data->display.pipes[pipe],
> -					IGT_CRTC_GLOBAL_HISTOGRAM);
> -	if (blob_id == 0)
> -		return NULL;
> -
> -	return drmModeGetPropertyBlob(data->drm_fd, blob_id);
> -}
> -
> -static void read_global_histogram(data_t *data, enum pipe pipe,
> -				  drmModePropertyBlobRes **hist_blob_ptr)
> -{
> -	uint32_t *histogram_ptr;
> -	drmModePropertyBlobRes *global_hist_blob = NULL;
> -
> -	igt_set_timeout(GLOBAL_HIST_DELAY, "Waiting to read global histogram blob.\n");
> -	do {
> -		global_hist_blob = get_global_histogram_data(data, pipe);
> -	} while (!global_hist_blob);
> -
> -	igt_reset_timeout();
> -
> -	*hist_blob_ptr = global_hist_blob;
> -	histogram_ptr = (uint32_t *)global_hist_blob->data;
> -	for (int i = 0; i < global_hist_blob->length / sizeof(*histogram_ptr); i++)
> -		igt_debug("Histogram[%d] = %d\n", i, *(histogram_ptr++));
> -}
> -
> -#ifdef HAVE_LIBGHE
> -static void set_pixel_factor(igt_pipe_t *pipe, uint32_t *dietfactor, size_t size)
> -{
> -	uint32_t i;
> -
> -	for (i = 0; i < size; i++) {
> -		/* Displaying IET LUT */
> -		igt_debug("Pixel Factor[%d] = %d\n", i, *(dietfactor + i));
> -	}
> -
> -	igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_GLOBAL_HIST_PIXEL_FACTOR,
> -				       dietfactor, size);
> -}
> -
> -static struct globalhist_args *algo_get_pixel_factor(drmModePropertyBlobRes *global_hist_blob,
> -						     igt_output_t *output)
> -{
> -	struct globalhist_args *argsPtr =
> -		(struct globalhist_args *)malloc(sizeof(struct globalhist_args));
> -
> -	drmModeModeInfo *mode;
> -
> -	mode = igt_output_get_mode(output);
> -
> -	memcpy(argsPtr->histogram, global_hist_blob->data, global_hist_blob->length);
> -	argsPtr->resolution_x = mode->hdisplay;
> -	argsPtr->resolution_y = mode->vdisplay;
> -
> -	igt_debug("Making call to global histogram algorithm.\n");
> -	histogram_compute_generate_data_bin(argsPtr);
> -
> -	return argsPtr;
> -}
> -
> -static void algo_image_enhancement_factor(data_t *data, enum pipe pipe,
> -					  igt_output_t *output,
> -					  drmModePropertyBlobRes *global_hist_blob)
> -{
> -	struct globalhist_args *args = algo_get_pixel_factor(global_hist_blob, output);
> -
> -	igt_assert(args);
> -	igt_debug("Writing pixel factor blob.\n");
> -
> -	set_pixel_factor(&data->display.pipes[pipe], args->dietfactor,
> -			 ARRAY_SIZE(args->dietfactor));
> -	free(args);
> -
> -	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> -}
> -#endif
> -
> -static void create_monochrome_fbs(data_t *data, drmModeModeInfo *mode)
> -{
> -	/* TODO: Extend the tests for different formats/modifiers. */
> -	/* These frame buffers used to flip monochrome fbs to get histogram event. */
> -	igt_assert(igt_create_color_pattern_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> -					       DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> -					       0, 0, 0, &data->fb[0]));
> -
> -	igt_assert(igt_create_color_pattern_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> -					       DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> -					       1, 1, 1, &data->fb[1]));
> -}
> -
> -static void create_color_fbs(data_t *data, drmModeModeInfo *mode)
> -{
> -	/* TODO: Extend the tests for different formats/modifiers. */
> -	/* These frame buffers used to flip color fbs to get histogram event. */
> -	igt_assert(igt_create_color_pattern_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> -					       DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> -					       0.5, 0, 0.5, &data->fb[0]));
> -
> -	igt_assert(igt_create_color_pattern_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> -					       DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> -					       1, 0, 0, &data->fb[1]));
> -
> -	igt_assert(igt_create_color_pattern_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> -					       DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> -					       0, 1, 0, &data->fb[2]));
> -
> -	igt_assert(igt_create_color_pattern_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> -					       DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> -					       0, 0, 1, &data->fb[3]));
> -
> -	igt_assert(igt_create_color_pattern_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> -					       DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> -					       1, 0, 1, &data->fb[4]));
> -}
> -
> -static void flip_fb(data_t *data, enum pipe pipe, igt_output_t *output, struct igt_fb *fb)
> -{
> -	igt_plane_set_fb(igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY), fb);
> -	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> -}
> -
> -static void prepare_pipe(data_t *data, enum pipe pipe, igt_output_t *output, bool color_fb)
> -{
> -	int i;
> -	struct udev_monitor *mon = igt_watch_uevents();
> -	drmModeModeInfo *mode = igt_output_get_mode(output);
> -	bool event_detected = false;
> -	int fb_count = color_fb ? 5 : 2;
> -
> -	if (color_fb)
> -		create_color_fbs(data, mode);
> -	else
> -		create_monochrome_fbs(data, mode);
> -
> -	flip_fb(data, pipe, output, &data->fb[0]);
> -	enable_and_verify_global_histogram(data, pipe);
> -
> -	igt_flush_uevents(mon);
> -	for (i = 1; i <= FLIP_COUNT; i++) {
> -		flip_fb(data, pipe, output, &data->fb[i % fb_count]);
> -
> -		/* Check for histogram event on every flip and break the loop if detected. */
> -		if (igt_global_histogram_event_detected(mon, 0)) {
> -			event_detected = true;
> -			break;
> -		}
> -	}
> -
> -	igt_cleanup_uevents(mon);
> -	igt_assert_f(event_detected, "Histogram event not generated.\n");
> -}
> -
> -static void run_global_histogram_pipeline(data_t *data, enum pipe pipe, igt_output_t *output,
> -					  bool color_fb, test_t test_pixel_factor)
> -{
> -	drmModePropertyBlobRes *global_hist_blob = NULL;
> -
> -	prepare_pipe(data, pipe, output, color_fb);
> -
> -	read_global_histogram(data, pipe, &global_hist_blob);
> -
> -	if (test_pixel_factor)
> -		test_pixel_factor(data, pipe, output, global_hist_blob);
> -
> -	drmModeFreePropertyBlob(global_hist_blob);
> -	cleanup_pipe(data, pipe, output);
> -}
> -
> -static void run_tests_for_global_histogram(data_t *data, bool color_fb,
> -					   test_t test_pixel_factor)
> -{
> -	enum pipe pipe;
> -	igt_output_t *output;
> -
> -	for_each_connected_output(&data->display, output) {
> -		for_each_pipe(&data->display, pipe) {
> -			if (!igt_pipe_obj_has_prop(&data->display.pipes[pipe], IGT_CRTC_HISTOGRAM))
> -				continue;
> -
> -			igt_display_reset(&data->display);
> -
> -			igt_output_set_pipe(output, pipe);
> -			if (!intel_pipe_output_combo_valid(&data->display))
> -				continue;
> -
> -			igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output))
> -				run_global_histogram_pipeline(data, pipe, output, color_fb, test_pixel_factor);
> -		}
> -	}
> -}
> -
> -static void run_algo_test(data_t *data, bool color_fb)
> -{
> -#ifdef HAVE_LIBGHE
> -	run_tests_for_global_histogram(data, color_fb, algo_image_enhancement_factor);
> -#else
> -	igt_skip("Histogram algorithm library not found.\n");
> -#endif
> -}
> -
> -igt_main
> -{
> -	data_t data = {};
> -
> -	igt_fixture {
> -		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
> -		kmstest_set_vt_graphics_mode();
> -		igt_display_require(&data.display, data.drm_fd);
> -		igt_display_require_output(&data.display);
> -		igt_require(data.display.is_atomic);
> -	}
> -
> -	igt_describe("Test to enable histogram, flip monochrome fbs, wait for histogram "
> -		     "event and then read the histogram data.");
> -	igt_subtest_with_dynamic("global-basic")
> -		run_tests_for_global_histogram(&data, false, NULL);
> -
> -	igt_describe("Test to enable histogram, flip color fbs, wait for histogram event "
> -		     "and then read the histogram data.");
> -	igt_subtest_with_dynamic("global-color")
> -		run_tests_for_global_histogram(&data, true, NULL);
> -
> -	igt_describe("Test to enable histogram, flip monochrome fbs, wait for histogram "
> -		     "event and then read the histogram data and enhance pixels by multiplying "
> -		     "by a pixel factor using algo.");
> -	igt_subtest_with_dynamic("algo-basic")
> -		run_algo_test(&data, false);
> -
> -	igt_describe("Test to enable histogram, flip color fbs, wait for histogram event "
> -		     "and then read the histogram data and enhance pixels by multiplying "
> -		     "by a pixel factor using algo.");
> -	igt_subtest_with_dynamic("algo-color")
> -		run_algo_test(&data, true);
> -
> -	igt_fixture {
> -		igt_display_fini(&data.display);
> -		drm_close_driver(data.drm_fd);
> -	}
> -}
> diff --git a/tests/meson.build b/tests/meson.build
> index 89bba6454..2724c7a9a 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -37,7 +37,6 @@ test_progs = [
>  	'kms_getfb',
>  	'kms_hdmi_inject',
>  	'kms_hdr',
> -	'kms_histogram',
>  	'kms_invalid_mode',
>  	'kms_lease',
>  	'kms_multipipe_modeset',
> @@ -381,7 +380,6 @@ extra_dependencies = {
>  	'gem_eio': [ realtime ],
>  	'gem_exec_balancer': [ lib_igt_perf ],
>  	'gem_mmap_offset': [ libatomic ],
> -	'kms_histogram': [ libghe ],
>  	'i915_pm_freq_mult': [ lib_igt_perf ],
>  	'i915_pm_rc6_residency': [ lib_igt_perf ],
>  	'perf': [ lib_igt_i915_perf ],
> -- 
> 2.47.0
> 


More information about the igt-dev mailing list