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

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

[Paul Kocialkowski]

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.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@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);
 
-- 
2.13.1

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

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

[Paul Kocialkowski]

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@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@linux.intel.com>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Lyude Paul]

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.

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@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);
> >  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Paul Kocialkowski]

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:

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@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@linux.intel.com>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Paul Kocialkowski]

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.

> 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@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@linux.intel.com>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@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@linux.intel.com>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx