[igt-dev] [PATCH i-g-t 1/2] tests/kms_dp_dsc: Create subtest if connector is connected.

[igt-dev] [PATCH i-g-t 1/2] tests/kms_dp_dsc: Create subtest if connector is connected.

[Anusha]

From: Anusha Srivatsa <anusha.srivatsa@intel.com>

Instead of creating a subtest and skipping, check the
conditions and only if they satisfy, go ahead and create
the subtest.

Check if the connector is connected, supports DSC and
if DP then it supports FEC as well. Only if these three
conditions are satisfied then create then try to force
DSC on the connector.

Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 tests/kms_dp_dsc.c | 54 ++++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c
index e2e3aaa0..b99037af 100644
--- a/tests/kms_dp_dsc.c
+++ b/tests/kms_dp_dsc.c
@@ -271,35 +271,33 @@ igt_main
 	}
 
 	for (test_cnt = 0; test_cnt < ARRAY_SIZE(tests); test_cnt++) {
-
-		igt_subtest_f("basic-dsc-enable-%s",
-			      kmstest_connector_type_str(tests[test_cnt])) {
-			test_conn_cnt = 0;
-			for (i = 0; i < res->count_connectors; i++) {
-				connector = drmModeGetConnectorCurrent(data.drm_fd,
-								       res->connectors[i]);
-				if (connector->connection != DRM_MODE_CONNECTED ||
-				    connector->connector_type !=
-				    tests[test_cnt])
-					continue;
-				output = igt_output_from_connector(&data.display, connector);
-				sprintf(data.conn_name, "%s-%d",
-					kmstest_connector_type_str(connector->connector_type),
-					connector->connector_type_id);
-				if(!is_dp_dsc_supported(&data)) {
-					igt_debug("DSC not supported on connector %s \n",
-						  data.conn_name);
-					continue;
-				}
-				if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort &&
-				    !is_dp_fec_supported(&data)) {
-					igt_debug("DSC cannot be enabled without FEC on %s\n",
-						  data.conn_name);
-					continue;
-				}
-				test_conn_cnt++;
-				run_test(&data, output, test_basic_dsc_enable);
+		test_conn_cnt = 0;
+		for (i = 0; i < res->count_connectors; i++) {
+			connector = drmModeGetConnectorCurrent(data.drm_fd,
+							       res->connectors[i]);
+			if (connector->connection != DRM_MODE_CONNECTED ||
+			    connector->connector_type !=
+			    tests[test_cnt])
+				continue;
+			output = igt_output_from_connector(&data.display, connector);
+			sprintf(data.conn_name, "%s-%d",
+				kmstest_connector_type_str(connector->connector_type),
+				connector->connector_type_id);
+			if(!is_dp_dsc_supported(&data)) {
+				igt_debug("DSC not supported on connector %s \n",
+					  data.conn_name);
+				continue;
+			}
+			if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort &&
+			    !is_dp_fec_supported(&data)) {
+				igt_debug("DSC cannot be enabled without FEC on %s\n",
+					  data.conn_name);
+				continue;
 			}
+			test_conn_cnt++;
+			igt_subtest_f("basic-dsc-enable-%s",
+				      kmstest_connector_type_str(tests[test_cnt]))
+			run_test(&data, output, test_basic_dsc_enable);
 			igt_skip_on(test_conn_cnt == 0);
 		}
 	}
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 2/2] tests/kms_dp_dsc: Add a subtest to force DSC output BPP

[Anusha]

From: Anusha Srivatsa <anusha.srivatsa@intel.com>

This subtest uses the accepted DSC BPPs and tries to
force a modeset by setting a certain BPP as the output
BPP for a connector.

Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 tests/kms_dp_dsc.c | 60 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c
index b99037af..5dde1e79 100644
--- a/tests/kms_dp_dsc.c
+++ b/tests/kms_dp_dsc.c
@@ -46,7 +46,8 @@
 
 enum dsc_test_type
 {
-	test_basic_dsc_enable
+	test_basic_dsc_enable,
+	test_basic_dsc_enable_bpp
 };
 
 typedef struct {
@@ -67,6 +68,7 @@ typedef struct {
 
 bool force_dsc_en_orig;
 int force_dsc_restore_fd = -1;
+int new_bpp;
 
 static inline void manual(const char *expected)
 {
@@ -159,6 +161,18 @@ static void restore_force_dsc_en(void)
 	force_dsc_restore_fd = -1;
 }
 
+static void force_dp_dsc_enable_bpp(data_t *data)
+{
+	char file_name[128] = {0};
+	int ret;
+	char buffer[20];
+	sprintf(buffer, "%d", new_bpp);
+	strcpy(file_name, data->conn_name);
+	strcat(file_name, "/i915_dsc_bpp_slice_support");
+	igt_debug ("Forcing DSC BPP to %d on %s\n", new_bpp, data->conn_name);
+	ret = igt_sysfs_write(data->debugfs_fd, file_name, buffer, sizeof(buffer));
+}
+
 static void test_cleanup(data_t *data)
 {
 	igt_plane_t *primary;
@@ -231,9 +245,38 @@ static void update_display(data_t *data, enum dsc_test_type test_type)
 			     "Default DSC enable failed on Connector: %s Pipe: %s\n",
 			     data->conn_name,
 			     kmstest_pipe_name(data->pipe));
-	} else {
+	} else if (test_type == test_basic_dsc_enable_bpp) {
+
+	igt_debug("DSC is supported on %s\n", data->conn_name);
+
+	save_force_dsc_en(data);
+	force_dp_dsc_enable(data);
+
+	igt_debug("Trying to set BPP to %d\n", new_bpp);
+
+	force_dp_dsc_enable_bpp(data);
+
+	igt_output_set_pipe(data->output, data->pipe);
+	igt_create_pattern_fb(data->drm_fd, data->mode->hdisplay,
+			      data->mode->vdisplay,
+			      DRM_FORMAT_XRGB8888,
+			      LOCAL_DRM_FORMAT_MOD_NONE,
+			      &data->fb_test_pattern);
+	primary = igt_output_get_plane_type(data->output,
+					    DRM_PLANE_TYPE_PRIMARY);
+
+	/* Now set the output to the desired mode */
+	igt_plane_set_fb(primary, &data->fb_test_pattern);
+	igt_display_commit(&data->display);
+
+	/*
+	 * Until we have CRC check support, manually check if RGB test
+	 * pattern has no corruption.
+	 */
+	manual("RGB test pattern without corruption");
+
+	} else
 		igt_assert(!"Unknown test type\n");
-	}
 }
 
 static void run_test(data_t *data, igt_output_t *output,
@@ -258,7 +301,8 @@ igt_main
 	igt_output_t *output;
 	drmModeRes *res;
 	drmModeConnector *connector;
-	int i, test_conn_cnt, test_cnt;
+	int i, j, test_conn_cnt, test_cnt;
+	int dp_dsc_supported_compressed_bpp[] = {8, 10, 12, 15};
 	int tests[] = {DRM_MODE_CONNECTOR_eDP, DRM_MODE_CONNECTOR_DisplayPort};
 
 	igt_fixture {
@@ -298,6 +342,14 @@ igt_main
 			igt_subtest_f("basic-dsc-enable-%s",
 				      kmstest_connector_type_str(tests[test_cnt]))
 			run_test(&data, output, test_basic_dsc_enable);
+
+			/* Loop through all accepted DSC BPP's */
+			for (j = 0; j < ARRAY_SIZE(dp_dsc_supported_compressed_bpp); j++) {
+				new_bpp = dp_dsc_supported_compressed_bpp[j];
+				igt_subtest_f("basic-dsc-enable-%dbpp-%s", new_bpp,
+					      kmstest_connector_type_str(tests[test_cnt]))
+				run_test(&data, output, test_basic_dsc_enable_bpp);
+			}
 			igt_skip_on(test_conn_cnt == 0);
 		}
 	}
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/2] tests/kms_dp_dsc: Create subtest if connector is connected.

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/2] tests/kms_dp_dsc: Create subtest if connector is connected.
URL   : https://patchwork.freedesktop.org/series/61356/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
f414756be2ac57e194919973da7b86644ba61241 i915/gem_workarounds: Verify regs directly

258/273 testcase check: amdgpu/amd_basic        OK       0.11 s 
259/273 testcase check: amdgpu/amd_cs_nop       OK       0.09 s 
260/273 testcase check: amdgpu/amd_prime        OK       0.12 s 
261/273 runner                                  OK       1.42 s 
262/273 runner_json                             OK       0.05 s 
263/273 assembler: test/mov                     OK       0.08 s 
264/273 assembler: test/frc                     OK       0.08 s 
265/273 assembler: test/regtype                 OK       0.09 s 
266/273 assembler: test/rndd                    OK       0.08 s 
267/273 assembler: test/rndu                    OK       0.07 s 
268/273 assembler: test/rnde                    OK       0.03 s 
269/273 assembler: test/rnde-intsrc             OK       0.04 s 
270/273 assembler: test/rndz                    OK       0.07 s 
271/273 assembler: test/lzd                     OK       0.03 s 
272/273 assembler: test/not                     OK       0.06 s 
273/273 assembler: test/immediate               OK       0.06 s 

OK:       272
FAIL:       1
SKIP:       0
TIMEOUT:    0


The output from the failed tests:

 44/273 testcase check: kms_dp_dsc              FAIL     0.29 s (exit status 1)

--- command ---
/home/cidrm/igt-gpu-tools/tests/igt_command_line.sh kms_dp_dsc
--- stdout ---
tests/kms_dp_dsc:
  Checking invalid option handling...
  Checking valid option handling...
  Checking subtest enumeration...
FAIL: tests/kms_dp_dsc
--- stderr ---
Received signal SIGSEGV.
Stack trace: 
 #0 [fatal_sig_handler+0xd6]
 #1 [killpg+0x40]
 #2 [__real_main298+0xbc]
 #3 [main+0x27]
 #4 [__libc_start_main+0xe7]
 #5 [_start+0x2a]
-------

Full log written to /home/cidrm/igt-gpu-tools/build/meson-logs/testlog.txt
FAILED: meson-test 
/usr/bin/python3 -u /usr/bin/meson test --no-rebuild --print-errorlogs
ninja: build stopped: subcommand failed.

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 2/2] tests/kms_dp_dsc: Add a subtest to force DSC output BPP

[Anusha Srivatsa]

This subtest uses the accepted DSC BPPs and tries to
force a modeset by setting a certain BPP as the output
BPP for a connector.

Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 tests/kms_dp_dsc.c | 94 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 92 insertions(+), 2 deletions(-)

diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c
index 7f2bf276..f936eb33 100644
--- a/tests/kms_dp_dsc.c
+++ b/tests/kms_dp_dsc.c
@@ -46,7 +46,8 @@
 
 enum dsc_test_type
 {
-	test_basic_dsc_enable
+	test_basic_dsc_enable,
+	test_basic_dsc_enable_bpp
 };
 
 typedef struct {
@@ -67,6 +68,7 @@ typedef struct {
 
 bool force_dsc_en_orig;
 int force_dsc_restore_fd = -1;
+int new_bpp;
 
 static inline void manual(const char *expected)
 {
@@ -157,6 +159,17 @@ static void restore_force_dsc_en(void)
 	force_dsc_restore_fd = -1;
 }
 
+static void force_dp_dsc_enable_bpp(data_t *data)
+{
+	char file_name[128] = {0};
+	char buffer[20];
+	sprintf(buffer, "%d", new_bpp);
+	strcpy(file_name, data->conn_name);
+	strcat(file_name, "/i915_dsc_bpp_slice_support");
+	igt_debug ("Forcing DSC BPP to %d on %s\n", new_bpp, data->conn_name);
+	igt_sysfs_write(data->debugfs_fd, file_name, buffer, sizeof(buffer));
+}
+
 static void test_cleanup(data_t *data)
 {
 	igt_plane_t *primary;
@@ -229,6 +242,44 @@ static void update_display(data_t *data, enum dsc_test_type test_type)
 			     "Default DSC enable failed on Connector: %s Pipe: %s\n",
 			     data->conn_name,
 			     kmstest_pipe_name(data->pipe));
+	} else if (test_type == test_basic_dsc_enable_bpp) {
+		bool enabled;
+
+		igt_debug("DSC is supported on %s\n", data->conn_name);
+
+		save_force_dsc_en(data);
+		force_dp_dsc_enable(data);
+
+		igt_debug("Trying to set BPP to %d\n", new_bpp);
+
+		force_dp_dsc_enable_bpp(data);
+
+		igt_output_set_pipe(data->output, data->pipe);
+		igt_create_pattern_fb(data->drm_fd, data->mode->hdisplay,
+				      data->mode->vdisplay,
+				      DRM_FORMAT_XRGB8888,
+				      LOCAL_DRM_FORMAT_MOD_NONE,
+				      &data->fb_test_pattern);
+		primary = igt_output_get_plane_type(data->output,
+					    DRM_PLANE_TYPE_PRIMARY);
+		/* Now set the output to the desired mode */
+		igt_plane_set_fb(primary, &data->fb_test_pattern);
+		igt_display_commit(&data->display);
+
+		/*
+		 * Until we have CRC check support, manually check if RGB test
+		 * pattern has no corruption.
+		 */
+		manual("RGB test pattern without corruption");
+
+		enabled = is_dp_dsc_enabled(data);
+		restore_force_dsc_en();
+
+		igt_assert_f(enabled,
+			     "Default DSC BPP enable failed on Connector: %s Pipe: %s\n",
+			     data->conn_name,
+			     kmstest_pipe_name(data->pipe));
+
 	} else {
 		igt_assert(!"Unknown test type\n");
 	}
@@ -256,7 +307,9 @@ igt_main
 	igt_output_t *output;
 	drmModeRes *res;
 	drmModeConnector *connector;
-	int i, test_conn_cnt, test_cnt;
+	int i, j, test_conn_cnt, test_cnt;
+	int dp_dsc_supported_compressed_bpp[] = {8, 10, 12, 15};
+
 	int tests[] = {DRM_MODE_CONNECTOR_eDP, DRM_MODE_CONNECTOR_DisplayPort};
 
 	igt_fixture {
@@ -300,6 +353,43 @@ igt_main
 			}
 			igt_skip_on(test_conn_cnt == 0);
 		}
+
+		for (j = 0; j < ARRAY_SIZE(dp_dsc_supported_compressed_bpp); j++) {
+			new_bpp = dp_dsc_supported_compressed_bpp[j];
+			igt_subtest_f("basic-dsc-enable-%dbpp-%s", new_bpp,
+				      kmstest_connector_type_str(tests[test_cnt])) {
+			test_conn_cnt = 0;
+			for (i = 0; i < res->count_connectors; i++) {
+				connector = drmModeGetConnectorCurrent(data.drm_fd,
+								       res->connectors[i]);
+				if (connector->connection != DRM_MODE_CONNECTED ||
+				    connector->connector_type !=
+				    tests[test_cnt])
+					continue;
+				output = igt_output_from_connector(&data.display, connector);
+				sprintf(data.conn_name, "%s-%d",
+					kmstest_connector_type_str(connector->connector_type),
+					connector->connector_type_id);
+				printf("connector name: %s test_conn_count %d\n", data.conn_name, test_conn_cnt++);
+				if(!is_dp_dsc_supported(&data)) {
+				   igt_debug("DSC not supported on connector %s \n",
+					     data.conn_name);
+					continue;
+                                 }
+				if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort &&
+				    !is_dp_fec_supported(&data)) {
+					igt_debug("DSC cannot be enabled without FEC on %s\n",
+						  data.conn_name);
+					continue;
+				}
+
+				test_conn_cnt++;
+				run_test(&data, output, test_basic_dsc_enable_bpp);
+				}
+				igt_skip_on(test_conn_cnt == 0);
+			}
+		}
+
 	}
 
 	igt_fixture {
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/2] tests/kms_dp_dsc: Add a subtest to force DSC output BPP

[Manasi Navare]

On Mon, Jun 10, 2019 at 04:26:17PM -0700, Anusha Srivatsa wrote:
> This subtest uses the accepted DSC BPPs and tries to
> force a modeset by setting a certain BPP as the output
> BPP for a connector.
> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  tests/kms_dp_dsc.c | 94 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 92 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c
> index 7f2bf276..f936eb33 100644
> --- a/tests/kms_dp_dsc.c
> +++ b/tests/kms_dp_dsc.c
> @@ -46,7 +46,8 @@
>  
>  enum dsc_test_type
>  {
> -	test_basic_dsc_enable
> +	test_basic_dsc_enable,
> +	test_basic_dsc_enable_bpp
>  };
>  
>  typedef struct {
> @@ -67,6 +68,7 @@ typedef struct {
>  
>  bool force_dsc_en_orig;
>  int force_dsc_restore_fd = -1;
> +int new_bpp;
>  
>  static inline void manual(const char *expected)
>  {
> @@ -157,6 +159,17 @@ static void restore_force_dsc_en(void)
>  	force_dsc_restore_fd = -1;
>  }
>  
> +static void force_dp_dsc_enable_bpp(data_t *data)
> +{
> +	char file_name[128] = {0};
> +	char buffer[20];
> +	sprintf(buffer, "%d", new_bpp);
> +	strcpy(file_name, data->conn_name);
> +	strcat(file_name, "/i915_dsc_bpp_slice_support");
> +	igt_debug ("Forcing DSC BPP to %d on %s\n", new_bpp, data->conn_name);
> +	igt_sysfs_write(data->debugfs_fd, file_name, buffer, sizeof(buffer));

Check the return value of igt_sysfs_write and assert if write failed

> +}
> +
>  static void test_cleanup(data_t *data)
>  {
>  	igt_plane_t *primary;
> @@ -229,6 +242,44 @@ static void update_display(data_t *data, enum dsc_test_type test_type)
>  			     "Default DSC enable failed on Connector: %s Pipe: %s\n",
>  			     data->conn_name,
>  			     kmstest_pipe_name(data->pipe));
> +	} else if (test_type == test_basic_dsc_enable_bpp) {
> +		bool enabled;
> +
> +		igt_debug("DSC is supported on %s\n", data->conn_name);
> +

Unnecessary newline

> +		save_force_dsc_en(data);
> +		force_dp_dsc_enable(data);
> +
> +		igt_debug("Trying to set BPP to %d\n", new_bpp);
> +
> +		force_dp_dsc_enable_bpp(data);
> +
> +		igt_output_set_pipe(data->output, data->pipe);
> +		igt_create_pattern_fb(data->drm_fd, data->mode->hdisplay,
> +				      data->mode->vdisplay,
> +				      DRM_FORMAT_XRGB8888,
> +				      LOCAL_DRM_FORMAT_MOD_NONE,
> +				      &data->fb_test_pattern);
> +		primary = igt_output_get_plane_type(data->output,
> +					    DRM_PLANE_TYPE_PRIMARY);
> +		/* Now set the output to the desired mode */
> +		igt_plane_set_fb(primary, &data->fb_test_pattern);
> +		igt_display_commit(&data->display);
> +
> +		/*
> +		 * Until we have CRC check support, manually check if RGB test
> +		 * pattern has no corruption.
> +		 */
> +		manual("RGB test pattern without corruption");
> +
> +		enabled = is_dp_dsc_enabled(data);
> +		restore_force_dsc_en();

Not sure if we need to restore the DSC BPP value? What does the kernel code set the force_dsc_bpp value to if
not forced by IGT?

> +
> +		igt_assert_f(enabled,
> +			     "Default DSC BPP enable failed on Connector: %s Pipe: %s\n",
> +			     data->conn_name,
> +			     kmstest_pipe_name(data->pipe));
> +
>  	} else {
>  		igt_assert(!"Unknown test type\n");
>  	}
> @@ -256,7 +307,9 @@ igt_main
>  	igt_output_t *output;
>  	drmModeRes *res;
>  	drmModeConnector *connector;
> -	int i, test_conn_cnt, test_cnt;
> +	int i, j, test_conn_cnt, test_cnt;
> +	int dp_dsc_supported_compressed_bpp[] = {8, 10, 12, 15};

 May be a add a comment here saying these are the supported compressed BPPs on Gen 11

> +
>  	int tests[] = {DRM_MODE_CONNECTOR_eDP, DRM_MODE_CONNECTOR_DisplayPort};
>  
>  	igt_fixture {
> @@ -300,6 +353,43 @@ igt_main
>  			}
>  			igt_skip_on(test_conn_cnt == 0);
>  		}
> +
> +		for (j = 0; j < ARRAY_SIZE(dp_dsc_supported_compressed_bpp); j++) {
> +			new_bpp = dp_dsc_supported_compressed_bpp[j];
> +			igt_subtest_f("basic-dsc-enable-%dbpp-%s", new_bpp,
> +				      kmstest_connector_type_str(tests[test_cnt])) {
> +			test_conn_cnt = 0;

Check the indentation for the code within the igt_subtest_f, it needs one more tab

> +			for (i = 0; i < res->count_connectors; i++) {
> +				connector = drmModeGetConnectorCurrent(data.drm_fd,
> +								       res->connectors[i]);
> +				if (connector->connection != DRM_MODE_CONNECTED ||
> +				    connector->connector_type !=
> +				    tests[test_cnt])
> +					continue;
> +				output = igt_output_from_connector(&data.display, connector);
> +				sprintf(data.conn_name, "%s-%d",
> +					kmstest_connector_type_str(connector->connector_type),
> +					connector->connector_type_id);
> +				printf("connector name: %s test_conn_count %d\n", data.conn_name, test_conn_cnt++);

printf is redundant here, if you want you could have this in igt_debug so its printed only for debug

> +				if(!is_dp_dsc_supported(&data)) {
> +				   igt_debug("DSC not supported on connector %s \n",
> +					     data.conn_name);
> +					continue;
> +                                 }
> +				if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort &&
> +				    !is_dp_fec_supported(&data)) {
> +					igt_debug("DSC cannot be enabled without FEC on %s\n",
> +						  data.conn_name);
> +					continue;
> +				}
> +

Redundant newline

Manasi


> +				test_conn_cnt++;
> +				run_test(&data, output, test_basic_dsc_enable_bpp);
> +				}
> +				igt_skip_on(test_conn_cnt == 0);
> +			}
> +		}
> +
>  	}
>  
>  	igt_fixture {
> -- 
> 2.17.1
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/2] tests/kms_dp_dsc: Add a subtest to force DSC output BPP

[Srivatsa, Anusha]

>-----Original Message-----
>From: Navare, Manasi D
>Sent: Tuesday, June 11, 2019 12:55 PM
>To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>Cc: igt-dev@lists.freedesktop.org; Latvala, Petri <petri.latvala@intel.com>
>Subject: Re: [PATCH i-g-t 2/2] tests/kms_dp_dsc: Add a subtest to force DSC
>output BPP
>
>On Mon, Jun 10, 2019 at 04:26:17PM -0700, Anusha Srivatsa wrote:
>> This subtest uses the accepted DSC BPPs and tries to force a modeset
>> by setting a certain BPP as the output BPP for a connector.
>>
>> Cc: Petri Latvala <petri.latvala@intel.com>
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> ---
>>  tests/kms_dp_dsc.c | 94
>> +++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 92 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c index
>> 7f2bf276..f936eb33 100644
>> --- a/tests/kms_dp_dsc.c
>> +++ b/tests/kms_dp_dsc.c
>> @@ -46,7 +46,8 @@
>>
>>  enum dsc_test_type
>>  {
>> -	test_basic_dsc_enable
>> +	test_basic_dsc_enable,
>> +	test_basic_dsc_enable_bpp
>>  };
>>
>>  typedef struct {
>> @@ -67,6 +68,7 @@ typedef struct {
>>
>>  bool force_dsc_en_orig;
>>  int force_dsc_restore_fd = -1;
>> +int new_bpp;
>>
>>  static inline void manual(const char *expected)  { @@ -157,6 +159,17
>> @@ static void restore_force_dsc_en(void)
>>  	force_dsc_restore_fd = -1;
>>  }
>>
>> +static void force_dp_dsc_enable_bpp(data_t *data) {
>> +	char file_name[128] = {0};
>> +	char buffer[20];
>> +	sprintf(buffer, "%d", new_bpp);
>> +	strcpy(file_name, data->conn_name);
>> +	strcat(file_name, "/i915_dsc_bpp_slice_support");
>> +	igt_debug ("Forcing DSC BPP to %d on %s\n", new_bpp, data-
>>conn_name);
>> +	igt_sysfs_write(data->debugfs_fd, file_name, buffer,
>> +sizeof(buffer));
>
>Check the return value of igt_sysfs_write and assert if write failed

Yes. Makes sense.

>> +}
>> +
>>  static void test_cleanup(data_t *data)  {
>>  	igt_plane_t *primary;
>> @@ -229,6 +242,44 @@ static void update_display(data_t *data, enum
>dsc_test_type test_type)
>>  			     "Default DSC enable failed on Connector: %s Pipe:
>%s\n",
>>  			     data->conn_name,
>>  			     kmstest_pipe_name(data->pipe));
>> +	} else if (test_type == test_basic_dsc_enable_bpp) {
>> +		bool enabled;
>> +
>> +		igt_debug("DSC is supported on %s\n", data->conn_name);
>> +
>
>Unnecessary newline
>
>> +		save_force_dsc_en(data);
>> +		force_dp_dsc_enable(data);
>> +
>> +		igt_debug("Trying to set BPP to %d\n", new_bpp);
>> +
>> +		force_dp_dsc_enable_bpp(data);
>> +
>> +		igt_output_set_pipe(data->output, data->pipe);
>> +		igt_create_pattern_fb(data->drm_fd, data->mode->hdisplay,
>> +				      data->mode->vdisplay,
>> +				      DRM_FORMAT_XRGB8888,
>> +				      LOCAL_DRM_FORMAT_MOD_NONE,
>> +				      &data->fb_test_pattern);
>> +		primary = igt_output_get_plane_type(data->output,
>> +					    DRM_PLANE_TYPE_PRIMARY);
>> +		/* Now set the output to the desired mode */
>> +		igt_plane_set_fb(primary, &data->fb_test_pattern);
>> +		igt_display_commit(&data->display);
>> +
>> +		/*
>> +		 * Until we have CRC check support, manually check if RGB test
>> +		 * pattern has no corruption.
>> +		 */
>> +		manual("RGB test pattern without corruption");
>> +
>> +		enabled = is_dp_dsc_enabled(data);
>> +		restore_force_dsc_en();
>
>Not sure if we need to restore the DSC BPP value? What does the kernel code set
>the force_dsc_bpp value to if not forced by IGT?

I don't think we need it. Force_enable_bpp gets set iff we are forcing the dsc. If we are not forcing dsc, and the kernel does DSC because of the resolution restrictions, it computes the compressed bpp according to the calculation that are part of compute_dsc_params() and nothing to do with this value.

>
>> +
>> +		igt_assert_f(enabled,
>> +			     "Default DSC BPP enable failed on Connector: %s
>Pipe: %s\n",
>> +			     data->conn_name,
>> +			     kmstest_pipe_name(data->pipe));
>> +
>>  	} else {
>>  		igt_assert(!"Unknown test type\n");
>>  	}
>> @@ -256,7 +307,9 @@ igt_main
>>  	igt_output_t *output;
>>  	drmModeRes *res;
>>  	drmModeConnector *connector;
>> -	int i, test_conn_cnt, test_cnt;
>> +	int i, j, test_conn_cnt, test_cnt;
>> +	int dp_dsc_supported_compressed_bpp[] = {8, 10, 12, 15};
>
> May be a add a comment here saying these are the supported compressed BPPs
>on Gen 11

Sure. 
>> +
>>  	int tests[] = {DRM_MODE_CONNECTOR_eDP,
>> DRM_MODE_CONNECTOR_DisplayPort};
>>
>>  	igt_fixture {
>> @@ -300,6 +353,43 @@ igt_main
>>  			}
>>  			igt_skip_on(test_conn_cnt == 0);
>>  		}
>> +
>> +		for (j = 0; j < ARRAY_SIZE(dp_dsc_supported_compressed_bpp);
>j++) {
>> +			new_bpp = dp_dsc_supported_compressed_bpp[j];
>> +			igt_subtest_f("basic-dsc-enable-%dbpp-%s", new_bpp,
>> +				      kmstest_connector_type_str(tests[test_cnt]))
>{
>> +			test_conn_cnt = 0;
>
>Check the indentation for the code within the igt_subtest_f, it needs one more
>tab
>
>> +			for (i = 0; i < res->count_connectors; i++) {
>> +				connector =
>drmModeGetConnectorCurrent(data.drm_fd,
>> +								       res-
>>connectors[i]);
>> +				if (connector->connection !=
>DRM_MODE_CONNECTED ||
>> +				    connector->connector_type !=
>> +				    tests[test_cnt])
>> +					continue;
>> +				output =
>igt_output_from_connector(&data.display, connector);
>> +				sprintf(data.conn_name, "%s-%d",
>> +					kmstest_connector_type_str(connector-
>>connector_type),
>> +					connector->connector_type_id);
>> +				printf("connector name: %s test_conn_count
>%d\n", data.conn_name,
>> +test_conn_cnt++);
>
>printf is redundant here, if you want you could have this in igt_debug so its
>printed only for debug

Yes I agree. This was for my debugging. I will remove this. I dont think we need it to be in a igt_debug also.

Anusha
>
>> +				if(!is_dp_dsc_supported(&data)) {
>> +				   igt_debug("DSC not supported on connector
>%s \n",
>> +					     data.conn_name);
>> +					continue;
>> +                                 }
>> +				if (connector->connector_type ==
>DRM_MODE_CONNECTOR_DisplayPort &&
>> +				    !is_dp_fec_supported(&data)) {
>> +					igt_debug("DSC cannot be enabled
>without FEC on %s\n",
>> +						  data.conn_name);
>> +					continue;
>> +				}
>> +
>
>Redundant newline
>
>Manasi
>
>
>> +				test_conn_cnt++;
>> +				run_test(&data, output,
>test_basic_dsc_enable_bpp);
>> +				}
>> +				igt_skip_on(test_conn_cnt == 0);
>> +			}
>> +		}
>> +
>>  	}
>>
>>  	igt_fixture {
>> --
>> 2.17.1
>>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/2] tests/kms_dp_dsc: Add a subtest to force DSC output BPP

[Srivatsa, Anusha]

The subtest  is still unstable.
The debugfs node - i915_dsc_fec_support is both read and write. We use it to check if sink supports DSC and to force the DSC, Which I think leave it unstable when used for the second time - for the bpp subtest.
IT is either the debugfs node state or maybe the crtc state from the force-dsc subtest that is causing the unstable nature,

Is it a good option to separate the debugfs nodes to read and write specific ?

Manasi, what do you think?

Anusha 

>-----Original Message-----
>From: Navare, Manasi D
>Sent: Tuesday, June 11, 2019 12:55 PM
>To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>Cc: igt-dev@lists.freedesktop.org; Latvala, Petri <petri.latvala@intel.com>
>Subject: Re: [PATCH i-g-t 2/2] tests/kms_dp_dsc: Add a subtest to force DSC
>output BPP
>
>On Mon, Jun 10, 2019 at 04:26:17PM -0700, Anusha Srivatsa wrote:
>> This subtest uses the accepted DSC BPPs and tries to force a modeset
>> by setting a certain BPP as the output BPP for a connector.
>>
>> Cc: Petri Latvala <petri.latvala@intel.com>
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> ---
>>  tests/kms_dp_dsc.c | 94
>> +++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 92 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c index
>> 7f2bf276..f936eb33 100644
>> --- a/tests/kms_dp_dsc.c
>> +++ b/tests/kms_dp_dsc.c
>> @@ -46,7 +46,8 @@
>>
>>  enum dsc_test_type
>>  {
>> -	test_basic_dsc_enable
>> +	test_basic_dsc_enable,
>> +	test_basic_dsc_enable_bpp
>>  };
>>
>>  typedef struct {
>> @@ -67,6 +68,7 @@ typedef struct {
>>
>>  bool force_dsc_en_orig;
>>  int force_dsc_restore_fd = -1;
>> +int new_bpp;
>>
>>  static inline void manual(const char *expected)  { @@ -157,6 +159,17
>> @@ static void restore_force_dsc_en(void)
>>  	force_dsc_restore_fd = -1;
>>  }
>>
>> +static void force_dp_dsc_enable_bpp(data_t *data) {
>> +	char file_name[128] = {0};
>> +	char buffer[20];
>> +	sprintf(buffer, "%d", new_bpp);
>> +	strcpy(file_name, data->conn_name);
>> +	strcat(file_name, "/i915_dsc_bpp_slice_support");
>> +	igt_debug ("Forcing DSC BPP to %d on %s\n", new_bpp, data-
>>conn_name);
>> +	igt_sysfs_write(data->debugfs_fd, file_name, buffer,
>> +sizeof(buffer));
>
>Check the return value of igt_sysfs_write and assert if write failed
>
>> +}
>> +
>>  static void test_cleanup(data_t *data)  {
>>  	igt_plane_t *primary;
>> @@ -229,6 +242,44 @@ static void update_display(data_t *data, enum
>dsc_test_type test_type)
>>  			     "Default DSC enable failed on Connector: %s Pipe:
>%s\n",
>>  			     data->conn_name,
>>  			     kmstest_pipe_name(data->pipe));
>> +	} else if (test_type == test_basic_dsc_enable_bpp) {
>> +		bool enabled;
>> +
>> +		igt_debug("DSC is supported on %s\n", data->conn_name);
>> +
>
>Unnecessary newline
>
>> +		save_force_dsc_en(data);
>> +		force_dp_dsc_enable(data);
>> +
>> +		igt_debug("Trying to set BPP to %d\n", new_bpp);
>> +
>> +		force_dp_dsc_enable_bpp(data);
>> +
>> +		igt_output_set_pipe(data->output, data->pipe);
>> +		igt_create_pattern_fb(data->drm_fd, data->mode->hdisplay,
>> +				      data->mode->vdisplay,
>> +				      DRM_FORMAT_XRGB8888,
>> +				      LOCAL_DRM_FORMAT_MOD_NONE,
>> +				      &data->fb_test_pattern);
>> +		primary = igt_output_get_plane_type(data->output,
>> +					    DRM_PLANE_TYPE_PRIMARY);
>> +		/* Now set the output to the desired mode */
>> +		igt_plane_set_fb(primary, &data->fb_test_pattern);
>> +		igt_display_commit(&data->display);
>> +
>> +		/*
>> +		 * Until we have CRC check support, manually check if RGB test
>> +		 * pattern has no corruption.
>> +		 */
>> +		manual("RGB test pattern without corruption");
>> +
>> +		enabled = is_dp_dsc_enabled(data);
>> +		restore_force_dsc_en();
>
>Not sure if we need to restore the DSC BPP value? What does the kernel code set
>the force_dsc_bpp value to if not forced by IGT?
>
>> +
>> +		igt_assert_f(enabled,
>> +			     "Default DSC BPP enable failed on Connector: %s
>Pipe: %s\n",
>> +			     data->conn_name,
>> +			     kmstest_pipe_name(data->pipe));
>> +
>>  	} else {
>>  		igt_assert(!"Unknown test type\n");
>>  	}
>> @@ -256,7 +307,9 @@ igt_main
>>  	igt_output_t *output;
>>  	drmModeRes *res;
>>  	drmModeConnector *connector;
>> -	int i, test_conn_cnt, test_cnt;
>> +	int i, j, test_conn_cnt, test_cnt;
>> +	int dp_dsc_supported_compressed_bpp[] = {8, 10, 12, 15};
>
> May be a add a comment here saying these are the supported compressed BPPs
>on Gen 11
>
>> +
>>  	int tests[] = {DRM_MODE_CONNECTOR_eDP,
>> DRM_MODE_CONNECTOR_DisplayPort};
>>
>>  	igt_fixture {
>> @@ -300,6 +353,43 @@ igt_main
>>  			}
>>  			igt_skip_on(test_conn_cnt == 0);
>>  		}
>> +
>> +		for (j = 0; j < ARRAY_SIZE(dp_dsc_supported_compressed_bpp);
>j++) {
>> +			new_bpp = dp_dsc_supported_compressed_bpp[j];
>> +			igt_subtest_f("basic-dsc-enable-%dbpp-%s", new_bpp,
>> +				      kmstest_connector_type_str(tests[test_cnt]))
>{
>> +			test_conn_cnt = 0;
>
>Check the indentation for the code within the igt_subtest_f, it needs one more
>tab
>
>> +			for (i = 0; i < res->count_connectors; i++) {
>> +				connector =
>drmModeGetConnectorCurrent(data.drm_fd,
>> +								       res-
>>connectors[i]);
>> +				if (connector->connection !=
>DRM_MODE_CONNECTED ||
>> +				    connector->connector_type !=
>> +				    tests[test_cnt])
>> +					continue;
>> +				output =
>igt_output_from_connector(&data.display, connector);
>> +				sprintf(data.conn_name, "%s-%d",
>> +					kmstest_connector_type_str(connector-
>>connector_type),
>> +					connector->connector_type_id);
>> +				printf("connector name: %s test_conn_count
>%d\n", data.conn_name,
>> +test_conn_cnt++);
>
>printf is redundant here, if you want you could have this in igt_debug so its
>printed only for debug
>
>> +				if(!is_dp_dsc_supported(&data)) {
>> +				   igt_debug("DSC not supported on connector
>%s \n",
>> +					     data.conn_name);
>> +					continue;
>> +                                 }
>> +				if (connector->connector_type ==
>DRM_MODE_CONNECTOR_DisplayPort &&
>> +				    !is_dp_fec_supported(&data)) {
>> +					igt_debug("DSC cannot be enabled
>without FEC on %s\n",
>> +						  data.conn_name);
>> +					continue;
>> +				}
>> +
>
>Redundant newline
>
>Manasi
>
>
>> +				test_conn_cnt++;
>> +				run_test(&data, output,
>test_basic_dsc_enable_bpp);
>> +				}
>> +				igt_skip_on(test_conn_cnt == 0);
>> +			}
>> +		}
>> +
>>  	}
>>
>>  	igt_fixture {
>> --
>> 2.17.1
>>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev