[PATCH i-g-t v2] tests/chamelium: Add common suspend and hibernate tests for DP and HDMI

[PATCH i-g-t v2] tests/chamelium: Add common suspend and hibernate tests for DP and HDMI

[Paul Kocialkowski]

This adds two new tests: common-hpd-after-suspend and
common-hpd-after-hibernate that are aimed at testing HPD change during
suspend/hibernate for both DP and HDMI, at the same time.

The interest in bringing this test up is to reduce the time spent in
testing, with the downside of less precision regarding the test's
outcome. The per-connector tests are still available to get a more
precise idea of which connector causes a failure, when that happens.

VGA is explicitly excluded from this test as there is currently no
way of doing delayed hpd pulses for it.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
---
 tests/chamelium.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/tests/chamelium.c b/tests/chamelium.c
index 76b14e95..d569c8ee 100644
--- a/tests/chamelium.c
+++ b/tests/chamelium.c
@@ -241,6 +241,80 @@ test_suspend_resume_hpd(data_t *data, struct chamelium_port *port,
 }
 
 static void
+test_suspend_resume_hpd_common(data_t *data, enum igt_suspend_state state,
+			       enum igt_suspend_test test)
+{
+	struct udev_monitor *mon = igt_watch_hotplug();
+	struct chamelium_port *port;
+	int p;
+
+	for (p = 0; p < data->port_count; p++) {
+		port = data->ports[p];
+		if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
+			continue;
+
+		reset_state(data, port);
+
+		igt_debug("Testing port %s\n", chamelium_port_get_name(port));
+	}
+
+	igt_set_autoresume_delay(SUSPEND_RESUME_DELAY);
+	igt_flush_hotplugs(mon);
+
+	/* Make sure we notice new connectors after resuming */
+	for (p = 0; p < data->port_count; p++) {
+		port = data->ports[p];
+		if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
+			continue;
+
+		chamelium_async_hpd_pulse_start(data->chamelium, port, false,
+						SUSPEND_RESUME_DELAY / 2);
+	}
+
+	igt_system_suspend_autoresume(state, test);
+	chamelium_async_hpd_pulse_finish(data->chamelium);
+
+	igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
+
+	for (p = 0; p < data->port_count; p++) {
+		port = data->ports[p];
+		if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
+			continue;
+
+		igt_assert_eq(reprobe_connector(data, port),
+			      DRM_MODE_CONNECTED);
+	}
+
+	igt_flush_hotplugs(mon);
+
+	/* Now make sure we notice disconnected connectors after resuming */
+	for (p = 0; p < data->port_count; p++) {
+		port = data->ports[p];
+		if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
+			continue;
+
+		chamelium_async_hpd_pulse_start(data->chamelium, port, true,
+						SUSPEND_RESUME_DELAY / 2);
+	}
+
+	igt_system_suspend_autoresume(state, test);
+	chamelium_async_hpd_pulse_finish(data->chamelium);
+
+	igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
+
+	for (p = 0; p < data->port_count; p++) {
+		port = data->ports[p];
+		if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
+			continue;
+
+		igt_assert_eq(reprobe_connector(data, port),
+			      DRM_MODE_DISCONNECTED);
+	}
+
+	igt_cleanup_hotplug(mon);
+}
+
+static void
 test_suspend_resume_edid_change(data_t *data, struct chamelium_port *port,
 				enum igt_suspend_state state,
 				enum igt_suspend_test test,
@@ -750,6 +824,18 @@ igt_main
 			test_hpd_without_ddc(&data, port);
 	}
 
+	igt_subtest_group {
+		igt_subtest("common-hpd-after-suspend")
+			test_suspend_resume_hpd_common(&data,
+						       SUSPEND_STATE_MEM,
+						       SUSPEND_TEST_NONE);
+
+		igt_subtest("common-hpd-after-hibernate")
+			test_suspend_resume_hpd_common(&data,
+						       SUSPEND_STATE_DISK,
+						       SUSPEND_TEST_DEVICES);
+	}
+
 	igt_fixture {
 		close(data.drm_fd);
 	}
-- 
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 v2] tests/chamelium: Add common suspend and hibernate tests for DP and HDMI

[Lyude Paul]

On Fri, 2017-06-16 at 17:17 +0300, Paul Kocialkowski wrote:
> This adds two new tests: common-hpd-after-suspend and
> common-hpd-after-hibernate that are aimed at testing HPD change during
> suspend/hibernate for both DP and HDMI, at the same time.
> 
> The interest in bringing this test up is to reduce the time spent in
> testing, with the downside of less precision regarding the test's
> outcome. The per-connector tests are still available to get a more
> precise idea of which connector causes a failure, when that happens.
> 
> VGA is explicitly excluded from this test as there is currently no
> way of doing delayed hpd pulses for it.
> 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> ---
>  tests/chamelium.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
> 
> diff --git a/tests/chamelium.c b/tests/chamelium.c
> index 76b14e95..d569c8ee 100644
> --- a/tests/chamelium.c
> +++ b/tests/chamelium.c
> @@ -241,6 +241,80 @@ test_suspend_resume_hpd(data_t *data, struct chamelium_port *port,
>  }
>  
>  static void
> +test_suspend_resume_hpd_common(data_t *data, enum igt_suspend_state state,
> +			       enum igt_suspend_test test)
> +{
> +	struct udev_monitor *mon = igt_watch_hotplug();
> +	struct chamelium_port *port;
> +	int p;
> +
> +	for (p = 0; p < data->port_count; p++) {
> +		port = data->ports[p];
> +		if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
> +			continue;
> +
> +		reset_state(data, port);
> +
> +		igt_debug("Testing port %s\n", chamelium_port_get_name(port));
> +	}
> +
> +	igt_set_autoresume_delay(SUSPEND_RESUME_DELAY);
> +	igt_flush_hotplugs(mon);
> +
> +	/* Make sure we notice new connectors after resuming */
> +	for (p = 0; p < data->port_count; p++) {
> +		port = data->ports[p];
> +		if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
> +			continue;
> +
> +		chamelium_async_hpd_pulse_start(data->chamelium, port, false,
> +						SUSPEND_RESUME_DELAY / 2);
> +	}
> +
> +	igt_system_suspend_autoresume(state, test);
> +	chamelium_async_hpd_pulse_finish(data->chamelium);
> +
> +	igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
> +
> +	for (p = 0; p < data->port_count; p++) {
> +		port = data->ports[p];
> +		if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
> +			continue;
> +
> +		igt_assert_eq(reprobe_connector(data, port),
> +			      DRM_MODE_CONNECTED);
> +	}
> +
> +	igt_flush_hotplugs(mon);
> +
> +	/* Now make sure we notice disconnected connectors after resuming */
> +	for (p = 0; p < data->port_count; p++) {
> +		port = data->ports[p];
> +		if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
> +			continue;
> +
> +		chamelium_async_hpd_pulse_start(data->chamelium, port, true,
> +						SUSPEND_RESUME_DELAY / 2);
> +	}
> +
> +	igt_system_suspend_autoresume(state, test);
> +	chamelium_async_hpd_pulse_finish(data->chamelium);
> +
> +	igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
> +
> +	for (p = 0; p < data->port_count; p++) {
> +		port = data->ports[p];
> +		if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
> +			continue;
> +
> +		igt_assert_eq(reprobe_connector(data, port),
> +			      DRM_MODE_DISCONNECTED);
> +	}
> +
> +	igt_cleanup_hotplug(mon);
> +}
> +
> +static void

For smaller bits of code this isn't a big deal since we're talking
about tests here, but this is a lot of code to have for a single
function like this. Mind splitting this into something like (feel free
to take some liberty with the function naming)

test_suspend_resume_hpd(data_t *data, enum igt_suspend_state state, enum igt_suspend_test test)
  → try_suspend_resume_hpd(data_t *data, enum igt_suspend_state state, enum igt_suspend_test test, bool connected)

Other then that, looks good. With the changes mentioned:

Reviewed-by: Lyude Paul <lyude@redhat.com>

>  test_suspend_resume_edid_change(data_t *data, struct chamelium_port *port,
>  				enum igt_suspend_state state,
>  				enum igt_suspend_test test,
> @@ -750,6 +824,18 @@ igt_main
>  			test_hpd_without_ddc(&data, port);
>  	}
>  
> +	igt_subtest_group {
> +		igt_subtest("common-hpd-after-suspend")
> +			test_suspend_resume_hpd_common(&data,
> +						       SUSPEND_STATE_MEM,
> +						       SUSPEND_TEST_NONE);
> +
> +		igt_subtest("common-hpd-after-hibernate")
> +			test_suspend_resume_hpd_common(&data,
> +						       SUSPEND_STATE_DISK,
> +						       SUSPEND_TEST_DEVICES);
> +	}
> +
>  	igt_fixture {
>  		close(data.drm_fd);
>  	}
-- 
Cheers,
	Lyude
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v2] tests/chamelium: Add common suspend and hibernate tests for DP and HDMI

[Paul Kocialkowski]

Hi,

On Fri, 2017-06-16 at 19:28 -0400, Lyude Paul wrote:
> On Fri, 2017-06-16 at 17:17 +0300, Paul Kocialkowski wrote:
> > This adds two new tests: common-hpd-after-suspend and
> > common-hpd-after-hibernate that are aimed at testing HPD change during
> > suspend/hibernate for both DP and HDMI, at the same time.
> > 
> > The interest in bringing this test up is to reduce the time spent in
> > testing, with the downside of less precision regarding the test's
> > outcome. The per-connector tests are still available to get a more
> > precise idea of which connector causes a failure, when that happens.
> > 
> > VGA is explicitly excluded from this test as there is currently no
> > way of doing delayed hpd pulses for it.
> > 
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> > 
> > ---
> >  tests/chamelium.c | 86
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 86 insertions(+)
> > 
> > diff --git a/tests/chamelium.c b/tests/chamelium.c
> > index 76b14e95..d569c8ee 100644
> > --- a/tests/chamelium.c
> > +++ b/tests/chamelium.c
> > @@ -241,6 +241,80 @@ test_suspend_resume_hpd(data_t *data, struct
> > chamelium_port *port,
> >  }
> >  
> >  static void
> > +test_suspend_resume_hpd_common(data_t *data, enum igt_suspend_state state,
> > +			       enum igt_suspend_test test)
> > +{
> > +	struct udev_monitor *mon = igt_watch_hotplug();
> > +	struct chamelium_port *port;
> > +	int p;
> > +
> > +	for (p = 0; p < data->port_count; p++) {
> > +		port = data->ports[p];
> > +		if (chamelium_port_get_type(port) ==
> > DRM_MODE_CONNECTOR_VGA)
> > +			continue;
> > +
> > +		reset_state(data, port);
> > +
> > +		igt_debug("Testing port %s\n",
> > chamelium_port_get_name(port));
> > +	}
> > +
> > +	igt_set_autoresume_delay(SUSPEND_RESUME_DELAY);
> > +	igt_flush_hotplugs(mon);
> > +
> > +	/* Make sure we notice new connectors after resuming */
> > +	for (p = 0; p < data->port_count; p++) {
> > +		port = data->ports[p];
> > +		if (chamelium_port_get_type(port) ==
> > DRM_MODE_CONNECTOR_VGA)
> > +			continue;
> > +
> > +		chamelium_async_hpd_pulse_start(data->chamelium, port,
> > false,
> > +						SUSPEND_RESUME_DELAY / 2);
> > +	}
> > +
> > +	igt_system_suspend_autoresume(state, test);
> > +	chamelium_async_hpd_pulse_finish(data->chamelium);
> > +
> > +	igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
> > +
> > +	for (p = 0; p < data->port_count; p++) {
> > +		port = data->ports[p];
> > +		if (chamelium_port_get_type(port) ==
> > DRM_MODE_CONNECTOR_VGA)
> > +			continue;
> > +
> > +		igt_assert_eq(reprobe_connector(data, port),
> > +			      DRM_MODE_CONNECTED);
> > +	}
> > +
> > +	igt_flush_hotplugs(mon);
> > +
> > +	/* Now make sure we notice disconnected connectors after resuming
> > */
> > +	for (p = 0; p < data->port_count; p++) {
> > +		port = data->ports[p];
> > +		if (chamelium_port_get_type(port) ==
> > DRM_MODE_CONNECTOR_VGA)
> > +			continue;
> > +
> > +		chamelium_async_hpd_pulse_start(data->chamelium, port,
> > true,
> > +						SUSPEND_RESUME_DELAY / 2);
> > +	}
> > +
> > +	igt_system_suspend_autoresume(state, test);
> > +	chamelium_async_hpd_pulse_finish(data->chamelium);
> > +
> > +	igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
> > +
> > +	for (p = 0; p < data->port_count; p++) {
> > +		port = data->ports[p];
> > +		if (chamelium_port_get_type(port) ==
> > DRM_MODE_CONNECTOR_VGA)
> > +			continue;
> > +
> > +		igt_assert_eq(reprobe_connector(data, port),
> > +			      DRM_MODE_DISCONNECTED);
> > +	}
> > +
> > +	igt_cleanup_hotplug(mon);
> > +}
> > +
> > +static void
> 
> For smaller bits of code this isn't a big deal since we're talking
> about tests here, but this is a lot of code to have for a single
> function like this. Mind splitting this into something like (feel free
> to take some liberty with the function naming)

That is quite right, this is indeed starting to look too big. Thanks for
pointing it out. I'll send v2 with a dedicated function as suggested.

> test_suspend_resume_hpd(data_t *data, enum igt_suspend_state state, enum
> igt_suspend_test test)
>   → try_suspend_resume_hpd(data_t *data, enum igt_suspend_state state, enum
> igt_suspend_test test, bool connected)
> 
> Other then that, looks good. With the changes mentioned:
> 
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> 
> >  test_suspend_resume_edid_change(data_t *data, struct chamelium_port *port,
> >  				enum igt_suspend_state state,
> >  				enum igt_suspend_test test,
> > @@ -750,6 +824,18 @@ igt_main
> >  			test_hpd_without_ddc(&data, port);
> >  	}
> >  
> > +	igt_subtest_group {
> > +		igt_subtest("common-hpd-after-suspend")
> > +			test_suspend_resume_hpd_common(&data,
> > +						       SUSPEND_STATE_MEM,
> > +						       SUSPEND_TEST_NONE);
> > +
> > +		igt_subtest("common-hpd-after-hibernate")
> > +			test_suspend_resume_hpd_common(&data,
> > +						       SUSPEND_STATE_DISK,
> > +						       SUSPEND_TEST_DEVICES
> > );
> > +	}
> > +
> >  	igt_fixture {
> >  		close(data.drm_fd);
> >  	}
-- 
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