On Fri, Oct 28, 2016 at 11:03:44AM +0100, Chris Wilson wrote:
> On Fri, Oct 28, 2016 at 12:31:29PM +0300, Marius Vlad wrote:
> > v2:
> > - don't read cached values (Chris Wilson)
> > - warn on per connector, and fail per mean (Chris Wilson)
> > 
> > These are synthetic: 5us per connector, and 600us for all (as
> > threshold).
> > 
> > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> > ---
> >  tests/Makefile.sources        |   2 +-
> >  tests/kms_sysfs_edid_timing   |  25 ----------
> >  tests/kms_sysfs_edid_timing.c | 107 ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 108 insertions(+), 26 deletions(-)
> >  delete mode 100755 tests/kms_sysfs_edid_timing
> >  create mode 100644 tests/kms_sysfs_edid_timing.c
> > 
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index b1511c6..8af455a 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -212,6 +212,7 @@ TESTS_progs = \
> >  	kms_sink_crc_basic \
> >  	prime_udl \
> >  	drv_module_reload \
> > +	kms_sysfs_edid_timing \
> >  	$(NULL)
> >  
> >  # IMPORTANT: The ZZ_ tests need to be run last!
> > @@ -222,7 +223,6 @@ TESTS_scripts_M = \
> >  TESTS_scripts = \
> >  	debugfs_emon_crash \
> >  	drv_debugfs_reader \
> > -	kms_sysfs_edid_timing \
> >  	sysfs_l3_parity \
> >  	test_rte_check \
> >  	tools_test \
> > diff --git a/tests/kms_sysfs_edid_timing b/tests/kms_sysfs_edid_timing
> > deleted file mode 100755
> > index 46ea540..0000000
> > --- a/tests/kms_sysfs_edid_timing
> > +++ /dev/null
> > @@ -1,25 +0,0 @@
> > -#!/bin/bash
> > -#
> > -# 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
> > -#
> > -
> > -DRM_LIB_ALLOW_NO_MASTER=1
> > -
> > -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> > -. $SOURCE_DIR/drm_lib.sh
> > -
> > -TIME1=$(date +%s%N)
> > -cat $(find /sys/devices/|grep drm | grep /status) > /dev/null
> > -TIME2=$(date +%s%N)
> > -
> > -# time in ms
> > -RES=$(((TIME2 - TIME1) / 1000000))
> > -
> > -if [ $RES -gt 600 ]; then
> > -	echo "Talking to outputs took ${RES}ms, something is wrong"
> > -	exit $IGT_EXIT_FAILURE
> > -fi
> > -
> > -exit $IGT_EXIT_SUCCESS
> > diff --git a/tests/kms_sysfs_edid_timing.c b/tests/kms_sysfs_edid_timing.c
> > new file mode 100644
> > index 0000000..3b8bca0
> > --- /dev/null
> > +++ b/tests/kms_sysfs_edid_timing.c
> > @@ -0,0 +1,107 @@
> > +/*
> > + * Copyright © 2016 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 AUTHORS OR COPYRIGHT HOLDERS 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.
> > + */
> > +#include "igt.h"
> > +#include "igt_debugfs.h"
> > +#include "igt_sysfs.h"
> > +#include "igt_core.h"
> > +
> > +#include <dirent.h>
> > +#include <fcntl.h>
> > +#include <sys/stat.h>
> > +#include <time.h>
> > +
> > +#define USEC_PER_SEC		(1000*1000)
> > +#define THRESHOLD_PER_CONNECTOR	5
> > +#define THRESHOLD_TOTAL		600
> > +
> > +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");
> > +
> > +static uint64_t
> > +mean(uint64_t *a, size_t a_len)
> > +{
> > +	int p;
> > +	uint64_t __sum;
> > +
> > +	for (p = 0, __sum = 0; p < a_len; p++) {
> > +		__sum += a[p];
> > +	}
> > +
> > +	return (__sum / a_len);
> > +}
> 
> See igt_stats.h: igt_mean
Alright will use that.
> 
> > +igt_simple_main
> > +{
> > +	int dir = igt_sysfs_open(-1, NULL);
> 
> Hmm, that will try to find intel - I was hoping to be a little more
> generic and do every cardN-connectorM in /sys/class/drm (not
> /sys/class/drm/card0 as sysfs_open will return)
Right, misunderstood you the first time.
> 
> > +	DIR *dirp = fdopendir(dir);
> > +	struct dirent *de;
> > +
> > +	int i, fd = 0;
> > +	int fds[32];
> > +
> > +	uint64_t elapsed, elapsed_mean;
> > +	uint64_t a[32];
> > +
> > +	struct stat st;
> > +	struct timespec start = {};
> > +
> > +	memset(fds, -1, sizeof(fds));
> > +	memset(a, -1, sizeof(a));
> > +
> > +	while ((de = readdir(dirp))) {
> > +
> > +		if (*de->d_name == '.')
> > +			continue;
> > +
> > +		if (fstatat(dir, de->d_name, &st, 0))
> > +			continue;
> > +
> > +		if (S_ISDIR(st.st_mode) && !strncmp(de->d_name, "card0-", 6)) {
> 
> sysfs_open would have reported the index as assuming card0 fails if vgem
> was loaded first, for example.
> 
> > +			int dfd = openat(dir, de->d_name, O_RDONLY);
> > +			igt_assert(dfd > 0);
> > +			fds[fd++] = dfd;
> > +		}
> > +	}
> > +	closedir(dirp);
> > +
> > +	for (i = 0; i < fd; i++) {
> > +		igt_sysfs_set(fds[i], "status", "detect");
> > +
> > +		elapsed = igt_nsec_elapsed(&start);
> > +		igt_sysfs_get(fds[i], "status");
> > +		elapsed = igt_nsec_elapsed(&start);
> > +
> > +		igt_warn_on_f(elapsed > (THRESHOLD_PER_CONNECTOR * USEC_PER_SEC),
> > +		"Threshold per connector exceeded (elapsed %lu ns)\n", elapsed);
> 
> I was suggesting we repeat the probe of each connector several times.
> 
> DIR *dir = opendir("/sys/class/drm");
> while ((de = readdir(dir)) {
> 	struct igt_mean mean = {};
> 	struct stat st;
> 	char path[128];
> 
> 	if (*de->d_name == '.') continue;
> 	snprintf(path, sizeof(path), "/sys/class/drm/%s/status", de->d_name);
> 	if (stat(path, &st)
> 		continue;
> 
> 	for (int i = 0; i < 15; i++) {
> 		struct timespec tv;
> 		int fd;
> 
> 		fd = open(path, O_WRONLY);
> 
> 		igt_nsec_elapsed(&tv);
> 		write(fd, "detect\n", 7);
> 		igt_mean_add(&mean, igt_nsec_elapsed(&tv));
> 
> 		close(fd);
> 	}
> 
> 	if (mean.max > 10e6) /* 10 milisecond */
> 		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 < 50e6,
> 		     "%s: average probe time exceeded 50ms, max=%.2fms, avg=%.2fms\n",
> 		     de->d_name, mean.max / 1e6, mean.mean / 1e6);
> }
Okay will do that.
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre