[v3] tests/kms: increase max threshold time for edid read

Submitted by Clinton Taylor on Aug. 9, 2017, 10:51 p.m.

Details

Message ID 1502319112-7758-1-git-send-email-clinton.a.taylor@intel.com
State New
Headers show
Series "tests/kms: increase max threshold time for edid read" ( rev: 3 ) in IGT (deprecated)

Not browsing as part of any series.

Commit Message

Clinton Taylor Aug. 9, 2017, 10:51 p.m.
From: Clint Taylor <clinton.a.taylor@intel.com>

Current 50ms max threshold timing for an EDID read is very close to the
actual time for a 2 block HDMI EDID read. Adjust the timings base on
connector type as DP reads are at 1 MBit and HDMI at 100K bit. If an LSPcon
is connected to device under test the -l option should be passed to update
the threshold timing to allow the LSPcon to read the EDID at the HDMI
timing. The -l option should be used when LSPcon is on the motherboard or
if a USB_C->HDMI converter is present

V2: Adjust timings based on connector type, EDID size, and Add an option to
specify if an LSPcon is present.
V3: added bugzilla to commit message

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100047

Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Marta Lofstedt <marta.lofstedt@intel.com>
Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
---
 tests/kms_sysfs_edid_timing.c | 76 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 62 insertions(+), 14 deletions(-)

Patch hide | download patch | download mbox

diff --git a/tests/kms_sysfs_edid_timing.c b/tests/kms_sysfs_edid_timing.c
index 1201388..441dfee 100644
--- a/tests/kms_sysfs_edid_timing.c
+++ b/tests/kms_sysfs_edid_timing.c
@@ -26,21 +26,46 @@ 
 #include <fcntl.h>
 #include <sys/stat.h>
 
-#define THRESHOLD_PER_CONNECTOR	10
-#define THRESHOLD_TOTAL		50
-#define CHECK_TIMES		15
+#define THRESHOLD_FOR_EMPTY_CONNECTOR	10
+#define THRESHOLD_PER_EDID_BLOCK		5
+#define HDMI_THRESHOLD_MULTIPLIER		10
+#define CHECK_TIMES				10
 
 IGT_TEST_DESCRIPTION("This check the time we take to read the content of all "
 		     "the possible connectors. Without the edid -ENXIO patch "
 		     "(http://permalink.gmane.org/gmane.comp.video.dri.devel/62083), "
-		     "we sometimes take a *really* long time. "
-		     "So let's just check for some reasonable timing here");
+		     "we sometimes take a *really* long time. So let's check "
+		     "an approximate time per edid block based on connector "
+		     "type. The -l option adjusts DP timing to reflect HDMI read "
+		     "timings from LSPcon.");
+
+/* The -l option has been added to correctly handle timings when an LSPcon is
+ * present. This could be on the platform itself or in a USB_C->HDMI converter.
+ * With LSPCon EDID read timing will need to change from the 1 Mbit AUX
+ * bus speed to the 100 Kbit HDMI DDC bus speed
+ */
+bool lspcon_present;
 
+static int opt_handler(int opt, int opt_index, void *data)
+{
+	if (opt == 'l') {
+		lspcon_present = true;
+		igt_info("set LSPcon present on DP ports\n");
+	}
 
-igt_simple_main
+	return 0;
+}
+
+int main(int argc, char **argv)
 {
 	DIR *dirp;
 	struct dirent *de;
+	lspcon_present = false;
+
+	igt_simple_init_parse_opts(&argc, argv, "l", NULL, NULL,
+				      opt_handler, NULL);
+
+	igt_skip_on_simulation();
 
 	dirp = opendir("/sys/class/drm");
 	igt_assert(dirp != NULL);
@@ -49,17 +74,36 @@  igt_simple_main
 		struct igt_mean mean = {};
 		struct stat st;
 		char path[PATH_MAX];
-		int i;
+		char edid_path[PATH_MAX];
+		char edid[512]; /* enough for 4 block edid */
+		unsigned long edid_size = 0;
+		int i, fd_edid;
+		unsigned int threshold = 0;
 
 		if (*de->d_name == '.')
 			continue;;
 
 		snprintf(path, sizeof(path), "/sys/class/drm/%s/status",
 				de->d_name);
+		snprintf(edid_path, sizeof(edid_path), "/sys/class/drm/%s/edid",
+				de->d_name);
 
 		if (stat(path, &st))
 			continue;
 
+		fd_edid = open(edid_path, O_RDONLY);
+		if (fd_edid == -1) {
+			igt_warn("Read Error EDID\n");
+			continue;
+		}
+
+		edid_size = read(fd_edid, edid, 512);
+		threshold = THRESHOLD_PER_EDID_BLOCK * (edid_size / 128);
+		if (lspcon_present || (edid_size > 128 &&
+		    !strncmp(de->d_name, "card0-HDMI", 10))) {
+			threshold *= HDMI_THRESHOLD_MULTIPLIER;
+		}
+
 		igt_mean_init(&mean);
 		for (i = 0; i < CHECK_TIMES; i++) {
 			struct timespec ts = {};
@@ -76,22 +120,26 @@  igt_simple_main
 		}
 
 		igt_debug("%s: mean.max %.2fns, %.2fus, %.2fms, "
-			  "mean.avg %.2fns, %.2fus, %.2fms\n",
+			  "mean.avg %.2fns, %.2fus, %.2fms, "
+			  "edid_size %lu, threshold %d\n",
 			  de->d_name,
 			  mean.max, mean.max / 1e3, mean.max / 1e6,
-			  mean.mean, mean.mean / 1e3, mean.mean / 1e6);
+			  mean.mean, mean.mean / 1e3, mean.mean / 1e6,
+			  edid_size, threshold);
 
-		if (mean.max > (THRESHOLD_PER_CONNECTOR * 1e6)) {
+		if (edid_size == 0 &&
+		   (mean.max > (THRESHOLD_FOR_EMPTY_CONNECTOR * 1e6))) {
 			igt_warn("%s: probe time exceed 10ms, "
 				 "max=%.2fms, avg=%.2fms\n", de->d_name,
 				 mean.max / 1e6, mean.mean / 1e6);
 		}
-		igt_assert_f(mean.mean < (THRESHOLD_TOTAL * 1e6),
-			     "%s: average probe time exceeded 50ms, "
-			     "max=%.2fms, avg=%.2fms\n", de->d_name,
+		if (edid_size > 0)
+			igt_assert_f(mean.mean < (threshold * 1e6),
+			     "%s: average probe time exceeded %dms, "
+			     "max=%.2fms, avg=%.2fms\n", de->d_name, threshold,
 			     mean.max / 1e6, mean.mean / 1e6);
 
 	}
 	closedir(dirp);
-
+	igt_exit();
 }

Comments

Sorry Clinton I fail the test again on my BDW NUCi5 with this V3.

Check: https://bugs.freedesktop.org/show_bug.cgi?id=100047
For details.

/Marta 

> -----Original Message-----
> From: Taylor, Clinton A
> Sent: Thursday, August 10, 2017 1:52 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Taylor, Clinton A <clinton.a.taylor@intel.com>; Vetter, Daniel
> <daniel.vetter@intel.com>; Lofstedt, Marta <marta.lofstedt@intel.com>
> Subject: [PATCH v3 i-g-t] tests/kms: increase max threshold time for edid
> read
> 
> From: Clint Taylor <clinton.a.taylor@intel.com>
> 
> Current 50ms max threshold timing for an EDID read is very close to the
> actual time for a 2 block HDMI EDID read. Adjust the timings base on
> connector type as DP reads are at 1 MBit and HDMI at 100K bit. If an LSPcon is
> connected to device under test the -l option should be passed to update the
> threshold timing to allow the LSPcon to read the EDID at the HDMI timing.
> The -l option should be used when LSPcon is on the motherboard or if a
> USB_C->HDMI converter is present
> 
> V2: Adjust timings based on connector type, EDID size, and Add an option to
> specify if an LSPcon is present.
> V3: added bugzilla to commit message
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100047
> 
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Marta Lofstedt <marta.lofstedt@intel.com>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
>  tests/kms_sysfs_edid_timing.c | 76
> +++++++++++++++++++++++++++++++++++--------
>  1 file changed, 62 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/kms_sysfs_edid_timing.c b/tests/kms_sysfs_edid_timing.c
> index 1201388..441dfee 100644
> --- a/tests/kms_sysfs_edid_timing.c
> +++ b/tests/kms_sysfs_edid_timing.c
> @@ -26,21 +26,46 @@
>  #include <fcntl.h>
>  #include <sys/stat.h>
> 
> -#define THRESHOLD_PER_CONNECTOR	10
> -#define THRESHOLD_TOTAL		50
> -#define CHECK_TIMES		15
> +#define THRESHOLD_FOR_EMPTY_CONNECTOR	10
> +#define THRESHOLD_PER_EDID_BLOCK		5
> +#define HDMI_THRESHOLD_MULTIPLIER		10
> +#define CHECK_TIMES				10
> 
>  IGT_TEST_DESCRIPTION("This check the time we take to read the content of
> all "
>  		     "the possible connectors. Without the edid -
> ENXIO patch "
> 
> "(http://permalink.gmane.org/gmane.comp.video.dri.devel/62083), "
> -		     "we sometimes take a *really* long time. "
> -		     "So let's just check for some reasonable
> timing here");
> +		     "we sometimes take a *really* long time. So
> let's check "
> +		     "an approximate time per edid block based on
> connector "
> +		     "type. The -l option adjusts DP timing to
> reflect HDMI read "
> +		     "timings from LSPcon.");
> +
> +/* The -l option has been added to correctly handle timings when an
> +LSPcon is
> + * present. This could be on the platform itself or in a USB_C->HDMI
> converter.
> + * With LSPCon EDID read timing will need to change from the 1 Mbit AUX
> + * bus speed to the 100 Kbit HDMI DDC bus speed  */ bool
> +lspcon_present;
> 
> +static int opt_handler(int opt, int opt_index, void *data) {
> +	if (opt == 'l') {
> +		lspcon_present = true;
> +		igt_info("set LSPcon present on DP ports\n");
> +	}
> 
> -igt_simple_main
> +	return 0;
> +}
> +
> +int main(int argc, char **argv)
>  {
>  	DIR *dirp;
>  	struct dirent *de;
> +	lspcon_present = false;
> +
> +	igt_simple_init_parse_opts(&argc, argv, "l", NULL, NULL,
> +				      opt_handler,
> NULL);
> +
> +	igt_skip_on_simulation();
> 
>  	dirp = opendir("/sys/class/drm");
>  	igt_assert(dirp != NULL);
> @@ -49,17 +74,36 @@ igt_simple_main
>  		struct igt_mean mean = {};
>  		struct stat st;
>  		char path[PATH_MAX];
> -		int i;
> +		char edid_path[PATH_MAX];
> +		char edid[512]; /* enough for 4 block edid */
> +		unsigned long edid_size = 0;
> +		int i, fd_edid;
> +		unsigned int threshold = 0;
> 
>  		if (*de->d_name == '.')
>  			continue;;
> 
>  		snprintf(path, sizeof(path),
> "/sys/class/drm/%s/status",
>  				de->d_name);
> +		snprintf(edid_path, sizeof(edid_path),
> "/sys/class/drm/%s/edid",
> +				de->d_name);
> 
>  		if (stat(path, &st))
>  			continue;
> 
> +		fd_edid = open(edid_path, O_RDONLY);
> +		if (fd_edid == -1) {
> +			igt_warn("Read Error EDID\n");
> +			continue;
> +		}
> +
> +		edid_size = read(fd_edid, edid, 512);
> +		threshold = THRESHOLD_PER_EDID_BLOCK *
> (edid_size / 128);
> +		if (lspcon_present || (edid_size > 128 &&
> +		    !strncmp(de->d_name, "card0-HDMI", 10))) {
> +			threshold *=
> HDMI_THRESHOLD_MULTIPLIER;
> +		}
> +
>  		igt_mean_init(&mean);
>  		for (i = 0; i < CHECK_TIMES; i++) {
>  			struct timespec ts = {};
> @@ -76,22 +120,26 @@ igt_simple_main
>  		}
> 
>  		igt_debug("%s: mean.max %.2fns, %.2fus,
> %.2fms, "
> -			  "mean.avg %.2fns, %.2fus,
> %.2fms\n",
> +			  "mean.avg %.2fns, %.2fus,
> %.2fms, "
> +			  "edid_size %lu, threshold %d\n",
>  			  de->d_name,
>  			  mean.max, mean.max / 1e3,
> mean.max / 1e6,
> -			  mean.mean, mean.mean / 1e3,
> mean.mean / 1e6);
> +			  mean.mean, mean.mean / 1e3,
> mean.mean / 1e6,
> +			  edid_size, threshold);
> 
> -		if (mean.max > (THRESHOLD_PER_CONNECTOR
> * 1e6)) {
> +		if (edid_size == 0 &&
> +		   (mean.max >
> (THRESHOLD_FOR_EMPTY_CONNECTOR * 1e6))) {
>  			igt_warn("%s: probe time exceed
> 10ms, "
>  				 "max=%.2fms,
> avg=%.2fms\n", de->d_name,
>  				 mean.max / 1e6,
> mean.mean / 1e6);
>  		}
> -		igt_assert_f(mean.mean < (THRESHOLD_TOTAL
> * 1e6),
> -			     "%s: average probe time
> exceeded 50ms, "
> -			     "max=%.2fms, avg=%.2fms\n",
> de->d_name,
> +		if (edid_size > 0)
> +			igt_assert_f(mean.mean <
> (threshold * 1e6),
> +			     "%s: average probe time
> exceeded %dms, "
> +			     "max=%.2fms, avg=%.2fms\n",
> de->d_name, threshold,
>  			     mean.max / 1e6, mean.mean /
> 1e6);
> 
>  	}
>  	closedir(dirp);
> -
> +	igt_exit();
>  }
> --
> 1.9.1
With this tweak the test pass:

> -----Original Message-----
> From: Lofstedt, Marta
> Sent: Thursday, August 10, 2017 9:41 AM
> To: Taylor, Clinton A <clinton.a.taylor@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Vetter, Daniel <daniel.vetter@intel.com>
> Subject: RE: [PATCH v3 i-g-t] tests/kms: increase max threshold time for edid
> read
> 
> Sorry Clinton I fail the test again on my BDW NUCi5 with this V3.
> 
> Check: https://bugs.freedesktop.org/show_bug.cgi?id=100047
> For details.
> 
> /Marta
> 
> > -----Original Message-----
> > From: Taylor, Clinton A
> > Sent: Thursday, August 10, 2017 1:52 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Taylor, Clinton A <clinton.a.taylor@intel.com>; Vetter, Daniel
> > <daniel.vetter@intel.com>; Lofstedt, Marta <marta.lofstedt@intel.com>
> > Subject: [PATCH v3 i-g-t] tests/kms: increase max threshold time for
> > edid read
> >
> > From: Clint Taylor <clinton.a.taylor@intel.com>
> >
> > Current 50ms max threshold timing for an EDID read is very close to
> > the actual time for a 2 block HDMI EDID read. Adjust the timings base
> > on connector type as DP reads are at 1 MBit and HDMI at 100K bit. If
> > an LSPcon is connected to device under test the -l option should be
> > passed to update the threshold timing to allow the LSPcon to read the EDID
> at the HDMI timing.
> > The -l option should be used when LSPcon is on the motherboard or if a
> > USB_C->HDMI converter is present
> >
> > V2: Adjust timings based on connector type, EDID size, and Add an
> > option to specify if an LSPcon is present.
> > V3: added bugzilla to commit message
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100047
> >
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Marta Lofstedt <marta.lofstedt@intel.com>
> > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> > ---
> >  tests/kms_sysfs_edid_timing.c | 76
> > +++++++++++++++++++++++++++++++++++--------
> >  1 file changed, 62 insertions(+), 14 deletions(-)
> >
> > diff --git a/tests/kms_sysfs_edid_timing.c
> > b/tests/kms_sysfs_edid_timing.c index 1201388..441dfee 100644
> > --- a/tests/kms_sysfs_edid_timing.c
> > +++ b/tests/kms_sysfs_edid_timing.c
> > @@ -26,21 +26,46 @@
> >  #include <fcntl.h>
> >  #include <sys/stat.h>
> >
> > -#define THRESHOLD_PER_CONNECTOR	10
> > -#define THRESHOLD_TOTAL		50
> > -#define CHECK_TIMES		15
> > +#define THRESHOLD_FOR_EMPTY_CONNECTOR	10
> > +#define THRESHOLD_PER_EDID_BLOCK		5
> > +#define HDMI_THRESHOLD_MULTIPLIER		10
> > +#define CHECK_TIMES				10
> >
> >  IGT_TEST_DESCRIPTION("This check the time we take to read the content
> > of all "
> >  		     "the possible connectors. Without the edid -
> ENXIO patch "
> >
> > "(http://permalink.gmane.org/gmane.comp.video.dri.devel/62083), "
> > -		     "we sometimes take a *really* long time. "
> > -		     "So let's just check for some reasonable
> > timing here");
> > +		     "we sometimes take a *really* long time. So
> > let's check "
> > +		     "an approximate time per edid block based on
> > connector "
> > +		     "type. The -l option adjusts DP timing to
> > reflect HDMI read "
> > +		     "timings from LSPcon.");
> > +
> > +/* The -l option has been added to correctly handle timings when an
> > +LSPcon is
> > + * present. This could be on the platform itself or in a USB_C->HDMI
> > converter.
> > + * With LSPCon EDID read timing will need to change from the 1 Mbit
> > +AUX
> > + * bus speed to the 100 Kbit HDMI DDC bus speed  */ bool
> > +lspcon_present;
> >
> > +static int opt_handler(int opt, int opt_index, void *data) {
> > +	if (opt == 'l') {
> > +		lspcon_present = true;
> > +		igt_info("set LSPcon present on DP ports\n");
> > +	}
> >
> > -igt_simple_main
> > +	return 0;
> > +}
> > +
> > +int main(int argc, char **argv)
> >  {
> >  	DIR *dirp;
> >  	struct dirent *de;
> > +	lspcon_present = false;
> > +
> > +	igt_simple_init_parse_opts(&argc, argv, "l", NULL, NULL,
> > +				      opt_handler,
> > NULL);
> > +
> > +	igt_skip_on_simulation();
> >
> >  	dirp = opendir("/sys/class/drm");
> >  	igt_assert(dirp != NULL);
> > @@ -49,17 +74,36 @@ igt_simple_main
> >  		struct igt_mean mean = {};
> >  		struct stat st;
> >  		char path[PATH_MAX];
> > -		int i;
> > +		char edid_path[PATH_MAX];
> > +		char edid[512]; /* enough for 4 block edid */
> > +		unsigned long edid_size = 0;
> > +		int i, fd_edid;
> > +		unsigned int threshold = 0;
> >
> >  		if (*de->d_name == '.')
> >  			continue;;
> >
> >  		snprintf(path, sizeof(path),
> > "/sys/class/drm/%s/status",
> >  				de->d_name);
> > +		snprintf(edid_path, sizeof(edid_path),
> > "/sys/class/drm/%s/edid",
> > +				de->d_name);
> >
> >  		if (stat(path, &st))
> >  			continue;
> >
> > +		fd_edid = open(edid_path, O_RDONLY);
> > +		if (fd_edid == -1) {
> > +			igt_warn("Read Error EDID\n");
> > +			continue;
> > +		}
> > +
> > +		edid_size = read(fd_edid, edid, 512);
> > +		threshold = THRESHOLD_PER_EDID_BLOCK *
> > (edid_size / 128);
> > +		if (lspcon_present || (edid_size > 128 &&
> > +		    !strncmp(de->d_name, "card0-HDMI", 10))) {
> > +			threshold *=
> > HDMI_THRESHOLD_MULTIPLIER;
> > +		}
> > +
> >  		igt_mean_init(&mean);
> >  		for (i = 0; i < CHECK_TIMES; i++) {
> >  			struct timespec ts = {};
> > @@ -76,22 +120,26 @@ igt_simple_main
> >  		}
> >
> >  		igt_debug("%s: mean.max %.2fns, %.2fus,
> %.2fms, "
> > -			  "mean.avg %.2fns, %.2fus,
> > %.2fms\n",
> > +			  "mean.avg %.2fns, %.2fus,
> > %.2fms, "
> > +			  "edid_size %lu, threshold %d\n",
> >  			  de->d_name,
> >  			  mean.max, mean.max / 1e3,
> > mean.max / 1e6,
> > -			  mean.mean, mean.mean / 1e3,
> > mean.mean / 1e6);
> > +			  mean.mean, mean.mean / 1e3,
> > mean.mean / 1e6,
> > +			  edid_size, threshold);
> >
> > -		if (mean.max > (THRESHOLD_PER_CONNECTOR
> > * 1e6)) {
> > +		if (edid_size == 0 &&
> > +		   (mean.max >
> > (THRESHOLD_FOR_EMPTY_CONNECTOR * 1e6))) {
> >  			igt_warn("%s: probe time exceed
> > 10ms, "
> >  				 "max=%.2fms,
> > avg=%.2fms\n", de->d_name,
> >  				 mean.max / 1e6,
> > mean.mean / 1e6);
> >  		}
> > -		igt_assert_f(mean.mean < (THRESHOLD_TOTAL
> > * 1e6),
> > -			     "%s: average probe time
> > exceeded 50ms, "
> > -			     "max=%.2fms, avg=%.2fms\n",
> > de->d_name,
> > +		if (edid_size > 0)
> > +			igt_assert_f(mean.mean <
> > (threshold * 1e6),
> > +			     "%s: average probe time
> > exceeded %dms, "
> > +			     "max=%.2fms, avg=%.2fms\n",
> > de->d_name, threshold,
> >  			     mean.max / 1e6, mean.mean /
> > 1e6);
> >
> >  	}
> >  	closedir(dirp);
> > -
> > +	igt_exit();
> >  }
> > --
> > 1.9.1
On 08/10/2017 12:00 AM, Lofstedt, Marta wrote:
> With this tweak the test pass:
> --- a/tests/kms_sysfs_edid_timing.c
> +++ b/tests/kms_sysfs_edid_timing.c
> @@ -99,7 +99,7 @@ int main(int argc, char **argv)
>   
>                  edid_size = read(fd_edid, edid, 512);
>                  threshold = THRESHOLD_PER_EDID_BLOCK * (edid_size / 128);
> -               if (lspcon_present || (edid_size > 128 &&
> +               if (lspcon_present || (edid_size >= 128 &&
Good catch. DVI monitors only have 128 byte EDIDs and connected to an 
HDMI port would not fall into the HDMI_THRESHOLD multiplier. There is no 
reason to check edid_size here.

I might need a special DVI multiplier as the 128 byte read is taking an 
average of 49.77ms when the threshold is being computed currently as 
50ms. As the testing progresses with this code we might need to make 
further tweaks to the timings. I will submit a v4 shortly.

-Clint


>                      !strncmp(de->d_name, "card0-HDMI", 10))) {
>                          threshold *= HDMI_THRESHOLD_MULTIPLIER;
>                  }
>
>> -----Original Message-----
>> From: Lofstedt, Marta
>> Sent: Thursday, August 10, 2017 9:41 AM
>> To: Taylor, Clinton A <clinton.a.taylor@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Vetter, Daniel <daniel.vetter@intel.com>
>> Subject: RE: [PATCH v3 i-g-t] tests/kms: increase max threshold time for edid
>> read
>>
>> Sorry Clinton I fail the test again on my BDW NUCi5 with this V3.
>>
>> Check: https://bugs.freedesktop.org/show_bug.cgi?id=100047
>> For details.
>>
>> /Marta
>>
>>> -----Original Message-----
>>> From: Taylor, Clinton A
>>> Sent: Thursday, August 10, 2017 1:52 AM
>>> To: intel-gfx@lists.freedesktop.org
>>> Cc: Taylor, Clinton A <clinton.a.taylor@intel.com>; Vetter, Daniel
>>> <daniel.vetter@intel.com>; Lofstedt, Marta <marta.lofstedt@intel.com>
>>> Subject: [PATCH v3 i-g-t] tests/kms: increase max threshold time for
>>> edid read
>>>
>>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>>
>>> Current 50ms max threshold timing for an EDID read is very close to
>>> the actual time for a 2 block HDMI EDID read. Adjust the timings base
>>> on connector type as DP reads are at 1 MBit and HDMI at 100K bit. If
>>> an LSPcon is connected to device under test the -l option should be
>>> passed to update the threshold timing to allow the LSPcon to read the EDID
>> at the HDMI timing.
>>> The -l option should be used when LSPcon is on the motherboard or if a
>>> USB_C->HDMI converter is present
>>>
>>> V2: Adjust timings based on connector type, EDID size, and Add an
>>> option to specify if an LSPcon is present.
>>> V3: added bugzilla to commit message
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100047
>>>
>>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>>> Cc: Marta Lofstedt <marta.lofstedt@intel.com>
>>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>>> ---
>>>   tests/kms_sysfs_edid_timing.c | 76
>>> +++++++++++++++++++++++++++++++++++--------
>>>   1 file changed, 62 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/tests/kms_sysfs_edid_timing.c
>>> b/tests/kms_sysfs_edid_timing.c index 1201388..441dfee 100644
>>> --- a/tests/kms_sysfs_edid_timing.c
>>> +++ b/tests/kms_sysfs_edid_timing.c
>>> @@ -26,21 +26,46 @@
>>>   #include <fcntl.h>
>>>   #include <sys/stat.h>
>>>
>>> -#define THRESHOLD_PER_CONNECTOR	10
>>> -#define THRESHOLD_TOTAL		50
>>> -#define CHECK_TIMES		15
>>> +#define THRESHOLD_FOR_EMPTY_CONNECTOR	10
>>> +#define THRESHOLD_PER_EDID_BLOCK		5
>>> +#define HDMI_THRESHOLD_MULTIPLIER		10
>>> +#define CHECK_TIMES				10
>>>
>>>   IGT_TEST_DESCRIPTION("This check the time we take to read the content
>>> of all "
>>>   		     "the possible connectors. Without the edid -
>> ENXIO patch "
>>> "(http://permalink.gmane.org/gmane.comp.video.dri.devel/62083),"
>>> -		     "we sometimes take a *really* long time. "
>>> -		     "So let's just check for some reasonable
>>> timing here");
>>> +		     "we sometimes take a *really* long time. So
>>> let's check "
>>> +		     "an approximate time per edid block based on
>>> connector "
>>> +		     "type. The -l option adjusts DP timing to
>>> reflect HDMI read "
>>> +		     "timings from LSPcon.");
>>> +
>>> +/* The -l option has been added to correctly handle timings when an
>>> +LSPcon is
>>> + * present. This could be on the platform itself or in a USB_C->HDMI
>>> converter.
>>> + * With LSPCon EDID read timing will need to change from the 1 Mbit
>>> +AUX
>>> + * bus speed to the 100 Kbit HDMI DDC bus speed  */ bool
>>> +lspcon_present;
>>>
>>> +static int opt_handler(int opt, int opt_index, void *data) {
>>> +	if (opt == 'l') {
>>> +		lspcon_present = true;
>>> +		igt_info("set LSPcon present on DP ports\n");
>>> +	}
>>>
>>> -igt_simple_main
>>> +	return 0;
>>> +}
>>> +
>>> +int main(int argc, char **argv)
>>>   {
>>>   	DIR *dirp;
>>>   	struct dirent *de;
>>> +	lspcon_present = false;
>>> +
>>> +	igt_simple_init_parse_opts(&argc, argv, "l", NULL, NULL,
>>> +				      opt_handler,
>>> NULL);
>>> +
>>> +	igt_skip_on_simulation();
>>>
>>>   	dirp = opendir("/sys/class/drm");
>>>   	igt_assert(dirp != NULL);
>>> @@ -49,17 +74,36 @@ igt_simple_main
>>>   		struct igt_mean mean = {};
>>>   		struct stat st;
>>>   		char path[PATH_MAX];
>>> -		int i;
>>> +		char edid_path[PATH_MAX];
>>> +		char edid[512]; /* enough for 4 block edid */
>>> +		unsigned long edid_size = 0;
>>> +		int i, fd_edid;
>>> +		unsigned int threshold = 0;
>>>
>>>   		if (*de->d_name == '.')
>>>   			continue;;
>>>
>>>   		snprintf(path, sizeof(path),
>>> "/sys/class/drm/%s/status",
>>>   				de->d_name);
>>> +		snprintf(edid_path, sizeof(edid_path),
>>> "/sys/class/drm/%s/edid",
>>> +				de->d_name);
>>>
>>>   		if (stat(path, &st))
>>>   			continue;
>>>
>>> +		fd_edid = open(edid_path, O_RDONLY);
>>> +		if (fd_edid == -1) {
>>> +			igt_warn("Read Error EDID\n");
>>> +			continue;
>>> +		}
>>> +
>>> +		edid_size = read(fd_edid, edid, 512);
>>> +		threshold = THRESHOLD_PER_EDID_BLOCK *
>>> (edid_size / 128);
>>> +		if (lspcon_present || (edid_size > 128 &&
>>> +		    !strncmp(de->d_name, "card0-HDMI", 10))) {
>>> +			threshold *=
>>> HDMI_THRESHOLD_MULTIPLIER;
>>> +		}
>>> +
>>>   		igt_mean_init(&mean);
>>>   		for (i = 0; i < CHECK_TIMES; i++) {
>>>   			struct timespec ts = {};
>>> @@ -76,22 +120,26 @@ igt_simple_main
>>>   		}
>>>
>>>   		igt_debug("%s: mean.max %.2fns, %.2fus,
>> %.2fms, "
>>> -			  "mean.avg %.2fns, %.2fus,
>>> %.2fms\n",
>>> +			  "mean.avg %.2fns, %.2fus,
>>> %.2fms, "
>>> +			  "edid_size %lu, threshold %d\n",
>>>   			  de->d_name,
>>>   			  mean.max, mean.max / 1e3,
>>> mean.max / 1e6,
>>> -			  mean.mean, mean.mean / 1e3,
>>> mean.mean / 1e6);
>>> +			  mean.mean, mean.mean / 1e3,
>>> mean.mean / 1e6,
>>> +			  edid_size, threshold);
>>>
>>> -		if (mean.max > (THRESHOLD_PER_CONNECTOR
>>> * 1e6)) {
>>> +		if (edid_size == 0 &&
>>> +		   (mean.max >
>>> (THRESHOLD_FOR_EMPTY_CONNECTOR * 1e6))) {
>>>   			igt_warn("%s: probe time exceed
>>> 10ms, "
>>>   				 "max=%.2fms,
>>> avg=%.2fms\n", de->d_name,
>>>   				 mean.max / 1e6,
>>> mean.mean / 1e6);
>>>   		}
>>> -		igt_assert_f(mean.mean < (THRESHOLD_TOTAL
>>> * 1e6),
>>> -			     "%s: average probe time
>>> exceeded 50ms, "
>>> -			     "max=%.2fms, avg=%.2fms\n",
>>> de->d_name,
>>> +		if (edid_size > 0)
>>> +			igt_assert_f(mean.mean <
>>> (threshold * 1e6),
>>> +			     "%s: average probe time
>>> exceeded %dms, "
>>> +			     "max=%.2fms, avg=%.2fms\n",
>>> de->d_name, threshold,
>>>   			     mean.max / 1e6, mean.mean /
>>> 1e6);
>>>
>>>   	}
>>>   	closedir(dirp);
>>> -
>>> +	igt_exit();
>>>   }
>>> --
>>> 1.9.1