[Intel-gfx] [PATCH i-g-t] tests/chamelium: Require HPD storm control for basic hotplug test

[Paul Kocialkowski]

On Wed, 2017-06-14 at 10:51 +0300, Paul Kocialkowski wrote:
> On Wed, 2017-06-14 at 10:47 +0300, Paul Kocialkowski wrote:
> > Hi,
> > 
> > On Tue, 2017-06-13 at 14:20 -0400, Lyude Paul wrote:
> > > NAK, igt_hpd_storm_set_threshold is expected to be called in both
> > > situations with and without hpd storm control support. The function
> > > should be able to notice when the host doesn't have the debugfs nodes
> > > for hpd storm control, and just return without doing anything in that
> > > case:
> > > 
> > > void igt_hpd_storm_set_threshold(int drm_fd, unsigned int threshold)
> > > {
> > > 	int fd = igt_debugfs_open(drm_fd, "i915_hpd_storm_ctl", O_WRONLY);
> > > 	char buf[16];
> > > 
> > > 	/* Right here is where we should be exiting if we don't have the debugfs
> > > file present */
> > > 	if (fd < 0)
> > > 		return;
> > > 
> > > 	igt_debug("Setting HPD storm threshold to %d\n", threshold);
> > > 	snprintf(buf, sizeof(buf), "%d", threshold);
> > > 	igt_assert_eq(write(fd, buf, strlen(buf)), strlen(buf));
> > > 
> > > 	close(fd);
> > > 	igt_install_exit_handler(igt_hpd_storm_exit_handler);
> > > }
> > > 
> > > The igt asserts that come after that are there because if we do have
> > > the i915_hpd_storm_ctl file but it doesn't do what we expect it to,
> > > then that's probably an error. This being said, adding a call to
> > > igt_require_hpd_storm_ctl(data->drm_fd); will result in the hotplugging
> > > tests being skipped on every non-intel platform.
> > 
> > That is the expected behavior, the test should definitely be skipped when
> > the
> > debugfs node is not available. Without requiring hpd storm ctl,
> > test_basic_hotplug will fail at reprobe_connector according to the following
> > log:
> 
> Ah wait, I just thought twice about this. This is a basic HPD test, so it
> should
>  work regardless of the storm detect feature, I suppose.
> 
> I guess this means there is another issue there then. I will look into it.

Okay, looks like this was a regression with i915 that occurs with 4.11 but was
fixed in 4.12-rc, so let's forget about this patch.

I thought the failure was related to the debugfs node, but it obviously is not.

> > IGT-Version: 1.19-g5d494bda (x86_64) (Linux: 4.11.3-1-ARCH x86_64)
> > (chamelium:523) CRITICAL: Test assertion failure function
> > test_basic_hotplug,
> > file chamelium.c:166:
> > (chamelium:523) CRITICAL: Failed assertion: reprobe_connector(data, port) ==
> > DRM_MODE_CONNECTED
> > (chamelium:523) CRITICAL: error: 2 != 1
> > Stack trace:
> >   #0 [__igt_fail_assert+0xef]
> >   #1 [test_basic_hotplug.constprop.20+0x157]
> >   #2 [__real_main588+0xe88]
> >   #3 [main+0x33]
> >   #4 [__libc_start_main+0xea]
> >   #5 [_start+0x2a]
> > Subtest dp-hpd failed.
> > **** DEBUG ****
> > (chamelium:523) igt-chamelium-DEBUG: Resetting the chamelium
> > (chamelium:523) DEBUG: Waiting for DP-1 to disconnect...
> > (chamelium:523) DEBUG: Reprobing DP-1...
> > (chamelium:523) igt-debugfs-DEBUG: Opening debugfs directory
> > '/sys/kernel/debug/dri/0'
> > (chamelium:523) igt-chamelium-DEBUG: Plugging DP-1
> > (chamelium:523) DEBUG: Reprobing DP-1...
> > (chamelium:523) CRITICAL: Test assertion failure function
> > test_basic_hotplug,
> > file chamelium.c:166:
> > (chamelium:523) CRITICAL: Failed assertion: reprobe_connector(data, port) ==
> > DRM_MODE_CONNECTED
> > (chamelium:523) CRITICAL: error: 2 != 1
> > ****  END  ****
> > Subtest dp-hpd: FAIL (1.383s)
> > Test requirement not met in function require_connector_present, file
> > chamelium.c:99:
> > Test requirement: found
> > No port of type HDMI-A was found
> > Test requirement not met in function require_connector_present, file
> > chamelium.c:99:
> > Test requirement: found
> > No port of type VGA was found
> > 
> > > Are these tests no longer working on systems without hpd storm control?
> > > The last time I tested it I didn't see any issues, but that was a while
> > > ago.
> > > 
> > > On Tue, 2017-06-13 at 15:46 +0300, Paul Kocialkowski wrote:
> > > > On Tue, 2017-06-13 at 15:44 +0300, Paul Kocialkowski wrote:
> > > > > The basic hotplug test (test_basic_hotplug) makes calls to
> > > > > igt_hpd_storm_set_threshold, which requires HPD storm control.
> > > > > 
> > > > > This adds a check for that control, since it was missing.
> > > > 
> > > > This is a resend of yesterday's patch with the right subject prefix
> > > > so that
> > > > patchwork can pick it up.
> > > > 
> > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski at linux.intel.com
> > > > > > 
> > > > > 
> > > > > ---
> > > > >  tests/chamelium.c | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > > 
> > > > > diff --git a/tests/chamelium.c b/tests/chamelium.c
> > > > > index 2e14ccb9..bd7772f8 100644
> > > > > --- a/tests/chamelium.c
> > > > > +++ b/tests/chamelium.c
> > > > > @@ -153,6 +153,8 @@ test_basic_hotplug(data_t *data, struct
> > > > > chamelium_port
> > > > > *port)
> > > > >  	struct udev_monitor *mon = igt_watch_hotplug();
> > > > >  	int i;
> > > > >  
> > > > > +	igt_require_hpd_storm_ctl(data->drm_fd);
> > > > > +
> > > > >  	reset_state(data, port);
> > > > >  	igt_hpd_storm_set_threshold(data->drm_fd, 0);
> > > > >  
-- 
Paul Kocialkowski <paul.kocialkowski at linux.intel.com>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo