[Intel-gfx] [PATCH v4 i-g-t] tests/kms: increase max threshold time for edid read

Clint Taylor clinton.a.taylor at intel.com
Mon Aug 14 17:21:51 UTC 2017



On 08/14/2017 07:40 AM, Daniel Vetter wrote:
> On Thu, Aug 10, 2017 at 10:50:19AM -0700, clinton.a.taylor at intel.com wrote:
>> From: Clint Taylor <clinton.a.taylor at 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: 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 at intel.com>
>> Cc: Marta Lofstedt <marta.lofstedt at intel.com>
>> Signed-off-by: Clint Taylor <clinton.a.taylor at intel.com>
>> ---
>>   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
> This is blantantly wrong. EDID reads are done at 100kbit, even over dp
> aux. There's some panels which have forgoe the i2c bus that
> i2c-over-dp-aux drivers and givey ou the EDID a bit faster, but edids read
> at 100kbit.

Actually most modern devices deliver DP EDID much faster than 100kbit. I 
have several 4K DP monitors that deliver 256 bytes EDID in < 8ms. eDP 
panels deliver 128 bytes in <4 ms.
I also have seen devices that takes almost 50ms to deliver a 128 byte 
EDID block.

 From kms_sysfs_edid_timing to a Asus MG24UQ monitor:
card0-DP-3: mean.max 7758610.00ns, 7758.61us, 7.76ms, mean.avg 
7758610.00ns, 7758.61us, 7.76ms, edid_size 256, threshold 10

showing < 8ms to read 2 blocks of EDID.

 From kms_sysfs_edid_timing connected to a QD980B DP analyzer:
card0-DP-3: mean.max 74251273.00ns, 74251.27us, 74.25ms, mean.avg 
74251273.00ns, 74251.27us, 74.25ms, edid_size 256, threshold 10

showing > 74ms to read 2 blocks of EDID.

These times were confirmed with a Unigraf DPA400 to confirm both timing 
and that 256 bytes of data were transferred.

>
> That means 25ms per edid block and no special cases anywhere. I think you
> can still keep the 10ms for basic probe (but really shouldn't be needed).
>
> Note your limits are off by 2x, since you use 50ms per 128b edid block. We
> can totally read a CEA edid with 2 blocks in 50ms.
A single value like 25ms has been proven not to be enough time depending 
on the downstream device and connector type.
> -Daniel
>
>> + */
>> +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,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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



More information about the Intel-gfx mailing list