[igt-dev] [PATCH i-g-t] tests/i915/i915_pm_backlight: Add new subtest to validate dual panel backlight
Hogander, Jouni
jouni.hogander at intel.com
Mon Oct 3 11:01:34 UTC 2022
On Thu, 2022-09-29 at 14:58 +0530, Nidhi Gupta wrote:
> Since driver can now support multiple eDPs and Debugfs structure for
> backlight changed per connector the test should then iterate through
> all eDP connectors.
>
> Signed-off-by: Nidhi Gupta <nidhi1.gupta at intel.com>
> ---
> tests/i915/i915_pm_backlight.c | 110 ++++++++++++++++++++++++-------
> --
> 1 file changed, 80 insertions(+), 30 deletions(-)
>
> diff --git a/tests/i915/i915_pm_backlight.c
> b/tests/i915/i915_pm_backlight.c
> index cafae7f7..bf67b50c 100644
> --- a/tests/i915/i915_pm_backlight.c
> +++ b/tests/i915/i915_pm_backlight.c
> @@ -34,9 +34,12 @@
> #include <errno.h>
> #include <unistd.h>
> #include <time.h>
> +#include "igt_device.h"
> +#include "igt_device_scan.h"
>
> struct context {
> int max;
> + const char *path;
> };
>
>
> @@ -48,14 +51,14 @@ struct context {
>
> IGT_TEST_DESCRIPTION("Basic backlight sysfs test");
>
> -static int backlight_read(int *result, const char *fname)
> +static int backlight_read(int *result, const char *fname, struct
> context *context)
> {
> int fd;
> char full[PATH_MAX];
> char dst[64];
> int r, e;
>
> - igt_assert(snprintf(full, PATH_MAX, "%s/%s", BACKLIGHT_PATH,
> fname) < PATH_MAX);
> + igt_assert(snprintf(full, PATH_MAX, "%s/%s/%s",
> BACKLIGHT_PATH, context->path, fname) < PATH_MAX);
>
> fd = open(full, O_RDONLY);
> if (fd == -1)
> @@ -73,14 +76,14 @@ static int backlight_read(int *result, const char
> *fname)
> return errno;
> }
>
> -static int backlight_write(int value, const char *fname)
> +static int backlight_write(int value, const char *fname, struct
> context *context)
> {
> int fd;
> char full[PATH_MAX];
> char src[64];
> int len;
>
> - igt_assert(snprintf(full, PATH_MAX, "%s/%s", BACKLIGHT_PATH,
> fname) < PATH_MAX);
> + igt_assert(snprintf(full, PATH_MAX, "%s/%s/%s",
> BACKLIGHT_PATH, context->path, fname) < PATH_MAX);
> fd = open(full, O_WRONLY);
> if (fd == -1)
> return -errno;
> @@ -100,12 +103,12 @@ static void test_and_verify(struct context
> *context, int val)
> const int tolerance = val * TOLERANCE / 100;
> int result;
>
> - igt_assert_eq(backlight_write(val, "brightness"), 0);
> - igt_assert_eq(backlight_read(&result, "brightness"), 0);
> + igt_assert_eq(backlight_write(val, "brightness", context),
> 0);
> + igt_assert_eq(backlight_read(&result, "brightness", context),
> 0);
> /* Check that the exact value sticks */
> igt_assert_eq(result, val);
>
> - igt_assert_eq(backlight_read(&result, "actual_brightness"),
> 0);
> + igt_assert_eq(backlight_read(&result, "actual_brightness",
> context), 0);
> /* Some rounding may happen depending on hw */
> igt_assert_f(result >= max(0, val - tolerance) &&
> result <= min(context->max, val + tolerance),
> @@ -124,16 +127,16 @@ static void test_bad_brightness(struct context
> *context)
> {
> int val;
> /* First write some sane value */
> - backlight_write(context->max / 2, "brightness");
> + backlight_write(context->max / 2, "brightness", context);
> /* Writing invalid values should fail and not change the
> value */
> - igt_assert_lt(backlight_write(-1, "brightness"), 0);
> - backlight_read(&val, "brightness");
> + igt_assert_lt(backlight_write(-1, "brightness", context), 0);
> + backlight_read(&val, "brightness", context);
> igt_assert_eq(val, context->max / 2);
> - igt_assert_lt(backlight_write(context->max + 1,
> "brightness"), 0);
> - backlight_read(&val, "brightness");
> + igt_assert_lt(backlight_write(context->max + 1, "brightness",
> context), 0);
> + backlight_read(&val, "brightness", context);
> igt_assert_eq(val, context->max / 2);
> - igt_assert_lt(backlight_write(INT_MAX, "brightness"), 0);
> - backlight_read(&val, "brightness");
> + igt_assert_lt(backlight_write(INT_MAX, "brightness",
> context), 0);
> + backlight_read(&val, "brightness", context);
> igt_assert_eq(val, context->max / 2);
> }
>
> @@ -186,6 +189,8 @@ igt_main
> igt_display_t display;
> igt_output_t *output;
> struct igt_fb fb;
> + const char *paths[2][50];
> + char path1[PATH_MAX];
>
> igt_fixture {
> enum pipe pipe;
> @@ -202,10 +207,6 @@ igt_main
> kmstest_set_vt_graphics_mode();
> igt_display_require(&display,
> drm_open_driver(DRIVER_INTEL));
>
> - /* Get the max value and skip the whole test if sysfs
> interface not available */
> - igt_skip_on(backlight_read(&old, "brightness"));
> - igt_assert(backlight_read(&context.max,
> "max_brightness") > -1);
> -
> /* should be ../../cardX-$output */
> igt_assert_lt(12, readlink(BACKLIGHT_PATH "/device",
> full_name, sizeof(full_name) - 1));
> name = basename(full_name);
> @@ -233,22 +234,71 @@ igt_main
>
> igt_display_commit2(&display, display.is_atomic ?
> COMMIT_ATOMIC : COMMIT_LEGACY);
> igt_pm_enable_sata_link_power_management();
> +
> + memcpy(paths[0], "intel_backlight", 8);
> + for (int i = 0; (i = display.n_outputs); i++) {
> + output = &display.outputs[i];
> + igt_output_set_pipe(output, PIPE_ANY);
What igt_output_set_pipe(output, PIPE_ANY) is doing?
> + snprintf(path1, 100 , "card%i-%s-backlight",
> igt_device_get_card_index(display.drm_fd), igt_output_name(output));
I'm not sure if card* can be something else than card0? Definetely it
has nothing to do with n_outputs.
> + int fd = open(path1, O_WRONLY);
You can't expect cardx-xxx-backlight to succeed. There has to be some
path (e.g. /sys/dev/char/226\:0/cardx-xxx-backlight). Maybe
igt_sysfs_path could be used here?
> + if (fd == -1)
> + continue;
close(fd);
> + memcpy(paths[1], name, 30);
You have paths[0] = intel_backlight and paths[1] = card0-eDP1? You
wanted to copy path1?
> + break;
> + }
This loop generally looks strange. You are checking path1 copying
variable "name" into paths[1] ?
>
> + /* Get the max value and skip the whole test if sysfs
> interface not available */
> + for (size_t i = 0; i < sizeof(paths) /
> sizeof(paths[0]); i++) {
> + memcpy(&context.path, paths[i], 8);
is 8 enough? I think you should use pointer here instead of copying it
> + igt_skip_on(backlight_read(&old,
> "brightness", &context));
> + igt_assert(backlight_read(&context.max,
> "max_brightness", &context) > -1);
> + }
> + }
> +
> + igt_subtest_with_dynamic("basic-brightness") {
> + for (size_t j = 0; j < sizeof(paths) /
> sizeof(paths[0]); j++) {
> + igt_dynamic_f("connector-%zu", j) {
> + memcpy(&context.path, paths[j], 8);
> + test_brightness(&context);
> + }
> + }
> + }
> + igt_subtest_with_dynamic("bad-brightness") {
> + for (size_t j = 0; j < sizeof(paths) /
> sizeof(paths[0]); j++) {
> + igt_dynamic_f("connector-%zu", j) {
> + memcpy(&context.path, paths[j], 8);
> + test_bad_brightness(&context);
> + }
> + }
> + }
> +
> + igt_subtest_with_dynamic("fade") {
> + for (size_t j = 0; j < sizeof(paths) /
> sizeof(paths[0]); j++) {
> + igt_dynamic_f("connector-%zu", j) {
> + memcpy(&context.path, paths[j], 8);
> + test_fade(&context);
> + }
> + }
> }
>
> - igt_subtest("basic-brightness")
> - test_brightness(&context);
> - igt_subtest("bad-brightness")
> - test_bad_brightness(&context);
> - igt_subtest("fade")
> - test_fade(&context);
> - igt_subtest("fade_with_dpms")
> - test_fade_with_dpms(&context, output);
> - igt_subtest("fade_with_suspend")
> - test_fade_with_suspend(&context, output);
> + igt_subtest_with_dynamic("fade_with_dpms") {
> + for (size_t j = 0; j < sizeof(paths) /
> sizeof(paths[0]); j++) {
> + igt_dynamic_f("connector-%zu", j) {
> + memcpy(&context.path, paths[j], 8);
> + test_fade_with_dpms(&context,
> output);
> + }
> + }
> + }
> +
> + igt_subtest_with_dynamic("fade_with_suspend") {
> + for (size_t j = 0; j < sizeof(paths) /
> sizeof(paths[0]); j++) {
> + igt_dynamic_f("connector-%zu", j) {
> + memcpy(&context.path, paths[j], 8);
> + test_fade_with_suspend(&context,
> output);
> + }
> + }
> + }
>
> igt_fixture {
> - /* Restore old brightness */
> - backlight_write(old, "brightness");
>
> igt_display_fini(&display);
> igt_remove_fb(display.drm_fd, &fb);
More information about the igt-dev
mailing list