All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marius Vlad <marius.c.vlad@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	intel-gfx@lists.freedesktop.org, petri.latvala@intel.com,
	jani.nikula@linux.intel.com, daniel@ffwll.ch
Subject: Re: [PATCH i-g-t 4/4 v4] tests/kms_sysfs_edid_timing: Convert sh to C version.
Date: Sun, 30 Oct 2016 13:41:25 +0200	[thread overview]
Message-ID: <20161030114125.rbgwgcrb6hcxz46e@mcvlad-wk.rb.intel.com> (raw)
In-Reply-To: <20161028100344.GC29769@nuc-i3427.alporthouse.com>


[-- Attachment #1.1: Type: text/plain, Size: 7535 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

      reply	other threads:[~2016-10-30 11:35 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-28  9:31 [PATCH i-g-t 0/4 v4] Convert sh scripts to C variants Marius Vlad
2016-10-28  9:31 ` [PATCH i-g-t 1/4 v4] lib/{igt_sysfs, igt_aux}: Make available to other users kick_fbcon() (unbind_fbcon()), and added helpers to lib/igt_aux, lib/igt_kmod Marius Vlad
2016-11-26 15:31   ` Chris Wilson
2016-10-28  9:31 ` [PATCH i-g-t 2/4 v4] lib/igt_gvt: Make use of libkmod helpers and fix reading gvt parameter Marius Vlad
2016-10-28 10:08   ` Chris Wilson
2016-10-30 12:47     ` Marius Vlad
2016-10-30 12:59       ` Chris Wilson
2016-10-28  9:31 ` [PATCH i-g-t 3/4 v4] tests/drv_module_reload: Convert sh script to C version Marius Vlad
2016-10-28  9:48   ` Chris Wilson
2016-10-30 10:53     ` Marius Vlad
2016-10-28  9:31 ` [PATCH i-g-t 4/4 v4] tests/kms_sysfs_edid_timing: Convert sh " Marius Vlad
2016-10-28 10:03   ` Chris Wilson
2016-10-30 11:41     ` Marius Vlad [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161030114125.rbgwgcrb6hcxz46e@mcvlad-wk.rb.intel.com \
    --to=marius.c.vlad@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=petri.latvala@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.