Re: [igt-dev] [PATCH i-g-t 15/24] tests/kms_dither: New IGT to validate crtc Dithering

["Shankar, Uma" <uma.shankar@intel.com>]

> -----Original Message-----
> From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of
> venkata.sai.patnana@intel.com
> Sent: Thursday, June 3, 2021 5:50 PM
> To: igt-dev@lists.freedesktop.org
> Subject: [igt-dev] [PATCH i-g-t 15/24] tests/kms_dither: New IGT to validate crtc
> Dithering
> 
> From: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> 
> Many of the display devices have color format support less than the color format of
> the input data (Ex: 8-bit input data and 6-bit display color depth). Then the input data
> will be either truncated or rounded, but this approach usually causes loss of detail
> and also produce large banded areas of a single color that significantly differs from
> the original image.
> 
> Dithering is a technique used to enhance these colors by creating the illusion of
> smoothness by adjusting the nearby pixel color.
> For Eg: Converting 8-bit to 6-bit by modifying the pixel information.
> 
> Dithering should be enabled when a panel color depth is lower than the color depth
> of the framebuffer.
> 
> Even though feature Dithering is there since legacy, there is no specific IGT for this.
> This patch will create New IGT for Dithering.
> And below is the truth table for CRTC Dithering.
> 
> |----------------|---------|-------------|
> |  Frame buffer  |  Panel  |  Dithering  |
> |----------------|---------|-------------|
> |      8 BPC     |  6 BPC  |     Yes     |
> |----------------|---------|-------------|
> |      8 BPC     |  8 BPC  |     No      |
> |----------------|---------|-------------|
> |      8 BPC     | 10 BPC  |     No      |
> |----------------|---------|-------------|
> |     10 BPC     |  6 BPC  |     Yes     |
> |----------------|---------|-------------|
> |     10 BPC     |  8 BPC  |     Yes     |
> |----------------|---------|-------------|
> |     10 BPC     | 10 BPC  |     No      |
> |----------------|---------|-------------|
> |     16 BPC     |  6 BPC  |     Yes     |
> |----------------|---------|-------------|
> |     16 BPC     |  8 BPC  |     Yes     |
> |----------------|---------|-------------|
> |     16 BPC     | 10 BPC  |     Yes     |
> |----------------|---------|-------------|

This test depends on the corresponding kernel driver changes.
https://patchwork.freedesktop.org/series/91383/ : Dithering support 
https://patchwork.freedesktop.org/series/79664/: Debugfs changes
The above are in review. 

I would suggest we consolidate both in 1 common series.

Also the dither IGT tests depends on a debugfs in driver. It also needs to be
merged and available in driver, before this test can go in.

Regards,
Uma Shankar

> v2:
> * Fix commit message (Karthik)
> * Add file author (Karthik)
> * Dynamic subtests for pipe-output combinations (Karthik)
> * Update deprecated pipe name (Karthik)
> * Remove redundant fb creation logic (Karthik)
> 
> v3:
> * Use for_each_pipe_with_valid_output() to simplify the logic (Bhanu)
> * Remove magic numbers to create fb (Karthik)
> * Fix typos & comments (Swati)
> 
> v4:
> * Remove explicit igt_require for a valid output found,
>   igt_subtest_with_dynamic will handle the SKIP (Petri)
> * Limit the execution to a single pipe (Uma)
> 
> v5:
> * SKIP subtest for HDMI with 6 BPC (Bhanu)
> * Fix the usage of valid format to create 8 BPC Framebuffer (Bhanu)
> * Add support for suspend/resume tests (Bhanu)
> * Add few debug prints (Bhanu)
> 
> v6:
> * Limit the execution to 8 BPC panels (Uma)
> * Remove suspend/resume tests (Bhanu)
> 
> v7:
> * Drop max_bpc from debugfs (Uma)
> 
> v8:
> * Run test on all connected outputs (Bhanu)
> * Before exiting the test reset the "max bpc" prop (Bhanu)
> * Before computing the result check that the crtc bpc is updated
>   with the requested one (Bhanu)
> 
> Cc: Swati Sharma <swati2.sharma@intel.com>
> Cc: Karthik B S <karthik.b.s@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Cc: Nischal Varide <nischal.varide@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com> (cherry
> picked from commit 24d58fef5070ec7557d8bf778b12becb93dd3315)
> ---
>  tests/kms_dither.c | 250 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/meson.build  |   1 +
>  2 files changed, 251 insertions(+)
>  create mode 100644 tests/kms_dither.c
> 
> diff --git a/tests/kms_dither.c b/tests/kms_dither.c new file mode 100644 index
> 0000000000..c25d623f81
> --- /dev/null
> +++ b/tests/kms_dither.c
> @@ -0,0 +1,250 @@
> +/*
> + * Copyright © 2020 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person
> +obtaining a
> + * copy of this software and associated documentation files (the
> +"Software"),
> + * to deal in the Software without restriction, including without
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> +next
> + * paragraph) shall be included in all copies or substantial portions
> +of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
> +SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM,
> +DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> +OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
> +OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * Authors:
> + *     Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> + *
> + */
> +
> +#include "igt.h"
> +#include <fcntl.h>
> +#include <termios.h>
> +#include <unistd.h>
> +
> +IGT_TEST_DESCRIPTION("Test Dithering block status");
> +
> +/* Connector BPC */
> +#define IGT_CONNECTOR_BPC_6		6
> +#define IGT_CONNECTOR_BPC_8		8
> +#define IGT_CONNECTOR_BPC_10		10
> +#define IGT_CONNECTOR_BPC_12		12
> +
> +/* Framebuffer BPC */
> +#define IGT_FRAME_BUFFER_BPC_8		8
> +#define IGT_FRAME_BUFFER_BPC_10		10
> +#define IGT_FRAME_BUFFER_BPC_16		16
> +
> +/* Common test data. */
> +typedef struct data {
> +	igt_display_t display;
> +	igt_plane_t *primary;
> +	igt_output_t *output;
> +	igt_pipe_t *pipe;
> +	drmModeModeInfo *mode;
> +	enum pipe pipe_id;
> +	int drm_fd;
> +	igt_fb_t fb;
> +} data_t;
> +
> +typedef struct {
> +	unsigned int bpc;
> +	unsigned int dither;
> +} dither_status_t;
> +
> +/* Prepare test data. */
> +static void prepare_test(data_t *data, igt_output_t *output, enum pipe
> +pipe) {
> +	igt_display_t *display = &data->display;
> +
> +	data->pipe_id = pipe;
> +	data->pipe = &data->display.pipes[data->pipe_id];
> +	igt_assert(data->pipe);
> +
> +	igt_display_reset(display);
> +
> +	data->output = output;
> +	igt_assert(data->output);
> +
> +	data->mode = igt_output_get_mode(data->output);
> +	igt_assert(data->mode);
> +
> +	data->primary =
> +		igt_pipe_get_plane_type(data->pipe,
> DRM_PLANE_TYPE_PRIMARY);
> +
> +	igt_output_set_pipe(data->output, data->pipe_id); }
> +
> +/* Returns the current state of dithering from the crtc debugfs. */
> +static dither_status_t get_dither_state(data_t *data) {
> +	char buf[256];
> +	char crtc_name[7];
> +	char *start_loc;
> +	int fd, res;
> +	dither_status_t status;
> +
> +	snprintf(crtc_name, 7, "crtc-%d", data->pipe_id);
> +	fd = igt_debugfs_open(data->drm_fd, crtc_name, O_RDONLY);
> +	igt_assert(fd >= 0);
> +
> +	res = igt_debugfs_simple_read(fd, "dither", buf, sizeof(buf));
> +	igt_require(res > 0);
> +	close(fd);
> +
> +	igt_assert(start_loc = strstr(buf, "bpc: "));
> +	igt_assert_eq(sscanf(start_loc, "bpc: %u", &status.bpc), 1);
> +
> +	igt_assert(start_loc = strstr(buf, "Dither: "));
> +	igt_assert_eq(sscanf(start_loc, "Dither: %u", &status.dither), 1);
> +
> +	return status;
> +}
> +
> +static void test_dithering(data_t *data, enum pipe pipe,
> +			   igt_output_t *output,
> +			   int fb_bpc, int fb_format,
> +			   int output_bpc)
> +{
> +	igt_display_t *display = &data->display;
> +	dither_status_t status;
> +	int bpc;
> +
> +	igt_info("Dithering test execution on %s PIPE_%s\n",
> +			output->name, kmstest_pipe_name(pipe));
> +	prepare_test(data, output, pipe);
> +
> +	igt_assert(igt_create_fb(data->drm_fd, data->mode->hdisplay,
> +				 data->mode->vdisplay, fb_format,
> +				 LOCAL_DRM_FORMAT_MOD_NONE, &data->fb));
> +	igt_plane_set_fb(data->primary, &data->fb);
> +	igt_plane_set_size(data->primary, data->mode->hdisplay,
> +data->mode->vdisplay);
> +
> +	bpc = igt_output_get_prop(output, IGT_CONNECTOR_MAX_BPC);
> +	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC,
> output_bpc);
> +	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC :
> +COMMIT_LEGACY);
> +
> +	/*
> +	 * Check the status of Dithering block:
> +	 *
> +	 * Preserve the result & compute later (after clean-up).
> +	 * If fb_bpc is greater than output_bpc, Dithering should be enabled
> +	 * Else disabled
> +	 */
> +	status = get_dither_state(data);
> +
> +	igt_debug("FB BPC:%d, Panel BPC:%d, Pipe BPC:%d, Expected Dither:%s,
> Actual result:%s\n",
> +		  fb_bpc, output_bpc, status.bpc,
> +		  (fb_bpc > output_bpc) ? "Enable": "Disable",
> +		  status.dither ? "Enable": "Disable");
> +
> +       /*
> +	* We must update the Connector max_bpc property back
> +	* Otherwise, previously updated value will stay forever and
> +	* may cause the failures for next/other subtests.
> +	*/
> +	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC,
> bpc);
> +	igt_plane_set_fb(data->primary, NULL);
> +	igt_output_set_pipe(output, PIPE_NONE);
> +	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC :
> COMMIT_LEGACY);
> +	igt_remove_fb(data->drm_fd, &data->fb);
> +
> +	/* Check if crtc bpc is updated with requested one. */
> +	igt_require_f((status.bpc == output_bpc),
> +			"%s can support max %u-bpc, but requested %d-bpc\n",
> +				output->name, status.bpc, output_bpc);
> +
> +	/* Compute the result. */
> +	if (fb_bpc > output_bpc)
> +		igt_assert_f(status.dither, "(fb_%dbpc > output_%dbpc): Dither
> should be enabled\n",
> +				fb_bpc, output_bpc);
> +	else
> +		igt_assert_f(!status.dither, "(fb_%dbpc <= output_%dbpc): Dither
> should be disabled\n",
> +				fb_bpc, output_bpc);
> +
> +	return;
> +}
> +
> +/* Returns true if an output supports max bpc property & max_bpc >=
> +requested. */ static bool is_supported(igt_output_t *output, int bpc) {
> +        return igt_output_has_prop(output, IGT_CONNECTOR_MAX_BPC) &&
> +		(igt_output_get_prop(output, IGT_CONNECTOR_MAX_BPC) >= bpc);
> }
> +
> +static void
> +run_dither_test(data_t *data, int fb_bpc, int fb_format, int
> +output_bpc) {
> +	igt_output_t *output;
> +	igt_display_t *display = &data->display;
> +
> +	for_each_connected_output(display, output) {
> +		enum pipe pipe;
> +
> +		if (!is_supported(output, output_bpc))
> +			continue;
> +
> +		if ((strstr(output->name, "HDMI") != NULL) &&
> +		    (output_bpc == IGT_CONNECTOR_BPC_6))
> +			continue;
> +
> +		for_each_pipe(display, pipe) {
> +			if (igt_pipe_connector_valid(pipe, output)) {
> +				igt_dynamic_f("%s-pipe-%s", output->name,
> kmstest_pipe_name(pipe))
> +					test_dithering(data, pipe, output, fb_bpc,
> +							fb_format, output_bpc);
> +
> +				/* One pipe is enough */
> +				break;
> +			}
> +		}
> +	}
> +}
> +
> +igt_main
> +{
> +	struct {
> +		int fb_bpc;
> +		int format;
> +		int output_bpc;
> +	} tests[] = {
> +		{ IGT_FRAME_BUFFER_BPC_8, DRM_FORMAT_XRGB8888,
> IGT_CONNECTOR_BPC_6 },
> +		{ IGT_FRAME_BUFFER_BPC_8, DRM_FORMAT_XRGB8888,
> IGT_CONNECTOR_BPC_8 },
> +	};
> +	int i;
> +	data_t data = { 0 };
> +
> +	igt_fixture {
> +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> +		kmstest_set_vt_graphics_mode();
> +
> +		igt_display_require(&data.display, data.drm_fd);
> +		igt_display_require_output(&data.display);
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(tests); i++) {
> +		igt_describe_f("Framebuffer BPC:%d, Panel BPC:%d, Expected
> Dither:%s\n",
> +			       tests[i].fb_bpc, tests[i].output_bpc,
> +			       (tests[i].fb_bpc > tests[i].output_bpc) ?
> +							"Enable": "Disable");
> +
> +		igt_subtest_with_dynamic_f("FB-%dBPC-Vs-Panel-%dBPC",
> +				tests[i].fb_bpc, tests[i].output_bpc)
> +			run_dither_test(&data,
> +					tests[i].fb_bpc,
> +					tests[i].format,
> +					tests[i].output_bpc);
> +	}
> +
> +	igt_fixture {
> +		igt_display_fini(&data.display);
> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build index 19cc4ebe0d..ff3979207d
> 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -28,6 +28,7 @@ test_progs = [
>  	'kms_cursor_crc',
>  	'kms_cursor_edge_walk',
>  	'kms_cursor_legacy',
> +	'kms_dither',
>  	'kms_dp_aux_dev',
>  	'kms_dp_dsc',
>  	'kms_dp_tiled_display',
> --
> 2.25.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev