Re: [PATCH i-g-t 7/7] tests/chamelium: Disconnect connectors without extra reset

[Lyude Paul <lyude@redhat.com>]

On Mon, 2017-06-26 at 16:59 +0300, Paul Kocialkowski wrote:
> This removes the reset call from the reset_state function and renames
> it
> to disconnect_connector. Since a call to reset is already done in
> chamelium_init, there is no need to do an extra reset in each test.
> 
> This allows reducing the execution time a bit.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>

NAK
It's annoying that we have to reset the connectors so much, but
unfortunately there is a reason in this case. The problem is that
chamelium_init() does not get called once per test. At first glance
when running through ./scripts/run_tests.sh this might appear to be the
case, however if you run the test by manually calling the built
chamelium binary (generally useful for debugging the tests themselves
with gdb) it won't actually get called in-between tests.

This being said, I'm all for getting rid of a couple extraneous hotplug
resets if possible, but I'd prefer to avoid breaking the ability to run
the chamelium test binary manually.

> ---
>  tests/chamelium.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/tests/chamelium.c b/tests/chamelium.c
> index 01ae4cd7..7d6893da 100644
> --- a/tests/chamelium.c
> +++ b/tests/chamelium.c
> @@ -144,17 +144,17 @@ wait_for_connector(data_t *data, struct
> chamelium_port *port,
>  }
>  
>  static void
> -reset_state(data_t *data, struct chamelium_port *port)
> +disconnect_connector(data_t *data, struct chamelium_port *port)
>  {
>  	int p;
>  
> -	chamelium_reset(data->chamelium);
> -
>  	if (port) {
> +		chamelium_unplug(data->chamelium, port);
>  		wait_for_connector(data, port,
> DRM_MODE_DISCONNECTED, false);
>  	} else {
>  		for (p = 0; p < data->port_count; p++) {
>  			port = data->ports[p];
> +			chamelium_unplug(data->chamelium, port);
>  			wait_for_connector(data, port,
> DRM_MODE_DISCONNECTED,
>  					   false);
>  		}
> @@ -167,7 +167,7 @@ test_basic_hotplug(data_t *data, struct
> chamelium_port *port, int toggle_count)
>  	struct udev_monitor *mon = igt_watch_hotplug();
>  	int i;
>  
> -	reset_state(data, NULL);
> +	disconnect_connector(data, NULL);
>  	igt_hpd_storm_set_threshold(data->drm_fd, 0);
>  
>  	for (i = 0; i < toggle_count; i++) {
> @@ -201,7 +201,7 @@ test_edid_read(data_t *data, struct
> chamelium_port *port,
>  	    data->chamelium, port, false);
>  	uint64_t edid_blob_id;
>  
> -	reset_state(data, port);
> +	disconnect_connector(data, port);
>  
>  	chamelium_port_set_edid(data->chamelium, port, edid_id);
>  	chamelium_plug(data->chamelium, port);
> @@ -270,7 +270,7 @@ test_suspend_resume_hpd(data_t *data, struct
> chamelium_port *port,
>  {
>  	struct udev_monitor *mon = igt_watch_hotplug();
>  
> -	reset_state(data, port);
> +	disconnect_connector(data, port);
>  
>  	/* Make sure we notice new connectors after resuming */
>  	try_suspend_resume_hpd(data, port, state, test, mon, false);
> @@ -294,7 +294,7 @@ test_suspend_resume_hpd_common(data_t *data, enum
> igt_suspend_state state,
>  		igt_debug("Testing port %s\n",
> chamelium_port_get_name(port));
>  	}
>  
> -	reset_state(data, NULL);
> +	disconnect_connector(data, NULL);
>  
>  	/* Make sure we notice new connectors after resuming */
>  	try_suspend_resume_hpd(data, NULL, state, test, mon, false);
> @@ -315,7 +315,7 @@ test_suspend_resume_edid_change(data_t *data,
> struct chamelium_port *port,
>  	struct udev_monitor *mon = igt_watch_hotplug();
>  	int delay = chamelium_get_suspend_resume_delay(data-
> >chamelium);
>  
> -	reset_state(data, port);
> +	disconnect_connector(data, port);
>  
>  	igt_set_autoresume_delay(delay);
>  
> @@ -585,7 +585,7 @@ test_hpd_without_ddc(data_t *data, struct
> chamelium_port *port)
>  {
>  	struct udev_monitor *mon = igt_watch_hotplug();
>  
> -	reset_state(data, port);
> +	disconnect_connector(data, port);
>  	igt_flush_hotplugs(mon);
>  
>  	/* Disable the DDC on the connector and make sure we still
> get a
> @@ -607,7 +607,7 @@ test_hpd_storm_detect(data_t *data, struct
> chamelium_port *port, int width)
>  	int count = 0;
>  
>  	igt_require_hpd_storm_ctl(data->drm_fd);
> -	reset_state(data, port);
> +	disconnect_connector(data, port);
>  
>  	igt_hpd_storm_set_threshold(data->drm_fd, 1);
>  	chamelium_fire_hpd_pulses(data->chamelium, port, width, 10);
> @@ -632,7 +632,7 @@ static void
>  test_hpd_storm_disable(data_t *data, struct chamelium_port *port,
> int width)
>  {
>  	igt_require_hpd_storm_ctl(data->drm_fd);
> -	reset_state(data, port);
> +	disconnect_connector(data, port);
>  
>  	igt_hpd_storm_set_threshold(data->drm_fd, 0);
>  	chamelium_fire_hpd_pulses(data->chamelium, port,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx