[igt-dev] [RFC, i-g-t] tests/i915/i915_pm_backlight: Add new subtest to validate dual panel backlight

[Hogander, Jouni]

On Wed, 2022-09-07 at 18:45 +0530, Nidhi Gupta wrote:
> Added a new subtest as a part of i915_pm_backlight to validate
> dual panel support.
> 
> Signed-off-by: Nidhi Gupta <nidhi1.gupta at intel.com>
> ---
>  tests/i915/i915_pm_backlight.c | 74
> +++++++++++++++++++++++++++++++++-
>  1 file changed, 73 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/i915/i915_pm_backlight.c
> b/tests/i915/i915_pm_backlight.c
> index cafae7f7..d4b25f3f 100644
> --- a/tests/i915/i915_pm_backlight.c
> +++ b/tests/i915/i915_pm_backlight.c
> @@ -42,6 +42,7 @@ struct context {
>  
>  #define TOLERANCE 5 /* percent */
>  #define BACKLIGHT_PATH "/sys/class/backlight/intel_backlight"
> +#define BACKLIGHT_PATH_DUAL "/sys/class/backlight/card0-eDP-2-
> backlight"
>  
>  #define FADESTEPS 10
>  #define FADESPEED 100 /* milliseconds between steps */
> @@ -94,7 +95,52 @@ static int backlight_write(int value, const char
> *fname)
>  
>         return 0;
>  }
> +static int backlight_read_dual(int *result, const char *fname)
> +{
> +        int fd;
> +        char full[PATH_MAX];
> +        char dst[64];
> +        int r, e;
> +
> +        igt_assert(snprintf(full, PATH_MAX, "%s/%s",
> BACKLIGHT_PATH_DUAL, fname) < PATH_MAX);
> +
> +        fd = open(full, O_RDONLY);
> +        if (fd == -1)
> +                return -errno;
> +
> +        r = read(fd, dst, sizeof(dst));
> +        e = errno;
> +        close(fd);
> +
> +        if (r < 0)
> +                return -e;
> +
> +        errno = 0;
> +        *result = strtol(dst, NULL, 10);
> +        return errno;
> +}
> +
> +static int backlight_write_dual(int value, const char *fname)
> +{
> +        int fd;
> +        char full[PATH_MAX];
> +        char src[64];
> +        int len;
> +
> +        igt_assert(snprintf(full, PATH_MAX, "%s/%s",
> BACKLIGHT_PATH_DUAL, fname) < PATH_MAX);/sys/class/backlight/card0-
> eDP-2-backlight
> +        fd = open(full, O_WRONLY);
> +        if (fd == -1)
> +                return -errno;
>  
> +        len = snprintf(src, sizeof(src), "%i", value);
> +        len = write(fd, src, len);
> +        close(fd);
> +
> +        if (len < 0)
> +                return len;
> +
> +        return 0;
> +}

Instead of duplicating these functions maybe you could have path as a
parameter i.e. int backlight_write/read_dual(int value, const char
*path, const char *fname)

>  static void test_and_verify(struct context *context, int val)
>  {
>         const int tolerance = val * TOLERANCE / 100;
> @@ -112,7 +158,6 @@ static void test_and_verify(struct context
> *context, int val)
>                      "actual_brightness [%d] did not match expected
> brightness [%d +- %d]\n",
>                      result, val, tolerance);
>  }
> -
>  static void test_brightness(struct context *context)
>  {
>         test_and_verify(context, 0);
> @@ -120,6 +165,31 @@ static void test_brightness(struct context
> *context)
>         test_and_verify(context, context->max / 2);
>  }
>  
> +static void test_and_verify_dual(struct context *context, int val)
> +{
> +        const int tolerance = val * TOLERANCE / 100;
> +        int result;
> +
> +        igt_assert_eq(backlight_write_dual(val, "brightness"), 0);
> +        igt_assert_eq(backlight_read_dual(&result, "brightness"),
> 0);
> +        /* Check that the exact value sticks */
> +        igt_assert_eq(result, val);
> +
> +        igt_assert_eq(backlight_read_dual(&result,
> "actual_brightness"), 0);
> +        /* Some rounding may happen depending on hw */
> +        igt_assert_f(result >= max(0, val - tolerance) &&
> +                     result <= min(context->max, val + tolerance),
> +                     "actual_brightness [%d] did not match expected
> brightness [%d +- %d]\n",
> +                     result, val, tolerance);
> +}

You could consider adding path (/sys/class/backlight/intel_backlight
and /sys/class/backlight/card0-eDP-2-backlight) into context. That way
you can drop this function completely and just slightly modify existing
test_and_verify.

> +
> +static void test_brightness_dual(struct context *context)
> +{
> +        test_and_verify_dual(context, 0);
> +        test_and_verify_dual(context, context->max);
> +        test_and_verify_dual(context, context->max / 2);
> +}
> +
>  static void test_bad_brightness(struct context *context)
>  {
>         int val;
> @@ -237,6 +307,8 @@ igt_main
>  
>         igt_subtest("basic-brightness")
>                 test_brightness(&context);
> +       igt_subtest("basic-brightness-dual")
> +               test_brightness_dual(&context);
>         igt_subtest("bad-brightness")
>                 test_bad_brightness(&context);
>         igt_subtest("fade")

You could consider adding like:

static const char *paths[] = {
	"/sys/class/backlight/intel_backlight",
	"/sys/class/backlight/card0-eDP-2-backlight"
};

Then iterate that array and add subtests:

 context.path = path;
 igt_subtest_f("basic-brightness%s", edp_name(path))
	test_brightness(&context);

where edp is returning either "-edp1" or "-edp2" or alternatively
<empty> for edp1 and "-dual" for edp2 if you prefer that one.

BR,

Jouni Högander