On 08/11/2017 12:49 AM, Lofstedt, Marta wrote:
The test would need to know if an LSPcon is connected on a port by port basis. This is easy if the LSPcon driver is loaded but in the case of USB_C->HDMI is gets a little more complicated (not impossible) to figure out. Even if we know exactly what device is connected failures can still occur if the TCON/Monitor clock stretches the EDID read.-----Original Message----- From: Taylor, Clinton A Sent: Thursday, August 10, 2017 8:50 PM To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx Cc: Taylor, Clinton A <clinton.a.taylor@xxxxxxxxx>; Vetter, Daniel <daniel.vetter@xxxxxxxxx>; Lofstedt, Marta <marta.lofstedt@xxxxxxxxx> Subject: [PATCH v4 i-g-t] tests/kms: increase max threshold time for edid read From: Clint Taylor <clinton.a.taylor@xxxxxxxxx> 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: Add bugzilla to commit message V4: remove edid_size check from HDMI multiplier. Fixes DVI on HDMI. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100047 Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx> Cc: Marta Lofstedt <marta.lofstedt@xxxxxxxxx> Signed-off-by: Clint Taylor <clinton.a.taylor@xxxxxxxxx> --- tests/kms_sysfs_edid_timing.c | 74 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 60 insertions(+), 14 deletions(-) diff --git a/tests/kms_sysfs_edid_timing.c b/tests/kms_sysfs_edid_timing.c index 1201388..0382610 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);We can't have this lspcon as an argument to the test, it will not work with automated testing using piglit as we do for CI. Theoretically we could add a "has_lspcon" to debugfs, but I believe that this test has started to be over complicated.
Unfortunately with the timing differences (3ms to 96ms) based on the monitor type connected and EDID size there is no way for a one size fits all sanity check to be reliable. If the original point of this test was to figure out if probe caused more than 1 EDID read, maybe we should actually count EDID reads and not infer it by measuring time.The intention of the test is to do a sanity check so that we don't drift off here, so I actually prefer the V1.
-Clint
Your detailed work could however be used in a benchmark, where the result would be the actually timing, I am sure there is a performance team who would like that. /Marta+ + igt_skip_on_simulation(); dirp = opendir("/sys/class/drm"); igt_assert(dirp != NULL); @@ -49,17 +74,34 @@ 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 || !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 +118,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
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
