[Intel-gfx] [PATCH v4 i-g-t] tests/kms: increase max threshold time for edid read
Clint Taylor
clinton.a.taylor at intel.com
Tue Aug 15 18:04:21 UTC 2017
On 08/15/2017 12:58 AM, Daniel Vetter wrote:
> On Mon, Aug 14, 2017 at 10:21:51AM -0700, Clint Taylor wrote:
>>
>> 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.
> Ok, I think I've been proven wrong here. I guess this happens with fancy
> new DP screens that drop the internal i2c bus and just emulate it all.
>
>> 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.
> Hm, DP analyzer ... Does this also happen with real hw? If it's just DP
> analyzer I think we still should just shrug it off.
Validation has a DVI monitor connected to a BDW NUC in CI that takes
49.77ms to read a 128 Byte EDID. This is the reason V4 was submitted
with the threshold time doubled to 50ms.
-Clint
>
>> 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.
> Besides the DP analyzer, what else requires more than 25ms per EDID block?
> -Daniel
>
>>> -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