[PATCH i-g-t 1/5] lib/igt_chamelium: Add support for HPD toggle scheduling

[PATCH i-g-t 1/5] lib/igt_chamelium: Add support for HPD toggle scheduling

[Paul Kocialkowski]

This adds support for the newly-introduced ScheduleHpdToggle XMLRPC
method of the Chamelium's interface. It allows scheduling an HPD toggle,
for which the call will return immediately.

This is especially useful for testing HPD during suspend/resume.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
---
 lib/igt_chamelium.c | 22 ++++++++++++++++++++++
 lib/igt_chamelium.h |  3 +++
 2 files changed, 25 insertions(+)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index 4a2af796..523cc853 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -447,6 +447,28 @@ void chamelium_fire_mixed_hpd_pulses(struct chamelium *chamelium,
 	xmlrpc_DECREF(pulse_widths);
 }
 
+/**
+ * chamelium_schedule_hpd_toggle:
+ * @chamelium: The Chamelium instance to use
+ * @port: The port to fire the HPD pulses on
+ * @delay_ms: Delay in milli-second before the toggle takes place
+ * @rising_edge: Whether the toggle should be a rising edge or a falling edge
+ *
+ * Instructs the chamelium to schedule an hpd toggle (either a rising edge or
+ * a falling edge, depending on @rising_edg) after @delay_ms have passed.
+ * This is useful for testing things such as hpd after a suspend/resume cycle.
+ */
+void chamelium_schedule_hpd_toggle(struct chamelium *chamelium,
+				   struct chamelium_port *port, int delay_ms,
+				   bool rising_edge)
+{
+	igt_debug("Scheduling HPD toggle on %s in %d ms\n", port->name,
+		  delay_ms);
+
+	xmlrpc_DECREF(chamelium_rpc(chamelium, NULL, "ScheduleHpdToggle",
+				    "(iii)", port->id, delay_ms, rising_edge));
+}
+
 static void async_rpc_handler(const char *server_url, const char *method_name,
 			      xmlrpc_value *param_array, void *user_data,
 			      xmlrpc_env *fault, xmlrpc_value *result)
diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
index 15f60246..408a4b85 100644
--- a/lib/igt_chamelium.h
+++ b/lib/igt_chamelium.h
@@ -61,6 +61,9 @@ void chamelium_fire_mixed_hpd_pulses(struct chamelium *chamelium,
 void chamelium_fire_hpd_pulses(struct chamelium *chamelium,
 			       struct chamelium_port *port,
 			       int width_msec, int count);
+void chamelium_schedule_hpd_toggle(struct chamelium *chamelium,
+				   struct chamelium_port *port, int delay_ms,
+				   bool rising_edge);
 void chamelium_async_hpd_pulse_start(struct chamelium *chamelium,
 				     struct chamelium_port *port,
 				     bool high, int delay_secs);
-- 
2.13.1

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

[PATCH i-g-t 2/5] tests/chamelium: Use HPD toggle scheduling instead of async pulses

[Paul Kocialkowski]

This switches over to HPD toggle scheduling instead of starting pulses
with an asynchronous call, suspending and dealing with the result at
resume.

The XMLRPC library does not guarantee that the call will be made before
caring for its outcome and this is in fact what was happening:
the call was being delayed until resume time, as can be seen from the
Chamelium's logs. The quite generous timeout for HPD event detection
would then catch the toggle, that was sent after resume.

Thus, HPD toggle during suspend was not properly tested before and this
changes fixes that.

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

diff --git a/tests/chamelium.c b/tests/chamelium.c
index 3cf9f3b5..c7114987 100644
--- a/tests/chamelium.c
+++ b/tests/chamelium.c
@@ -226,25 +226,24 @@ try_suspend_resume_hpd(data_t *data, struct chamelium_port *port,
 	igt_flush_hotplugs(mon);
 
 	if (port) {
-		chamelium_async_hpd_pulse_start(data->chamelium, port,
-						connected,
-						SUSPEND_RESUME_DELAY / 2);
+		chamelium_schedule_hpd_toggle(data->chamelium, port,
+					      SUSPEND_RESUME_DELAY * 1000 / 2,
+					      !connected);
 	} else {
 		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,
-							connected,
-							SUSPEND_RESUME_DELAY / 2);
+			chamelium_schedule_hpd_toggle(data->chamelium, port,
+						      SUSPEND_RESUME_DELAY * 1000 / 2,
+						      !connected);
 		}
 
 		port = NULL;
 	}
 
 	igt_system_suspend_autoresume(state, test);
-	chamelium_async_hpd_pulse_finish(data->chamelium);
 
 	igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
 	if (port) {
-- 
2.13.1

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

[PATCH i-g-t 3/5] lib/igt_chamelium: Get rid of async hpd pulse functions

[Paul Kocialkowski]

This removes the functions related to async hpd pulse handling, that
were used for hpd toggle during suspend testing. Since the XMLRPC
library does not guarantee that the call will be made before caring
for its result, this function did not work properly.

These functions are deprecated in favor of HPD toggle scheduling, that
was recently added to the Chamelium's XMLRPC API and IGT.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
---
 lib/igt_chamelium.c | 72 -----------------------------------------------------
 lib/igt_chamelium.h |  4 ---
 2 files changed, 76 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index 523cc853..225f98c3 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -469,78 +469,6 @@ void chamelium_schedule_hpd_toggle(struct chamelium *chamelium,
 				    "(iii)", port->id, delay_ms, rising_edge));
 }
 
-static void async_rpc_handler(const char *server_url, const char *method_name,
-			      xmlrpc_value *param_array, void *user_data,
-			      xmlrpc_env *fault, xmlrpc_value *result)
-{
-	/* We don't care about the responses */
-}
-
-/**
- * chamelium_async_hpd_pulse_start:
- * @chamelium: The Chamelium instance to use
- * @port: The port to fire the HPD pulses on
- * @high: Whether to fire a high pulse (e.g. simulate a connect), or a low
- * pulse (e.g. simulate a disconnect)
- * @delay_secs: How long to wait before sending the HPD pulse.
- *
- * Instructs the chamelium to send an hpd pulse after @delay_secs seconds have
- * passed, without waiting for the chamelium to finish. This is useful for
- * testing things such as hpd after a suspend/resume cycle, since we can't tell
- * the chamelium to send a hotplug at the same time that our system is
- * suspended.
- *
- * It is required that the user eventually call
- * #chamelium_async_hpd_pulse_finish, to clean up the leftover XML-RPC
- * responses from the chamelium.
- */
-void chamelium_async_hpd_pulse_start(struct chamelium *chamelium,
-				     struct chamelium_port *port,
-				     bool high, int delay_secs)
-{
-	xmlrpc_value *pulse_widths = xmlrpc_array_new(&chamelium->env), *width;
-
-	/* TODO: Actually implement something in the chameleon server to allow
-	 * for delayed actions such as hotplugs. This would work a bit better
-	 * and allow us to test suspend/resume on ports without hpd like VGA
-	 */
-
-	igt_debug("Sending HPD pulse (%s) on %s with %d second delay\n",
-		  high ? "high->low" : "low->high", port->name, delay_secs);
-
-	/* If we're starting at high, make the first pulse width 0 so we keep
-	 * the port connected */
-	if (high) {
-		width = xmlrpc_int_new(&chamelium->env, 0);
-		xmlrpc_array_append_item(&chamelium->env, pulse_widths, width);
-		xmlrpc_DECREF(width);
-	}
-
-	width = xmlrpc_int_new(&chamelium->env, delay_secs * 1000);
-	xmlrpc_array_append_item(&chamelium->env, pulse_widths, width);
-	xmlrpc_DECREF(width);
-
-	xmlrpc_client_start_rpcf(&chamelium->env, chamelium->client,
-				 chamelium->url,
-				 "FireMixedHpdPulses", async_rpc_handler, NULL,
-				 "(iA)", port->id, pulse_widths);
-	xmlrpc_DECREF(pulse_widths);
-}
-
-/**
- * chamelium_async_hpd_pulse_finish:
- * @chamelium: The Chamelium instance to use
- *
- * Waits for any asynchronous RPC started by #chamelium_async_hpd_pulse_start
- * to complete, and then cleans up any leftover responses from the chamelium.
- * If all of the RPC calls have already completed, this function returns
- * immediately.
- */
-void chamelium_async_hpd_pulse_finish(struct chamelium *chamelium)
-{
-	xmlrpc_client_event_loop_finish(chamelium->client);
-}
-
 /**
  * chamelium_new_edid:
  * @chamelium: The Chamelium instance to use
diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
index 408a4b85..81322ad2 100644
--- a/lib/igt_chamelium.h
+++ b/lib/igt_chamelium.h
@@ -64,10 +64,6 @@ void chamelium_fire_hpd_pulses(struct chamelium *chamelium,
 void chamelium_schedule_hpd_toggle(struct chamelium *chamelium,
 				   struct chamelium_port *port, int delay_ms,
 				   bool rising_edge);
-void chamelium_async_hpd_pulse_start(struct chamelium *chamelium,
-				     struct chamelium_port *port,
-				     bool high, int delay_secs);
-void chamelium_async_hpd_pulse_finish(struct chamelium *chamelium);
 int chamelium_new_edid(struct chamelium *chamelium, const unsigned char *edid);
 void chamelium_port_set_edid(struct chamelium *chamelium,
 			     struct chamelium_port *port, int edid_id);
-- 
2.13.1

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

[PATCH i-g-t 4/5] tests/chamelium: Test HPD toggle after suspend and hibernate for VGA

[Paul Kocialkowski]

This adds HPD toggle after suspend and hibernate testing for VGA, in the
same fashion as currently done for DP and HDMI.
---
 tests/chamelium.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tests/chamelium.c b/tests/chamelium.c
index c7114987..07dc4849 100644
--- a/tests/chamelium.c
+++ b/tests/chamelium.c
@@ -810,10 +810,15 @@ igt_main
 				       igt_kms_get_alt_edid());
 		}
 
-		/* FIXME: Right now there isn't a way to do any sort of delayed
-		 * psuedo-hotplug with VGA, so testing detection after a
-		 * suspend/resume cycle isn't possible yet
-		 */
+		connector_subtest("vga-hpd-after-suspend", VGA)
+			test_suspend_resume_hpd(&data, port,
+						SUSPEND_STATE_MEM,
+						SUSPEND_TEST_NONE);
+
+		connector_subtest("vga-hpd-after-hibernate", VGA)
+			test_suspend_resume_hpd(&data, port,
+						SUSPEND_STATE_DISK,
+						SUSPEND_TEST_DEVICES);
 
 		connector_subtest("vga-hpd-without-ddc", VGA)
 			test_hpd_without_ddc(&data, port);
-- 
2.13.1

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

[PATCH i-g-t 5/5] tests/chamelium: Also test VGA for common suspend and hibernate tests

[Paul Kocialkowski]

Now that HPD toggle during suspend and hibernate can be tested for VGA
as well, there is no need to exclude it in the common tests.

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

diff --git a/tests/chamelium.c b/tests/chamelium.c
index 07dc4849..b7eaa2fa 100644
--- a/tests/chamelium.c
+++ b/tests/chamelium.c
@@ -232,9 +232,6 @@ try_suspend_resume_hpd(data_t *data, struct chamelium_port *port,
 	} else {
 		for (p = 0; p < data->port_count; p++) {
 			port = data->ports[p];
-			if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
-				continue;
-
 			chamelium_schedule_hpd_toggle(data->chamelium, port,
 						      SUSPEND_RESUME_DELAY * 1000 / 2,
 						      !connected);
@@ -252,9 +249,6 @@ try_suspend_resume_hpd(data_t *data, struct chamelium_port *port,
 	} else {
 		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), connected ?
 				      DRM_MODE_DISCONNECTED :
 				      DRM_MODE_CONNECTED);
@@ -292,9 +286,6 @@ test_suspend_resume_hpd_common(data_t *data, enum igt_suspend_state state,
 
 	for (p = 0; p < data->port_count; p++) {
 		port = data->ports[p];
-		if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
-			continue;
-
 		igt_debug("Testing port %s\n", chamelium_port_get_name(port));
 	}
 
-- 
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 1/5] lib/igt_chamelium: Add support for HPD toggle scheduling

[Lyude Paul]

This all looks great! I'm happy to see us finally getting rid of the
very hacky workaround I had to make async hotplugging work (to be fair,
it was only partly broken ;). Thanks for dealing with chameleond
upstream too, I couldn't stand how ugly the code in that thing is...

Anyway, this whole patchset looks good. The only thing I ask is that
you condense the patches a bit, mainly:

1-3 can all be one patch for adding real async HPD
4-5 can be another single patch for enabling VGA tests

On Mon, 2017-06-26 at 10:22 +0300, Paul Kocialkowski wrote:
> This adds support for the newly-introduced ScheduleHpdToggle XMLRPC
> method of the Chamelium's interface. It allows scheduling an HPD
> toggle,
> for which the call will return immediately.
> 
> This is especially useful for testing HPD during suspend/resume.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> ---
>  lib/igt_chamelium.c | 22 ++++++++++++++++++++++
>  lib/igt_chamelium.h |  3 +++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index 4a2af796..523cc853 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -447,6 +447,28 @@ void chamelium_fire_mixed_hpd_pulses(struct
> chamelium *chamelium,
>  	xmlrpc_DECREF(pulse_widths);
>  }
>  
> +/**
> + * chamelium_schedule_hpd_toggle:
> + * @chamelium: The Chamelium instance to use
> + * @port: The port to fire the HPD pulses on
> + * @delay_ms: Delay in milli-second before the toggle takes place
> + * @rising_edge: Whether the toggle should be a rising edge or a
> falling edge
> + *
> + * Instructs the chamelium to schedule an hpd toggle (either a
> rising edge or
> + * a falling edge, depending on @rising_edg) after @delay_ms have
> passed.
> + * This is useful for testing things such as hpd after a
> suspend/resume cycle.
> + */
> +void chamelium_schedule_hpd_toggle(struct chamelium *chamelium,
> +				   struct chamelium_port *port, int
> delay_ms,
> +				   bool rising_edge)
> +{
> +	igt_debug("Scheduling HPD toggle on %s in %d ms\n", port-
> >name,
> +		  delay_ms);
> +
> +	xmlrpc_DECREF(chamelium_rpc(chamelium, NULL,
> "ScheduleHpdToggle",
> +				    "(iii)", port->id, delay_ms,
> rising_edge));
> +}
> +
>  static void async_rpc_handler(const char *server_url, const char
> *method_name,
>  			      xmlrpc_value *param_array, void
> *user_data,
>  			      xmlrpc_env *fault, xmlrpc_value
> *result)
> diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
> index 15f60246..408a4b85 100644
> --- a/lib/igt_chamelium.h
> +++ b/lib/igt_chamelium.h
> @@ -61,6 +61,9 @@ void chamelium_fire_mixed_hpd_pulses(struct
> chamelium *chamelium,
>  void chamelium_fire_hpd_pulses(struct chamelium *chamelium,
>  			       struct chamelium_port *port,
>  			       int width_msec, int count);
> +void chamelium_schedule_hpd_toggle(struct chamelium *chamelium,
> +				   struct chamelium_port *port, int
> delay_ms,
> +				   bool rising_edge);
>  void chamelium_async_hpd_pulse_start(struct chamelium *chamelium,
>  				     struct chamelium_port *port,
>  				     bool high, int delay_secs);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 1/5] lib/igt_chamelium: Add support for HPD toggle scheduling

[Paul Kocialkowski]

On Mon, 2017-06-26 at 17:07 -0400, Lyude Paul wrote:
> This all looks great! I'm happy to see us finally getting rid of the
> very hacky workaround I had to make async hotplugging work (to be fair,
> it was only partly broken ;). Thanks for dealing with chameleond
> upstream too, I couldn't stand how ugly the code in that thing is...

Sure, my pleasure :)

> Anyway, this whole patchset looks good. The only thing I ask is that
> you condense the patches a bit, mainly:
> 
> 1-3 can all be one patch for adding real async HPD
> 4-5 can be another single patch for enabling VGA tests

Okay then, I will prepare v2 that way (and rebased on top of current master).

Thanks for the review!

> On Mon, 2017-06-26 at 10:22 +0300, Paul Kocialkowski wrote:
> > This adds support for the newly-introduced ScheduleHpdToggle XMLRPC
> > method of the Chamelium's interface. It allows scheduling an HPD
> > toggle,
> > for which the call will return immediately.
> > 
> > This is especially useful for testing HPD during suspend/resume.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> > ---
> >  lib/igt_chamelium.c | 22 ++++++++++++++++++++++
> >  lib/igt_chamelium.h |  3 +++
> >  2 files changed, 25 insertions(+)
> > 
> > diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> > index 4a2af796..523cc853 100644
> > --- a/lib/igt_chamelium.c
> > +++ b/lib/igt_chamelium.c
> > @@ -447,6 +447,28 @@ void chamelium_fire_mixed_hpd_pulses(struct
> > chamelium *chamelium,
> >  	xmlrpc_DECREF(pulse_widths);
> >  }
> >  
> > +/**
> > + * chamelium_schedule_hpd_toggle:
> > + * @chamelium: The Chamelium instance to use
> > + * @port: The port to fire the HPD pulses on
> > + * @delay_ms: Delay in milli-second before the toggle takes place
> > + * @rising_edge: Whether the toggle should be a rising edge or a
> > falling edge
> > + *
> > + * Instructs the chamelium to schedule an hpd toggle (either a
> > rising edge or
> > + * a falling edge, depending on @rising_edg) after @delay_ms have
> > passed.
> > + * This is useful for testing things such as hpd after a
> > suspend/resume cycle.
> > + */
> > +void chamelium_schedule_hpd_toggle(struct chamelium *chamelium,
> > +				   struct chamelium_port *port, int
> > delay_ms,
> > +				   bool rising_edge)
> > +{
> > +	igt_debug("Scheduling HPD toggle on %s in %d ms\n", port-
> > > name,
> > 
> > +		  delay_ms);
> > +
> > +	xmlrpc_DECREF(chamelium_rpc(chamelium, NULL,
> > "ScheduleHpdToggle",
> > +				    "(iii)", port->id, delay_ms,
> > rising_edge));
> > +}
> > +
> >  static void async_rpc_handler(const char *server_url, const char
> > *method_name,
> >  			      xmlrpc_value *param_array, void
> > *user_data,
> >  			      xmlrpc_env *fault, xmlrpc_value
> > *result)
> > diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
> > index 15f60246..408a4b85 100644
> > --- a/lib/igt_chamelium.h
> > +++ b/lib/igt_chamelium.h
> > @@ -61,6 +61,9 @@ void chamelium_fire_mixed_hpd_pulses(struct
> > chamelium *chamelium,
> >  void chamelium_fire_hpd_pulses(struct chamelium *chamelium,
> >  			       struct chamelium_port *port,
> >  			       int width_msec, int count);
> > +void chamelium_schedule_hpd_toggle(struct chamelium *chamelium,
> > +				   struct chamelium_port *port, int
> > delay_ms,
> > +				   bool rising_edge);
> >  void chamelium_async_hpd_pulse_start(struct chamelium *chamelium,
> >  				     struct chamelium_port *port,
> >  				     bool high, int delay_secs);
-- 
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